diff --git a/api/metadata/permissions.py b/api/metadata/permissions.py index 65cd54a57881..3e76dec01a9a 100644 --- a/api/metadata/permissions.py +++ b/api/metadata/permissions.py @@ -74,9 +74,10 @@ def has_permission(self, request, view): # type: ignore[no-untyped-def] if view.action == "create": field = MetadataField.objects.get(id=request.data.get("field")) - return ( + return (organisation_pk == field.organisation.id) and ( request.user.is_organisation_admin(organisation_pk) - and organisation_pk == field.organisation.id + or (project_id := field.project) + and request.user.is_project_admin(project_id) ) return False @@ -86,6 +87,10 @@ def has_object_permission(self, request, view, obj): # type: ignore[no-untyped- return request.user.belongs_to(obj.field.organisation.id) if view.action in ("update", "destroy", "partial_update"): - return request.user.is_organisation_admin(obj.field.organisation) + return ( + request.user.is_organisation_admin(obj.field.organisation) + or (project_id := obj.field.project) + and request.user.is_project_admin(project_id) + ) return False diff --git a/api/tests/unit/metadata/test_views.py b/api/tests/unit/metadata/test_views.py index 308ec67836b6..cb5a2ab5bbbf 100644 --- a/api/tests/unit/metadata/test_views.py +++ b/api/tests/unit/metadata/test_views.py @@ -14,7 +14,7 @@ ) from metadata.views import SUPPORTED_REQUIREMENTS_MAPPING # type: ignore[attr-defined] from organisations.models import Organisation -from projects.models import Project +from projects.models import Project, UserProjectPermission from users.models import FFAdminUser @@ -511,8 +511,6 @@ def test_create_metadata_field__project_admin__returns_201( project: Project, ) -> None: # Given - from projects.models import UserProjectPermission - UserProjectPermission.objects.create(user=staff_user, project=project, admin=True) url = reverse("api-v1:metadata:metadata-fields-list") @@ -942,8 +940,6 @@ def test_modify_metadata_field__project_admin__permission_check( request: pytest.FixtureRequest, ) -> None: # Given - from projects.models import UserProjectPermission - UserProjectPermission.objects.create(user=staff_user, project=project, admin=True) field_project = ( request.getfixturevalue(field_project_attr) if field_project_attr else None @@ -969,3 +965,172 @@ def test_modify_metadata_field__project_admin__permission_check( # Then assert response.status_code == expected_status + + +def test_create_model_metadata_field__project_admin__returns_201( + staff_user: FFAdminUser, + staff_client: APIClient, + organisation: Organisation, + project: Project, + feature_content_type: ContentType, +) -> None: + # Given - a project-scoped MetadataField and a user who is project admin (not org admin) + UserProjectPermission.objects.create(user=staff_user, project=project, admin=True) + project_field = MetadataField.objects.create( + name="proj_field", + type="int", + organisation=organisation, + project=project, + ) + url = reverse( + "api-v1:organisations:metadata-model-fields-list", args=[organisation.id] + ) + data = { + "field": project_field.id, + "content_type": feature_content_type.id, + "is_required_for": [], + } + + # When + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then - project admin should be allowed to bind a project-scoped field + assert response.status_code == status.HTTP_201_CREATED + assert response.json()["field"] == project_field.id + + +def test_create_model_metadata_field__project_admin_org_scoped_field__returns_403( + staff_user: FFAdminUser, + staff_client: APIClient, + organisation: Organisation, + project: Project, + feature_content_type: ContentType, +) -> None: + # Given - an org-scoped MetadataField (project=None) and a user who is project admin only + UserProjectPermission.objects.create(user=staff_user, project=project, admin=True) + org_field = MetadataField.objects.create( + name="org_field", + type="int", + organisation=organisation, + ) + url = reverse( + "api-v1:organisations:metadata-model-fields-list", args=[organisation.id] + ) + data = { + "field": org_field.id, + "content_type": feature_content_type.id, + "is_required_for": [], + } + + # When + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then - project admin must not be able to bind org-scoped fields + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.parametrize( + "field_project_attr, expected_status", + [ + ("project", status.HTTP_200_OK), + (None, status.HTTP_403_FORBIDDEN), + ("project_b", status.HTTP_403_FORBIDDEN), + ], + ids=[ + "own_project_field_allowed", + "org_scoped_field_denied", + "other_project_field_denied", + ], +) +def test_update_model_metadata_field__project_admin__returns_expected_status( + staff_user: FFAdminUser, + staff_client: APIClient, + organisation: Organisation, + project: Project, + project_b: Project, + feature_content_type: ContentType, + field_project_attr: str | None, + expected_status: int, + request: pytest.FixtureRequest, +) -> None: + # Given - a project admin for `project`, and a MetadataModelField whose parent + # MetadataField belongs to one of: `project`, `project_b`, or the org (None) + UserProjectPermission.objects.create(user=staff_user, project=project, admin=True) + field_project = ( + request.getfixturevalue(field_project_attr) if field_project_attr else None + ) + field = MetadataField.objects.create( + name="model_field", type="int", organisation=organisation, project=field_project + ) + model_field = MetadataModelField.objects.create( + field=field, content_type=feature_content_type + ) + url = reverse( + "api-v1:organisations:metadata-model-fields-detail", + args=[organisation.id, model_field.id], + ) + data = { + "field": field.id, + "content_type": feature_content_type.id, + "is_required_for": [], + } + + # When + response = staff_client.put( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == expected_status + + +@pytest.mark.parametrize( + "field_project_attr, expected_status", + [ + ("project", status.HTTP_204_NO_CONTENT), + (None, status.HTTP_403_FORBIDDEN), + ("project_b", status.HTTP_403_FORBIDDEN), + ], + ids=[ + "own_project_field_allowed", + "org_scoped_field_denied", + "other_project_field_denied", + ], +) +def test_delete_model_metadata_field__project_admin__returns_expected_status( + staff_user: FFAdminUser, + staff_client: APIClient, + organisation: Organisation, + project: Project, + project_b: Project, + feature_content_type: ContentType, + field_project_attr: str | None, + expected_status: int, + request: pytest.FixtureRequest, +) -> None: + # Given - a project admin for `project`, and a MetadataModelField whose parent + # MetadataField belongs to one of: `project`, `project_b`, or the org (None) + UserProjectPermission.objects.create(user=staff_user, project=project, admin=True) + field_project = ( + request.getfixturevalue(field_project_attr) if field_project_attr else None + ) + field = MetadataField.objects.create( + name="model_field", type="int", organisation=organisation, project=field_project + ) + model_field = MetadataModelField.objects.create( + field=field, content_type=feature_content_type + ) + url = reverse( + "api-v1:organisations:metadata-model-fields-detail", + args=[organisation.id, model_field.id], + ) + + # When + response = staff_client.delete(url) + + # Then + assert response.status_code == expected_status