Skip to content

Add an authorize endpoint that uses JSON instead of a Django template… #1306

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.

Sign up for GitHub

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

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
+84 −78
Draft

Add an authorize endpoint that uses JSON instead of a Django template… #1306

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8b76b0d
Add an authorize endpoint that uses JSON instead of a Django template…
jhnbyrn Aug 12, 2023
29a084d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 12, 2023
701bacb
Add post handler
jhnbyrn Aug 16, 2023
6155ce0
Merge branch 'issue-1305-json-auth-endpoint' of github.com:jhnbyrn/dj…
jhnbyrn Aug 16, 2023
855296d
Remove JSON view
jhnbyrn Aug 31, 2023
1bbbe49
Clean up imports
jhnbyrn Aug 31, 2023
514ff85
Merge branch 'master' into issue-1305-json-auth-endpoint
jhnbyrn Aug 31, 2023
d0c03e5
Handle other responses from AuthorizationView GET
jhnbyrn Sep 2, 2023
78bd3e8
Merge branch 'issue-1305-json-auth-endpoint' of github.com:jhnbyrn/dj…
jhnbyrn Sep 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
162 changes: 84 additions & 78 deletions oauth2_provider/views/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,83 @@ def redirect(self, redirect_to, application):
RFC3339 = "%Y-%m-%dT%H:%M:%SZ"


class AuthorizationView(BaseAuthorizationView, FormView):
class AuthorizationMixin:
Copy link
Member

@dopry dopry Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename AuthorizationMixin to AuthorizationViewMixin.

def get_context(self, request, *args, **kwargs):
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The get_context method lacks a docstring. This method performs complex authorization logic and has multiple return types (dict or HttpResponse). Add a docstring explaining its purpose, parameters, return values (including the dual return type behavior), and any exceptions that may be raised.

Suggested change
def get_context(self, request, *args, **kwargs):
def get_context(self, request, *args, **kwargs):
"""
Process the OAuth2 authorization request and build the context for the authorization view.
This method performs complex authorization logic, including validating the authorization request,
handling special prompt parameters, checking for prior user consent, and preparing context data
for the authorization form.
Parameters:
request (HttpRequest): The current HTTP request object.
*args: Additional positional arguments.
**kwargs: Additional keyword arguments used to build the context.
Returns:
dict: A context dictionary containing authorization data for rendering the authorization form,
if user consent is required.
HttpResponse: An HTTP response (such as a redirect or error response) if the authorization
request is invalid, or if user consent can be skipped.
Side Effects:
Sets self.oauth2_data to the context dictionary if consent is required.
Exceptions:
OAuthToolkitError: Raised if the authorization request is invalid. In this case, an error
response is returned.
"""

Copilot uses AI. Check for mistakes.
try:
scopes, credentials = self.validate_authorization_request(request)
except OAuthToolkitError as error:
# Application is not available at this time.
return self.error_response(error, application=None)

prompt = request.GET.get("prompt")
if prompt == "login":
return self.handle_prompt_login()

all_scopes = get_scopes_backend().get_all_scopes()
kwargs["scopes_descriptions"] = [all_scopes[scope] for scope in scopes]
kwargs["scopes"] = scopes
# at this point we know an Application instance with such client_id exists in the database

# TODO: Cache this!
application = get_application_model().objects.get(client_id=credentials["client_id"])

kwargs["application"] = application
kwargs["client_id"] = credentials["client_id"]
kwargs["redirect_uri"] = credentials["redirect_uri"]
kwargs["response_type"] = credentials["response_type"]
kwargs["state"] = credentials["state"]
if "code_challenge" in credentials:
kwargs["code_challenge"] = credentials["code_challenge"]
if "code_challenge_method" in credentials:
kwargs["code_challenge_method"] = credentials["code_challenge_method"]
if "nonce" in credentials:
kwargs["nonce"] = credentials["nonce"]
if "claims" in credentials:
kwargs["claims"] = json.dumps(credentials["claims"])

self.oauth2_data = kwargs

# Check to see if the user has already granted access and return
# a successful response depending on "approval_prompt" url parameter
require_approval = request.GET.get("approval_prompt", oauth2_settings.REQUEST_APPROVAL_PROMPT)

try:
# If skip_authorization field is True, skip the authorization screen even
# if this is the first use of the application and there was no previous authorization.
# This is useful for in-house applications-> assume an in-house applications
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammar error: 'an in-house applications' should be 'in-house applications' (remove 'an'). Also, there's a missing space after the arrow: 'applications->' should be 'applications ->'.

Suggested change
# This is useful for in-house applications-> assume an in-house applications
# This is useful for in-house applications -> assume in-house applications

Copilot uses AI. Check for mistakes.
# are already approved.
if application.skip_authorization:
uri, headers, body, status = self.create_authorization_response(
request=self.request, scopes=" ".join(scopes), credentials=credentials, allow=True
)
return self.redirect(uri, application)

