fix(auth_v2): close mTLS spoofing, introspection audience, SAML replay, and deactivated-user gaps

Address the SSO and credential-flow security review findings:

- mTLS (HIGH): the forwarded subject-DN header was trusted unconditionally, so
  any caller could send it and mint a service-account principal. Trust it only
  when the immediate peer is inside trusted_proxy_cidrs (same model as XFF) and
  fail closed otherwise; the ASGI-TLS-extension path already fails closed when
  no verified cert is present.
- OAuth2 introspection (HIGH): RFC 7662 responses were accepted regardless of
  audience. Enforce the response aud against OAuth2IntrospectionConfig.audience
  and reject active tokens whose audience does not match.
- SAML (HIGH): default allow_unsolicited to False so IdP-initiated/login-CSRF
  responses are rejected, add a single-use assertion-id replay cache, and bind
  the post-login redirect to the RelayState stored against the matched
  InResponseTo request rather than trusting the echoed form field.
- Deactivated users (M1): the resolver now rejects a credential that resolves to
  a SCIM user with active=False, so deactivation actually blocks authentication.
- Stop carrying underscore-prefixed carrier keys (raw api key, basic password)
  into Principal.claims, which is documented for audit logging.
This commit is contained in:
Yassin Kortam 2026-06-10 18:52:26 -07:00
parent ca896ac073
commit 6f3fc5eba4
6 changed files with 75 additions and 16 deletions

View file

@ -23,6 +23,7 @@ from .config import (
MutualTlsConfig,
OAuth2IntrospectionConfig,
OidcProviderConfig,
TrustedProxyConfig,
)
from .models import (
AuthMethod,
@ -31,6 +32,7 @@ from .models import (
CredentialRef,
SecuritySchemeType,
)
from .network import ip_in_trusted_proxies
AT_JWT_TYPES = {"at+jwt", "application/at+jwt"}
@ -323,12 +325,15 @@ class OAuth2Authenticator:
body = response.json()
if not body.get("active"):
raise errors.invalid_token("token inactive")
token_audience = _normalize_audience(body.get("aud"))
if config.audience and not set(token_audience) & set(config.audience):
raise errors.invalid_token("audience mismatch")
return Credential(
scheme=self.scheme,
method=AuthMethod.OAUTH2_INTROSPECTION,
subject=str(body.get(config.subject_field, "")),
issuer=body.get("iss"),
audience=_normalize_audience(body.get("aud")),
audience=token_audience,
scopes=_split_scope(body.get("scope")),
claims=body,
)
@ -360,8 +365,9 @@ class OidcAuthenticator:
class MutualTlsAuthenticator:
scheme = SecuritySchemeType.MUTUAL_TLS
def __init__(self, config: MutualTlsConfig) -> None:
def __init__(self, config: MutualTlsConfig, network: TrustedProxyConfig) -> None:
self._config = config
self._network = network
async def authenticate(self, request: Request) -> Optional[Credential]:
cert = self._read_client_cert(request)
@ -376,6 +382,9 @@ class MutualTlsAuthenticator:
def _read_client_cert(self, request: Request) -> Optional[ClientCertificate]:
if self._config.forwarded_subject_header:
peer = request.client.host if request.client else None
if not ip_in_trusted_proxies(peer, self._network):
return None
dn = request.headers.get(self._config.forwarded_subject_header)
return ClientCertificate(subject_dn=dn) if dn else None
tls = request.scope.get("extensions", {}).get("tls", {})
@ -402,6 +411,6 @@ def build_authenticators(
)
if config.mutual_tls.enabled:
by_scheme[SecuritySchemeType.MUTUAL_TLS] = MutualTlsAuthenticator(
config.mutual_tls
config.mutual_tls, config.network
)
return [by_scheme[scheme] for scheme in config.scheme_order if scheme in by_scheme]

View file

@ -47,6 +47,7 @@ class OAuth2IntrospectionConfig(BaseModel):
client_id: str
client_secret: SecretStr
subject_field: str = "sub"
audience: List[str] = Field(default_factory=list)
class MutualTlsConfig(BaseModel):
@ -66,7 +67,7 @@ class SamlConfig(BaseModel):
idp_metadata: str = ""
sp_key_file: Optional[str] = None
sp_cert_file: Optional[str] = None
allow_unsolicited: bool = True
allow_unsolicited: bool = False
session_cookie: str = "saml_session"
cookie_secure: bool = True
session_ttl_seconds: int = 3600

View file

@ -48,3 +48,7 @@ def forbidden_role() -> AuthError:
def forbidden_permission() -> AuthError:
return AuthError(403, "Forbidden")
def account_disabled() -> AuthError:
return AuthError(403, "Account disabled")

View file

@ -30,6 +30,10 @@ def _ip_in_cidrs(ip: Optional[str], cidrs: List[str]) -> bool:
return False
def ip_in_trusted_proxies(ip: Optional[str], config: TrustedProxyConfig) -> bool:
return _ip_in_cidrs(ip, config.trusted_proxy_cidrs)
def resolve_client_ip(
request: Request, config: TrustedProxyConfig
) -> Tuple[Optional[str], bool]:

View file

@ -48,6 +48,10 @@ def _roles_from_claims(claims: Dict[str, Any]) -> List[Role]:
return [Role(value) for value in raw if value in valid]
def _public_claims(claims: Dict[str, Any]) -> Dict[str, Any]:
return {key: value for key, value in claims.items() if not key.startswith("_")}
def _teams_from_claims(claims: Dict[str, Any]) -> List[TeamIdentity]:
groups = claims.get("groups", [])
if not isinstance(groups, list):
@ -70,8 +74,29 @@ class InMemoryIdentityStore(IdentityResolver, ProvisioningStore):
async def resolve(self, credential: Credential) -> Principal:
if credential.method == AuthMethod.API_KEY:
return self._resolve_api_key(credential)
return self._resolve_subject(credential)
principal = self._resolve_api_key(credential)
else:
principal = self._resolve_subject(credential)
self._reject_if_deactivated(principal)
return principal
def _reject_if_deactivated(self, principal: Principal) -> None:
user = self._lookup_scim_user(principal)
if user is not None and user.active is False:
raise errors.account_disabled()
def _lookup_scim_user(self, principal: Principal) -> Optional[ScimUser]:
if principal.user is None:
return None
by_id = self._users.get(principal.user.id)
if by_id is not None:
return by_id
external = principal.user.external_id
if external:
for user in self._users.values():
if user.external_id == external:
return user
return None
def _resolve_api_key(self, credential: Credential) -> Principal:
raw = credential.claims.get("_raw_api_key")
@ -99,7 +124,7 @@ class InMemoryIdentityStore(IdentityResolver, ProvisioningStore):
scopes=list(credential.scopes),
auth_method=credential.method,
credential_ref=credential.credential_ref,
claims=dict(claims),
claims=_public_claims(claims),
)
return Principal(
principal_type=PrincipalType.HUMAN,
@ -118,7 +143,7 @@ class InMemoryIdentityStore(IdentityResolver, ProvisioningStore):
scopes=list(credential.scopes),
auth_method=credential.method,
credential_ref=credential.credential_ref,
claims=dict(claims),
claims=_public_claims(claims),
)
async def upsert_user(self, user: ScimUser) -> ScimUser:

