Enforce redirect URI scheme policy at authorization time

docs/oauth/phase-2-authorization-request-and-user-consent.md

@@ -96,12 +96,23 @@ Custom validation is implemented in ActivityPubOAuth2Validator:
     - Ensures the activitypub_account_portability scope is present.
     - Logs warnings for invalid or missing scopes.
     - Enforces adherence to LOLA-specific security constraints.
-- **Redirect URI Validation:**
-    - Delegates to Django OAuth Toolkit for standard matching.
-    - Logs invalid URIs and allows for future enhancements (e.g., HTTPS enforcement).
+- **Redirect URI Validation:** two gates, both fail-closed.
+    1. **Registered** — delegates to Django OAuth Toolkit, which matches the URI against the Application's `redirect_uris` allow-list.
+    2. **Allowed scheme** — the URI's scheme must appear in `OAUTH2_PROVIDER["ALLOWED_REDIRECT_URI_SCHEMES"]`. Being registered is *not* sufficient on its own.
 
 This ensures that only valid, registered redirect URIs and scopes can proceed through the authorization process.
 
+#### Redirect URI scheme policy, per environment
+
+| Environment | Allowed schemes | Why |
+|---|---|---|
+| development, test, CI | `http`, `https` | Local callbacks are `http://localhost`; the test factories register one by default |
+| production, staging | `https` only | An authorization code is the credential a destination trades for an access token; delivering one over plaintext undermines LOLA §6.1 |
+
+Why the second gate exists even though DOT owns the setting: DOT enforces it in `Application.clean()`, which only runs under `full_clean()` — and Django's `Model.save()` never calls it, so code paths that write `redirect_uris` directly bypass the check. DOT's remaining backstop fires when the redirect response is built, raising `DisallowedRedirect` (a bare `400`) only *after* the user has already approved. Checking in `validate_redirect_uri` rejects early, with a proper OAuth error, regardless of how the URI was stored.
+
+**When it runs:** the authorization request only. Token exchange uses a different hook (`confirm_redirect_uri`, an exact match against the stored `Grant`), so a refusal here means no authorization code is ever issued for a disallowed URI.
+
 ## Interaction Flow
 
 ### 1. Initiation
@@ -129,7 +140,7 @@ This ensures that only valid, registered redirect URIs and scopes can proceed th
 ## Security Considerations
 
 - **State Parameter** – Prevents CSRF and replay attacks.
-- **Redirect URI Validation** – Defends against open redirect and code interception attacks.
+- **Redirect URI Validation** – Defends against open redirect and code interception attacks. Requires both registration *and* an allowed scheme; a rejection raises a fatal client error, so the browser is never redirected to a URI we refused.
 - **Scope Enforcement** – Limits granted permissions strictly to account portability.
 - **Explicit User Consent** – Ensures the user has control over data transfer.
 - **Logging** – Records key events for auditability (invalid scopes, URIs, approvals).

testbed/core/oauth/validators.py

@@ -1,8 +1,10 @@
 import logging
+from urllib.parse import urlparse
 
 from oauthlib.oauth2.rfc6749.errors import InvalidRequestFatalError
 from oauth2_provider.models import get_access_token_model
 from oauth2_provider.oauth2_validators import OAuth2Validator
+from oauth2_provider.settings import oauth2_settings
 
 from .scopes import LOLA_PORTABILITY_SCOPE, scope_grants_portability
 
@@ -53,18 +55,52 @@ def validate_scopes(self, client_id, scopes, client, request, *args, **kwargs):
         logger.info("Client %s requested valid scopes: %s", client_id, scopes)
         return super().validate_scopes(client_id, scopes, client, request, *args, **kwargs)
 
-    # Additional validation for redirect URIs in ActivityPub context
     def validate_redirect_uri(self, client_id, redirect_uri, request, *args, **kwargs):
+        """
+        Approve a redirect URI only if it is BOTH registered AND uses an allowed scheme.
+
+        Two gates, both fail-closed:
+
+        1. Registration: `super()` matches the URI against the Application's
+           `redirect_uris` allow-list (django-oauth-toolkit's own check).
+        2. Scheme -- the URI's scheme must appear in
+           OAUTH2_PROVIDER["ALLOWED_REDIRECT_URI_SCHEMES"].
+
+        Gate 2 exists because DOT's enforces it in `Application.clean()`, which only
+        runs under `full_clean()`, and Django's `Model.save()` never calls it.
 
-        # Standard validation first
+        Args:
+            client_id: OAuth client identifier
+            redirect_uri: the callback URI the client asked us to redirect to
+            request: the oauthlib Request object
+
+        Returns:
+            bool: True only when both gates pass. Returning False makes oauthlib
+            raise `InvalidRedirectURIError`, a fatal client error, so DOT renders
+            an error page instead of redirecting.
+        """
+        # Gate 1: is it registered?
         valid = super().validate_redirect_uri(client_id, redirect_uri, request, *args, **kwargs)
 
         if not valid:
             logger.warning("Client %s requested invalid redirect URI: %s", client_id, redirect_uri)
             return False
 
