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

Any interest in adding the 'samesite' flag to cookies? #376

Closed
nathanahtan opened this issue Sep 4, 2019 · 3 comments
Closed

Any interest in adding the 'samesite' flag to cookies? #376

nathanahtan opened this issue Sep 4, 2019 · 3 comments

Comments

@nathanahtan
Copy link

I was interested in enabling the samesite cookie flag and was surprised to find it doesn't seem to be supported (or I completely missed how to enable). I guess it is not yet an 'official' RFC documented flag, so maybe that is the reason?

I seem to have been able to get it to work here (on Chrome/Firefox anyway) by adding the parameter to:

  • aiohttp_session.AbstractStorage
  • aiohttp.web_response.StreamResponse.set_cookie()

in the same way that the other cookie params are handled.

One also has to 'monkey patch' the dependency http.cookies.Morsel._reserved to include the 'samesite' keyword (python 3.7.3).

I wouldn't mind submitting a PR but thought I'd ask to see if there is any interest or objection.

@samuelcolvin
Copy link
Member

samuelcolvin commented Oct 18, 2019

I think this would be good. @asvetlov, would you accept a PR to add SameSite?

@asvetlov
Copy link
Member

Monkey-patching is not an option.
Code should raise TypeError: set_cookie() got an unexpected keyword argument 'samesite' for Python < 3.8 if samesite is not None (default).

We need two pull requests: one for aiohttp and another for aiohttp-session.

I would postpone the fix until Azure and Travis will get Python 3.8 in their build matrices.

Upd:
Looks like Travis already has it: https://docs.travis-ci.com/user/languages/python/
Azure has opened issue, the work is in progress: https://github.com/microsoft/azure-pipelines-image-generation/issues/1317

@samuelcolvin
Copy link
Member

Discuss on aio-libs/aiohttp#4224

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

No branches or pull requests

3 participants