diff --git a/api/metadata/permissions.py b/api/metadata/permissions.py index 65cd54a57881..6af2ef0cb940 100644 --- a/api/metadata/permissions.py +++ b/api/metadata/permissions.py @@ -74,10 +74,14 @@ 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 ( - request.user.is_organisation_admin(organisation_pk) - and organisation_pk == field.organisation.id - ) + if organisation_pk != field.organisation.id: + return False + + if request.user.is_organisation_admin(organisation_pk): + return True + + if field.project is not None: + return request.user.is_project_admin(field.project) return False @@ -86,6 +90,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) + if request.user.is_organisation_admin(obj.field.organisation): + return True + + if obj.field.project is not None: + return request.user.is_project_admin(obj.field.project) return False diff --git a/api/tests/unit/metadata/test_views.py b/api/tests/unit/metadata/test_views.py index 308ec67836b6..3a0bfe378bc7 100644 --- a/api/tests/unit/metadata/test_views.py +++ b/api/tests/unit/metadata/test_views.py @@ -969,3 +969,140 @@ 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) + from projects.models import UserProjectPermission + + 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 + from projects.models import UserProjectPermission + + 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, action, expected_status", + [ + ("project", "put", status.HTTP_200_OK), + ("project", "delete", status.HTTP_204_NO_CONTENT), + (None, "put", status.HTTP_403_FORBIDDEN), + (None, "delete", status.HTTP_403_FORBIDDEN), + ("project_b", "put", status.HTTP_403_FORBIDDEN), + ("project_b", "delete", status.HTTP_403_FORBIDDEN), + ], + ids=[ + "own_project_field_update_allowed", + "own_project_field_delete_allowed", + "org_scoped_field_update_denied", + "org_scoped_field_delete_denied", + "other_project_field_update_denied", + "other_project_field_delete_denied", + ], +) +def test_modify_model_metadata_field__project_admin__permission_check( + staff_user: FFAdminUser, + staff_client: APIClient, + organisation: Organisation, + project: Project, + project_b: Project, + feature_content_type: ContentType, + field_project_attr: str | None, + action: str, + 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) + 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 + ) + 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 + if action == "put": + data = { + "field": field.id, + "content_type": feature_content_type.id, + "is_required_for": [], + } + response = staff_client.put( + url, data=json.dumps(data), content_type="application/json" + ) + else: + response = staff_client.delete(url) + + # Then + assert response.status_code == expected_status