From ed3260ef33cb9ed4822269907874af7759dec485 Mon Sep 17 00:00:00 2001 From: Deep Santoshwar Date: Mon, 31 Aug 2026 00:52:45 +0530 Subject: [PATCH] fix(auth): allow invite-based signup when PREVENT_SIGNUP is enabled IsSignupAllowed blocked every registration once PREVENT_SIGNUP was on, including ones carrying a valid invite link or invited email, so the serializer's own invite validation never even ran. Self-hosted admins who enable PREVENT_SIGNUP to stop self-serve signup could no longer onboard teammates via invite. The invite check is now shared between the permission gate and the serializer instead of duplicated. --- api/custom_auth/permissions.py | 14 ++- api/custom_auth/serializers.py | 20 ++-- api/organisations/invites/services.py | 15 +++ .../test_unit_custom_auth_permissions.py | 107 ++++++++++++++++++ 4 files changed, 142 insertions(+), 14 deletions(-) create mode 100644 api/organisations/invites/services.py 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..8dd0435c7b87 100644 --- a/api/custom_auth/permissions.py +++ b/api/custom_auth/permissions.py @@ -3,6 +3,8 @@ from rest_framework.permissions import AllowAny, IsAuthenticated from rest_framework.request import Request +from organisations.invites.services import is_valid_registration_invite + class CurrentUser(IsAuthenticated): """ @@ -18,7 +20,17 @@ def has_object_permission(self, request, view, obj): # type: ignore[no-untyped- class IsSignupAllowed(AllowAny): def has_permission(self, request: Request, view: View) -> bool: - return not settings.PREVENT_SIGNUP + if not settings.PREVENT_SIGNUP: + return True + + # Signups are otherwise prevented, but a valid invite should still + # let someone through: `PREVENT_SIGNUP` is meant to stop self-serve + # signup, not registration via an invite link or invited email. + return is_valid_registration_invite( + sign_up_type=request.data.get("sign_up_type"), + email=request.data.get("email") or "", + invite_hash=request.data.get("invite_hash"), + ) class IsPasswordLoginAllowed(AllowAny): diff --git a/api/custom_auth/serializers.py b/api/custom_auth/serializers.py index f1128bd980c2..aa914979a23e 100644 --- a/api/custom_auth/serializers.py +++ b/api/custom_auth/serializers.py @@ -7,10 +7,10 @@ from rest_framework.authtoken.models import Token from rest_framework.exceptions import PermissionDenied -from organisations.invites.models import Invite, InviteLink +from organisations.invites.services import is_valid_registration_invite from users.auth_type import AuthType from users.constants import DEFAULT_DELETE_ORPHAN_ORGANISATIONS_VALUE -from users.models import FFAdminUser, SignUpType +from users.models import FFAdminUser from .constants import ( FIELD_BLANK_ERROR, @@ -32,17 +32,11 @@ def _validate_registration_invite(self, email: str, sign_up_type: str) -> None: if settings.ALLOW_REGISTRATION_WITHOUT_INVITE: return - valid = False - - match sign_up_type: - case SignUpType.INVITE_LINK.value: - valid = InviteLink.objects.filter( - hash=self.initial_data.get("invite_hash") # type: ignore[attr-defined] - ).exists() - case SignUpType.INVITE_EMAIL.value: - valid = Invite.objects.filter(email__iexact=email.lower()).exists() - - if not valid: + if not is_valid_registration_invite( + sign_up_type=sign_up_type, + email=email, + invite_hash=self.initial_data.get("invite_hash"), # type: ignore[attr-defined] + ): raise PermissionDenied(USER_REGISTRATION_WITHOUT_INVITE_ERROR_MESSAGE) diff --git a/api/organisations/invites/services.py b/api/organisations/invites/services.py new file mode 100644 index 000000000000..a5e0eeacda80 --- /dev/null +++ b/api/organisations/invites/services.py @@ -0,0 +1,15 @@ +from users.models import SignUpType + +from .models import Invite, InviteLink + + +def is_valid_registration_invite( + *, sign_up_type: str | None, email: str, invite_hash: str | None +) -> bool: + match sign_up_type: + case SignUpType.INVITE_LINK.value: + return InviteLink.objects.filter(hash=invite_hash).exists() + case SignUpType.INVITE_EMAIL.value: + return Invite.objects.filter(email__iexact=email.lower()).exists() + case _: + return False 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..4878c14bf65c --- /dev/null +++ b/api/tests/unit/custom_auth/test_unit_custom_auth_permissions.py @@ -0,0 +1,107 @@ +from unittest import mock + +from pytest_django.fixtures import SettingsWrapper + +from custom_auth.permissions import IsSignupAllowed +from organisations.invites.models import Invite, InviteLink +from organisations.models import Organisation +from users.models import SignUpType + + +def test_is_signup_allowed__prevent_signup_disabled__returns_true( + settings: SettingsWrapper, +) -> None: + # Given + settings.PREVENT_SIGNUP = False + permission = IsSignupAllowed() + mock_request = mock.MagicMock(data={}) + + # When + result = permission.has_permission(mock_request, mock.MagicMock()) + + # Then + assert result is True + + +def test_is_signup_allowed__prevent_signup_enabled_no_invite__returns_false( + settings: SettingsWrapper, +) -> None: + # Given + settings.PREVENT_SIGNUP = True + permission = IsSignupAllowed() + mock_request = mock.MagicMock(data={"email": "test@example.com"}) + + # When + result = permission.has_permission(mock_request, mock.MagicMock()) + + # Then + assert result is False + + +def test_is_signup_allowed__prevent_signup_enabled_valid_invite_link__returns_true( + db: None, + settings: SettingsWrapper, + organisation: Organisation, +) -> None: + # Given + settings.PREVENT_SIGNUP = True + invite_link = InviteLink.objects.create(organisation=organisation) + permission = IsSignupAllowed() + mock_request = mock.MagicMock( + data={ + "email": "test@example.com", + "sign_up_type": SignUpType.INVITE_LINK.value, + "invite_hash": invite_link.hash, + } + ) + + # When + result = permission.has_permission(mock_request, mock.MagicMock()) + + # Then + assert result is True + + +def test_is_signup_allowed__prevent_signup_enabled_invalid_invite_hash__returns_false( + db: None, + settings: SettingsWrapper, +) -> None: + # Given + settings.PREVENT_SIGNUP = True + permission = IsSignupAllowed() + mock_request = mock.MagicMock( + data={ + "email": "test@example.com", + "sign_up_type": SignUpType.INVITE_LINK.value, + "invite_hash": "invalid-hash", + } + ) + + # When + result = permission.has_permission(mock_request, mock.MagicMock()) + + # Then + assert result is False + + +def test_is_signup_allowed__prevent_signup_enabled_valid_invite_email__returns_true( + db: None, + settings: SettingsWrapper, + organisation: Organisation, +) -> None: + # Given + settings.PREVENT_SIGNUP = True + Invite.objects.create(email="test@example.com", organisation=organisation) + permission = IsSignupAllowed() + mock_request = mock.MagicMock( + data={ + "email": "Test@Example.com", + "sign_up_type": SignUpType.INVITE_EMAIL.value, + } + ) + + # When + result = permission.has_permission(mock_request, mock.MagicMock()) + + # Then + assert result is True