Merge pull request #1 from pacnpal/copilot/fix-admin-role-claim-issues

pacnpal · web-flow · commit cf6f69ba9fcd · 2026-03-09T14:35:19.000-04:00
Fix OIDC login downgrading existing user roles when provider sends no role claims
diff --git a/backend/handler/auth/base_handler.py b/backend/handler/auth/base_handler.py
@@ -337,7 +337,9 @@ async def get_current_active_user_from_openid_token(self, token: Any):
             )
 
         role = Role.VIEWER
+        claims_provided = False
         if OIDC_CLAIM_ROLES and OIDC_CLAIM_ROLES in userinfo:
+            claims_provided = True
             roles = userinfo[OIDC_CLAIM_ROLES] or []
             if OIDC_ROLE_ADMIN and OIDC_ROLE_ADMIN in roles:
                 role = Role.ADMIN
@@ -368,7 +370,7 @@ async def get_current_active_user_from_openid_token(self, token: Any):
                 role=role,
             )
             user = db_user_handler.add_user(new_user)
-        elif OIDC_CLAIM_ROLES and user.role != role:
+        elif claims_provided and user.role != role:
             user = db_user_handler.update_user(user.id, {"role": role})
 
         if not user.enabled:
diff --git a/backend/tests/handler/auth/test_oidc.py b/backend/tests/handler/auth/test_oidc.py
@@ -350,6 +350,38 @@ async def test_oidc_token_without_email_verified_claim(
     assert userinfo.get("email") == mock_jwt_payload.claims.get("email")
 
 
+async def test_oidc_valid_no_edit_user_role_if_claim_not_in_userinfo(
+    mocker,
+    mock_oidc_enabled,
+    mock_token,
+    mock_openid_configuration,
+):
+    """Test that role is not changed for existing user on login if OIDC role claim is configured but not provided by the OIDC provider."""
+    mocker.patch("handler.auth.base_handler.OIDC_CLAIM_ROLES", "roles")
+    mocker.patch("handler.auth.base_handler.OIDC_ROLE_ADMIN", "admin")
+    # The OIDC provider does NOT include the roles claim in userinfo
+    mock_token["userinfo"].pop("roles", None)
+    mock_user = MagicMock(enabled=True, role=Role.ADMIN)
+    mocker.patch(
+        "handler.database.db_user_handler.get_user_by_email", return_value=mock_user
+    )
+    mock_edit_user = mocker.patch(
+        "handler.database.db_user_handler.update_user", return_value=mock_user
+    )
+    mocker.patch.object(
+        StarletteOAuth2App,
+        "load_server_metadata",
+        return_value=mock_openid_configuration,
+    )
+
+    oidc_handler = OpenIDHandler()
+    user, _ = await oidc_handler.get_current_active_user_from_openid_token(mock_token)
+
+    # The user's existing ADMIN role should be preserved
+    mock_edit_user.assert_not_called()
+    assert user.role == Role.ADMIN
+
+
 async def test_oidc_invalid_token_signature(mock_oidc_enabled):
     """Test token decoding raises exception for invalid signature."""
     oidc_handler = OpenIDHandler()
