feat: Dynamic Client Registration Protocol (RFC 7591 / RFC 7592)#1667
feat: Dynamic Client Registration Protocol (RFC 7591 / RFC 7592)#1667zacharypodbela wants to merge 1 commit intodjango-oauth:masterfrom
Conversation
|
@dopry could I get a review on this as well? |
…o-oauth#670) Implements OAuth 2.0 Dynamic Client Registration (RFC 7591) and the Dynamic Client Registration Management Protocol (RFC 7592) via two new views, configurable permission classes, and four new settings. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1119be4 to
8b15437
Compare
There was a problem hiding this comment.
Pull request overview
Implements OAuth 2.0 Dynamic Client Registration (RFC 7591) and Client Registration Management (RFC 7592) in Django OAuth Toolkit, gated behind new DCR_* settings.
Changes:
- Added DCR registration + management views, URL routes, settings, and permission helpers.
- Added comprehensive test suite for DCR flows and configurable settings.
- Added user-facing documentation and changelog entry for the new feature.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_dcr_views.py | New end-to-end and settings coverage tests for DCR POST/GET/PUT/DELETE flows. |
| tests/presets.py | Adds DCR_SETTINGS preset used across the new DCR test suite. |
| oauth2_provider/views/dynamic_client_registration.py | Implements RFC 7591 registration endpoint and RFC 7592 management endpoints. |
| oauth2_provider/views/init.py | Exposes the new DCR views via the public views module. |
| oauth2_provider/urls.py | Registers /register/ and /register/{client_id}/ endpoints under the provider URLs. |
| oauth2_provider/settings.py | Adds DCR_* configuration defaults and marks permission classes as non-empty. |
| oauth2_provider/dcr.py | Adds built-in permission classes used by DCR registration. |
| docs/views/views.rst | Adds DCR docs page to the views documentation toctree. |
| docs/views/dynamic_client_registration.rst | New documentation for endpoints, settings, and examples. |
| CHANGELOG.md | Documents the new DCR feature addition. |
| return result | ||
|
|
||
|
|
||
| @method_decorator(csrf_exempt, name="dispatch") |
There was a problem hiding this comment.
The registration endpoint is csrf_exempt while the default permission class is session-authenticated (IsAuthenticatedDCRPermission). This combination enables CSRF: a third-party site could trigger an authenticated browser to POST /o/register/ using the victim’s session cookies and create clients without the user’s intent. Consider removing csrf_exempt for the registration view (or conditionally enforcing CSRF when request.user.is_authenticated via session), and reserve csrf_exempt for non-cookie auth schemes (e.g., initial-access tokens via Authorization header).
| @method_decorator(csrf_exempt, name="dispatch") |
| # Permission check | ||
| if not _check_permissions(request): | ||
| return _error_response( | ||
| "access_denied", | ||
| "Authentication required to register a client", | ||
| status=401, | ||
| ) |
There was a problem hiding this comment.
The registration endpoint is csrf_exempt while the default permission class is session-authenticated (IsAuthenticatedDCRPermission). This combination enables CSRF: a third-party site could trigger an authenticated browser to POST /o/register/ using the victim’s session cookies and create clients without the user’s intent. Consider removing csrf_exempt for the registration view (or conditionally enforcing CSRF when request.user.is_authenticated via session), and reserve csrf_exempt for non-cookie auth schemes (e.g., initial-access tokens via Authorization header).
| def _check_permissions(request): | ||
| """Run all DCR_REGISTRATION_PERMISSION_CLASSES; return True if all pass.""" | ||
| permission_classes = oauth2_settings.DCR_REGISTRATION_PERMISSION_CLASSES | ||
| for cls in permission_classes: | ||
| instance = cls() | ||
| if not instance.has_permission(request): | ||
| return False | ||
| return True |
There was a problem hiding this comment.
DCR_REGISTRATION_PERMISSION_CLASSES is configured as dotted import paths in settings/docs, but _check_permissions assumes each entry is directly callable (a class). If oauth2_settings does not pre-resolve dotted paths into objects, this will raise at runtime (e.g., 'str' object is not callable). Fix by either (a) ensuring the settings layer resolves this setting to callables, or (b) supporting strings here via django.utils.module_loading.import_string before instantiation.
| # redirect_uris | ||
| redirect_uris = data.get("redirect_uris", []) | ||
| if not isinstance(redirect_uris, list): | ||
| return None, _error_response("invalid_client_metadata", "redirect_uris must be an array") | ||
| kwargs["redirect_uris"] = " ".join(redirect_uris) |
There was a problem hiding this comment.
Two concrete validation issues can cause incorrect behavior or server errors: (1) \" \".join(redirect_uris) will raise TypeError if any element is non-string, resulting in a 500 instead of a 400; validate that all items are strings (and reject otherwise). (2) Any unrecognized token_endpoint_auth_method is silently accepted and treated as confidential while responses always report client_secret_basic, which can misrepresent the registered metadata; validate supported values (e.g., only none and client_secret_basic) and return invalid_client_metadata for others.
| # token_endpoint_auth_method → client_type | ||
| auth_method = data.get("token_endpoint_auth_method", "client_secret_basic") | ||
| if auth_method == "none": | ||
| kwargs["client_type"] = "public" | ||
| else: | ||
| kwargs["client_type"] = "confidential" |
There was a problem hiding this comment.
Two concrete validation issues can cause incorrect behavior or server errors: (1) \" \".join(redirect_uris) will raise TypeError if any element is non-string, resulting in a 500 instead of a 400; validate that all items are strings (and reject otherwise). (2) Any unrecognized token_endpoint_auth_method is silently accepted and treated as confidential while responses always report client_secret_basic, which can misrepresent the registered metadata; validate supported values (e.g., only none and client_secret_basic) and return invalid_client_metadata for others.
| def delete(self, request, client_id, *args, **kwargs): | ||
| application, result = self._get_application_from_registration_token(request, client_id) | ||
| if application is None: | ||
| return result | ||
|
|
||
| application.delete() | ||
| return JsonResponse({}, status=204) |
There was a problem hiding this comment.
A 204 No Content response should not include a response body. Returning JsonResponse({}, status=204) will still send a JSON payload and Content-Type. Prefer returning an empty HttpResponse(status=204) (or similar) for correct HTTP semantics.
| (`RFC 7591 <https://datatracker.ietf.org/doc/html/rfc7591>`_) and the OAuth 2.0 Dynamic Client | ||
| Registration Management Protocol (`RFC 7592 <https://datatracker.ietf.org/doc/html/rfc7592>`_). | ||
|
|
||
| These views are automatically included in ``base_urlpatterns`` when you use |
There was a problem hiding this comment.
This is inaccurate relative to the implementation: the DCR routes are added via urlpatterns = ... + dcr_urlpatterns, not included in base_urlpatterns. Please adjust the wording to avoid pointing readers to the wrong internal URL list (e.g., say the endpoints are included when include(\"oauth2_provider.urls\") is used, without naming base_urlpatterns).
| These views are automatically included in ``base_urlpatterns`` when you use | |
| These views are automatically available when you use |
| dcr_urlpatterns = [ | ||
| path("register/", views.DynamicClientRegistrationView.as_view(), name="dcr-register"), | ||
| path( | ||
| "register/<str:client_id>/", | ||
| views.DynamicClientRegistrationManagementView.as_view(), | ||
| name="dcr-register-management", | ||
| ), | ||
| ] |
There was a problem hiding this comment.
The indentation inside dcr_urlpatterns is inconsistent (line 61 is over-indented relative to the rest of the list). This is easy to miss in reviews and makes the file harder to scan; align the indentation consistently with the surrounding URL pattern lists.
| """ | ||
|
|
||
| import json | ||
| import logging |
There was a problem hiding this comment.
logger is defined but not used anywhere in this module. Either remove the import/variable to keep the module minimal, or add logging in concrete places where it would be actionable (e.g., invalid metadata failures or token mismatch scenarios) to justify keeping it.
| from ..settings import oauth2_settings | ||
|
|
||
|
|
||
| logger = logging.getLogger(__name__) |
There was a problem hiding this comment.
logger is defined but not used anywhere in this module. Either remove the import/variable to keep the module minimal, or add logging in concrete places where it would be actionable (e.g., invalid metadata failures or token mismatch scenarios) to justify keeping it.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
@zacharypodbela can you address the copilot comments and rebase the branch? |
|
It would be useful to somehow mark Applications as created by DCR. |
@magicbrothers This seems like a valuable bit of UX. I don't work in environments with DCR. What kind of DCR volume do you see in your projects? |
Summary
Implements the OAuth 2.0 Dynamic Client Registration Protocol (RFC 7591) and the Dynamic Client Registration Management Protocol (RFC 7592), allowing clients to programmatically register without manual admin action.
Closes #670
Implementation
Most of the implementation details stem directly from RFC requirements.
Here are the design decisions I made that were not directly specified by the RFCs. Whenever possible I created settings that allow developers to adjust in case they want different behavior:
Using
AccessTokenmodel withDCR_REGISTRATION_SCOPE="oauth2_provider:registration"for the registration_access_token — RFC 7592 just says "issue a token"; using the existing model is my implementation choice. The default scope having oauth2_provider namespace should be unique enough to avoid collisions, but developers are free to override if needed/desired.DCR_REGISTRATION_TOKEN_EXPIRE_SECONDS=None→ year 9999 — defaulting to "never expire" and using year 9999 to satisfy the model's requiredexpiresfield are both judgment calls.DCR_ROTATE_REGISTRATION_TOKEN_ON_UPDATE=True— RFC 7592 says the server "MAY" issue a new registration access token after a PUT. I choose a default of always rotating as theres no downside, but developers are free to configure otherwise.URL paths
register/andregister/{client_id}/— RFC specifies no URL structure.Throw 400 if the client submits multiple grant types — RFC 7591 allows clients to register with an array of grant types, e.g.:
{"grant_types": ["authorization_code", "refresh_token", "client_credentials"]}. DOT's Application model only supports oneauthorization_grant_typecurrently. Rather than altering the model, we'll just have to not comply with this requirement for now and we'll throw 400 if client tries to submit multiple (better to provide clients with clarity than silently fail and choose one of the submitted).Default permission class being
IsAuthenticatedDCRPermission— the RFC says thePOST register/endpoint "MAY" be protected; I chose a secure default. The library also providesAllowAllDCRPermissionout of the box, or developers can roll their own Permission class. Easy to customize.Follow same View-layer guard pattern as OIDC, with
DCR_ENABLED=False— New routes are always registered and returned, but DCREnabledMixin returns 404/403 whenDCR_ENABLEDsetting is False.DCR_ENABLEDdefaults to False so that DCR is not automatically enabled for current package users. This is the safest non-breaking approach that follows the existing paradigms.New endpoints
POST/o/register/GET/o/register/{client_id}/PUT/o/register/{client_id}/DELETE/o/register/{client_id}/Both endpoints are gated behind a new
DCR_ENABLEDsetting (defaultFalse), so existing installations are unaffected until they opt in.New settings (
OAUTH2_PROVIDER)DCR_ENABLEDFalseTrueto activate the endpointsDCR_REGISTRATION_PERMISSION_CLASSES("oauth2_provider.dcr.IsAuthenticatedDCRPermission",)DCR_REGISTRATION_SCOPE"oauth2_provider:registration"DCR_REGISTRATION_TOKEN_EXPIRE_SECONDSNone(year 9999)DCR_ROTATE_REGISTRATION_TOKEN_ON_UPDATETruePUTBuilt-in permission classes (
oauth2_provider.dcr)IsAuthenticatedDCRPermission— requires Django session authentication (default)AllowAllDCRPermission— open registration, no auth requiredCustom classes only need to implement
has_permission(request) -> bool, making it straightforward to support initial-access tokens or any other scheme.RFC 7592 management token
After a successful
POST /register/, the response includes aregistration_access_token(stored as anAccessTokenwithscope = DCR_REGISTRATION_SCOPE) that must be presented as aBearertoken to the management endpoints.Test plan
tests/test_dcr_views.pycovering registration, management (GET/PUT/DELETE), all new settings, and a full register→GET→PUT→DELETE roundtrip