diff --git a/api/permissions/permission_service.py b/api/permissions/permission_service.py index 07b25b4124c4..6a0a28e263b3 100644 --- a/api/permissions/permission_service.py +++ b/api/permissions/permission_service.py @@ -1,3 +1,4 @@ +import typing from typing import TYPE_CHECKING, List, Set, Union from django.conf import settings @@ -202,23 +203,33 @@ def user_has_organisation_permission( if is_user_organisation_admin(user, organisation): return True - # NOTE: since we store organisation admin slightly differently - # compared to project and environment `get_base_permission_filter` - # with allow_admin=True will not work for organisation - base_filter = get_base_permission_filter( - user, - Organisation, # type: ignore[arg-type] - permission_key, - allow_admin=False, - ) - filter_ = base_filter & Q(id=organisation.id) + # Base query to ensure the user actually belongs to the organisation + base_qs = Organisation.objects.filter(id=organisation.id, users=user) - queryset = Organisation.objects.filter(filter_) + # 1. Check direct user permissions (Fastest) + user_filter = get_user_permission_filter(user, permission_key, allow_admin=False) + if base_qs.filter(user_filter).exists(): + return True - # Final check to verify that user belongs to organisation - queryset = queryset.filter(users=user) + # 2. Check group permissions + group_filter = get_group_permission_filter(user, permission_key, allow_admin=False) + if base_qs.filter(group_filter).exists(): + return True - return queryset.exists() # type: ignore[no-any-return] + # 3. Check role permissions (only if RBAC is installed) + if settings.IS_RBAC_INSTALLED: # pragma: no cover + role_filter = get_role_permission_filter( + user, + # Type gap: get_role_permission_filter type hint expects an instance, + # but safely handles the model class at runtime. + typing.cast(typing.Any, Organisation), + permission_key, + allow_admin=False, + ) + if base_qs.filter(role_filter).exists(): + return True + + return False def master_api_key_has_organisation_permission( diff --git a/api/tests/unit/permissions/permission_service/test_user_has_organisation_permissions.py b/api/tests/unit/permissions/permission_service/test_user_has_organisation_permissions.py index 1798892cc473..84f883e2660c 100644 --- a/api/tests/unit/permissions/permission_service/test_user_has_organisation_permissions.py +++ b/api/tests/unit/permissions/permission_service/test_user_has_organisation_permissions.py @@ -1,3 +1,5 @@ +import typing + from organisations.models import Organisation, UserOrganisation from organisations.permissions.models import ( OrganisationPermissionModel, @@ -156,3 +158,34 @@ def test_user_has_organisation_permission__user_removed_from_organisation__retur organisation=organisation, permission_key=CREATE_PROJECT, ) + + +def test_user_has_organisation_permission__evaluating_permission__executes_exact_queries( + django_assert_num_queries: typing.Any, + django_user_model: typing.Any, + organisation: typing.Any, +) -> None: + # Given + user = django_user_model.objects.create(email="test_sequential_eval@example.com") + user.add_organisation(organisation) + + # When + with django_assert_num_queries(3) as ctx: + has_permission = user_has_organisation_permission( + user=user, organisation=organisation, permission_key="MANAGE_USER_GROUPS" + ) + + # Then + assert has_permission is False + + # Verify the exact queries executed match the expected sequential EXISTS pattern + queries = [query["sql"].lower() for query in ctx.captured_queries] + + # Query 1: Base user organisation role check + assert "organisations_userorganisation" in queries[0] + + # Query 2: User-specific permission check + assert "organisation_permissions_userorganisationpermission" in queries[1] + + # Query 3: Group-specific permission check + assert "organisation_permissions_userpermissiongroup" in queries[2] diff --git a/api/tests/unit/users/test_unit_users_models.py b/api/tests/unit/users/test_unit_users_models.py index 475a28af9647..e95e622d62dc 100644 --- a/api/tests/unit/users/test_unit_users_models.py +++ b/api/tests/unit/users/test_unit_users_models.py @@ -9,7 +9,10 @@ from organisations.permissions.permissions import ORGANISATION_PERMISSIONS from projects.models import Project from tests.types import WithProjectPermissionsCallable -from users.models import FFAdminUser, UserPermissionGroup +from users.models import ( + FFAdminUser, + UserPermissionGroup, +) def test_belongs_to__user_in_organisation__returns_true(