Skip to content

[Security] Handle concurency in Csrf DoctrineTokenProvider #41910

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

Merged
merged 1 commit into from
Jun 30, 2021

Conversation

jderusse
Copy link
Member

@jderusse jderusse commented Jun 30, 2021

Q A
Branch? 5.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

When the PersistentRememberMeHandler class process a RememberMe cookie older than 1 minute it can tells the tokenVerifier to update it. This method performs a delete then an insert. This could be an issue with concurrent requests leading to UniqueConstraintViolationException.

This PR wrap the delete/insert in a transaction to prevent this.

@jderusse jderusse requested review from chalasr and wouterj as code owners June 30, 2021 07:02
@carsonbot carsonbot added this to the 5.3 milestone Jun 30, 2021
@carsonbot carsonbot changed the title Handle concurency in Csrf DoctrineTokenProvider [Security] Handle concurency in Csrf DoctrineTokenProvider Jun 30, 2021
@fabpot
Copy link
Member

fabpot commented Jun 30, 2021

Thank you @jderusse.

@fabpot fabpot merged commit 2888e40 into symfony:5.3 Jun 30, 2021
@fabpot fabpot mentioned this pull request Jun 30, 2021
fabpot added a commit that referenced this pull request Jul 3, 2021
This PR was merged into the 5.3 branch.

Discussion
----------

Rethrow exception in `DoctrineTokenProvider`

| Q             | A
| ------------- | ---
| Branch?       | 5.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Introduced in #41910

Commits
-------

246e679 rethrow caught exception
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.

3 participants