lib: move webhelpers2 and friends to webutils

Gives less of the unfortunate use of helpers - especially in low level libs.

db: consistently use the name settings for the return value from get_app_settings

Get rid of yet another old rc reference.

(The "settings" in auth is slightly different, but pretty much the same.)

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).

imports: always import the whole kallithea module to use top level kallithea variables

This is slightly more lazy and might avoid some depeendency issues.

lib: drop own asbool implementation and consistently use tg.support.converters as utils2.asbool

str2bool never reported error on odd input such as '' or '-1', but the tg
asbool behaviour of raising ValueError("String is not true/false: %r" % obj) in
that case seems fine.

db: drop SA caching_query and FromCache, and thus sql_cache_short beaker cache

It is not a good idea to have dead ORM objects. If we want caching, we should
do it explicit.

It is unknown how much this cache helps, but we can profile and introduce
better caching of simple data where relevant.

py3: fix non-ASCII URLs - decode unicode correctly before passing them to controllers as unicode strings

This is needed for supporting localized repo path names in the path of URLs.

Some references:

https://www.python.org/dev/peps/pep-0333/#unicode-issues
https://bugs.python.org/issue16679
http://lucumr.pocoo.org/2010/5/25/wsgi-on-python-3/
https://bugs.launchpad.net/pecan/+bug/1451842
https://github.com/tipabu/eventlet/commit/a5a7751b013fe99b6d30acbca79e819770e9ae5d

py3: remove safe_unicode in places where it no longer is needed because all strings (except bytes) already *are* unicode strings

(The remaining safe_unicode calls are still needed and can't just be removed,
generally because we in these cases still have to convert from bytes to unicode
strings.)

lib: clean up ext_json and how it is used - avoid monkey patching

Note that py3 json.dumps will return ASCII (with all unicode escaped) encoded
as str. But we generally want JSON as bytes (which json.loads also can read),
so also wrap the result with ascii_bytes in many places.

lib: handle both HTML, unsafe strings, and exceptions passed to helpers.flash()

Before, h.flash would trust any input to contain html ... and callers would
convert exceptions to string, often with a simple str() or unicode() ... which
really didn't deserve to be trusted.

Instead, only trust messages that have a __html__ and escape anything else ...
but also apply str/unicode on the parameter so the caller doesn't have to but
*can* pass an exception directly.

lib: refactor _get_access_path as get_path_info

We will need it later when it gets more tricky to get the path from environ ...

py3: only use safe_str for string conversion - not for arbitrary __str__ invocation

Usually, we actually just want unicode string, and it is better to just unicode().

cleanup: minor refactorings and simplification of dict usage

Makes it more py3 compatible.

lib: drop redundant json.dumps encoding=utf8

Not supported in py3 - json is by definition(?) always UTF-8.

app: drop finally handling in BaseVCSController.__call__

Our optional 'wrapper' middleware is logging response times more correctly
after b42ee1bdf082 - there is no point in logging the timing of the __call__ in
the main app.

Similarly, the session was removed too early. But TurboGears is already picking
up our db session and using DBSessionRemoverMiddleware to .remove() it ... and
at the right time. So just stop trying.

ssh: show ssh URL on summary page

Original change has been heavily modified by Mads Kiilerich.

ssh: introduce ini setting 'ssh_enabled', disabled by default

Administrators should control the use of SSH and may want to disable SSH
access, temporarily or permanently.

An explicit setting ssh_enabled is better than e.g. checking for a valid
ssh_authorized_keys setting, to allow such trivial temporary disabling.

To keep the controllers simple, introduce a decorator IfSshEnabled instead
of repeating the same config checks in every method.

clone_url: always pass a clone_uri_tmpl, with Repository.DEFAULT_CLONE_URI as last resort

clone_url() had a layering violation of using c.clone_uri_tmpl . This
refactoring now makes it clear that this only was used from
PullRequest.__json__(), so move the hack there and simplify it.

hooks: move _handle_rc_scm_extras to utils2 as set_hook_environment and get_hook_environment

This is also a step away from using the name 'extras' ... but still a lot of
traces left behind ...

hooks: drop 'server_url' parameter

The parameter might be conceptually nice, but it was only available for 2
hooks. To be really useful, it should have been available everywhere. It also
only reported the URL from the web request that initiated the hook ... and as
such it does some layering violations. The user URL might be the address that
should be used internally from the hook. And it can conceptually not be made
available actions that doesn't originate from a user web request. It seems much
better that custom hooks configure what they want to do. Perhaps by reading the
.ini file and using canonical_url.

middleware: unify Mercurial and Git _handle_request in the VCS base class

Finally, it is more clear in what the VCSs are different ... and what generic
setup code is needed.

middleware: introduce BaseVCSController parse_request retrieving repo_name and use that for VCS dispatch

Return a parsed_request namespace. For now, it just contains repo_name, thus
replacing is_hg / is_git and __get_repository.

The fallback to the wrapped application now happens in the base class and the
VCS implementations can be simplified.

middleware: move handling of permanent repo URLs to separate middleware

This is about the handling of repo URLs like '_123' for the repo with repo_id
123. The de-mangling of such URLs was spread out across multiple layers. It
fits much more nicely as a middleware layer. The code in routing and simplehg /
simplegit can thus be removed.

The base _get_by_id function was confusing - fix it by removing it. To do that,
refactor utils introducing fix_repo_id_name to replace get_repo_by_id.

We now assume in the application that we never have any extra leading '/' in
URL paths.

And while trailing extra '/' might be fine in actual URLs, they must be handled
at the routing level, not propagated through all layers. This changeset is not
really changing that.

auth: drop authenticating_api_key from AuthUser

It doesn't belong as a user property - it is more of a session property ...
which is what we already use instead.

auth: move CSRF checks from the optional LoginRequired to the more basic BaseController._before

_before is not called for the CSRF-immune JSON-API controller and is thus a
good place to check CSRF. This also apply CSRF protection to the login
controller.

The flag for needing CSRF checking is stored in the thread global request
object when passed from __call__ to _before for regular controllers. It is thus
also set for requests to the JSON-RPC controller, but not used.

auth: simplify API key auth - move it out of _determine_auth_user

This gives less of the special handling of API key auth in LoginRequired ...
but we still need to disable the LoginRequired CSRF protection for API key
auth.

auth: make User.get_by_api_key more strict about only returning active non-default users

Thus drop some extra checks against default user.

auth: remove AuthUser __init__ magic for fallback to default user instead of the requested user

Be reliably explicit about what user we expect. If we want default user /
anonymous user, say so explicitly.

auth: drop unused AuthUser.is_authenticated

It seems like other ways of tracking authentication state are better. AuthUser
is a *potentially* authenticated user. We prefer to keep it as that, without
modifying the AuthUser object if the user actually should be authenticated.

The primariy indicator that a user is authenticated is when the AuthUser is set
as request.authuser .

(Alternatively, we could create an AuthenticatedUser sub-class and move things
like access control checks there. That would help ensuring it is used
correctly, without having to check an is_authenticated flag.)

locking: drop the pull-to-lock / push-to-unlock functionality

The feature is not worth the maintenance cost. The locking is too coarse and
unflexible with insufficient UI and UX. The implementation is also quite
invasive in tricky areas of the code, and thus high maintenance.
Dropping this will enable other cleanup ... or at least make it easier.

cleanup: remove unnecessary (and potentially problematic) use of 'literal'

webhelpers.html.literal (kallithea.lib.helpers.literal) is only needed when
the passed string may contain HTML that needs to be interpreted literally.
It is unnecessary for plain strings.

Incorrect usage of literal can lead to XSS issues, via a malicious user
controlling data which will be rendered in other users' browsers. The data
could either be stored previously in the system or be part of a forged URL
the victim clicks on.

For example, when a user browses to a forged URL where a repository
changeset or branch name contains a javascript snippet, the snippet
was executed when printed on the page using 'literal'.

Remaining uses of 'literal' have been reviewed with no apparent problems
found.

Reported by Bob Hogg <wombat@rwhogg.site> (thanks!).

cleanup: remove unnecessary (and potentially problematic) use of 'literal'

webhelpers.html.literal (kallithea.lib.helpers.literal) is only needed when
the passed string may contain HTML that needs to be interpreted literally.
It is unnecessary for plain strings.

Incorrect usage of literal can lead to XSS issues, via a malicious user
controlling data which will be rendered in other users' browsers. The data
could either be stored previously in the system or be part of a forged URL
the victim clicks on.

For example, when a user browses to a forged URL where a repository
changeset or branch name contains a javascript snippet, the snippet
was executed when printed on the page using 'literal'.

Remaining uses of 'literal' have been reviewed with no apparent problems
found.

Reported by Bob Hogg <wombat@rwhogg.site> (thanks!).

lib: BaseRepoController: use webob.exc.HTTPNotFound if possible

In the entire code base, the use of 'paste' is very limited.
In particular, 'paste.httpexceptions' is only still used in lib/base.py, in
three occurrences:
- two of them in class BasicAuth which derives from paste.auth.something.
Here it probably makes sense to use paste.httpexceptions.
- one in BaseRepoController, which has no specific relation to paste. This
occurrence can be replaced with webob.exc like in the rest of the code
base.

auth: drop UserCreationError handling for AuthUser.from_cookie

Simplify the code a bit, and take an unnecessary concern out of sight.

Users with cookie auth must already exist in the system, and they will thus
never be created on the fly when using a cookie for authentication.

If completely weird things should happen anyway, it would probably be better to
get a clear stack trace.

auth: consistently use request.authuser - drop request.user

This seems like old tech debt. Now, just get rid of it.

templates: remove notification count from user profile button

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.

This commit removes the notification count 'badge' next to the username
top-right, and the count in the expanded field when clicking that username.

cache: move cache invalidation from web handler to post push hook

We need the post push hook anyway ... and having the cache invalidation here
will also work for ssh pushes in the future.

The name log_push_action is thus no longer spot-on. That might change later,
but requires some care as it also is used directly as hook name.

Note that having cache invalidation in the hook will do that debug logging no
longer will appear in the server log.

Based on a patch by Dominik Ruf.

less: use .label and .label-* style instead of custom .*tag

This reduces the code and makes it more consistent.

Visualy this results in bold texts (which in case of small fonts is easier to read)
and a bit more padding left and right.

For a proper spacing, this requires to remove whitespaces before and after
the texts inside the labels.

Since the name changed from tag to label, also rename the variables and functions.

auth: consume request body before responding 401 or 403 during authentication

In order to work correctly with reverse proxies like Apache, the application
needs to consume the whole body before returning and closing the connection.
Otherwise the reverse proxy may complain about a broken pipe.

For example, if the client sends a lot of data and kallithea doesn't read all
that data before sending 401, the connection will be closed before the reverse
proxy has sent all the data. In this case an apache reverse proxy will fail
with a broken pipe error.

This is not necessary for all wsgi servers. Waitress automatically buffers (and
therefore reads) all the data and uwsgi has a 'post-buffering' option to do the
same. But AFAIK there is no way to push to a password protected hg repository
when using gunicorn without this changeset.

codingstyle: trivial whitespace fixes

Reported by flake8.

middleware: convert check_locking_state to be a separate function

Prepare for use with SSH.

lib: move special Mercurial HTTP listkey exception out of _check_locking_state

Prepare for use with SSH.

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/TurboGears

Changes 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

auth: refactor auth plugin importing

The list of authentication plugins, configured in the database, may contain
plugins which are no longer available. Therefore directly import the plugin in
get_auth_plugins and report the ImportError to the log instead of
breaking kallithea completely.

Patch modified by Mads Kiilerich.

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.

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.

model: remove BaseModel class

The only remaining purpose of this class was to provide the "sa" field,
allowing a custom SQLAlchemy session to be used for model operations.
However, this never actually worked, nor was it used anywhere.

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.

vcs: restructure authorization check

This is a pure refactoring, except for some changed debug log messages.

With this change, we simply return early if anonymous (= default user)
access is enabled, which should help overall readability.

(Diff becomes clearer if whitespace changes are ignored.)

vcs: dedup auth code between Hg and Git middleware

There's a lot of verbatim duplicated code in the Hg and Git VCS
middleware. This attempts to deduplicate a bit of it.

The _authorize function is a bit awkward, but for now the goal is simply
to remove duplicated code, not improving program structure and design.

As such, the code in _authorize is almost a verbatim copy of the code
removed in the two controllers.

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.

lib: move jsonify from utils to base

Suggested by Mads Kiilerich.

