Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[Ldap] Allow to use ldap in a chain provider #51231

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
wants to merge 1 commit into
base: 7.3
Choose a base branch
Loading
from

Conversation

l-vo
Copy link
Contributor

@l-vo l-vo commented Aug 2, 2023

Q A
Branch? 6.4
Bug fix? no
New feature? yes
Deprecations? yes
Tickets #41892
License MIT
Doc PR

This PR allows to fix that the LdapUserProvider that not work properly when used in the ChainProvider.

This fix relies on two ideas:

  • The LdapBadge is not a badge that must be resolved, it's only used to carry Ldap data
  • Allow to prevent CheckLdapCredentialsListener to check against the ldap directory the password for a non-ldap user using a new #[WithLdapPassword] attribute

@l-vo l-vo requested review from wouterj and chalasr as code owners August 2, 2023 14:51
@carsonbot carsonbot added this to the 5.4 milestone Aug 2, 2023
@l-vo l-vo force-pushed the userprovider_chain_with_ldap branch 5 times, most recently from c63cdc0 to ee6ec00 Compare August 2, 2023 15:19
@carsonbot carsonbot changed the title Allow to use ldap in a chain provider [Ldap] Allow to use ldap in a chain provider Aug 4, 2023
@nicolas-grekas nicolas-grekas modified the milestones: 5.4, 6.4 Aug 4, 2023
@l-vo l-vo force-pushed the userprovider_chain_with_ldap branch 2 times, most recently from c1a964d to 731dcd3 Compare August 4, 2023 08:08
@derrabus derrabus changed the base branch from 5.4 to 6.4 August 4, 2023 08:12
@l-vo l-vo force-pushed the userprovider_chain_with_ldap branch 3 times, most recently from fa50a1e to cbe2c95 Compare August 4, 2023 09:05
@l-vo l-vo force-pushed the userprovider_chain_with_ldap branch 3 times, most recently from 2120810 to 726db70 Compare August 8, 2023 11:32
@l-vo l-vo force-pushed the userprovider_chain_with_ldap branch from 726db70 to 8dbeb60 Compare August 8, 2023 12:51
@nicolas-grekas nicolas-grekas modified the milestones: 6.4, 7.1 Nov 15, 2023
@chalasr
Copy link
Member

chalasr commented Mar 29, 2025

This looks like a feature

@l-vo
Copy link
Contributor Author

l-vo commented Apr 2, 2025

@chalasr I don't know if it's really a feature, it worked with the Guard system and the feature has seamlessly disappeared with the replacement of Guard by Authenticators (and there was no announcement that the feature was not supported with Authenticators).

Copy link
Member

@stof stof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure the configuration described in the linked issue (using the same check_path for form_login and form_login_ldap) actually makes sense (and so whether this solution is valid as solution for that use case anyway)

@@ -171,6 +196,9 @@ public static function queryForDnProvider(): iterable
yield ['{user_identifier}', '{user_identifier}_test'];
}

/**
* @group legacy
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this becoming a legacy tests ? Why would we remove this test when removing deprecated features related to this PR ?

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.

8 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.