[airflow] 01/03: Unify user session lifetime configuration (#11970)

[ka...@apache.org]

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

kaxilnaik pushed a commit to branch v1-10-test
in repository https://gitbox.apache.org/repos/asf/airflow.git

commit 765b6964c1ad539cf46ab5a561eb4df1d9b626f4
Author: Michał Misiewicz <mi...@gmail.com>
AuthorDate: Wed Nov 11 13:28:58 2020 +0100

    Unify user session lifetime configuration (#11970)
    
    * Unify user session lifetime configuration
    
    * align with new linting rules
    
    * exit app when removed args are provided in conf
    
    * add one more test
    
    * extract stopping gunicorn to method
    
    * add docstring to stop_webserver method
    
    * use lazy formatting
    
    * exit webserver when removed options are provided
    
    * align with markdown lint
    
    * Move unify user session lifetime configuration section to master
    
    * add new line
    
    * remove quotes
---
 UPDATING.md                                  | 23 +++++++++++++++++++++++
 airflow/config_templates/config.yml          | 19 ++++++-------------
 airflow/config_templates/default_airflow.cfg |  9 +++------
 airflow/www_rbac/app.py                      | 28 +++++++++++++++++-----------
 tests/www_rbac/test_app.py                   | 25 +++++++++++++++++++++++++
 5 files changed, 74 insertions(+), 30 deletions(-)

diff --git a/UPDATING.md b/UPDATING.md
index e2285df..6dbe8cc 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -78,6 +78,29 @@ support for SSL connection to HDFS).
 
 SSL support still works for WebHDFS hook.
 
+### Unify user session lifetime configuration
+
+In previous version of Airflow user session lifetime could be configured by
+`session_lifetime_days` and `force_log_out_after` options. In practise only `session_lifetime_days`
+had impact on session lifetime, but it was limited to values in day.
+We have removed mentioned options and introduced new `session_lifetime_minutes`
+option which simplify session lifetime configuration.
+
+Before
+
+ ```ini
+[webserver]
+force_log_out_after = 0
+session_lifetime_days = 30
+ ```
+
+After
+
+ ```ini
+[webserver]
+session_lifetime_minutes = 43200
+ ```
+
 ## Airflow 1.10.12
 
 ### Clearing tasks skipped by SkipMixin will skip them
diff --git a/airflow/config_templates/config.yml b/airflow/config_templates/config.yml
index d2621eb..e89df22 100644
--- a/airflow/config_templates/config.yml
+++ b/airflow/config_templates/config.yml
@@ -1001,21 +1001,14 @@
       type: string
       example: ~
       default: "True"
-    - name: force_log_out_after
+    - name: session_lifetime_minutes
       description: |
-        Minutes of non-activity before logged out from UI
-        0 means never get forcibly logged out
-      version_added: 1.10.8
-      type: string
-      example: ~
-      default: "0"
-    - name: session_lifetime_days
-      description: |
-        The UI cookie lifetime in days
-      version_added: 1.10.8
-      type: string
+        The UI cookie lifetime in minutes. User will be logged out from UI after
+        ``session_lifetime_minutes`` of non-activity
+      version_added: 1.10.13
+      type: int
       example: ~
-      default: "30"
+      default: "43200"
 
 - name: email
   description: ~
diff --git a/airflow/config_templates/default_airflow.cfg b/airflow/config_templates/default_airflow.cfg
index 71a5172..ea64d8f 100644
--- a/airflow/config_templates/default_airflow.cfg
+++ b/airflow/config_templates/default_airflow.cfg
@@ -484,12 +484,9 @@ x_frame_enabled = True
 # on webserver startup
 update_fab_perms = True
 
-# Minutes of non-activity before logged out from UI
-# 0 means never get forcibly logged out
-force_log_out_after = 0
-
-# The UI cookie lifetime in days
-session_lifetime_days = 30
+# The UI cookie lifetime in minutes. User will be logged out from UI after
+# ``session_lifetime_minutes`` of non-activity
+session_lifetime_minutes = 43200
 
 [email]
 email_backend = airflow.utils.email.send_email_smtp
diff --git a/airflow/www_rbac/app.py b/airflow/www_rbac/app.py
index 29a364b..0b1a8c7 100644
--- a/airflow/www_rbac/app.py
+++ b/airflow/www_rbac/app.py
@@ -18,6 +18,7 @@
 # under the License.
 #
 import logging
+import sys
 import socket
 from datetime import timedelta
 from typing import Any
@@ -62,8 +63,22 @@ def create_app(config=None, session=None, testing=False, app_name="Airflow"):
         )
     app.secret_key = conf.get('webserver', 'SECRET_KEY')
 
