Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: master api key org api access #3817

Merged
merged 1 commit into from
Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion api/api_keys/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion api/organisations/permissions/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,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)
)


Expand Down
83 changes: 83 additions & 0 deletions api/tests/unit/api_keys/test_user.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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
25 changes: 19 additions & 6 deletions api/tests/unit/organisations/test_unit_organisations_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down