8000 [Security] fix #41891 Save hashed tokenValue in RememberMe cookie by qurben · Pull Request #41897 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Security] fix #41891 Save hashed tokenValue in RememberMe cookie #41897

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
Merged
merged 1 commit into from
Jul 16, 2021
Merged

[Security] fix #41891 Save hashed tokenValue in RememberMe cookie #41897

merged 1 commit into from
Jul 16, 2021

Conversation

qurben
Copy link
Contributor
@qurben qurben commented Jun 29, 2021
Q A
Branch? 5.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #41891
License MIT

The hashed tokenValue is expected in the RememberMe cookie. This was not the case when this branch was executed.

@qurben qurben requested review from chalasr and wouterj as code owners June 29, 2021 10:02
@carsonbot carsonbot added this to the 5.3 milestone Jun 29, 2021
@qurben
Copy link
Contributor Author
qurben commented Jun 29, 2021

I am not sure if it is possible to create a test for this. testConsumeRememberMeCookieValid could test that the value that is passed to the tokenProvider mock is also found in the cookie, but when the mock is called the cookie is not yet set.

@chalasr chalasr mentioned this pull request Jul 16, 2021
Closed
@chalasr
Copy link
Member
chalasr commented Jul 16, 2021

Thanks for fixing this bug @qurben.

@chalasr chalasr merged commit 643e29a into symfony:5.3 Jul 16, 2021
@fabpot fabpot mentioned this pull request Jul 26, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Seldaek Seldaek Seldaek approved these changes

@jderusse jderusse jderusse approved these changes

@chalasr chalasr chalasr approved thes 634E e changes

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

Assignees
No one assigned
Labels
Bug Security Status: Reviewed
Projects
None yet
Milestone
5.3
Development

Successfully merging this pull request may close these issues.
5 participants
@qurben @chalasr @Seldaek @jderusse @carsonbot
0