8000 [Security] set configured claim as userIdentifier on OidcUser using OidcUserInfoTokenHandler by smatyas · Pull Request #52625 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Security] set configured claim as userIdentifier on OidcUser using OidcUserInfoTokenHandler #52625

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[Security] set configured claim as userIdentifier on OidcUser using OidcUserInfoTokenHandler #52625

wants to merge 1 commit into from

Conversation

smatyas
Copy link
Contributor
@smatyas smatyas commented Nov 17, 2023 •
edited
Loading
Q A
Branch? 7.1
Bug fix? yes
New feature? no
Deprecations? no
Issues
License MIT

As I was trying to use the oidc_user_info token handler with a custom claim setting, I noticed (debugged) that the specified claim is only set on the UserBadge, but not on the actual OidcUser object. The latter gets the original $claims array as the constructor parameters through the \Symfony\Component\Security\Http\AccessToken\Oidc\OidcTrait::createUser, that's why the selected claim does not get set currently.

This patch does solve this case.

I.e. using this example from the doc does not set the email claim as OidcUser::userIdentifier, that property will never be set.

# config/packages/security.yaml
security:
    firewalls:
        main:
            access_token:
                token_handler:
                    oidc_user_info:
                        claim: email
                        base_uri: https://www.example.com/realms/demo/protocol/openid-connect/userinfo

Please let me know if it's not the intended way to set up the user class.

If this is should be the correct flow (as I assume), I'm willing to cover it with tests in this PR.

@smatyas smatyas requested a review from chalasr as a code owner November 17, 2023 11:29
@carsonbot carsonbot added this to the 7.1 milestone Nov 17, 2023
@xabbuh xabbuh modified the milestones: 7.1, 6.4 May 15, 2024
@Spomky
Copy link
Contributor
Spomky commented Jun 1, 2025

Hi,
This should be fixed now. See #60166.
Can you confirm?

@chalasr
Copy link
Member
chalasr commented Jun 1, 2025

Indeed. Closing, please reopen if you think otherwise.

@chalasr chalasr closed this Jun 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chalasr chalasr Awaiting requested review from chalasr chalasr is a code owner

Assignees
No one assigned
Labels
Bug Security Status: Needs Review
Projects
None yet
Milestone
6.4
Development

Successfully merging this pull request may close these issues.
5 participants
@smatyas @Spomky @chalasr @xabbuh @carsonbot
0