From 8e9005d17fdf06bdc4a1e9e9501cffc197b070f9 Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 31 Mar 2026 10:28:57 +0200 Subject: [PATCH 01/17] feat: added-dcr-endpoints --- api/app/urls.py | 10 +++- api/oauth2_metadata/serializers.py | 78 ++++++++++++++++++++++++++++++ api/oauth2_metadata/services.py | 52 ++++++++++++++++++++ api/oauth2_metadata/views.py | 41 ++++++++++++++++ 4 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 api/oauth2_metadata/serializers.py create mode 100644 api/oauth2_metadata/services.py diff --git a/api/app/urls.py b/api/app/urls.py index b9a7e1181b32..5df2790853fc 100644 --- a/api/app/urls.py +++ b/api/app/urls.py @@ -6,7 +6,10 @@ from django.urls import include, path, re_path from django.views.generic.base import TemplateView -from oauth2_metadata.views import authorization_server_metadata +from oauth2_metadata.views import ( + DynamicClientRegistrationView, + authorization_server_metadata, +) from users.views import password_reset_redirect from . import views @@ -53,6 +56,11 @@ "robots.txt", TemplateView.as_view(template_name="robots.txt", content_type="text/plain"), ), + path( + "o/register/", + DynamicClientRegistrationView.as_view(), + name="oauth2-dcr-register", + ), # Authorize template view for testing: this will be moved to the frontend in following issues path("o/", include("oauth2_provider.urls", namespace="oauth2_provider")), ] diff --git a/api/oauth2_metadata/serializers.py b/api/oauth2_metadata/serializers.py new file mode 100644 index 000000000000..ef29c56a828f --- /dev/null +++ b/api/oauth2_metadata/serializers.py @@ -0,0 +1,78 @@ +import re + +from django.core.exceptions import ValidationError as DjangoValidationError +from rest_framework import serializers + +from oauth2_metadata.services import validate_redirect_uri + +# Allow letters, digits, spaces, hyphens, underscores, dots, and parentheses. +_CLIENT_NAME_RE = re.compile(r"^[\w\s.\-()]+$", re.UNICODE) + + +class DCRRequestSerializer(serializers.Serializer[None]): + client_name = serializers.CharField(max_length=255, required=True) + redirect_uris = serializers.ListField( + child=serializers.URLField(), + min_length=1, + max_length=5, + required=True, + ) + grant_types = serializers.ListField( + child=serializers.CharField(), + required=False, + default=["authorization_code", "refresh_token"], + ) + response_types = serializers.ListField( + child=serializers.CharField(), + required=False, + default=["code"], + ) + token_endpoint_auth_method = serializers.CharField( + required=False, + default="none", + ) + + def validate_client_name(self, value: str) -> str: + if not _CLIENT_NAME_RE.match(value): + raise serializers.ValidationError( + "Client name may only contain letters, digits, spaces, " + "hyphens, underscores, dots, and parentheses." + ) + return value + + def validate_redirect_uris(self, value: list[str]) -> list[str]: + errors: list[str] = [] + for uri in value: + try: + validate_redirect_uri(uri) + except DjangoValidationError as e: + errors.append(str(e.message)) + if errors: + raise serializers.ValidationError(errors) + return value + + def validate_token_endpoint_auth_method(self, value: str) -> str: + if value != "none": + raise serializers.ValidationError( + "Only public clients are supported; " + "token_endpoint_auth_method must be 'none'." + ) + return value + + def validate_grant_types(self, value: list[str]) -> list[str]: + allowed = {"authorization_code", "refresh_token"} + invalid = set(value) - allowed + if invalid: + raise serializers.ValidationError( + f"Unsupported grant types: {', '.join(sorted(invalid))}" + ) + return value + + def validate_response_types(self, value: list[str]) -> list[str]: + allowed = {"code"} + invalid = set(value) - allowed + if invalid: + raise serializers.ValidationError( + f"Unsupported response types: {', '.join(sorted(invalid))}" + ) + return value diff --git a/api/oauth2_metadata/services.py b/api/oauth2_metadata/services.py new file mode 100644 index 000000000000..951cd5f0d460 --- /dev/null +++ b/api/oauth2_metadata/services.py @@ -0,0 +1,52 @@ +from urllib.parse import urlparse + +from django.core.exceptions import ValidationError +from oauth2_provider.models import Application + + +def validate_redirect_uri(uri: str) -> str: + """Validate a single redirect URI per DCR policy. + + Rules: + - HTTPS required for all redirect URIs + - No wildcards, exact match only + - No fragment components + - localhost exception: http://localhost:* and http://127.0.0.1:* permitted + """ + parsed = urlparse(uri) + + if not parsed.scheme or not parsed.netloc: + raise ValidationError(f"Invalid URI: {uri}") + + if "*" in uri: + raise ValidationError( + f"Wildcards are not permitted in redirect URIs: {uri}" + ) + + if parsed.fragment: + raise ValidationError(f"Fragment components are not permitted: {uri}") + + is_localhost = parsed.hostname in ("localhost", "127.0.0.1") + + if parsed.scheme != "https" and not (parsed.scheme == "http" and is_localhost): + raise ValidationError( + f"HTTPS is required for redirect URIs (localhost excepted): {uri}" + ) + + return uri + + +def create_oauth2_application( + *, + client_name: str, + redirect_uris: list[str], +) -> Application: + """Create a public OAuth2 application for dynamic client registration.""" + application: Application = Application.objects.create( + name=client_name, + client_type=Application.CLIENT_PUBLIC, + authorization_grant_type=Application.GRANT_AUTHORIZATION_CODE, + redirect_uris=" ".join(redirect_uris), + skip_authorization=False, + ) + return application diff --git a/api/oauth2_metadata/views.py b/api/oauth2_metadata/views.py index 25cbc77071d5..2609d4eaa039 100644 --- a/api/oauth2_metadata/views.py +++ b/api/oauth2_metadata/views.py @@ -4,6 +4,15 @@ from django.http import HttpRequest, JsonResponse from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_GET +from rest_framework import status as drf_status +from rest_framework.permissions import AllowAny +from rest_framework.request import Request +from rest_framework.response import Response +from rest_framework.throttling import ScopedRateThrottle +from rest_framework.views import APIView + +from oauth2_metadata.serializers import DCRRequestSerializer +from oauth2_metadata.services import create_oauth2_application @csrf_exempt @@ -35,3 +44,35 @@ def authorization_server_metadata(request: HttpRequest) -> JsonResponse: } return JsonResponse(metadata) + + +class DynamicClientRegistrationView(APIView): + """RFC 7591 Dynamic Client Registration endpoint.""" + + authentication_classes: list[type] = [] + permission_classes = [AllowAny] + throttle_classes = [ScopedRateThrottle] + throttle_scope = "dcr_register" + + def post(self, request: Request) -> Response: + serializer = DCRRequestSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + data = serializer.validated_data + + application = create_oauth2_application( + client_name=data["client_name"], + redirect_uris=data["redirect_uris"], + ) + + return Response( + { + "client_id": application.client_id, + "client_name": application.name, + "redirect_uris": data["redirect_uris"], + "grant_types": data["grant_types"], + "response_types": data["response_types"], + "token_endpoint_auth_method": data["token_endpoint_auth_method"], + "client_id_issued_at": int(application.created.timestamp()), + }, + status=drf_status.HTTP_201_CREATED, + ) From df96c27e7165f34c3fc0be0d140db0bf5e571ec2 Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 31 Mar 2026 10:30:53 +0200 Subject: [PATCH 02/17] feat: added-throttle-on-dcr-registration-endpoint --- api/app/settings/common.py | 2 ++ api/app/settings/test.py | 1 + 2 files changed, 3 insertions(+) diff --git a/api/app/settings/common.py b/api/app/settings/common.py index e82fd1b8241a..0b667b0f5467 100644 --- a/api/app/settings/common.py +++ b/api/app/settings/common.py @@ -304,6 +304,7 @@ LOGIN_THROTTLE_RATE = env("LOGIN_THROTTLE_RATE", "20/min") +DCR_THROTTLE_RATE = env("DCR_THROTTLE_RATE", "10/min") SIGNUP_THROTTLE_RATE = env("SIGNUP_THROTTLE_RATE", "10000/min") USER_THROTTLE_RATE = env("USER_THROTTLE_RATE", default=None) MASTER_API_KEY_THROTTLE_RATE = env("MASTER_API_KEY_THROTTLE_RATE", default=None) @@ -322,6 +323,7 @@ "DEFAULT_THROTTLE_CLASSES": DEFAULT_THROTTLE_CLASSES, "DEFAULT_THROTTLE_RATES": { "login": LOGIN_THROTTLE_RATE, + "dcr_register": DCR_THROTTLE_RATE, "signup": SIGNUP_THROTTLE_RATE, "master_api_key": MASTER_API_KEY_THROTTLE_RATE, "mfa_code": "5/min", diff --git a/api/app/settings/test.py b/api/app/settings/test.py index 1f0ab33f395c..edcead60e710 100644 --- a/api/app/settings/test.py +++ b/api/app/settings/test.py @@ -7,6 +7,7 @@ REST_FRAMEWORK["DEFAULT_THROTTLE_CLASSES"] = ["core.throttling.UserRateThrottle"] REST_FRAMEWORK["DEFAULT_THROTTLE_RATES"] = { "login": "100/min", + "dcr_register": "100/min", "mfa_code": "5/min", "invite": "10/min", "signup": "100/min", From 5e28088aafda7e082e67aa6671b9152f9fa17ad2 Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 31 Mar 2026 10:31:30 +0200 Subject: [PATCH 03/17] feat: clean-up-stale-apps --- api/oauth2_metadata/tasks.py | 43 ++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/api/oauth2_metadata/tasks.py b/api/oauth2_metadata/tasks.py index 372078267f9e..fd8c3d2cfc57 100644 --- a/api/oauth2_metadata/tasks.py +++ b/api/oauth2_metadata/tasks.py @@ -1,9 +1,52 @@ +import logging from datetime import timedelta from django.core.management import call_command +from django.utils import timezone from task_processor.decorators import register_recurring_task +logger = logging.getLogger(__name__) + @register_recurring_task(run_every=timedelta(hours=24)) def clear_expired_oauth2_tokens() -> None: call_command("cleartokens") + + +@register_recurring_task(run_every=timedelta(hours=24)) +def log_new_oauth2_application_registrations() -> None: + from oauth2_provider.models import Application + + since = timezone.now() - timedelta(hours=24) + count: int = Application.objects.filter(created__gte=since).count() + total: int = Application.objects.count() + logger.info( + "OAuth2 DCR monitoring: %d new application(s) registered in the last 24h " + "(total: %d).", + count, + total, + ) + + +@register_recurring_task(run_every=timedelta(hours=24)) +def cleanup_stale_oauth2_applications() -> None: + """Remove DCR applications that were never used to obtain a token. + + An application is considered stale if it was registered more than 14 days + ago and has no associated access tokens, refresh tokens, or grants. + """ + from oauth2_provider.models import AccessToken, Application, Grant, RefreshToken + + threshold = timezone.now() - timedelta(days=14) + stale = ( + Application.objects.filter( + created__lt=threshold, + user__isnull=True, # Only DCR-created apps (no user) + ) + .exclude(pk__in=AccessToken.objects.values("application_id")) + .exclude(pk__in=RefreshToken.objects.values("application_id")) + .exclude(pk__in=Grant.objects.values("application_id")) + ) + count, _ = stale.delete() + if count: + logger.info("OAuth2 DCR cleanup: removed %d stale application(s).", count) From 8a92a8aeb169b92a846051cb51a2e64d1da4b355 Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 31 Mar 2026 10:31:51 +0200 Subject: [PATCH 04/17] feat: added-dcr-tests --- api/tests/unit/oauth2_metadata/test_dcr.py | 323 +++++++++++++++++++ api/tests/unit/oauth2_metadata/test_tasks.py | 111 ++++++- 2 files changed, 433 insertions(+), 1 deletion(-) create mode 100644 api/tests/unit/oauth2_metadata/test_dcr.py diff --git a/api/tests/unit/oauth2_metadata/test_dcr.py b/api/tests/unit/oauth2_metadata/test_dcr.py new file mode 100644 index 000000000000..d06a20b9b4d4 --- /dev/null +++ b/api/tests/unit/oauth2_metadata/test_dcr.py @@ -0,0 +1,323 @@ +from unittest.mock import patch + +import pytest +from django.urls import reverse +from oauth2_provider.models import Application +from rest_framework import status +from rest_framework.test import APIClient + +DCR_URL = reverse("oauth2-dcr-register") + + +@pytest.fixture() +def api_client() -> APIClient: + return APIClient() + + +def _valid_payload(**overrides: object) -> dict[str, object]: + payload: dict[str, object] = { + "client_name": "Test MCP Client", + "redirect_uris": ["https://example.com/callback"], + } + payload.update(overrides) + return payload + + +@pytest.mark.django_db() +def test_dcr_register__valid_request__returns_201_with_client_id( + api_client: APIClient, +) -> None: + # Given + payload = _valid_payload() + + # When + response = api_client.post(DCR_URL, data=payload, format="json") + + # Then + assert response.status_code == status.HTTP_201_CREATED + data = response.json() + assert data["client_id"] + assert data["client_name"] == "Test MCP Client" + assert data["redirect_uris"] == ["https://example.com/callback"] + assert data["grant_types"] == ["authorization_code", "refresh_token"] + assert data["response_types"] == ["code"] + assert data["token_endpoint_auth_method"] == "none" + assert isinstance(data["client_id_issued_at"], int) + + +@pytest.mark.django_db() +def test_dcr_register__localhost_http__returns_201( + api_client: APIClient, +) -> None: + # Given + payload = _valid_payload( + redirect_uris=["http://localhost:8080/callback"], + ) + + # When + response = api_client.post(DCR_URL, data=payload, format="json") + + # Then + assert response.status_code == status.HTTP_201_CREATED + + +@pytest.mark.django_db() +def test_dcr_register__127_0_0_1_http__returns_201( + api_client: APIClient, +) -> None: + # Given + payload = _valid_payload( + redirect_uris=["http://127.0.0.1:3000/callback"], + ) + + # When + response = api_client.post(DCR_URL, data=payload, format="json") + + # Then + assert response.status_code == status.HTTP_201_CREATED + + +def test_dcr_register__missing_client_name__returns_400( + api_client: APIClient, +) -> None: + # Given + payload = {"redirect_uris": ["https://example.com/callback"]} + + # When + response = api_client.post(DCR_URL, data=payload, format="json") + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "client_name" in response.json() + + +def test_dcr_register__missing_redirect_uris__returns_400( + api_client: APIClient, +) -> None: + # Given + payload = {"client_name": "Test"} + + # When + response = api_client.post(DCR_URL, data=payload, format="json") + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "redirect_uris" in response.json() + + +def test_dcr_register__empty_redirect_uris__returns_400( + api_client: APIClient, +) -> None: + # Given + payload = _valid_payload(redirect_uris=[]) + + # When + response = api_client.post(DCR_URL, data=payload, format="json") + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "redirect_uris" in response.json() + + +def test_dcr_register__http_redirect_uri__returns_400( + api_client: APIClient, +) -> None: + # Given + payload = _valid_payload( + redirect_uris=["http://example.com/callback"], + ) + + # When + response = api_client.post(DCR_URL, data=payload, format="json") + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "redirect_uris" in response.json() + + +def test_dcr_register__wildcard_redirect_uri__returns_400( + api_client: APIClient, +) -> None: + # Given + payload = _valid_payload( + redirect_uris=["https://*.example.com/callback"], + ) + + # When + response = api_client.post(DCR_URL, data=payload, format="json") + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "redirect_uris" in response.json() + + +def test_dcr_register__fragment_in_redirect_uri__returns_400( + api_client: APIClient, +) -> None: + # Given + payload = _valid_payload( + redirect_uris=["https://example.com/callback#frag"], + ) + + # When + response = api_client.post(DCR_URL, data=payload, format="json") + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "redirect_uris" in response.json() + + +def test_dcr_register__unsupported_grant_type__returns_400( + api_client: APIClient, +) -> None: + # Given + payload = _valid_payload(grant_types=["implicit"]) + + # When + response = api_client.post(DCR_URL, data=payload, format="json") + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "grant_types" in response.json() + + +def test_dcr_register__unsupported_response_type__returns_400( + api_client: APIClient, +) -> None: + # Given + payload = _valid_payload(response_types=["token"]) + + # When + response = api_client.post(DCR_URL, data=payload, format="json") + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "response_types" in response.json() + + +def test_dcr_register__unsupported_auth_method__returns_400( + api_client: APIClient, +) -> None: + # Given + payload = _valid_payload(token_endpoint_auth_method="client_secret_basic") + + # When + response = api_client.post(DCR_URL, data=payload, format="json") + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "token_endpoint_auth_method" in response.json() + + +@pytest.mark.django_db() +def test_dcr_register__defaults_applied__returns_expected_defaults( + api_client: APIClient, +) -> None: + # Given - only required fields + payload = _valid_payload() + + # When + response = api_client.post(DCR_URL, data=payload, format="json") + + # Then + data = response.json() + assert data["grant_types"] == ["authorization_code", "refresh_token"] + assert data["response_types"] == ["code"] + assert data["token_endpoint_auth_method"] == "none" + + +@pytest.mark.django_db() +def test_dcr_register__creates_public_application_in_database( + api_client: APIClient, +) -> None: + # Given + payload = _valid_payload() + + # When + response = api_client.post(DCR_URL, data=payload, format="json") + + # Then + client_id = response.json()["client_id"] + application = Application.objects.get(client_id=client_id) + assert application.client_type == Application.CLIENT_PUBLIC + assert application.authorization_grant_type == Application.GRANT_AUTHORIZATION_CODE + assert application.name == "Test MCP Client" + assert "https://example.com/callback" in application.redirect_uris + assert application.user is None + assert application.skip_authorization is False + + +def test_dcr_register__too_many_redirect_uris__returns_400( + api_client: APIClient, +) -> None: + # Given + payload = _valid_payload( + redirect_uris=[f"https://example.com/cb{i}" for i in range(6)], + ) + + # When + response = api_client.post(DCR_URL, data=payload, format="json") + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "redirect_uris" in response.json() + + +def test_dcr_register__html_in_client_name__returns_400( + api_client: APIClient, +) -> None: + # Given + payload = _valid_payload(client_name="") + + # When + response = api_client.post(DCR_URL, data=payload, format="json") + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "client_name" in response.json() + + +@pytest.mark.django_db() +def test_dcr_register__valid_client_name_with_special_chars__returns_201( + api_client: APIClient, +) -> None: + # Given — parentheses, dots, hyphens are allowed + payload = _valid_payload(client_name="Claude Desktop (v2.1-beta)") + + # When + response = api_client.post(DCR_URL, data=payload, format="json") + + # Then + assert response.status_code == status.HTTP_201_CREATED + assert response.json()["client_name"] == "Claude Desktop (v2.1-beta)" + + +def test_dcr_register__get_request__returns_405( + api_client: APIClient, +) -> None: + # When + response = api_client.get(DCR_URL) + + # Then + assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED + + +@pytest.mark.django_db() +def test_dcr_register__rate_limited__returns_429( + api_client: APIClient, +) -> None: + # Given + payload = _valid_payload() + + with patch( + "rest_framework.throttling.ScopedRateThrottle.allow_request", + return_value=False, + ), patch( + "rest_framework.throttling.ScopedRateThrottle.wait", + return_value=60.0, + ): + # When + response = api_client.post(DCR_URL, data=payload, format="json") + + # Then + assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS diff --git a/api/tests/unit/oauth2_metadata/test_tasks.py b/api/tests/unit/oauth2_metadata/test_tasks.py index d5ea32e9bc0c..0ff01c8ff95b 100644 --- a/api/tests/unit/oauth2_metadata/test_tasks.py +++ b/api/tests/unit/oauth2_metadata/test_tasks.py @@ -1,6 +1,15 @@ +from datetime import timedelta from unittest.mock import MagicMock -from oauth2_metadata.tasks import clear_expired_oauth2_tokens +import pytest +from django.utils import timezone +from oauth2_provider.models import AccessToken, Application + +from oauth2_metadata.tasks import ( + cleanup_stale_oauth2_applications, + clear_expired_oauth2_tokens, + log_new_oauth2_application_registrations, +) def test_clear_expired_oauth2_tokens__called__invokes_cleartokens_command( @@ -14,3 +23,103 @@ def test_clear_expired_oauth2_tokens__called__invokes_cleartokens_command( # Then mock_call_command.assert_called_once_with("cleartokens") + + +@pytest.mark.django_db() +def test_log_new_oauth2_application_registrations__called__logs_count( + mocker: MagicMock, +) -> None: + # Given + Application.objects.create( + name="Test App", + client_type=Application.CLIENT_PUBLIC, + authorization_grant_type=Application.GRANT_AUTHORIZATION_CODE, + redirect_uris="https://example.com/callback", + ) + mock_logger = mocker.patch("oauth2_metadata.tasks.logger") + + # When + log_new_oauth2_application_registrations() + + # Then + mock_logger.info.assert_called_once() + args = mock_logger.info.call_args + assert args[0][1] == 1 # 1 new application + assert args[0][2] == 1 # 1 total + + +@pytest.mark.django_db() +def test_cleanup_stale_oauth2_applications__stale_app__deletes_it( + mocker: MagicMock, +) -> None: + # Given - an app created 15 days ago with no tokens + app = Application.objects.create( + name="Stale App", + client_type=Application.CLIENT_PUBLIC, + authorization_grant_type=Application.GRANT_AUTHORIZATION_CODE, + redirect_uris="https://example.com/callback", + ) + Application.objects.filter(pk=app.pk).update( + created=timezone.now() - timedelta(days=15), + ) + + # When + cleanup_stale_oauth2_applications() + + # Then + assert not Application.objects.filter(pk=app.pk).exists() + + +@pytest.mark.django_db() +def test_cleanup_stale_oauth2_applications__app_with_token__keeps_it( + admin_user: None, + mocker: MagicMock, +) -> None: + # Given - an old app that has an access token + from django.contrib.auth import get_user_model + + User = get_user_model() + user = User.objects.first() + app = Application.objects.create( + name="Active App", + client_type=Application.CLIENT_PUBLIC, + authorization_grant_type=Application.GRANT_AUTHORIZATION_CODE, + redirect_uris="https://example.com/callback", + ) + Application.objects.filter(pk=app.pk).update( + created=timezone.now() - timedelta(days=15), + ) + AccessToken.objects.create( + user=user, + application=app, + token="test-token", + expires=timezone.now() + timedelta(hours=1), + ) + + # When + cleanup_stale_oauth2_applications() + + # Then + assert Application.objects.filter(pk=app.pk).exists() + + +@pytest.mark.django_db() +def test_cleanup_stale_oauth2_applications__recent_app__keeps_it( + mocker: MagicMock, +) -> None: + # Given - an app created 5 days ago with no tokens + app = Application.objects.create( + name="Recent App", + client_type=Application.CLIENT_PUBLIC, + authorization_grant_type=Application.GRANT_AUTHORIZATION_CODE, + redirect_uris="https://example.com/callback", + ) + Application.objects.filter(pk=app.pk).update( + created=timezone.now() - timedelta(days=5), + ) + + # When + cleanup_stale_oauth2_applications() + + # Then + assert Application.objects.filter(pk=app.pk).exists() From d7418082f30f423139bf4b23cce4498e87304e3c Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 1 Apr 2026 09:57:15 +0200 Subject: [PATCH 05/17] feat: use-standard-rfc7591-errors --- api/oauth2_metadata/services.py | 4 +- api/oauth2_metadata/views.py | 30 ++- api/oauth2_test_server.mjs | 2 +- api/tests/unit/oauth2_metadata/test_dcr.py | 269 ++++++++------------- 4 files changed, 132 insertions(+), 173 deletions(-) diff --git a/api/oauth2_metadata/services.py b/api/oauth2_metadata/services.py index 951cd5f0d460..092d282c1904 100644 --- a/api/oauth2_metadata/services.py +++ b/api/oauth2_metadata/services.py @@ -19,9 +19,7 @@ def validate_redirect_uri(uri: str) -> str: raise ValidationError(f"Invalid URI: {uri}") if "*" in uri: - raise ValidationError( - f"Wildcards are not permitted in redirect URIs: {uri}" - ) + raise ValidationError(f"Wildcards are not permitted in redirect URIs: {uri}") if parsed.fragment: raise ValidationError(f"Fragment components are not permitted: {uri}") diff --git a/api/oauth2_metadata/views.py b/api/oauth2_metadata/views.py index 2609d4eaa039..7a592016f66e 100644 --- a/api/oauth2_metadata/views.py +++ b/api/oauth2_metadata/views.py @@ -54,9 +54,20 @@ class DynamicClientRegistrationView(APIView): throttle_classes = [ScopedRateThrottle] throttle_scope = "dcr_register" + # Map DRF serializer field names to RFC 7591 error codes. + _rfc7591_error_codes: dict[str, str] = { + "redirect_uris": "invalid_redirect_uri", + "client_name": "invalid_client_metadata", + "grant_types": "invalid_client_metadata", + "response_types": "invalid_client_metadata", + "token_endpoint_auth_method": "invalid_client_metadata", + } + def post(self, request: Request) -> Response: serializer = DCRRequestSerializer(data=request.data) - serializer.is_valid(raise_exception=True) + if not serializer.is_valid(): + return self._rfc7591_error_response(serializer.errors) + data = serializer.validated_data application = create_oauth2_application( @@ -76,3 +87,20 @@ def post(self, request: Request) -> Response: }, status=drf_status.HTTP_201_CREATED, ) + + def _rfc7591_error_response(self, errors: dict[str, list[str]]) -> Response: + """Format validation errors per RFC 7591 section 3.2.2.""" + first_field = next(iter(errors)) + error_code = self._rfc7591_error_codes.get( + first_field, "invalid_client_metadata" + ) + messages = errors[first_field] + description = messages[0] if isinstance(messages[0], str) else str(messages[0]) + + return Response( + { + "error": error_code, + "error_description": description, + }, + status=drf_status.HTTP_400_BAD_REQUEST, + ) diff --git a/api/oauth2_test_server.mjs b/api/oauth2_test_server.mjs index 1d0029a59faf..d93af0f29bee 100644 --- a/api/oauth2_test_server.mjs +++ b/api/oauth2_test_server.mjs @@ -1,7 +1,7 @@ import { createServer } from "node:http"; import { randomBytes, createHash } from "node:crypto"; -const CLIENT_ID = "ZLsLu3hhJI4GlhNsGeFVC3K2U3QBGfXtmc0EcyiG"; +const CLIENT_ID = "B4wAl37pg9y1PRsIvAXZ14cTp0FpqpNCtMSI7ETC"; const REDIRECT_URI = "http://localhost:3000/oauth/callback"; const API_URL = "http://localhost:8000"; const PORT = 3000; diff --git a/api/tests/unit/oauth2_metadata/test_dcr.py b/api/tests/unit/oauth2_metadata/test_dcr.py index d06a20b9b4d4..35e594a1406e 100644 --- a/api/tests/unit/oauth2_metadata/test_dcr.py +++ b/api/tests/unit/oauth2_metadata/test_dcr.py @@ -46,13 +46,21 @@ def test_dcr_register__valid_request__returns_201_with_client_id( @pytest.mark.django_db() -def test_dcr_register__localhost_http__returns_201( +@pytest.mark.parametrize( + "redirect_uri", + [ + "http://localhost:8080/callback", + "http://127.0.0.1:3000/callback", + "https://example.com/callback", + ], + ids=["localhost", "127.0.0.1", "https"], +) +def test_dcr_register__valid_redirect_uri__returns_201( api_client: APIClient, + redirect_uri: str, ) -> None: # Given - payload = _valid_payload( - redirect_uris=["http://localhost:8080/callback"], - ) + payload = _valid_payload(redirect_uris=[redirect_uri]) # When response = api_client.post(DCR_URL, data=payload, format="json") @@ -62,151 +70,28 @@ def test_dcr_register__localhost_http__returns_201( @pytest.mark.django_db() -def test_dcr_register__127_0_0_1_http__returns_201( +@pytest.mark.parametrize( + "client_name", + [ + "Claude Desktop (v2.1-beta)", + "My_App.test", + "Simple", + ], + ids=["special-chars", "underscores-dots", "simple"], +) +def test_dcr_register__valid_client_name__returns_201( api_client: APIClient, + client_name: str, ) -> None: # Given - payload = _valid_payload( - redirect_uris=["http://127.0.0.1:3000/callback"], - ) + payload = _valid_payload(client_name=client_name) # When response = api_client.post(DCR_URL, data=payload, format="json") # Then assert response.status_code == status.HTTP_201_CREATED - - -def test_dcr_register__missing_client_name__returns_400( - api_client: APIClient, -) -> None: - # Given - payload = {"redirect_uris": ["https://example.com/callback"]} - - # When - response = api_client.post(DCR_URL, data=payload, format="json") - - # Then - assert response.status_code == status.HTTP_400_BAD_REQUEST - assert "client_name" in response.json() - - -def test_dcr_register__missing_redirect_uris__returns_400( - api_client: APIClient, -) -> None: - # Given - payload = {"client_name": "Test"} - - # When - response = api_client.post(DCR_URL, data=payload, format="json") - - # Then - assert response.status_code == status.HTTP_400_BAD_REQUEST - assert "redirect_uris" in response.json() - - -def test_dcr_register__empty_redirect_uris__returns_400( - api_client: APIClient, -) -> None: - # Given - payload = _valid_payload(redirect_uris=[]) - - # When - response = api_client.post(DCR_URL, data=payload, format="json") - - # Then - assert response.status_code == status.HTTP_400_BAD_REQUEST - assert "redirect_uris" in response.json() - - -def test_dcr_register__http_redirect_uri__returns_400( - api_client: APIClient, -) -> None: - # Given - payload = _valid_payload( - redirect_uris=["http://example.com/callback"], - ) - - # When - response = api_client.post(DCR_URL, data=payload, format="json") - - # Then - assert response.status_code == status.HTTP_400_BAD_REQUEST - assert "redirect_uris" in response.json() - - -def test_dcr_register__wildcard_redirect_uri__returns_400( - api_client: APIClient, -) -> None: - # Given - payload = _valid_payload( - redirect_uris=["https://*.example.com/callback"], - ) - - # When - response = api_client.post(DCR_URL, data=payload, format="json") - - # Then - assert response.status_code == status.HTTP_400_BAD_REQUEST - assert "redirect_uris" in response.json() - - -def test_dcr_register__fragment_in_redirect_uri__returns_400( - api_client: APIClient, -) -> None: - # Given - payload = _valid_payload( - redirect_uris=["https://example.com/callback#frag"], - ) - - # When - response = api_client.post(DCR_URL, data=payload, format="json") - - # Then - assert response.status_code == status.HTTP_400_BAD_REQUEST - assert "redirect_uris" in response.json() - - -def test_dcr_register__unsupported_grant_type__returns_400( - api_client: APIClient, -) -> None: - # Given - payload = _valid_payload(grant_types=["implicit"]) - - # When - response = api_client.post(DCR_URL, data=payload, format="json") - - # Then - assert response.status_code == status.HTTP_400_BAD_REQUEST - assert "grant_types" in response.json() - - -def test_dcr_register__unsupported_response_type__returns_400( - api_client: APIClient, -) -> None: - # Given - payload = _valid_payload(response_types=["token"]) - - # When - response = api_client.post(DCR_URL, data=payload, format="json") - - # Then - assert response.status_code == status.HTTP_400_BAD_REQUEST - assert "response_types" in response.json() - - -def test_dcr_register__unsupported_auth_method__returns_400( - api_client: APIClient, -) -> None: - # Given - payload = _valid_payload(token_endpoint_auth_method="client_secret_basic") - - # When - response = api_client.post(DCR_URL, data=payload, format="json") - - # Then - assert response.status_code == status.HTTP_400_BAD_REQUEST - assert "token_endpoint_auth_method" in response.json() + assert response.json()["client_name"] == client_name @pytest.mark.django_db() @@ -227,7 +112,7 @@ def test_dcr_register__defaults_applied__returns_expected_defaults( @pytest.mark.django_db() -def test_dcr_register__creates_public_application_in_database( +def test_dcr_register__valid_request__creates_public_application_in_database( api_client: APIClient, ) -> None: # Given @@ -247,55 +132,100 @@ def test_dcr_register__creates_public_application_in_database( assert application.skip_authorization is False -def test_dcr_register__too_many_redirect_uris__returns_400( +@pytest.mark.parametrize( + ("redirect_uris", "expected_fragment"), + [ + (["http://example.com/callback"], "HTTPS"), + (["https://example.com/callback#frag"], "Fragment"), + (["https://*.example.com/callback"], ""), # Rejected by URLField + ([], ""), # Empty list + ([f"https://example.com/cb{i}" for i in range(6)], ""), # Too many + ], + ids=["http-non-localhost", "fragment", "wildcard", "empty-list", "too-many"], +) +def test_dcr_register__invalid_redirect_uris__returns_rfc7591_error( api_client: APIClient, + redirect_uris: list[str], + expected_fragment: str, ) -> None: # Given - payload = _valid_payload( - redirect_uris=[f"https://example.com/cb{i}" for i in range(6)], - ) + payload = _valid_payload(redirect_uris=redirect_uris) # When response = api_client.post(DCR_URL, data=payload, format="json") # Then assert response.status_code == status.HTTP_400_BAD_REQUEST - assert "redirect_uris" in response.json() - - -def test_dcr_register__html_in_client_name__returns_400( + data = response.json() + assert data["error"] == "invalid_redirect_uri" + assert "error_description" in data + if expected_fragment: + assert expected_fragment in data["error_description"] + + +@pytest.mark.parametrize( + ("overrides", "expected_fragment"), + [ + ({"client_name": ""}, ""), + ({"grant_types": ["implicit"]}, "grant type"), + ({"response_types": ["token"]}, "response type"), + ({"token_endpoint_auth_method": "client_secret_basic"}, "public clients"), + ], + ids=["xss-client-name", "bad-grant-type", "bad-response-type", "bad-auth-method"], +) +def test_dcr_register__invalid_client_metadata__returns_rfc7591_error( api_client: APIClient, + overrides: dict[str, object], + expected_fragment: str, ) -> None: # Given - payload = _valid_payload(client_name="") + payload = _valid_payload(**overrides) # When response = api_client.post(DCR_URL, data=payload, format="json") # Then assert response.status_code == status.HTTP_400_BAD_REQUEST - assert "client_name" in response.json() - - -@pytest.mark.django_db() -def test_dcr_register__valid_client_name_with_special_chars__returns_201( + data = response.json() + assert data["error"] == "invalid_client_metadata" + assert "error_description" in data + if expected_fragment: + assert expected_fragment in data["error_description"].lower() + + +@pytest.mark.parametrize( + ("payload", "expected_error"), + [ + ( + {"redirect_uris": ["https://example.com/callback"]}, + "invalid_client_metadata", + ), + ( + {"client_name": "Test"}, + "invalid_redirect_uri", + ), + ], + ids=["missing-client-name", "missing-redirect-uris"], +) +def test_dcr_register__missing_required_field__returns_rfc7591_error( api_client: APIClient, + payload: dict[str, object], + expected_error: str, ) -> None: - # Given — parentheses, dots, hyphens are allowed - payload = _valid_payload(client_name="Claude Desktop (v2.1-beta)") - - # When + # Given / When response = api_client.post(DCR_URL, data=payload, format="json") # Then - assert response.status_code == status.HTTP_201_CREATED - assert response.json()["client_name"] == "Claude Desktop (v2.1-beta)" + assert response.status_code == status.HTTP_400_BAD_REQUEST + data = response.json() + assert data["error"] == expected_error + assert "error_description" in data def test_dcr_register__get_request__returns_405( api_client: APIClient, ) -> None: - # When + # Given / When response = api_client.get(DCR_URL) # Then @@ -309,12 +239,15 @@ def test_dcr_register__rate_limited__returns_429( # Given payload = _valid_payload() - with patch( - "rest_framework.throttling.ScopedRateThrottle.allow_request", - return_value=False, - ), patch( - "rest_framework.throttling.ScopedRateThrottle.wait", - return_value=60.0, + with ( + patch( + "rest_framework.throttling.ScopedRateThrottle.allow_request", + return_value=False, + ), + patch( + "rest_framework.throttling.ScopedRateThrottle.wait", + return_value=60.0, + ), ): # When response = api_client.post(DCR_URL, data=payload, format="json") From 2783d3e6750322747668079fb0956533edffd273 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 1 Apr 2026 16:36:31 +0200 Subject: [PATCH 06/17] feat: removed-daily-logging-of-created-apps --- api/oauth2_metadata/tasks.py | 15 ------------- api/tests/unit/oauth2_metadata/test_tasks.py | 23 -------------------- 2 files changed, 38 deletions(-) diff --git a/api/oauth2_metadata/tasks.py b/api/oauth2_metadata/tasks.py index fd8c3d2cfc57..8731a3ad8673 100644 --- a/api/oauth2_metadata/tasks.py +++ b/api/oauth2_metadata/tasks.py @@ -13,21 +13,6 @@ def clear_expired_oauth2_tokens() -> None: call_command("cleartokens") -@register_recurring_task(run_every=timedelta(hours=24)) -def log_new_oauth2_application_registrations() -> None: - from oauth2_provider.models import Application - - since = timezone.now() - timedelta(hours=24) - count: int = Application.objects.filter(created__gte=since).count() - total: int = Application.objects.count() - logger.info( - "OAuth2 DCR monitoring: %d new application(s) registered in the last 24h " - "(total: %d).", - count, - total, - ) - - @register_recurring_task(run_every=timedelta(hours=24)) def cleanup_stale_oauth2_applications() -> None: """Remove DCR applications that were never used to obtain a token. diff --git a/api/tests/unit/oauth2_metadata/test_tasks.py b/api/tests/unit/oauth2_metadata/test_tasks.py index 0ff01c8ff95b..93a91eaf3e5c 100644 --- a/api/tests/unit/oauth2_metadata/test_tasks.py +++ b/api/tests/unit/oauth2_metadata/test_tasks.py @@ -8,7 +8,6 @@ from oauth2_metadata.tasks import ( cleanup_stale_oauth2_applications, clear_expired_oauth2_tokens, - log_new_oauth2_application_registrations, ) @@ -25,28 +24,6 @@ def test_clear_expired_oauth2_tokens__called__invokes_cleartokens_command( mock_call_command.assert_called_once_with("cleartokens") -@pytest.mark.django_db() -def test_log_new_oauth2_application_registrations__called__logs_count( - mocker: MagicMock, -) -> None: - # Given - Application.objects.create( - name="Test App", - client_type=Application.CLIENT_PUBLIC, - authorization_grant_type=Application.GRANT_AUTHORIZATION_CODE, - redirect_uris="https://example.com/callback", - ) - mock_logger = mocker.patch("oauth2_metadata.tasks.logger") - - # When - log_new_oauth2_application_registrations() - - # Then - mock_logger.info.assert_called_once() - args = mock_logger.info.call_args - assert args[0][1] == 1 # 1 new application - assert args[0][2] == 1 # 1 total - @pytest.mark.django_db() def test_cleanup_stale_oauth2_applications__stale_app__deletes_it( From 38ed2b9c04b4aa3399b7755103c6e7f016125586 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 1 Apr 2026 14:57:02 +0000 Subject: [PATCH 07/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- api/tests/unit/oauth2_metadata/test_tasks.py | 1 - 1 file changed, 1 deletion(-) diff --git a/api/tests/unit/oauth2_metadata/test_tasks.py b/api/tests/unit/oauth2_metadata/test_tasks.py index 93a91eaf3e5c..a958121b744c 100644 --- a/api/tests/unit/oauth2_metadata/test_tasks.py +++ b/api/tests/unit/oauth2_metadata/test_tasks.py @@ -24,7 +24,6 @@ def test_clear_expired_oauth2_tokens__called__invokes_cleartokens_command( mock_call_command.assert_called_once_with("cleartokens") - @pytest.mark.django_db() def test_cleanup_stale_oauth2_applications__stale_app__deletes_it( mocker: MagicMock, From 610acbd790212d69905b00813f847c10a4ef790f Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 1 Apr 2026 17:14:30 +0200 Subject: [PATCH 08/17] feat: added-test-coverage --- api/tests/unit/oauth2_metadata/test_dcr.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/api/tests/unit/oauth2_metadata/test_dcr.py b/api/tests/unit/oauth2_metadata/test_dcr.py index 35e594a1406e..b5ecb6cd7ed2 100644 --- a/api/tests/unit/oauth2_metadata/test_dcr.py +++ b/api/tests/unit/oauth2_metadata/test_dcr.py @@ -1,11 +1,14 @@ from unittest.mock import patch import pytest +from django.core.exceptions import ValidationError from django.urls import reverse from oauth2_provider.models import Application from rest_framework import status from rest_framework.test import APIClient +from oauth2_metadata.services import validate_redirect_uri + DCR_URL = reverse("oauth2-dcr-register") @@ -254,3 +257,20 @@ def test_dcr_register__rate_limited__returns_429( # Then assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS + + +@pytest.mark.parametrize( + ("uri", "expected_message"), + [ + ("not-a-uri", "Invalid URI"), + ("https://*.example.com/callback", "Wildcards"), + ], + ids=["invalid-uri", "wildcard"], +) +def test_validate_redirect_uri__invalid_input__raises_validation_error( + uri: str, + expected_message: str, +) -> None: + # Given / When / Then + with pytest.raises(ValidationError, match=expected_message): + validate_redirect_uri(uri) From 8098090afdb2bdbd7a47e89398f5f619c279c513 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 1 Apr 2026 17:23:27 +0200 Subject: [PATCH 09/17] feat: removed-cleanup-task-antijoin-pattern --- api/oauth2_metadata/tasks.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/api/oauth2_metadata/tasks.py b/api/oauth2_metadata/tasks.py index 8731a3ad8673..ffb64d4bf1bd 100644 --- a/api/oauth2_metadata/tasks.py +++ b/api/oauth2_metadata/tasks.py @@ -20,17 +20,18 @@ def cleanup_stale_oauth2_applications() -> None: An application is considered stale if it was registered more than 14 days ago and has no associated access tokens, refresh tokens, or grants. """ + from django.db.models import Exists, OuterRef + from oauth2_provider.models import AccessToken, Application, Grant, RefreshToken threshold = timezone.now() - timedelta(days=14) - stale = ( - Application.objects.filter( - created__lt=threshold, - user__isnull=True, # Only DCR-created apps (no user) - ) - .exclude(pk__in=AccessToken.objects.values("application_id")) - .exclude(pk__in=RefreshToken.objects.values("application_id")) - .exclude(pk__in=Grant.objects.values("application_id")) + stale = Application.objects.filter( + created__lt=threshold, + user__isnull=True, # Only DCR-created apps (no user) + ).exclude( + Exists(AccessToken.objects.filter(application=OuterRef("pk"))) + | Exists(RefreshToken.objects.filter(application=OuterRef("pk"))) + | Exists(Grant.objects.filter(application=OuterRef("pk"))) ) count, _ = stale.delete() if count: From 990f3e37304bc005ca256c856775b9c7b17a51d3 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 1 Apr 2026 17:26:22 +0200 Subject: [PATCH 10/17] feat: added-ipv6-local-in-whitelist --- api/oauth2_metadata/services.py | 4 ++-- api/tests/unit/oauth2_metadata/test_dcr.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/api/oauth2_metadata/services.py b/api/oauth2_metadata/services.py index 092d282c1904..943f4147e59d 100644 --- a/api/oauth2_metadata/services.py +++ b/api/oauth2_metadata/services.py @@ -11,7 +11,7 @@ def validate_redirect_uri(uri: str) -> str: - HTTPS required for all redirect URIs - No wildcards, exact match only - No fragment components - - localhost exception: http://localhost:* and http://127.0.0.1:* permitted + - localhost exception: http://localhost:*, http://127.0.0.1:*, and http://[::1]:* permitted """ parsed = urlparse(uri) @@ -24,7 +24,7 @@ def validate_redirect_uri(uri: str) -> str: if parsed.fragment: raise ValidationError(f"Fragment components are not permitted: {uri}") - is_localhost = parsed.hostname in ("localhost", "127.0.0.1") + is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1") if parsed.scheme != "https" and not (parsed.scheme == "http" and is_localhost): raise ValidationError( diff --git a/api/tests/unit/oauth2_metadata/test_dcr.py b/api/tests/unit/oauth2_metadata/test_dcr.py index b5ecb6cd7ed2..0aeee827c361 100644 --- a/api/tests/unit/oauth2_metadata/test_dcr.py +++ b/api/tests/unit/oauth2_metadata/test_dcr.py @@ -54,9 +54,10 @@ def test_dcr_register__valid_request__returns_201_with_client_id( [ "http://localhost:8080/callback", "http://127.0.0.1:3000/callback", + "http://[::1]:3000/callback", "https://example.com/callback", ], - ids=["localhost", "127.0.0.1", "https"], + ids=["localhost", "127.0.0.1", "::1", "https"], ) def test_dcr_register__valid_redirect_uri__returns_201( api_client: APIClient, From 6806acb7b4936fbb3332169a6c79974b0da4e261 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 1 Apr 2026 17:29:27 +0200 Subject: [PATCH 11/17] feat: restricted-client-application-to-ascii --- api/oauth2_metadata/serializers.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/api/oauth2_metadata/serializers.py b/api/oauth2_metadata/serializers.py index ef29c56a828f..da0648743bb1 100644 --- a/api/oauth2_metadata/serializers.py +++ b/api/oauth2_metadata/serializers.py @@ -5,8 +5,9 @@ from oauth2_metadata.services import validate_redirect_uri -# Allow letters, digits, spaces, hyphens, underscores, dots, and parentheses. -_CLIENT_NAME_RE = re.compile(r"^[\w\s.\-()]+$", re.UNICODE) +# Allow ASCII letters, digits, spaces, hyphens, underscores, dots, and parentheses. +# ASCII-only to prevent Unicode homoglyph spoofing on the consent screen. +_CLIENT_NAME_RE = re.compile(r"^[\w\s.\-()]+$", re.ASCII) class DCRRequestSerializer(serializers.Serializer[None]): From 6a98fa6951e4c4d657d1b989607ac1cb44731a6b Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 1 Apr 2026 15:29:47 +0000 Subject: [PATCH 12/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- api/oauth2_metadata/tasks.py | 1 - 1 file changed, 1 deletion(-) diff --git a/api/oauth2_metadata/tasks.py b/api/oauth2_metadata/tasks.py index ffb64d4bf1bd..b8620b95b9b7 100644 --- a/api/oauth2_metadata/tasks.py +++ b/api/oauth2_metadata/tasks.py @@ -21,7 +21,6 @@ def cleanup_stale_oauth2_applications() -> None: ago and has no associated access tokens, refresh tokens, or grants. """ from django.db.models import Exists, OuterRef - from oauth2_provider.models import AccessToken, Application, Grant, RefreshToken threshold = timezone.now() - timedelta(days=14) From 03717aaecf1b971e13c9cab0577b7e9a9aa5d0e8 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 1 Apr 2026 17:54:49 +0200 Subject: [PATCH 13/17] feat: misc-cleanup --- api/oauth2_metadata/serializers.py | 3 +++ api/oauth2_metadata/services.py | 9 +++++++++ api/oauth2_metadata/tasks.py | 1 - api/tests/unit/oauth2_metadata/test_dcr.py | 3 ++- api/tests/unit/oauth2_metadata/test_tasks.py | 18 +++++------------- 5 files changed, 19 insertions(+), 15 deletions(-) diff --git a/api/oauth2_metadata/serializers.py b/api/oauth2_metadata/serializers.py index da0648743bb1..51527e579d25 100644 --- a/api/oauth2_metadata/serializers.py +++ b/api/oauth2_metadata/serializers.py @@ -34,6 +34,9 @@ class DCRRequestSerializer(serializers.Serializer[None]): ) def validate_client_name(self, value: str) -> str: + value = value.strip() + if not value: + raise serializers.ValidationError("Client name must not be blank.") if not _CLIENT_NAME_RE.match(value): raise serializers.ValidationError( "Client name may only contain letters, digits, spaces, " diff --git a/api/oauth2_metadata/services.py b/api/oauth2_metadata/services.py index 943f4147e59d..837c3a87fd71 100644 --- a/api/oauth2_metadata/services.py +++ b/api/oauth2_metadata/services.py @@ -1,8 +1,11 @@ +import logging from urllib.parse import urlparse from django.core.exceptions import ValidationError from oauth2_provider.models import Application +logger = logging.getLogger(__name__) + def validate_redirect_uri(uri: str) -> str: """Validate a single redirect URI per DCR policy. @@ -44,7 +47,13 @@ def create_oauth2_application( name=client_name, client_type=Application.CLIENT_PUBLIC, authorization_grant_type=Application.GRANT_AUTHORIZATION_CODE, + client_secret="", redirect_uris=" ".join(redirect_uris), skip_authorization=False, ) + logger.info( + "OAuth2 DCR: registered application %s (client_id=%s).", + client_name, + application.client_id, + ) return application diff --git a/api/oauth2_metadata/tasks.py b/api/oauth2_metadata/tasks.py index ffb64d4bf1bd..b8620b95b9b7 100644 --- a/api/oauth2_metadata/tasks.py +++ b/api/oauth2_metadata/tasks.py @@ -21,7 +21,6 @@ def cleanup_stale_oauth2_applications() -> None: ago and has no associated access tokens, refresh tokens, or grants. """ from django.db.models import Exists, OuterRef - from oauth2_provider.models import AccessToken, Application, Grant, RefreshToken threshold = timezone.now() - timedelta(days=14) diff --git a/api/tests/unit/oauth2_metadata/test_dcr.py b/api/tests/unit/oauth2_metadata/test_dcr.py index 0aeee827c361..c18e93c274e1 100644 --- a/api/tests/unit/oauth2_metadata/test_dcr.py +++ b/api/tests/unit/oauth2_metadata/test_dcr.py @@ -272,6 +272,7 @@ def test_validate_redirect_uri__invalid_input__raises_validation_error( uri: str, expected_message: str, ) -> None: - # Given / When / Then + # Given / When + # Then with pytest.raises(ValidationError, match=expected_message): validate_redirect_uri(uri) diff --git a/api/tests/unit/oauth2_metadata/test_tasks.py b/api/tests/unit/oauth2_metadata/test_tasks.py index a958121b744c..1f14faa4c609 100644 --- a/api/tests/unit/oauth2_metadata/test_tasks.py +++ b/api/tests/unit/oauth2_metadata/test_tasks.py @@ -2,6 +2,7 @@ from unittest.mock import MagicMock import pytest +from django.contrib.auth.models import AbstractUser from django.utils import timezone from oauth2_provider.models import AccessToken, Application @@ -25,9 +26,7 @@ def test_clear_expired_oauth2_tokens__called__invokes_cleartokens_command( @pytest.mark.django_db() -def test_cleanup_stale_oauth2_applications__stale_app__deletes_it( - mocker: MagicMock, -) -> None: +def test_cleanup_stale_oauth2_applications__stale_app__deletes_it() -> None: # Given - an app created 15 days ago with no tokens app = Application.objects.create( name="Stale App", @@ -48,14 +47,9 @@ def test_cleanup_stale_oauth2_applications__stale_app__deletes_it( @pytest.mark.django_db() def test_cleanup_stale_oauth2_applications__app_with_token__keeps_it( - admin_user: None, - mocker: MagicMock, + admin_user: AbstractUser, ) -> None: # Given - an old app that has an access token - from django.contrib.auth import get_user_model - - User = get_user_model() - user = User.objects.first() app = Application.objects.create( name="Active App", client_type=Application.CLIENT_PUBLIC, @@ -66,7 +60,7 @@ def test_cleanup_stale_oauth2_applications__app_with_token__keeps_it( created=timezone.now() - timedelta(days=15), ) AccessToken.objects.create( - user=user, + user=admin_user, application=app, token="test-token", expires=timezone.now() + timedelta(hours=1), @@ -80,9 +74,7 @@ def test_cleanup_stale_oauth2_applications__app_with_token__keeps_it( @pytest.mark.django_db() -def test_cleanup_stale_oauth2_applications__recent_app__keeps_it( - mocker: MagicMock, -) -> None: +def test_cleanup_stale_oauth2_applications__recent_app__keeps_it() -> None: # Given - an app created 5 days ago with no tokens app = Application.objects.create( name="Recent App", From a1fea8ed64c0b626b9b432d60b8e24e37455bd75 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 1 Apr 2026 18:38:53 +0200 Subject: [PATCH 14/17] feat: coverage-on-blank-client-name --- api/tests/unit/oauth2_metadata/test_dcr.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/tests/unit/oauth2_metadata/test_dcr.py b/api/tests/unit/oauth2_metadata/test_dcr.py index c18e93c274e1..0adf7851ae23 100644 --- a/api/tests/unit/oauth2_metadata/test_dcr.py +++ b/api/tests/unit/oauth2_metadata/test_dcr.py @@ -171,11 +171,12 @@ def test_dcr_register__invalid_redirect_uris__returns_rfc7591_error( ("overrides", "expected_fragment"), [ ({"client_name": ""}, ""), + ({"client_name": " "}, "blank"), ({"grant_types": ["implicit"]}, "grant type"), ({"response_types": ["token"]}, "response type"), ({"token_endpoint_auth_method": "client_secret_basic"}, "public clients"), ], - ids=["xss-client-name", "bad-grant-type", "bad-response-type", "bad-auth-method"], + ids=["xss-client-name", "blank-client-name", "bad-grant-type", "bad-response-type", "bad-auth-method"], ) def test_dcr_register__invalid_client_metadata__returns_rfc7591_error( api_client: APIClient, From 779b0ebaad34559c88cd623b5a7368a2789900fb Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 1 Apr 2026 16:39:51 +0000 Subject: [PATCH 15/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- api/tests/unit/oauth2_metadata/test_dcr.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/api/tests/unit/oauth2_metadata/test_dcr.py b/api/tests/unit/oauth2_metadata/test_dcr.py index 0adf7851ae23..f3b75fe19dae 100644 --- a/api/tests/unit/oauth2_metadata/test_dcr.py +++ b/api/tests/unit/oauth2_metadata/test_dcr.py @@ -176,7 +176,13 @@ def test_dcr_register__invalid_redirect_uris__returns_rfc7591_error( ({"response_types": ["token"]}, "response type"), ({"token_endpoint_auth_method": "client_secret_basic"}, "public clients"), ], - ids=["xss-client-name", "blank-client-name", "bad-grant-type", "bad-response-type", "bad-auth-method"], + ids=[ + "xss-client-name", + "blank-client-name", + "bad-grant-type", + "bad-response-type", + "bad-auth-method", + ], ) def test_dcr_register__invalid_client_metadata__returns_rfc7591_error( api_client: APIClient, From 0dcd6b7cf6964f72849909ea1fedc8610ae9e135 Mon Sep 17 00:00:00 2001 From: wadii Date: Thu, 2 Apr 2026 11:10:51 +0200 Subject: [PATCH 16/17] feat: blank-client-name-validation-with-drf --- api/oauth2_metadata/serializers.py | 3 --- api/tests/unit/oauth2_metadata/test_dcr.py | 16 ++++++---------- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/api/oauth2_metadata/serializers.py b/api/oauth2_metadata/serializers.py index 51527e579d25..da0648743bb1 100644 --- a/api/oauth2_metadata/serializers.py +++ b/api/oauth2_metadata/serializers.py @@ -34,9 +34,6 @@ class DCRRequestSerializer(serializers.Serializer[None]): ) def validate_client_name(self, value: str) -> str: - value = value.strip() - if not value: - raise serializers.ValidationError("Client name must not be blank.") if not _CLIENT_NAME_RE.match(value): raise serializers.ValidationError( "Client name may only contain letters, digits, spaces, " diff --git a/api/tests/unit/oauth2_metadata/test_dcr.py b/api/tests/unit/oauth2_metadata/test_dcr.py index f3b75fe19dae..cb7fbc949a00 100644 --- a/api/tests/unit/oauth2_metadata/test_dcr.py +++ b/api/tests/unit/oauth2_metadata/test_dcr.py @@ -141,9 +141,9 @@ def test_dcr_register__valid_request__creates_public_application_in_database( [ (["http://example.com/callback"], "HTTPS"), (["https://example.com/callback#frag"], "Fragment"), - (["https://*.example.com/callback"], ""), # Rejected by URLField - ([], ""), # Empty list - ([f"https://example.com/cb{i}" for i in range(6)], ""), # Too many + (["https://*.example.com/callback"], "valid URL"), + ([], "at least 1"), + ([f"https://example.com/cb{i}" for i in range(6)], "no more than 5"), ], ids=["http-non-localhost", "fragment", "wildcard", "empty-list", "too-many"], ) @@ -162,15 +162,13 @@ def test_dcr_register__invalid_redirect_uris__returns_rfc7591_error( assert response.status_code == status.HTTP_400_BAD_REQUEST data = response.json() assert data["error"] == "invalid_redirect_uri" - assert "error_description" in data - if expected_fragment: - assert expected_fragment in data["error_description"] + assert expected_fragment.lower() in data["error_description"].lower() @pytest.mark.parametrize( ("overrides", "expected_fragment"), [ - ({"client_name": ""}, ""), + ({"client_name": ""}, "letters"), ({"client_name": " "}, "blank"), ({"grant_types": ["implicit"]}, "grant type"), ({"response_types": ["token"]}, "response type"), @@ -199,9 +197,7 @@ def test_dcr_register__invalid_client_metadata__returns_rfc7591_error( assert response.status_code == status.HTTP_400_BAD_REQUEST data = response.json() assert data["error"] == "invalid_client_metadata" - assert "error_description" in data - if expected_fragment: - assert expected_fragment in data["error_description"].lower() + assert expected_fragment.lower() in data["error_description"].lower() @pytest.mark.parametrize( From a10dfd8dd0e177b14030262aa2d40fd6318a6949 Mon Sep 17 00:00:00 2001 From: wadii Date: Thu, 2 Apr 2026 11:15:27 +0200 Subject: [PATCH 17/17] feat: renamed-tests --- api/tests/unit/oauth2_metadata/test_tasks.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/tests/unit/oauth2_metadata/test_tasks.py b/api/tests/unit/oauth2_metadata/test_tasks.py index 1f14faa4c609..2ad72b75ea09 100644 --- a/api/tests/unit/oauth2_metadata/test_tasks.py +++ b/api/tests/unit/oauth2_metadata/test_tasks.py @@ -26,8 +26,8 @@ def test_clear_expired_oauth2_tokens__called__invokes_cleartokens_command( @pytest.mark.django_db() -def test_cleanup_stale_oauth2_applications__stale_app__deletes_it() -> None: - # Given - an app created 15 days ago with no tokens +def test_cleanup_stale_oauth2_applications__old_app_with_no_token__deletes_it() -> None: + # Given app = Application.objects.create( name="Stale App", client_type=Application.CLIENT_PUBLIC, @@ -46,10 +46,10 @@ def test_cleanup_stale_oauth2_applications__stale_app__deletes_it() -> None: @pytest.mark.django_db() -def test_cleanup_stale_oauth2_applications__app_with_token__keeps_it( +def test_cleanup_stale_oauth2_applications__old_app_with_token__keeps_it( admin_user: AbstractUser, ) -> None: - # Given - an old app that has an access token + # Given app = Application.objects.create( name="Active App", client_type=Application.CLIENT_PUBLIC,