-
-
Notifications
You must be signed in to change notification settings - Fork 254
feat(oauth): add branded oath consent page template #7340
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
Changes from all commits
fa7ff6e
9de5a9f
8b5064b
6767392
1c807b6
f4f718d
e458573
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| {% extends "base.html" %} | ||
|
|
||
| {% block title %}{% if error %}Authorization error{% else %}Authorize {{ application.name }}{% endif %} – CourtListener.com{% endblock %} | ||
|
|
||
| {% block privacy %} | ||
| <meta name="robots" content="noindex, noodp, noarchive, noimageindex"/> | ||
| {% endblock %} | ||
|
|
||
| {% block footer-scripts %}{% endblock %} | ||
| {% block sidebar %}{% endblock %} | ||
|
|
||
| {% block content %} | ||
| <div class="col-xs-1 col-sm-2 col-md-3"></div> | ||
| <div class="col-xs-10 col-sm-8 col-md-6 well"> | ||
| {% if not error %} | ||
| <h2>Authorize <em>{{ application.name }}</em>?</h2> | ||
| <p class="gray"> | ||
| Signed in as <strong>{{ request.user.username }}</strong>. | ||
| </p> | ||
|
|
||
| <p class="lead v-offset-above-2"> | ||
| The application <strong>{{ application.name }}</strong> is | ||
| requesting access to your CourtListener account. If you | ||
| approve, it will be able to: | ||
| </p> | ||
|
|
||
| <ul> | ||
| {% for scope in scopes_descriptions %} | ||
| <li>{{ scope }}</li> | ||
| {% endfor %} | ||
| </ul> | ||
|
|
||
| {% if form.redirect_uri.value %} | ||
| <p> | ||
| After you decide, you will be redirected to: | ||
| <br> | ||
| <code>{{ form.redirect_uri.value }}</code> | ||
| </p> | ||
| {% endif %} | ||
|
|
||
| <div class="alert alert-warning v-offset-above-2" role="alert"> | ||
| <i class="fa fa-exclamation-triangle"></i> | ||
| Only authorize applications you trust. They will be able to | ||
| act on your behalf within the permissions listed above until | ||
| you <a href="{% url "view_oauth_authorizations" %}">revoke access</a>. | ||
| </div> | ||
|
|
||
| {% if form.errors %} | ||
| <div class="alert alert-danger"> | ||
| {{ form.errors }} | ||
| {{ form.non_field_errors }} | ||
| </div> | ||
| {% endif %} | ||
|
nadahlberg marked this conversation as resolved.
|
||
|
|
||
| <form method="post" id="authorizationForm">{% csrf_token %} | ||
| {% for field in form %} | ||
| {% if field.is_hidden %}{{ field }}{% endif %} | ||
| {% endfor %} | ||
| <div class="text-right v-offset-above-2"> | ||
| <button type="submit" class="btn btn-default btn-lg">Cancel</button> | ||
| <button type="submit" name="allow" value="Authorize" class="btn btn-primary btn-lg">Allow access</button> | ||
| </div> | ||
| </form> | ||
| {% else %} | ||
| <h2>Something went wrong</h2> | ||
| <p class="lead">We couldn’t complete this authorization request.</p> | ||
| <p> | ||
| <strong>Error:</strong> {{ error.error }} | ||
| {% if error.description %} | ||
| <br><strong>Details:</strong> {{ error.description }} | ||
| {% endif %} | ||
| </p> | ||
| <p> | ||
| Try returning to the application that sent you here and | ||
| starting over. If the problem continues, please | ||
| <a href="{% url "contact" %}">contact us</a>. | ||
| </p> | ||
| {% endif %} | ||
| </div> | ||
| <div class="col-xs-1 col-sm-2 col-md-3"></div> | ||
| {% endblock %} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| {% extends "profile/nav.html" %} | ||
| {% load humanize %} | ||
|
|
||
| {% block title %}{{ page_title }} – CourtListener.com{% endblock %} | ||
|
|
||
| {% block nav-api %}active{% endblock %} | ||
|
|
||
| {% block content %} | ||
| {# Navigation Tabs #} | ||
| {% include "includes/developer-tabs.html" %} | ||
|
|
||
| <div class="hidden-xs col-sm-1 col-md-2"></div> | ||
| <div class="col-xs-12 col-sm-10 col-md-8 text-center"> | ||
| <h1>Authorized Apps</h1> | ||
| <p class="gray"> | ||
| Apps you have granted access to your CourtListener account. | ||
| </p> | ||
|
|
||
| {% if messages %} | ||
| {% for message in messages %} | ||
| <p class="alert alert-success">{{ message|escape }}</p> | ||
| {% endfor %} | ||
| {% endif %} | ||
|
|
||
| {% if authorizations %} | ||
| <div class="table-responsive v-offset-above-2"> | ||
| <table class="table settings-table"> | ||
| <thead> | ||
| <tr class="active"> | ||
| <th class="text-left">Application</th> | ||
| <th class="text-left">Permissions</th> | ||
| <th class="text-left">First granted</th> | ||
| <th class="text-left">Last used</th> | ||
| <th></th> | ||
| </tr> | ||
| </thead> | ||
| <tbody> | ||
| {% for auth in authorizations %} | ||
| <tr> | ||
| <td class="text-left"> | ||
| <strong>{{ auth.application.name }}</strong> | ||
| </td> | ||
| <td class="text-left"> | ||
| <ul class="list-unstyled bottom"> | ||
| {% for scope in auth.scopes %} | ||
| <li>{{ scope }}</li> | ||
| {% endfor %} | ||
| </ul> | ||
| </td> | ||
| <td class="text-left"> | ||
| {{ auth.first_granted|naturaltime }} | ||
| </td> | ||
| <td class="text-left"> | ||
| {{ auth.last_used|naturaltime }} | ||
| </td> | ||
| <td> | ||
| <form method="post" class="bottom"> | ||
| {% csrf_token %} | ||
| <input type="hidden" name="application_id" | ||
| value="{{ auth.application.id }}"> | ||
| <button type="submit" class="btn btn-danger btn-sm"> | ||
| <i class="fa fa-trash"></i> Revoke | ||
| </button> | ||
| </form> | ||
| </td> | ||
| </tr> | ||
| {% endfor %} | ||
| </tbody> | ||
| </table> | ||
| </div> | ||
| {% else %} | ||
| <p class="v-offset-above-2"> | ||
| You haven’t authorized any apps yet. | ||
| </p> | ||
| <p class="gray"> | ||
| When you grant a third-party application access to your account | ||
| through the OAuth flow, it will appear here so you can review | ||
| or revoke its access at any time. | ||
| </p> | ||
| {% endif %} | ||
| </div> | ||
| <div class="hidden-xs col-sm-1 col-md-2"></div> | ||
| {% endblock %} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,10 @@ | |
| sensitive_variables, | ||
| ) | ||
| from django.views.decorators.http import require_http_methods | ||
| from oauth2_provider.models import ( | ||
| get_access_token_model, | ||
| get_refresh_token_model, | ||
| ) | ||
| from rest_framework.renderers import JSONRenderer | ||
| from waffle import switch_is_active | ||
|
|
||
|
|
@@ -786,6 +790,73 @@ def view_webhooks(request: AuthenticatedHttpRequest) -> HttpResponse: | |
| ) | ||
|
|
||
|
|
||
| @login_required | ||
| @never_cache | ||
| @require_http_methods(["GET", "POST"]) | ||
| def view_oauth_authorizations( | ||
| request: AuthenticatedHttpRequest, | ||
| ) -> HttpResponse: | ||
| """List the OAuth applications the user has granted access to.""" | ||
| AccessToken = get_access_token_model() | ||
| RefreshToken = get_refresh_token_model() | ||
|
|
||
| if request.method == "POST": | ||
| app_id = request.POST.get("application_id") | ||
| if app_id: | ||
| AccessToken.objects.filter( | ||
| user=request.user, application_id=app_id | ||
| ).delete() | ||
| RefreshToken.objects.filter( | ||
| user=request.user, application_id=app_id | ||
| ).delete() | ||
| messages.success(request, "Access revoked.") | ||
| return HttpResponseRedirect(reverse("view_oauth_authorizations")) | ||
|
|
||
| scopes_settings = settings.OAUTH2_PROVIDER.get("SCOPES", {}) | ||
| tokens = ( | ||
| AccessToken.objects.filter( | ||
| user=request.user, application__isnull=False | ||
| ) | ||
| .select_related("application") | ||
| .order_by("-updated") | ||
| ) | ||
| by_app: dict[int, dict] = {} | ||
| for tok in tokens: | ||
| entry = by_app.setdefault( | ||
| tok.application_id, | ||
| { | ||
| "application": tok.application, | ||
| "first_granted": tok.created, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this label accurate for the value being displayed? I'm not very familiar with OAuth, but I see we have If so, would it be better to label this as something |
||
| "last_used": tok.updated, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the token |
||
| "scope_names": set(), | ||
| }, | ||
| ) | ||
| entry["first_granted"] = min(entry["first_granted"], tok.created) | ||
| entry["last_used"] = max(entry["last_used"], tok.updated) | ||
| entry["scope_names"].update(tok.scope.split()) | ||
|
|
||
| # One row per Application, aggregated across any access/refresh | ||
| # tokens the user holds for that application. | ||
| authorizations = [] | ||
| for entry in by_app.values(): | ||
| entry["scopes"] = [ | ||
| scopes_settings.get(s, s) for s in sorted(entry["scope_names"]) | ||
| ] | ||
| authorizations.append(entry) | ||
| authorizations.sort(key=lambda e: e["last_used"], reverse=True) | ||
|
|
||
| return TemplateResponse( | ||
| request, | ||
| "profile/oauth_authorizations.html", | ||
| { | ||
| "private": True, | ||
| "page": "api_oauth", | ||
| "page_title": "Authorized Apps", | ||
| "authorizations": authorizations, | ||
| }, | ||
| ) | ||
|
|
||
|
|
||
| @login_required | ||
| @never_cache | ||
| def view_webhook_logs( | ||
|
|
||
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.
🔴 "Revoke access" link on the consent screen points to
{% url "view_api" %}(/profile/api/), which only renders the API Documentation/Token/Usage developer tabs and has no UI for listing or revoking authorized OAuth applications — users following the link to revoke an app will land on an unrelated page. Consider pointing to django-oauth-toolkit's{% url "oauth2_provider:authorized-token-list" %}(already routed under/o/viacl/oauth/urls.py) or a custom revocation page.Extended reasoning...
What the bug is
On the OAuth consent screen, the warning alert tells users:
The
revoke accesslink is wired to{% url "view_api" %}. That named route resolves to/profile/api/(seecl/users/urls.py:119) and is served byview_api, which renderscl/users/templates/profile/api.html. That template only displays the developer-tools tab set (api_info,api_token,api_usage,api_webhooks, viacl/users/templates/includes/developer-tabs.html). There is no UI on any of those tabs for listing the user's authorized OAuth applications or revoking a previously granted authorization.Why existing code doesn't prevent it
Nothing currently links the API profile page to OAuth authorization management — that page predates the OAuth integration and is purely about REST API tokens/usage. The consent template is a new file in this PR, so this is the first place that needed to know about an OAuth revocation URL, and it picked the wrong one.
Step-by-step proof
<a href="{% url "view_api" %}">revoke access</a>. Django reversesview_api→/profile/api/./profile/api/, which (perprofile/api.htmllines 15–20) shows a "Developer Tools" heading, a description of the REST API, and a link to API documentation. The sibling tabs (api_token,api_usage,api_webhooks) similarly have no OAuth-app management.Impact
This is on a security-sensitive page: the consent screen explicitly tells the user the authorization is reversible and points them at the (claimed) revocation control. Sending them to an unrelated developer-tools page undermines the safety message and could leave users unable to find the real revocation flow. Severity is functional/UX, but on a trust-relevant surface.
Suggested fix
django-oauth-toolkit already exposes an authorized-token list view via the
base_urlpatternsthatcl/oauth/urls.py:36includes under/o/. The simplest fix is to point the link at that named URL:Alternatively, swap in a custom revocation page if one is planned. Either way the current target should not ship as-is.