Add LDAP attribute mapping env variables. #344
Conversation
Specifically made sure that LDAP auth works the same if the new environment variables aren't set, in order to maintain behavior for users who are already using LDAP if they don't set the new envvars.
…g environment variables.
…ing why there are multiple.
|
The current version doesn't work right, I'm still figuring out what's wrong. |
|
I figured out what I did wrong, stupid mistakes because I'm not a python programmer. Everything appears to work as desired now. |
|
Thanks for your addition. @DanielBatteryStapler is our resident LDAP expert who created the original implementation. Can you take a look at these changes? Then you might have seen, the linter is unhappy with the formatting. Mostly about line length and whitespaces. In the deploy.sh script there is a validation function so you can run the linter locally. |
|
I see what the linter trying to get at. I wonder why it didn't have an issue with the original version, seems similar. I'm about to start my workday, I'll get back to it and fix it some time later today. |
|
Changes look great to me, I approve. Thank you for your additions! I thought about including exactly this in my initial LDAP additions, but I got lazy and thought that the defaults would be good enough -- guess I was wrong about that one. Glad to see that get fixed. |
|
This should be fixed according to the linter. |
|
That's odd, the linter accepted it on my end... |
|
Almost there, flake8 doesn't like the long lines in settings.py between line 108 and 115. Unfortunately I need to run the action here manually for fist time contributors, it's a github thing... |
|
oh, i was pretty sure I fixed those too, I'll check. |
|
Python also has this nifty multi line comment feature, basically wrap it in """
this is a multi
line comment
spanning over
multiple lines,
nicely.
"""Does the same as your solution, but you look more like a Python pro. :-) |
|
I'm just gonna pop in to be annoying and say those aren't multi-line comments, those are multi-line string literals and you're simply ignoring their value! Still useful as comments though lol |
|
... I'm not sure if I love or hate that. |
|
The language behavior, not you popping in. I enjoy "technically correct", I am a programmer, a after all. |
|
Yeah, technically correct yes, bot not really what people use, e.g. if you google But I had to look it up, PEP8 technically calls them And yes, I always enjoy the technically things too. :-) |
|
Shortened line lengths. |
|
I officially renounce Python, lol |
|
Sorry, I took the liberty to fix the remaining few whitespace issues. I know, python linting needs some time to get used to. Please don't be discouraged, there are lots of great opportunities to help with this project! :-) |
|
I just need to take the time to actually get a decent python environment set up, i've been using a docker container to run black, but it's not ideal, and VSCode doesn't seem to be getting along with my Python environment. |
|
Yeah, after some time fixing the same issues over and over, you'll get the hang of it. I know it's the equivalent of a grammar naz*, but it does make the code much more readable, particularly if you have a few people working on the same code base... Anyways, thanks for your contribution, all help is welcome with this project! |
|
I'm actually a huge stickler for linting and keeping code clean, it's just that I don't care as just about EOL white space, and I have my line length set as warnings. |
When using a default Samba DC LDAP instance,
uidisn't used to hold the username, so for this, and other LDAP implementations, it's necessary to be able to specify which LDAP attributes are actually used for first name, last name, username, email, etc.This doesn't change the default behavior, as it uses the current hardcoded values as the default values instead of requiring admins to specify it if they are upgrading to a version containing this feature.