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

Configuration option user_id_claim doesn't work as described in deprecation message. #1083

Closed
b3k opened this issue Nov 2, 2022 · 1 comment

Comments

@b3k
Copy link

b3k commented Nov 2, 2022

Using user_identity_field in configuration generates depreacted notification:

The "%path%.%user_identity_field%" configuration key is deprecated since version 2.16, use "%path%.user_id_claim" or implement "' . UserInterface::class . '::getUserIdentifier()" instead.

But using user_id_claim will not behave as we expect when using value other than username, the method JWTManager::addUserIdentityToPayload will always consider values from user_identity_field (as default value which is username).

    protected function addUserIdentityToPayload(UserInterface $user, array &$payload)
    {
        $accessor = PropertyAccess::createPropertyAccessor();

        if ($user instanceof InMemoryUser && ('username' === $this->userIdClaim || 'username' === $this->userIdentityField)) {
            $payload[$this->userIdClaim ?: $this->userIdentityField] = $accessor->getValue($user, 'userIdentifier');

            return;
        }

        $payload[$this->userIdClaim ?: $this->userIdentityField] = $accessor->getValue($user, $accessor->isReadable($user, $this->userIdentityField) ? $this->userIdentityField : 'user_identifier');
    }

So if we have configuration like this:

lexik_jwt_authentication:
    # ...
    user_id_claim:    id

Then last line will be evaulated like this:

$payload['id'] = $accessor->getValue($user, $accessor->isReadable($user, 'username') ? 'username' : 'user_identifier');
// which will give `username` if property is readable

Instead it should first consider $this->userIdClaim field first and then try to access field using $this->userIdentityField.

So in my opinion this should behave maybe like this:

$valueFiled = $this->userIdClaim ?: $this->userIdentityField;
$payload[$valueFiled] = 
    $accessor->getValue($user, $accessor->isReadable($user, $valueFiled) ? $valueFiled : 'user_identifier');
@koftikes
Copy link

Hello, all. I pushed MR for fix this issue.

chalasr added a commit that referenced this issue Apr 11, 2023
…itvinov)

This PR was merged into the 2.x branch.

Discussion
----------

#1083. Fixed issue with option user_id_claim.

Fixed issue with configuration option `user_id_claim`

Commits
-------

d5d2b6f #1083. Fixed issue with option user_id_claim.
Spomky pushed a commit to Spomky/LexikJWTAuthenticationBundle that referenced this issue May 13, 2023
@chalasr chalasr closed this as completed Dec 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants