|
|
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
|
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
|
1ecd6c0e2787
|
5 years ago
|
|
auth: refactor permissions
Avoid using complex vague typing in dict-of-dicts.
|
|
|
Mads Kiilerich
|
c59e914c4887
|
6 years ago
|
|
py3: use exception .args instead of .message
Args seems slightly more fragile and *could* introduce problems for trivial use if args is empty. But .message is gone.
|
|
|
Mads Kiilerich
|
3cab6bc45cc3
|
6 years ago
|
|
ssh: use fingerprint when deleting public keys
Avoid relying on a database index of the full public key string.
|
|
|
Mads Kiilerich
|
fe4086096758
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
0a277465fddf
|
6 years ago
|
|
|
|
|
Christian Oyarzun
|
955bbbc29655
|
11 years ago
|
|
|
|
|
Christian Oyarzun
|
3b147c38b674
|
11 years ago
|
|
|
|
|
Tim Freund
|
66c208bf56fe
|
11 years ago
|
|
ssh: user management of ssh keys
Add user interface for managing SSH based access.
The work in this commit is based heavily off of the existing API key code for the sake of consistency.
Updates to use Bootstrap, request.authuser, POST methods and pytest by Anton Schur <tonich.sh@gmail.com>.
Additional Bootstrap fixes by Dominik Ruf.
The original code has been heavily modified by Mads Kiilerich.
|
|
|
Mads Kiilerich
|
2e3e1dacdbb7
|
7 years ago
|
|
auth: drop confusing and layering-violating User.AuthUser property
Keep it simple and just explicitly create an AuthUser if "needed". That can perhaps be simplified further later on.
|
|
|
Mads Kiilerich
|
af938280e76a
|
8 years ago
|
|
|
|
|
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.
|
|
|
Lars Kruse
|
7691290837d2
|
8 years ago
|
|
codingstyle: trivial whitespace fixes
Reported by flake8.
|
|
|
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
|
eea19c23b741
|
9 years ago
|
|
cleanup: refer less to User.DEFAULT_USER
Down the road we might want to identify the default user in another way than by username.
|
|
|
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
|
90af0cdf02d8
|
9 years ago
|
|
cleanup: don't check if currently logged-in user exists
Don't check if the currently logged-in User exists. Every request runs in a database transaction; if the object existed at the start, it still exists (unless we add a way for a user to delete themselves).
|
|
|
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.
|
|
|
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
|
|
|
Mads Kiilerich
|
12bc5b6057a7
|
9 years ago
|
|
auth: cleanup of EXTERN_TYPE_INTERNAL
Don't set it in top level namespace - it is a weak link between the database and the actual implementation. Don't make it more than that.
Don't hardcode in that many places that 'internal' is the default - just call it DEFAULT_AUTH_TYPE.
Don't use it for extern_name - it is only intended for use as extern_type.
Remove unused uses.
|
|
|
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
|
69750b518137
|
9 years ago
|
|
|
|
|
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).
|
|
|
Mads Kiilerich
|
7d0727d11104
|
10 years ago
|
|
cleanup: remove unused imports
Found with pyflakes.
|
|
|
Mads Kiilerich
|
d69aa464f373
|
10 years ago
|
|
cleanup: consistently use 'except ... as ...:'
Use the Python 2.6+ syntax instead of the old confusing 'except ..., ...' syntax.
|
|
|
Mads Kiilerich
|
de9a3152c206
|
10 years ago
|
|
|
|
|
Mads Kiilerich
|
0a0595b15c6c
|
10 years ago
|
|
auth: make sure that users only can manage their own primary data if self registration is enabled
With the UI showing exactly which fields are used and which are ignored, there is no reason to show the 'External Source of Record' warning.
|
|
|
Mads Kiilerich
|
39bac9410169
|
10 years ago
|
|
|
|
|
Mads Kiilerich
|
e50a56e61b23
|
10 years ago
|
|
users: avoid unnecessary handling of c.extern_type and c.extern_name when editing users
It is user data as anything else.
|
|
|
Mads Kiilerich
|
63bed817308c
|
10 years ago
|
|
cleanup: check for None object identity in cases where that is what the 'contract' says
Avoid applying bool() on complex objects - it might return something unexpected such as the key (which might be 0 and thus be false). Checking for None is more safe and faster.
|
|
|
Mads Kiilerich
|
148360f533a4
|
10 years ago
|
|
|
|
|
Mads Kiilerich
|
9a02f9ef28d7
|
10 years ago
|
|
utils: make API key generator more random
The API key generator abused temporary filenames in what seems to be an attempt of creating keys that unambiguously specified the user and thus were unique across users. A final hashing did however remove that property.
More importantly, tempfile is not documented to use secure random numbers ... and it only uses 6 characters, giving approximately 36 bits of entropy.
Instead, use the cryptographically secure os.urandom directly to generate keys with the same length but with the full 160 bits of entropy.
Reported and fixed by Andrew Bartlett.
|
|
|
Søren Løvborg
|
4a2a66bf93c5
|
10 years ago
|
|
AuthUser: Drop ip_addr field
None of the AuthUser consumers actually need to get the IP address from the AuthUser object, so it's just redundant.
Also, AuthUser represents a user session, and should not be used as a generic user + IP address data structure.
|
|
|
Mads Kiilerich
|
86b1f3cfe836
|
11 years ago
|
|
spelling: fix title casing on various translated strings
Primarily captions on other not-just-text and data.
|
|
|
Mads Kiilerich
|
e3aab61a9411
|
11 years ago
|
|
|
|
|
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
|
395be5fa6eef
|
11 years ago
|
|
|
|
|
Mads Kiilerich
|
d51a6f5e57d1
|
11 years ago
|
|
|
|
|
Mads Kiilerich
|
99997d8f31eb
|
12 years ago
|
|
|
|
|
Bradley M. Kuhn
|
f5c9018a5cf0
|
11 years ago
|
|
|
|
|
Bradley M. Kuhn
|
24c0d584ba86
|
11 years ago
|
|
|
|
|
Bradley M. Kuhn
|
1948ede028ef
|
11 years ago
|
|
|
|
|
Bradley M. Kuhn
|
de26de99ac5b
|
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.
|