fix(db): refresh user after role update to avoid DetachedInstanceError (#6638)

Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>

Author: mvanhorn

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()
+        session.commit()

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
         )

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
         )

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"