From 5f7f1902cbaaae64c6070b1fa09635f9a6710dd1 Mon Sep 17 00:00:00 2001 From: Sebastiaan ten Pas <st981@cam.ac.uk> Date: Wed, 12 Feb 2025 07:55:20 +0000 Subject: [PATCH] feat!: add internal API endpoint for deleting lockouts - changing DATA_MANAGER_ENABLED to INTERNAL_API_ENABLED --- activate_account_project/settings/base.py | 4 +- activate_account_project/urls.py | 4 +- authentication/serializers.py | 6 ++ authentication/tests/test_lockout.py | 72 +++++++++++++++++++++-- authentication/urls.py | 15 +++-- authentication/views.py | 25 +++++++- docker-compose.yml | 2 +- pyproject.toml | 2 +- 8 files changed, 114 insertions(+), 16 deletions(-) diff --git a/activate_account_project/settings/base.py b/activate_account_project/settings/base.py index f849310..87c8ef9 100644 --- a/activate_account_project/settings/base.py +++ b/activate_account_project/settings/base.py @@ -17,7 +17,7 @@ DATABASES = { }, } -DATA_MANAGER_ENABLED = False +INTERNAL_API_ENABLED = False DATA_MANAGER_READ_ONLY = True FAKE_RESET_TOKEN_IF_MISSING = False @@ -43,7 +43,7 @@ externalsettings.load_external_settings( "EMAIL_HOST_PASSWORD", "EMAIL_HOST_USER", "EMAIL_PORT", - "DATA_MANAGER_ENABLED", + "INTERNAL_API_ENABLED", "DATA_MANAGER_READ_ONLY", "FAKE_RESET_TOKEN_IF_MISSING", ], diff --git a/activate_account_project/urls.py b/activate_account_project/urls.py index 1cbdda3..7a996cc 100644 --- a/activate_account_project/urls.py +++ b/activate_account_project/urls.py @@ -33,7 +33,7 @@ urlpatterns = [ lambda request: HttpResponse("ok", content_type="text/plain"), name="healthy", ), - path("token/", include("authentication.urls")), + path("", include("authentication.urls")), # You'll also need to update api/tests/test_versions_view.py # Include the base API urls - which gives a view of the available API versions path("", include("api.urls")), @@ -42,7 +42,7 @@ urlpatterns = [ path("v1alpha1/", include(("api.v1alpha.urls", "v1alpha1"), namespace="v1alpha1")), ] -if settings.DATA_MANAGER_ENABLED: +if settings.INTERNAL_API_ENABLED: urlpatterns += [ path( "data-manager-api/", diff --git a/authentication/serializers.py b/authentication/serializers.py index 8e33884..b24e979 100644 --- a/authentication/serializers.py +++ b/authentication/serializers.py @@ -171,6 +171,12 @@ class TokenErrorSerializer(serializers.Serializer): error_uri = serializers.URLField(required=False) +class LockoutSerializer(serializers.ModelSerializer): + class Meta: + model = Lockout + fields = ("id", "identity_key", "date_of_birth", "attempts", "lockout_until") + + class EmptySerializer(serializers.Serializer): """ Defines an empty response. diff --git a/authentication/tests/test_lockout.py b/authentication/tests/test_lockout.py index 1efd35d..ad9eb59 100644 --- a/authentication/tests/test_lockout.py +++ b/authentication/tests/test_lockout.py @@ -1,6 +1,8 @@ from datetime import timedelta +from urllib.parse import urlencode import pytest +from django.urls import reverse from django.utils import timezone from freezegun import freeze_time @@ -12,7 +14,9 @@ from authentication.constants import ( SESSION_GRANT_TYPE, ) from authentication.errors import InvalidGrantError -from authentication.serializers import TokenRequestSerializer +from authentication.serializers import LockoutSerializer, TokenRequestSerializer + +pytestmark = pytest.mark.django_db def _build_data(identity_field, identity_value, identifier): @@ -26,7 +30,6 @@ def _build_data(identity_field, identity_value, identifier): @pytest.mark.parametrize("identity_field", ["crsid", "last_name"]) -@pytest.mark.django_db def test_lockout_existing_active(identity_field): account = AccountFactory(account_identifier=True) identifier = account.account_identifier.first() @@ -47,7 +50,6 @@ def test_lockout_existing_active(identity_field): @pytest.mark.parametrize("identity_field", ["crsid", "last_name"]) -@pytest.mark.django_db def test_lockout_created_on_invalid_credentials(identity_field): account = AccountFactory(account_identifier=True) identifier = account.account_identifier.first() @@ -69,7 +71,6 @@ def test_lockout_created_on_invalid_credentials(identity_field): @pytest.mark.parametrize("identity_field", ["crsid", "last_name"]) -@pytest.mark.django_db def test_successful_login_clears_lockout(identity_field): account = AccountFactory(account_identifier=True) identifier = account.account_identifier.first() @@ -96,7 +97,6 @@ def test_successful_login_clears_lockout(identity_field): @pytest.mark.parametrize("identity_field", ["crsid", "last_name"]) -@pytest.mark.django_db def test_exponential_backoff_lockout(identity_field): with freeze_time("2025-01-01 00:00:00") as frozen_datetime: account = AccountFactory(account_identifier=True) @@ -131,3 +131,65 @@ def test_exponential_backoff_lockout(identity_field): expected_duration = LOCKOUT_INITIAL_SECONDS * (2 ** (lockout.attempts - LOCKOUT_ATTEMPTS)) expected_lockout_time = timezone.now() + timedelta(seconds=expected_duration) assert (lockout.lockout_until - expected_lockout_time).total_seconds() == 0 + + +@pytest.mark.parametrize("identity_field", ["crsid", "last_name"]) +@freeze_time("2025-01-01 00:00:00") +def test_lockout_clear_from_api_endpoint_flow(identity_field, api_client): + account = AccountFactory(account_identifier=True) + identifier = account.account_identifier.first() + identity_value = account.crsid if identity_field == "crsid" else identifier.last_name + + data = _build_data(identity_field, identity_value, identifier) + wrong_data = { + **data, + "code": "wrong-code", + } + + # Fail LOCK_ATTEMPTS times + for _ in range(LOCKOUT_ATTEMPTS): + response = api_client.post( + reverse("knox_login"), + data=urlencode(wrong_data), + content_type="application/x-www-form-urlencoded", + ) + + assert response.status_code == 400 + + lockout_kwargs = { + "identity_key": identity_value, + "date_of_birth": identifier.date_of_birth, + } + + # Confirm account is locked out + assert Lockout.objects.filter(**lockout_kwargs).exists() + + response = api_client.post( + reverse("knox_login"), + data=urlencode(data), + content_type="application/x-www-form-urlencoded", + ) + + assert response.status_code == 400 + + lockout = Lockout.objects.get(**lockout_kwargs) + + response = api_client.get(reverse("lockout-list")) + assert response.status_code == 200 + assert response.data == LockoutSerializer([lockout], many=True).data + + # Now remove the lockout + response = api_client.delete(reverse("lockout-detail", args=[lockout.id])) + assert response.status_code == 204 + + # And confirm it's gone + assert not Lockout.objects.filter(**lockout_kwargs).exists() + + # And that we can login again + response = api_client.post( + reverse("knox_login"), + data=urlencode(data), + content_type="application/x-www-form-urlencoded", + ) + + assert response.status_code == 200 diff --git a/authentication/urls.py b/authentication/urls.py index 47b320c..3905ffc 100644 --- a/authentication/urls.py +++ b/authentication/urls.py @@ -1,9 +1,16 @@ +from django.conf import settings from django.urls import path +from rest_framework import routers -from .views import LoginView, LogoutAllView, LogoutView +from .views import LockoutViewSet, LoginView, LogoutAllView, LogoutView urlpatterns = [ - path("", LoginView.as_view(), name="knox_login"), - path("revoke/", LogoutView.as_view(), name="knox_logout"), - path("revoke/all/", LogoutAllView.as_view(), name="knox_logoutall"), + path("token/", LoginView.as_view(), name="knox_login"), + path("token/revoke/", LogoutView.as_view(), name="knox_logout"), + path("token/revoke/all/", LogoutAllView.as_view(), name="knox_logoutall"), ] + +if settings.INTERNAL_API_ENABLED: + router = routers.SimpleRouter() + router.register("lockout", LockoutViewSet, basename="lockout") + urlpatterns += router.urls diff --git a/authentication/views.py b/authentication/views.py index 6cc548b..61cc1aa 100644 --- a/authentication/views.py +++ b/authentication/views.py @@ -1,11 +1,21 @@ from drf_spectacular.utils import OpenApiResponse, extend_schema from knox.views import LoginView as KnoxLoginView -from rest_framework import parsers, renderers, serializers, status, views +from rest_framework import ( + mixins, + parsers, + renderers, + serializers, + status, + views, + viewsets, +) from rest_framework.response import Response +from activate_account.models import Lockout from authentication.errors import OAuth2Error from authentication.serializers import ( EmptySerializer, + LockoutSerializer, TokenErrorSerializer, TokenRequestSerializer, TokenResponseSerializer, @@ -154,3 +164,16 @@ class LogoutAllView(views.APIView): def post(self, request, format=None): request.user.auth_token_set.all().delete() return self.get_post_response(request) + + +class LockoutViewSet( + mixins.RetrieveModelMixin, + mixins.DestroyModelMixin, + mixins.ListModelMixin, + viewsets.GenericViewSet, +): + permission_classes = () + authentication_classes = () + queryset = Lockout.objects.all() + serializer_class = LockoutSerializer + versioning_class = None diff --git a/docker-compose.yml b/docker-compose.yml index 954a6a4..3540665 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -11,7 +11,7 @@ x-django-application-environment: &django-application-environment # disabled and one instance that is internal and has these endpoints enabled. In local # development we simplify the setup and just run one instance with all endpoints, internal and # external. - EXTERNAL_SETTING_DATA_MANAGER_ENABLED: "1" + EXTERNAL_SETTING_INTERNAL_API_ENABLED: "1" EXTERNAL_SETTING_DATA_MANAGER_READ_ONLY: "0" # Connect to mock password app reset token request endpoint EXTERNAL_SETTING_PASSWORD_APP_RESET_TOKEN_URL: "http://mock-password-app:8010/acc-reset-token" diff --git a/pyproject.toml b/pyproject.toml index 005bccd..e9a31cb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -147,7 +147,7 @@ env = [ "DJANGO_SETTINGS_MODULE=activate_account_project.settings.testing", "D:R:EXTERNAL_SETTING_DATABASES={\"default\":{}}", "D:EXTERNAL_SETTING_SECRET_KEY=fake-secret-key", - "D:EXTERNAL_SETTING_DATA_MANAGER_ENABLED=1", + "D:EXTERNAL_SETTING_INTERNAL_API_ENABLED=1", "D:EXTERNAL_SETTING_DATA_MANAGER_READ_ONLY=0", "D:EXTERNAL_SETTING_PASSWORD_APP_RESET_TOKEN_URL=http://placeholder/", "D:EXTERNAL_SETTING_PASSWORD_APP_TOKEN=fake", -- GitLab