diff --git a/api/api_keys/user.py b/api/api_keys/user.py index 5ce5125acb99..f058bb5e1b79 100644 --- a/api/api_keys/user.py +++ b/api/api_keys/user.py @@ -2,7 +2,7 @@ from django.db.models import QuerySet -from organisations.models import Organisation +from organisations.models import Organisation, OrganisationRole from permissions.permission_service import ( get_permitted_environments_for_master_api_key, get_permitted_projects_for_master_api_key, @@ -35,9 +35,29 @@ def pk(self) -> str: def is_master_api_key_user(self) -> bool: return True + @property + def organisations(self) -> QuerySet[Organisation]: + return Organisation.objects.filter(id=self.key.organisation_id) + def belongs_to(self, organisation_id: int) -> bool: return self.key.organisation_id == organisation_id + def is_organisation_admin( + self, organisation: typing.Union["Organisation", int] + ) -> bool: + org_id = organisation.id if hasattr(organisation, "id") else organisation + return self.key.is_admin and self.key.organisation_id == org_id + + def get_organisation_role(self, organisation: Organisation) -> typing.Optional[str]: + if self.key.organisation_id != organisation.id: + return None + + return ( + OrganisationRole.ADMIN.value + if self.key.is_admin + else OrganisationRole.USER.value + ) + def is_project_admin(self, project: "Project") -> bool: return is_master_api_key_project_admin(self.key, project) diff --git a/api/organisations/permissions/permissions.py b/api/organisations/permissions/permissions.py index c962d8eda77e..7e225c4c4247 100644 --- a/api/organisations/permissions/permissions.py +++ b/api/organisations/permissions/permissions.py @@ -103,7 +103,7 @@ def has_permission(self, request, view): def has_object_permission(self, request, view, obj): return request.user.is_organisation_admin(obj) or ( - view.action == "my_permissions" and obj in request.user.organisations.all() + view.action == "my_permissions" and request.user.belongs_to(obj) ) diff --git a/api/tests/unit/api_keys/test_user.py b/api/tests/unit/api_keys/test_user.py index 9e5626e2bacc..d1cff9b6d646 100644 --- a/api/tests/unit/api_keys/test_user.py +++ b/api/tests/unit/api_keys/test_user.py @@ -1,8 +1,10 @@ import pytest from pytest_lazyfixture import lazy_fixture +from api_keys.models import MasterAPIKey from api_keys.user import APIKeyUser from environments.permissions.models import EnvironmentPermissionModel +from organisations.models import Organisation, OrganisationRole from organisations.permissions.models import OrganisationPermissionModel from projects.models import ProjectPermissionModel @@ -286,3 +288,84 @@ def test_get_permitted_environments( else: assert environments.count() == 1 assert environments.first() == expected_environment + + +def test_is_organisation_admin_for_admin_key( + admin_master_api_key_object: MasterAPIKey, + organisation: Organisation, + organisation_two: Organisation, +) -> None: + # Given + user = APIKeyUser(admin_master_api_key_object) + + # Then + # Return True for the associated organisation + assert user.is_organisation_admin(organisation) is True + assert user.is_organisation_admin(organisation.id) is True + + # Returns False for other organisation + assert user.is_organisation_admin(organisation_two) is False + assert user.is_organisation_admin(organisation_two.id) is False + + +def test_is_organisation_admin_for_non_admin_key( + master_api_key_object: MasterAPIKey, + organisation: Organisation, + organisation_two: Organisation, +) -> None: + # Given + user = APIKeyUser(master_api_key_object) + + # Then + assert user.is_organisation_admin(organisation) is False + assert user.is_organisation_admin(organisation.id) is False + + assert user.is_organisation_admin(organisation_two) is False + assert user.is_organisation_admin(organisation_two.id) is False + + +def test_organisation_property( + master_api_key_object: MasterAPIKey, + organisation: Organisation, +): + # Given + user = APIKeyUser(master_api_key_object) + + # When + organisations = user.organisations + + # Then + assert organisations.count() == 1 + assert organisations.first().id == organisation.id + + +def test_get_organisation_role_for_admin_key( + admin_master_api_key_object: MasterAPIKey, + organisation: Organisation, + organisation_two: Organisation, +) -> None: + # Given + user = APIKeyUser(admin_master_api_key_object) + + # When/Then + # Returns ADMIN for the associated organisation + assert user.get_organisation_role(organisation) == OrganisationRole.ADMIN.value + + # Returns None for other organisation + assert user.get_organisation_role(organisation_two) is None + + +def test_get_organisation_role_for_non_admin_key( + master_api_key_object: MasterAPIKey, + organisation: Organisation, + organisation_two: Organisation, +) -> None: + # Given + user = APIKeyUser(master_api_key_object) + + # When/Then + # Returns USER for the associated organisation + assert user.get_organisation_role(organisation) == OrganisationRole.USER.value + + # Returns None for other organisation + assert user.get_organisation_role(organisation_two) is None diff --git a/api/tests/unit/organisations/test_unit_organisations_views.py b/api/tests/unit/organisations/test_unit_organisations_views.py index 865a2396104f..428ebb04c5d1 100644 --- a/api/tests/unit/organisations/test_unit_organisations_views.py +++ b/api/tests/unit/organisations/test_unit_organisations_views.py @@ -14,6 +14,7 @@ from django.utils import timezone from freezegun import freeze_time from pytest_django.fixtures import SettingsWrapper +from pytest_lazyfixture import lazy_fixture from pytest_mock import MockerFixture from pytz import UTC from rest_framework import status @@ -110,8 +111,12 @@ def test_create_new_orgnisation_returns_403_with_non_superuser( ) +@pytest.mark.parametrize( + "client", + [lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")], +) def test_should_update_organisation_data( - admin_client: APIClient, + client: APIClient, organisation: Organisation, ) -> None: # Given @@ -120,7 +125,7 @@ def test_should_update_organisation_data( data = {"name": new_organisation_name, "restrict_project_create_to_admin": True} # When - response = admin_client.put(url, data=data) + response = client.put(url, data=data) # Then organisation.refresh_from_db() @@ -325,16 +330,20 @@ def test_can_invite_user_as_user( assert Invite.objects.get(email=invited_email).role == OrganisationRole.USER.name +@pytest.mark.parametrize( + "client", + [lazy_fixture("admin_master_api_key_client"), lazy_fixture("staff_client")], +) def test_user_can_get_projects_for_an_organisation( organisation: Organisation, - staff_client: APIClient, + client: APIClient, project: Project, ) -> None: # Given url = reverse("api-v1:organisations:organisation-projects", args=[organisation.pk]) # When - response = staff_client.get(url) + response = client.get(url) # Then assert response.status_code == status.HTTP_200_OK @@ -423,9 +432,13 @@ def test_update_subscription_gets_subscription_data_from_chargebee( assert organisation.subscription.customer_id == customer_id +@pytest.mark.parametrize( + "client", + [lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")], +) def test_delete_organisation( organisation: Organisation, - admin_client: APIClient, + client: APIClient, project: Project, environment: Environment, feature: Feature, @@ -435,7 +448,7 @@ def test_delete_organisation( url = reverse("api-v1:organisations:organisation-detail", args=[organisation.id]) # When - response = admin_client.delete(url) + response = client.delete(url) # Then assert response.status_code == status.HTTP_204_NO_CONTENT