|
|
mads
|
aafca212c8e2
|
5 years ago
|
|
celery: move send_email task to a better home in notification model
Avoid bundling everything from many different layers in one big task library.
This is more feasible now when we don't need kallithea.CELERY_APP set at import time.
|
|
|
mads
|
341e4bb9e227
|
5 years ago
|
|
|
|
|
mads
|
710512deb83d
|
5 years ago
|
|
|
|
|
mads
|
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
|
5e46f73f0d1c
|
5 years ago
|
|
|
|
|
mads
|
b095e2fbba44
|
5 years ago
|
|
|
|
|
mads
|
fdde16d7cea0
|
6 years ago
|
|
celery: fix send_email to work with JSON encoding (Issue #363) Long time ago, c935bcaf7086 introduced an optional User object parameter to the send_email task and used the computed full_name_or_username property. Due to the magic of pickle, that also worked when using Celery to run the task async. Now, Celery 4 changed the default encoding from Pickle to JSON, which we anticipated in e539db6cc0da. That broke send_email in some cases, for example when a user comments on another user's changeset. Fixed by passing the "From" name as string instead of passing the whole User object. Thanks to vyom for reporting.
|
|
|
mads
|
e63bcce18fef
|
6 years ago
|
|
|
|
|
mads
|
7172f3b0042b
|
6 years ago
|
|
|
|
|
mads
|
7e9d3865b4f9
|
6 years ago
|
|
py3: support new stdlib module names configparser, urllib and http lib
From "2to3 -f imports".
|
|
|
Thomas De Schampheleire
|
24db2cd42881
|
6 years ago
|
|
|
|
|
Thomas De Schampheleire
|
cec84c4675ce
|
6 years ago
|
|
tests: remove race condition in test_forgot_password One in so many times, test_forgot_password failed with: kallithea/tests/functional/test_login.py:427: in test_forgot_password assert '\n%s\n' % token in body E assert ('\n%s\n' % ' d71ad3ed3c6ca637ad00b7098828d33c56579201') in "Password Reset Request\n\nHello passwd reset,\n\nWe have received a request to reset the password for your account.\n\nTo s... 7e89326ca372ade1d424dafb106d824cddb\n\nIf it weren't you who requested the password reset, just disregard this message.\n" i.e. the expected token is not the one in the email. The token is calculated based on a timestamp (among others). And the token is calculated twice: once in the real code and once in the test, each time on a slightly different timestamp. Even though there is flooring of the timestamp to a second resolution, there will always be a race condition where the two timestamps floor to a different second, e.g. 4.99 vs 5.01. The problem can be reproduced reliably by adding a sleep of e.g. 2 seconds before generating the password reset mail (after the test has already calculated the expected token). Solve this problem by mocking the time.time() used to generate the timestamp, so that the timestamp used for the real token is the same as the one used for the expected token in the test.
|
|
|
mads
|
8f468d08f463
|
6 years ago
|
|
|
|
|
mads
|
8b47181750a8
|
6 years ago
|
|
login: fix incorrect CSRF rejection of "Reset Your Password" form (Issue #350) htmlfill would remove the CSRF token from the form when substituting the query parameters, causing password reset to break. By default, htmlfill will clear all input fields that doesn't have a new "default" value provided. It could be fixed by setting force_defaults to False - see http://www.formencode.org/en/1.2-branch/modules/htmlfill.html . It could also be fixed by providing the CSRF token in the defaults to be substituted in the form. Instead, refactor password_reset_confirmation to have more explicitly safe handling of query parameters. Replace htmlfill with the usual template variables. The URLs are generated in kallithea/model/user.py send_reset_password_email() and should only contain email, timestamp (integer as digit string) and a hex token from get_reset_password_token() .
|
|
|
mads
|
bbbfee57fb96
|
6 years ago
|
|
|
|
|
mads
|
e527cc2ce8dc
|
6 years ago
|
|
cleanup: get rid of most "import *"
Apply script generated with the following hack: ( hg loc '*.py'|xargs pyflakes-2 | sed -rn "s/([^:]*):.*'(.*)' may be undefined, or defined from star imports.*/sed -ri 's,\\\\<\2\\\\>([^=]|$),XXXX.\2\\\\1,g' \1/gp" | sort -u hg loc '*.py'|xargs pyflakes-2 | sed -rn "s/([^:]*):.* undefined name '(.*)'$/sed -ri 's,\\\\<\2\\\\>([^=]|$),XXXX.\2\\\\1,g' \1/gp" | sort -u hg loc '*.py'|xargs pyflakes-2 | sed -rn "s/([^:]*):.*'(from .*)\.([^.]*) import \*' used.*/sed -ri 's,\\\\<XXXX\\\\.,\3.,g' \1/gp" | sort -u hg loc '*.py'|xargs pyflakes-2 | sed -rn "s/([^:]*):.*'(from .*)\.([^.]*) import \*' used.*/sed -ri 's,\2\\\\.\3 .*,\2 import \3,g' \1/gp" | sort -u ) | grep -v kallithea/bin/kallithea_cli_ishell.py > fix2.sh
|
|
|
mads
|
43ee4d4d68f2
|
6 years ago
|
|
tests: refactor test_redirection_to_login_form_preserves_get_args to test more correctly
Preparing for py3.
|
|
|
mads
|
fe4086096758
|
7 years ago
|
|
|
|
|
mads
|
0a277465fddf
|
7 years ago
|
|
|
|
|
mads
|
09100b3b8f42
|
7 years ago
|
|
|
|
|
mads
|
9c2fc1390291
|
7 years ago
|
|
tests: prepare for adding CSRF protection on login forms
CSRF is about avoiding abuse of credentials by doing things in existing sessions. The login form does not have any previous credentials, so there is nothing to abuse and no real need for CSRF protection. But there is still an unauth session, so we *can* have CSRF protection.
CSRF protection is currently in LoginRequired (which obviously isn't applied to the login form), but let's prepare for changing that.
|
|
|
mads
|
0e3e0864f210
|
7 years ago
|
|
auth: drop api_access_controllers_whitelist and give API key auth same access as other kinds of auth
All authentication methods are created equal. There is no point in discriminating api key authentication and limit it to few APIs.
|
|
|
Thomas De Schampheleire
|
a33d1337db05
|
7 years ago
|
|
tests: remove tests of UI notifications
This commit is part of the removal of the UI notification feature from Kallithea, which is not deemed useful in its current form. Only email notifications are preserved.
|
|
|
mads
|
2a96678c8cd9
|
8 years ago
|
|
|
|
|
mads
|
fefd7279e798
|
8 years ago
|
|
login: fix crash when entering non-ASCII password for login (Issue #300) Avoid errors like UnicodeEncodeError: 'ascii' codec can't encode characters in position X: ordinal not in range(128) when the user enters non-ASCII passwords for existing internal accounts in the login prompt. The password forms have "always" rejected non-ASCII passwords with Invalid characters (non-ASCII) in password
|
|
|
Lars Kruse
|
48a00daba2f2
|
9 years ago
|
|
codingstyle: replace comparison for equality against None with "is" expression
Both style and correctness.
Reported by flake8.
|
|
|
Lars Kruse
|
7691290837d2
|
9 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
|
9f8a1212177e
|
9 years ago
|
|
tests: use test_context for tests needing internationalization (bis) Commit 8e3137064ab6 already introduced the use of test_context to cover internationalization in the test suite, instead of setting it up globally. When making changes related to formencode internationalization, a new batch of internationalization errors popped up and a commit was made to fix them. However, after some later refactoring, it looked as if the commit was not needed anymore. In Turbogears context, it was indeed not necessary as long as we still had some places that used the dummy formencode.api._ rather than a real version of _ (ugettext). After cleaning up that forgotten import, the test internationalization errors popped up again. Hence, we need to reapply the earlier commit (with some changes).
|
|
|
Søren Løvborg
|
9cf90371d0f1
|
9 years ago
|
|
auth: add support for "Bearer" auth scheme (API key variant)
This allows the API key to be passed in a header instead of the query string, reducing the risk of accidental API key leaks:
Authorization: Bearer <api key>
The Bearer authorization scheme is standardized in RFC 6750, though used here outside the full OAuth 2.0 authorization framework. (Full OAuth can still be added later without breaking existing users.)
|
|
|
Søren Løvborg
|
06398585de03
|
9 years ago
|
|
auth: track API key used for authentication in AuthUser
This allows us to define only once how an API key is passed to the app. We might e.g. allow API keys to be passed in an HTTP header; with this change, we only need to update the code in one place.
Also change the code to verify up front that the API key resolved to a valid and active user, so LoginRequired doesn't need to do that.
Also return plain 403 Forbidden for bad API keys instead of redirecting to the login form, which makes more sense for non-interactive clients (the typical users of API keys).
|
|
|
Søren Løvborg
|
aa2542a6538b
|
9 years ago
|
|
tests: fix incorrect API key tests
These parameterized tests were supposed to (among other things) test what happens when the "api_key" query string parameter is not present. Instead, they literally tested "api_key=None".
|
|
|
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
|
8d98924c58b1
|
9 years ago
|
|
tests: add as little code as possible in __init__.py
kallithea/tests/__init__.py contained quite a lot of code, including the test base class TestController. This in itself may be considered bad practice.
Specifically, this poses a problem when using pytest 3.0+, in which asserts in some files are not automatically rewritten to give improved assert output. That problem can be fixed by explicitly registering such files for assertion rewriting, but that register call should be executed _before_ said files are imported. I.e. if the register call is in kallithea/tests/__init__.py, assert calls in __init__.py itself can not be rewritten.
Since the TestController base class does effectively contain asserts, and we do not want to execute the register call from somewhere outside the kallithea/tests directory, we need to move the TestController class to another file (kallithea/tests/base.py) so we can have a register call in __init__.py before loading base.py.
While not strictly necessary to fix the mentioned pytest problem, we take the opportunity to fully clean __init__.py and move everything to the new kallithea/tests/base.py. While doing so, unnecessary imports are removed, and imports are ordered alphabetically. Explicit imports of symbols from modules that were already imported as a whole, are removed in favor of fully qualifying the references (e.g. tempfile._RandomNameSequence).
|
|
|
Søren Løvborg
|
38e418408c58
|
10 years ago
|
|
|
|
|
Thomas De Schampheleire
|
7f2aa3ec2931
|
10 years ago
|
|
pytest migration: rename TestControllerPytest back to TestController
The name TestControllerPytest was introduced to allow a temporary situation where nose/unittest and pytest-based tests could coexist. This situation is now over, so the base test class can be renamed again.
|
|
|
Thomas De Schampheleire
|
fed129fb8533
|
10 years ago
|
|
pytest migration: backout declassification of remove_all_notifications In order to accomodate both nose/unittest and pytest at the same time, method remove_all_notifications has been extracted from BaseTestCase in commit 37d713674f63. Now that nose/unittest is no longer used, we can backout this change again.
|
|
|
Thomas De Schampheleire
|
be1d366f461c
|
10 years ago
|
|
pytest migration: functional: switch to standard assert statements
Use unittest2pytest to replace unittest-style assert statements (e.g. assertEqual) with standard Python assert statements to benefit from pytest's improved reporting on assert failures.
The conversion by unittest2pytest was correct, except for line wrapping problems.
|
|
|
timeless@gmail.com
|
1048307eb1f5
|
10 years ago
|
|
|
|
|
Thomas De Schampheleire
|
72e8508d9758
|
10 years ago
|
|
|
|
|
Thomas De Schampheleire
|
37d713674f63
|
10 years ago
|
|
tests: move remove_all_notifications outside of BaseTestCase
In preparation of allowing real pytest-style test cases (instead of unittest-style ones), some reorganization is needed in the base test classes, for one because we want a transition period where pytest and unittest style test cases can live alongside each other, and secondly because the pytest style test classes cannot have an __init__ method.
The BaseTestCase class will not be reused for the pytest test cases, but the remove_all_notifications method will. To avoid having to duplicate it, and since it does not use any resources from the class (self), move the method out of the BaseTestCase class to top-level, and export it in kallithea.tests.
|
|
|
Andrew Shadura
|
b24e015a4174
|
10 years ago
|
|
auth: allow web login with email addresses
Let users log in using their email addresses instead of their user names. This only applies to the web login, not git+http or hg+http protocols.
|
|
|
Mads Kiilerich
|
bd4840ad72d3
|
10 years ago
|
|
tests: more consistently use unicode where unicode is expected
Nothing but extra u annotation to turn str constants into unicode.
This has been verified by hacking sqlalchemy to fail if wrong string types are passed.
|
|
|
Søren Løvborg
|
38d1c99cd000
|
10 years ago
|
|
login: enhance came_from validation
Drop urlparse and just validate that came_from is a RFC 3986 compliant path.
This blocks an HTTP header injection vulnerability discovered by Gjoko Krstic <gjoko@zeroscience.mk> of Zero Science Lab (CVE-2015-5285)
|
|
|
Søren Løvborg
|
b537babcf966
|
10 years ago
|
|
login: include query parameters in came_from
The login controller uses the came_from query argument to determine the page to continue to after login.
Previously, came_from specified only the URL path (obtained using h.url.current), and any URL query parameters were passed along as separate (additional) URL query parameters; to obtain the final redirect target, h.url was used to combine came_from with the request.GET.
As of this changeset, came_from specifies both the URL path and query string (obtained using request.path_qs), which means that came_from can be used directly as the redirect target (as always, WebOb handles the task of expanding the server relative path to a fully qualified URL). The mangling of request.GET can also be removed.
The login code appended arbitrary, user-supplied query parameters to URLs by calling the Routes URLGenerator (h.url) with user-supplied keyword arguments. This construct is unfortunate, since url only appends _unknown_ keyword arguments as query parameters, and the parameter names could overlap with known keyword arguments, possibly affecting the generated URL in various ways. This changeset removes this usage from the login code, but other instances remain.
(In practice, the damage is apparently limited to causing an Internal Server Error when going to e.g. "/_admin/login?host=foo", since WebOb returns Unicode strings and URLGenerator only allows byte strings for these keyword arguments.)
|
|
|
Søren Løvborg
|
a0a9ae753cc4
|
10 years ago
|
|
login: simplify came_from validation
Even though only server-relative came_from URLs were ever generated, the login controller allowed fully qualified URLs (URLs including scheme and server). To avoid an open HTTP redirect (CWE-601), the code included logic to prevent redirects to other servers. By requiring server-relative URLs, this logic can simply be removed.
Note: SCRIPT_NAME is still not validated and it is thus possible to redirect from one app to another on the same netloc.
|
|
|
Søren Løvborg
|
12b47803189f
|
10 years ago
|
|
cleanup: use example.com for tests and examples
example.com is explicitly reserved for this purpose. Using that means we won't accidentally hammer a real server or real email address if an example value escapes into the wild, e.g. in an automated test.
The domain "kallithea.example.com" has been used throughout to refer to the example Kallithea server.
|
|
|
Søren Løvborg
|
431689d7f37d
|
11 years ago
|
|
remove vestiges of Python 2.5 support
We only support Python 2.6 and 2.7; hence we do not need to import with-statement support from __future__.
|
|
|
Andrew Shadura
|
f629e9a0c376
|
11 years ago
|
|
auth: secure password reset implementation
This is a better implementation of password reset function, which doesn't involve sending a new password to the user's email address in clear text, and at the same time is stateless.
The old implementation generated a new password and sent it in clear text to whatever email assigned to the user currently, so that any user, possibly unauthenticated, could request a reset for any username or email. Apart from potential insecurity, this made it possible for anyone to disrupt users' workflow by repeatedly resetting their passwords.
The idea behind this implementation is to generate an authentication token which is dependent on the user state at the time before the password change takes place, so the token is one-time and can't be reused, and also to bind the token to the browser session.
The token is calculated as SHA1 hash of the following:
* user's identifier (number, not a name) * timestamp * hashed user's password * session identifier * per-application secret
We use numeric user's identifier, as it's fixed and doesn't change, so renaming users doesn't affect the mechanism. Timestamp is added to make it possible to limit the token's validness (currently hard coded to 24h), and we don't want users to be able to fake that field easily. Hashed user's password is needed to prevent using the token again once the password has been changed. Session identifier is an additional security measure to ensure someone else stealing the token can't use it. Finally, per-application secret is just another way to make it harder for an attacker to guess all values in an attempt to generate a valid token.
When the token is generated, an anonymous user is directed to a confirmation page where the timestamp and the usernames are already preloaded, so the user needs to specify the token. User can either click the link in the email if it's really them reading it, or to type the token manually.
Using the right token in the same session as it was requested directs the user to a password change form, where the user is supposed to specify a new password (twice, of course). Upon completing the form (which is POSTed) the password change happens and a notification mail is sent.
The test is updated to test the basic functionality with a bad and a good token, but it doesn't (yet) cover all code paths.
The original work from Andrew has been thorougly reviewed and heavily modified by Søren Løvborg.
|
|
|
Andrew Shadura
|
b75f1d0753d6
|
11 years ago
|
|
privacy: don't tell users what is the reason for a failed login
Makes it harder for strangers to probe the instance for presence of certain users. This can make it harder to break in, as it is now harder to tell is a username or a password are wrong, so bruteforcing should probably take a bit longer if you don't know what exactly are you doing.
|
|
|
Mads Kiilerich
|
caaf0d07c168
|
11 years ago
|
|
|
|
|
Søren Løvborg
|
81d8affd08f4
|
11 years ago
|
|
auth: remove username from AuthUser session cookie
There's no reason to store the username when we store the user ID. We have load the user from database anyway under all circumstances, to verify e.g. that the user is (still) active.
This does not impact application code, but does impact a number of test cases which explicitly checks the username stored in the session.
|
|
|
Mads Kiilerich
|
82ea8d67fc62
|
11 years ago
|
|
validators: cleanup of message wording
NotReviewedRevisions was unused and could thus got the big cleanup ...
|
|
|
Søren Løvborg
|
3d3bec370fa5
|
11 years ago
|
|
|
|
|
Mads Kiilerich
|
148360f533a4
|
11 years ago
|
|
|
|
|
Mads Kiilerich
|
c0da0ef508da
|
11 years ago
|
|
auth: only API keys with 40 alpha-numeric characters are valid
This makes it easy to disable API keys in the database without violating the uniqueness constraint, using something like:
UPDATE users SET api_key='-'||api_key; UPDATE user_api_keys SET api_key='-'||api_key;
|
|
|
Mads Kiilerich
|
9a02f9ef28d7
|
11 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
|
9b2c5e8b37ea
|
11 years ago
|
|
notification tests: delete notifications before (not after) tests
Don't clean notifications (and changeset comments) *after* the test, but *before* the test. Other unit tests don't care if they leave notifications in the database, and neither should these. Rather, they should ensure their *own* preconditions before testing.
Admittedly, currently only one test leaves a notification in the database, but more could come along at any time (and why worry?): TestPullrequestsController.test_create_with_existing_reviewer
|
|
|
Thomas De Schampheleire
|
222a36d30368
|
11 years ago
|
|
|
|
|
Thomas De Schampheleire
|
414142964b62
|
11 years ago
|
|
|
|
|
Thomas De Schampheleire
|
01abb838b7a8
|
11 years ago
|
|
|
|
|
Thomas De Schampheleire
|
29d0ed59e674
|
11 years ago
|
|
|
|
|
Søren Løvborg
|
17281153a4c0
|
11 years ago
|
|
|
|
|
Andrew Shadura
|
ab01e0a5d240
|
11 years ago
|
|
|
|
|
Thomas De Schampheleire
|
4c5c59b96adc
|
11 years ago
|
|
login: preserve GET arguments throughout login redirection (issue #104) When redirecting a user to the login page and while handling this login and redirecting to the original page, the GET arguments passed to the original URL are lost through the login redirection process. For example, when creating a pull request for a specific revision from the repository changelog, there are rev_start and rev_end arguments passed in the URL. Through the login redirection, they are lost. Fix the issue by passing along the GET arguments to the login page, in the login form action, and when redirecting back to the original page. Tests are added to cover these cases, including tests with unicode GET arguments (in URL encoding).
|
|
|
Mads Kiilerich
|
e3aab61a9411
|
11 years ago
|
|
|
|
|
Thomas De Schampheleire
|
e04106e46d6f
|
11 years ago
|
|
auth: return early in LoginRequired on API key validation
Simplify the logic in the LoginRequired decorator when Kallithea is accessed using an API key. Either: - the key is valid and API access is allowed for the accessed method (continue), or - the key is invalid (redirect to login page), or - the accessed method does not allow API access (403 Forbidden)
In none of these cases does it make sense to continue checking for user authentication, so return early.
|
|
|
Mads Kiilerich
|
790da8154ef8
|
11 years ago
|
|
|
|
|
Mads Kiilerich
|
c154dc461bd5
|
11 years ago
|
|
|
|
|
Na'Tosha Bard
|
dacdea9fda2a
|
11 years ago
|
|
|
|
|
Bradley M. Kuhn
|
24c0d584ba86
|
12 years ago
|
|
|
|
|
Bradley M. Kuhn
|
d208416c84c6
|
12 years ago
|
|
|
|
|
Bradley M. Kuhn
|
d1addaf7a91e
|
12 years ago
|
|
Second step in two-part process to rename directories. This is the actual directory rename.
|