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

Allow Setting Pinning Cookie Attributes #53

Merged
merged 8 commits into from
Sep 22, 2021

Conversation

dchukhin
Copy link
Contributor

@dchukhin dchukhin commented Apr 28, 2021

This pull request allows users to set the pinning cookie's 'Secure', 'HttpOnly', and 'SameSite' attributes by using settings, which are based on Django's CSRF_COOKIE_SECURE, CSRF_COOKIE_HTTPONLY, and CSRF_COOKIE_SAMESITE settings.
The defaults for these settings keep the current behavior: not Secure, not HttpOnly, and 'Lax' SameSite (currently, the SameSite attribute is unset, which should default to 'Lax' in browsers).

A part of this work also changes the PINNING_COOKIE and PINNING_SECONDS in multidb/middleware.py to be functions, so they can be tested more easily.

@tobiasmcnulty
Copy link

Hey @jbalogh, hope you are well! I just wanted to check to see if this change is something you would consider accepting at some point. (Full disclosure, @dchukhin is on my team at Caktus). We're happy to help in any way with merge and/or release. Thanks as always for the numerous Django packages we seem to find handy! 🙂

@diox
Copy link
Collaborator

diox commented Aug 25, 2021

Hey, I'm not @jbalogh but I can review that, sorry for not catching it earlier.

@diox diox self-requested a review August 25, 2021 15:31
multidb/tests.py Show resolved Hide resolved
@diox
Copy link
Collaborator

diox commented Aug 26, 2021

Also: if you rebase/merge with master this will allow Github Actions to run, with updated Django/Python versions we want to support.

@Jdsleppy
Copy link
Contributor

Hi @diox , I think these changes are what you were looking for. GitHub says you need to manually approve running CI on this PR because it's from a first-time contributor to this repository. Cheers!

@diox diox self-requested a review September 21, 2021 19:58
Copy link
Collaborator

@diox diox left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for doing this

@diox diox merged commit dd33730 into jbalogh:master Sep 22, 2021
@diox
Copy link
Collaborator

diox commented Sep 23, 2021

0.10 has been released with this change.

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.

None yet

4 participants