Potential SF6 Compat Issue - JWTManager::addUserIdentityToPayload() by default requires a username property or getUsername method

[mbabker]

With the default bundle configuration, JWTManager::addUserIdentityToPayload() adds the user identity to the JWT payload by using a property accessor to fetch the configured userIdentityField from the User object, which defaults to "username". With UserInterface::getUsername() going away in Symfony 6, this default configuration looks like it could start to cause problems in one of two ways:

• This bundle will require each UserInterface implementation to have either a $username property or a getUsername() method with the out-of-the-box configuration
• Changing the $userIdentityField property to "userIdentifier" to match the updated UserInterface (since the implementation details of the property accessor say it will try to use a getter for that field name) by default also changes the name of the user ID claim in the JWT payload, unless you also configure the user_id_claim config node which cascades down to the $userIdClaim property; IMO these types of changes to the defaults in the bundle would be a B/C break so couldn't really be considered before a 3.0 release (but, bundle users can make updates through the bundle config)

Personally, I'd say to deprecate the user_identity_field bundle config and the userIdentityField property in the manager and just use the UserInterface by default, and if someone needs to use something that isn't the result of getUsername() or getUserIdentifier(), they can subclass the manager and override the addUserIdentityToPayload() method, but I also realize #40 added this for a specific reason so maybe that isn't the greatest of solutions.

[chalasr]

Thanks for the issue. You're right here.
We probably need to change the default identity field to user_identifier instead of username.
I'll take care of this.

[slowwriter]

Thanks for the issue. You're right here.
We probably need to change the default identity field to user_identifier instead of username.
I'll take care of this.

@chalasr Exactly as you write. It is a simple and very important thing. When can we count on a correction?

[chalasr]

Thanks for the ping @malkavi6, I totally forgot about this. I'm going to look at it in the coming days

[slowwriter]

Bump. I need to be able to replace the username with another user property.

[tarlepp]

same issue here, is there a way to configure what field is used?

[tarlepp]

maybe there should just be check like;

if (method_exists($user, 'getUserIdentifier')

and not rely on hard coded userIdentityField

[mbabker]

This is what I'm using right now, we haven't moved up to Symfony 6 yet but from what I remember this shouldn't cause any issues:

lexik_jwt_authentication:
    user_identity_field: 'email' # Any of the properties/getters in the user object
    user_id_claim: 'username' # The fieldname used in the JWT payload for the user identity

[chalasr]

I will look into this asap, hopefully next week.

[chalasr]

I agree that config option is not needed.
I'm going to deprecate both the user_identity_field config option and the set/getUserIdentityField() methods to just rely on Symfony UserInterface::getUserIdentifier() instead. Please tell me if you see any cons.

[slowwriter]

Perfect, that's what I meant.

[CaptainFalcon92]

Hello.
Any commit yet for this ? I'm still having this error.
Meanwhile i do use user_identity_field in config.

[chalasr]

See #1037

[Aweptimum]

Should the docs be updated to reflect this change? I was looking at setting up the bundle, realized it assumed a username field, and then went hunting for how to instead use something like email. I found this issue after a while, and it looks like the linked PR switches to using getUserIdentifier which, if I understand, means there's not really any config needed to handle the identifying field/key? If so, the Configuration Reference doc still contains the user_identity_field config. I also cannot find any reference in the docs on the 2.1x branch explaining that users of the bundle can rely on their UserProvider's UserInterface:getUserIdentifier() method.

I'd be willing to pr removing user_identity_field from the configuration reference, but not sure where to add some language for getUserIdentifier. Do you have a good spot in mind, chalasr?

Thanks for adding this change though 👌

[chalasr]

@Aweptimum I think a small comment around the user_id_claim in the config reference (yaml) would do the job. It should explain that the default value of that claim will equal the return value of UserInterface::getUserIdentifier() by default or, in case that method is not implemented, the value of the corresponding property (e.g. if user_id_claim is set to some_id, the bundle will try to fill it with the value of $user::$someId via a public accessor).
Note that I would welcome a PR that just removes the user_identity_field from the config reference so if phrasing what I said above feels too complicated, don't worry about it. Thanks!