[GitHub] [airflow] ephraimbuddy commented on pull request #15295: Prevent creating flask sessions on REST API requests

[GitBox <gi...@apache.org>]

ephraimbuddy commented on pull request #15295:
URL: https://github.com/apache/airflow/pull/15295#issuecomment-816614188


   > There are two approaches I would choose from to deal with the session interface setup. The first is to make the session interface an setup global to the entire Airflow web app. The session interface would simply be named `AirflowSessionInterface`, and the `app.session_intercace = AirflowSessionInterface()` line moved to `airflow.www.app.create_app` to reflect the fact the configuration is not local to `api_connextion`.
   > 
   > The other approach is to take a composite approach, and make `app.session_interface` additive instead of overwriting it, so the session interface provided by `api_connextion` add to the existing interface instead of overwriting. Something like:
   > 
   > ```python
   > class APIConnexionSessionInterface:
   >     """Session interface that avoids creating session from API requests.
   > 
   >     Breifly explain how this is done (by setting ``g.login_from_api`` on user creation,
   >     and reading this before session creation to avoid it when we already loaded a user).
   >     """
   >     def __init__(self, wrapped: SessionInterface) -> None:
   >         self._wrapped = wrapped
   > 
   >     @user_loaded_from_header.connect
   >     def user_loaded_from_header(self, user=None):
   >         g.login_from_api = True
   > 
   >     def open_session(self, app, request):
   >         return self._wrapped.open_session(app, request)
   > 
   >     def save_session(self, app, session, response):
   >         if g.get('login_from_api'):
   >             return None
   >         return self._wrapped.save_session(app, session, response)
   > 
   > def init_api_connexion(app: Flask) -> None:
   >     ...
   >     app.session_interface = APIConnexionInterface(app.session_interface)
   > ```
   > 
   > This makes the custom session interface local to the `api_connextion` module, and allows other modules to also modify the session interface by composition.
   
   I think that we should have this implementation as it is. The reason is that apart from the REST API we would not have the need of modifying the 'flask' session or stopping its creation in another module.  What do you think?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org