Skip to content

Commit e185cf0

Browse files
larrysingleton007ntkathole
larrysingleton007
authored and
ntkathole
committed
fix: Address review nitpicks on Entra OIDC PR
- Type the discovery_data test fixture as Dict[str, str]. - Split the Entra group-claim caveat into its own bullet in authz_manager.md. - Log the token's claim keys at debug level before raising on a missing username claim, to aid diagnosis (keys only, no values). Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.com>
1 parent 36dc33e commit e185cf0

3 files changed

Lines changed: 7 additions & 2 deletions

File tree

‎docs/getting-started/components/authz_manager.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ Some assumptions are made in the OIDC server configuration:
4747
* The roles are exposed in the access token under `resource_access.<client_id>.roles` (Keycloak) or in the top-level `roles` claim (Entra ID app roles). Roles found in both are merged.
4848
* The JWT token is expected to have a verified signature and not be expired. The Feast OIDC token parser logic validates for `verify_signature` and `verify_exp` so make sure that the given OIDC provider is configured to meet these requirements.
4949
* The username is read from the first of `preferred_username`, `upn`, `azp`, `appid`, `sub` present in the token. Entra ID client-credentials (app-only) tokens carry no user claim, so they authenticate as the calling application.
50-
* For `GroupBasedPolicy` support, the `groups` claim should be present in the access token (requires a "Group Membership" protocol mapper in Keycloak). Entra ID emits group object IDs (GUIDs) in this claim rather than names, and omits it once a principal exceeds Entra's group overage limit, so on Entra a `GroupBasedPolicy` must reference those IDs and cannot rely on the claim for very large group memberships.
50+
* For `GroupBasedPolicy` support, the `groups` claim should be present in the access token (requires a "Group Membership" protocol mapper in Keycloak).
51+
* **Entra ID limitation**: Group claims use object IDs (GUIDs) instead of names, and are omitted entirely when a user exceeds the group overage threshold. GroupBasedPolicy must reference GUIDs and cannot be used for principals with large group memberships.
5152

5253
(*) Please note that **the role match is case-sensitive**, e.g. the name of the role in the OIDC server and in the `Permission` configuration
5354
must be exactly the same.

‎sdk/python/feast/permissions/auth/oidc_token_parser.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@ def _extract_username_or_raise_error(data: dict) -> str:
7878
value = data.get(claim)
7979
if isinstance(value, str):
8080
return value
81+
logger.debug(
82+
f"No usable username claim; token claims present: {list(data.keys())}"
83+
)
8184
raise AuthenticationError(
8285
"Missing username claim in access token: expected one of "
8386
"preferred_username, upn, azp, appid or sub."

‎sdk/python/tests/unit/permissions/auth/conftest.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
from typing import Dict
12
from unittest.mock import MagicMock
23

34
import pytest
@@ -65,7 +66,7 @@ def oidc_config() -> OidcAuthConfig:
6566

6667

6768
@pytest.fixture
68-
def discovery_data() -> dict:
69+
def discovery_data() -> Dict[str, str]:
6970
return {
7071
"authorization_endpoint": "https://localhost:8080/realms/master/protocol/openid-connect/auth",
7172
"token_endpoint": "https://localhost:8080/realms/master/protocol/openid-connect/token",

0 commit comments

Comments
 (0)