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

LDAP Source Bind from Federation & Social login is Broken Since Commit 1ca8feb #5920

Closed
ReaHe opened this issue Jun 9, 2023 · 6 comments · Fixed by #5927
Closed

LDAP Source Bind from Federation & Social login is Broken Since Commit 1ca8feb #5920

ReaHe opened this issue Jun 9, 2023 · 6 comments · Fixed by #5927
Labels
bug Something isn't working

Comments

@ReaHe
Copy link

ReaHe commented Jun 9, 2023

Describe the bug
LDAP Sources are broken since commit 1ca8feb. This is due to a double bind that is attempted on the Connection object. The second bind consistently causes a LDAPInvalidCredentailResult exception even if it is successful the first bind. I found this bug after updating Authentik to 2023.5.3 wondering why LDAP logins no longer worked. The LDAP backend is consistently changing passwords due to TOTP so a cached password was never hit always forcing a LDAP bind. Which is what made me find this issue.

To Reproduce

  1. Set up a LDAP Source that works
  2. Login in as any user using the LDAP Password for that users THATS NOT CACHED
  3. User Always gets Invalid Credentials

Expected behavior
User should successfully login

Logs
None Provided

Version and Deployment (please complete the following information):

  • authentik version: 2023.5.3
  • Deployment: docker-compose

Additional context
I confirmed this bug on my own stack by creating multiple logging lines using LOGGER and checking DEBUG output. For some reason a second bind always causes this Exception with Authentik if the first was successful. I'm not sure if the state is reset per successful connection which is causing this. But by removing the extra bind performed in /authentik/sources/ldap/auth.py I no longer get erroneous invalid credential exceptions.

@ReaHe ReaHe added the bug Something isn't working label Jun 9, 2023
@septatrix
Copy link
Contributor

I can confirm that this regression is present in 2023.5.3 whereas 2023.1.2 is working. We are seeing the same behaviour.

@BeryJu
Copy link
Member

BeryJu commented Jun 11, 2023

Yeah that extra bind here 1ca8feb#diff-8e72f28797d7b000f7675c5e282aed66bb14164300c9537fcb7cee8e0bfaa413R66 should be removed, however I wasn't able to reproduce any error testing against Samba

@ReaHe
Copy link
Author

ReaHe commented Jun 11, 2023

Thx for fixing it already and thats interesting that Samba did not have any errors. I am using FreeIPA which uses the 389 Directory Service (https://www.port389.org/). I've never used any LDAP features with Samba so I wonder how it and ldap3 (pip package) are communicating differently. I'm also curious if anonymous binds (or something similar) are allowed in your Samba setup, causing you to bypass the root of the issue. Regardless, thank you for fixing it!

@BeryJu
Copy link
Member

BeryJu commented Jun 12, 2023

The double bind doesn't really do anything wrong according to the LDAP spec, it opens an LDAP connection, sends a bind request, and afterwards sends another bind request on the same connection. Besides costing us some more time, it shouldn't cause any issues in theory

@septatrix
Copy link
Contributor

We are running a custom LDAP server based on ldaptor so this might also be something which we should have a look into ourselves though no other client seemed to struggle with this so far.

@strazto
Copy link

strazto commented Jun 21, 2023

Maybe related:
#5199

MaartenGrothus pushed a commit to cde-ev/cdedb2 that referenced this issue Jul 30, 2023
There is a regression in the newer version:
goauthentik/authentik#5920
Other versions would also work
but this is the one already tested on the keycloak-vm.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants