hg: return 400 Bad Request for hg commands that not are commands
Avoid throwing bare Exceptions which requires framework specific testing. Instead, return a reasonable http error code and make the test more framework independent.
The "helpful" message will just be a description of the http exception and not sent to the client.
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.)
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).
auth: add AuthUser.is_anonymous, along with some exposition
This reveals the name of the NotAnonymous decorator to be misleading, an unfortunate detail only documented here, but which must be properly resolved in a later changeset.
Note that NotAnonymous behaves as advertised as long as it is used together with LoginRequired, which is always the case in the current code, so there's no actual security issue here, the code is just weird, hard to read and fragile.
---
Some thoughts on cleaning this up in a future changeset: As it turns out, every controller (except the login page!) should be LoginRequired decorated (since it doesn't actually block anonymous users, as long as anonymous access is enabled in the Kallithea config). Thus the most obvious solution would be to move the LoginRequired functionality into BaseController (with an override for LoginController), and delete the decorator entirely. However, LoginRequired does one other thing: it carries information about whether API access is enabled for individual controller methods ("@LoginRequired(api_key=True)"), and also performs the check for this, something which is not easily moved into the base controller class, since the base controller doesn't know which method is about to be called. Possibly that can be determined by poking Pylons, but such code is likely to break with the upcoming TurboGears 2 move. Thus such cleanup is probably better revisited after the switch to TG2.
auth: perform basic HTTP security checks already in BaseController
There's no reason to postpone these to a LoginRequired decorated controller function. This way, they run unconditionally for all subclasses of BaseController (so everything except JSON-RPC and VCS controllers).
My recent changes broke it. Add some band aid to help it. The pull request page might not be a perfect fit for Bootstrap forms, but that is a better approximation than nothing.
tests: give test_pullrequests a test_context in addition to the usual app_fixture
test_pullrequests has unit-ish tests that invoke library functions directly ... but these happen to require localization initialized. For TG2 we only have localization initialized when we have a test_context. Thus, additionally wrap these tests in a test_context.
The M class is actually a wrapper around formencode.api.Validator.message, inserting a translator into the 'state' object. Setting a translator into the state object is indeed what is mentioned in the formencode docs.
But, if you work this way, the custom state should be set both for custom validators, as well as for validators that are simple wrappers around formencode's default validators (for example wrappers that just set a custom message string). And the latter is what Kallithea is currently _not_ doing.
Also, when using formencode.api.Validator.message correctly, you should not use the translator function _ on your validator strings manually.
Remove the inconsistency in Kallithea validators as follows: - remove M and StateObj classes - replace the usage of M by direct calls to self.message (which is now no longer in charge of translating but simply of mapping a keyword onto the right message) - translation will be done by explicit _ calls at Kallithea side, so that formencode directly receives translated strings.
forms: wrap LoginForm inside function like other forms
All forms except LoginForm are wrapped inside a function. The original purpose of this wrapping seems to be the ability to pass parameters to tweak the form.
But, this also has another desired effect: translation of strings wrapped with _ is no longer attempted when reading the class definition, but only when the function is instantiated. In the former case, it is not guaranteed that a translator is actually available because we are not running in standard application context.
tests: introduce test_context to handle internationalization
As preparation to the migration to Turbogears2, introduce a test_context context manager to wrap certain tests. The context manager provides a translator for tests that need it. This translator was previously provided globally to all tests via the TestController.
When swapping Pylons with Turbogears2, the new file kallithea/tests/test_context.py can be removed, and the test_context from tg2/tg/util/webtest.py should be used.
conftest.py does not contain an import of pylons (only of pylons.test) so usage of e.g. pylons.config can never work, so it cannot be used. Since the tests run fine regardless, we can get rid of this code.
These references are either not used, or do not provide value. The Windows installation and email pages still references documents from the Pylons website -- alternative pages from Turbogears2 need to be found.
The img and style methods in the error controller are standard Pylons boilerplate. They are used from the Pylons default error document template. The purpose of these methods is to serve images and stylesheets from the Pylons module, rather than having to copy these files into a particular application installation.
Since Kallithea uses a custom error template with our own styles and images, these methods are not actually used.
pullrequests: fix 'upgrade' from revision to branch when creating PR
This will do that the default title for the PR created from a non-head revision will contain a short hash instead a long.
org_ref_type was set to 'branch' while org_ref_name remained the full hash. h.short_ref would thus not truncate. Instead, we keep the org_ref_type as 'rev' while storing the branch name in the database.
auth: prevent LDAP query language injection of usernames
This could cause odd LDAP queries that could fail but couldn't give access without a valid user query and credentials. It thus had no security implications.
A location was hardcoded. The location was wrong for many systems and prevented actual TLS from working. Also, it should not be necessary with modern Pythons.
For some reason, instead of removing it, we now decide to expose it to the user. Choice FTW!
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".
The database lookups of the user and repositories are moved outside, into the callers, which have already looked up the relevant objects, thus saving two database lookups when creating a PR.
db: rename UserFollowing.follows_repo_id to follows_repository_id
Since the relationship is 'follows_repository', rename the column to be 'follows_repository_id', not 'follows_repo_id'. This also makes the Python column name match the actual database column name.
(The inconsistency dates back to early RhodeCode days.)
Many calls to Session().flush() were completely superfluous and have been removed. (See also the note on "flush" in the contributor docs.) For the remaining calls, a comment has been added to explain why it's necessary.