Skip to content

Commit 72f8c22

Browse files
committed
fix(oidc): trusted clients need trusted redirect URIs
1 parent 90f6a12 commit 72f8c22

8 files changed

Lines changed: 76 additions & 8 deletions

File tree

CHANGES.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ Fixed
2727
- Request objects and client key sets, downloaded at the ``request_uri`` and
2828
``jwks_uri`` addresses clients indicate, were read in memory without any size
2929
limit. They are now bounded to 64kB.
30+
- Trusted clients, which skip the user consent page, need both their
31+
``client_uri`` and their ``redirect_uris`` to match
32+
:attr:`~canaille.oidc.configuration.OIDCSettings.TRUSTED_DOMAINS`.
3033

3134
[0.3.6] - 2026-08-04
3235
--------------------

canaille/oidc/basemodels.py

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
from canaille.backends.models import Model
88
from canaille.core.models import User
9-
from canaille.oidc.utils import is_trusted_domain
9+
from canaille.oidc.utils import is_trusted_client
1010

1111

1212
class Client(Model):
@@ -24,13 +24,12 @@ class Client(Model):
2424
def trusted(self) -> bool:
2525
"""Trusted clients don't require to display the consent page to users.
2626
27-
A client is trusted if its client_uri domain matches one of the domains
28-
listed in TRUSTED_DOMAINS configuration. Supports:
29-
- Exact match: "example.com" matches "example.com"
30-
- Subdomain match: "example.com" also matches "sub.example.com"
31-
- Wildcard match: ".example.com" matches "example.com" and all its subdomains
27+
A client is trusted when its :attr:`client_uri` **and** all its
28+
:attr:`redirect_uris` match the domains listed in the
29+
:attr:`~canaille.oidc.configuration.OIDCSettings.TRUSTED_DOMAINS`
30+
configuration parameter, that documents the supported patterns.
3231
"""
33-
return is_trusted_domain(self.client_uri)
32+
return is_trusted_client(self.client_uri, self.redirect_uris)
3433

3534
# keep 'List' instead of 'list' do not break py310 with the memory backend
3635
audience: List["Client"] = [] # noqa: UP006

canaille/oidc/utils.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,3 +75,15 @@ def is_trusted_domain(domain):
7575
return True
7676

7777
return False
78+
79+
80+
def is_trusted_client(client_uri: str | None, redirect_uris: list[str] | None) -> bool:
81+
"""Whether a client with those URIs is trusted enough to skip the consent page.
82+
83+
The redirect URIs are checked in addition to the client URI, as the client
84+
URI is a purely declarative value that is never fetched: trusting it alone
85+
would let a client have authorization codes delivered to any address.
86+
"""
87+
return is_trusted_domain(client_uri) and all(
88+
is_trusted_domain(redirect_uri) for redirect_uri in redirect_uris or []
89+
)

