fix(API): Nested viewsets expose data across projects and organisations

api/environments/permissions/views.py

@@ -52,7 +52,7 @@ def perform_update(self, serializer):  # type: ignore[no-untyped-def]
 
 class UserPermissionGroupEnvironmentPermissionsViewSet(viewsets.ModelViewSet):  # type: ignore[type-arg]
     pagination_class = None
-    permission_classes = [IsAuthenticated]
+    permission_classes = [IsAuthenticated, NestedEnvironmentPermissions]
 
     def get_queryset(self):  # type: ignore[no-untyped-def]
         if getattr(self, "swagger_fake_view", False):

api/features/feature_external_resources/views.py

@@ -1,5 +1,6 @@
 import re
 
+from common.projects.permissions import VIEW_PROJECT
 from django.shortcuts import get_object_or_404
 from django.utils.decorators import method_decorator
 from drf_spectacular.utils import extend_schema
@@ -41,9 +42,14 @@ def get_queryset(self):  # type: ignore[no-untyped-def]
 
         if "pk" in self.kwargs:
             return FeatureExternalResource.objects.filter(id=self.kwargs["pk"])
-        else:
-            features_pk = self.kwargs["feature_pk"]
-            return FeatureExternalResource.objects.filter(feature=features_pk)
+
+        feature = get_object_or_404(
+            Feature.objects.filter(
+                project__in=self.request.user.get_permitted_projects(VIEW_PROJECT)  # type: ignore[union-attr]
+            ),
+            pk=self.kwargs["feature_pk"],
+        )
+        return feature.external_resources.all()
 
     def list(self, request, *args, **kwargs) -> Response:  # type: ignore[no-untyped-def]
         queryset = self.get_queryset()  # type: ignore[no-untyped-call]

api/features/multivariate/views.py

@@ -76,7 +76,10 @@ def get_queryset(self):  # type: ignore[no-untyped-def]
         if getattr(self, "swagger_fake_view", False):
             return MultivariateFeatureOption.objects.none()
 
-        feature = get_object_or_404(Feature, pk=self.kwargs["feature_pk"])
+        feature = get_object_or_404(
+            Feature.objects.filter(project__id=self.kwargs["project_pk"]),
+            pk=self.kwargs["feature_pk"],
+        )
         return feature.multivariate_options.all()
 
 

api/integrations/github/permissions.py

@@ -1,5 +1,7 @@
 from rest_framework.permissions import BasePermission
 
