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

support Same-site cookies #2733

Closed
philtay opened this issue Aug 6, 2016 · 15 comments
Closed

support Same-site cookies #2733

philtay opened this issue Aug 6, 2016 · 15 comments

Comments

@philtay
Copy link

philtay commented Aug 6, 2016

Please support Same-site cookies in pyramid.response.set_cookie. This new standard provides protection against CSRF attacks and Google Chrome has already implemented it.

@mmerickel
Copy link
Member

This is actually an issue for webob and a duplciate of Pylons/webob#255. However I'm going to leave this open and rephrase it.

Pyramid should add support for same_site=True on AuthTktAuthenticationPolicy and SignedCookieSessionFactory once webob supports it in its CookieProfile API.

@digitalresistor
Copy link
Member

Is this actually RFC yet? Or is it still experimental? I need to write some docs with the experimental stuff so that rapid changes can be made as the standard evolves rather than having to worry about backwards compatibility.

@ztane
Copy link
Contributor

ztane commented Aug 9, 2016

Actually now that I read it, there is no same_site=True but there are 2 distinct behaviours, strict and lax; strict would mean that even a GET request wouldn't send the cookie if it originates from a link on another site, so if you Google something and it directs to your site, then open the link, you are not logged in to your site in that tab.

@AvnerCohen
Copy link

@mmerickel @ztane Any ideas if this planned any time soon?

@digitalresistor
Copy link
Member

I have merged support for it into WebOb, but it is not yet in a release.

@alex
Copy link
Contributor

alex commented Mar 11, 2018

What is the next step here, now that this is landed in webob. Is this something that I could help with?

@ztane
Copy link
Contributor

ztane commented Mar 11, 2018

Well, pyramid.response is a WebOb response so it supports it.

@alex
Copy link
Contributor

alex commented Mar 11, 2018

Should this bug be closed then? Or is there more work to do?

@mmerickel
Copy link
Member

mmerickel commented Mar 11, 2018

@alex thanks for checking! Webob itself (thus pyramid's response object) supports samesite cookies on response.set_cookie but this issue relates to Pyramid's usage / support of same-site cookies in APIs that wrap or use cookies. This means adding relevant support to BaseCookieSessionFactory, SignedCookieSessionFactory, AuthTktAuthenticationPolicy and probably CookieCSRFStoragePolicy. This basically entails adding an option to each policy, docs for the option, and tests to each object to ensure they support the option.

@mmerickel mmerickel added this to the 1.10 milestone Jul 30, 2018
@mmerickel
Copy link
Member

mmerickel commented Jul 30, 2018

Sessions were updated via #3300 and auth/csrf in #3319.

  • BaseCookieSessionFactory
  • SignedCookieSessionFactory
  • AuthTktAuthenticationPolicy
  • CookieCSRFStoragePolicy

@persho
Copy link

persho commented Aug 12, 2020

@bertjwregeer @mmerickel I am using Pyramid 1.10.4 and I had issues setting samesite=None like
AuthTktAuthenticationPolicy(settings['auth.secret'], hashalg='sha512', max_age=3600 * 24, samesite=None, http_only=True, secure=True)

it never came to cookie, so the cookie always used default samesite='Lax'. We tried to find a reason, and finally got it to work using 'None' as a string.

AuthTktAuthenticationPolicy(settings['auth.secret'], hashalg='sha512', max_age=3600 * 24, samesite='None', http_only=True, secure=True)

I am not sure this is expected, but it works.

Thanks
Gabrijel

@mmerickel
Copy link
Member

mmerickel commented Aug 12, 2020

The string 'None' is the correct usage. The None sentinel in Python would normally indicate to not set samesite at all on the cookie, but may also trigger the default Lax like you mentioned. If you think we should improve the docs could you submit a PR or point out specific locations?

@persho
Copy link

persho commented Aug 12, 2020

Hey @mmerickel yes actually I think docs are misleading https://docs.pylonsproject.org/projects/pyramid/en/latest/api/authentication.html

check samesite section. It talks about 'Lax' and None as a non string. It would be awesome to mention explicitly that None has to be passed as a string. Hope it is not just me who got confused :)

@mmerickel
Copy link
Member

mmerickel commented Aug 13, 2020

I'm more concerned that it sounds like you're saying samesite=None is triggering a SameSite=Lax header, which is counter to the current docs that say it should not add a SameSite directive to the cookie at all. This would be a bug.

If that's the case, please open a new ticket with that bug report.

@digitalresistor
Copy link
Member

samesite=None currently tells the code that you don't want to send a SameSite flag in the cookie that is being sent to the remote user.

The browser then converts that to Lax or whatever Chrome has decided today by default... the docs should be updated to use the string, and it looks like @stevepiercy did so.

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

No branches or pull requests

7 participants