tests: clarify that default parameters are for form - direct model access requires different types

_get_repo_group_create_params parent_group_id is thus set to the form value
'-1' instead of None. It worked before anyway because the model failed to find
the repo '-1' and thus got pretty much the same as None.

db: drop and re-create old schemas instead of Base.metadata.drop_all()

Sadly, Base.metadata.drop_all() sometimes doesn't work because of foreign key
constraints. We only know this is the case on PostgreSQL and MySQL - use
workarounds for these.

db: upgrade to SQLAlchemy 1.1, fixing invalid PostgreSQL SQL in User.is_default_user

eea19c23b741 triggered a bug in SQLAlchemy 1.0.17, causing invalid SQL to be
emitted for PostgreSQL: For the (Python) query '(x == y) == z', SQLAlchemy
generates the SQL 'x = y = z'. SQLite and MySQL accepts this, but PostgreSQL
does not. This bug has been fixed in 1.1.

Testing and review shows no problems with 1.1

alembic: check database version before starting kallithea

Running kallithea with the wrong database version could be dangerous.
Therefore, check for the correct version before starting Kallithea.

lock: fix encoding error when Mercurial emits a localized warning with the Kallithea "repository locked" message

Mercurial use (utf8) encoded localized messages. Things failed when it tried to
convert the encoded Mercurial format string to unicode so it could insert
Kallithea's unicode (but ascii compatible) "repository locked" exception
string.

That could for example be reproduced by running the (manual) vcs test
test_push_on_locked_repo_by_other_user_hg with environment LANG=de_DE for the
server side.

To fix this, make sure the "repository locked" message is encoded so Mercurial
handles it correctly.

tests: refactor _construct_url in the manual vcs test to actually return a url - dest is handled separately

Based on work by Dominik Ruf.

tests: use relative path when adding file to git repo (fixes error with newer git versions)

Newer git versions would fail 'git add /path/to/file' with:
fatal: /path/to/file: '/path/to/file' is outside repository
That caused test_push_unlocks_repository_git to fail.

To fix that, use relative instead of absolute path.

tests: add workaround for failing manual vcs tests test_ip_restriction_*

Back out a part of ee88c8c07111; invalidate_all_caches would expire beaker
caches ... but that doesn't work so well for expiring the cache in the separate
server process used for vcs tests.

tests: add context fixture to fix test_ip_restriction_hg/git

test_ip_restriction_hg/git needs the test context fixture or otherwise
localization in form validation fails with:

TypeError: No object (name: context) has been registered for this thread

git: add references for Git pull request heads

When a pull request is created, stick a reference to it in the refs/pull
namespace, similarly to what GitHub does, and remove it when the pull
request is deleted. That reference guarantees the commits stay around
and don't get garbage-collected when a PR is rebased or revised.
Also, that makes it possible to access head of historic pull requests
using Git by fetching a ref from the source repository.

Unlike GitHub though, we don't put the ref into the destination repository
and don't copy commits there to prevent undesired repository growth.
Kallithea uses global pull request identifiers, so there should not be
any confusion as to what pull request the ref corresponds to.
We may later provide a shim to redirect users to the source repository
if that deems needed.

admin: clean-up of Gravatar and IP in user profile headers

Move things around and use Bootstrap compatible markup.

The user IP will rarely be relevant - especially not on the admin pages; the IP
configuration there is more for other user's IP. It might however be somewhat
relevant for a (non-admin) user to know his own IP. There is no obvious place
to show that - it doesn't really fit in as a part of the user profile. Anyway,
just show it under Show Permissions.