elif require_approval == "auto":
tokens = (
get_access_token_model()
.objects.filter(
Copy link

@dashdanw dashdanw Sep 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could potentially avoid iteration by filtering by your desired scopes (assuming the scopes var here is the required scopes of the view)

Suggested change
.objects.filter(
scopes_filter = { 'scopes__icontains': scope for scope in scopes }
.objects.filter(
user=request.user,
application=kwargs["application"],
expires__gt=timezone.now(),
**scopes_filter
).first()

user=request.user, application=kwargs["application"], expires__gt=timezone.now()
)
.all()
)

# check past authorizations regarded the same scopes as the current one
for token in tokens:
if token.allow_scopes(scopes):
uri, headers, body, status = self.create_authorization_response(
request=self.request,
scopes=" ".join(scopes),
credentials=credentials,
allow=True,
)
return self.redirect(uri, application)

except OAuthToolkitError as error:
Copy link
Member

@dopry dopry Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having the function return the context or response is awkard. It should just return the context.

let the exception throw and hoist the try/except to the get method, so the get method is returning the error_response

return self.error_response(error, application)
return kwargs


class AuthorizationView(BaseAuthorizationView, FormView, AuthorizationMixin):
"""
Implements an endpoint to handle *Authorization Requests* as in :rfc:`4.1.1` and prompting the
user with a form to determine if she authorizes the client application to access her data.
Expand Down Expand Up @@ -128,7 +204,6 @@ def form_valid(self, form):

scopes = form.cleaned_data.get("scope")
allow = form.cleaned_data.get("allow")

try:
uri, headers, body, status = self.create_authorization_response(
request=self.request, scopes=scopes, credentials=credentials, allow=allow
Expand All @@ -141,82 +216,13 @@ def form_valid(self, form):
return self.redirect(self.success_url, application)

def get(self, request, *args, **kwargs):
try:
scopes, credentials = self.validate_authorization_request(request)
except OAuthToolkitError as error:
# Application is not available at this time.
return self.error_response(error, application=None)

prompt = request.GET.get("prompt")
if prompt == "login":
return self.handle_prompt_login()

all_scopes = get_scopes_backend().get_all_scopes()
kwargs["scopes_descriptions"] = [all_scopes[scope] for scope in scopes]
kwargs["scopes"] = scopes
# at this point we know an Application instance with such client_id exists in the database

# TODO: Cache this!
application = get_application_model().objects.get(client_id=credentials["client_id"])

kwargs["application"] = application
kwargs["client_id"] = credentials["client_id"]
kwargs["redirect_uri"] = credentials["redirect_uri"]
kwargs["response_type"] = credentials["response_type"]
kwargs["state"] = credentials["state"]
if "code_challenge" in credentials:
kwargs["code_challenge"] = credentials["code_challenge"]
if "code_challenge_method" in credentials:
kwargs["code_challenge_method"] = credentials["code_challenge_method"]
if "nonce" in credentials:
kwargs["nonce"] = credentials["nonce"]
if "claims" in credentials:
kwargs["claims"] = json.dumps(credentials["claims"])

self.oauth2_data = kwargs
# following two loc are here only because of https://code.djangoproject.com/ticket/17795
form = self.get_form(self.get_form_class())
kwargs["form"] = form

# Check to see if the user has already granted access and return
# a successful response depending on "approval_prompt" url parameter
require_approval = request.GET.get("approval_prompt", oauth2_settings.REQUEST_APPROVAL_PROMPT)

try:
# If skip_authorization field is True, skip the authorization screen even
# if this is the first use of the application and there was no previous authorization.
# This is useful for in-house applications-> assume an in-house applications
# are already approved.
if application.skip_authorization:
uri, headers, body, status = self.create_authorization_response(
request=self.request, scopes=" ".join(scopes), credentials=credentials, allow=True
)
return self.redirect(uri, application)

elif require_approval == "auto":
tokens = (
get_access_token_model()
.objects.filter(
user=request.user, application=kwargs["application"], expires__gt=timezone.now()
)
.all()
)

# check past authorizations regarded the same scopes as the current one
for token in tokens:
if token.allow_scopes(scopes):
uri, headers, body, status = self.create_authorization_response(
request=self.request,
scopes=" ".join(scopes),
credentials=credentials,
allow=True,
)
return self.redirect(uri, application)

except OAuthToolkitError as error:
return self.error_response(error, application)

return self.render_to_response(self.get_context_data(**kwargs))
context = self.get_context(request, *args, **kwargs)
if isinstance(context, dict):
form = self.get_form(self.get_form_class())
context["form"] = form
return self.render_to_response(self.get_context_data(**context))
else:
return context
Comment on lines +219 to +225
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The dual return type pattern (dict vs HttpResponse) from get_context() creates implicit behavior that's difficult to follow. Consider using a more explicit pattern, such as a tuple (should_render, data) or separate methods for validation vs. context preparation to make the control flow clearer.

Suggested change
context = self.get_context(request, *args, **kwargs)
if isinstance(context, dict):
form = self.get_form(self.get_form_class())
context["form"] = form
return self.render_to_response(self.get_context_data(**context))
else:
return context
should_render, data = self.get_context(request, *args, **kwargs)
if should_render:
form = self.get_form(self.get_form_class())
data["form"] = form
return self.render_to_response(self.get_context_data(**data))
else:
return data

Copilot uses AI. Check for mistakes.

def handle_prompt_login(self):
path = self.request.build_absolute_uri()
Expand Down