From 206b9ea44c6a28c14f0251fb57246ae6e6112756 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Andr=C3=A9s=20Planch=C3=B3n=20Prestes?= Date: Wed, 15 Jul 2026 19:03:46 -0300 Subject: [PATCH] fix: omit client_secret for public OAuth clients in token exchange MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AbstractOAuthProvider.exchange_code always included client_secret in the token request body. For a public client (PKCE-only, token_endpoint_auth_method=none) client_secret is empty, so the request sent client_secret="", which several IdPs reject since a public client must not send client authentication. Include the field only when it is set; the PKCE code_verifier, sent either way, is the public client's proof. Confidential clients are unchanged. Adds test_token_exchange.py capturing the token POST body for both the confidential and public modes. Signed-off-by: Carlos Andrés Planchón Prestes --- crudauth/oauth/provider.py | 10 ++++- tests/oauth/test_token_exchange.py | 66 ++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 tests/oauth/test_token_exchange.py diff --git a/crudauth/oauth/provider.py b/crudauth/oauth/provider.py index e3e5233..f9f103d 100644 --- a/crudauth/oauth/provider.py +++ b/crudauth/oauth/provider.py @@ -163,15 +163,23 @@ async def exchange_code( Raises: httpx.HTTPStatusError: If the token endpoint returns an error status. + + Note: + ``client_secret`` is included only when the provider actually has + one. A public client (PKCE-only, ``token_endpoint_auth_method=none``) + must not send client authentication - several IdPs reject an empty + ``client_secret`` outright - and its proof is the PKCE verifier, + which is sent either way. """ httpx = _require_httpx() data = { "client_id": self.client_id, - "client_secret": self.client_secret, "code": code, "redirect_uri": self.redirect_uri, "grant_type": "authorization_code", } + if self.client_secret: + data["client_secret"] = self.client_secret if code_verifier: data["code_verifier"] = code_verifier req_headers = {"Accept": "application/json"} diff --git a/tests/oauth/test_token_exchange.py b/tests/oauth/test_token_exchange.py new file mode 100644 index 0000000..2dc1e40 --- /dev/null +++ b/tests/oauth/test_token_exchange.py @@ -0,0 +1,66 @@ +"""Token-exchange client authentication: confidential vs public clients.""" + +from __future__ import annotations + +import httpx + +from crudauth.oauth.providers.google import GoogleOAuthProvider + + +def _fake_async_client(captured: dict): + """An httpx.AsyncClient stand-in that records the POST it receives.""" + + class FakeResponse: + def raise_for_status(self) -> None: + pass + + def json(self) -> dict: + return {"access_token": "tok", "token_type": "Bearer"} + + class FakeAsyncClient: + def __init__(self, *args, **kwargs): + pass + + async def __aenter__(self): + return self + + async def __aexit__(self, *exc): + return False + + async def post(self, url, data=None, headers=None): + captured["url"] = url + captured["data"] = dict(data) + captured["headers"] = dict(headers or {}) + return FakeResponse() + + return FakeAsyncClient + + +# --- confidential client (has a secret): client auth rides in the body -------- +async def test_exchange_code_sends_secret_for_confidential_client(monkeypatch) -> None: + captured: dict = {} + monkeypatch.setattr(httpx, "AsyncClient", _fake_async_client(captured)) + + prov = GoogleOAuthProvider("cid", "s3cret", "https://app/cb") + result = await prov.exchange_code("code-1", code_verifier="ver-1") + + assert result["access_token"] == "tok" + assert captured["data"]["client_secret"] == "s3cret" + assert captured["data"]["code_verifier"] == "ver-1" + assert captured["data"]["grant_type"] == "authorization_code" + + +# --- public client (no secret): the field must be absent, not empty ----------- +async def test_exchange_code_omits_secret_for_public_client(monkeypatch) -> None: + # A PKCE-only public client (token_endpoint_auth_method=none) must not send + # client authentication; several IdPs reject client_secret="" outright. + captured: dict = {} + monkeypatch.setattr(httpx, "AsyncClient", _fake_async_client(captured)) + + prov = GoogleOAuthProvider("cid", "", "https://app/cb") + await prov.exchange_code("code-1", code_verifier="ver-1") + + assert "client_secret" not in captured["data"] + assert captured["data"]["client_id"] == "cid" + assert captured["data"]["code_verifier"] == "ver-1" + assert captured["data"]["grant_type"] == "authorization_code"