Merge branch '232-ensure-authorization-requests-have-a-valid-redirect-uri' into 'main'

Resolve "Ensure authorization requests have a valid redirect URI"

Closes #232

See merge request yaal/canaille!241

Author: azmeuk

CHANGES.rst

@@ -9,6 +9,7 @@ Added
 Fixed
 ^^^^^
 - Prevent clients from registering with fragment components in their redirect uri :issue:`235`
+- Ensure there is a `redirect_uri` in authorization requests from clients. :issue:`232`
 
 [0.0.64] - 2025-02-12
 ---------------------

canaille/oidc/endpoints/oauth.py

@@ -6,6 +6,7 @@
 from authlib.jose import jwt
 from authlib.jose.errors import JoseError
 from authlib.oauth2 import OAuth2Error
+from authlib.oauth2.rfc6749.errors import InvalidRequestError
 from flask import Blueprint
 from flask import abort
 from flask import current_app
@@ -124,6 +125,12 @@ def authorize_login(user):
 
 
 def authorize_consent(client, user):
+    redirect_uri = request.args.get("redirect_uri")
+    # Ensures the request contains a redirect_uri until resolved upstream in Authlib
+    # https://github.com/lepture/authlib/issues/712
+    if not redirect_uri:
+        raise InvalidRequestError('Missing "redirect_uri" in request.')
+
     requested_scopes = request.args.get("scope", "").split(" ")
     allowed_scopes = client.get_allowed_scope(requested_scopes).split(" ")
     consents = Backend.instance.query(

canaille/oidc/models.py

@@ -49,9 +49,6 @@ def __init__(self, *args, **kwargs):
     def get_client_id(self):
         return self.client_id
 
-    def get_default_redirect_uri(self):
-        return self.redirect_uris[0]
-
     def get_allowed_scope(self, scope):
         return util.list_to_scope(
             [scope_piece for scope_piece in self.scope if scope_piece in scope]

tests/oidc/test_authorization_code_flow.py

@@ -3,8 +3,10 @@
 from urllib.parse import parse_qs
 from urllib.parse import urlsplit
 
+import pytest
 import time_machine
 from authlib.jose import jwt
+from authlib.oauth2.rfc6749.errors import InvalidRequestError
 from authlib.oauth2.rfc7636 import create_s256_code_challenge
 from flask import g
 from werkzeug.security import gen_salt
@@ -27,6 +29,7 @@ def test_nominal_case(
             client_id=client.client_id,
             scope="openid profile email groups address phone",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )
@@ -119,6 +122,7 @@ def test_invalid_client(testclient, logged_user, keypair):
             client_id="invalid",
             scope="openid profile email groups address phone",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=400,
     )
@@ -186,6 +190,7 @@ def test_preconsented_client(
             client_id=client.client_id,
             scope="openid profile",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=302,
     )
@@ -240,6 +245,7 @@ def test_logout_login(testclient, logged_user, client, backend):
             client_id=client.client_id,
             scope="openid profile",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )
@@ -311,6 +317,7 @@ def test_deny(testclient, logged_user, client, backend):
             client_id=client.client_id,
             scope="openid profile",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )
@@ -342,6 +349,7 @@ def test_code_challenge(testclient, logged_user, client, backend):
             client_id=client.client_id,
             scope="openid profile",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )
@@ -399,6 +407,7 @@ def test_consent_already_given(testclient, logged_user, client, backend):
             client_id=client.client_id,
             scope="openid profile",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )
@@ -434,6 +443,7 @@ def test_consent_already_given(testclient, logged_user, client, backend):
             client_id=client.client_id,
             scope="openid profile",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=302,
     )
@@ -455,6 +465,7 @@ def test_consent_with_openid_scope_only(testclient, logged_user, client, backend
             client_id=client.client_id,
             scope="openid",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )
@@ -474,6 +485,7 @@ def test_consent_with_no_scope(testclient, logged_user, client, backend):
             response_type="code",
             client_id=client.client_id,
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )
@@ -483,6 +495,22 @@ def test_consent_with_no_scope(testclient, logged_user, client, backend):
     res.mustcontain(no="Accept")
 
 
+def test_consent_with_no_redirect_uri(testclient, logged_user, client, backend):
+    with pytest.raises(
+        InvalidRequestError,
+        match=r'Missing "redirect_uri" in request.',
+    ):
+        testclient.get(
+            "/oauth/authorize",
+            params=dict(
+                response_type="code",
+                client_id=client.client_id,
+                nonce="somenonce",
+            ),
+            status=200,
+        )
+
+
 def test_when_consent_already_given_but_for_a_smaller_scope(
     testclient, logged_user, client, backend
 ):
@@ -495,6 +523,7 @@ def test_when_consent_already_given_but_for_a_smaller_scope(
             client_id=client.client_id,
             scope="openid profile",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )
@@ -531,6 +560,7 @@ def test_when_consent_already_given_but_for_a_smaller_scope(
             client_id=client.client_id,
             scope="openid profile groups",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )
@@ -564,6 +594,7 @@ def test_user_cannot_use_oidc(
             client_id=client.client_id,
             scope="openid profile",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
     )
     res = res.follow()
@@ -584,6 +615,7 @@ def test_nonce_required_in_oidc_requests(testclient, logged_user, client):
             response_type="code",
             client_id=client.client_id,
             scope="openid profile",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )
@@ -601,6 +633,7 @@ def test_nonce_not_required_in_oauth_requests(testclient, logged_user, client, b
             response_type="code",
             client_id=client.client_id,
             scope="profile",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )
@@ -624,6 +657,7 @@ def test_request_scope_too_large(testclient, logged_user, keypair, client, backe
             client_id=client.client_id,
             scope="openid profile email",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )
@@ -690,6 +724,7 @@ def test_code_expired(testclient, user, client):
                 client_id=client.client_id,
                 scope="openid profile email groups address phone",
                 nonce="somenonce",
+                redirect_uri="https://client.test/redirect1",
             ),
         )
         res = res.follow()
@@ -738,6 +773,7 @@ def test_code_with_invalid_user(testclient, admin, client, backend):
             client_id=client.client_id,
             scope="openid profile email groups address phone",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
     ).follow()
 
@@ -782,6 +818,7 @@ def test_locked_account(
             client_id=client.client_id,
             scope="openid profile email groups address phone",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )
@@ -822,6 +859,7 @@ def test_missing_client_id(
             response_type="code",
             scope="openid profile email groups address phone",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=400,
     )
@@ -844,6 +882,7 @@ def test_logout_login_with_intruder_lockout(testclient, logged_user, client, bac
                 client_id=client.client_id,
                 scope="openid profile",
                 nonce="somenonce",
+                redirect_uri="https://client.test/redirect1",
             ),
             status=200,
         )
@@ -892,6 +931,7 @@ def test_rfc9207(
             client_id=client.client_id,
             scope="openid profile email groups address phone",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )

tests/oidc/test_authorization_prompt.py

@@ -32,6 +32,7 @@ def test_prompt_none(testclient, logged_user, client, backend):
             scope="openid profile",
             nonce="somenonce",
             prompt="none",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=302,
     )
@@ -67,6 +68,7 @@ def test_prompt_not_logged(testclient, user, client, backend):
             scope="openid profile",
             nonce="somenonce",
             prompt="none",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )
@@ -92,6 +94,7 @@ def test_prompt_no_consent(testclient, logged_user, client):
             scope="openid profile",
             nonce="somenonce",
             prompt="none",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )
@@ -118,6 +121,7 @@ def test_prompt_create_logged(testclient, logged_user, client, backend):
             scope="openid profile",
             nonce="somenonce",
             prompt="create",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=302,
     )
@@ -144,6 +148,7 @@ def test_prompt_create_registration_disabled(testclient, trusted_client, smtpd):
             scope="openid profile",
             nonce="somenonce",
             prompt="create",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=400,
     )
@@ -169,6 +174,7 @@ def test_prompt_create_not_logged(testclient, trusted_client, smtpd):
             scope="openid profile",
             nonce="somenonce",
             prompt="create",
+            redirect_uri=trusted_client.redirect_uris[0],
         ),
     )
 

tests/oidc/test_consent.py

@@ -114,6 +114,7 @@ def test_oidc_authorization_after_revokation(
             client_id=client.client_id,
             scope="openid profile",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )

tests/oidc/test_hybrid_flow.py

@@ -14,6 +14,7 @@ def test_oauth_hybrid(testclient, backend, user, client):
             client_id=client.client_id,
             scope="openid profile",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
     ).follow()
     assert "text/html" == res.content_type, res.json
@@ -55,6 +56,7 @@ def test_oidc_hybrid(testclient, backend, logged_user, client, keypair, trusted_
             client_id=client.client_id,
             scope="openid profile",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
     )
     assert "text/html" == res.content_type, res.json

tests/oidc/test_implicit_flow.py

@@ -19,6 +19,7 @@ def test_oauth_implicit(testclient, user, client, backend):
             client_id=client.client_id,
             scope="profile",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
     ).follow()
     assert "text/html" == res.content_type
@@ -64,6 +65,7 @@ def test_oidc_implicit(testclient, keypair, user, client, trusted_client, backen
             client_id=client.client_id,
             scope="openid profile",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
     ).follow()
     assert "text/html" == res.content_type
@@ -119,6 +121,7 @@ def test_oidc_implicit_with_group(
             client_id=client.client_id,
             scope="openid profile groups",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
     ).follow()
     assert "text/html" == res.content_type

tests/oidc/test_refresh_token.py

@@ -18,6 +18,7 @@ def test_refresh_token(testclient, logged_user, client, backend, caplog):
             client_id=client.client_id,
             scope="openid profile",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
     )
     res = res.form.submit(name="answer", value="accept")
@@ -96,6 +97,7 @@ def test_refresh_token_with_invalid_user(testclient, client, backend):
             client_id=client.client_id,
             scope="openid profile",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
     ).follow()
 
@@ -154,6 +156,7 @@ def test_cannot_refresh_token_for_locked_users(
             client_id=client.client_id,
             scope="openid profile",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
     )
     res = res.form.submit(name="answer", value="accept")

tests/oidc/test_token_expiration.py

@@ -19,6 +19,7 @@ def test_token_default_expiration_date(
             client_id=client.client_id,
             scope="openid profile email groups address phone",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )
@@ -79,6 +80,7 @@ def test_token_custom_expiration_date(
             client_id=client.client_id,
             scope="openid profile email groups address phone",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )

tests/oidc/test_token_introspection.py

@@ -85,6 +85,7 @@ def test_full_flow(testclient, logged_user, client, user, trusted_client, backen
             client_id=client.client_id,
             scope="profile",
             nonce="somenonce",
+            redirect_uri="https://client.test/redirect1",
         ),
         status=200,
     )