Since the formglobals variable is used only to populate the template context, just assign values directly to `c` instead. This does involve dropping the (very large) debug log entry listing all the formglobals.
AuthSettingsController: rename dictionary key for consistency
Rename 'auth_plugins_shortnames' to 'plugin_shortnames' for consistency with 'plugin_settings', and to emphasize that it's not one of the values retrieved from the database using get_auth_settings (all of which start with 'auth_').
In formglobals, the provided 'auth_plugins' default value is always replaced with the real database value, returned along with other plugin settings by Setting.get_auth_settings.
We can safely assume that the database value exists, since not only does db_manage initialize the 'auth_plugins' option upon database creation, but other authentication modules already assume its existence.
For instance, Settings.get_auth_plugins() is equivalent to
Settings.get_auth_settings()['auth_plugins']
except that get_auth_plugins will throw AttributeError, not KeyError, if the 'auth_plugins' key does not exist in the database.
log_in_user: extract user session setup from LoginController
The next changeset will need to set up a user login session outside LoginController. 'log_in_user' extracted and added as a top-level function in kallithea.lib.base since the code doesn't need access to the current controller.
Code refactored to take a User object instead of a username, to allow callers flexibility in how the user should be looked up.
Untangle session cookie authentication. If no session cookie is set, AuthUser constructor will be called with user_id set to None (the argument default value), and will never raise a UserCreationError. Thus that case can safely be moved to the end of _determine_auth_user and outside the try-except block.
If a session cookie *is* set, but we get a UserCreationError, we fall through to the "no cookie" case, which is also effectively the same behavior as before. (Not sure what circumstances, if any, can actually trigger a UserCreationError here, since the user is already logged in and - presumably - created, plus the user is identified by the Kallithea database ID, not user name, which would make it difficult to create a new user... but judging from the existing code, it's possible.)
Untangle API key authentication. Creating an AuthUser from an API key can leave the AuthUser authenticated or not, depending on key validity and Kallithea configuration; but either way, _determine_auth_user will not change this fact, and we can return early.
AuthUser: simplify check_ip_allowed and drop is_ip_allowed
check_ip_allowed is always called with user_id and inherit_from_default arguments taken from the same User/AuthUser object, so just take that object instead. This simplifies the is_ip_allowed method to the point where it can be removed.
pullrequest: re-add username when adding reviewers
Commit 90e982b6bca8 removed the username when adding reviewers, to align the way new and existing reviewers are displayed. However, when a user does not have a name set, the list of reviewers becomes useless unless the username is displayed too.
Some miscellaneous changes that didn't really fit in the previous patches: - clarification of variable names - removal of unnecessary underscore in MembersAutoComplete - avoid hardcoded div specifications
The owner field of a repository setting was supposed to be autocompletable, but never really did (at least not in Kallithea, probably it once did in Rhodecode).
Instead of making yet another 'OwnerAutoComplete', make a generic SimpleUserAutoComplete that can be reused in other places that only need completion of a text input field.
autocomplete: introduce function to set up basic autocomplete functionality
To set up an autocomplete instance, there is quite some boilerplate that is with minimal differences between instances. Move this to a helper function to reduce duplication, while still allowing each specific case to tweak behavior.
autocomplete: remove redundant specification of responseSchema
YAHOO.util.DataSource.responseSchema determines which fields from the data source are returned upon requests. In the case of user/group autocompletion, the data source is a static JavaScript array containing only the relevant fields. When not specifying a responseSchema, all fields of the selected entry are returned, which is fine (in fact, the responseSchema currently specified just mentions all fields, redundantly).
autocomplete: clean up handlers for itemSelectEvent
The handlers that execute when an autocompleted item is selected are really specific and cannot be factored out. Nevertheless, some cleanup:
- rationalize indentation - remove redundant copy/pasted comments - remove redundant group handling in MentionsAutoComplete - remove redundant checks on 'oData.nname != undefined', used to differentiate user vs. group completion, in places that only allow user autocompletion
The three blocks of autocomplete code are clearly copy/pasted from one another, with dead code remaining due to group-autocomplete not being relevant for some cases.
Unlike nosetest, pytest would show the database setup queries at the beginning of the test run, which don't bring added value. Hide them by disabling logging during this time.
Note that this does not provide an easy method of enabling the logging on demand (what could be done with the -s switch in nosetest).
i18n: synchronised translations from the development branch
* updated the translation template * updated translation for Hungarian * updated translation for Russian * updated translation for French * added translation for Belarusian * updated translation for German * updated translation for Dutch (Belgium)
tooltips: fix unsafe insertion of userdata into the DOM as html
This fixes js injection in the admin journal ... and probably also in other places.
Tooltips are used both with hardcoded strings (which is safe and simple) and with user provided strings wrapped in html formatting (which requires careful escaping before being put into the DOM as html). The templating will automatically take care of one level of escaping, but here it requires two levels to do it correctly ... and that was not always done correctly.
Instead, by default, just insert it into the DOM as text, not as html.
The few places where we know the tooltip contains safe html are handled specially - the element is given the safe-html-title class. That is the case in file annotation and in display of tip revision in repo lists.
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.
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
The notifications tests make the assumption that there are no notifications at the start of the test, which is explicitly asserted.
However, any other test, like the one introduced in commit 9a23b444a7fe (pullrequests: detect invalid reviewers and raise HTTPBadRequest), could add new notifications to the database, and thus fail these assertions.
Just like the notifications tests already cleaned all notifications at the end of the test (tearDown), make sure to clean them at the start (setUp) too.
pullrequest: when adding a reviewer, show full name only
Existing reviewers are shown with full name (Firstname Lastname) without username. In symmetry, newly added reviewers should be displayed the same way.
pullrequsts: really create a comment when creating a PR and setting status to 'under review'
14d75d4b03cd changed the pullrequest creation status change comment text from redundant blurb to just an empty string. Empty comments are however only created if changing the status ... and in this case we didn't tell the comment creator that it actually was a status change.
As a result of this, we ended up breaking the implicit invariant that all status updates have a comment. That showed up as errors dereferencing None when displaying changesets.
compare: backout 51c4d2e74898 to fix ancestor calculation to empty repository
This case can not just be simplified as I thought it could.
Mercurial 3.4 has d2de20e1451f 'revset: extend fullreposet to make "null" revision magically appears in set' Which makes ancestor(id(0000000000),1) 'correctly' return the null revision as ancestor.
We can however not rely on that when supporting Mercurial 3.3.
comments: remove dysfunctional comment bubble on compare and file views (issue #84)
Several compare and file views would show a comment bubble when hovering code lines, even though clicking on this bubble would not do anything.
Instead, make sure that the bubble only appears when the diff block is in a class 'commentable-diff', which can be set from the appropriate places (changeset and pullrequest views) where a click handler is also attached.
routing: remove dysfunctional route to pullrequest copy_update
Commit d42d7b2a3b2fec0cbf8380a22f4dd96fc836f5c5 refactored the pullrequest update functionality, making the route 'pullrequest_copy_update' redundant, but forgot to remove that route.
pullrequests: detect invalid reviewers and raise HTTPBadRequest
Normally, the creation of a pullrequest with invalid reviewers is not possible because the list of reviewers is populated from a form element that only shows valid reviewers. However, if creating a pullrequest through an API call, invalid reviewers can be specified but would not be detected. The reviewer would be encoded in the database as 'NULL'/None, and opening such a pull request would cause a server error.
Instead, detect invalid reviewers at pullrequest creation/update time and raise HTTPBadRequest.
changeset: reduce log level of stack trace on innocent exceptions
When the user performs an unallowed action and a flash is displayed, there is no need to log the stack trace at 'error' level. Reduce the stack trace log to debug instead.
The letter l and the number 1 are very similar in the Consolas font, unlike in Lucida Console (available on almost all Windows versions [1]) where the difference is more obvious.
In identifiers like foo_Vl_bar_V1 a clear visual difference is important during reviews.
diffs: avoid conflicts between inline diff mechanism and special markup
It would sometimes emit markup like <pre><ins><u</ins> <ins>class</ins><ins>=</ins><ins>"cr</ins><ins>"></u></ins></pre> instead of <pre><ins><u class="cr"></u></ins></pre>
helpers: make desc_stylize work when given html escaped strings
The function returns strings with html markup. The result can thus not be escaped and we must assume that the input already has been escaped. That may or may not the case yet.
privacy: on password reset, don't tell strangers if email is valid or not
Password reset form might be used to check if users with specific email addresses have accounts in the system by requesting their password to be reset. It's probably not a good idea to give this sort of information to complete strangers.
users: add extra checks on editing the default user
There is no need to be able to edit e-mails or permissions of the default user, so add the same checks as present in many other methods in the users controller.
Note that one specific unittest has been commented because it relies on pytest features (monkeypatch). When pytest is the default test runner, the test should be uncommented.
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).
pullrequest/compare: add logical changeset index to clarify the order
Is the parent-most changeset in a changeset the one at the top or at the bottom? When the revision numbers are not shown, it is not obvious to determine this.
This commit adds a logical changeset index to the commit list in a pullrequest or compare view. The index starts at 1 (the parent-most commit) and has no relation whatsoever with the commit hash or revision number.
select2: move "exact prefix matches" to the top of the search
Further improvements to this could be to sort by the position of your filter in the results so searching for foo means that release/foo comes before a/branch/of/doom//foo .
issue: when deleting comments in a list of comments on the same line, sometimes the add new comment button stops working
root cause: when deleting a comment in a list of comments that are all on the same line, the wrong previous tr is chosen, the chosen tr could be an inline comments instead of a line tr, resulting in the fact that injectinlineform function will return immediatly
solution: loop over the found tr until its no longer an inline comment
remark: could probably be optimised futher to immediatly search for the line tr object
The Last Revision column of the file browser linked to URLs with the changeset as: r14:abcdef0123 which is not a valid changeset id. Instead, use .raw_id.