Skip to content

Commit f843c63

Browse files
larrysingleton007larrysingleton007
authored
fix: Support Entra ID (Azure AD) token claims in OIDC auth (#6631)
* fix: Support Entra ID (Azure AD) token claims in OIDC auth The OIDC token parser was written against Keycloak's token shape, which rejected Microsoft Entra ID tokens in two independent ways. Username extraction accepted only preferred_username or upn. Entra client-credentials (app-only) tokens carry neither, so every machine-to-machine caller failed authentication. Fall back to the calling application's own identity when no human claim is present: azp on v2 tokens, appid on v1, then sub, which every issuer sets. A token with none of the five claims still raises AuthenticationError. Roles were read only from Keycloak's nested resource_access.<client_id>.roles. Entra emits app roles in the top-level roles claim, so the extracted list was always empty and RoleBasedPolicy could never match. Merge the top-level claim into the existing extraction, preserving order and dropping duplicates. Both changes are additive, so Keycloak behavior is unchanged: the username fallbacks only fire when preferred_username and upn are both absent, and the merge only adds roles. Token validation is untouched. Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.com> * docs: Document Entra ID claim support in OIDC authorization The OIDC assumptions listed for the auth manager described only Keycloak's token shape. Record that roles are also read from the top-level roles claim and merged, and that the username falls back through upn, azp, appid and sub when preferred_username is absent. Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.com> * fix: Harden Entra ID OIDC claim handling from review Refine the initial Entra ID support in response to review: - Merge roles with list(dict.fromkeys(...)), the codebase's order- preserving dedup idiom (feast/utils.py), so duplicates within the top-level roles claim are also collapsed, not only cross-claim ones. - Skip username claims whose value is null or non-string instead of returning them, so a present-but-null early claim no longer shadows a usable later claim; the -> str contract now holds. - Correct the _extract_username_or_raise_error docstring: the raise fires only when a token provides none of the five claims as a string. Extend the tests: intra-claim role de-duplication, human-claim precedence over appid/sub, and rejection of tokens whose only identity claims are null or non-string. Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.com> * docs: Add Entra ID token example and group-claim caveat Add an app-only (client-credentials) Entra ID token example alongside the Keycloak one, and note that Entra emits group object IDs (GUIDs) rather than names and omits the groups claim under the overage limit, so GroupBasedPolicy on Entra must reference those IDs. Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.com> * test: Cover OIDC verified-decode sourcing and trim redundant asserts Add a test that gives the unverified routing decode and the verified decode different payloads and asserts identity and roles come from the verified one, so a regression that read claims from the unverified decode would be caught. Drop the has_matching_role assertions in the roles-merge test: the exact roles-list equality already pins the result, and has_matching_role has its own coverage in test_user.py. Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.com> * test: Extract shared OIDC discovery and signing-key fixtures The OIDC token-parser tests each re-declared the same mock discovery document and JWKS signing key. Move both into conftest.py fixtures and have the tests consume them, removing the repeated setup blocks. Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.com> * 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> --------- Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.com>
1 parent 0399380 commit f843c63

4 files changed

Lines changed: 305 additions & 62 deletions

File tree

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,11 @@ The server, in turn, uses the same OIDC server to validate the token and extract
4444

4545
Some assumptions are made in the OIDC server configuration:
4646
* The OIDC token refers to a client with roles matching the RBAC roles of the configured `Permission`s (*)
47-
* The roles are exposed in the access token under `resource_access.<client_id>.roles`
47+
* 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.
49-
* The `preferred_username` should be part of the JWT token claim.
49+
* 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.
5050
* 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.
@@ -69,6 +70,16 @@ For example, the access token for a client `app` of a user with `reader` role an
6970
}
7071
```
7172

73+
A Microsoft Entra ID (Azure AD) client-credentials (app-only) token has no user claim; the application authenticates as itself, and its app roles arrive in the top-level `roles` claim:
74+
```json
75+
{
76+
"azp": "11111111-2222-3333-4444-555555555555",
77+
"roles": [
78+
"reader"
79+
]
80+
}
81+
```
82+
7283
#### Server-Side Configuration
7384

7485
The server requires `auth_discovery_url` and `client_id` to validate incoming JWT tokens via JWKS:

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

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,17 +61,29 @@ async def _validate_token(self, access_token: str):
6161

6262
@staticmethod
6363
def _extract_username_or_raise_error(data: dict) -> str:
64-
"""Extract the username from the decoded JWT. Raises if missing — identity is mandatory.
65-
66-
Checks ``preferred_username`` first (Keycloak default), then falls back
67-
to ``upn`` (Azure AD / Entra ID).
64+
"""Extract the username from the decoded JWT, or raise if the token
65+
carries none of the recognised identity claims.
66+
67+
Human identity claims take precedence: ``preferred_username`` (Keycloak
68+
default), then ``upn`` (Azure AD / Entra ID). Client-credentials
69+
(app-only) tokens carry neither, so the calling application's own
70+
identity is used instead: ``azp`` (Entra ID v2 tokens), ``appid``
71+
(Entra ID v1 tokens), and finally ``sub``, which every issuer sets.
72+
A claim present with a null or non-string value is skipped rather than
73+
returned, so it never shadows a usable later claim. Because ``sub`` is a
74+
mandatory string JWT claim, the raise below fires only for a malformed
75+
token that provides none of the five as a string.
6876
"""
69-
if "preferred_username" in data:
70-
return data["preferred_username"]
71-
if "upn" in data:
72-
return data["upn"]
77+
for claim in ("preferred_username", "upn", "azp", "appid", "sub"):
78+
value = data.get(claim)
79+
if isinstance(value, str):
80+
return value
81+
logger.debug(
82+
f"No usable username claim; token claims present: {list(data.keys())}"
83+
)
7384
raise AuthenticationError(
74-
"Missing preferred_username or upn field in access token."
85+
"Missing username claim in access token: expected one of "
86+
"preferred_username, upn, azp, appid or sub."
7587
)
7688

7789
@staticmethod
@@ -194,6 +206,10 @@ async def user_details_from_access_token(self, access_token: str) -> User:
194206
if self._auth_config.client_id
195207
else []
196208
)
209+
# Entra ID emits app roles in the top-level `roles` claim instead of
210+
# Keycloak's nested `resource_access.<client_id>.roles`. Merge both
211+
# shapes for every issuer, preserving order and dropping duplicates.
212+
roles = list(dict.fromkeys(roles + self._extract_claim(data, "roles")))
197213
groups = self._extract_claim(data, "groups")
198214

199215
logger.info(

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
from typing import Dict
2+
from unittest.mock import MagicMock
3+
14
import pytest
25
from kubernetes import client
36

@@ -62,6 +65,22 @@ def oidc_config() -> OidcAuthConfig:
6265
)
6366

6467

68+
@pytest.fixture
69+
def discovery_data() -> Dict[str, str]:
70+
return {
71+
"authorization_endpoint": "https://localhost:8080/realms/master/protocol/openid-connect/auth",
72+
"token_endpoint": "https://localhost:8080/realms/master/protocol/openid-connect/token",
73+
"jwks_uri": "https://localhost:8080/realms/master/protocol/openid-connect/certs",
74+
}
75+
76+
77+
@pytest.fixture
78+
def signing_key() -> MagicMock:
79+
key = MagicMock()
80+
key.key = "a-key"
81+
return key
82+
83+
6584
@pytest.fixture(
6685
scope="module",
6786
params=[

0 commit comments

Comments
 (0)