View file

@ -141,6 +141,7 @@ def build_sp_client(config: SamlConfig) -> Saml2Client:
class SamlSessionStore:
def __init__(self, ttl_seconds: int = 3600, max_size: int = 10000) -> None:
self._sessions: Dict[str, Tuple[float, Dict[str, Any]]] = {}
self._seen_assertions: Dict[str, float] = {}
self.outstanding: Dict[str, str] = {}
self._ttl = ttl_seconds
self._max_size = max_size
@ -148,6 +149,16 @@ class SamlSessionStore:
def remember_request(self, request_id: str, relay_state: str = "/") -> None:
self.outstanding[request_id] = relay_state
def consume_assertion(self, assertion_id: str) -> bool:
now = time.time()
self._seen_assertions = {
aid: exp for aid, exp in self._seen_assertions.items() if exp >= now
}
if assertion_id in self._seen_assertions:
return False
self._seen_assertions[assertion_id] = now + self._ttl
return True
def create_session(self, identity: Dict[str, Any]) -> str:
now = time.time()
self._evict(now)
@ -244,6 +255,18 @@ def build_saml_router(config: SamlConfig, session_store: SamlSessionStore) -> AP
if authn_response is None:
raise HTTPException(status_code=401, detail="invalid SAML response")
in_response_to = getattr(authn_response, "in_response_to", None)
bound_relay = (
session_store.outstanding.pop(in_response_to, None)
if in_response_to
else None
)
assertion = getattr(authn_response, "assertion", None)
assertion_id = getattr(assertion, "id", None)
if assertion_id and not session_store.consume_assertion(assertion_id):
raise HTTPException(status_code=401, detail="SAML assertion replay")
name_id = authn_response.get_subject().text
ava = authn_response.get_identity() or {}
mapped = _map_attributes(ava, config.attribute_map)
@ -252,9 +275,6 @@ def build_saml_router(config: SamlConfig, session_store: SamlSessionStore) -> AP
store: ProvisioningStore = request.app.state.auth_v2.resolver
await store.upsert_user(user)
in_response_to = getattr(authn_response, "in_response_to", None)
if in_response_to:
session_store.outstanding.pop(in_response_to, None)
session_id = session_store.create_session(
{
"name_id": name_id,
@ -262,11 +282,7 @@ def build_saml_router(config: SamlConfig, session_store: SamlSessionStore) -> AP
"claims": _claims_from_mapped(mapped),
}
)
relay_state = form.get("RelayState")
target = _safe_relay_state(
relay_state if isinstance(relay_state, str) else None,
config.default_redirect_path,
)
target = _safe_relay_state(bound_relay, config.default_redirect_path)
response = RedirectResponse(target, status_code=303)
response.set_cookie(
config.session_cookie,