tests/oidc/test_authorization_code_flow.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@ def test_trusted_client(testclient, logged_user, client, trusted_client, backend
293293
assert not backend.query(models.Consent, client=client, subject=logged_user)
294294

295295
client.client_uri = "https://client.trusted.test"
296+
client.redirect_uris = ["https://client.trusted.test/redirect1"]
296297
backend.save(client)
297298

298299
res = testclient.get(
@@ -302,7 +303,7 @@ def test_trusted_client(testclient, logged_user, client, trusted_client, backend
302303
client_id=client.client_id,
303304
scope="openid profile",
304305
nonce="somenonce",
305-
redirect_uri="https://client.test/redirect1",
306+
redirect_uri=client.redirect_uris[0],
306307
),
307308
status=302,
308309
)

tests/oidc/test_client_admin.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,12 @@ def test_client_edit_preauth(testclient, client, logged_admin, trusted_client, b
248248

249249
res = testclient.get("/admin/client/edit/" + client.client_id)
250250
res.forms["clienteditform"]["client_uri"] = "https://client.trusted.test"
251+
res.forms["clienteditform"]["redirect_uris-0"] = (
252+
"https://client.trusted.test/redirect1"
253+
)
254+
res.forms["clienteditform"]["redirect_uris-1"] = (
255+
"https://client.trusted.test/redirect2"
256+
)
251257
res = res.forms["clienteditform"].submit(name="action", value="edit")
252258

253259
assert ("success", "The client has been edited.") in res.flashes
@@ -263,6 +269,26 @@ def test_client_edit_preauth(testclient, client, logged_admin, trusted_client, b
263269
assert not client.trusted
264270

265271

272+
def test_client_edit_preauth_needs_trusted_redirect_uris(
273+
testclient, client, logged_admin, trusted_client, backend
274+
):
275+
"""A trusted client_uri is not enough: the redirect URIs are where codes are sent."""
276+
res = testclient.get("/admin/client/edit/" + client.client_id)
277+
res.forms["clienteditform"]["client_uri"] = "https://client.trusted.test"
278+
res.forms["clienteditform"]["redirect_uris-0"] = (
279+
"https://client.trusted.test/redirect1"
280+
)
281+
res = res.forms["clienteditform"].submit(name="action", value="edit")
282+
283+
assert ("success", "The client has been edited.") in res.flashes
284+
backend.reload(client)
285+
assert client.redirect_uris == [
286+
"https://client.trusted.test/redirect1",
287+
"https://client.test/redirect2",
288+
]
289+
assert not client.trusted
290+
291+
266292
def test_client_edit_invalid_uri(testclient, client, logged_admin, trusted_client):
267293
res = testclient.get("/admin/client/edit/" + client.client_id)
268294
res.forms["clienteditform"]["client_uri"] = "invalid"

tests/oidc/test_consent.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ def test_trusted_client_appears_in_consent_list(
165165
res.mustcontain(no=client.client_name)
166166

167167
client.client_uri = "https://client.trusted.test"
168+
client.redirect_uris = ["https://client.trusted.test/redirect1"]
168169
backend.save(client)
169170

170171
res = testclient.get("/consent/trusted-applications")
@@ -174,6 +175,7 @@ def test_trusted_client_appears_in_consent_list(
174175
def test_revoke_trusted_client(testclient, client, logged_user, token, backend):
175176
"""Test that revoking a trusted client creates a revoked consent entry."""
176177
client.client_uri = "https://client.trusted.test"
178+
client.redirect_uris = ["https://client.trusted.test/redirect1"]
177179
backend.save(client)
178180
assert not backend.get(models.Consent, client=client, subject=logged_user)
179181
assert not token.revoked
@@ -214,6 +216,7 @@ def test_revoke_trusted_client_with_manual_consent(
214216
):
215217
"""Test that revoking a trusted client with existing manual consent works correctly."""
216218
client.client_uri = "https://client.trusted.test"
219+
client.redirect_uris = ["https://client.trusted.test/redirect1"]
217220
backend.save(client)
218221
res = testclient.get(f"/consent/revoke-trusted/{client.client_id}", status=302)
219222
res = res.follow()
@@ -225,6 +228,7 @@ def test_revoke_trusted_client_with_manual_revokation(
225228
):
226229
"""Test that revoking an already revoked trusted client displays an error."""
227230
client.client_uri = "https://client.trusted.test"
231+
client.redirect_uris = ["https://client.trusted.test/redirect1"]
228232
backend.save(client)
229233
consent.revoke()
230234
backend.save(consent)

tests/oidc/test_jwt_authorization_grant.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ def test_nominal_case(testclient, logged_user, client, backend, server_jwk):
1010
"""Test JWT grant for a client with consent."""
1111
now = time.time()
1212
client.client_uri = "https://client.trusted.test"
13+
client.redirect_uris = ["https://client.trusted.test/redirect1"]
1314
backend.save(client)
1415

1516
header = {"alg": "RS256"}

tests/oidc/test_trusted_domains.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,28 @@ def test_trusted_domains_property(testclient, backend):
4646
assert not client_no_uri.trusted
4747

4848

49+
def test_trusted_domains_check_the_redirect_uris(testclient, backend):
50+
"""A trusted client_uri is not enough, as it is purely declarative.
51+
52+
The redirect URIs are the addresses authorization codes are actually sent to.
53+
"""
54+
client = models.Client(
55+
client_id="mixed-uris-client",
56+
client_name="Mixed URIs Client",
57+
client_uri="https://client.trusted.test",
58+
redirect_uris=["https://client.trusted.test/redirect1"],
59+
)
60+
backend.save(client)
61+
assert client.trusted
62+
63+
client.redirect_uris = [
64+
"https://client.trusted.test/redirect1",
65+
"https://evil.example.com/callback",
66+
]
67+
backend.save(client)
68+
assert not client.trusted
69+
70+
4971
def test_trusted_domains_consent_bypass(testclient, logged_user, backend):
5072
"""Test that trusted clients bypass the consent page."""
5173
client = models.Client(

0 commit comments

Comments
 (0)