Skip to content

Commit 0bcada4

Browse files
larrysingleton007larrysingleton007
committed
feat: Add optional OIDC token audience and issuer verification
The OIDC token parser verifies signature (JWKS via discovery) and expiry, but never the audience or issuer: any validly-signed, unexpired token from the configured IdP authenticates regardless of which resource it was minted for, leaving RBAC role matching as the only gate. Add two optional OidcAuthConfig fields, audience and issuer, both unset by default. When set, the corresponding claim must match or the token is rejected at authentication; when unset, the decode options are identical to before, so existing deployments are unaffected. Opt-in rather than strict-by-default because IdPs commonly mint tokens whose claims differ from the discovery metadata: Entra ID issues v1.0 tokens (iss under sts.windows.net, api:// audience) that are validated against a v2.0 discovery URL, which works because discovery is used only to source JWKS keys. A dedicated test pins that setup so it cannot silently regress. Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.com>
1 parent ca355cb commit 0bcada4

3 files changed

Lines changed: 226 additions & 3 deletions

File tree

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,15 @@ def _is_ssl_error(exc: BaseException) -> bool:
116116
return False
117117

118118
def _decode_token(self, access_token: str) -> dict:
119-
"""Fetch the JWKS signing key and decode + verify the JWT."""
119+
"""Fetch the JWKS signing key and decode + verify the JWT.
120+
121+
Signature and expiry are always verified. Audience and issuer are
122+
verified only when ``audience`` / ``issuer`` are set on
123+
``OidcAuthConfig``; both default to off, because the claim values a
124+
provider puts in the token can legitimately differ from its discovery
125+
metadata (e.g. Entra ID v1.0 tokens validated against a v2.0
126+
discovery document).
127+
"""
120128
optional_custom_headers = {"User-agent": "custom-user-agent"}
121129
ssl_ctx = ssl.create_default_context()
122130
if not self._auth_config.verify_ssl:
@@ -132,15 +140,21 @@ def _decode_token(self, access_token: str) -> dict:
132140
ssl_context=ssl_ctx,
133141
)
134142
signing_key = jwks_client.get_signing_key_from_jwt(access_token)
143+
expected_audience = self._auth_config.audience
144+
expected_issuer = self._auth_config.issuer
135145
return jwt.decode(
136146
access_token,
137147
signing_key.key,
138148
algorithms=["RS256"],
139-
audience="account",
149+
# "account" preserves the historical Keycloak-shaped default; it
150+
# is inert while verify_aud is off.
151+
audience=expected_audience if expected_audience is not None else "account",
152+
issuer=expected_issuer,
140153
options={
141-
"verify_aud": False,
154+
"verify_aud": expected_audience is not None,
142155
"verify_signature": True,
143156
"verify_exp": True,
157+
"verify_iss": expected_issuer is not None,
144158
},
145159
leeway=10, # accepts tokens generated up to 10 seconds in the past, in case of clock skew
146160
)

‎sdk/python/feast/permissions/auth_model.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@ class OidcAuthConfig(AuthConfig):
4040
ui_client_id: Optional[str] = None
4141
verify_ssl: bool = True
4242
ca_cert_path: str = ""
43+
# When set, incoming tokens must carry a matching `aud` / `iss` claim;
44+
# when left unset (the default), the corresponding claim is not verified.
45+
# Set these to the values your IdP puts in the token itself, which may
46+
# differ from the discovery document (e.g. Entra ID v1.0 tokens validated
47+
# against a v2.0 discovery URL).
48+
audience: Optional[str] = None
49+
issuer: Optional[str] = None
4350

4451

4552
class OidcClientAuthConfig(OidcAuthConfig):

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

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,19 @@
11
import asyncio
22
import os
3+
import time
34
from unittest import mock
45
from unittest.mock import MagicMock, patch
56

67
import assertpy
8+
import jwt
79
import pytest
810
from starlette.authentication import (
911
AuthenticationError,
1012
)
1113

1214
from feast.permissions.auth.kubernetes_token_parser import KubernetesTokenParser
1315
from feast.permissions.auth.oidc_token_parser import OidcTokenParser
16+
from feast.permissions.auth_model import OidcAuthConfig
1417
from feast.permissions.user import User
1518

1619
_CLIENT_ID = "test"
@@ -469,6 +472,205 @@ async def mock_oath2(self, request):
469472
assertpy.assert_that(user.has_matching_role(["updater"])).is_false()
470473

471474

