diff --git a/packages/oauth/src/keycardai/oauth/pkce/__init__.py b/packages/oauth/src/keycardai/oauth/pkce/__init__.py index e83a443..a13f07c 100644 --- a/packages/oauth/src/keycardai/oauth/pkce/__init__.py +++ b/packages/oauth/src/keycardai/oauth/pkce/__init__.py @@ -31,10 +31,12 @@ client_id="my-app", issuer="https://auth.example.com", redirect_uri="https://app.example.com/oauth/callback", + resources=["https://api.example.com", "https://files.example.com"], ) session["oauth_flow"] = { "state": redirect.state, "code_verifier": redirect.code_verifier, + "resources": redirect.resources, } # Redirect the browser to ``redirect.url``. In the callback route: flow = session.pop("oauth_flow") diff --git a/packages/oauth/src/keycardai/oauth/pkce/web.py b/packages/oauth/src/keycardai/oauth/pkce/web.py index 3fd6c3d..7cd0657 100644 --- a/packages/oauth/src/keycardai/oauth/pkce/web.py +++ b/packages/oauth/src/keycardai/oauth/pkce/web.py @@ -27,7 +27,7 @@ TokenResponse, ) from ..utils.pkce import PKCEGenerator -from ._issuer import _resolve_auth_server_url +from ._issuer import resolve_issuer_from_challenge class AuthorizationRedirect(BaseModel): @@ -38,18 +38,25 @@ class AuthorizationRedirect(BaseModel): state: The generated CSRF value to store until the callback. code_verifier: The PKCE verifier to store until the callback. This value must never be sent to the browser. + resources: The resources the authorization request was scoped to, + carried so the application can persist them alongside ``state`` + and ``code_verifier``. They are not needed to redeem the code — + the authorization server derives the issued token's audience from + the code itself — but applications commonly need to know which + resources a session was authorized for. """ url: str state: str code_verifier: str + resources: list[str] | None = None async def begin_authorization( *, client_id: str, redirect_uri: str, - resource_url: str | None = None, + resources: list[str] | None = None, www_authenticate_header: str | None = None, issuer: str | None = None, metadata: AuthorizationServerMetadata | None = None, @@ -65,8 +72,10 @@ async def begin_authorization( Args: client_id: OAuth client ID. redirect_uri: Registered redirect URI handled by the web application. - resource_url: The protected resource the caller is targeting. Passed - as the RFC 8707 ``resource`` parameter when provided. + resources: The protected resources the caller is targeting. Each entry + is sent as its own RFC 8707 ``resource`` query parameter, so a + single authorization can cover several resources and the issued + token's audience covers all of them. www_authenticate_header: The ``WWW-Authenticate`` challenge from the protected resource. Must contain a ``resource_metadata`` URL per RFC 9728. Mutually exclusive with ``issuer``. @@ -82,16 +91,16 @@ async def begin_authorization( Returns: ``AuthorizationRedirect`` containing the authorization URL, generated - state, and PKCE code verifier. Store the state and verifier in - application-controlled session state. + state, PKCE code verifier, and the requested resources. Store the + state and verifier in application-controlled session state. Raises: keycardai.oauth.ConfigError: If anything other than exactly one of ``issuer``, ``www_authenticate_header``, or ``metadata`` is - provided, or challenge mode omits ``resource_url``. - ValueError: If the authorization endpoint is missing from the supplied - metadata or discovered server metadata, or if challenge discovery - metadata is incomplete. + provided, or challenge mode omits ``resources``. + ValueError: If the authorization endpoint is missing from the + supplied metadata or discovered server metadata, or if challenge + discovery metadata is incomplete. httpx.HTTPStatusError: If fetching protected resource metadata fails. keycardai.oauth.OAuthHttpError: If authorization server discovery returns an HTTP error. @@ -111,10 +120,14 @@ async def begin_authorization( ) authorization_endpoint = metadata.authorization_endpoint else: - auth_server_url = await _resolve_auth_server_url( + if www_authenticate_header is not None and not resources: + raise ConfigError( + "'resources' is required when authenticating from a " + "WWW-Authenticate challenge" + ) + auth_server_url = await _resolve_issuer( issuer=issuer, www_authenticate_header=www_authenticate_header, - resource_url=resource_url, http_client=http_client, ) config = ClientConfig( @@ -138,7 +151,7 @@ async def begin_authorization( client_id=client_id, redirect_uri=redirect_uri, pkce=pkce, - resources=[resource_url] if resource_url else None, + resources=resources, scope=" ".join(scopes) if scopes else None, state=state, ) @@ -147,6 +160,7 @@ async def begin_authorization( url=url, state=state, code_verifier=pkce.code_verifier, + resources=resources, ) @@ -157,7 +171,6 @@ async def complete_authorization( code_verifier: str, client_id: str, redirect_uri: str, - resource_url: str | None = None, www_authenticate_header: str | None = None, issuer: str | None = None, metadata: AuthorizationServerMetadata | None = None, @@ -170,6 +183,11 @@ async def complete_authorization( The application supplies the ``state`` and ``code_verifier`` retained from :func:`begin_authorization`. + No RFC 8707 ``resource`` parameter is sent on the token request: the + authorization server derives the issued token's audience from the + authorization code, which already records the resources the user + authorized in the begin step. + Args: callback_params: Query parameters received by the application's callback route, including ``code`` and ``state`` or an OAuth @@ -179,8 +197,6 @@ async def complete_authorization( value must never be sent to the browser. client_id: OAuth client ID. redirect_uri: The same registered redirect URI used in the begin step. - resource_url: The protected resource the caller is targeting. Passed - as the RFC 8707 ``resource`` parameter when provided. www_authenticate_header: The ``WWW-Authenticate`` challenge from the protected resource. Must contain a ``resource_metadata`` URL per RFC 9728. Mutually exclusive with ``issuer``. @@ -202,7 +218,7 @@ async def complete_authorization( Raises: keycardai.oauth.ConfigError: If anything other than exactly one of ``issuer``, ``www_authenticate_header``, or ``metadata`` is - provided, or challenge mode omits ``resource_url``. + provided. ValueError: If the token endpoint is missing from the supplied metadata or discovered server metadata, or if challenge discovery metadata is incomplete. @@ -250,10 +266,9 @@ async def complete_authorization( ) auth_server_url = metadata.issuer else: - auth_server_url = await _resolve_auth_server_url( + auth_server_url = await _resolve_issuer( issuer=issuer, www_authenticate_header=www_authenticate_header, - resource_url=resource_url, http_client=http_client, ) @@ -281,10 +296,28 @@ async def complete_authorization( redirect_uri=redirect_uri, code_verifier=code_verifier, client_id=client_id, - resource=resource_url, ) + +async def _resolve_issuer( + *, + issuer: str | None, + www_authenticate_header: str | None, + http_client: httpx.AsyncClient | None, +) -> str: + if issuer is not None: + return issuer.rstrip("/") + if www_authenticate_header is None: + raise ConfigError( + "Provide exactly one of 'issuer', 'www_authenticate_header', " + "or 'metadata'" + ) + return await resolve_issuer_from_challenge( + www_authenticate_header, http_client=http_client + ) + + def _validate_entry_mode( *, issuer: str | None, diff --git a/packages/oauth/tests/keycardai/oauth/pkce/test_web.py b/packages/oauth/tests/keycardai/oauth/pkce/test_web.py index 5a2b8aa..8eeab69 100644 --- a/packages/oauth/tests/keycardai/oauth/pkce/test_web.py +++ b/packages/oauth/tests/keycardai/oauth/pkce/test_web.py @@ -44,12 +44,16 @@ async def test_begin_returns_redirect_and_pkce_values(monkeypatch): issuer="https://auth.example.com", redirect_uri="https://app.example.com/callback", scopes=["openid", "profile"], - resource_url="https://api.example.com", + resources=["https://api.example.com", "https://files.example.com"], ) assert isinstance(result, AuthorizationRedirect) assert result.state assert result.code_verifier + assert result.resources == [ + "https://api.example.com", + "https://files.example.com", + ] params = parse_qs(urlsplit(result.url).query) assert params["state"] == [result.state] assert params["code_challenge"] == [ @@ -57,10 +61,41 @@ async def test_begin_returns_redirect_and_pkce_values(monkeypatch): ] assert params["code_challenge_method"] == ["S256"] assert params["scope"] == ["openid profile"] - assert params["resource"] == ["https://api.example.com"] + assert params["resource"] == [ + "https://api.example.com", + "https://files.example.com", + ] assert captured["issuer"] == "https://auth.example.com" +@pytest.mark.asyncio +async def test_begin_rejects_removed_resource_url(): + with pytest.raises(TypeError, match="resource_url"): + await begin_authorization( + client_id="my-app", + issuer="https://auth.example.com", + redirect_uri="https://app.example.com/callback", + resource_url="https://api.example.com", + ) + + +@pytest.mark.asyncio +async def test_begin_without_resources_sends_no_resource_parameter(monkeypatch): + monkeypatch.setattr( + "keycardai.oauth.pkce.web.AsyncClient", + _async_client_factory(), + ) + + result = await begin_authorization( + client_id="my-app", + issuer="https://auth.example.com", + redirect_uri="https://app.example.com/callback", + ) + + assert result.resources is None + assert "resource" not in parse_qs(urlsplit(result.url).query) + + @pytest.mark.asyncio async def test_begin_uses_metadata_without_constructing_client(monkeypatch): async_client = MagicMock() @@ -100,7 +135,6 @@ async def test_complete_exchanges_matching_state(monkeypatch): client_id="my-app", issuer="https://auth.example.com", redirect_uri="https://app.example.com/callback", - resource_url="https://api.example.com", ) assert result is token @@ -109,10 +143,23 @@ async def test_complete_exchanges_matching_state(monkeypatch): "redirect_uri": "https://app.example.com/callback", "code_verifier": "stored-verifier", "client_id": "my-app", - "resource": "https://api.example.com", } +@pytest.mark.asyncio +async def test_complete_rejects_removed_resource_url(): + with pytest.raises(TypeError, match="resource_url"): + await complete_authorization( + callback_params={"code": "auth-code", "state": "stored-state"}, + state="stored-state", + code_verifier="stored-verifier", + client_id="my-app", + issuer="https://auth.example.com", + redirect_uri="https://app.example.com/callback", + resource_url="https://api.example.com", + ) + + @pytest.mark.asyncio async def test_complete_uses_metadata_without_discovery(monkeypatch): captured = {} @@ -299,7 +346,7 @@ async def test_begin_resolves_issuer_from_challenge(monkeypatch): await begin_authorization( client_id="my-app", redirect_uri="https://app.example.com/callback", - resource_url="https://api.example.com", + resources=["https://api.example.com"], www_authenticate_header=WWW_AUTHENTICATE, http_client=http_client, ) @@ -346,8 +393,8 @@ async def test_flow_requires_exactly_one_issuer_entry(function, kwargs): @pytest.mark.asyncio -async def test_begin_challenge_mode_requires_resource_url(): - with pytest.raises(ConfigError, match="resource_url"): +async def test_begin_challenge_mode_requires_resources(): + with pytest.raises(ConfigError, match="resources"): await begin_authorization( client_id="my-app", redirect_uri="https://app.example.com/callback",