feat: add mTLS (RFC 8705) client authentication - #159
Conversation
9ef2878 to
0915865
Compare
| if self._use_mtls: | ||
| # The client certificate presented in the TLS handshake is the sole | ||
| # credential; no body credential or HTTP basic auth is sent. | ||
| return None |
There was a problem hiding this comment.
This is where mTLS drops the body credential, and the alias resolver above does the endpoint routing. Both are applied on the interactive-login, refresh, backchannel, connection, custom-exchange, passkey, and MFA paths, but passwordless verify does not go through either. Under mTLS it still posts to the standard token endpoint and still puts client_secret in the body, and since a mTLS client has no secret, that call goes out with an empty credential to an endpoint that never sees the certificate, so the token it gets back is not certificate-bound.
Shall we route passwordless verify through the same resolver and drop its body credential when mTLS is on? If passwordless under mTLS is under consideration.
| session_establisher=self._establish_session_from_mfa_verify_response, | ||
| mfa_token_ttl=mfa_token_ttl, | ||
| apply_client_authentication=self._apply_client_authentication, | ||
| use_mtls=self._use_mtls, |
There was a problem hiding this comment.
The My Account client built just above here does not get use_mtls or ssl_context, so under mTLS its API calls go out without the client certificate. The MFA client right here has both threaded in.
Shall we pass the same use_mtls and ssl_context through to the My Account client, or is My Account intentionally left off mTLS for now?
| raise ApiError( | ||
| "token_error", f"Token exchange failed: {str(e)}", e) | ||
|
|
||
| self._warn_if_not_cert_bound(token_response.get("access_token")) |
There was a problem hiding this comment.
The certificate-bound warning fires here on interactive login and on the refresh path, but the backchannel, connection, custom-token-exchange, and passkey token responses all return without it.
Shall we call the warning on each path that returns a token, or funnel the token responses through one spot that always runs it?
| options: dict[str, Any], | ||
| store_options: Optional[dict[str, Any]] = None, | ||
| dpop_key: Optional["jwk.JWK"] = None, | ||
| token_endpoint_override: Optional[str] = None, |
There was a problem hiding this comment.
The new token_endpoint_override param, and the ConfigurationError this now raises when dpop_key is combined with mTLS, are not in the docstring. The Args and Raises sections still describe the previous surface.
Worth adding both so the documented contract matches what the method does now.
| raise MissingRequiredArgumentError("auth_session") | ||
| if authn_response is None: | ||
| raise MissingRequiredArgumentError("authn_response") | ||
| if self._use_mtls and dpop_key is not None: |
There was a problem hiding this comment.
This now raises ConfigurationError when dpop_key is combined with mTLS, but the Raises section of the docstring still lists only the missing-argument, passkey, and organization errors.
We should add it there too.
|
|
||
| When the target API has **Token Sender-Constraining (mTLS)** enabled, issued access tokens carry a `cnf.x5t#S256` claim binding the token to the certificate thumbprint. The SDK warns if it receives a token that lacks this claim: | ||
|
|
||
| > `UserWarning: mTLS is enabled but the access token is not certificate-bound (no cnf.x5t#S256). Sender-constraining is not active — configure Token Sender-Constraining (mTLS) on the API resource server.` |
There was a problem hiding this comment.
Em dash in the warning quote: "Sender-constraining is not active — configure Token Sender-Constraining".
| algorithms=["HS256", "RS256", "ES256", "PS256"], | ||
| ) | ||
| except Exception: | ||
| return # opaque or unparseable token — nothing to assert |
There was a problem hiding this comment.
Em dash in the inline comment: "opaque or unparseable token — nothing to assert".
| if not (isinstance(cnf, dict) and cnf.get("x5t#S256")): | ||
| warnings.warn( | ||
| "mTLS is enabled but the access token is not certificate-bound " | ||
| "(no cnf.x5t#S256). Sender-constraining is not active — configure " |
There was a problem hiding this comment.
Em dash in the warning string: "Sender-constraining is not active — configure".
| if ssl_context is None: | ||
| raise ConfigurationError( | ||
| "use_mtls=True requires an ssl_context with the client certificate " | ||
| "loaded (ssl.create_default_context() + load_cert_chain())." |
There was a problem hiding this comment.
This look very cryptic. Can we change it to actual wordings?
| if self._use_mtls and dpop_key is not None: | ||
| raise ConfigurationError( | ||
| "dpop_key cannot be combined with use_mtls. DPoP and mTLS bind tokens " | ||
| "differently; DPoP would take precedence and the token would not be " |
There was a problem hiding this comment.
The semicolon here splices two clauses. We can add two sentences which will increase the readibility.
| return # opaque or unparseable token — nothing to assert | ||
| cnf = claims.get("cnf") if isinstance(claims, dict) else None | ||
| if not (isinstance(cnf, dict) and cnf.get("x5t#S256")): | ||
| warnings.warn( |
There was a problem hiding this comment.
This is the only place the SDK calls warnings.warn for a runtime condition, and there's no other advisory of this kind in the codebase. The thing it flags is a setting on the API resource server rather than anything wrong at this call site, and the token that comes back is still valid and usable.
Is UserWarning the channel we want here, or would documenting in the docs the sender-constraining requirement and keeping this check out of the token path carry the same message without the once-only behavior? WDYT?
|
|
||
| try: | ||
| token_endpoint = f"{base_url}/oauth/token" | ||
| token_endpoint = token_endpoint_override or f"{base_url}/oauth/token" |
There was a problem hiding this comment.
Under mTLS this falls back to the standard host because nothing ever passes token_endpoint_override, so MFA verify would fail with invalid_client. Could we resolve the alias inside MfaClient or via a ServerClient method?
| **({"verify": self._ssl_context} if self._use_mtls else {}), | ||
| ) | ||
|
|
||
| self._my_account_client = MyAccountClient( |
There was a problem hiding this comment.
The My Account client is built here, and MyAccountClient accepts no use_mtls or ssl_context, so it never attaches the certificate. If a token bound to the certificate is presented to a My Account resource server that enforces mTLS binding, the call would be rejected. Could you confirm whether My Account enforces binding?
|
|
||
| try: | ||
| token_endpoint = self._oauth.metadata["token_endpoint"] | ||
| token_endpoint = self._resolve_token_endpoint(self._oauth.metadata) |
There was a problem hiding this comment.
This path used to read self._oauth.metadata["token_endpoint"], which raised KeyError when the endpoint was missing. It now calls _resolve_token_endpoint, which returns None when mTLS is off, and it does not add the if not token_endpoint: raise check the other five call sites carry. A missing endpoint would now pass None into fetch_token rather than failing early. Adding that check here would keep the behavior consistent.
| kwargs["verify"] = self._ssl_context | ||
| return httpx.AsyncClient(headers=headers, **kwargs) | ||
|
|
||
| def _resolve_token_endpoint(self, metadata: dict) -> Optional[str]: |
There was a problem hiding this comment.
This resolves the mTLS alias for the token endpoint only. The PAR path reads pushed_authorization_request_endpoint straight from standard metadata, so PAR plus mTLS posts to the standard host and would hit the same invalid_client. Is PAR to be supported in this flow? If yes, could the alias resolution here be extended to cover the PAR endpoint, or PAR fail closed when both are enabled until it is?
Summary
Adds Mutual TLS (RFC 8705) client authentication to
auth0-server-python. When enabled, the SDK presents a TLS client certificate during the Auth0 token-endpoint handshake instead of a client secret — no credential travels in the request body.ServerClientconstructor params:use_mtls: bool = Falseandssl_context: Optional[ssl.SSLContext] = None. The caller builds theSSLContext(ssl.create_default_context()+load_cert_chain); the SDK forwards it asverify=ssl_contextto everyhttpx.AsyncClientit constructs (including the authlib client used for the authorization-code exchange)._resolve_token_endpoint(metadata)helper returnsmtls_endpoint_aliases.token_endpointfrom the discovery document when mTLS is on, raisingConfigurationErrorif the alias is absent. All six token-endpoint call sites inserver_client.pyare routed through it._apply_client_authenticationreturnsNoneunder mTLS — the certificate in the TLS handshake is the sole credential; noclient_secretorclient_assertionis added to the body.MfaClientreceivesuse_mtlsandssl_contextfromServerClient(cert on all MFA calls).verify()accepts an optionaltoken_endpoint_overrideso the mTLS alias can be passed in for the token exchange while challenge/enroll calls remain on the standard host.signin_with_passkeyandMfaClient.verifyraiseConfigurationErrorwhen bothdpop_keyanduse_mtlsare active — DPoP would bind the token to its own key and suppresscnf.x5t#S256, silently defeating mTLS token binding.cnf.x5t#S256advisory warning: after obtaining a token under mTLS,_warn_if_not_cert_boundchecks whether the access token is certificate-bound and emits aUserWarningif not (silent on opaque tokens; never raises).ConfigurationError, fail-fast):use_mtls=Truewithoutssl_context; combined withclient_secret; combined withclient_assertion_signing_key.Changed files
auth_server/server_client.pyuse_mtls/ssl_contextparams, validation,_resolve_token_endpoint,_apply_client_authenticationmTLS branch,_warn_if_not_cert_bound, DPoP guard, 6 call-site routings,_get_http_client+AsyncOAuth2Clientverify=injectionauth_server/mfa_client.pyuse_mtls/ssl_contextparams,_get_http_clientverify=injection,verify()token_endpoint_overrideparam + DPoP guardtests/test_server_client.py_resolve_token_endpoint,_apply_client_authenticationmTLS branch, SSLContext threading,_warn_if_not_cert_bound, DPoP exclusion, end-to-end alias-routing testtests/test_mfa_client.pytoken_endpoint_override, DPoP exclusionexamples/MutualTLS.mdcnfverificationREADME.mdreferences/flow-map.mdTest plan
poetry run pytest— all tests pass (491 tests)poetry run ruff check .— no lint errorsConfigurationErrorfor: missingssl_contextwithuse_mtls=True;client_secret+use_mtls;client_assertion_signing_key+use_mtls_resolve_token_endpointreturns the mTLS alias when present, raisesConfigurationErrorwhen absent under mTLS, returns the standard endpoint when mTLS is offcomplete_interactive_loginunder mTLS hitsmtls_endpoint_aliases.token_endpointsignin_with_passkeyraisesConfigurationErrorwhendpop_key+use_mtlsmfa.verifyraisesConfigurationErrorwhendpop_key+use_mtls; usestoken_endpoint_overridewhen provided_warn_if_not_cert_boundwarns on JWT withoutcnf.x5t#S256, silent on JWT with it, silent on opaque token