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

Add samesite cookie option (fixes #212) #213

Merged
merged 5 commits into from
Aug 12, 2021

Conversation

schneider8357
Copy link
Contributor

@schneider8357 schneider8357 commented Mar 31, 2021

Fixes #212

Copy link
Owner

@ahopkins ahopkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. This is a good add. Thank you.

docs/source/pages/configuration.rst Outdated Show resolved Hide resolved
@ahopkins
Copy link
Owner

ahopkins commented Apr 4, 2021

I didn't check the problems with the tests, but run make black if its linting.

@schneider8357
Copy link
Contributor Author

I didn't check the problems with the tests, but run make black if its linting.

The build failed because I forked the main branch, which was failing the Travis build as well.

@ahopkins
Copy link
Owner

ahopkins commented Apr 4, 2021

I didn't check the problems with the tests, but run make black if its linting.

The build failed because I forked the main branch, which was failing the Travis build as well.

Yikes. I better go check that out then.

@schneider8357
Copy link
Contributor Author

schneider8357 commented Apr 4, 2021

I didn't check the problems with the tests, but run make black if its linting.

Still, I ran make black and this was the output:
(isort output was ommited because it didn't fix any of the files modified in this PR)

$ make black
black ./ -l 79 --safe
reformatted /home/lucas/sanic-jwt/setup.py
reformatted /home/lucas/sanic-jwt/sanic_jwt/configuration.py
reformatted /home/lucas/sanic-jwt/sanic_jwt/authentication.py
reformatted /home/lucas/sanic-jwt/tests/conftest.py
reformatted /home/lucas/sanic-jwt/tests/test_configuration.py
reformatted /home/lucas/sanic-jwt/tests/test_claims.py
reformatted /home/lucas/sanic-jwt/tests/test_endpoints_cbv.py
reformatted /home/lucas/sanic-jwt/tests/test_endpoints_basic.py
reformatted /home/lucas/sanic-jwt/tests/test_endpoints_init_on_bp_single.py
reformatted /home/lucas/sanic-jwt/tests/test_exceptions.py
reformatted /home/lucas/sanic-jwt/tests/test_user_secret.py
All done! ✨ 🍰 ✨
11 files reformatted, 69 files left unchanged.

And of the 4 files that I changed in this PR, only configuration.py was reformatted (and it was a docstring on line 252 unrelated to this issue), so I don't think that it will be a problem.

@schneider8357
Copy link
Contributor Author

Regarding the tests, I did modify tests/test_endpoints_cookies.py. But I only did minor changes, mostly based on the other cookie flags' tests.

@Paladiamors
Copy link

While not urgent. I am curious about the status of this as I'd like to get rid of the samesite cookie warnings if possible

@ahopkins
Copy link
Owner

ahopkins commented Aug 8, 2021

Let's get the tests passing and then merge/release.

@schneider8357
Copy link
Contributor Author

Sadly, I can't do anything about the Travis build failing.

@ahopkins
Copy link
Owner

Sadly, I can't do anything about the Travis build failing.

Working on it

@ahopkins ahopkins merged commit 2aa881a into ahopkins:main Aug 12, 2021
@Paladiamors
Copy link

@ahopkins Thanks for getting this merged!

@schneider8357 schneider8357 deleted the issue_212_samesite branch August 18, 2021 04:33
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.

Add option to set SameSite cookie
3 participants