From 767fec9dbe1c19b2bfc45089f5dc1c7946f36a30 Mon Sep 17 00:00:00 2001 From: Matt Van Horn Date: Mon, 20 Jul 2026 10:18:51 -0700 Subject: [PATCH] fix(db): refresh user after role update to avoid DetachedInstanceError (#6638) Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> --- keep/api/core/db.py | 7 +++- .../test_filtering_sort_search_on_alerts.py | 13 +++--- ...test_filtering_sort_search_on_incidents.py | 13 +++--- tests/test_change_password.py | 41 +++++++++++++++++++ 4 files changed, 60 insertions(+), 14 deletions(-) diff --git a/keep/api/core/db.py b/keep/api/core/db.py index 878d0d877b..6cda3595a6 100644 --- a/keep/api/core/db.py +++ b/keep/api/core/db.py @@ -2139,10 +2139,13 @@ def update_user_role(tenant_id, username, role): .where(User.tenant_id == tenant_id) .where(User.username == username) ).first() - if user and user.role != role: + if not user: + return None + if user.role != role: user.role = role session.add(user) session.commit() + session.refresh(user) return user @@ -5989,4 +5992,4 @@ def recover_prev_alert_status(alert: Alert, session: Optional[Session] = None): ) ) session.exec(query) - session.commit() \ No newline at end of file + session.commit() diff --git a/tests/e2e_tests/incidents_alerts_tests/test_filtering_sort_search_on_alerts.py b/tests/e2e_tests/incidents_alerts_tests/test_filtering_sort_search_on_alerts.py index 2eb1558363..dfbde94000 100644 --- a/tests/e2e_tests/incidents_alerts_tests/test_filtering_sort_search_on_alerts.py +++ b/tests/e2e_tests/incidents_alerts_tests/test_filtering_sort_search_on_alerts.py @@ -66,7 +66,9 @@ def init_test(browser: Page, alerts, max_retries=3): else: raise e - browser.wait_for_selector("[data-testid='facet-value']", timeout=10000) + browser.get_by_role("main").locator("[data-testid='facet-value']").first.wait_for( + timeout=30000 + ) browser.wait_for_selector(f"text={alerts[0]['name']}", timeout=10000) rows_count = browser.locator("[data-testid='alerts-table'] table tbody tr").count() # check that required alerts are loaded and displayed @@ -88,9 +90,10 @@ def select_one_facet_option(browser, facet_name, option_name): def assert_facet(browser, facet_name, alerts, alert_property_name: str): counters_dict = {} - expect( - browser.locator("[data-testid='facet']", has_text=facet_name) - ).to_be_visible() + facet_locator = browser.get_by_role("main").locator( + "[data-testid='facet']", has_text=facet_name + ) + expect(facet_locator).to_be_visible() for alert in alerts: prop_value = None for prop in alert_property_name.split("."): @@ -106,8 +109,6 @@ def assert_facet(browser, facet_name, alerts, alert_property_name: str): counters_dict[prop_value] += 1 for facet_value, count in counters_dict.items(): - facet_locator = browser.locator("[data-testid='facet']", has_text=facet_name) - expect(facet_locator).to_be_visible() facet_value_locator = facet_locator.locator( "[data-testid='facet-value']", has_text=facet_value ) diff --git a/tests/e2e_tests/incidents_alerts_tests/test_filtering_sort_search_on_incidents.py b/tests/e2e_tests/incidents_alerts_tests/test_filtering_sort_search_on_incidents.py index b13afc7e44..c149254823 100644 --- a/tests/e2e_tests/incidents_alerts_tests/test_filtering_sort_search_on_incidents.py +++ b/tests/e2e_tests/incidents_alerts_tests/test_filtering_sort_search_on_incidents.py @@ -32,7 +32,9 @@ def init_test(browser: Page, incidents, max_retries=3): else: raise e - browser.wait_for_selector("[data-testid='facet-value']") + browser.get_by_role("main").locator("[data-testid='facet-value']").first.wait_for( + timeout=30000 + ) browser.wait_for_selector("table[data-testid='incidents-table']") @@ -65,9 +67,10 @@ def select_one_facet_option(browser, facet_name, option_name): def assert_facet(browser, facet_name, alerts, alert_property_name: str): counters_dict = {} - expect( - browser.locator("[data-testid='facet']", has_text=facet_name) - ).to_be_visible() + facet_locator = browser.get_by_role("main").locator( + "[data-testid='facet']", has_text=facet_name + ) + expect(facet_locator).to_be_visible() for alert in alerts: prop_value = None for prop in alert_property_name.split("."): @@ -86,8 +89,6 @@ def assert_facet(browser, facet_name, alerts, alert_property_name: str): counters_dict[value] += 1 for facet_value, count in counters_dict.items(): - facet_locator = browser.locator("[data-testid='facet']", has_text=facet_name) - expect(facet_locator).to_be_visible() facet_value_locator = facet_locator.locator( "[data-testid='facet-value']", has_text=facet_value ) diff --git a/tests/test_change_password.py b/tests/test_change_password.py index 88665dce88..5aaf3a8853 100644 --- a/tests/test_change_password.py +++ b/tests/test_change_password.py @@ -177,3 +177,44 @@ def test_admin_can_reset_user_password_via_update(db_session, client, test_app): # managed_user can sign in with new password assert _signin(client, "managed_user", "resetpass").status_code == 200 assert _signin(client, "managed_user", "initialpass").status_code == 401 + + +@pytest.mark.parametrize( + "test_app", + [{"AUTH_TYPE": "DB", "KEEP_JWT_SECRET": "somesecret"}], + indirect=True, +) +def test_admin_can_update_user_role_via_update(db_session, client, test_app): + """An admin can update a local user's role via the update endpoint.""" + _create_db_user(db_session, "admin_user", "adminpass", role="admin") + _create_db_user(db_session, "managed_user", "managedpass", role="noc") + + signin = _signin(client, "admin_user", "adminpass") + assert signin.status_code == 200 + token = signin.json()["accessToken"] + headers = {"Authorization": f"Bearer {token}"} + + response = client.put( + "/auth/users/managed_user", + json={"role": "admin"}, + headers=headers, + ) + assert response.status_code == 200 + assert response.json()["role"] == "admin" + assert _signin(client, "managed_user", "managedpass").json()["role"] == "admin" + + response = client.put( + "/auth/users/managed_user", + json={"role": "admin"}, + headers=headers, + ) + assert response.status_code == 200 + assert response.json()["role"] == "admin" + + response = client.put( + "/auth/users/missing_user", + json={"role": "admin"}, + headers=headers, + ) + assert response.status_code == 404 + assert response.json()["detail"] == "User not found"