This fixes a vulnerability that allowed logged-in users to edit or delete open pull requests associated with any repository to which they had read access, plus a related vulnerability allowing logged-in users to delete any comment from any repository, provided they could determine the comment ID and had read access to just one repository.
tests: finally block also needs to be executed if api_call itself fails
The cleanup code (finally block) should be executed as soon as something what changes the db fails.
I don't remember which one but one of these tests failed and caused all the others to fail as well because the database wasn't cleaned up after the first failure. That made finding the root cause more difficult then it should have been.
pytest migration: model: convert all tests to TestControllerPytest
The model tests were based on BaseTestCase which does not exist currently for pytest-style tests. Nevertheless, there seems to be no advantage of directly subclassing BaseTestCase over subclassing TestControllerPytest. Thus, keep things simple and use TestControllerPytest.
pytest migration: simplify hg+git test class hierarchies
No need to use multiple inheritance when single inheritance can do it just as well. Let the git/hg test cases just derive from _BaseTestCase which in turn derives from TestControllerPytest.
Also remove empty setup_class/teardown_class methods.
pytest migration: convert functional tests with setup/teardown methods
pytest also supports setup/teardown methods like unittest (even though pytest fixtures are more powerful and should be the end goal). Only difference is the naming and signature of setUp (setup_method) and tearDown (teardown_method).
test_admin_notifications: fix index test dependency
The index test only worked because another test had prepared some database state it relied on.
More specifically, the index test creates a notification as a newly created user, but that user had not been committed to the database yet. When running the index test standalone, this causes the error:
tests: fix test_nodes.py and test_files.py which fail on windows due to mimetype differences
The mimetype of files may differ on different platforms (e.g. the mimetype of .py files on windows is not text/x-python but text/plain). Therefore, compare the mimetype to the value the platform uses.
* Recommend setting global Apache config WSGIRestrictEmbedded On * Don't set the default value for processes=1 - it has side effects * Use python-home instead of python-path * Fix confusing mentioning of root and clarify the use of specifying user and group * Include WSGIProcessGroup in configuration excerpt
docs: add notes about IIS, Windows Authentication and Mercurial
(The original patch from Konstantin has been heavily copyedited and modified by Mads Kiilerich but is still [based on] Konstantin's feedback and contribution.)
vcs: fix processing of git commands that return output on stderr
The subprocessio module used for interfacing with Git is using threading. It might not be fully deterministic.
The subprocessio module also had the "feature" that it stopped reading and reported failure once it found output on stderr - even if the command completed (or would complete) with success.
Couldn't run git command (['git', '-c', 'core.quotepath=false', 'checkout', 'foobranch']). Original error was: Subprocess exited due to an error: Switched to branch 'foobranch'
On Windows it would fail even more often.
To fix that, ignore stderr while processing output. There is a risk that it in some cases can make the process block on stderr and thus never finish ... but there is no reports of that yet.
db: cache SCM instance short-term (tied to SQLAlchemy session lifetime)
Repeatedly checking whether SCM instances are invalidated is slow, and we don't actually _want_ SCM instances to invalidate half-way through a request either.
Therefore cache them in on the db.Repository object, the lifetime of which is directly tied to the lifetime of the SQLAlchemy session, the lifetime of which is tied directly to the individual HTTP request. This way, we only check for invalidation the first time the SCM instance is accessed in a request.
This will improve performance in cases where we have (by definition) badly written code that retrieves repo objects several times.
pullrequests: optimize iteration over reviewers - avoid fetching users one by one
.reviewers was mainly used for iteration and then dereferencing .user one by one. That gave lots of queries and round trips to the database and was slow.
Instead, do something else. Either query directly or use a new method for getting the list of reviewer users.
Reviewers will explicitly be shown in the order they have been added (assuming database id's are monotonic).
This method is not referenced anywhere in the Kallithea or Paste code. Additionally, calling it with any true-ish `log` argument would trigger a TypeError exception.
python-pam 1.8.* provides a new API replacing the previously available authenticate() function. The latest unreleased version brings it back, but until it's available, use a custom shim instead.
tests: clarify user IP range test dependency on beaker caching of user objects
Requests with invalid request address would pass after configuring user IP ranges because the IP range would not be validated as long as the user object was found in the beaker cache.
Instead, wait until the beaker cache has expired and verify the user cannot log in without a valid IP. Then provide a valid IP for later requests until the IP range is removed again.
Based on original patch and research by Dominik Ruf.
graph: detect git branches and colourise them properly without rainbow effect (Issue #188)
This is based on research and patches by Andrew Shadura.
When using Git, only commits pointed to by branch references have their branches set. In general, there's no way in Git to find out which branch a commit belongs to, so we can try to deduce that from the merge topology.
In other words, if we know the branch name, we know it's a different colour than any different branch. If we don't (it is None), we guesstimate the first parent is probably on the same branch. The relevant part of the code before 2da0dc09 used a similar, yet simpler, algorithm.
graph: avoid collision between "heir has been found" and "child has no name" both using None as branch name
Instead of using None for both, introduce a new flag for searching for heir.
This will do that Git changesets without branch name will inherit colors from each other instead of getting a new one every time. (Inheriting from from changesets with branch name is still phony.)
summary: add tooltip and link to status change icons (same as on changelog page)
This change makes the changeset list on the summary page more similar to the one on the changeset list. It adds the same tooltip ("Changeset status") to the changeset status change icons that is already found on the changelog page. It also adds a link to the first comment (as in the changelog page).
pullrequest: add URL changesets in txt notification email to reviewers
Similar to adding the URL for each changeset in the html notification email (commit xxxxxxxxxxxx) do the same for txt emails. The e-mail client presumably makes these URLs clickable.
vcs: better handling of invalid email addresses: don't consider them email addresses
13da89053853 was in principle right in always returning email adresses as string ... but unfortunately the function also returned invalid email addresses that didn't fit into strings.
To fix this, the function is refactored to always use regexp matching of valid email addresses ... and to be simpler. The behaviour should be the same as before for all valid email addresses.
pytest migration: files: convert to TestControllerPytest and parametrize
Migrate the 'files' tests to pytest and its parametrize decorator. The syntax of this decorator is slightly different: a first argument is a comma-separated string with the method argument names for which the values are provided as second argument.
pytest migration: make pytest's parametrize functionality available
To provide parameterized tests, a custom implementation is currently provided at kallithea/tests/parameterized.py because nose does not provide that out-of-the-box. pytest, on the other hand, does have a built-in 'parametrize' (note: different spelling) functionality. Therefore, once all tests have been converted to pytest, we can get rid of the custom 'parameterized' implementation. Also, the existing 'parameterized' implementation does not seem to work under pytest-style tests.
This commit makes pytest's 'parametrize' decorator available alongside the custom 'parameterized' decorator. The names are confusing but it is the intention to kill the original 'parameterized' soon.
pytest migration: convert simple functional tests to TestControllerPytest
Replace usage of TestController with TestControllerPytest for those files in tests/functional where there is no setUp/tearDown method (the pytest equivalent to be investigated) and that do not use test parametrization.
pytest migration: search: use tmpdir fixture instead of 'current dir'
The test_empty_search method needs to refer to an existing directory that is guaranteed to not contain a search index. The current implementation chose the 'current' directory '.' but it is more of a hack than careful planning. A temporary empty directory would be cleaner but was more involved to create.
With the introduction of pytest-style test classes, this can very easily be improved. Creating a temporary directory with pytest is as simple as accepting the magic 'tmpdir' argument to the test method. This 'tmpdir' is a fixture provided by pytest. The variable is initialized with a py.path.local object referring a temporary directory. For details, see: http://pytest.org/latest/tmpdir.html
In order to allow tests to benefit from pytest specific functionality, like fixtures, they can no longer derive from unittest.TestCase. What's more, while they can derive from any user-defined class, none of the classes involved (the test class itself nor any of the base classes) can have an __init__ method.
Converting all tests from unittest-style to pytest-style in one commit is not realistic. Hence, a more gradual approach is needed.
Most existing test classes derive from TestController, which in turn derives from BaseTestCase, which derives from unittest.TestCase. Some test classes derive directly from BaseTestCase. Supporting both unittest-style and pytest-style from TestController directly is not possible: pytest-style _cannot_ and unittest-style _must_ derive from unittest.TestCase. Thus, in any case, an extra level in the class hierarchy is needed (TestController deriving from Foo and from unittest.TestCase; pytest-style test classes would then directly derive from Foo).
The requirement that pytest-style test classes cannot have an __init__ method anywhere in the class hierarchy imposes another restriction that makes it difficult to support both unittest-style and pytest-style test classes with one class. Any init code needs to be placed in another method than __init__ and be called explicitly when the test class is initialized. For unittest-style test classes this would naturally be done with a setupClass method, but several test classes already use that. Thus, there would need to be explicit 'super' calls from the test classes. This is technically possible but not very nice.
A more transparent approach (from the existing test classes point of view), implemented by this patch, works as follows: - the implementation of the existing TestController class is now put under a new class BaseTestController. To accomodate pytest, the __init__ method is renamed init. - contrary to the original TestController, BaseTestController does not derive from BaseTestCase (and neither from unittest.TestCase). Instead, the 'new' TestController derives both from BaseTestCase, which is untouched, and from BaseTestController. - TestController has an __init__ method that calls the base classes' __init__ methods and the renamed 'init' method of BaseTestController. - a new class TestControllerPytest is introduced that derives from BaseTestController but not from BaseTestCase. It uses a pytest fixture to automatically call the setup functionality previously provided by BaseTestCase and also calls 'init' on BaseTestController. This means a little code duplication but is hard to avoid.
The app setup fixture is scoped on the test method, which means that the app is recreated for every test (unlike for the unittest-style tests where the app is created per test class). This has the advantage of detecting current inter-test dependencies and thus improve the health of our test suite. This in turn is one step closer to allowing parallel test execution.
The unittest-style assert methods (assertEqual, assertIn, ...) do not exist for pytest-style tests. To avoid having to change all existing test cases upfront, provide transitional implementations of these methods. The conversion of the unittest asserts to the pytest/python asserts can happen gradually over time.
Test test_index_trending depended on previous actions performed by the test test_index_statistics. Such inter-test dependencies should not be present.
This would become a problem when converting the test class to pytest-style, because the app is recreated for each test, while the original TestController creates it once per class.
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.
pytest migration: use pytest test skipping instead of nose's
While the nose test skipping still worked, we want to remove all remnants of nose, so switch to the pytest equivalent.
Note: due to a bug in pytest-sugar (reported at [1]) the skipped tests are not shown explicitly when using the 'skipif' approach, while they were with the nose approach. Since pytest-sugar is not enabled yet in setup.py, this is not deemed a problem.
tests: gists: comment out always-skipped test of put functionality
There is little sense in always skipping a test because the tested functionality is not implemented. Instead, comment out the test and leave it to the future implementer to re-enable it.