Skip to content

Include samesite and secure keys when removing session cookie #3726

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
davidism merged 1 commit into from
Nov 5, 2020
Merged

Include samesite and secure keys when removing session cookie #3726

davidism merged 1 commit into from
Nov 5, 2020

Conversation

@matush-v
Copy link
Contributor

@matush-v matush-v commented Aug 10, 2020 •
edited by davidism
Loading

The samesite key is required on empty cookies. When a flask session is cleared, the delete cookie logic needs to set the configured samesite and secure keys.

samesite missing on cookie

Requires that pallets/werkzeug#1889 is merged first.

@matush-v matush-v changed the title include samesite and secure keys when removing session cookie Include samesite and secure keys when removing session cookie Aug 10, 2020
@matush-v
Copy link
Contributor Author

matush-v commented Aug 10, 2020

Running pytest succeded but the full suite via tox is failing on the delete_cookie method I'm utilizing. Seems like I'll need to first update https://github.com/pallets/werkzeug/blob/b45ac05b7feb30d4611d6b754bd94334ece4b1cd/src/werkzeug/test.py#L886 to support samesite and secure

@pgjones
Copy link
Member

pgjones commented Aug 10, 2020

See, pallets/werkzeug#1889 which I think is a blocker for this.

@matush-v
Copy link
Contributor Author

matush-v commented Aug 24, 2020

@pgjones thanks for the reference. It definitely looks like that PR will be necessary for this functionality. I'll also need to update the werkzeug test session class to have nearly identical changes to your PR.

@matush-v matush-v mentioned this pull request Aug 24, 2020
Merged
@davidism davidism added this to the 2.0.0 milestone Oct 15, 2020
@davidism davidism force-pushed the empty-session-samesite branch from 696d1b4 to 929fa37 Compare October 15, 2020 21:07
@davidism davidism force-pushed the empty-session-samesite branch from 929fa37 to 18c4fbc Compare November 5, 2020 02:12
@davidism davidism merged commit 22987b6 into pallets:master Nov 5, 2020
@matush-v matush-v deleted the empty-session-samesite branch November 5, 2020 02:46
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@matush-v @pgjones @davidism