-
Notifications
You must be signed in to change notification settings - Fork 886
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
Should cookie-based sessions default to secure if using HTTPS? #3610
Comments
I think I'm ok with this but I think it should be done universally for our various cookie-generating APIs (auth, sessions, csrf). I suspect the API would be that if I'm not sure what is considered better - "dynamic / smart" security policies or more explicit ones like what we have right now. |
Sounds good, I can include those in a PR.
Yeah, that's what I was imagining.
Generally I would prefer explicit, but in this case Speaking of explicit—maybe a topic for another issue—but should we change default of |
In the interest of alternatives that do not require changing Pyramid to solve this problem - all the apps I write source these settings from the ini file. So I commonly support settings like |
Considering both a docs/tutorials/cookie cutter updating and getting started/newbie perspectives, my preferences would be in order:
Forcing it to prefer https seems harmful to me. I would strongly support some added documentation about how to use https throughout the entire stack, even though that is primarily the responsibility of the web server, because secure cookies touches on this topic. At the very least, a list of requirements with references for more information. Currently on master there is no mention of "certificate" in the docs. |
@luhn you may also want to take a look at I am okay with setting the cookie to |
It would be convenient to have a
Are you saying we should remove Pyramid's implementation of signed cookies and use WebOb's instead? |
@luhn we already used the serializers from WebOb, I thought that the BaseCookieSessionFactory was also using Since we aren't, we should see if we can change it to use the |
Pretty simple change that I can whip up a PR for, figured now's the time to shoehorn it in before 2.0 release.
Basically, default SignedCookieSessionFactory would default the "secure" parameter to
request.scheme == 'https'
. So by default if the request is HTTPS, sessions will use a secure cookie. Implementation looks something like this.Pros:
Cons:
Would a PR for this be welcome?
The text was updated successfully, but these errors were encountered: