From 77a507d2f8dd4c7c4956d20c59edd3d4d55f5ab9 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sat, 3 Jul 2021 15:59:01 +0200 Subject: [PATCH] providers/oauth2: add revoked field, create suspicious event when previous token is used Signed-off-by: Jens Langhammer --- authentik/events/middleware.py | 11 ++- ...lowstagebinding_invalid_response_action.py | 26 ++++++ authentik/providers/oauth2/api/tokens.py | 1 + .../migrations/0015_auto_20210703_1313.py | 23 ++++++ authentik/providers/oauth2/models.py | 1 + .../providers/oauth2/tests/test_token.py | 81 ++++++++++++++++++- authentik/providers/oauth2/utils.py | 20 +++-- authentik/providers/oauth2/views/token.py | 22 +++-- 8 files changed, 169 insertions(+), 16 deletions(-) create mode 100644 authentik/flows/migrations/0022_alter_flowstagebinding_invalid_response_action.py create mode 100644 authentik/providers/oauth2/migrations/0015_auto_20210703_1313.py diff --git a/authentik/events/middleware.py b/authentik/events/middleware.py index 1543fb8ef..4a8838a0f 100644 --- a/authentik/events/middleware.py +++ b/authentik/events/middleware.py @@ -3,6 +3,7 @@ from functools import partial from typing import Callable from django.conf import settings +from django.core.exceptions import SuspiciousOperation from django.db.models import Model from django.db.models.signals import post_save, pre_delete from django.http import HttpRequest, HttpResponse @@ -63,7 +64,15 @@ class AuditMiddleware: if settings.DEBUG: return - if before_send({}, {"exc_info": (None, exception, None)}) is not None: + # Special case for SuspiciousOperation, we have a special event action for that + if isinstance(exception, SuspiciousOperation): + thread = EventNewThread( + EventAction.SUSPICIOUS_REQUEST, + request, + message=str(exception), + ) + thread.run() + elif before_send({}, {"exc_info": (None, exception, None)}) is not None: thread = EventNewThread( EventAction.SYSTEM_EXCEPTION, request, diff --git a/authentik/flows/migrations/0022_alter_flowstagebinding_invalid_response_action.py b/authentik/flows/migrations/0022_alter_flowstagebinding_invalid_response_action.py new file mode 100644 index 000000000..bb858cc3b --- /dev/null +++ b/authentik/flows/migrations/0022_alter_flowstagebinding_invalid_response_action.py @@ -0,0 +1,26 @@ +# Generated by Django 3.2.4 on 2021-07-03 13:13 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("authentik_flows", "0021_flowstagebinding_invalid_response_action"), + ] + + operations = [ + migrations.AlterField( + model_name="flowstagebinding", + name="invalid_response_action", + field=models.TextField( + choices=[ + ("retry", "Retry"), + ("restart", "Restart"), + ("restart_with_context", "Restart With Context"), + ], + default="retry", + help_text="Configure how the flow executor should handle an invalid response to a challenge. RETRY returns the error message and a similar challenge to the executor. RESTART restarts the flow from the beginning, and RESTART_WITH_CONTEXT restarts the flow while keeping the current context.", + ), + ), + ] diff --git a/authentik/providers/oauth2/api/tokens.py b/authentik/providers/oauth2/api/tokens.py index 8d77ff4ce..05037de6b 100644 --- a/authentik/providers/oauth2/api/tokens.py +++ b/authentik/providers/oauth2/api/tokens.py @@ -51,6 +51,7 @@ class RefreshTokenModelSerializer(ExpiringBaseGrantModelSerializer): "expires", "scope", "id_token", + "revoked", ] depth = 2 diff --git a/authentik/providers/oauth2/migrations/0015_auto_20210703_1313.py b/authentik/providers/oauth2/migrations/0015_auto_20210703_1313.py new file mode 100644 index 000000000..79654b521 --- /dev/null +++ b/authentik/providers/oauth2/migrations/0015_auto_20210703_1313.py @@ -0,0 +1,23 @@ +# Generated by Django 3.2.4 on 2021-07-03 13:13 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("authentik_providers_oauth2", "0014_alter_oauth2provider_rsa_key"), + ] + + operations = [ + migrations.AddField( + model_name="authorizationcode", + name="revoked", + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name="refreshtoken", + name="revoked", + field=models.BooleanField(default=False), + ), + ] diff --git a/authentik/providers/oauth2/models.py b/authentik/providers/oauth2/models.py index 9e710a60b..97b6bc077 100644 --- a/authentik/providers/oauth2/models.py +++ b/authentik/providers/oauth2/models.py @@ -318,6 +318,7 @@ class BaseGrantModel(models.Model): provider = models.ForeignKey(OAuth2Provider, on_delete=models.CASCADE) user = models.ForeignKey(User, verbose_name=_("User"), on_delete=models.CASCADE) _scope = models.TextField(default="", verbose_name=_("Scopes")) + revoked = models.BooleanField(default=False) @property def scope(self) -> list[str]: diff --git a/authentik/providers/oauth2/tests/test_token.py b/authentik/providers/oauth2/tests/test_token.py index fc9c39c67..41a48981e 100644 --- a/authentik/providers/oauth2/tests/test_token.py +++ b/authentik/providers/oauth2/tests/test_token.py @@ -6,6 +6,8 @@ from django.urls import reverse from django.utils.encoding import force_str from authentik.core.models import Application, User +from authentik.crypto.models import CertificateKeyPair +from authentik.events.models import Event, EventAction from authentik.flows.models import Flow from authentik.providers.oauth2.constants import ( GRANT_TYPE_AUTHORIZATION_CODE, @@ -39,7 +41,8 @@ class TestToken(OAuthTestCase): client_id=generate_client_id(), client_secret=generate_client_secret(), authorization_flow=Flow.objects.first(), - redirect_uris="http://local.invalid", + redirect_uris="http://testserver", + rsa_key=CertificateKeyPair.objects.first(), ) header = b64encode( f"{provider.client_id}:{provider.client_secret}".encode() @@ -53,11 +56,13 @@ class TestToken(OAuthTestCase): data={ "grant_type": GRANT_TYPE_AUTHORIZATION_CODE, "code": code.code, - "redirect_uri": "http://local.invalid", + "redirect_uri": "http://testserver", }, HTTP_AUTHORIZATION=f"Basic {header}", ) - params = TokenParams.from_request(request) + params = TokenParams.parse( + request, provider, provider.client_id, provider.client_secret + ) self.assertEqual(params.provider, provider) def test_request_refresh_token(self): @@ -68,6 +73,7 @@ class TestToken(OAuthTestCase): client_secret=generate_client_secret(), authorization_flow=Flow.objects.first(), redirect_uris="http://local.invalid", + rsa_key=CertificateKeyPair.objects.first(), ) header = b64encode( f"{provider.client_id}:{provider.client_secret}".encode() @@ -87,7 +93,9 @@ class TestToken(OAuthTestCase): }, HTTP_AUTHORIZATION=f"Basic {header}", ) - params = TokenParams.from_request(request) + params = TokenParams.parse( + request, provider, provider.client_id, provider.client_secret + ) self.assertEqual(params.provider, provider) def test_auth_code_view(self): @@ -98,6 +106,7 @@ class TestToken(OAuthTestCase): client_secret=generate_client_secret(), authorization_flow=Flow.objects.first(), redirect_uris="http://local.invalid", + rsa_key=CertificateKeyPair.objects.first(), ) # Needs to be assigned to an application for iss to be set self.app.provider = provider @@ -141,6 +150,7 @@ class TestToken(OAuthTestCase): client_secret=generate_client_secret(), authorization_flow=Flow.objects.first(), redirect_uris="http://local.invalid", + rsa_key=CertificateKeyPair.objects.first(), ) # Needs to be assigned to an application for iss to be set self.app.provider = provider @@ -193,6 +203,7 @@ class TestToken(OAuthTestCase): client_secret=generate_client_secret(), authorization_flow=Flow.objects.first(), redirect_uris="http://local.invalid", + rsa_key=CertificateKeyPair.objects.first(), ) header = b64encode( f"{provider.client_id}:{provider.client_secret}".encode() @@ -230,3 +241,65 @@ class TestToken(OAuthTestCase): ), }, ) + + def test_refresh_token_revoke(self): + """test request param""" + provider = OAuth2Provider.objects.create( + name="test", + client_id=generate_client_id(), + client_secret=generate_client_secret(), + authorization_flow=Flow.objects.first(), + redirect_uris="http://testserver", + rsa_key=CertificateKeyPair.objects.first(), + ) + # Needs to be assigned to an application for iss to be set + self.app.provider = provider + self.app.save() + header = b64encode( + f"{provider.client_id}:{provider.client_secret}".encode() + ).decode() + user = User.objects.get(username="akadmin") + token: RefreshToken = RefreshToken.objects.create( + provider=provider, + user=user, + refresh_token=generate_client_id(), + ) + # Create initial refresh token + response = self.client.post( + reverse("authentik_providers_oauth2:token"), + data={ + "grant_type": GRANT_TYPE_REFRESH_TOKEN, + "refresh_token": token.refresh_token, + "redirect_uri": "http://testserver", + }, + HTTP_AUTHORIZATION=f"Basic {header}", + ) + new_token: RefreshToken = ( + RefreshToken.objects.filter(user=user).exclude(pk=token.pk).first() + ) + # Post again with initial token -> get new refresh token + # and revoke old one + response = self.client.post( + reverse("authentik_providers_oauth2:token"), + data={ + "grant_type": GRANT_TYPE_REFRESH_TOKEN, + "refresh_token": new_token.refresh_token, + "redirect_uri": "http://local.invalid", + }, + HTTP_AUTHORIZATION=f"Basic {header}", + ) + self.assertEqual(response.status_code, 200) + # Post again with old token, is now revoked and should error + response = self.client.post( + reverse("authentik_providers_oauth2:token"), + data={ + "grant_type": GRANT_TYPE_REFRESH_TOKEN, + "refresh_token": new_token.refresh_token, + "redirect_uri": "http://local.invalid", + }, + HTTP_AUTHORIZATION=f"Basic {header}", + ) + self.assertEqual(response.status_code, 400) + self.assertTrue( + Event.objects.filter(action=EventAction.SUSPICIOUS_REQUEST).exists() + ) diff --git a/authentik/providers/oauth2/utils.py b/authentik/providers/oauth2/utils.py index a8c65d5f8..c97cf67d6 100644 --- a/authentik/providers/oauth2/utils.py +++ b/authentik/providers/oauth2/utils.py @@ -10,6 +10,7 @@ from django.http.response import HttpResponseRedirect from django.utils.cache import patch_vary_headers from structlog.stdlib import get_logger +from authentik.events.models import Event, EventAction from authentik.providers.oauth2.errors import BearerTokenError from authentik.providers.oauth2.models import RefreshToken @@ -132,22 +133,31 @@ def protected_resource_view(scopes: list[str]): raise BearerTokenError("invalid_token") try: - kwargs["token"] = RefreshToken.objects.get( + token: RefreshToken = RefreshToken.objects.get( access_token=access_token ) except RefreshToken.DoesNotExist: LOGGER.debug("Token does not exist", access_token=access_token) raise BearerTokenError("invalid_token") - if kwargs["token"].is_expired: + if token.is_expired: LOGGER.debug("Token has expired", access_token=access_token) raise BearerTokenError("invalid_token") - if not set(scopes).issubset(set(kwargs["token"].scope)): + if token.revoked: + LOGGER.warning("Revoked token was used", access_token=access_token) + Event.new( + action=EventAction.SUSPICIOUS_REQUEST, + message="Revoked refresh token was used", + token=access_token, + ).from_http(request) + raise BearerTokenError("invalid_token") + + if not set(scopes).issubset(set(token.scope)): LOGGER.warning( "Scope missmatch.", required=set(scopes), - token_has=set(kwargs["token"].scope), + token_has=set(token.scope), ) raise BearerTokenError("insufficient_scope") except BearerTokenError as error: @@ -156,7 +166,7 @@ def protected_resource_view(scopes: list[str]): "WWW-Authenticate" ] = f'error="{error.code}", error_description="{error.description}"' return response - + kwargs["token"] = token return view(request, *args, **kwargs) return view_wrapper diff --git a/authentik/providers/oauth2/views/token.py b/authentik/providers/oauth2/views/token.py index 40bb55c1e..d192cba50 100644 --- a/authentik/providers/oauth2/views/token.py +++ b/authentik/providers/oauth2/views/token.py @@ -8,6 +8,7 @@ from django.http import HttpRequest, HttpResponse from django.views import View from structlog.stdlib import get_logger +from authentik.events.models import Event, EventAction from authentik.lib.utils.time import timedelta_from_string from authentik.providers.oauth2.constants import ( GRANT_TYPE_AUTHORIZATION_CODE, @@ -59,6 +60,7 @@ class TokenParams: client_id: str, client_secret: str, ) -> "TokenParams": + """Parse params for request""" return TokenParams( # Init vars raw_code=request.POST.get("code", ""), @@ -107,7 +109,14 @@ class TokenParams: token=raw_token, ) raise TokenError("invalid_grant") - + if self.refresh_token.revoked: + LOGGER.warning("Refresh token is revoked", token=raw_token) + Event.new( + action=EventAction.SUSPICIOUS_REQUEST, + message="Revoked refresh token was used", + token=raw_token, + ).from_http(request) + raise TokenError("invalid_grant") else: LOGGER.warning("Invalid grant type", grant_type=self.grant_type) raise TokenError("unsupported_grant_type") @@ -178,15 +187,15 @@ class TokenView(View): try: client_id, client_secret = extract_client_auth(request) try: - self.provider: OAuth2Provider = OAuth2Provider.objects.get( - client_id=client_id - ) + self.provider = OAuth2Provider.objects.get(client_id=client_id) except OAuth2Provider.DoesNotExist: LOGGER.warning( "OAuth2Provider does not exist", client_id=self.client_id ) raise TokenError("invalid_client") + if not self.provider: + raise ValueError self.params = TokenParams.parse( request, self.provider, client_id, client_secret ) @@ -265,8 +274,9 @@ class TokenView(View): # Store the refresh_token. refresh_token.save() - # Forget the old token. - self.params.refresh_token.delete() + # Mark old token as revoked + self.params.refresh_token.revoked = True + self.params.refresh_token.save() return { "access_token": refresh_token.access_token,