-
-
reviewboard/accounts/backends.py (Diff revision 1) Can you format this like:
uidattr = '...' % { 'userattr': ..., 'username': ..., } -
reviewboard/accounts/backends.py (Diff revision 1) No need for parens around
uidattr, or the semicolon. -
-
reviewboard/accounts/forms.py (Diff revision 1) 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.
Improve LDAP user lookups
Review Request #5203 — Created Jan. 6, 2014 and submitted
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/J6W1o9Eb2IYSome 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': ..., } |
 chipx86
|
|
|
No need for parens around uidattr, or the semicolon. |
chipx86
|
|
|
Should have a trailing period. |
chipx86
|
|
|
I know this was like this earlier, but I believe you can add <tt>...</tt> around the example strings, and it … |
chipx86
|
|
|
Can you compress this a bit with the wrapping? search_result = ldapo.search_s(userdn, ldap.SCOPE_BASE) |
 david
|
|
|
These should fit on one line. |
david
|
Change Summary:
Made style corrections as indicated by earlier review.
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
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 2 (+66 -31) |
-
Just a couple formatting comments:
-
reviewboard/accounts/backends.py (Diff revision 2) Can you compress this a bit with the wrapping?
search_result = ldapo.search_s(userdn, ldap.SCOPE_BASE) -