This is an automated email from the ASF dual-hosted git repository.
EnxDev pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 1b0a37cf223 fix(users): stop update_me setting self-referential
changed_by_fk (#44866)
1b0a37cf223 is described below
commit 1b0a37cf223849abb7613d650ffec1fae464a02d
Author: Younes Beriane <[email protected]>
AuthorDate: Fri Oct 2 15:10:55 2026 +0200
fix(users): stop update_me setting self-referential changed_by_fk (#44866)
Co-authored-by: Claude Sonnet 5.5 <[email protected]>
---
superset/security/manager.py | 7 +++++++
superset/views/users/api.py | 1 -
.../test_superset_user_api_subject_sync.py | 23 ++++++++++++++++++++++
tests/unit_tests/views/test_current_user_api.py | 23 ++++++++++++++++++++++
4 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/superset/security/manager.py b/superset/security/manager.py
index ffdd95218a4..42d7d19b842 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -618,6 +618,13 @@ class SupersetUserApi(UserApi):
from superset.daos.user import UserDAO
item.roles = []
+ # Rows written before the self-referencing audit columns were guarded
+ # can point at themselves, which SQLAlchemy cannot order for DELETE
+ # (CircularDependencyError). Clear them so the delete can proceed.
+ for column in ("changed_by_fk", "created_by_fk"):
+ if getattr(item, column, None) == item.id:
+ setattr(item, column, None)
+ self.datamodel.session.flush()
UserDAO._delete_subject(item.id)
def post_add(self, item: Model) -> None:
diff --git a/superset/views/users/api.py b/superset/views/users/api.py
index 942e950c29c..bfec6f54593 100644
--- a/superset/views/users/api.py
+++ b/superset/views/users/api.py
@@ -50,7 +50,6 @@ class CurrentUserRestApi(BaseSupersetApi):
def pre_update(self, item: User, data: Dict[str, Any]) -> None:
item.changed_on = datetime.now()
- item.changed_by_fk = g.user.id
# Pop unconditionally: this key is only meaningful for verifying a
# password change below, and it isn't a real column on the user
# model -- it must never reach ``UserDAO.update``'s ``setattr`` loop.
diff --git a/tests/unit_tests/security/test_superset_user_api_subject_sync.py
b/tests/unit_tests/security/test_superset_user_api_subject_sync.py
index e7af3ace2b3..d074deb21fc 100644
--- a/tests/unit_tests/security/test_superset_user_api_subject_sync.py
+++ b/tests/unit_tests/security/test_superset_user_api_subject_sync.py
@@ -35,6 +35,7 @@ from __future__ import annotations
from types import SimpleNamespace
from unittest.mock import MagicMock, patch
+import pytest
from flask_appbuilder.models.sqla.interface import SQLAInterface
from flask_appbuilder.security.sqla.models import User
@@ -227,3 +228,25 @@ def
test_pre_update_failure_rolls_back_subject_sync_without_orphaning(
assert db.session.query(User).filter_by(id=user_id).one_or_none() is not
None
# But the subject sync this request attempted never landed.
assert _subject_for(user_id) is None
+
+
[email protected]("column", ["changed_by_fk", "created_by_fk"])
+def test_pre_delete_clears_self_referencing_audit_column(
+ column: str,
+ after_each: None, # noqa: F811
+) -> None:
+ """Existing installs can hold a user row whose ``changed_by_fk`` or
+ ``created_by_fk`` points at itself; SQLAlchemy cannot order the DELETE of
+ such a row (``CircularDependencyError``). ``pre_delete`` clears it so the
+ delete through ``SupersetUserApi`` succeeds.
+ """
+ user = _make_persisted_user(f"self_ref_{column}")
+ user_id = user.id
+ setattr(user, column, user_id)
+ db.session.commit()
+
+ api = _api_for()
+ api.pre_delete(user)
+ api.datamodel.delete(user)
+
+ assert db.session.query(User).filter_by(id=user_id).one_or_none() is None
diff --git a/tests/unit_tests/views/test_current_user_api.py
b/tests/unit_tests/views/test_current_user_api.py
index 7694e0f32c7..77c3ea6cbc1 100644
--- a/tests/unit_tests/views/test_current_user_api.py
+++ b/tests/unit_tests/views/test_current_user_api.py
@@ -245,3 +245,26 @@ def
test_update_me_password_change_clears_flag_in_the_same_unit_of_work(
mock_commit.assert_not_called()
assert attr.password_must_change is False
assert check_password_hash(admin_user.password, "BrandNewPassw0rd!")
+
+
+def test_update_me_does_not_set_self_referential_changed_by(
+ admin_user: User, # noqa: F811
+ after_each: None, # noqa: F811
+) -> None:
+ """A user editing their own profile must not end up with ``changed_by_fk``
+ pointing at their own row. ``User.changed_by`` is a self-referential
+ relationship without ``post_update``, so SQLAlchemy cannot order the
+ DELETE of such a row and raises ``CircularDependencyError``, which made
+ the user undeletable.
+ """
+ db.session.flush()
+
+ _run_update_me(admin_user, {"first_name": "Changed"})
+ db.session.flush()
+
+ assert admin_user.first_name == "Changed"
+ assert admin_user.changed_by_fk != admin_user.id
+
+ # The user can still be deleted.
+ db.session.delete(admin_user)
+ db.session.flush()