475+
# ---------------------------------------------------------------------------
476+
# Optional audience / issuer verification (opt-in via OidcAuthConfig)
477+
# ---------------------------------------------------------------------------
478+
479+
480+
def _oidc_config_with(**overrides) -> OidcAuthConfig:
481+
return OidcAuthConfig(
482+
auth_discovery_url="https://localhost:8080/realms/master/.well-known/openid-configuration",
483+
client_id=_CLIENT_ID,
484+
type="oidc",
485+
**overrides,
486+
)
487+
488+
489+
@pytest.fixture(scope="module")
490+
def rsa_keypair() -> tuple:
491+
"""A real RSA keypair, so the aud/iss tests exercise the real ``jwt.decode``."""
492+
from cryptography.hazmat.primitives import serialization
493+
from cryptography.hazmat.primitives.asymmetric import rsa
494+
495+
private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
496+
private_pem = private_key.private_bytes(
497+
encoding=serialization.Encoding.PEM,
498+
format=serialization.PrivateFormat.PKCS8,
499+
encryption_algorithm=serialization.NoEncryption(),
500+
)
501+
public_pem = private_key.public_key().public_bytes(
502+
encoding=serialization.Encoding.PEM,
503+
format=serialization.PublicFormat.SubjectPublicKeyInfo,
504+
)
505+
return private_pem, public_pem
506+
507+
508+
def _make_token(private_pem: bytes, claims: dict) -> str:
509+
now = int(time.time())
510+
return jwt.encode(
511+
{"iat": now, "exp": now + 300, **claims}, private_pem, algorithm="RS256"
512+
)
513+
514+
515+
@pytest.mark.parametrize(
516+
"audience,issuer",
517+
[
518+
(None, None),
519+
("api://feast-server", None),
520+
(None, "https://idp.example.com/realm"),
521+
("api://feast-server", "https://idp.example.com/realm"),
522+
],
523+
)
524+
@patch(
525+
"feast.permissions.auth.oidc_token_parser.OAuth2AuthorizationCodeBearer.__call__"
526+
)
527+
@patch("feast.permissions.auth.oidc_token_parser.PyJWKClient.get_signing_key_from_jwt")
528+
@patch("feast.permissions.auth.oidc_token_parser.jwt.decode")
529+
@patch("feast.permissions.oidc_service.OIDCDiscoveryService._fetch_discovery_data")
530+
def test_oidc_decode_verification_options_follow_config(
531+
mock_discovery_data,
532+
mock_jwt,
533+
mock_signing_key,
534+
mock_oauth2,
535+
audience,
536+
issuer,
537+
discovery_data,
538+
signing_key,
539+
):
540+
"""The verified decode enables aud/iss checks exactly when the config
541+
provides expected values, and stays permissive otherwise."""
542+
mock_signing_key.return_value = signing_key
543+
mock_discovery_data.return_value = discovery_data
544+
mock_jwt.return_value = {"preferred_username": "my-name"}
545+
546+
token_parser = OidcTokenParser(
547+
auth_config=_oidc_config_with(audience=audience, issuer=issuer)
548+
)
549+
asyncio.run(token_parser.user_details_from_access_token(access_token="aaa-bbb-ccc"))
550+
551+
verified_calls = [
552+
c
553+
for c in mock_jwt.call_args_list
554+
if c.kwargs.get("options", {}).get("verify_signature") is not False
555+
]
556+
assertpy.assert_that(verified_calls).is_length(1)
557+
kwargs = verified_calls[0].kwargs
558+
assertpy.assert_that(kwargs["options"]["verify_aud"]).is_equal_to(
559+
audience is not None
560+
)
561+
assertpy.assert_that(kwargs["options"]["verify_iss"]).is_equal_to(
562+
issuer is not None
563+
)
564+
assertpy.assert_that(kwargs["audience"]).is_equal_to(
565+
audience if audience is not None else "account"
566+
)
567+
assertpy.assert_that(kwargs["issuer"]).is_equal_to(issuer)
568+
569+
570+
@pytest.mark.parametrize(
571+
"config_kwargs,claims,should_authenticate",
572+
[
573+
# Opt-in audience: match accepted, mismatch and missing rejected.
574+
({"audience": "api://feast-server"}, {"aud": "api://feast-server"}, True),
575+
({"audience": "api://feast-server"}, {"aud": "api://another-app"}, False),
576+
({"audience": "api://feast-server"}, {}, False),
577+
# Opt-in issuer: match accepted, mismatch rejected.
578+
(
579+
{"issuer": "https://idp.example.com/expected"},
580+
{"iss": "https://idp.example.com/expected"},
581+
True,
582+
),
583+
(
584+
{"issuer": "https://idp.example.com/expected"},
585+
{"iss": "https://idp.example.com/other"},
586+
False,
587+
),
588+
# Default config: neither claim is verified, so a token minted for a
589+
# different resource still authenticates (pre-existing behavior).
590+
({}, {"aud": "api://another-app", "iss": "https://idp.example.com/any"}, True),
591+
],
592+
)
593+
@patch(
594+
"feast.permissions.auth.oidc_token_parser.OAuth2AuthorizationCodeBearer.__call__"
595+
)
596+
@patch("feast.permissions.auth.oidc_token_parser.PyJWKClient.get_signing_key_from_jwt")
597+
@patch("feast.permissions.oidc_service.OIDCDiscoveryService._fetch_discovery_data")
598+
def test_oidc_audience_issuer_verification_end_to_end(
599+
mock_discovery_data,
600+
mock_signing_key,
601+
mock_oauth2,
602+
config_kwargs,
603+
claims,
604+
should_authenticate,
605+
discovery_data,
606+
rsa_keypair,
607+
):
608+
"""Real RS256-signed tokens through the real ``jwt.decode``: opt-in checks
609+
reject mismatched aud/iss and the default stays permissive."""
610+
private_pem, public_pem = rsa_keypair
611+
mock_discovery_data.return_value = discovery_data
612+
key = MagicMock()
613+
key.key = public_pem
614+
mock_signing_key.return_value = key
615+
616+
token = _make_token(private_pem, {"preferred_username": "my-name", **claims})
617+
token_parser = OidcTokenParser(auth_config=_oidc_config_with(**config_kwargs))
618+
619+
if should_authenticate:
620+
user = asyncio.run(
621+
token_parser.user_details_from_access_token(access_token=token)
622+
)
623+
assertpy.assert_that(user).is_type_of(User)
624+
if isinstance(user, User):
625+
assertpy.assert_that(user.username).is_equal_to("my-name")
626+
else:
627+
with pytest.raises(AuthenticationError):
628+
asyncio.run(token_parser.user_details_from_access_token(access_token=token))
629+
630+
631+
@patch(
632+
"feast.permissions.auth.oidc_token_parser.OAuth2AuthorizationCodeBearer.__call__"
633+
)
634+
@patch("feast.permissions.auth.oidc_token_parser.PyJWKClient.get_signing_key_from_jwt")
635+
@patch("feast.permissions.oidc_service.OIDCDiscoveryService._fetch_discovery_data")
636+
def test_oidc_default_supports_v1_tokens_against_v2_discovery(
637+
mock_discovery_data,
638+
mock_signing_key,
639+
mock_oauth2,
640+
discovery_data,
641+
rsa_keypair,
642+
):
643+
"""Pins the Entra ID v1-token-against-v2-discovery setup: with no expected
644+
audience or issuer configured, a v1.0-shaped app-only token (issuer under
645+
``sts.windows.net``, ``api://`` audience, ``appid`` identity) validates
646+
against a v2.0-style discovery document, because discovery is used only to
647+
source the JWKS signing keys. A future strict-by-default change would
648+
break real deployments and must fail here first."""
649+
private_pem, public_pem = rsa_keypair
650+
mock_discovery_data.return_value = discovery_data
651+
key = MagicMock()
652+
key.key = public_pem
653+
mock_signing_key.return_value = key
654+
655+
token = _make_token(
656+
private_pem,
657+
{
658+
"iss": "https://sts.windows.net/11111111-2222-3333-4444-555555555555/",
659+
"aud": "api://66666666-7777-8888-9999-000000000000",
660+
"appid": "client-app-id",
661+
"roles": ["reader"],
662+
},
663+
)
664+
token_parser = OidcTokenParser(auth_config=_oidc_config_with())
665+
666+
user = asyncio.run(token_parser.user_details_from_access_token(access_token=token))
667+
668+
assertpy.assert_that(user).is_type_of(User)
669+
if isinstance(user, User):
670+
assertpy.assert_that(user.username).is_equal_to("client-app-id")
671+
assertpy.assert_that(user.roles).is_equal_to(["reader"])
672+
673+
472674
# TODO RBAC: Move role bindings to a reusable fixture
473675
@patch("feast.permissions.auth.kubernetes_token_parser.config.load_incluster_config")
474676
@patch("feast.permissions.auth.kubernetes_token_parser.jwt.decode")

0 commit comments

Comments
 (0)