Skip to content

[Security] Make ChainUserProvider::loadUserByIdentifier() to pass all arguments to its providers #58773

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 5 commits into
base: 7.4
Choose a base branch
from

Conversation

jasiolpn
Copy link
Contributor

@jasiolpn jasiolpn commented Nov 5, 2024

Q A
Branch? 7.2
Bug fix? no
New feature? yes
Deprecations? no
License MIT

ChainUserProvider exists in Symfony since forever, but there is a relatively new feature in Security Component - AttributesBasedUserProviderInterface which allows to pass custom array from authenticator to the user provider. Unfortunately it does not work if the instance of class that implements AttributesBasedUserProviderInterface is "hidden" behind ChainUserProvider. The method ChainUserProvider::loadUserByIdentifier passes always only its first argument to the chained provider and therefore we loose features intorduced by the AttributesBasedUserProviderInterface.

My solution to this problem assumes that we treat ChainUserProvider as a "proxy class" to the internal providers, so we should pass all arguments passed to ChainUserProvider::loadUserByIdentifier to the <internal_provider>::loadUserByIdentifier.

Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

Can you please add a test case?

@jasiolpn
Copy link
Contributor Author

jasiolpn commented Nov 6, 2024

Hey, I added test case

@OskarStark OskarStark changed the title [Security] Make ChainUserProvider::loadUserByIdentifier() to pass all arguments to its providers [Security] Make ChainUserProvider::loadUserByIdentifier() to pass all arguments to its providers Nov 6, 2024
@chalasr
Copy link
Member

chalasr commented Nov 6, 2024

I would merge this as a bugfix on 6.4

@fabpot fabpot modified the milestones: 7.2, 7.3 Nov 20, 2024
@fabpot fabpot modified the milestones: 7.3, 7.4 May 26, 2025
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.

5 participants