The jsonify method is the only thing in utils that directly uses pylons.
Move it to base where it fits better and we can use existing global imports.

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: 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).

db: add some PullRequest.query() shortcuts

This makes database query code more explicit and increases readability.

E.g. the function name get_pullrequest_cnt_for_user was bad, because the
concept of "pullrequest for user" is incredibly vague, and could refer
to any kind of association between PRs and users. (Quiz time! Does it
mean that the user is the PR owner, that the user is reviewing, or that
the user has commented on the PR and thus is receiving notifications?)

A descriptive name could be "get_open_pull_request_count_for_reviewer",
because the function is indeed only concerned with reviewers and only
with open pull requests. But at this point, we might as well say
PullRequest.query(reviewer_id=user, include_closed=False).count()
which is only slightly longer, and doesn't require us to write dozens
of little wrapper functions (including, any moment now, a separate
function for listing the PRs instead of counting them).

Note that we're not actually going down an abstraction level by doing
this. We're still operating on the concepts of "pull request", "open"
and "reviewer", and are not leaking database implementation details.

The query() shortcuts are designed so they default to not altering
the query. Any processing requires explicit opt-in by the caller.

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

hg: drop pointless push_ssl configuration setting - if there is a risk push can be compromised, credentials can also easily be stolen for pull

Everybody should have a ssl-only setup now. Alternatively, there is a use case
for 'only anonymous traffic on ssl - all authentication and authenticated
traffic must be on ssl'. That can be done with proper web server configuration.

protocols: fix assertion error when accessing repositories with "permanent" urls (Issue #202)

I am not aware of any good way to test this, so it is tested with a Mercurial only hack.

db: fix Ui.get_by_key to also filter on section

Key alone is not enough to identify a Ui row.

auth: note that we never emit authuser "cookies" for the default user

The only place where we set "authuser" in the session is in log_in_user,
which is called only by the internal auth system and by auth plugins.
The internal auth system cannot log a user in as the default user,
because the default user doesn't have a password (and cannot have a
password assigned). Auth plugins cannot log a user in as the default
user, because the user doesn't have the right extern_type. As such, it's
a bug if log_in_user is ever called with the default user (which this
commit documents with an assert).

This realization makes the is_authenticated field of the authuser cookie
redundant, as it's always True. It also emphasizes that is_default_user
and is_authenticated are mutually exclusive.

auth: inline AuthUser.set_authenticated

This makes the following commits easier to follow, and makes it more
explicit that something weird is going on, with more cleanup needed.

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).

vcs: invalidate repo caches _after_ a push finishes

Caches were invalidated right after creating the result iterator, before
actually applying the change. Caches would thus be refreshed too early and soon
be outdated.

This bug was especially seen causing errors with missing revisions when
creating PRs right after pushing.

docs: improve documentation of beaker session configuration

beaker.session.auto is dropped; it defaults to false and there is no reason to
ever set it true for Kallithea.

beaker.session.cookie_path and secure are dropped; like cookie_domain, they
should automatically be set to the right value.
* * *
beaker.session.cookie_expires MUST have the default value of True to provide the default value of 'browser session lifetime' when not enabling 'remember' in the login box. The cookie life is hardcoded to 365 days when remember is selected.

auth: construct AuthUser from either user_id or db.User object

If the caller already has the database User object, there's no reason
for AuthUser to look it up again.

The `api_key` lookup functionality is dropped, because 1) it's only
used in one place, and 2) it's simple enough for the caller to do the
lookup itself.

The `user_id` lookup functionality is kept, because 1) it's frequently
used, and 2) far from a simple `User.get(id)` lookup, it has a complex
interaction with UserModel. (That cleanup will have to wait for another
day.)

All calls of the form `AuthUser(user_id=x.user_id)` can be replaced with
`AuthUser(dbuser=x)`, assuming `x` is a db.User. However, verifying that
assumption requires a manual audit of every call site, since `x` might
also be another `AuthUser` object, for instance. Therefore, only the
most obvious call sites have been fixed here.

auth: actually use _determine_auth_user argument

Fix silly mistake which slipped through the review. (We should not look
up the session cookie again, when it's passed as a function argument.)

BaseController: hide "Log out" link for external login sessions

If user is authorized by external means (API key or container auth),
Kallithea is not actually able to log the user out and should not show
the "Log out" link.