gearbox: fix setup-db failure when db is created for the first time (Issue #272)

The port to TG2 in e1ab82613133 replaced pylonsconfig.init_app for gearbox
commands with middleware setup code. That introduced reading of the database
for commands that don't need a database - for example the command for creating
databases ... which thus failed.

To fix that, move the middleware setup with database access so it only is run
for gearbox command that requires a db session. There is apparently no need for
a direct replacement for pylonsconfig.init_app .

gearbox: avoid duplicate logging setup when calling make_app

make_app initialized logging, while the gearbox common code does it too.
Split make_app in two to be able to avoid the redundant call.

The logging setup in make_app cannot be removed because it is needed for
'gearbox serve' and direct WSGI invocation, which does not pass through gearbox
common.py.

lib: fix Git hook failure after TG2 port

This fixes manual_test_vcs_operations.py failures after e1ab82613133 dropped
test_env and test_index parameters to load_environment without fixing
handle_git_receive().

lib: reduce impact of bad import cycle

Follow-up to e0f31c7d0f5e.

It is unfortunate that generic lib code depends on
kallithea.lib.paster_commands.common - it would make more sense if it was the
other way around. We thus move the import down to inside the function that
needs it.

The import of kallithea.lib.paster_commands.common in db_manage triggered its
import of logging.config . middleware.py thus also got logging.config when it
just imported logging and things worked. Without this import of
paster_commands, we also have to fix the import in db_manage.

pullrequests: fix broken "new PR iteration" handling of ancestor changes

An earlier refactor (5d60c9a391cd) flubbed this code and accidentally
assumed the most recent common ancestor would not change when iterating,
which is of course only the case when there are no merges from 'other' to
'org' among the new revisions.

This was not only not caught during manual testing (nor review), but
neither did the test suite test this. That has now also been rectified.

Jenkinsfile: run each py.test on a separate executor

Running all py.test processes in parallel on the same executor is not a good idea.
If a node has not much RAM, it could run out of memory when all py.test processes
run at the same time. And If there are only 2 CPU cores, it doesn't make sense
to run more then 2 processes either.
Therefore use a separate executor for each one. The py.test processes will thus
only run in parallel if there are multiple Jenkins executors available.

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.

config: activate logging on 'gearbox serve'

With the switch from paster to gearbox, early logging did not show on the
console anymore. This includes the initialization logging of TurboGears2.

For our own gearbox commands, the activation of logging is done in
kallithea/lib/paster_commands/common.py:_bootstrap_config, but for 'serve'
this method is not used. 'gearbox serve' immediately runs the
'paste.app_factory' specified in setup.py, which is make_app.

Extend make_app with logging initialization in the same way as is done in
_bootstrap_config.

config: initialize routes directly from RootController

Let RootController directly initialize routes instead of app_cfg injecting
the mapper.

For test initialization, we also need a handle to the mapper. We could
either recreate it (but fragile if the real mapper initialization is changed
later) or obtain it from a fresh RootController. This commit opts for the
latter.

TurboGears2 migration: update ini files for error email settings

Error emails are now handled by backlash, which is configured through
TurboGears2's ErrorReporter. ErrorReporter expects different configuration
key names than Pylons did, moreover under a new 'trace_errors' namespace.

Since some of the email-related settings are shared between application and
error emails, we cannot just rename the existing settings (it would be very
odd to have application settings under a 'trace_errors' namespace).
Requiring the user to duplicate its settings is also undesirable.

Instead, use 'get' to populate the trace_errors namespace based on the
existing settings we already had. Unfortunately, 'get' expects the setting
to be actually present, so we need to provide an out-of-the-box value for
the error-related email settings or there will be an error at startup. We
use empty values because there is no realistic default value we can provide.

docs: smtp_port is now respected by backlash

Support for a custom smtp_port was recently added to backlash, so bump
backlash and update the documentation to reflect that.

docs: update email documentation for TurboGears2

Small update to the documentation.

ini: set 'debug' only in one place

In Pylons-based Kallithea, the 'debug' option was set first in [DEFAULTS]
and then overridden with 'set debug = X' in [app:main]. Even when the value
under [DEFAULTS] was commented out, the use of the 'set' keyword (providing
override semantics) was required, because a default value for the global
'debug' was provided in the framework.

The presence of these two debug assignments is confusing.
Moreover, TurboGears2 makes the situation more easy and does not expect
'debug' to be in [DEFAULTS] nor does it provide a default at that level.
As a result, we can simple use 'debug = X' under [app:main].

Additionally, clarify the Big Fat Warning: the wording 'the line below' is
ambiguous and could be misunderstood in an ini file that had previously been
changed.

ini: remove 'pdebug' setting (profiling)

The mysterious 'pdebug' was used in Pylons-based Kallithea to enable
ProfilingMiddleware. Profiling should now be handled by TurboGears2.

In any case, the pdebug variable is no longer checked anywhere, so can be
removed.

backend: add TurboGears2 DebugBar support

DebugBar is a toolbar overlayed over the Kallithea web interface, allowing
you to see:

* timing information of the current request, including profiling information
* request data, including GET data, POST data, cookies, headers and
environment variables
* a list of executed database queries, including timing and result values

Enable it automatically when in debug mode and all requirements are met.
Add documentation.

tests: remove custom test_context

All test_context callers now use tg.util.webtest.test_context.
The custom kallithea.tests.test_context.test_context was only temporarily
added for the transition from Pylons to Turbogears2.

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

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

auth: catch all exceptions while loading auth plugin - not just ImportError

A third party plugin could raise any kind of exception, so we should be
prepared for that.

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.

auth: refactor auth plugin importing - rename enabled_plugin to enabled_plugin_names

Patch modified by Mads Kiilerich.

config: move bulk of make_app to app_cfg.py

In preparation of migrating to TurboGears2, where almost all setup code is
in one file app_cfg.py, move the existing Pylons-based configuration to the
same location. This will make it more clear which changes are really done
for TurboGears2.

The current move does not make functional changes, the code is moved
unmodified.

config: move bulk of load_environment to new file app_cfg.py

In preparation of migrating to TurboGears2, where almost all setup code is
in one file app_cfg.py, move the existing Pylons-based configuration to the
same location. This will make it more clear which changes are really done
for TurboGears2.

The current move does not make functional changes, the code is moved
unmodified. Only real change is removal of some unused import statements.

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.

controllers: remove empty __before__ methods

__before__ methods that only call the super __before__ method are redundant
and can be removed. The super's method will be called directly.

tests: add global test_context_fixture

Move the existing app_test_context_fixture from test_pullrequests.py to
conftest.py to make it available to all test modules.

It is useful in two cases:

1. there is test setup code (xUnit style) that needs to execute in the same
test context as the actual test.

2. even without test setup code, an entire test needs to be executed in a
test context. In this case, the fixture just reduces code complexity by not
requiring changes in the test code (compared to standard 'with
test_context').

It is possible to apply this (or any) fixture to an entire test class using
the class decorator
@pytest.mark.usefixtures("...")
This is similar to 'autouse=True' but can be used even if the fixture is
defined elsewhere.

gearbox: make a make-config sub-command available again

Drop the old kallithea-config command line tool and rework it to a replacement
for the make-config paster command. Make-config was experimental in paste, it
doesn't exist with gearbox, it was tied to pylons, and we already have the
luxury problem of having two ways of doing almost the same thing. This is a
good opportunity to get rid of one of them.

This replacement tool is Kallithea specific and doesn't need to be told that it
is Kallithea it has to create a config file for. The command should thus
perhaps be given a Kallithea specific name instead of the very generic name ...

gearbox: replace paster with something TurboGears2-ish that still works with the Pylons stack

This is a step towards moving away from the Pylons stack to TurboGears2, but
still independent of it.


Some notes from the porting - it could perhaps be the missing(?) documentation
for migrating from paster to gearbox:

Note: 'gearbox' without parameters will crash - specify '-h' to get started
testing.

Replace paster
summary = 'yada yada'
with the first line of the docstring of the Command class ... or override
get_description.

Note: All newlines in the docstring will be collapsed and mangle the long help
text.

Grouping of commands is not possible. Standard commands (for development) can't
be customized under the same name or hidden. (Like for paster, the conceptual
model also assumes that the sub-command naming is namespaced so commands from
other packages won't conflict.)

The usage help is fully automated from the declared options.

For all deprecated Commands, replace paster
hidden = True
with gearbox
deprecated = True

Note: config_file, takes_config_file, min_args and max_args are not available /
relevant.

The gearbox parser is customized by overriding get_parser - there is nothing
like paster update_parser.

Gearbox is using argparse instead of optparse ... but argparse add_argument is
mostly backwards compatible with optparse add_option.

Instead of overriding command or run as in paster, override take_action in
gearbox. The parsed arguments are passed to take_action, not available on the
command instance.

Paster BadCommand is not available and must be handled manually, terminating
with sys.exit(1).

There is no standard make-config command in gearbox.

Paster appinstall has been replaced by the somewhat different setup_app module
in gearbox. There is still no clean way to pass parameters to SetupAppCommand
and it relies on websetup and other apparently unnecessary complexity. Instead,
implement setup-db from scratch.


Minor change by Thomas De Schampheleire: add gearbox logging configuration.
Because we use logging.config.fileConfig(.inifile) during gearbox command
execution, the logging settings need to be correct and contain a block for
gearbox logging itself. Otherwise, errors in command processing are not even
visible and the command exits silently.

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.

jenkinsfile: delete all old files in the workspace

Especial important when files have been renamed and there are *.pyc files
left over from the last build.

jenkinsfile: create venv in special folder instead of jenkins workspace

The path to a jenkins workspace tends to get very long. But the maximum for a
shebang line is usually 127 characters. This can cause problems with git hook
scripts or when calling pip. To avoid these problems create the virtualenv
in the a folder under $JENKINS_HOME which is usually much shorter.

jenkinsfile: remove unnecessary lines

We use TEST_DB environment variable instead of editing test.ini to change the DB
configuration.

tests: use the DB set in TEST_DB for git hooks

Obviously the git hooks and the web app need to use the same database.
Therefore, if the web app uses the TEST_DB environment variable, the git hooks
needs to do the same.

tests: don't compare MTIME header of gzip file

The gzip header contains a MTIME part (Modification TIME).
http://www.zlib.org/rfc-gzip.html
Because is takes a little bit of time, from one fill_archive call to the next,
this part may differ and fail the test from time to time.
So we should not include that part in the comparison.

compare: refactor drop-down UI code

Don't rely on fake ref_names in the controller; instead put the code in
the HTML together with the other UI code.

auth: improve API key documentation for users

Add a warning about API key implications on the actual My Accounts/
API keys page where users are likely to see it.

No warning is added to the admin page equivalent, under the assumptions
that admins can be trusted to either know what API keys are (or at least
not mess around with them when editing other users), and thus don't need
the admonishment.

pullrequests: rename "as_form" to something more descriptive

This parameter is (only) used for showing the PR contents preview on the
"New Pull Request" page.

templates: addPermAction JS escaping bugfix (by eliminating expansion bugfix)

In 33b71a130b16, the addPermAction template was incorrectly escaped via
h.jshtml, where it should've been plain h.js.

Instead of merely fixing the escaping, refactor the code to completely
remove the need for escaping anything, by avoiding the template variable
expansion inside the JavaScript.

admin: apply LOWER() on journal filtering term for suffix/infix matching

Before this revision, LOWER() (via func.lower() of sqlalchemy) isn't
applied on journal filtering term for suffix and infix matching, even
though it is applied for prefix or immediate value matching.

This causes unexpected result with DBMS, which compares strings case-
sensitively by default.

This revision applies LOWER() on journal filtering term for
suffix/infix matching, to filter journal case-insensitively.

tests: add Jenkinsfile for automatic creation of Jenkins projects for testing Kallithea itself

This is the new way of how to build and run tests with jenkins.
It runs pylint and py.test with 4 different settings:
1. default
2. with German language settings
3. with MySQL DB
4. with PostgreSQL DB

A major advantage of this new Jenkinsfile approach is that Jenkins will be able
to automatically create new projects for each branch (or pull request) that
contains a jenkinsfile. There is thus no need for maintaining multiple jenkins
projects.

tests: add pytest-cov .coveragerc file

When using pytest-cov this file can be used to configure the coverage report.
See https://coverage.readthedocs.io/en/latest/config.html for details.
So when you start py.test like this

py.test --cov-config .coveragerc --cov=kallithea --cov-report xml

the settings in this file will be used.

tests: remove obsolete .travis.yml file

Travis has never been used for Kallithea and is unmaintained and probably
broken. For a starter, it requires hosting on GitHub...

pullrequests: introduce "action objects" for PR creation

Inspired by the command design pattern, this attempts the following:

* move policy and business logic from controllers into the model,
* move validation, authorization and execution logic closer together in
the code,
* establish a reusable pattern for modelling higher-level concepts
(like "create a new PR iteration"),
* make error handling more well-defined, and independent of the
controller layer, and
* provide clear separation between, one one hand, the validation and
authorization of a request, and on the other, the actual execution.

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)
&lt;script&gt;alert("It's a trap!");&lt;/script&gt;

2) Escaped for insertion into HTML element or attribute context

>>> print h.escape(s)
&lt;script&gt;alert(&#34;It&#39;s a trap!&#34;);&lt;/script&gt;

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.

style: use Bootstrap-ish nav-pills markup for settings menu

Based on work by Dominik Ruf.

style: fix help texts in Global Default Permissions

A display:block span inside form-inline flowed flawed.

Instead, put the help block as its own div.

pullrequests: pass around reviewer User objects, not IDs

This moves reviewer user validation into the controller and eliminates
the UserInvalidException.

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.

cleanup: drop superfluous Session.add calls

Session.add should only be called on newly created database objects.

Per the Kallithea contribution guidelines:

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 part of the session, and should not be added again.

cleanup: do Session.add directly inside _create_default_perms

Let the various _create_default_perms functions add the created object
to the SQLAlchemy database session, instead of having the caller do it.

This is in accordance with the Kallithea contribution guidelines:

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.

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.

css: additional fixes

The fix in 56bce741176f was incorrect, as it robbed the login dialog of
its panel styling. Using on .panel-primary instead of #main solves this.

Additionally, while the rules deleted in a90acf3a1422 were indeed unused,
it didn't actually solve the problem of select2 dropdowns being white on
white, due to a separate selector also causing this effect. This should
be fixed now.

cleanup: refer less to User.DEFAULT_USER

Down the road we might want to identify the default user in another
way than by username.

cleanup: use list comprehensions

It's often the same number of lines, but avoids introducing a needless
"result" variable, and makes the item expression stand out more clearly.

It's also a tiny bit faster, but the readability gains is what matters.

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.

auth: simplify repository group permission checks

In practice, Kallithea has the 'group.admin' permission imply the
'group.write' permission, which again implies 'group.read'.

This codifies this practice by replacing HasRepoGroupPermissionAny
"perm function" with the new HasRepoGroupLevel function, reducing the
risk of errors and saving quite a lot of typing.

auth: simplify repository permission checks

In practice, Kallithea has the 'repository.admin' permission imply the
'repository.write' permission, which again implies 'repository.read'.

This codifies/enforces this practice by replacing HasRepoPermissionAny
"perm function" with the new HasRepositoryLevel function, reducing the
risk of errors and saving quite a lot of typing.

repogroups: fix private repo recursion check

The purpose of this check is to ensure that we don't recursively assign
"default" user perms for a repo with the "private" flag set (because in
that case, the "default" user perms should always be "no access").

(The check, and this fix, is of course only applicable to Kallithea
instances that have anonymous access enabled to begin with.)

However, the check was only functional if the user was specified as a
username. This is apparently always the case when Kallithea is running,
but was not e.g. the case in the test suite, which consistently passed
a user ID instead of a username.

This commit ensures that the user is always resolved before the check is
made. There's no significant overhead to this, as the code immediately
calls RepoModel().grant_user_permission, which resolved the user anyway.
This change just moves the database lookup a bit earlier.

Fixing this revealed the matching test case to be broken, so it has been
fixed as well.

Down the road, we should eliminate Kallithea's bizarre practice of
passing around usernames and user IDs, in favor of passing actual User
objects. That'll get rid of mistakes like these, as well as repeated
needless database lookups.

comments: for PR comments, link directly to comment on PR page

Comments with approval changes made in the context of PRs also show up
on the individual PR changesets, and currently link to the (top of the)
PR page. If the user wants to see the surrounding comments, it can be
difficult to find the comment.

This change makes the link go to the comment anchor on the PR page, so
the user immediately sees the relevant comment thread. The PR metadata
is still readily available (by scrolling up or pressing "Home").

css: fix checkbox alignment

Checkboxes are a bit off-alignment throughout Kallithea, but floating
them left makes it noticably worse, and also seems like an odd thing
to do (checkboxes are usually rendered inline with the label).

css: don't force permission tables to 100% width

They become visually really awkward, with the None/Read/Write/Admin
columns spanning most of the table width.

css: fix redundant/overly broad rules

Remove rule coloring "#content div.panel div.panel-heading .pull-left a"
(and ditto .pull-right) white, as it caused the branch filter drop-down
box on the changelog page to become white-on-white.

I have not found anywhere this rule actually serves a purpose.

On several pages, the rules matched our standard grey buttons, but was
overruled by an !important style for .btn elements.

On repository group pages, it matches the repo group parent path link,
and in /_admin/journal, it matches the "Watched Repositories" and "My
Repositories" links; but all of these are already white from the
"#content div.panel div.panel-heading a" rule.

style: use more Bootstrap pull-left and pull-right

Based on work by Dominik Ruf.

style: introduce Bootstrap input-group - especially for repo summary pages

Based on work by Dominik Ruf.

style: introduce Bootstrap list-group and list-group-item

Based on work by Dominik Ruf.