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

Open LDAP early, otherwise user-manager is not available. #3632

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rotdrop
Copy link
Contributor

@rotdrop rotdrop commented Feb 21, 2022

Call openLDAP() early, seemingly $this->cando['getUsers'] is set too late such that the user-manager in the admin panel is not made available.

@Klap-in
Copy link
Collaborator

Klap-in commented Jun 5, 2022

Anyone with LDAP experience who could review this?

Seems logic because $this->cando['getUsers'] is set at the end of openLDAP(). Does this has consequences e.g. opening to many connections? or duplicated steps? in a lot of functions this method is already called at the start of the function. There is probably a reason to delay the call to the openLDAP() function?

@rotdrop rotdrop force-pushed the bugfix/authldap-usermanager branch 3 times, most recently from ab1fa0a to 5c4cce7 Compare June 24, 2022 13:40
@rotdrop rotdrop force-pushed the bugfix/authldap-usermanager branch from 5c4cce7 to 53b3bf8 Compare June 24, 2022 13:41
@splitbrain
Copy link
Collaborator

There is probably a reason to delay the call to the openLDAP() function?

Ideally a connection to ldap would only be opened when needed. Eg, if I am not logged in, there is no need to connect to LDAP.

I think it would make more sense to simply set the cando capability in the constructor, even if a connection might fail later.

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

Successfully merging this pull request may close these issues.

3 participants