Skip to content

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 7.3
Bug fix? no
New feature? no
Deprecations? no
Issues -
License MIT

When the user object implement EquatableInterface, we never read the roles stored in the token object that wraps the user in the session storage.

This PR ensures we don't store these roles either - they're just wasting space.

@stof
Copy link
Member

stof commented Jan 20, 2025

When the user object implement EquatableInterface, we never read the roles stored in the token object that wraps the user in the session storage.

This affirmation looks weird to me. The RoleVoter reads them when voting on roles.

@nicolas-grekas
Copy link
Member Author

Yes sure, I meant from the pov of ContextListenet::hasUserChanged, which contains this bypass:

if ($originalUser instanceof EquatableInterface) {
return !$originalUser->isEqualTo($refreshedUser);
}

@nicolas-grekas
Copy link
Member Author

Friendly ping @symfony/mergers

@nicolas-grekas nicolas-grekas merged commit eff9b52 into symfony:7.3 Jan 29, 2025
11 checks passed
@nicolas-grekas nicolas-grekas deleted the sec-optim branch January 29, 2025 10:25
nicolas-grekas added a commit that referenced this pull request Jun 4, 2025
…kas)

This PR was merged into the 7.3 branch.

Discussion
----------

[Security] Keep roles when serializing tokens

| Q             | A
| ------------- | ---
| Branch?       | 7.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #60656, Fix #60687
| License       | MIT

Looks like I missed something in #59558

Commits
-------

8092ffd [Security] Keep roles when serializing tokens
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.

4 participants