-    session_lifetime_days = conf.getint('webserver', 'SESSION_LIFETIME_DAYS', fallback=30)
-    app.config['PERMANENT_SESSION_LIFETIME'] = timedelta(days=session_lifetime_days)
+    if conf.has_option('webserver', 'SESSION_LIFETIME_DAYS') or conf.has_option(
+        'webserver', 'FORCE_LOG_OUT_AFTER'
+    ):
+        logging.error(
+            '`SESSION_LIFETIME_DAYS` option from `webserver` section has been '
+            'renamed to `SESSION_LIFETIME_MINUTES`. New option allows to configure '
+            'session lifetime in minutes. FORCE_LOG_OUT_AFTER option has been removed '
+            'from `webserver` section. Please update your configuration.'
+        )
+        # Stop gunicorn server https://github.com/benoitc/gunicorn/blob/20.0.4/gunicorn/arbiter.py#L526
+        # sys.exit(4)
+    else:
+        session_lifetime_minutes = conf.getint('webserver', 'SESSION_LIFETIME_MINUTES', fallback=43200)
+        logging.info('User session lifetime is set to %s minutes.', session_lifetime_minutes)
+
+    app.config['PERMANENT_SESSION_LIFETIME'] = timedelta(minutes=session_lifetime_minutes)
 
     app.config.from_pyfile(settings.WEBSERVER_CONFIG, silent=True)
     app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False
@@ -268,15 +283,6 @@ def create_app(config=None, session=None, testing=False, app_name="Airflow"):
         def shutdown_session(exception=None):
             settings.Session.remove()
 
-        @app.before_request
-        def before_request():
-            _force_log_out_after = conf.getint('webserver', 'FORCE_LOG_OUT_AFTER', fallback=0)
-            if _force_log_out_after > 0:
-                flask.session.permanent = True
-                app.permanent_session_lifetime = timedelta(minutes=_force_log_out_after)
-                flask.session.modified = True
-                flask.g.user = flask_login.current_user
-
         @app.after_request
         def apply_caching(response):
             _x_frame_enabled = conf.getboolean('webserver', 'X_FRAME_ENABLED', fallback=True)
diff --git a/tests/www_rbac/test_app.py b/tests/www_rbac/test_app.py
index 0fe8508..a0607cb 100644
--- a/tests/www_rbac/test_app.py
+++ b/tests/www_rbac/test_app.py
@@ -18,11 +18,13 @@
 # under the License.
 import json
 import unittest
+from datetime import timedelta
 
 import pytest
 import six
 from werkzeug.middleware.proxy_fix import ProxyFix
 
+from airflow.configuration import conf
 from airflow.settings import Session
 from airflow.www_rbac import app as application
 from tests.compat import mock
@@ -80,3 +82,26 @@ class TestApp(unittest.TestCase):
         if six.PY2:
             engine_params = json.dumps(engine_params)
         self.assertEqual(app.config['SQLALCHEMY_ENGINE_OPTIONS'], engine_params)
+
+    @conf_vars(
+        {
+            ('webserver', 'session_lifetime_minutes'): '3600',
+        }
+    )
+    @mock.patch("airflow.www_rbac.app.app", None)
+    def test_should_set_permanent_session_timeout(self):
+        app, _ = application.cached_appbuilder(testing=True)
+        self.assertEqual(app.config['PERMANENT_SESSION_LIFETIME'], timedelta(minutes=3600))
+
+    @conf_vars(
+        {
+            ('webserver', 'session_lifetime_days'): '30',
+            ('webserver', 'force_log_out_after'): '30',
+        }
+    )
+    @mock.patch("airflow.www_rbac.app.app", None)
+    def test_should_stop_app_when_removed_options_are_provided(self):
+        with self.assertRaises(SystemExit) as e:
+            conf.remove_option('webserver', 'session_lifetime_minutes')
+            application.cached_appbuilder(testing=True)
+        self.assertEqual(e.exception.code, 4)