+from integrations.github.models import GithubConfiguration
+
 
 class HasPermissionToGithubConfiguration(BasePermission):
     """
@@ -10,4 +12,17 @@ class HasPermissionToGithubConfiguration(BasePermission):
     def has_permission(self, request, view):  # type: ignore[no-untyped-def]
         organisation_id = view.kwargs.get("organisation_pk")
 
-        return request.user.belongs_to(organisation_id=int(organisation_id))
+        if not request.user.belongs_to(organisation_id=int(organisation_id)):
+            return False
+
+        # For nested routes, ensure the GitHub configuration in the URL belongs
+        # to the organisation the caller is authorised for
+        if (github_pk := view.kwargs.get("github_pk")) is not None and str(
+            github_pk
+        ).isdigit():
+            return GithubConfiguration.objects.filter(
+                id=github_pk,
+                organisation_id=organisation_id,
+            ).exists()
+
+        return True

api/tests/unit/environments/permissions/test_unit_environments_views.py

@@ -14,7 +14,7 @@
     UserEnvironmentPermission,
     UserPermissionGroupEnvironmentPermission,
 )
-from organisations.models import Organisation
+from organisations.models import Organisation, OrganisationRole
 from tests.types import WithEnvironmentPermissionsCallable
 from users.models import FFAdminUser, UserPermissionGroup
 
@@ -273,3 +273,73 @@ def test_delete_user_group_permission__existing_permission__removes_permission(
     assert not UserPermissionGroupEnvironmentPermission.objects.filter(
         id=user_group_environment_permission.id
     ).exists()
+
+
+def test_list_user_group_permission__different_organisation__returns_403(
+    environment: Environment,
+    organisation: Organisation,
+    organisation_two: Organisation,
+) -> None:
+    # Given
+    other_org_user = FFAdminUser.objects.create(email="other-org-user@example.com")
+    other_org_user.add_organisation(organisation_two, role=OrganisationRole.ADMIN)
+    other_org_client = APIClient()
+    other_org_client.force_authenticate(other_org_user)
+
+    user_permission_group = UserPermissionGroup.objects.create(
+        name="Test group",
+        organisation=organisation,
+    )
+    user_group_environment_permission = (
+        UserPermissionGroupEnvironmentPermission.objects.create(
+            group=user_permission_group,
+            environment=environment,
+        )
+    )
+    read_permission = EnvironmentPermissionModel.objects.get(key=VIEW_ENVIRONMENT)
+    user_group_environment_permission.permissions.set([read_permission])
+
+    url = f"/api/v1/environments/{environment.api_key}/user-group-permissions/"
+
+    # When
+    response = other_org_client.get(url)
+
+    # Then
+    assert response.status_code == status.HTTP_403_FORBIDDEN
+
+
+def test_delete_user_group_permission__different_organisation__returns_403(
+    environment: Environment,
+    organisation: Organisation,
+    organisation_two: Organisation,
+) -> None:
+    # Given
+    other_org_user = FFAdminUser.objects.create(email="other-org-user@example.com")
+    other_org_user.add_organisation(organisation_two, role=OrganisationRole.ADMIN)
+    other_org_client = APIClient()
+    other_org_client.force_authenticate(other_org_user)
+
+    user_permission_group = UserPermissionGroup.objects.create(
+        name="Test group",
+        organisation=organisation,
+    )
+    user_group_environment_permission = (
+        UserPermissionGroupEnvironmentPermission.objects.create(
+            group=user_permission_group,
+            environment=environment,
+        )
+    )
+
+    url = (
+        f"/api/v1/environments/{environment.api_key}"
+        f"/user-group-permissions/{user_group_environment_permission.id}/"
+    )
+
+    # When
+    response = other_org_client.delete(url)
+
+    # Then
+    assert response.status_code == status.HTTP_403_FORBIDDEN
+    assert UserPermissionGroupEnvironmentPermission.objects.filter(
+        id=user_group_environment_permission.id
+    ).exists()

api/tests/unit/features/multivariate/test_unit_multivariate_views.py

@@ -8,8 +8,12 @@
 from django.urls import reverse
 from pytest_lazyfixture import lazy_fixture  # type: ignore[import-untyped]
 from rest_framework import status
+from rest_framework.test import APIClient
 
+from features.models import Feature
+from features.multivariate.models import MultivariateFeatureOption
 from features.multivariate.views import MultivariateFeatureOptionViewSet
+from projects.models import Project
 from projects.permissions import NestedProjectPermissions
 
 
@@ -72,3 +76,32 @@ def test_get_mv_feature_option_by_uuid__nonexistent_uuid__returns_404(  # type:
 
     # Then
     assert response.status_code == status.HTTP_404_NOT_FOUND
+
+
+def test_list_mv_options__feature_in_other_project__returns_404(
+    admin_client: APIClient,
+    project: Project,
+    organisation_two_project_one: Project,
+) -> None:
+    # Given
+    feature = Feature.objects.create(
+        name="feature",
+        project=organisation_two_project_one,
+        type="MULTIVARIATE",
+        initial_value="control",
+    )
+    for percentage_allocation in (30, 30, 40):
+        MultivariateFeatureOption.objects.create(
+            feature=feature,
+            default_percentage_allocation=percentage_allocation,
+            type="unicode",
+            string_value=f"multivariate option for {percentage_allocation}%",
+        )
+
+    url = f"/api/v1/projects/{project.id}/features/{feature.id}/mv-options/"
+
+    # When
+    response = admin_client.get(url)
+
+    # Then
+    assert response.status_code == status.HTTP_404_NOT_FOUND

api/tests/unit/features/test_unit_feature_external_resources_views.py

@@ -934,3 +934,31 @@ def test_create_feature_external_resource__duplicate_feature_and_url__returns_40
         response.json()["non_field_errors"][0]
         == "The fields feature, url must make a unique set."
     )
+
+
+def test_list_feature_external_resources__feature_in_other_project__returns_404(
+    admin_client: APIClient,
+    project: Project,
+    organisation_two_project_one: Project,
+) -> None:
+    # Given
+    feature = Feature.objects.create(
+        name="feature", project=organisation_two_project_one
+    )
+    FeatureExternalResource.objects.create(
+        url="https://gitlab.com/owner/repo/-/issues/1",
+        type="GITLAB_ISSUE",
+        feature=feature,
+        metadata='{"status": "open"}',
+    )
+
+    url = (
+        f"/api/v1/projects/{project.id}"
+        f"/features/{feature.id}/feature-external-resources/"
+    )
+
+    # When
+    response = admin_client.get(url)
+
+    # Then
+    assert response.status_code == status.HTTP_404_NOT_FOUND

api/tests/unit/integrations/github/test_unit_github_views.py

@@ -261,6 +261,29 @@ def test_get_github_repository__github_pk_not_a_number__returns_400(  # type: ig
     assert response.json() == {"github_pk": ["Must be an integer"]}
 
 
+def test_get_github_repository__configuration_in_other_organisation__returns_403(
+    admin_client_new: APIClient,
+    organisation: Organisation,
+    organisation_two: Organisation,
+) -> None:
+    # Given
+    configuration = GithubConfiguration.objects.create(
+        organisation=organisation_two,
+        installation_id=7654321,
+    )
+
+    url = (
+        f"/api/v1/organisations/{organisation.id}"
+        f"/integrations/github/{configuration.id}/repositories/"
+    )
+
+    # When
+    response = admin_client_new.get(url)
+
+    # Then
+    assert response.status_code == status.HTTP_403_FORBIDDEN
+
+
 @responses.activate
 def test_create_github_repository__valid_data__returns_201(
     admin_client_new: APIClient,