|
|
Mads Kiilerich
|
a36a8804e7be
|
5 years ago
|
|
|
|
|
Mads Kiilerich
|
4f0de9468da3
|
5 years ago
|
|
controllers: move controllers base class from lib/base to controllers
TG quickstart put it in lib/base.py , but it fits better on the controllers layer as a base there.
The contributing docs were a bit ahead of time ... but with a typo.
|
|
|
Mads Kiilerich
|
7c541dc095c8
|
5 years ago
|
|
lib: clean up pointless use of h.person on db.User just to get .username
After cleaning up, h.person will never be invoked with a db.User .
|
|
|
Mads Kiilerich
|
259213d96dca
|
5 years ago
|
|
|
|
|
Mads Kiilerich
|
6a9e5841cc51
|
5 years ago
|
|
lib: consistently import helpers the same way
Make it easier to grep for any remaining potential layering-violating use of helpers.
|
|
|
Mads Kiilerich
|
67e5b90801aa
|
5 years ago
|
|
lib: move webhelpers2 and friends to webutils
Gives less of the unfortunate use of helpers - especially in low level libs.
|
|
|
Mads Kiilerich
|
b9b53e25a08d
|
5 years ago
|
|
lib: fix bad references to utils3 A problem introduced in when rebasing 5e46f73f0d1c after renaming the temporary utils3 name to webutils.
|
|
|
Mads Kiilerich
|
5e46f73f0d1c
|
5 years ago
|
|
|
|
|
Mads Kiilerich
|
b095e2fbba44
|
5 years ago
|
|
|
|
|
Mads Kiilerich
|
0be48652ca48
|
5 years ago
|
|
routing: separate url handling from routing - move it to webutils
This is a helper method relying on the thread local tg.request. We didn't have a good place to put it. Now we do.
This (re)moves unfortunate dependencies to the routing module (which almost is a controller).
|
|
|
Mads Kiilerich
|
3ccb302bb551
|
5 years ago
|
|
|
|
|
Mads Kiilerich
|
5725fa4cfecd
|
6 years ago
|
|
cleanup: minimize use of lambda expressions - we have 'def' for that purpose
Fix some flake8 warnings "E731 do not assign a lambda expression, use a def".
|
|
|
Mads Kiilerich
|
b6b6955981a5
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
08eec03c9485
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
67eeb02062fe
|
6 years ago
|
|
controllers: use unicode for internal rendering of template snippets Like 8e2313be042d.
|
|
|
Mads Kiilerich
|
ed78b4fbe2a3
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
394c7814e710
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
fe4086096758
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
0a277465fddf
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
93834966ae01
|
7 years ago
|
|
auth: global permissions given to the default user are the bare minimum and should apply to *all* other users too
Drop the "subtractive permission" config option "inherit_from_default" that when set to false would give users less global permissions than the default unauthenticated user.
Instead, think positive and merge all positive permissions.
At the end, filter the global permissions to make sure we for each kind of permissions only keep the one with most weight.
|
|
|
Thomas De Schampheleire
|
7aff9a999536
|
7 years ago
|
|
templates, controllers: replace webhelpers.html.literal() with webhelpers.html.HTML() where possible Usage of webhelpers.literal (h.literal) can be a problem when variables are not correctly escaped. Luckily, this function can be avoided in several cases. Several users of the construct: h.literal(_('..A..') % (..B..)) can be simplified if (..B..) just contains a call to h.link_to. In this case, there is actually no need to use h.literal, because the object returned by link_to is already a literal. It is sufficient to use webhelpers.html.HTML() like so: h.HTML(_('..A..')) % (..B..) which is better because it will escape the '..A..' part instead of passing it literally. The need to wrap the '..A..' part in HTML() is to make sure the (escaped) end result is not a plain string but a 'literal' to avoid double escaping later. See also the documentation: https://docs.pylonsproject.org/projects/webhelpers/en/latest/modules/html/builder.html" When literal is used in a mixed expression containing both literals and ordinary strings, it tries hard to escape the strings and return a literal. However, this depends on which value has “control” of the expression. literal seems to be able to take control with all combinations of the + operator, but with % and join it must be on the left side of the expression. So these all work: "A" + literal("B") literal(", ").join(["A", literal("B")]) literal("%s %s") % (16, literal("kg")) But these return an ordinary string which is prone to double-escaping later: "\n".join([literal('<span class="foo">Foo!</span>'), literal('Bar!')]) "%s %s" % (literal("16"), literal("<em>kg</em>")) " This same escaping with 'HTML()' was already done by default in mako templates for constructs like ${_("something")} that do not contain format specifiers. When the translated string _does_ contain format specifiers, we want to use the same escaping, but we have to do it explicit and earlier so the escaping happens already when strings are inserted into the template string.
|
|
|
Mads Kiilerich
|
aa25ef34ebab
|
8 years ago
|
|
auth: refactor to introduce @LoginRequired(allow_default_user=True) and deprecate @NotAnonymous() It was error prone that @LoginRequired defaulted to allow anonymous users (if 'default' user is enabled). See also 245b4e3abf39. Refactor code to make it more explicit and safe by default: Deprecate @NotAnonymous by making it the default of @LoginRequired. That will make it safe by default. To preserve same functionality, set allow_default_user=True in all the cases where @LoginRequired was *not* followed by @NotAnonymous or other permission checks - that was done with some script hacks: sed -i 's/ @LoginRequired(\(..*\))/ @LoginRequired(\1, allow_default_user=True)/g' `hg mani` sed -i 's/ @LoginRequired()/ @LoginRequired(allow_default_user=True)/g' `hg mani` perl -0pi -e 's/\ @LoginRequired\(allow_default_user=True\)\n\s*\ @NotAnonymous\(\)/\ @LoginRequired()/g' `hg mani` perl -0pi -e 's/\ @LoginRequired\(allow_default_user=True\)(\n\s*\ @Has(Repo)?Permission)/\ @LoginRequired()\1/g' `hg mani` It has been reviewed that all uses of allow_default_user=True are in places where the there indeed wasn't any checking for default user before. These may or may not be correct, but now they are explicit and can be spotted and fixed. The few remaining uses of @NotAnonymous should probably be removed somehow.
|
|
|
domruf
|
205daed7185b
|
8 years ago
|
|
users: remove code that is unused after most autocomplete has been switched to ajax
@mention support still require _USERS_AC_DATA as a global variable.
|
|
|
Mads Kiilerich
|
e81332582fc9
|
8 years ago
|
|
controllers: drop unused totalRecords and startIndex
We don't want it just for testing.
|
|
|
Lars Kruse
|
7691290837d2
|
8 years ago
|
|
codingstyle: trivial whitespace fixes
Reported by flake8.
|
|
|
Alessandro Molina
|
e1ab82613133
|
9 years ago
|
|
backend: replace Pylons with TurboGears2 Replace the no-longer-supported Pylons application framework by TurboGears2 which is largely compatible/similar to Pylons. Some interesting history is described at: https://en.wikipedia.org/wiki/TurboGearsChanges by Dominik Ruf: - fix sql config in test.ini Changes by Thomas De Schampheleire: - set-up of test suite - tests: 'fix' repo archival test failure Between Pylons and TurboGears2, there seems to be a small difference in the headers sent for repository archive files, related to character encoding. It is assumed that this difference is not important, and that the test should just align with reality. - remove need to import helpers/app_globals in lib TurboGears2 by default expects helpers and app_globals to be available in lib. For this reason kallithea/lib/__init__.py was originally changed to include those files. However, this triggered several types of circular import problems. If module A imported something from lib (e.g. lib.annotate), and lib.helpers imported (possibly indirectly) module A, then there was a circular import. Fix this by overruling the relevant method of tg AppConfig, which is also hinted in the TurboGears2 code. Hereby, the include of something from lib does not automatically import helpers, greatly reducing the chances of circular import problems. - make sure HTTP error '400' uses the custom error pages TurboGears2 does not by default handle HTTP status code '400 (Bad Request)' via the custom error page handling, causing a standard non-styled error page. - disable transaction manager Kallithea currently handles its own transactions and does not need the TurboGears2 transaction manager. However, TurboGears2 tries to enable it by default and fails, throwing an error during application initialization. The error itself seemed to be harmless for normal application functioning, but was nevertheless confusing. - add backlash as required dependency: backlash is meant as the WebError replacement in TurboGears2 (originally WebError is part of Pylons). When debug==true, it provides an interactive debugger in the browser. When debug==false, backlash is necessary to show backtraces on the console. - misc fixes
|
|
|
Thomas De Schampheleire
|
4517e212f09a
|
9 years ago
|
|
controllers: rename __before__ to _before in preparation of TurboGears2
__before__ in Pylons is called _before in TurboGears2. We can prepare this rename already in Pylons-based Kallithea, so that the real TG2 migration commit just changes the BaseController.
Since TurboGears2 _before can pass extra arguments, we add *args and **kwargs parameters as well.
|
|
|
Mads Kiilerich
|
e9ac5698281d
|
9 years ago
|
|
tg: minimize future diff by some mocking and replacing some pylons imports with tg
No actual tg dependency yet, just a temporary hack faking tg as an alias for pylons.
Based on work by Alessandro Molina.
|
|
|
Søren Løvborg
|
33b71a130b16
|
9 years ago
|
|
templates: properly escape inline JavaScript values
TLDR: Kallithea has issues with escaping values for use in inline JS. Despite judicious poking of the code, no actual security vulnerabilities have been found, just lots of corner-case bugs. This patch fixes those, and hardens the code against actual security issues.
The long version:
To embed a Python value (typically a 'unicode' plain-text value) in a larger file, it must be escaped in a context specific manner. Example:
>>> s = u'<script>alert("It\'s a trap!");</script>'
1) Escaped for insertion into HTML element context
>>> print cgi.escape(s) <script>alert("It's a trap!");</script>
2) Escaped for insertion into HTML element or attribute context
>>> print h.escape(s) <script>alert("It's a trap!");</script>
This is the default Mako escaping, as usually used by Kallithea.
3) Encoded as JSON
>>> print json.dumps(s) "<script>alert(\"It's a trap!\");</script>"
4) Escaped for insertion into a JavaScript file
>>> print '(' + json.dumps(s) + ')' ("<script>alert(\"It's a trap!\");</script>")
The parentheses are not actually required for strings, but may be needed to avoid syntax errors if the value is a number or dict (object).
5) Escaped for insertion into a HTML inline <script> element
>>> print h.js(s) ("\x3cscript\x3ealert(\"It's a trap!\");\x3c/script\x3e")
Here, we need to combine JS and HTML escaping, further complicated by the fact that "<script>" tag contents can either be parsed in XHTML mode (in which case '<', '>' and '&' must additionally be XML escaped) or HTML mode (in which case '</script>' must be escaped, but not using HTML escaping, which is not available in HTML "<script>" tags). Therefore, the XML special characters (which can only occur in string literals) are escaped using JavaScript string literal escape sequences.
(This, incidentally, is why modern web security best practices ban all use of inline JavaScript...)
Unsurprisingly, Kallithea does not do (5) correctly. In most cases, Kallithea might slap a pair of single quotes around the HTML escaped Python value. A typical benign example:
$('#child_link').html('${_('No revisions')}');
This works in English, but if a localized version of the string contains an apostrophe, the result will be broken JavaScript. In the more severe cases, where the text is user controllable, it leaves the door open to injections. In this example, the script inserts the string as HTML, so Mako's implicit HTML escaping makes sense; but in many other cases, HTML escaping is actually an error, because the value is not used by the script in an HTML context.
The good news is that the HTML escaping thwarts attempts at XSS, since it's impossible to inject syntactically valid JavaScript of any useful complexity. It does allow JavaScript errors and gibberish to appear on the page, though.
In these cases, the escaping has been fixed to use either the new 'h.js' helper, which does JavaScript escaping (but not HTML escaping), OR the new 'h.jshtml' helper (which does both), in those cases where it was unclear if the value might be used (by the script) in an HTML context. Some of these can probably be "relaxed" from h.jshtml to h.js later, but for now, using h.jshtml fixes escaping and doesn't introduce new errors.
In a few places, Kallithea JSON encodes values in the controller, then inserts the JSON (without any further escaping) into <script> tags. This is also wrong, and carries actual risk of XSS vulnerabilities. However, in all cases, security vulnerabilities were narrowly avoided due to other filtering in Kallithea. (E.g. many special characters are banned from appearing in usernames.) In these cases, the escaping has been fixed and moved to the template, making it immediately visible that proper escaping has been performed.
Mini-FAQ (frequently anticipated questions):
Q: Why do everything in one big, hard to review patch? Q: Why add escaping in specific case FOO, it doesn't seem needed?
Because the goal here is to have "escape everywhere" as the default policy, rather than identifying individual bugs and fixing them one by one by adding escaping where needed. As such, this patch surely introduces a lot of needless escaping. This is no different from how Mako/Pylons HTML escape everything by default, even when not needed: it's errs on the side of needless work, to prevent erring on the side of skipping required (and security critical) work.
As for reviewability, the most important thing to notice is not where escaping has been introduced, but any places where it might have been missed (or where h.jshtml is needed, but h.js is used).
Q: The added escaping is kinda verbose/ugly.
That is not a question, but yes, I agree. Hopefully it'll encourage us to move away from inline JavaScript altogether. That's a significantly larger job, though; with luck this patch will keep us safe and secure until such a time as we can implement the real fix.
Q: Why not use Mako filter syntax ("${val|h.js}")?
Because of long-standing Mako bug #140, preventing use of 'h' in filters.
Q: Why not work around bug #140, or even use straight "${val|js}"?
Because Mako still applies the default h.escape filter before the explicitly specified filters.
Q: Where do we go from here?
Longer term, we should stop doing variable expansions in script blocks, and instead pass data to JS via e.g. data attributes, or asynchronously using AJAX calls. Once we've done that, we can remove inline JavaScript altogether in favor of separate script files, and set a strict Content Security Policy explicitly blocking inline scripting, and thus also the most common kind of cross-site scripting attack.
|
|
|
Søren Løvborg
|
10f16cf8289e
|
9 years ago
|
|
cleanup: remove SQLAlchemy session argument to action_logger
There's always a global SQLAlchemy session associated with the current thread; using another session for a single function call does not make any sense (as sessions cannot be mixed), unless the code works carefully to ensure the two sessions (and all objects loaded from them) are kept completely separate. Suffice to say that Kallithea does no such thing, thus there's no need to pretend to support multiple concurrent sessions.
|
|
|
Søren Løvborg
|
ca77c6da2d34
|
9 years ago
|
|
auth: simplify user group permission checks
In practice, Kallithea has the 'usergroup.admin' permission imply the 'usergroup.write' permission, which again implies 'usergroup.read'.
This codifies this practice by replacing the HasUserGroupPermissionAny "perm function" with the new HasUserGroupLevel function, reducing the risk of errors and saving quite a lot of typing.
|
|
|
Mads Kiilerich
|
3dcf1f82311a
|
9 years ago
|
|
controllers: avoid setting request state in controller instances - set it in the thread global request variable
In TurboGears, controllers are singletons and we should avoid using instance variables for any volatile data. Instead, use the "global thread local" request context.
With everything in request, some use of c is dropped.
Note: kallithea/controllers/api/__init__.py still use instance variables that will cause problems with TurboGears.
|
|
|
Søren Løvborg
|
cd6176c0634a
|
9 years ago
|
|
db: PullRequest/Repository/RepoGroup/UserGroup: change 'user' to 'owner'
Rename the 'user' and 'user_id' fields on the four classes to something more informative. The database column names remain unchanged for now; a later Alembic script can fix the name of these and other columns to match their Python name.
This might break rcextensions, though, and external scripts that use the HTML form interface.
|
|
|
Søren Løvborg
|
4136526cce20
|
9 years ago
|
|
db: remove superfluous Session.add calls
Don't re-add objects to the SQLAlchemy Session just because they were modified. Session.add is only for freshly constructed objects that SQLAlchemy doesn't know about yet.
The rules are quite simple:
When creating a database object by calling the constructor directly, it must explicitly be added to the session.
When creating an object using a factory function (like "create_repo"), the returned object has already (by convention) been added to the session, and should not be added again.
When getting an object from the session (via Session.query or any of the utility functions that look up objects in the database), it's already added, and should not be added again. SQLAlchemy notices attribute modifications automatically for all objects it knows about.
|
|
|
Thomas De Schampheleire
|
af3539a458f6
|
9 years ago
|
|
Turbogears2 migration: replace pylons.url by kallithea.config.routing.url
In preparation for the migration to Turbogears2, introduce a kallithea.config.routing.url to replace pylons.url. The implementation is basically the same: wrap around routes.url().
This change involves: - a number of import statement changes - fixing some tests in test_libs.py; to avoid duplication, the different implementations of fake_url were grouped in one place.
This change was first proposed by Alessandro Molina in his initial port. Following changes were made afterwards: - move UrlGenerator from kallithea.lib.utils to kallithea.config.routing - add documentation to UrlGenerator - kallithea/lib/auth.py used url_for instead of url, for no apparent reason so this was changed. - fix libs tests - rebase onto Pylons-based Kallithea first
|
|
|
Thomas De Schampheleire
|
5eec79420ce3
|
9 years ago
|
|
Turbogears2 migration: remove some references to Pylons in comments
In order to minimize the diff of the actual Turbogears2 migration, this commit already removes certain unnecessary references to Pylons from the Kallithea source base. Places where the reference to Pylons is important are still kept for now, as well as references in kallithea/config where many changes are made for Turbogears2 anyway.
|
|
|
Mads Kiilerich
|
1522c37cbc62
|
9 years ago
|
|
|
|
|
Mads Kiilerich
|
4ebce2642888
|
9 years ago
|
|
|
|
|
Mads Kiilerich
|
c0a3519e7d2c
|
9 years ago
|
|
controllers: remove pointless comments
These comments are mostly trivial and sometimes wrong. We don't want to maintain or pretend we do.
|
|
|
Mads Kiilerich
|
edb24bc0f71a
|
10 years ago
|
|
|
|
|
Søren Løvborg
|
d9b78d8f1db3
|
10 years ago
|
|
cleanup: replace redirect with WebOb exceptions
All redirect does is to log "Generating 302 redirect" with logging the actual location and raise a WebOb HTTPFound exception, and the logging is redundant, as WebOb exceptions and their status codes are already logged.
Instead, just raise the exception directly, which is both explicit and simpler (and finally, gets rid of "return redirect" which never really returns).
|
|
|
Søren Løvborg
|
b361974171ea
|
10 years ago
|
|
spelling: fix inappropriate use of title case
These are not titles or labels, and should not be title cased.
|
|
|
Mads Kiilerich
|
7d0727d11104
|
10 years ago
|
|
cleanup: remove unused imports
Found with pyflakes.
|
|
|
Mads Kiilerich
|
0ad053c172fa
|
10 years ago
|
|
cleanup: make module self-naming consistent
The self naming is quite pointless ... but at least make it right.
|
|
|
Mads Kiilerich
|
d69aa464f373
|
10 years ago
|
|
cleanup: consistently use 'except ... as ...:'
Use the Python 2.6+ syntax instead of the old confusing 'except ..., ...' syntax.
|
|
|
Nick High
|
a8f2986afc18
|
11 years ago
|
|
security: Fix HTML and JavaScript injection.
This fixes CVE-2015-1864
|
|
|
Mads Kiilerich
|
c04c2734e32f
|
11 years ago
|
|
controllers: consistently use formfill.render with force_defaults=False
The inconsistency could cause confusion for developers. It seems to me like force_defaults=False should be the default ... and apparently it was that in older versions of formfill.
It could perhaps make sense for us to have a wrapper that added the defualt values once, instead of repeating it all over ;-)
|
|
|
Mads Kiilerich
|
8ec971873d9b
|
11 years ago
|
|
|
|
|
Mads Kiilerich
|
d51a6f5e57d1
|
11 years ago
|
|
|
|
|
Bradley M. Kuhn
|
1948ede028ef
|
11 years ago
|
|
|
|
|
Bradley M. Kuhn
|
ad38f9f93b3b
|
11 years ago
|
|
Correct licensing information in individual files.
The top-level license file is now LICENSE.md.
Also, in various places where there should have been joint copyright holders listed, a single copyright holder was listed. It does not appear easy to add a link to a large list of copyright holders in these places, so it simply refers to the fact that various authors hold copyright.
In future, if an easy method is discovered to link to a list from those places, we should do so.
Finally, text is added to LICENSE.md to point to where the full list of copyright holders is, and that Kallithea as a whole is GPLv3'd.
|
|
|
Bradley M. Kuhn
|
d208416c84c6
|
11 years ago
|
|
|
|
|
Bradley M. Kuhn
|
d1addaf7a91e
|
11 years ago
|
|
Second step in two-part process to rename directories. This is the actual directory rename.
|