This is an automated email from the ASF dual-hosted git repository.
potiuk 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 ff601cb5b75 Compare session and user identifiers consistently when
resetting a password (#72198)
ff601cb5b75 is described below
commit ff601cb5b75e77c1f28aaf014914f4e9d5cb0947
Author: Jarek Potiuk <[email protected]>
AuthorDate: Fri Aug 28 22:49:17 2026 +0200
Compare session and user identifiers consistently when resetting a password
(#72198)
reset_user_sessions() decoded each stored session and compared its _user_id
against user.id directly. Flask-Login stores whatever User.get_id() returns,
which is a string, while user.id is the integer column, so the comparison
was
never true and no session was ever deleted. Both sides are now compared as
strings, and a session row without a _user_id is skipped rather than
matched.
The existing test passed because its fixture wrote an integer _user_id into
the session row by hand, exercising a comparison that does not occur in
practice. It now stores what get_id() returns. Added coverage for sessions
written before get_id() returned a string, and for a row carrying no
_user_id at all.
---
.../fab/auth_manager/security_manager/override.py | 7 +++-
.../fab/www/views/test_views_custom_user_views.py | 48 ++++++++++++++++++----
2 files changed, 47 insertions(+), 8 deletions(-)
diff --git
a/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py
b/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py
index 7e9ced387cf..2bb4d6ea53c 100644
---
a/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py
+++
b/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py
@@ -619,7 +619,12 @@ class
FabAirflowSecurityManagerOverride(AirflowSecurityManagerV2):
else:
for s in session.scalars(select(user_session_model)).all():
session_details =
interface.serializer.decode(want_bytes(s.data))
- if session_details.get("_user_id") == user.id:
+ session_user_id = session_details.get("_user_id")
+ # Flask-Login stores whatever ``User.get_id()`` returns,
which is a
+ # string, while ``user.id`` is the integer column. Compare
both sides
+ # as strings so the two representations match; older
sessions written
+ # before ``get_id()`` returned a string are still handled.
+ if session_user_id is not None and str(session_user_id) ==
str(user.id):
session.delete(s)
session.commit()
else:
diff --git
a/providers/fab/tests/unit/fab/www/views/test_views_custom_user_views.py
b/providers/fab/tests/unit/fab/www/views/test_views_custom_user_views.py
index 7e61cdff785..2cdc69bf834 100644
--- a/providers/fab/tests/unit/fab/www/views/test_views_custom_user_views.py
+++ b/providers/fab/tests/unit/fab/www/views/test_views_custom_user_views.py
@@ -259,7 +259,7 @@ class TestResetUserSessions:
delete_user(app, "user_to_delete_1")
delete_user(app, "user_to_delete_2")
- def create_user_db_session(self, session_id: str, time_delta: timedelta,
user_id: int):
+ def create_user_db_session(self, session_id: str, time_delta: timedelta,
user_id: str | int):
self.session.add(
self.model(
session_id=session_id,
@@ -277,8 +277,8 @@ class TestResetUserSessions:
],
)
def test_reset_user_sessions_delete(self, time_delta: timedelta,
user_sessions_deleted: bool):
- self.create_user_db_session("session_id_1", time_delta, self.user_1.id)
- self.create_user_db_session("session_id_2", time_delta, self.user_2.id)
+ self.create_user_db_session("session_id_1", time_delta,
self.user_1.get_id())
+ self.create_user_db_session("session_id_2", time_delta,
self.user_2.get_id())
self.session.commit()
self.session.flush()
assert
self.session.scalar(select(func.count()).select_from(self.model)) == 2
@@ -296,6 +296,40 @@ class TestResetUserSessions:
assert
self.session.scalar(select(func.count()).select_from(self.model)) == 2
assert self.get_session_by_id("session_id_1") is not None
+ def test_reset_user_sessions_delete_legacy_integer_user_id(self):
+ """Sessions written before ``get_id()`` returned a string stored an
int."""
+ self.create_user_db_session("session_id_1", timedelta(days=1),
self.user_1.id)
+ self.create_user_db_session("session_id_2", timedelta(days=1),
self.user_2.id)
+ self.session.commit()
+ self.session.flush()
+
+ with self.app.app_context():
+ self.security_manager.reset_password(self.user_1.id,
"new_password")
+ self.session.commit()
+ self.session.flush()
+
+ assert self.get_session_by_id("session_id_1") is None
+ assert self.get_session_by_id("session_id_2") is not None
+
+ def test_reset_user_sessions_ignores_sessions_without_user_id(self):
+ """A session row with no ``_user_id`` must not raise and must not be
deleted."""
+ self.session.add(
+ self.model(
+ session_id="session_id_anon",
+ data=self.serializer.encode({}),
+ expiry=datetime.now() + timedelta(days=1),
+ )
+ )
+ self.session.commit()
+ self.session.flush()
+
+ with self.app.app_context():
+ self.security_manager.reset_password(self.user_1.id,
"new_password")
+ self.session.commit()
+ self.session.flush()
+
+ assert self.get_session_by_id("session_id_anon") is not None
+
def get_session_by_id(self, session_id: str):
return
self.session.scalar(select(self.model).where(self.model.session_id ==
session_id))
@@ -307,8 +341,8 @@ class TestResetUserSessions:
"airflow.providers.fab.auth_manager.security_manager.override.MAX_NUM_DATABASE_USER_SESSIONS",
1
)
def test_refuse_delete(self, _mock_has_context, flash_mock):
- self.create_user_db_session("session_id_1", timedelta(days=1),
self.user_1.id)
- self.create_user_db_session("session_id_2", timedelta(days=1),
self.user_2.id)
+ self.create_user_db_session("session_id_1", timedelta(days=1),
self.user_1.get_id())
+ self.create_user_db_session("session_id_2", timedelta(days=1),
self.user_2.get_id())
self.session.commit()
self.session.flush()
assert
self.session.scalar(select(func.count()).select_from(self.model)) == 2
@@ -344,8 +378,8 @@ class TestResetUserSessions:
"airflow.providers.fab.auth_manager.security_manager.override.MAX_NUM_DATABASE_USER_SESSIONS",
1
)
def test_refuse_delete_cli(self, log_mock):
- self.create_user_db_session("session_id_1", timedelta(days=1),
self.user_1.id)
- self.create_user_db_session("session_id_2", timedelta(days=1),
self.user_2.id)
+ self.create_user_db_session("session_id_1", timedelta(days=1),
self.user_1.get_id())
+ self.create_user_db_session("session_id_2", timedelta(days=1),
self.user_2.get_id())
self.session.commit()
self.session.flush()
assert
self.session.scalar(select(func.count()).select_from(self.model)) == 2
