Skip to content

add support for Active Directory sAMAccountName as username and login

username-removed-27992 requested to merge robert.brendler/gitlab-ce:master into master

I had problems with authenticating to Active Directory LDAP with following settings, as described here http://blog.dimaj.net/content/updated-howto-enable-ldap-filters-gitlab

  ## LDAP settings
  ldap:
    enabled: true
    host: '192.168.x.xx'
    port: 389
    uid: 'sAMAccountName'
    method: 'plain'
    bind_dn: 'CN=Automatic,CN=Users,DC=mycompany,DC=de'
    password: 'pa$$word'
    allow_username_or_email_login: true
    base: 'CN=Users,DC=iteratec,DC=de'
    user_filter: '(&(sAMAccountName=%{username})(memberOf=CN=Developers,CN=Users,DC=mycompany,DC=de))'

I found following problems: There are two ways the current mechanism authenticates to LDAP. Both construct the LDAP filter from the user_filter. The one in oauth/user.rb works fine, cause username can be resolved to the member of this user instance executing the filter.construct(). In the second one in adapter.rb this won't work, an exception is thrown but not caught and you see "Access denied for your LDAP account."

The former does successfully create a User on first login in application_controller.rb :authenticate_user! filter , the latter fails in :ldap_security_check filter so the just created user cannot log in.

Another problem as, I think, is in user.rb where the user name is set to a part before the email's @. Which means the filter will replace something that is generated in gitlab. In our case we do not have a field in Active Directory that would match this. So I introduced sAMAccountName as the value for username if it is present, which would leave the current OpenLDAP algorithm as is. As I do not have an OpenLDAP instance to test against, please verify before merging.

In general I tried to be as least invasive as possible, but this was my first ruby hack, so please review if possible.

Thanks Rob

Merge request reports