Skip to content

Commit 206b9ea

Browse files
fix: omit client_secret for public OAuth clients in token exchange
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 <carlosandresplanchonprestes@gmail.com>
1 parent 21cf51a commit 206b9ea

2 files changed

Lines changed: 75 additions & 1 deletion

File tree

crudauth/oauth/provider.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,15 +163,23 @@ async def exchange_code(
163163
164164
Raises:
165165
httpx.HTTPStatusError: If the token endpoint returns an error status.
166+
167+
Note:
168+
``client_secret`` is included only when the provider actually has
169+
one. A public client (PKCE-only, ``token_endpoint_auth_method=none``)
170+
must not send client authentication - several IdPs reject an empty
171+
``client_secret`` outright - and its proof is the PKCE verifier,
172+
which is sent either way.
166173
"""
167174
httpx = _require_httpx()
168175
data = {
169176
"client_id": self.client_id,
170-
"client_secret": self.client_secret,
171177
"code": code,
172178
"redirect_uri": self.redirect_uri,
173179
"grant_type": "authorization_code",
174180
}
181+
if self.client_secret:
182+
data["client_secret"] = self.client_secret
175183
if code_verifier:
176184
data["code_verifier"] = code_verifier
177185
req_headers = {"Accept": "application/json"}

tests/oauth/test_token_exchange.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
"""Token-exchange client authentication: confidential vs public clients."""
2+
3+
from __future__ import annotations
4+
5+
import httpx
6+
7+
from crudauth.oauth.providers.google import GoogleOAuthProvider
8+
9+
10+
def _fake_async_client(captured: dict):
11+
"""An httpx.AsyncClient stand-in that records the POST it receives."""
12+
13+
class FakeResponse:
14+
def raise_for_status(self) -> None:
15+
pass
16+
17+
def json(self) -> dict:
18+
return {"access_token": "tok", "token_type": "Bearer"}
19+
20+
class FakeAsyncClient:
21+
def __init__(self, *args, **kwargs):
22+
pass
23+
24+
async def __aenter__(self):
25+
return self
26+
27+
async def __aexit__(self, *exc):
28+
return False
29+
30+
async def post(self, url, data=None, headers=None):
31+
captured["url"] = url
32+
captured["data"] = dict(data)
33+
captured["headers"] = dict(headers or {})
34+
return FakeResponse()
35+
36+
return FakeAsyncClient
37+
38+
39+
# --- confidential client (has a secret): client auth rides in the body --------
40+
async def test_exchange_code_sends_secret_for_confidential_client(monkeypatch) -> None:
41+
captured: dict = {}
42+
monkeypatch.setattr(httpx, "AsyncClient", _fake_async_client(captured))
43+
44+
prov = GoogleOAuthProvider("cid", "s3cret", "https://app/cb")
45+
result = await prov.exchange_code("code-1", code_verifier="ver-1")
46+
47+
assert result["access_token"] == "tok"
48+
assert captured["data"]["client_secret"] == "s3cret"
49+
assert captured["data"]["code_verifier"] == "ver-1"
50+
assert captured["data"]["grant_type"] == "authorization_code"
51+
52+
53+
# --- public client (no secret): the field must be absent, not empty -----------
54+
async def test_exchange_code_omits_secret_for_public_client(monkeypatch) -> None:
55+
# A PKCE-only public client (token_endpoint_auth_method=none) must not send
56+
# client authentication; several IdPs reject client_secret="" outright.
57+
captured: dict = {}
58+
monkeypatch.setattr(httpx, "AsyncClient", _fake_async_client(captured))
59+
60+
prov = GoogleOAuthProvider("cid", "", "https://app/cb")
61+
await prov.exchange_code("code-1", code_verifier="ver-1")
62+
63+
assert "client_secret" not in captured["data"]
64+
assert captured["data"]["client_id"] == "cid"
65+
assert captured["data"]["code_verifier"] == "ver-1"
66+
assert captured["data"]["grant_type"] == "authorization_code"

0 commit comments

Comments
 (0)