Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/oauth/src/keycardai/oauth/pkce/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
73 changes: 53 additions & 20 deletions packages/oauth/src/keycardai/oauth/pkce/web.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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,
Expand All @@ -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``.
Expand All @@ -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.
Expand All @@ -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(
Expand All @@ -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,
)
Expand All @@ -147,6 +160,7 @@ async def begin_authorization(
url=url,
state=state,
code_verifier=pkce.code_verifier,
resources=resources,
)


Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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``.
Expand All @@ -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.
Expand Down Expand Up @@ -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,
)

Expand Down Expand Up @@ -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,
Expand Down
61 changes: 54 additions & 7 deletions packages/oauth/tests/keycardai/oauth/pkce/test_web.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,23 +44,58 @@ 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"] == [
PKCEGenerator.generate_code_challenge(result.code_verifier)
]
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()
Expand Down Expand Up @@ -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
Expand All @@ -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 = {}
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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",
Expand Down
Loading