[airflow] branch master updated: Prevent creating flask sessions on REST API requests (#15295)

[jh...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jhtimmins pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/airflow.git


The following commit(s) were added to refs/heads/master by this push:
     new 8711f90  Prevent creating flask sessions on REST API requests (#15295)
8711f90 is described below

commit 8711f90ab820ed420ef317b931e933a2062c891f
Author: Ephraim Anierobi <sp...@gmail.com>
AuthorDate: Mon Apr 19 22:24:20 2021 +0100

    Prevent creating flask sessions on REST API requests (#15295)
    
    * Prevent creating flask sessions on REST API requests
    
    Currently, flask sessions are created on API requests. This PR prevents it
    by setting a value in flask global object g and customizing cookie session
    creation
    
    * fixup! Prevent creating flask sessions on REST API requests
    
    * Move session interface modification logic to the security manager for easy debugging and customization
    
    * Update tests/api_connexion/test_security.py
    
    Co-authored-by: Kamil Breguła <mi...@users.noreply.github.com>
    
    * Add comment for better understanding
    
    * Remove login_from_api
    
    * Use blueprint to detect url
    
    * Move the DefaultSessionInterface to init_session and rename it
    
    Co-authored-by: Kamil Breguła <mi...@users.noreply.github.com>
---
 airflow/www/app.py                     |  3 ++-
 airflow/www/extensions/init_session.py | 22 +++++++++++++++-
 tests/api_connexion/test_security.py   | 47 ++++++++++++++++++++++++++++++++++
 3 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/airflow/www/app.py b/airflow/www/app.py
index 3302f36..632e1d2 100644
--- a/airflow/www/app.py
+++ b/airflow/www/app.py
@@ -35,7 +35,7 @@ from airflow.www.extensions.init_dagbag import init_dagbag
 from airflow.www.extensions.init_jinja_globals import init_jinja_globals
 from airflow.www.extensions.init_manifest_files import configure_manifest_files
 from airflow.www.extensions.init_security import init_api_experimental_auth, init_xframe_protection
-from airflow.www.extensions.init_session import init_permanent_session
+from airflow.www.extensions.init_session import init_airflow_session_interface, init_permanent_session
 from airflow.www.extensions.init_views import (
     init_api_connexion,
     init_api_experimental,
@@ -135,6 +135,7 @@ def create_app(config=None, testing=False):
         init_jinja_globals(flask_app)
         init_xframe_protection(flask_app)
         init_permanent_session(flask_app)
+        init_airflow_session_interface(flask_app)
     return flask_app
 
 
diff --git a/airflow/www/extensions/init_session.py b/airflow/www/extensions/init_session.py
index 37466c0..06e0ba5 100644
--- a/airflow/www/extensions/init_session.py
+++ b/airflow/www/extensions/init_session.py
@@ -15,7 +15,22 @@
 # specific language governing permissions and limitations
 # under the License.
 
-from flask import session as flask_session
+from flask import request, session as flask_session
+from flask.sessions import SecureCookieSessionInterface
+
+
+class AirflowSessionInterface(SecureCookieSessionInterface):
+    """
+    Airflow cookie session interface.
+    Modifications of sessions should be done here because
+    the change here is global.
+    """
+
+    def save_session(self, *args, **kwargs):
+        """Prevent creating session from REST API requests."""
+        if request.blueprint == '/api/v1':
+            return None
+        return super().save_session(*args, **kwargs)
 
 
 def init_permanent_session(app):
@@ -25,3 +40,8 @@ def init_permanent_session(app):
         flask_session.permanent = True
 
     app.before_request(make_session_permanent)
+
+
+def init_airflow_session_interface(app):
+    """Set airflow session interface"""
+    app.session_interface = AirflowSessionInterface()
diff --git a/tests/api_connexion/test_security.py b/tests/api_connexion/test_security.py
new file mode 100644
index 0000000..244a8a2
--- /dev/null
+++ b/tests/api_connexion/test_security.py
@@ -0,0 +1,47 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import pytest
+
+from airflow.security import permissions
+from tests.test_utils.api_connexion_utils import create_user, delete_user
+
+
+@pytest.fixture(scope="module")
+def configured_app(minimal_app_for_api):
+    app = minimal_app_for_api
+    create_user(
+        app,  # type:ignore
+        username="test",
+        role_name="Test",
+        permissions=[(permissions.ACTION_CAN_READ, permissions.RESOURCE_CONFIG)],  # type: ignore
+    )
+
+    yield minimal_app_for_api
+
+    delete_user(app, username="test")  # type: ignore
+
+
+class TestSession:
+    @pytest.fixture(autouse=True)
+    def setup_attrs(self, configured_app) -> None:
+        self.app = configured_app
+        self.client = self.app.test_client()  # type:ignore
+
+    def test_session_not_created_on_api_request(self):
+        self.client.get("api/v1/dags", environ_overrides={'REMOTE_USER': "test"})
+        assert all(cookie.name != "session" for cookie in self.client.cookie_jar)