-
Notifications
You must be signed in to change notification settings - Fork 671
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LDAP login fails, resulting in incorrect user permissions until the next sync #920
Comments
Hi Rob, Thanks for the report. If you can build from source then I would say that directly debugging the authentication code is the way to go. Another perhaps useful option would be to enable UnboundID (LDAP library) logging. If you startup Gitblit with some -D VM args then you should be able to enable this:
You'll have to play with the log levels. |
After several days of off and on debugging, we've discovered that we can private void getTeamsFromLdap(LDAPConnection ldapConnection, String Before this change, sync() worked fine, but authenticate() failed. After Obviously, there is something going on with the state of the LDAP Any insights? Is there still some way to work around this without a code BTW, we've beat our heads on this problem for a while because GitBlit Rob On Fri, Sep 18, 2015, 12:54 PM James Moger notifications@github.com wrote:
|
Sorry Rob, I'm not an LDAP expert. It does seem suspicious that reusing the connection fails. When you get a fresh connection, the connection is either bound to fixed credentials specified in the config or is not bound at all - depends on your config. When you re-use the connection from authenticate, the connection is bound to the logging-in user, I think. I'm sure you already know that but I'm wondering if that detail is significant here. |
Your comment about the rebinding was very helpful. There is a powerful At any rate, I don't see any way to work around without a code change. I see two ways of approaching it:
If you agree, is this something I can change then make a pull request? Rob On Thu, Sep 24, 2015 at 10:08 PM, James Moger notifications@github.com
|
Hi Rob, I'm not that familiar with LDAP but have successfully configured GitBlit via an LDAP service with 20k+ users. Whilst setting that up I found the Active Directory Explorer tool very useful for debugging (https://technet.microsoft.com/en-us/library/bb963907.aspx). A few things I notice with your config though I'm not sure how much is due to different LDAP structures:
As I don't have admin access to LDAP to manage teams I simply use OU's which thankfully are granular enough for my requirements. It does sound a bit odd for a user not to be able to list the teams it is a member of, could it be perhaps the user doesn't have access to the particular AD path defined such as HTH Paul |
Hi, i think this is the same issue as #247 |
Hi Rob, a PR would be nice. No one is actively working on this issue. |
We are experiencing the issue at a large enterprise client where the LDAP user does not have the permission to view LDAP groups. Go figure. Yes I am serious... The LDAP connection would need to be an elevated user that can query groups. |
We are also experiencing the same issue. The normal LDAP users are not allowed to list all LDAP groups while the user from the configuration file is allowed to do so. Any chance for a fix? Building gitblit on our own just for that does not seem to be a good solution at all. @gitblit Would you accept a new pull request of #247 for the develop branch? |
@kellerkindt My recollection is the proposed fix solved one problem while creating a new one. The new problem was either backwards-incompatibility for config or unexpected behavior changes or both - can't recall. If you can propose a fix that doesn't break others then I'm all for it. |
I, too, have that same problem now. From what I see this has come up a few times now. |
…cenarios. 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-org#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. This commit would fix gitblit-org#920
Hi everyone! |
We are using LDAP for authentication. When a user logs in, an error is logged and the user seems to be logged in correctly, but his permissions are not correct until the next LDAP sync.
We've debugged the code to line 523 in class LDAPAuthProvider. When we take the LDAP arguments (base, filter, attributes) from that point and manually make the LDAP call using Softerra LDAP browser, the manual call works fine.
The production instance in Linux, but I can reproduce on a machine running Windows 7, GitBlit go version 1.6.2.
Our next step will be to try to build from source and reproduce. We'd appreciate any suggestions about what might be wrong or how best to pinpoint the problem.
Rob Conaway
The following is the (redacted) log message:
Here is the LDAP portion of the gitblit.properties
The text was updated successfully, but these errors were encountered: