Get groups from OIDC before falling back to LDAP

hluk · hluk · commit cf61e200f91c · 2026-03-26T13:02:35.000+01:00
JIRA: RHELWF-13972

Assisted-by: Claude Code
diff --git a/resultsdb/authorization.py b/resultsdb/authorization.py
@@ -40,22 +40,49 @@ def match_testcase_permissions(testcase, permissions):
                 yield permission
 
 
-def verify_authorization(user, testcase, permissions, ldap_host, ldap_searches):
+def verify_authorization(
+    user, testcase, permissions, ldap_host, ldap_searches, oidc_groups=None
+):
     """
     Raises an exception if the user is not permitted to publish a result for
     the testcase.
     """
-    if not (ldap_host and ldap_searches):
-        raise InternalServerError(
-            "LDAP_HOST and LDAP_SEARCHES also need to be defined if PERMISSIONS is defined"
-        )
-
     allowed_groups = []
     for permission in match_testcase_permissions(testcase, permissions):
         if user in permission.get("users", []):
             return
         allowed_groups += permission.get("groups", [])
 
+    # OIDC groups check (no network calls needed)
+    if oidc_groups is not None:
+        if not oidc_groups:
+            log.warning(
+                "OIDC token for user %s contains an empty groups claim; "
+                "falling back to LDAP for test case %s",
+                user,
+                testcase,
+            )
+        elif not set(oidc_groups).isdisjoint(allowed_groups):
+            return
+        else:
+            log.warning(
+                "OIDC groups %r did not match any allowed groups %r for user %s "
+                "and test case %s; falling back to LDAP",
+                oidc_groups,
+                allowed_groups,
+                user,
+                testcase,
+            )
+
+    if not (ldap_host and ldap_searches):
+        if oidc_groups is None:
+            raise InternalServerError(
+                "LDAP_HOST and LDAP_SEARCHES also need to be defined if PERMISSIONS is defined"
+            )
+        raise Forbidden(
+            f"User {user} is not authorized to submit results for the test case {testcase}"
+        )
+
     try:
         import ldap
     except ImportError:
@@ -73,6 +100,14 @@ def verify_authorization(user, testcase, permissions, ldap_host, ldap_searches):
     for cur_ldap_search in ldap_searches:
         groups = get_group_membership(ldap, user, con, cur_ldap_search)
         if any(g in groups for g in allowed_groups):
+            if oidc_groups is not None:
+                log.warning(
+                    "LDAP authorized user %s for test case %s. "
+                    "Consider adding the appropriate groups to the user's "
+                    "OIDC token to avoid LDAP dependency.",
+                    user,
+                    testcase,
+                )
             return
         any_groups_found = any_groups_found or len(groups) > 0
 
diff --git a/resultsdb/config.py b/resultsdb/config.py
@@ -88,6 +88,7 @@ class Config:
 
     OIDC_CLIENT_SECRETS = "/etc/resultsdb/oauth2_client_secrets.json"
     OIDC_USERNAME_FIELD = "uid"
+    OIDC_GROUPS_FIELD = "realm_access.roles"
     OIDC_SESSION_REFRESH_INTERVAL_SECONDS = 300
     OIDC_SESSION_PERMANENT = False
     PERMANENT_SESSION_LIFETIME = 300
diff --git a/resultsdb/controllers/api_v3.py b/resultsdb/controllers/api_v3.py
@@ -28,15 +28,46 @@ def permissions():
     return app.config.get("PERMISSIONS", [])
 
 
+def get_oidc_groups():
+    groups_field = app.config.get("OIDC_GROUPS_FIELD")
+    if not groups_field:
+        return None
+
+    token_identity = app.oidc.current_token_identity
+
+    value = token_identity
+    for key in groups_field.split("."):
+        if not isinstance(value, dict):
+            app.logger.warning(
+                "OIDC groups claim %r not found in token (failed at %r)",
+                groups_field,
+                key,
+            )
+            return None
+        value = value.get(key)
+        if value is None:
+            app.logger.warning(
+                "OIDC groups claim %r not found in token (missing key %r)",
+                groups_field,
+                key,
+            )
+            return None
+
+    return value
+
+
 def get_authorized_user(testcase) -> str:
     """
     Raises an exception if the current user cannot publish a result for the
     testcase, otherwise returns the name of the current user.
     """
     user = app.oidc.current_token_identity[app.config["OIDC_USERNAME_FIELD"]]
+    oidc_groups = get_oidc_groups()
     ldap_host = app.config.get("LDAP_HOST")
     ldap_searches = app.config.get("LDAP_SEARCHES")
-    verify_authorization(user, testcase, permissions(), ldap_host, ldap_searches)
+    verify_authorization(
+        user, testcase, permissions(), ldap_host, ldap_searches, oidc_groups
+    )
     return user
 
 
diff --git a/tests/test_api_v3.py b/tests/test_api_v3.py
@@ -433,6 +433,110 @@ def test_api_v3_permission_no_groups_found(client, permissions, mock_ldap, caplo
     assert f"Permission denied: {expected_error}" in caplog.text
 
 
+def test_api_v3_permission_oidc_groups_match(client, permissions, mock_ldap, app):
+    """OIDC groups match — authorized without LDAP."""
+    permissions.append(
+        {
+            "groups": ["testgroup1"],
+            "testcases": ["testcase1*"],
+        }
+    )
+    app.oidc.current_token_identity = {
+        "uid": "testuser1",
+        "realm_access": {"roles": ["testgroup1"]},
+    }
+    data = brew_build_request_data()
+    r = client.post("/api/v3/results/brew-builds", json=data)
+    assert r.status_code == 201, r.text
+    mock_ldap.search_s.assert_not_called()
+    app.oidc.current_token_identity = {"uid": "testuser1"}
+
+
+def test_api_v3_permission_oidc_groups_no_match_ldap_fallback(
+    client, permissions, mock_ldap, caplog, app
+):
+    """OIDC groups don't match, LDAP authorizes with deprecation warning."""
+    permissions.append(
+        {
+            "groups": ["testgroup1"],
+            "testcases": ["testcase1*"],
+        }
+    )
+    app.oidc.current_token_identity = {
+        "uid": "testuser1",
+        "realm_access": {"roles": ["other_group"]},
+    }
+    data = brew_build_request_data()
+    r = client.post("/api/v3/results/brew-builds", json=data)
+    assert r.status_code == 201, r.text
+    mock_ldap.search_s.assert_called_once()
+    assert "falling back to LDAP" in caplog.text
+    assert "LDAP authorized" in caplog.text
+    app.oidc.current_token_identity = {"uid": "testuser1"}
+
+
+def test_api_v3_permission_oidc_groups_no_match_ldap_denies(
+    client, permissions, mock_ldap, caplog, app
+):
+    """OIDC groups don't match, LDAP doesn't authorize — Forbidden."""
+    permissions.append(
+        {
+            "groups": ["testgroup1"],
+            "testcases": ["testcase1*"],
+        }
+    )
+    app.oidc.current_token_identity = {
+        "uid": "testuser1",
+        "realm_access": {"roles": ["other_group"]},
+    }
+    mock_ldap.search_s.return_value = []
+    data = brew_build_request_data()
+    r = client.post("/api/v3/results/brew-builds", json=data)
+    assert r.status_code == 403, r.text
+    assert "falling back to LDAP" in caplog.text
+    app.oidc.current_token_identity = {"uid": "testuser1"}
+
+
+def test_api_v3_permission_oidc_groups_match_without_ldap(
+    client, permissions, mock_ldap, app
+):
+    """OIDC groups match — authorized even when LDAP is not configured."""
+    permissions.append(
+        {
+            "groups": ["testgroup1"],
+            "testcases": ["testcase1*"],
+        }
+    )
+    app.oidc.current_token_identity = {
+        "uid": "testuser1",
+        "realm_access": {"roles": ["testgroup1"]},
+    }
+    data = brew_build_request_data()
+    with patch.dict(app.config, {"LDAP_HOST": None, "LDAP_SEARCHES": None}):
+        r = client.post("/api/v3/results/brew-builds", json=data)
+    assert r.status_code == 201, r.text
+    mock_ldap.search_s.assert_not_called()
+    app.oidc.current_token_identity = {"uid": "testuser1"}
+
+
+def test_api_v3_permission_oidc_groups_none_falls_back_to_ldap(
+    client, permissions, mock_ldap, app
+):
+    """No OIDC groups claim — falls back to LDAP as before."""
+    permissions.append(
+        {
+            "groups": ["testgroup1"],
+            "testcases": ["testcase1*"],
+        }
+    )
+    # current_token_identity has no realm_access — so get_oidc_groups returns None
+    app.oidc.current_token_identity = {"uid": "testuser1"}
+    data = brew_build_request_data()
+    r = client.post("/api/v3/results/brew-builds", json=data)
+    assert r.status_code == 201, r.text
+    mock_ldap.search_s.assert_called_once()
+
+
 @pytest.mark.parametrize("params_class", RESULTS_PARAMS_CLASSES)
 def test_api_v3_consistency(params_class, client):
     """
