Allow to instansiate Session with custom session interface

[yaelmi3]

We're using Flask-Session with MongoDBSessionInterface and it's great! For our own internal requirements, we needed to extend the interface capabilities, by inheriting from the MongoDBSessionInterface (see usage examples below). The current Session signature does not allow us to use the custom interface (same goes for init_app) and we're first to first instantiate the Session with the "built-in" MongoDBSessionInterface and then replace it with our Custom one, in the following manner:

    app.config['SESSION_TYPE'] = 'mongodb'
    app.config['SESSION_MONGODB'] = mongo_instance.mongo_client_scans
    app.config['SESSION_MONGODB_DB'] = 'mayhem_db'
    app.config['SESSION_MONGODB_COLLECT'] = 'mayhem_sessions'
    Session(app)
    app.session_interface = CustomMongoDBSessionInterface(app=app, client=mongo_instance.mongo_client_scans,
                                                          db='mayhem_db',
                                                          collection='mayhem_sessions')

Could be nice addition to allow custom interface on Session instantiation. PR for this enhancement: https://github.com/pallets-eco/flask-session/pull/233

Usage examples:

class CustomMongoDBSessionInterface(MongoDBSessionInterface):
    def should_set_cookie(self, app, session):
        """
        Do not set cookie for healthz endpoint
        """
        if request.path in ['/healthz']:
            return False
        return super().should_set_cookie(app, session)

    @retry(retry_on_exception=lambda e: isinstance(e, ServerSelectionTimeoutError), stop_max_attempt_number=3)
    def _retrieve_session_data(self, sid):
        """
        Retry on mongo server selection timeout error
        """
        try:
            return super()._retrieve_session_data(sid)
        except ServerSelectionTimeoutError:
            logger.warning("Failed to connect to mongo. Attempting to reconnect.")
            mongo_instance.mongo_client_scans.server_info()
            raise

[Lxstr]

Thanks for the issue, when removing the null session I didn't realise that this made trying to subclass session interface more difficult. There would be two options, either merge your PR or bring back the null session. I disliked the null session because it gave no clear error message if you inadvertently misconfigured. Quart-session uses it in a more clear way but I'm still leaning towards merging your PR at this stage.

[Lxstr]

I am curious about your overrides if you don't mind explaining more.

I didn't implement a retry on mongo because I thought it was included by default in 3.9 https://pymongo.readthedocs.io/en/stable/changelog.html#changes-in-version-3-9-0, is this coverage inadequate for ServerSelectionTimeoutError?

Regarding the healthcheck endpoint, what flask-session version are you using and what information is being saved to session? I thought I had updated the code to ignore an endpoint if it doesn't add anything to session and (assuming nothing was added to session in the previous endpoint visited).

I am wondering if there are enough edge cases that we should have an option to ignore endpoints in flask-session such as SESSION_IGNORE = ["/healthz", "terms"]

[yaelmi3]

Thanks for the quick reply! Must admit I wasn't a fan of the NullSession either, since from the user's POV it's an implicit action that might indeed to wrong assumptions and misconfigurations. About my overrides:

1. The pymongo retry addition stemmed from a production incident, where the mongo server was restarted and connection was lost completely. If I'm not mistaken, retryWrites and retryReads aimed to perform a retry due to network failure, which is not the case when the server is restarted.
2. The healthz check skip addition was added when we started using Flask-Session~=0.6.0 (flask~=2.2.5, Werkzeug==3.0.1, uwsgi==2.0.22). Before this skip, it added new session entry 5 sec, since these checks are coming from K8s on the pod. The endpoint itself is very simple and session is not used in its context.

   @app.route("/healthz")
   def health_check():
   return "", HTTPStatus.NO_CONTENT

[Lxstr]

1. Thats seems reasonable. We do have a retries decorator for SQL which retries on any exception, we could possibly also apply that to mongo.

2. I am slightly concerned about how you could be getting new entries when not adding anything to session, even back in 0.6.0 which still check the boolean evaluation of session to see if it is not empty (ignoring the permanent flag) before continuing at all. I wonder if you could test the bool of session in the health check?

[yaelmi3]

I can test it. What trace should I add to the code?

[Lxstr]

Think just

@app.route("/healthz")
def health_check():
    print(session)
    print(session.__bool__())
    return "", HTTPStatus.NO_CONTENT

[Lxstr]

Result should be:

<RedisSession {'_permanent': True}>
False

With nothing saved to session storage, although I am using latest version

[yaelmi3]

So I got some interesting results here and I am in fault here :) I forgot to mention that we use @app.before_request for each request, for logging purposes (we use a custom DD logger) and some FlaskForm instantiation, this is the func:

@app.before_request
def before_request():
    if request.full_path not in FILTERED_PATHS:
        logger.bind(user=get_user_name(profile_name=True))
        try:
            logger.info("New request", path=request.full_path, method=request.method,
                        **request.form)
        except Exception:  # pylint: disable=broad-except
            logger.exception("Failed to parse request", path=request.full_path, method=request.method)
            return redirect(url_for("index"))
    search_form = SearchForm(**request.args)
    if search_form.redirect_url:
        return redirect(url_for(search_form.redirect_url))

When healthz endpoint is called without this decorator, we get False session as expected

<MongoDBSession {'_permanent': True}>
False

What causes the session to be altered is the FlaskForm usage, here's the simplified version of the func:

class TestForm(FlaskForm):
    pass

@app.before_request
def before_request():
    TestForm(**request.args)

The result is

<MongoDBSession {'_permanent': True, 'csrf_token': 'd8d5e36426706e6475622598b3273d3305af635e'}>
True

I'm a bit surprised to see that simply calling the FlaskForm class causes changes in the session, but it's obvious that it sets csrf_token in the session, so it makes some sense, I guess. Regardless to this finding, healthz call doesn't require Form creation on our end, so it's a minor bug, which obsoletes the override in the session interface

[Lxstr]

Ok cool, glad we figured it out. I am aware that csrf can be causing this kind of thing and I could probably add a note in the docs. Hopefully we don't have enough need to add the SESSION_IGNORE feature.

A possible feature might be SESSION_CLIENTSIDE_KEYS=["csrf_token"], which would put those keys on the client side and avoid having storage fill up. Quart-session has something similar.

Also for the override, can you try deleting all of the following lines, everything should ultimately be supplied to session_interface.

app.config['SESSION_TYPE'] = 'mongodb' app.config['SESSION_MONGODB'] = mongo_instance.mongo_client_scans app.config['SESSION_MONGODB_DB'] = 'mayhem_db' app.config['SESSION_MONGODB_COLLECT'] = 'mayhem_sessions' Session(app) app.session_interface = CustomMongoDBSessionInterface(app=app, client=mongo_instance.mongo_client_scans, db='mayhem_db', collection='mayhem_sessions')

[Lxstr]

Regarding the generic retry decorator, I'll look into adding it to the mongo methods.

[yaelmi3]

Yes, your suggestion indeed works. I will close my PR, since the above is the simplified and more elegant solution to my issue. Also, I've removed the should_set_cookie and skipping FlaskForm for healthz check altogether in the before_request

Thanks for your help!

[Lxstr]

No worries, glad it helps!