Search for LDAP entry by both username and e-mail + large refactor #47
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It may be 'username', 'email' or any other LDAP attribute that a search operation is performed with.
What's previously been called properties is properly called attributes: https://www.ldap.com/glossary-of-ldap-terms Replace this misnomer everywhere - in code and docs. Squashed-in is a general reformatting and expansion of the README, including mention of a previously-undocumented `LDAP_SEARCH_FILTER` plugin configuration parameter. Add a reminder to use `pip3` if necessary (happened previously:
`services` already expects this to be available, although still under a misleading name of 'username'.
…BUTE` from `connector.login()`. `username` is used to create a database entry. Previously, this attribute would not be searched for if not specified explicitly in `SEARCH_ATTRIBUTE`. If the latter instead contained something like `mail` (which oftentimes _can_ change), that would be fed to `services.ldap_register()`, which would try to use it as a username. Only being able to get `username` if it's searched for with `SEARCH_ATTRIBUTE` is a bug. This could have also resulted in a new Taiga/Django user being created every time a LDAP user changes their e-mail address. Login form in default theme `taiga` reads "Username or email". These are also the two most common attributes being searched for. Therefore remove `SEARCH_ATTRIBUTE`, search by `USERNAME_ATTRIBUTE` and `EMAIL_ATTRIBUTE` instead. Also removed `SEARCH_ATTRIBUTE_SUFFIX` (it was a workaround to search by e-mail). Squashed: renamed `SEARCH_FILTER` -> `SEARCH_FILTER_ADDITIONAL` to reflect its purpose. Modified its syntax to require parentheses. Should allow more complex filters without additional tech debt. Squashed: don't mention password in error message (it hasn't been tried).
Attempting to upgrade locally. The backward-incompatible changes warrant this.
…'s password. This was a refactoring-introduced bug that prevented to log in any user, because the service account's password was used instead.
This was referenced Dec 1, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Most important changes:
PROPERTYtoATTRIBUTEin settings - that's the proper term;LDAP_SEARCH_SUFFIX- guess this was a workaround for searching e-mails;LDAP_SEARCH_PROPERTYand addLDAP_USERNAME_ATTRIBUTE;LDAP_{USERNAME,EMAIL}_ATTRIBUTEby default - login form says "Username or email";LDAP_SEARCH_FILTERtoLDAP_SEARCH_FILTER_ADDITIONAL, don't add parentheses around it in code - making the configuration look more filter-like, e.g.(|(&(a=*)(b=*))(c=x));Server/Connectionobject creation);Server/Connectioncalls in a singletry- this will simplifiy debug logging (not included in this PR) and non-wrapping exception handling (if needed in the future);0.2.0;Let me know if something needs clarification.
Fixes #37.
Independent of #46 (no merge conflict).
Conflicts with #48 (see there).