-
Notifications
You must be signed in to change notification settings - Fork 501
feat: implement dynamic client registration #7096
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Zaimwa9
wants to merge
18
commits into
feat/setup-dot-and-as-metadata
Choose a base branch
from
feat/implement-dynamic-client-registration
base: feat/setup-dot-and-as-metadata
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
8e9005d
feat: added-dcr-endpoints
Zaimwa9 df96c27
feat: added-throttle-on-dcr-registration-endpoint
Zaimwa9 5e28088
feat: clean-up-stale-apps
Zaimwa9 8a92a8a
feat: added-dcr-tests
Zaimwa9 d741808
feat: use-standard-rfc7591-errors
Zaimwa9 2783d3e
feat: removed-daily-logging-of-created-apps
Zaimwa9 38ed2b9
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 610acbd
feat: added-test-coverage
Zaimwa9 8098090
feat: removed-cleanup-task-antijoin-pattern
Zaimwa9 990f3e3
feat: added-ipv6-local-in-whitelist
Zaimwa9 6806acb
feat: restricted-client-application-to-ascii
Zaimwa9 6a98fa6
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 03717aa
feat: misc-cleanup
Zaimwa9 910765b
Merge branch 'feat/implement-dynamic-client-registration' of github.c…
Zaimwa9 a1fea8e
feat: coverage-on-blank-client-name
Zaimwa9 779b0eb
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 0dcd6b7
feat: blank-client-name-validation-with-drf
Zaimwa9 a10dfd8
feat: renamed-tests
Zaimwa9 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| import re | ||
|
|
||
| from django.core.exceptions import ValidationError as DjangoValidationError | ||
| from rest_framework import serializers | ||
|
|
||
| from oauth2_metadata.services import validate_redirect_uri | ||
|
|
||
| # 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]): | ||
| 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| 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. | ||
|
|
||
| Rules: | ||
| - HTTPS required for all redirect URIs | ||
| - No wildcards, exact match only | ||
| - No fragment components | ||
| - localhost exception: http://localhost:*, http://127.0.0.1:*, and http://[::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", "::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, | ||
| 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,37 @@ | ||
| 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 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 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( | ||
| 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: | ||
| logger.info("OAuth2 DCR cleanup: removed %d stale application(s).", count) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the window of this shoulld be much bigger? something like 500/month?