-        # We could add additional validation here if needed later on
-        # For example, checking for HTTPS in production
+        # Gate 2: is the scheme allowed in this environment?
+        # lowercases the scheme, the allowed list is lowercased to match, as DOT's own validator does.
+        allowed_schemes = [scheme.lower() for scheme in oauth2_settings.ALLOWED_REDIRECT_URI_SCHEMES]
+        scheme = urlparse(redirect_uri).scheme
+
+        if scheme not in allowed_schemes:
+            logger.warning(
+                "Client %s requested redirect URI with disallowed scheme %r "
+                "(allowed: %s): %s",
+                client_id,
+                scheme,
+                allowed_schemes,
+                redirect_uri,
+            )
+            return False
 
         logger.info("Client %s requested valid redirect URI: %s", client_id, redirect_uri)
 

testbed/core/tests/test_oauth_validators.py

@@ -2,6 +2,7 @@
 from unittest.mock import MagicMock, patch
 
 from django.conf import settings
+from django.test import override_settings
 
 from testbed.core.factories import ApplicationFactory
 from testbed.core.oauth.scopes import LOLA_PORTABILITY_SCOPE, scope_grants_portability
@@ -164,3 +165,44 @@ def test_validate_scopes_rejects_lookalike_scope(oauth_validator, oauth_applicat
             mock_request
         )
     assert not result, "A scope that merely contains the portability scope must not be accepted"
+
+# Redirect URI scheme policy
+
+HTTPS_ONLY = {**settings.OAUTH2_PROVIDER, "ALLOWED_REDIRECT_URI_SCHEMES": ["https"]}
+
+# A registered redirect URI whose scheme is not allowed must still be  refused
+@override_settings(OAUTH2_PROVIDER=HTTPS_ONLY)
+@pytest.mark.django_db
+def test_validate_redirect_uri_rejects_disallowed_scheme(oauth_validator, oauth_application, mock_request):
+    with patch.object(oauth_validator.__class__.__bases__[0], 'validate_redirect_uri', return_value=True):
+        result = oauth_validator.validate_redirect_uri(
+            oauth_application.client_id,
+            'http://example.com/callback',
+            mock_request
+        )
+    assert not result, "A registered redirect URI with a disallowed scheme must be rejected"
+
+
+# Without this, a bug rejecting every URI would still satisfy the test above
+@override_settings(OAUTH2_PROVIDER=HTTPS_ONLY)
+@pytest.mark.django_db
+def test_validate_redirect_uri_allows_configured_scheme(oauth_validator, oauth_application, mock_request):
+    with patch.object(oauth_validator.__class__.__bases__[0], 'validate_redirect_uri', return_value=True):
+        result = oauth_validator.validate_redirect_uri(
+            oauth_application.client_id,
+            'https://example.com/callback',
+            mock_request
+        )
+    assert result, "A registered redirect URI using an allowed scheme must be accepted"
+
+
+# Default policy in settings/base.py must keep accepting http://localhost
+@pytest.mark.django_db
+def test_validate_redirect_uri_allows_http_under_default_policy(oauth_validator, oauth_application, mock_request):
+    with patch.object(oauth_validator.__class__.__bases__[0], 'validate_redirect_uri', return_value=True):
+        result = oauth_validator.validate_redirect_uri(
+            oauth_application.client_id,
+            'http://localhost:8000/callback/',
+            mock_request
+        )
+    assert result, "base.py must keep allowing http so local development and tests work"

testbed/settings/base.py

@@ -242,6 +242,7 @@
     'SCOPES': {
         'activitypub_account_portability': 'ActivityPub Account Portability',
     },
+    'ALLOWED_REDIRECT_URI_SCHEMES': ['http', 'https'],
     'ACCESS_TOKEN_EXPIRE_SECONDS': 3600,  # 1 hour
     'REFRESH_TOKEN_EXPIRE_SECONDS': 86400,  # 1 day
     'AUTHORIZATION_CODE_EXPIRE_SECONDS': 600,  # 10 minutes