Skip to content
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

Set secure and samesite when deleting remember cookie #873

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mo-lukecarr
Copy link

@mo-lukecarr mo-lukecarr commented Jul 15, 2024

This PR introduces a fix for the issue reported in #683. As per the linked issue, in some browsers, the expectation is to set the same values for Secure and SameSite when deleting as were set when creating the cookie.

The implementation in this PR changes the call to response.delete_cookie to set the same secure and samesite argument values as were set when response.set_cookie is invoked. In the implementation, I've also added code comments to have the structure of the _clear_cookie method mirror that of the _set_cookie method (where secure and samesite are initially set on cookies).

All checks are passing locally using tox:

image


As an aside, this is my first time contributing on this repository, and I noticed a dead link in the CONTRIBUTING.md file, so I've corrected it in this PR. Apologies if this makes the PR less granular and isn't strictly adhering to PR etiquette!

@coveralls
Copy link

Coverage Status

coverage: 95.968% (+0.02%) from 95.951%
when pulling 548f8b9 on mo-lukecarr:clear-cookie-fix
into 26d12ea on maxcountryman:main.

1 similar comment
@coveralls
Copy link

Coverage Status

coverage: 95.968% (+0.02%) from 95.951%
when pulling 548f8b9 on mo-lukecarr:clear-cookie-fix
into 26d12ea on maxcountryman:main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants