Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SourceLDAP3 nitpicks #9

Open
jonathanmaw opened this issue Oct 17, 2022 · 4 comments
Open

SourceLDAP3 nitpicks #9

jonathanmaw opened this issue Oct 17, 2022 · 4 comments

Comments

@jonathanmaw
Copy link
Contributor

Looking over the code I've noticed a few things:

  • anonymous_bind is in config but not passed to ldap3. It could be passed into the Connection constructor with ldap3.Connection(..., authentication=ldap3.ANONYMOUS)
  • use_ssl is in config but not used anywhere. This should be passed into ldap3.Server(..., use_ssl=use_ssl)
  • port is in config but not used anywhere. This should be passed into ldap3.Server(..., port=port)
  • Inconsistent use of case in fetch_users vs fetch_groups (uses objectclass in the user filter and objectClass in the group filter) - ldap3 is not sensitive to case in those fields, so this is harmless.
@jonathanmaw
Copy link
Contributor Author

also, if there are no users found it'll print No User accounts :(). Is this meant to be Info or an Error?

@thinkl33t
Copy link
Contributor

also, if there are no users found it'll print No User accounts :(). Is this meant to be Info or an Error?

I think this should probably be an error. While there may be occasions where we would have a source configured with no users in it, it is vastly more likely to be an accidental misconfiguration, and it being an error would make it easier for us to be actually aware of this.

@thinkl33t
Copy link
Contributor

  • Inconsistent use of case in fetch_users vs fetch_groups (uses objectclass in the user filter and objectClass in the group filter) - ldap3 is not sensitive to case in those fields, so this is harmless.

while LDAP isn't case sensitive for sending to it, it will always return things in the case it thinks it is correct, not the case it was sent in, so (objectclass=foo) will return all items with objectclass foo, but it'll be sent back as objectClass: foo

@jonathanmaw
Copy link
Contributor Author

I've checked the current state of the code against this issue:

  • use_ssl is still in config but not used. If the url is specified with ldaps:// this is overridden, however.
  • port is also in config but not used. This is also overridden by the port specified in the URL.
  • inconsistent casing of objectClass is still present, but confirmed harmless.
  • No User accounts is now a debugging message logged, but might be better as an error or warning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants