From 0dac490cf0a6c8c9ca40deb5c85684a7cf4694a1 Mon Sep 17 00:00:00 2001 From: wadii Date: Thu, 29 May 2025 12:49:14 +0200 Subject: [PATCH 1/4] fix: added-invitation-permission-in-is-signup-allowed --- api/custom_auth/permissions.py | 22 +++- .../test_unit_custom_auth_permissions.py | 124 ++++++++++++++++++ 2 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 api/tests/unit/custom_auth/test_unit_custom_auth_permissions.py diff --git a/api/custom_auth/permissions.py b/api/custom_auth/permissions.py index 00b7210d403c..83da2e190095 100644 --- a/api/custom_auth/permissions.py +++ b/api/custom_auth/permissions.py @@ -2,6 +2,8 @@ from django.views import View from rest_framework.permissions import AllowAny, IsAuthenticated from rest_framework.request import Request +from organisations.invites.models import InviteLink, Invite +from rest_framework.exceptions import PermissionDenied class CurrentUser(IsAuthenticated): @@ -17,8 +19,26 @@ def has_object_permission(self, request, view, obj): # type: ignore[no-untyped- class IsSignupAllowed(AllowAny): + message = "Signing-up without a valid invitation is disabled. Please contact your administrator." + def has_permission(self, request: Request, view: View) -> bool: - return not settings.PREVENT_SIGNUP + if not settings.PREVENT_SIGNUP: + return True + + email = request.data.get("email") + if email and Invite.objects.filter(email__iexact=email).exists(): + return True + + invite_hash = request.data.get("invite_hash") + if invite_hash: + try: + invite_link = InviteLink.objects.get(hash=invite_hash) + if not invite_link.is_expired: + return True + except InviteLink.DoesNotExist: + pass + + raise PermissionDenied(self.message) class IsPasswordLoginAllowed(AllowAny): diff --git a/api/tests/unit/custom_auth/test_unit_custom_auth_permissions.py b/api/tests/unit/custom_auth/test_unit_custom_auth_permissions.py new file mode 100644 index 000000000000..c874d8af88b9 --- /dev/null +++ b/api/tests/unit/custom_auth/test_unit_custom_auth_permissions.py @@ -0,0 +1,124 @@ +import pytest +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APIClient, override_settings # type: ignore[attr-defined] +from datetime import timedelta +from django.utils import timezone + +from organisations.invites.models import Invite, InviteLink +from organisations.models import Organisation +from users.models import FFAdminUser + + +@pytest.fixture +def signup_data(): + return { + "email": "test@example.com", + "password": "testpass123", + "first_name": "Test", + "last_name": "User", + } + + +def test_signup_allowed_when_prevent_signup_disabled( + api_client: APIClient, signup_data: dict, db: None +) -> None: + # Given + url = reverse("api-v1:custom_auth:ffadminuser-list") + + # When + response = api_client.post(url, data=signup_data) + + # Then + assert response.status_code == status.HTTP_201_CREATED + + +@override_settings(PREVENT_SIGNUP=True) +def test_signup_blocked_when_prevent_signup_enabled_and_no_invitation( + api_client: APIClient, signup_data: dict, db: None +) -> None: + # Given + url = reverse("api-v1:custom_auth:ffadminuser-list") + + # When + response = api_client.post(url, data=signup_data) + + # Then + assert response.status_code == status.HTTP_403_FORBIDDEN + assert ( + str(response.data["detail"]) + == "Signing-up without a valid invitation is disabled. Please contact your administrator." + ) + + +@override_settings(PREVENT_SIGNUP=True) +def test_signup_allowed_with_email_invite( + api_client: APIClient, signup_data: dict, db: None +) -> None: + # Given + organisation = Organisation.objects.create(name="Test Org") + Invite.objects.create(email=signup_data["email"], organisation=organisation) + url = reverse("api-v1:custom_auth:ffadminuser-list") + + # When + response = api_client.post(url, data=signup_data) + + # Then + assert response.status_code == status.HTTP_201_CREATED + + +@override_settings(PREVENT_SIGNUP=True) +def test_signup_allowed_with_valid_invite_hash( + api_client: APIClient, signup_data: dict, invite_link: InviteLink, db: None +) -> None: + # Given + signup_data_with_hash = {**signup_data, "invite_hash": invite_link.hash} + url = reverse("api-v1:custom_auth:ffadminuser-list") + + # When + response = api_client.post(url, data=signup_data_with_hash) + + # Then + assert response.status_code == status.HTTP_201_CREATED + + +@override_settings(PREVENT_SIGNUP=True) +def test_signup_blocked_with_invalid_invite_hash( + api_client: APIClient, signup_data: dict, db: None +) -> None: + # Given + signup_data_with_hash = {**signup_data, "invite_hash": "invalid-hash"} + url = reverse("api-v1:custom_auth:ffadminuser-list") + + # When + response = api_client.post(url, data=signup_data_with_hash) + + # Then + assert response.status_code == status.HTTP_403_FORBIDDEN + assert ( + str(response.data["detail"]) + == "Signing-up without a valid invitation is disabled. Please contact your administrator." + ) + + +@override_settings(PREVENT_SIGNUP=True) +def test_signup_blocked_with_expired_invite_link( + api_client: APIClient, signup_data: dict, db: None +) -> None: + # Given + organisation = Organisation.objects.create(name="Test Org") + expired_invite_link = InviteLink.objects.create( + organisation=organisation, expires_at=timezone.now() - timedelta(days=1) + ) + signup_data_with_hash = {**signup_data, "invite_hash": expired_invite_link.hash} + url = reverse("api-v1:custom_auth:ffadminuser-list") + + # When + response = api_client.post(url, data=signup_data_with_hash) + + # Then + assert response.status_code == status.HTTP_403_FORBIDDEN + assert ( + str(response.data["detail"]) + == "Signing-up without a valid invitation is disabled. Please contact your administrator." + ) From 7db2b3f75d9a3f54ff1f51aa3ab01c92ed4ba123 Mon Sep 17 00:00:00 2001 From: wadii Date: Thu, 29 May 2025 14:48:24 +0200 Subject: [PATCH 2/4] fix: parametrized-tests-and-show-error-in-frontend --- .../test_unit_custom_auth_permissions.py | 100 ++++++++---------- frontend/web/components/pages/HomePage.tsx | 6 +- 2 files changed, 49 insertions(+), 57 deletions(-) diff --git a/api/tests/unit/custom_auth/test_unit_custom_auth_permissions.py b/api/tests/unit/custom_auth/test_unit_custom_auth_permissions.py index c874d8af88b9..34ed8e005e25 100644 --- a/api/tests/unit/custom_auth/test_unit_custom_auth_permissions.py +++ b/api/tests/unit/custom_auth/test_unit_custom_auth_permissions.py @@ -4,14 +4,13 @@ from rest_framework.test import APIClient, override_settings # type: ignore[attr-defined] from datetime import timedelta from django.utils import timezone - +from typing import Any, Callable from organisations.invites.models import Invite, InviteLink from organisations.models import Organisation -from users.models import FFAdminUser @pytest.fixture -def signup_data(): +def signup_data() -> dict[str, Any]: return { "email": "test@example.com", "password": "testpass123", @@ -21,7 +20,7 @@ def signup_data(): def test_signup_allowed_when_prevent_signup_disabled( - api_client: APIClient, signup_data: dict, db: None + api_client: APIClient, signup_data: dict[str, Any], db: None ) -> None: # Given url = reverse("api-v1:custom_auth:ffadminuser-list") @@ -33,9 +32,9 @@ def test_signup_allowed_when_prevent_signup_disabled( assert response.status_code == status.HTTP_201_CREATED -@override_settings(PREVENT_SIGNUP=True) +@override_settings(PREVENT_SIGNUP=True) # type: ignore[misc] def test_signup_blocked_when_prevent_signup_enabled_and_no_invitation( - api_client: APIClient, signup_data: dict, db: None + api_client: APIClient, signup_data: dict[str, Any], db: None ) -> None: # Given url = reverse("api-v1:custom_auth:ffadminuser-list") @@ -51,9 +50,9 @@ def test_signup_blocked_when_prevent_signup_enabled_and_no_invitation( ) -@override_settings(PREVENT_SIGNUP=True) +@override_settings(PREVENT_SIGNUP=True) # type: ignore[misc] def test_signup_allowed_with_email_invite( - api_client: APIClient, signup_data: dict, db: None + api_client: APIClient, signup_data: dict[str, Any], db: None ) -> None: # Given organisation = Organisation.objects.create(name="Test Org") @@ -67,58 +66,51 @@ def test_signup_allowed_with_email_invite( assert response.status_code == status.HTTP_201_CREATED -@override_settings(PREVENT_SIGNUP=True) -def test_signup_allowed_with_valid_invite_hash( - api_client: APIClient, signup_data: dict, invite_link: InviteLink, db: None +@pytest.mark.parametrize( + "get_invite_hash, expected_status, expected_detail", + [ + ( + lambda organisation: InviteLink.objects.create( + organisation=organisation + ).hash, + status.HTTP_201_CREATED, + None, + ), + ( + lambda _: "invalid-hash", + status.HTTP_403_FORBIDDEN, + "Signing-up without a valid invitation is disabled. Please contact your administrator.", + ), + ( + lambda organisation: InviteLink.objects.create( + organisation=organisation, + expires_at=timezone.now() - timedelta(days=1), + ), + status.HTTP_403_FORBIDDEN, + "Signing-up without a valid invitation is disabled. Please contact your administrator.", + ), + ], +) +@override_settings(PREVENT_SIGNUP=True) # type: ignore[misc] +def test_signup_with_invite_hash_behavior( + api_client: APIClient, + signup_data: dict[str, Any], + db: None, + get_invite_hash: Callable[[Organisation], str], + expected_status: int, + expected_detail: str, + organisation: Organisation, ) -> None: # Given - signup_data_with_hash = {**signup_data, "invite_hash": invite_link.hash} - url = reverse("api-v1:custom_auth:ffadminuser-list") - - # When - response = api_client.post(url, data=signup_data_with_hash) - - # Then - assert response.status_code == status.HTTP_201_CREATED - + invite_hash = get_invite_hash(organisation) -@override_settings(PREVENT_SIGNUP=True) -def test_signup_blocked_with_invalid_invite_hash( - api_client: APIClient, signup_data: dict, db: None -) -> None: - # Given - signup_data_with_hash = {**signup_data, "invite_hash": "invalid-hash"} + signup_data_with_hash = {**signup_data, "invite_hash": invite_hash} url = reverse("api-v1:custom_auth:ffadminuser-list") # When response = api_client.post(url, data=signup_data_with_hash) # Then - assert response.status_code == status.HTTP_403_FORBIDDEN - assert ( - str(response.data["detail"]) - == "Signing-up without a valid invitation is disabled. Please contact your administrator." - ) - - -@override_settings(PREVENT_SIGNUP=True) -def test_signup_blocked_with_expired_invite_link( - api_client: APIClient, signup_data: dict, db: None -) -> None: - # Given - organisation = Organisation.objects.create(name="Test Org") - expired_invite_link = InviteLink.objects.create( - organisation=organisation, expires_at=timezone.now() - timedelta(days=1) - ) - signup_data_with_hash = {**signup_data, "invite_hash": expired_invite_link.hash} - url = reverse("api-v1:custom_auth:ffadminuser-list") - - # When - response = api_client.post(url, data=signup_data_with_hash) - - # Then - assert response.status_code == status.HTTP_403_FORBIDDEN - assert ( - str(response.data["detail"]) - == "Signing-up without a valid invitation is disabled. Please contact your administrator." - ) + assert response.status_code == expected_status + if expected_detail: + assert str(response.data["detail"]) == expected_detail diff --git a/frontend/web/components/pages/HomePage.tsx b/frontend/web/components/pages/HomePage.tsx index dafa9ef619fe..27aa3a7b3b83 100644 --- a/frontend/web/components/pages/HomePage.tsx +++ b/frontend/web/components/pages/HomePage.tsx @@ -267,7 +267,6 @@ const HomePage: React.FC = ({ history, location }) => { ) } } - return ( {( @@ -522,8 +521,9 @@ const HomePage: React.FC = ({ history, location }) => { > From 73e5711ce3045cbd91aced9ed51c51bfdc2419b5 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 29 May 2025 12:57:00 +0000 Subject: [PATCH 3/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- api/custom_auth/permissions.py | 5 +++-- .../test_unit_custom_auth_permissions.py | 13 +++++++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/api/custom_auth/permissions.py b/api/custom_auth/permissions.py index 83da2e190095..d64c326546ee 100644 --- a/api/custom_auth/permissions.py +++ b/api/custom_auth/permissions.py @@ -1,9 +1,10 @@ from django.conf import settings from django.views import View +from rest_framework.exceptions import PermissionDenied from rest_framework.permissions import AllowAny, IsAuthenticated from rest_framework.request import Request -from organisations.invites.models import InviteLink, Invite -from rest_framework.exceptions import PermissionDenied + +from organisations.invites.models import Invite, InviteLink class CurrentUser(IsAuthenticated): diff --git a/api/tests/unit/custom_auth/test_unit_custom_auth_permissions.py b/api/tests/unit/custom_auth/test_unit_custom_auth_permissions.py index 34ed8e005e25..313c5ed3da39 100644 --- a/api/tests/unit/custom_auth/test_unit_custom_auth_permissions.py +++ b/api/tests/unit/custom_auth/test_unit_custom_auth_permissions.py @@ -1,10 +1,15 @@ +from datetime import timedelta +from typing import Any, Callable + import pytest from django.urls import reverse -from rest_framework import status -from rest_framework.test import APIClient, override_settings # type: ignore[attr-defined] -from datetime import timedelta from django.utils import timezone -from typing import Any, Callable +from rest_framework import status +from rest_framework.test import ( # type: ignore[attr-defined] + APIClient, + override_settings, +) + from organisations.invites.models import Invite, InviteLink from organisations.models import Organisation From f5c0b6f6fc2fe0ae5a60a90bc012c60a41a11490 Mon Sep 17 00:00:00 2001 From: wadii Date: Thu, 29 May 2025 16:55:42 +0200 Subject: [PATCH 4/4] fix: misc-improvements --- api/custom_auth/permissions.py | 7 ++++--- .../unit/custom_auth/test_unit_custom_auth_permissions.py | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/api/custom_auth/permissions.py b/api/custom_auth/permissions.py index d64c326546ee..68978cddc315 100644 --- a/api/custom_auth/permissions.py +++ b/api/custom_auth/permissions.py @@ -20,7 +20,7 @@ def has_object_permission(self, request, view, obj): # type: ignore[no-untyped- class IsSignupAllowed(AllowAny): - message = "Signing-up without a valid invitation is disabled. Please contact your administrator." + message = "Signing up without an invitation is disabled. Please contact your administrator." def has_permission(self, request: Request, view: View) -> bool: if not settings.PREVENT_SIGNUP: @@ -34,11 +34,12 @@ def has_permission(self, request: Request, view: View) -> bool: if invite_hash: try: invite_link = InviteLink.objects.get(hash=invite_hash) - if not invite_link.is_expired: - return True except InviteLink.DoesNotExist: pass + if not invite_link.is_expired: + return True + raise PermissionDenied(self.message) diff --git a/api/tests/unit/custom_auth/test_unit_custom_auth_permissions.py b/api/tests/unit/custom_auth/test_unit_custom_auth_permissions.py index 313c5ed3da39..c360c6a6006a 100644 --- a/api/tests/unit/custom_auth/test_unit_custom_auth_permissions.py +++ b/api/tests/unit/custom_auth/test_unit_custom_auth_permissions.py @@ -51,7 +51,7 @@ def test_signup_blocked_when_prevent_signup_enabled_and_no_invitation( assert response.status_code == status.HTTP_403_FORBIDDEN assert ( str(response.data["detail"]) - == "Signing-up without a valid invitation is disabled. Please contact your administrator." + == "Signing up without an invitation is disabled. Please contact your administrator." ) @@ -84,7 +84,7 @@ def test_signup_allowed_with_email_invite( ( lambda _: "invalid-hash", status.HTTP_403_FORBIDDEN, - "Signing-up without a valid invitation is disabled. Please contact your administrator.", + "Signing up without an invitation is disabled. Please contact your administrator.", ), ( lambda organisation: InviteLink.objects.create( @@ -92,7 +92,7 @@ def test_signup_allowed_with_email_invite( expires_at=timezone.now() - timedelta(days=1), ), status.HTTP_403_FORBIDDEN, - "Signing-up without a valid invitation is disabled. Please contact your administrator.", + "Signing up without an invitation is disabled. Please contact your administrator.", ), ], )