pallets-eco / flask-session

Server side session extension for Flask
https://flask-session.readthedocs.io
BSD 3-Clause "New" or "Revised" License
501 stars 239 forks source link

Ability to set cookie SameSite #111

Closed vilhelmprytz closed 3 years ago

vilhelmprytz commented 4 years ago

Is it possible to add the SameSite=Strict; option to the Set-Cookie header?

According to this website there should be the option to set SESSION_COOKIE_SAMESITE='Strict' but it doesn't seem to work with flask-session.

Am I doing something wrong or is it not supported?

BakitD commented 4 years ago

@VilhelmPrytz according to latest version of Flask-Session using of SESSION_COOKIE_SAMESITE will no make sense. But you could use your own redefined Response class.

Example:

class MyResponse(Response):
    def set_cookie(self, *args, **kwargs):
        cookie = dump_cookie(*args, **kwargs)
        if 'samesite' in kwargs and kwargs['samesite'] is None or 'samesite' not in kwargs:
            cookie = "{}; {}".format(cookie, b'SameSite=Lax;')

        self.headers.add(
            'Set-Cookie',
            cookie
        )

Then change response class like:

app = Flask(...)
...
app.response_class = MyResponse
GChalony commented 4 years ago

Hi, May I ask why I wouldn't make sense to use flask's SESSION_COOKIE_SAMESITE ?

vilhelmprytz commented 4 years ago

I don't know why it won't make sense, but I know it's not possible to set it to strict.

BakitD commented 4 years ago

@GChalony hi, u can, but it seems like flask-session does not support SameSite option https://github.com/fengsp/flask-session/blob/master/flask_session/sessions.py

GChalony commented 4 years ago

Yeah that's what I realised reading the code, could it be added in a future version though? It seems especially weird as the native flask session supports it.

vilhelmprytz commented 4 years ago

Yeah that's what I realised reading the code, could it be added in a future version though? It seems especially weird as the native flask session supports it.

Yes, it should be fairly easy to implement. But it seems the author of this project is quite inactive.

benshanahan1 commented 4 years ago

+1 This is very important to have with Chrome's recent changes. There is currently an open PR for this (#116) ready to be merged.

fengsp commented 3 years ago

Fixed in #116