pallets-eco / flask-session

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

Implemented SESSION_COOKIE_SAMESITE #116

Closed tylersalminen closed 3 years ago

iTaybb commented 4 years ago

Would like to see this merged

brunlid commented 4 years ago

would be great to see this merged please


        httponly = self.get_cookie_httponly(app)
        secure = self.get_cookie_secure(app)
        expires = self.get_expiration_time(app, session)
        val = self.serializer.dumps(dict(session))
        samesite = self.get_cookie_samesite(app)
        self.redis.setex(name=self.key_prefix + session.sid, value=val,
                         time=total_seconds(app.permanent_session_lifetime))
        if self.use_signer:
            session_id = self._get_signer(app).sign(want_bytes(session.sid))
        else:
            session_id = session.sid
        response.set_cookie(app.session_cookie_name, session_id,
                            expires=expires, httponly=httponly,
                            domain=domain, path=path, secure=secure,
                            samesite=samesite)
eelkevdbos commented 4 years ago

@fengsp Do need you something from us to merge this issue? With chrome removing support for third party cookies without 'SameSite=None' this feature will become somewhat essential. I'm willing to help testing if required.

fengsp commented 4 years ago

Versions of Flask older than 1.0 would break.

eelkevdbos commented 4 years ago

@fengsp Would you be willing to merge if we create some kind of capabilities switch based on flask version?

icappello commented 4 years ago

Using some try-except blocks could be a rather quick solution to avoid breaking older Flask versions: try calling the set_cookie method with the samesite parameter and fallback to the current call in case of exceptions

tylersalminen commented 4 years ago

Ok sorry have not been looking at this, I updated my fork and commited an update that will conditionally resolve and pass the samesite cookie kwarg to the set_cookie method on the SessionInterface base. Should be compatible with older versions of flask that do not have get_cookie_samsite and/or do not have the samesite kwarg on set_cookie

ChangYuJonathanWu commented 4 years ago

I think it's pretty critical to get this fix in. Without it, the Flask SESSION_COOKIE_SAMESITE setting essentially goes into a void and does so silently. As others have mentioned, browsers have implemented default controls around the SameSite attribute. Unless developers are checking their session cookie manually, they may not even notice their security settings aren't being propagated. This is on-top of potentially breaking applications due to browser enforcement of cookie attributes.

This fix looks good, and I'm hoping it can be merged in soon.

scarpenter-avenues commented 4 years ago

This would be super helpful. This issue is breaking an app I am developing for my school.

benshanahan1 commented 4 years ago

+1 This should be merged, especially with Chrome's recent removal of 3rd party cookie support with SameSite=None.