Improve LDAP user lookups

Review Request #5203 — Created Jan. 6, 2014 and submitted

Information

Review Board
master

Reviewers

Improve LDAP user lookups

This patch simplifies configuration of user lookups in LDAP and
also provides better help text to aid the administrator setting it
up.

The email thread below has more details:
https://groups.google.com/forum/#!topic/reviewboard-dev/J6W1o9Eb2IY

Some additional minor changes:
* I reflowed the anonymous bind and service account bind sections
so that they always return a user dn which is authenticated in
a common location.
* I dropped the useless fallback of joining the username with the
base DN. It was unlikely to ever work in a real-world LDAP
environment and it made the code confusing.
* Added a debug log message indicating the DN that we will
attempt to authenticate against.
* Extended the get_or_create_user() routine to take the user DN
Since we already have this information above, it doesn't make
sense to search a second time. We'll just perform a BASE search
instead.
* Corrected a typo in the help text of the Full Name Attribute

Tested authenticating using TLS with and without using an LDAP service account to perform the search.

Description From Last Updated

Can you format this like: uidattr = '...' % { 'userattr': ..., 'username': ..., }

chipx86chipx86

No need for parens around uidattr, or the semicolon.

chipx86chipx86

Should have a trailing period.

chipx86chipx86

I know this was like this earlier, but I believe you can add <tt>...</tt> around the example strings, and it …

chipx86chipx86

Can you compress this a bit with the wrapping? search_result = ldapo.search_s(userdn, ldap.SCOPE_BASE)

daviddavid

These should fit on one line.

daviddavid
chipx86
  1. Just small stylistic nits, but overall this looks much more clear.

  2. reviewboard/accounts/backends.py (Diff revision 1)
     
     
     
     
    Show all issues

    Can you format this like:

    uidattr = '...' % {
        'userattr': ...,
        'username': ...,
    }
    
  3. reviewboard/accounts/backends.py (Diff revision 1)
     
     
    Show all issues

    No need for parens around uidattr, or the semicolon.

  4. reviewboard/accounts/forms.py (Diff revision 1)
     
     
    Show all issues

    Should have a trailing period.

  5. reviewboard/accounts/forms.py (Diff revision 1)
     
     
    Show all issues

    I know this was like this earlier, but I believe you can add <tt>...</tt> around the example strings, and it should render those parts a little more clearer.

  6. 
      
sgallagh
david
  1. Just a couple formatting comments:

  2. reviewboard/accounts/backends.py (Diff revision 2)
     
     
     
     
    Show all issues

    Can you compress this a bit with the wrapping?

    search_result = ldapo.search_s(userdn,
                                   ldap.SCOPE_BASE)
    
  3. reviewboard/accounts/backends.py (Diff revision 2)
     
     
     
    Show all issues

    These should fit on one line.

  4. 
      
sgallagh
david
  1. Ship It!
  2. 
      
sgallagh
Review request changed
Status:
Completed
Change Summary:
Pushed to master (b9ebaed). Thanks!