From efd5f481e989862c63bd8ac0bee55af8a4ef6516 Mon Sep 17 00:00:00 2001 From: Cosmin Stejerean Date: Sun, 1 Jan 2023 10:46:55 -0800 Subject: [PATCH] OAuth2 Fixes (#338) This implements a few oauth2 fixes: - passes along the state object - enforces authorization code expiration (currently set to 1 minute, we could make this configurable) - enforces redirect_uri - properly checks for client_secret when granting a token - handles pulling client authentication for token grant from basic auth - implement token revocation --- api/middleware.py | 2 +- ...oken_revoked_alter_token_token_and_more.py | 93 +++++++++ api/models/__init__.py | 1 + api/models/authorization.py | 44 ++++ api/models/token.py | 5 +- api/views/oauth.py | 197 ++++++++++++++---- takahe/urls.py | 2 +- templates/api/oauth_authorize.html | 1 + templates/api/oauth_error.html | 10 + 9 files changed, 312 insertions(+), 43 deletions(-) create mode 100644 api/migrations/0002_remove_token_code_token_revoked_alter_token_token_and_more.py create mode 100644 api/models/authorization.py create mode 100644 templates/api/oauth_error.html diff --git a/api/middleware.py b/api/middleware.py index 84eddca..0d55fb3 100644 --- a/api/middleware.py +++ b/api/middleware.py @@ -17,7 +17,7 @@ class ApiTokenMiddleware: if auth_header and auth_header.startswith("Bearer "): token_value = auth_header[7:] try: - token = Token.objects.get(token=token_value) + token = Token.objects.get(token=token_value, revoked=None) except Token.DoesNotExist: return HttpResponse("Invalid Bearer token", status=400) request.user = token.user diff --git a/api/migrations/0002_remove_token_code_token_revoked_alter_token_token_and_more.py b/api/migrations/0002_remove_token_code_token_revoked_alter_token_token_and_more.py new file mode 100644 index 0000000..15d968b --- /dev/null +++ b/api/migrations/0002_remove_token_code_token_revoked_alter_token_token_and_more.py @@ -0,0 +1,93 @@ +# Generated by Django 4.1.4 on 2023-01-01 00:38 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("users", "0008_follow_boosts"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("api", "0001_initial"), + ] + + operations = [ + migrations.RemoveField( + model_name="token", + name="code", + ), + migrations.AddField( + model_name="token", + name="revoked", + field=models.DateTimeField(blank=True, null=True), + ), + migrations.AlterField( + model_name="token", + name="token", + field=models.CharField(max_length=500, unique=True), + ), + migrations.CreateModel( + name="Authorization", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "code", + models.CharField( + blank=True, max_length=128, null=True, unique=True + ), + ), + ("scopes", models.JSONField()), + ("redirect_uri", models.TextField(blank=True, null=True)), + ("valid_for_seconds", models.IntegerField(default=60)), + ("created", models.DateTimeField(auto_now_add=True)), + ("updated", models.DateTimeField(auto_now=True)), + ( + "application", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="authorizations", + to="api.application", + ), + ), + ( + "identity", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="authorizations", + to="users.identity", + ), + ), + ( + "token", + models.OneToOneField( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + to="api.token", + ), + ), + ( + "user", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="authorizations", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + ), + ] diff --git a/api/models/__init__.py b/api/models/__init__.py index 663cd7e..b7b3b78 100644 --- a/api/models/__init__.py +++ b/api/models/__init__.py @@ -1,2 +1,3 @@ from .application import Application # noqa +from .authorization import Authorization # noqa from .token import Token # noqa diff --git a/api/models/authorization.py b/api/models/authorization.py new file mode 100644 index 0000000..728fb54 --- /dev/null +++ b/api/models/authorization.py @@ -0,0 +1,44 @@ +from django.db import models + + +class Authorization(models.Model): + """ + An authorization code as part of the OAuth flow + """ + + application = models.ForeignKey( + "api.Application", + on_delete=models.CASCADE, + related_name="authorizations", + ) + + user = models.ForeignKey( + "users.User", + blank=True, + null=True, + on_delete=models.CASCADE, + related_name="authorizations", + ) + + identity = models.ForeignKey( + "users.Identity", + blank=True, + null=True, + on_delete=models.CASCADE, + related_name="authorizations", + ) + + code = models.CharField(max_length=128, blank=True, null=True, unique=True) + token = models.OneToOneField( + "api.Token", + blank=True, + null=True, + on_delete=models.CASCADE, + ) + + scopes = models.JSONField() + redirect_uri = models.TextField(blank=True, null=True) + valid_for_seconds = models.IntegerField(default=60) + + created = models.DateTimeField(auto_now_add=True) + updated = models.DateTimeField(auto_now=True) diff --git a/api/models/token.py b/api/models/token.py index dc57cec..bec8bb3 100644 --- a/api/models/token.py +++ b/api/models/token.py @@ -30,10 +30,9 @@ class Token(models.Model): related_name="tokens", ) - token = models.CharField(max_length=500) - code = models.CharField(max_length=100, blank=True, null=True) - + token = models.CharField(max_length=500, unique=True) scopes = models.JSONField() created = models.DateTimeField(auto_now_add=True) updated = models.DateTimeField(auto_now=True) + revoked = models.DateTimeField(blank=True, null=True) diff --git a/api/views/oauth.py b/api/views/oauth.py index 56caa3c..e37323b 100644 --- a/api/views/oauth.py +++ b/api/views/oauth.py @@ -1,31 +1,46 @@ +import base64 import secrets from urllib.parse import urlparse, urlunparse from django.contrib.auth.mixins import LoginRequiredMixin -from django.http import HttpResponseRedirect, JsonResponse +from django.http import ( + HttpResponse, + HttpResponseForbidden, + HttpResponseRedirect, + JsonResponse, +) from django.shortcuts import render +from django.utils import timezone from django.utils.decorators import method_decorator from django.views.decorators.csrf import csrf_exempt -from django.views.generic import TemplateView, View +from django.views.generic import View -from api.models import Application, Token +from api.models import Application, Authorization, Token from api.parser import FormOrJsonParser class OauthRedirect(HttpResponseRedirect): - def __init__(self, redirect_uri, key, value): + def __init__(self, redirect_uri, **kwargs): url_parts = urlparse(redirect_uri) self.allowed_schemes = [url_parts.scheme] # Either add or join the query section url_parts = list(url_parts) - if url_parts[4]: - url_parts[4] = url_parts[4] + f"&{key}={value}" - else: - url_parts[4] = f"{key}={value}" + + query_string = url_parts[4] + + for key, value in kwargs.items(): + if value is None: + continue + if not query_string: + query_string = f"{key}={value}" + else: + query_string += f"&{key}={value}" + + url_parts[4] = query_string super().__init__(urlunparse(url_parts)) -class AuthorizationView(LoginRequiredMixin, TemplateView): +class AuthorizationView(LoginRequiredMixin, View): """ Asks the user to authorize access. @@ -33,23 +48,43 @@ class AuthorizationView(LoginRequiredMixin, TemplateView): POST manually. """ - template_name = "api/oauth_authorize.html" - - def get_context_data(self): + def get(self, request): redirect_uri = self.request.GET["redirect_uri"] scope = self.request.GET.get("scope", "read") - try: - application = Application.objects.get( - client_id=self.request.GET["client_id"] + state = self.request.GET.get("state") + + response_type = self.request.GET.get("response_type") + if response_type != "code": + return render( + request, + "api/oauth_error.html", + {"error": f"Invalid response type '{response_type}'"}, ) - except (Application.DoesNotExist, KeyError): - return OauthRedirect(redirect_uri, "error", "invalid_application") - return { + + application = Application.objects.filter( + client_id=self.request.GET.get("client_id"), + ).first() + + if application is None: + return render( + request, "api/oauth_error.html", {"error": "Invalid client_id"} + ) + + if application.redirect_uris and redirect_uri not in application.redirect_uris: + return render( + request, + "api/oauth_error.html", + {"error": "Invalid application redirect URI"}, + ) + + context = { "application": application, + "state": state, "redirect_uri": redirect_uri, "scope": scope, "identities": self.request.user.identities.all(), } + return render(request, "api/oauth_authorize.html", context) def post(self, request): post_data = FormOrJsonParser().parse_body(request) @@ -59,26 +94,59 @@ class AuthorizationView(LoginRequiredMixin, TemplateView): application = Application.objects.get(client_id=post_data["client_id"]) # Get the identity identity = self.request.user.identities.get(pk=post_data["identity"]) + + extra_args = {} + if post_data.get("state"): + extra_args["state"] = post_data["state"] + # Make a token - token = Token.objects.create( + token = Authorization.objects.create( application=application, user=self.request.user, identity=identity, - token=secrets.token_urlsafe(32), - code=secrets.token_urlsafe(16), + code=secrets.token_urlsafe(43), + redirect_uri=redirect_uri, scopes=scope.split(), ) # If it's an out of band request, show the code if redirect_uri == "urn:ietf:wg:oauth:2.0:oob": return render(request, "api/oauth_code.html", {"code": token.code}) # Redirect with the token's code - return OauthRedirect(redirect_uri, "code", token.code) + return OauthRedirect(redirect_uri, code=token.code, **extra_args) + + +def extract_client_info_from_basic_auth(request): + if "authorization" in request.headers: + auth = request.headers["authorization"].split() + if len(auth) == 2: + if auth[0].lower() == "basic": + client_id, client_secret = ( + base64.b64decode(auth[1]).decode("utf8").split(":", 1) + ) + + return client_id, client_secret + return None, None @method_decorator(csrf_exempt, name="dispatch") class TokenView(View): + def verify_code( + self, authorization: Authorization, client_id, client_secret, redirect_uri + ): + application = authorization.application + return ( + application.client_id == client_id + and application.client_secret == client_secret + and authorization.redirect_uri == redirect_uri + ) + def post(self, request): post_data = FormOrJsonParser().parse_body(request) + auth_client_id, auth_client_secret = extract_client_info_from_basic_auth( + request + ) + post_data.setdefault("client_id", auth_client_id) + post_data.setdefault("client_secret", auth_client_secret) grant_type = post_data.get("grant_type") if grant_type not in ( @@ -87,25 +155,57 @@ class TokenView(View): ): return JsonResponse({"error": "invalid_grant_type"}, status=400) - try: - application = Application.objects.get(client_id=post_data["client_id"]) - except (Application.DoesNotExist, KeyError): - return JsonResponse({"error": "invalid_client_id"}, status=400) - # TODO: Implement client credentials flow if grant_type == "client_credentials": - return JsonResponse({"error": "invalid_grant_type"}, status=400) + # TODO: Implement client credentials flow + return JsonResponse( + { + "error": "invalid_grant_type", + "error_description": "client credential flow not implemented", + }, + status=400, + ) elif grant_type == "authorization_code": code = post_data.get("code") if not code: - return JsonResponse({"error": "invalid_code"}, status=400) - # Retrieve the token by code - # TODO: Check code expiry based on created date - try: - token = Token.objects.get(code=code, application=application) - except Token.DoesNotExist: - return JsonResponse({"error": "invalid_code"}, status=400) - # Update the token to remove its code - token.code = None + return JsonResponse( + { + "error": "invalid_request", + "error_description": "Required param : code", + }, + status=400, + ) + + authorization = Authorization.objects.get(code=code) + if ( + not authorization + or timezone.now() - authorization.created + > timezone.timedelta(seconds=authorization.valid_for_seconds) + ): + return JsonResponse({"error": "access_denied"}, status=401) + + application = Application.objects.filter( + client_id=post_data["client_id"], + client_secret=post_data["client_secret"], + ).first() + + code_verified = self.verify_code( + authorization, + client_id=post_data.get("client_id"), + client_secret=post_data.get("client_secret"), + redirect_uri=post_data.get("redirect_uri"), + ) + + if not application or authorization.token or not code_verified: + # this authorization code has already been used + return JsonResponse({"error": "access_denied"}, status=401) + + token = Token.objects.create( + application=application, + user=authorization.user, + identity=authorization.identity, + token=secrets.token_urlsafe(43), + scopes=authorization.scopes, + ) token.save() # Return them the token return JsonResponse( @@ -118,5 +218,26 @@ class TokenView(View): ) +@method_decorator(csrf_exempt, name="dispatch") class RevokeTokenView(View): - pass + def post(self, request): + post_data = FormOrJsonParser().parse_body(request) + auth_client_id, auth_client_secret = extract_client_info_from_basic_auth( + request + ) + post_data.setdefault("client_id", auth_client_id) + post_data.setdefault("client_secret", auth_client_secret) + token_str = post_data["token"] + + application = Application.objects.filter( + client_id=post_data["client_id"], + client_secret=post_data["client_secret"], + ).first() + + token = Token.objects.filter(application=application, token=token_str).first() + if token is None: + return HttpResponseForbidden() + + token.revoked = timezone.now() + token.save() + return HttpResponse("") diff --git a/takahe/urls.py b/takahe/urls.py index 6603de7..76dd98f 100644 --- a/takahe/urls.py +++ b/takahe/urls.py @@ -244,7 +244,7 @@ urlpatterns = [ path("api/", api_router.urls), path("oauth/authorize", oauth.AuthorizationView.as_view()), path("oauth/token", oauth.TokenView.as_view()), - path("oauth/revoke_token", oauth.RevokeTokenView.as_view()), + path("oauth/revoke", oauth.RevokeTokenView.as_view()), # Stator path(".stator/", stator.RequestRunner.as_view()), # Django admin diff --git a/templates/api/oauth_authorize.html b/templates/api/oauth_authorize.html index 472083f..c06b828 100644 --- a/templates/api/oauth_authorize.html +++ b/templates/api/oauth_authorize.html @@ -32,6 +32,7 @@ {% if "push" in scope %}
  • Receive push notifications
    • {% endif %} + diff --git a/templates/api/oauth_error.html b/templates/api/oauth_error.html new file mode 100644 index 0000000..f7adc80 --- /dev/null +++ b/templates/api/oauth_error.html @@ -0,0 +1,10 @@ +{% extends "base_plain.html" %} + +{% block title %}Invalid OAuth Request{% endblock %} + +{% block content %} +

    Invalid OAuth Request

    +
    +

    Error: {{ error }}

    +
    +{% endblock %}