This is an automated email from the ASF dual-hosted git repository.
jedcunningham pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 9a034e678b Rotate session id during login (#25771)
9a034e678b is described below
commit 9a034e678bc6b5ef771b803d0186119b6fb82e67
Author: Jed Cunningham <[email protected]>
AuthorDate: Thu Aug 18 06:52:02 2022 -0700
Rotate session id during login (#25771)
---
airflow/www/fab_security/manager.py | 15 +++++++++++++++
tests/www/views/test_session.py | 28 ++++++++++++++++++++++++++--
2 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/airflow/www/fab_security/manager.py
b/airflow/www/fab_security/manager.py
index 8399f11df3..3317fb014b 100644
--- a/airflow/www/fab_security/manager.py
+++ b/airflow/www/fab_security/manager.py
@@ -25,6 +25,7 @@ import json
import logging
import re
from typing import Any, Dict, List, Optional, Set, Tuple, Type
+from uuid import uuid4
from flask import current_app, g, session, url_for
from flask_appbuilder import AppBuilder
@@ -70,6 +71,7 @@ from flask_jwt_extended import JWTManager, current_user as
current_user_jwt
from flask_login import AnonymousUserMixin, LoginManager, current_user
from werkzeug.security import check_password_hash, generate_password_hash
+from airflow.configuration import conf
from airflow.www.fab_security.sqla.models import Action, Permission,
RegisterUser, Resource, Role, User
from airflow.www.views import ResourceModelView
@@ -854,6 +856,14 @@ class BaseSecurityManager:
user.fail_login_count += 1
self.update_user(user)
+ def _rotate_session_id(self):
+ """
+ Upon successful authentication when using the database session backend,
+ we need to rotate the session id
+ """
+ if conf.get('webserver', 'SESSION_BACKEND') == 'database':
+ session.sid = str(uuid4())
+
def auth_user_db(self, username, password):
"""
Method for authenticating user, auth db style
@@ -878,6 +888,7 @@ class BaseSecurityManager:
log.info(LOGMSG_WAR_SEC_LOGIN_FAILED.format(username))
return None
elif check_password_hash(user.password, password):
+ self._rotate_session_id()
self.update_user_auth_stat(user, True)
return user
else:
@@ -1174,6 +1185,7 @@ class BaseSecurityManager:
# LOGIN SUCCESS (only if user is now registered)
if user:
+ self._rotate_session_id()
self.update_user_auth_stat(user)
return user
else:
@@ -1201,6 +1213,7 @@ class BaseSecurityManager:
log.info(LOGMSG_WAR_SEC_LOGIN_FAILED.format(email))
return None
else:
+ self._rotate_session_id()
self.update_user_auth_stat(user)
return user
@@ -1230,6 +1243,7 @@ class BaseSecurityManager:
log.info(LOGMSG_WAR_SEC_LOGIN_FAILED.format(username))
return None
+ self._rotate_session_id()
self.update_user_auth_stat(user)
return user
@@ -1315,6 +1329,7 @@ class BaseSecurityManager:
# LOGIN SUCCESS (only if user is now registered)
if user:
+ self._rotate_session_id()
self.update_user_auth_stat(user)
return user
else:
diff --git a/tests/www/views/test_session.py b/tests/www/views/test_session.py
index 9fb6f36469..0753f2069a 100644
--- a/tests/www/views/test_session.py
+++ b/tests/www/views/test_session.py
@@ -15,6 +15,8 @@
# specific language governing permissions and limitations
# under the License.
+from unittest import mock
+
import pytest
from airflow.exceptions import AirflowConfigException
@@ -23,12 +25,16 @@ from tests.test_utils.config import conf_vars
from tests.test_utils.decorators import dont_initialize_flask_app_submodules
+def get_session_cookie(client):
+ return next((cookie for cookie in client.cookie_jar if cookie.name ==
'session'), None)
+
+
def test_session_cookie_created_on_login(user_client):
- assert any(cookie.name == 'session' for cookie in user_client.cookie_jar)
+ assert get_session_cookie(user_client) is not None
def test_session_inaccessible_after_logout(user_client):
- session_cookie = next((cookie for cookie in user_client.cookie_jar if
cookie.name == 'session'), None)
+ session_cookie = get_session_cookie(user_client)
assert session_cookie is not None
resp = user_client.get('/logout/')
@@ -63,3 +69,21 @@ def test_invalid_session_backend_option():
)
with pytest.raises(AirflowConfigException, match=expected_exc_regex):
poorly_configured_app_factory()
+
+
+def test_session_id_rotates(app, user_client):
+ old_session_cookie = get_session_cookie(user_client)
+ assert old_session_cookie is not None
+
+ resp = user_client.get('/logout/')
+ assert resp.status_code == 302
+
+ patch_path = "airflow.www.fab_security.manager.check_password_hash"
+ with mock.patch(patch_path) as check_password_hash:
+ check_password_hash.return_value = True
+ resp = user_client.post("/login/", data={"username": "test_user",
"password": "test_user"})
+ assert resp.status_code == 302
+
+ new_session_cookie = get_session_cookie(user_client)
+ assert new_session_cookie is not None
+ assert old_session_cookie.value != new_session_cookie.value
