Florian Zschocke [Wed, 23 Nov 2016 01:59:39 +0000 (02:59 +0100)]
Extract LdapConnection into new class from LdapAuthProvider
Extract the inner class `LdapConnection` from the `LdapAuthProvider`
into a separate class, so that it can be used from multiple classes
that have to connect to an LDAP directory.
The new class is placed into the new package `com.gitblit.ldap`, since
it isn't specific to authentication.
Florian Zschocke [Wed, 23 Nov 2016 01:48:38 +0000 (02:48 +0100)]
Create base unit test class for LDAP tests.
Extract the creation of the in-memory servers and the interceptor
code to a base class that LDAP related unit tests can extend to
have the servers available.
Florian Zschocke [Fri, 18 Nov 2016 19:26:06 +0000 (20:26 +0100)]
Set "can admin" permission on LDAP users and teams correctly
The canAdmin permission is set on a LDAP user, when the user is listed
in `realm.ldap.admins` or is a member of a team listed in `realm.ldap.admins`.
This leads to inconsistent and surprising behaviour on the EditUser page
when clicking the "can admin" checkbox. Also, the "can admin" checkbox
is disabled, but not checked, for teams that are listed as admin teams.
The new behaviour implemented in this patch makes users and teams from
LDAP match local ones. That means:
* LDAP teams that are listed in `realm.ldap.admins` get the canAdmin
property set if teams are maintained in LDAP.
* LDAP users that are listed in `realm.ldap.admins` get the canAdmin
property set if teams are maintained in LDAP.
* LDAP users do not get the canAdmin property set, if they are only a
member of a team listed in `realm.ldap.admins`.
* The `supportsRoleChanges` method for users and teams of the
`LdapAuthProvider` unconditially returns false if teams are
maintained in LDAP, not only for users and teams listed in
`realm.ldap.admins`.
* Therefore, for all LDAP users and teams the "can admin" checkbox
is always disabled if teams are maintained in LDAP.
Florian Zschocke [Fri, 11 Nov 2016 18:22:17 +0000 (19:22 +0100)]
Clean up `LdapAuthProvider` to properly cover different LDAP search scenarios.
Gitblit allows in its configuration to set a "manager" user (and password) which can be used
to search for the entry of a user wanting to log in. If they are both not set, an anonymous search
is attempted. In the description below, when I say "...as manager", it is either as manager or
anonymous.
So far the behaviour of Gitblit, with respect to binding to and searching in LDAP,
has been the following when a user logs in:
**bind as manager**
**search for the user**
_bind as the user_
_search for the teams_
I'll call this code flow A.
Later an additional configuration option had been added: `realm.ldap.bindpattern`.
(PR gitblit/gitblit#162) It was meant to allow for not using a manager nor anonymous binds,
by searching the directory as the user logging in.
This is done in code flow B:
**bind as manager**
_bind as user_
_search for user_
_search for teams_
Both A and B are flawed, I think. In A, it looks like a mistake to me that the binding stays with the
user after authentication. The problem that this causes is, that in LDAP server configurations
where normal users are not allowed to read groups, the team information cannot be retrieved.
I tried but failed to understand how B is supposed to work. There will always be a bind request
as either anonymous or the manager DN when the LDAP connection is created. If neither is
possible, the authentication process will fail and the user cannot log in.
When synchronizing users and teams from LDAP, the following code flow is exercised:
F:
**bind as manager**
**search for users**
**search for teams**
This patch fixes both code flows by introducing a new flow.
C:
**bind as manager**
**search for user**
_bind as user to authenticate_
**bind as manager**
**search for teams**
And it changes code flow B to the following code flow D:
_bind as user_
_search for user_
_search for teams_
With code flows A, C, D and F the following usage (and authentication) scenarios are covered.
They are described from the view of a Gitblit administrator's intent and his LDAP setup.
* Users and team should be snychronized with LDAP
This means anonymous or a fixed account must be able to read users and groups.
=> covered by C and F
As the above allows for authentication and is required for synchronisation, all the others below
do not cover synchronization.
* No anonymous binding allowed and no special manager binding required
This means that users must be able to read user an group entries.
=> covered by D
* The user DN needs to be searched, e.g. because they are not all under the same parent DN.
This means that anonymous or a fixed account must be able to read users.
-- anonymous or the "manager" account can also read groups
=> covered by C
-- anonymous or the "manager" account cannot read groups but a user can
=> covered by A
I therefore believe that the new code will cover all common use cases. The implementation
either directly binds as the user, when `bindpattern` is not empty, or it binds anonymous or
against the manger DN to search for the user DN entry.
If it directly bound against the user DN, the user is already authenticated. It will then only check
that the user DN it found in the search is identical to the one it is currently bound against. If it
was bound against a manager DN (or anonymously) it will bind against the found user DN to
authenticate the user logging in, and will then rebind against the manager DN.
When searching for groups in LDAP, if the search fails with a result code other than SUCCESS,
the implementation will bind against the user DN, if it isn't already bound against it. It will then
repeat the search for groups under the user authorization. This is to keep backwards
compatible with the original behaviour A, in order to not break cases where the LDAP setup
would deny a manager account to search for groups but allow it for normal users.
To achieve this the implementation introduces an internal `LdapConnection` class that wraps
the connection and keeps bind state, so that a rebind as a user is possible.
This also fixes a resource leak where the connection was not closed in case that the initial bind
as the manager account did not succeed.
Extend LDAP tests to use LDAP servers with access restrictions.
Add access restrictions to the LDAP test server instances.
New modes used a test parameters are ANONYMOUS, DS_MANAGER and USR_MANAGER.
ANONYMOUS can bind anonymously and access users and groups.
In DS_MANAGER the server requires authentication and will only allow
the DIRECTORY_MANAGER user to search for users and groups.
In USR_MANAGER only the user can search groups, the USER_MANAGER, which
is used to bind in this mode, can not.
A third server instance is created because I did fear side effects should
the tests be run in parallel, had I tried to configure the access
restriction in Before.
Extend LDAP authentication tests to use different modes.
Instantiate two LDAP servers, one that allows anonymous access, and
one that requires authentication for all operations.
The JUnit test is parameterized to run all tests with both instances.
It uses different settings for each mode.
Paul Martin [Wed, 27 Apr 2016 22:58:06 +0000 (23:58 +0100)]
Ticket Reference handling #1048
+ Supports referencing:
+ Tickets from other tickets via comments
+ Tickets from commits on any branch
+ Common TicketLink class used for both commits and tickets
+ TicketLink is temporary and persisted to ticket as a Reference
+ Support deletion of ticket references
+ Rebasing patchsets/branches will generate new references
+ Deleting old patchsets/branches will remove the relevant references
+ Substantial testing of use cases
+ With and without patchsets, deleting, amending
+ BranchTicketService used during testing to allow end-to-end ref testing
+ Relocated common git helper functions to JGitUtils
Paul Martin [Mon, 11 Jan 2016 12:25:06 +0000 (12:25 +0000)]
Document edit capability via ProseMirror submodule #974
+ New docEdit page with links from docPage and docList
+ Bespoke menu system with full screen edit mode
+ npm required for building client side scripts
+ Ant script added for BuildUI which performs npm commands
+ Update font-awesome to 4.5.0
+ Factor out to JGitUtils common code in BranchTicketService for EditFilePage
+ getTreeEntries
+ commitIndex
+ Merge capability for document editing
Dariusz Bywalec [Mon, 4 Jan 2016 10:32:58 +0000 (11:32 +0100)]
Fix authentication failure warning log messages for FEDERATION_USER
The AuthenticationManager did not encounter for FEDERATION_USER and would unnecessarily
generate a lot of failure warning log messages, e.g:
Failed login attempt for $gitblit, invalid credentials from XXX.XX.XX.XX
A simple condition will prematurely return null bypassing the regular authentication path
and immediately make the authentication be routed via FederationManager.
Paul Martin [Fri, 25 Dec 2015 22:35:11 +0000 (22:35 +0000)]
Fix for #976 - Filestore links via browser
+ GitLFS client support
+ FilestoreModel now parses meta file
+ Read meta heading from cache if available
+ Authentication based on accept headers for browser view filestore login
+ PathModel & PathChangeModel now understands filestore items
+ Zip & Rar downloads contain include filestore items
+ Filestore servlet returns LFS JSON error only if accepted by client
+ DiffStat now knows repository to allow identification of filestore items
+ Filestore items identified and returned via view, raw & blob links on
blame, commitDiff, commit and Tree pages
Paul Martin [Wed, 9 Dec 2015 17:58:15 +0000 (17:58 +0000)]
fix for #978 - HTML5 date input support
+ JS patch/hack to coerce legacy wicket into talking to a HTML5 input type
+ JS script to hide inline help on date format when using HTML5 date picker
+ Date picker shown in user locale and standard does not support custom
format.
+ Always sent in ISO8601 format
Joel Johnson [Tue, 7 Jul 2015 22:31:39 +0000 (16:31 -0600)]
remove external account type in lieu of specific type
This was unused and causing provider lookup to fail in
AuthenticationManager.findProvider() by changing it out
from underneath. As a result, the supportXChanges methods
weren't being reported correctly.