services: slugify before DB lookup #48
Conversation
In `ldap_register()`, LDAP binding has already been performed, and the user is allowed to authenticate. However, the current logic is to look up the user entry in local DB using the string from the login form. If the latter fails, creating a new user is attempted - which might have been already created! This still does not allow special characters (as in ensky#43, which can be unsafe), but does address the underlying issue. Should fix ensky#15, ensky#17, ensky#45, and make ensky#43 obsolete.
veox
changed the title
|
This does not seem to work for me. It looks like slugify_uniquely() creates a unique instance of an object for a model. Meaning it ads "-1" or probably something similar when the name exists for the model (user_model). peter.streef becomes peterstreef the first time. but the second it becomes peterstreef-1. which does not exist (when getting) so it still tries to create a new one. As a fix I now just remove the known characters in the ldap server ("." and "-") with a replace from the username. |
|
Looks like I opened this PR without sufficient testing. :( Thanks @pstreef - I'll close this and reopen if fixed. |
|
@pstreef Well, there seems to be a general disconnect between LDAP and Taiga in this case, arising from the need to map usernames, determined by the LDAP schema The best I've come up with is a "pre-slugify" It's different from your approach in constraning at the time of LDAP registration. I won't be submitting a PR for this, seeing how development here is stalled. I've moved my own development to the organisation that I'm doing this work for instead (if relevant, see link above). |
|
Slept on it. Understood that this might as well just be a config option, e.g. |
In
ldap_register(), LDAP binding has already been performed, and the user is allowed to authenticate. However, the current logic is to look up the user entry in local DB using the string from the login form. If the latter fails, creating a new user is attempted - which might have been already created!This still does not allow special characters (as in #43, which can be unsafe), but does address the underlying issue.
Should fix #15, #17, #45, and make #43 obsolete.
This PR conflicts with #47 due to whitespace changes in the latter. If either gets merged first, I can rebase the other one.