pallets-eco / flask-session

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

Every CORS preflight request generates a Session in storage #193

Closed magzim21 closed 5 months ago

magzim21 commented 9 months ago
# this is hack to prevent Session extension to generate session per anonymous CORS preflight request (they are always anonymous)
@app.route('/graphql', methods=['OPTIONS'])
def options_for_your_endpoint():
    # Return appropriate CORS headers
    response = app.make_default_options_response()
    response.delete_cookie('session')
    session.clear()
    return response
Lxstr commented 7 months ago

@magzim21 thanks for the issue. I've suspected something like this is occurring. We'll look into

kgutwin commented 7 months ago

I was working through some of this today, and I think I can explain what is happening. For example, in the SqlAlchemySessionInterface, in save_session(), there is this code block which presumably tries to avoid writing a new session when the session is empty:

https://github.com/pallets-eco/flask-session/blob/a9d001ae9b40268c9189c468bb136e535d3358e1/src/flask_session/sessions.py#L578-L586

However, the implementation of SessionMixin stores the permanent attribute in the session dictionary:

https://github.com/pallets/flask/blob/d61198941adcb191ddb591f08d7d912e40bde8bc/src/flask/sessions.py#L21-L31

Since this attribute is set every time ServerSideSession is instantiated (which happens on every request, in open_session()) the value of bool(session) is always True and therefore a session is always saved on every request.

In my implementation, I've added this to ServerSideSession, which only allows bool(session) to return True if there is a key in the session dictionary beside "_permanent":

    def __bool__(self) -> bool:
        return bool(dict(self)) and self.keys() != {"_permanent"}
Lxstr commented 7 months ago

@kgutwin thanks for that! I'm unfortunately out of action coding wise for at least a week or two but If you or anyone else are willing to do some more, I'd be grateful. Do you know if/how this relates to #149 or #82 ?

Lxstr commented 6 months ago

@kgutwin Thanks again for this. Greatly helped clarify what was a multi-faceted issue. I've implemented some changes in development branch at this stage, would be glad for inputs.

Lxstr commented 5 months ago

Thanks @kgutwin for this code contribution, it is incorporated into 0.6.0