diff --git a/docs/getting-started/components/authz_manager.md b/docs/getting-started/components/authz_manager.md index 7dbb5470f5f..abc97594e61 100644 --- a/docs/getting-started/components/authz_manager.md +++ b/docs/getting-started/components/authz_manager.md @@ -45,7 +45,7 @@ The server, in turn, uses the same OIDC server to validate the token and extract Some assumptions are made in the OIDC server configuration: * The OIDC token refers to a client with roles matching the RBAC roles of the configured `Permission`s (*) * The roles are exposed in the access token under `resource_access..roles` (Keycloak) or in the top-level `roles` claim (Entra ID app roles). Roles found in both are merged. -* 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. +* 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. The token's audience and issuer claims are **not** verified by default; both checks can be enabled with the `audience` and `issuer` options (see [Server-Side Configuration](#server-side-configuration)). * 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. * For `GroupBasedPolicy` support, the `groups` claim should be present in the access token (requires a "Group Membership" protocol mapper in Keycloak). * **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. @@ -105,6 +105,23 @@ auth: Setting `verify_ssl: false` disables TLS certificate verification for all OIDC provider communication (discovery, JWKS, token endpoint). Only use this in development or internal environments where you accept the security risk. {% endhint %} +By default the server verifies only the token's signature and expiry: any validly-signed, unexpired token from the configured provider is accepted regardless of the audience it was minted for, and authorization (role matching) is the only remaining gate. For defense in depth, set `audience` and/or `issuer` to additionally require a matching `aud` / `iss` claim: + +```yaml +auth: + type: oidc + client_id: _CLIENT_ID_ + auth_discovery_url: https://login.example.com/.well-known/openid-configuration + audience: api://feast-feature-server + issuer: https://login.example.com/realms/master +``` + +A token whose `aud` (or `iss`) claim does not match is rejected at authentication. The two options are independent; leave one unset to skip that check. + +{% hint style="warning" %} +Set these to the values your IdP puts **in the token itself**, which are not always the ones in the discovery document. For example, Microsoft Entra ID commonly issues v1.0 tokens (`iss: https://sts.windows.net//`, `aud: api://`) even when `auth_discovery_url` points at the v2.0 endpoint. That setup keeps working with these options unset, or set to the v1.0 values — but copying the v2.0 issuer from the discovery document would reject every v1.0 token. +{% endhint %} + #### Client-Side Configuration The client supports multiple token source modes. The SDK resolves tokens in the following priority order: diff --git a/sdk/python/feast/permissions/auth/oidc_token_parser.py b/sdk/python/feast/permissions/auth/oidc_token_parser.py index 0f4f1b6e676..c02ec2b81ef 100644 --- a/sdk/python/feast/permissions/auth/oidc_token_parser.py +++ b/sdk/python/feast/permissions/auth/oidc_token_parser.py @@ -116,7 +116,15 @@ def _is_ssl_error(exc: BaseException) -> bool: return False def _decode_token(self, access_token: str) -> dict: - """Fetch the JWKS signing key and decode + verify the JWT.""" + """Fetch the JWKS signing key and decode + verify the JWT. + + Signature and expiry are always verified. Audience and issuer are + verified only when ``audience`` / ``issuer`` are set on + ``OidcAuthConfig``; both default to off, because the claim values a + provider puts in the token can legitimately differ from its discovery + metadata (e.g. Entra ID v1.0 tokens validated against a v2.0 + discovery document). + """ optional_custom_headers = {"User-agent": "custom-user-agent"} ssl_ctx = ssl.create_default_context() if not self._auth_config.verify_ssl: @@ -132,15 +140,21 @@ def _decode_token(self, access_token: str) -> dict: ssl_context=ssl_ctx, ) signing_key = jwks_client.get_signing_key_from_jwt(access_token) + expected_audience = self._auth_config.audience + expected_issuer = self._auth_config.issuer return jwt.decode( access_token, signing_key.key, algorithms=["RS256"], - audience="account", + # "account" preserves the historical Keycloak-shaped default; it + # is inert while verify_aud is off. + audience=expected_audience if expected_audience is not None else "account", + issuer=expected_issuer, options={ - "verify_aud": False, + "verify_aud": expected_audience is not None, "verify_signature": True, "verify_exp": True, + "verify_iss": expected_issuer is not None, }, leeway=10, # accepts tokens generated up to 10 seconds in the past, in case of clock skew ) diff --git a/sdk/python/feast/permissions/auth_model.py b/sdk/python/feast/permissions/auth_model.py index 105d6d01acc..03d65c5973d 100644 --- a/sdk/python/feast/permissions/auth_model.py +++ b/sdk/python/feast/permissions/auth_model.py @@ -40,6 +40,13 @@ class OidcAuthConfig(AuthConfig): ui_client_id: Optional[str] = None verify_ssl: bool = True ca_cert_path: str = "" + # When set, incoming tokens must carry a matching `aud` / `iss` claim; + # when left unset (the default), the corresponding claim is not verified. + # Set these to the values your IdP puts in the token itself, which may + # differ from the discovery document (e.g. Entra ID v1.0 tokens validated + # against a v2.0 discovery URL). + audience: Optional[str] = None + issuer: Optional[str] = None class OidcClientAuthConfig(OidcAuthConfig): diff --git a/sdk/python/tests/unit/permissions/auth/test_token_parser.py b/sdk/python/tests/unit/permissions/auth/test_token_parser.py index 41f201672dd..8f0c82367d5 100644 --- a/sdk/python/tests/unit/permissions/auth/test_token_parser.py +++ b/sdk/python/tests/unit/permissions/auth/test_token_parser.py @@ -1,9 +1,11 @@ import asyncio import os +import time from unittest import mock from unittest.mock import MagicMock, patch import assertpy +import jwt import pytest from starlette.authentication import ( AuthenticationError, @@ -11,6 +13,7 @@ from feast.permissions.auth.kubernetes_token_parser import KubernetesTokenParser from feast.permissions.auth.oidc_token_parser import OidcTokenParser +from feast.permissions.auth_model import OidcAuthConfig from feast.permissions.user import User _CLIENT_ID = "test" @@ -469,6 +472,205 @@ async def mock_oath2(self, request): assertpy.assert_that(user.has_matching_role(["updater"])).is_false() +# --------------------------------------------------------------------------- +# Optional audience / issuer verification (opt-in via OidcAuthConfig) +# --------------------------------------------------------------------------- + + +def _oidc_config_with(**overrides) -> OidcAuthConfig: + return OidcAuthConfig( + auth_discovery_url="https://localhost:8080/realms/master/.well-known/openid-configuration", + client_id=_CLIENT_ID, + type="oidc", + **overrides, + ) + + +@pytest.fixture(scope="module") +def rsa_keypair() -> tuple: + """A real RSA keypair, so the aud/iss tests exercise the real ``jwt.decode``.""" + from cryptography.hazmat.primitives import serialization + from cryptography.hazmat.primitives.asymmetric import rsa + + private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + private_pem = private_key.private_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PrivateFormat.PKCS8, + encryption_algorithm=serialization.NoEncryption(), + ) + public_pem = private_key.public_key().public_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PublicFormat.SubjectPublicKeyInfo, + ) + return private_pem, public_pem + + +def _make_token(private_pem: bytes, claims: dict) -> str: + now = int(time.time()) + return jwt.encode( + {"iat": now, "exp": now + 300, **claims}, private_pem, algorithm="RS256" + ) + + +@pytest.mark.parametrize( + "audience,issuer", + [ + (None, None), + ("api://feast-server", None), + (None, "https://idp.example.com/realm"), + ("api://feast-server", "https://idp.example.com/realm"), + ], +) +@patch( + "feast.permissions.auth.oidc_token_parser.OAuth2AuthorizationCodeBearer.__call__" +) +@patch("feast.permissions.auth.oidc_token_parser.PyJWKClient.get_signing_key_from_jwt") +@patch("feast.permissions.auth.oidc_token_parser.jwt.decode") +@patch("feast.permissions.oidc_service.OIDCDiscoveryService._fetch_discovery_data") +def test_oidc_decode_verification_options_follow_config( + mock_discovery_data, + mock_jwt, + mock_signing_key, + mock_oauth2, + audience, + issuer, + discovery_data, + signing_key, +): + """The verified decode enables aud/iss checks exactly when the config + provides expected values, and stays permissive otherwise.""" + mock_signing_key.return_value = signing_key + mock_discovery_data.return_value = discovery_data + mock_jwt.return_value = {"preferred_username": "my-name"} + + token_parser = OidcTokenParser( + auth_config=_oidc_config_with(audience=audience, issuer=issuer) + ) + asyncio.run(token_parser.user_details_from_access_token(access_token="aaa-bbb-ccc")) + + verified_calls = [ + c + for c in mock_jwt.call_args_list + if c.kwargs.get("options", {}).get("verify_signature") is not False + ] + assertpy.assert_that(verified_calls).is_length(1) + kwargs = verified_calls[0].kwargs + assertpy.assert_that(kwargs["options"]["verify_aud"]).is_equal_to( + audience is not None + ) + assertpy.assert_that(kwargs["options"]["verify_iss"]).is_equal_to( + issuer is not None + ) + assertpy.assert_that(kwargs["audience"]).is_equal_to( + audience if audience is not None else "account" + ) + assertpy.assert_that(kwargs["issuer"]).is_equal_to(issuer) + + +@pytest.mark.parametrize( + "config_kwargs,claims,should_authenticate", + [ + # Opt-in audience: match accepted, mismatch and missing rejected. + ({"audience": "api://feast-server"}, {"aud": "api://feast-server"}, True), + ({"audience": "api://feast-server"}, {"aud": "api://another-app"}, False), + ({"audience": "api://feast-server"}, {}, False), + # Opt-in issuer: match accepted, mismatch rejected. + ( + {"issuer": "https://idp.example.com/expected"}, + {"iss": "https://idp.example.com/expected"}, + True, + ), + ( + {"issuer": "https://idp.example.com/expected"}, + {"iss": "https://idp.example.com/other"}, + False, + ), + # Default config: neither claim is verified, so a token minted for a + # different resource still authenticates (pre-existing behavior). + ({}, {"aud": "api://another-app", "iss": "https://idp.example.com/any"}, True), + ], +) +@patch( + "feast.permissions.auth.oidc_token_parser.OAuth2AuthorizationCodeBearer.__call__" +) +@patch("feast.permissions.auth.oidc_token_parser.PyJWKClient.get_signing_key_from_jwt") +@patch("feast.permissions.oidc_service.OIDCDiscoveryService._fetch_discovery_data") +def test_oidc_audience_issuer_verification_end_to_end( + mock_discovery_data, + mock_signing_key, + mock_oauth2, + config_kwargs, + claims, + should_authenticate, + discovery_data, + rsa_keypair, +): + """Real RS256-signed tokens through the real ``jwt.decode``: opt-in checks + reject mismatched aud/iss and the default stays permissive.""" + private_pem, public_pem = rsa_keypair + mock_discovery_data.return_value = discovery_data + key = MagicMock() + key.key = public_pem + mock_signing_key.return_value = key + + token = _make_token(private_pem, {"preferred_username": "my-name", **claims}) + token_parser = OidcTokenParser(auth_config=_oidc_config_with(**config_kwargs)) + + if should_authenticate: + user = asyncio.run( + token_parser.user_details_from_access_token(access_token=token) + ) + assertpy.assert_that(user).is_type_of(User) + if isinstance(user, User): + assertpy.assert_that(user.username).is_equal_to("my-name") + else: + with pytest.raises(AuthenticationError): + asyncio.run(token_parser.user_details_from_access_token(access_token=token)) + + +@patch( + "feast.permissions.auth.oidc_token_parser.OAuth2AuthorizationCodeBearer.__call__" +) +@patch("feast.permissions.auth.oidc_token_parser.PyJWKClient.get_signing_key_from_jwt") +@patch("feast.permissions.oidc_service.OIDCDiscoveryService._fetch_discovery_data") +def test_oidc_default_supports_v1_tokens_against_v2_discovery( + mock_discovery_data, + mock_signing_key, + mock_oauth2, + discovery_data, + rsa_keypair, +): + """Pins the Entra ID v1-token-against-v2-discovery setup: with no expected + audience or issuer configured, a v1.0-shaped app-only token (issuer under + ``sts.windows.net``, ``api://`` audience, ``appid`` identity) validates + against a v2.0-style discovery document, because discovery is used only to + source the JWKS signing keys. A future strict-by-default change would + break real deployments and must fail here first.""" + private_pem, public_pem = rsa_keypair + mock_discovery_data.return_value = discovery_data + key = MagicMock() + key.key = public_pem + mock_signing_key.return_value = key + + token = _make_token( + private_pem, + { + "iss": "https://sts.windows.net/11111111-2222-3333-4444-555555555555/", + "aud": "api://66666666-7777-8888-9999-000000000000", + "appid": "client-app-id", + "roles": ["reader"], + }, + ) + token_parser = OidcTokenParser(auth_config=_oidc_config_with()) + + user = asyncio.run(token_parser.user_details_from_access_token(access_token=token)) + + assertpy.assert_that(user).is_type_of(User) + if isinstance(user, User): + assertpy.assert_that(user.username).is_equal_to("client-app-id") + assertpy.assert_that(user.roles).is_equal_to(["reader"]) + + # TODO RBAC: Move role bindings to a reusable fixture @patch("feast.permissions.auth.kubernetes_token_parser.config.load_incluster_config") @patch("feast.permissions.auth.kubernetes_token_parser.jwt.decode")