diff --git a/api/features/feature_segments/permissions.py b/api/features/feature_segments/permissions.py index 4914ac628ea9..468ed79b114b 100644 --- a/api/features/feature_segments/permissions.py +++ b/api/features/feature_segments/permissions.py @@ -6,6 +6,7 @@ from rest_framework.permissions import IsAuthenticated from environments.models import Environment +from features.models import Feature class FeatureSegmentPermissions(IsAuthenticated): @@ -17,7 +18,7 @@ def has_permission(self, request, view): # type: ignore[no-untyped-def] if view.detail: return True - # handle by the view + # handled by the view if view.action in ["list", "get_by_uuid", "update_priorities"]: return True @@ -26,13 +27,27 @@ def has_permission(self, request, view): # type: ignore[no-untyped-def] environment = request.data.get("environment") environment = Environment.objects.get(id=int(environment)) + if feature_id := request.data.get("feature"): + feature = Feature.objects.get( + id=feature_id, project=environment.project + ) + tag_ids = list(feature.tags.values_list("id", flat=True)) + else: + tag_ids = [] + return request.user.has_environment_permission( - MANAGE_SEGMENT_OVERRIDES, environment + permission=MANAGE_SEGMENT_OVERRIDES, + environment=environment, + tag_ids=tag_ids, ) return False def has_object_permission(self, request, view, obj): # type: ignore[no-untyped-def] + tag_ids = list(obj.feature.tags.values_list("id", flat=True)) + return request.user.has_environment_permission( - MANAGE_SEGMENT_OVERRIDES, environment=obj.environment + permission=MANAGE_SEGMENT_OVERRIDES, + environment=obj.environment, + tag_ids=tag_ids, ) diff --git a/api/features/permissions.py b/api/features/permissions.py index 55138be141a6..e818447cff61 100644 --- a/api/features/permissions.py +++ b/api/features/permissions.py @@ -211,9 +211,16 @@ def has_permission(self, request, view): # type: ignore[no-untyped-def] Environment, api_key=view.kwargs["environment_api_key"] ) + if feature_id := view.kwargs.get("feature_pk"): + feature = Feature.objects.get(id=feature_id, project=environment.project) + tag_ids = list(feature.tags.values_list("id", flat=True)) + else: + tag_ids = [] + return request.user.has_environment_permission( permission=MANAGE_SEGMENT_OVERRIDES, environment=environment, + tag_ids=tag_ids, ) diff --git a/api/tests/unit/features/feature_segments/test_unit_feature_segments_permissions.py b/api/tests/unit/features/feature_segments/test_unit_feature_segments_permissions.py new file mode 100644 index 000000000000..f94b81b66bd9 --- /dev/null +++ b/api/tests/unit/features/feature_segments/test_unit_feature_segments_permissions.py @@ -0,0 +1,149 @@ +from unittest.mock import MagicMock, patch + +from rest_framework.permissions import IsAuthenticated + +from features.feature_segments.permissions import FeatureSegmentPermissions + + +def test_has_permission_when_super_returns_false(): # type: ignore[no-untyped-def] # noqa: E501 + permission = FeatureSegmentPermissions() + request = MagicMock() + view = MagicMock() + with patch.object(IsAuthenticated, "has_permission", return_value=False): + result = permission.has_permission(request, view) # type: ignore[no-untyped-call] + assert result is False + + +def test_feature_permission_return_true_if_view_action_is_list(): # type: ignore[no-untyped-def] # noqa: E501 + # GIVEN + permission = FeatureSegmentPermissions() + + request = MagicMock() + view = MagicMock() + view.action = "list" + view.detail = None + + # WHEN + result = permission.has_permission(request, view) # type: ignore[no-untyped-call] + + # THEN + assert result is True + + +def test_feature_permission_return_true_if_view_action_is_get_by_uuid(): # type: ignore[no-untyped-def] # noqa: E501 + # GIVEN + permission = FeatureSegmentPermissions() + + request = MagicMock() + view = MagicMock() + view.action = "get_by_uuid" + view.detail = None + + # WHEN + result = permission.has_permission(request, view) # type: ignore[no-untyped-call] + + # THEN + assert result is True + + +def test_feature_permission_return_true_if_view_action_is_update_priorities(): # type: ignore[no-untyped-def] # noqa: E501 + # GIVEN + permission = FeatureSegmentPermissions() + + request = MagicMock() + view = MagicMock() + view.action = "update_priorities" + view.detail = None + + # WHEN + result = permission.has_permission(request, view) # type: ignore[no-untyped-call] + + # THEN + assert result is True + + +def test_feature_permission_return_true_if_view_details_is_not_falsy(): # type: ignore[no-untyped-def] # noqa: E501 + # GIVEN + permission = FeatureSegmentPermissions() + + request = MagicMock() + view = MagicMock() + view.action = "anything" + view.detail = "anything" + + # WHEN + result = permission.has_permission(request, view) # type: ignore[no-untyped-call] + + # THEN + assert result is True + + +def test_feature_permission_return_false_if_view_action_is_unsupported(): # type: ignore[no-untyped-def] # noqa: E501 + # GIVEN + permission = FeatureSegmentPermissions() + + request = MagicMock() + view = MagicMock() + view.action = "anything" + view.detail = None + + # WHEN + result = permission.has_permission(request, view) # type: ignore[no-untyped-call] + + # THEN + assert result is False + + +def test_feature_permission_pass_empty_list_of_tags_if_feature_id_is_not_provided(): # type: ignore[no-untyped-def] # noqa: E501 + # GIVEN + permission = FeatureSegmentPermissions() + fake_env = MagicMock() + + request = MagicMock() + request.data = {"environment": "1", "feature": None} + request.user.has_environment_permission.return_value = True + + view = MagicMock() + view.action = "create" + view.detail = None + + # WHEN + with patch("environments.models.Environment.objects.get", return_value=fake_env): + result = permission.has_permission(request, view) # type: ignore[no-untyped-call] + + # THEN + assert result is True + request.user.has_environment_permission.assert_called_once_with( + permission="MANAGE_SEGMENT_OVERRIDES", + environment=fake_env, + tag_ids=[], + ) + + +def test_feature_permission_pass_list_of_tags_if_feature_id_is_provided(): # type: ignore[no-untyped-def] # noqa: E501 + # GIVEN + permission = FeatureSegmentPermissions() + fake_env = MagicMock() + fake_feature = MagicMock() + fake_feature.tags.values_list.return_value = [1, 2, 3] + + request = MagicMock() + request.data = {"environment": "1", "feature": "1"} + request.user.has_environment_permission.return_value = True + + view = MagicMock() + view.action = "create" + view.detail = None + + # WHEN + with patch("environments.models.Environment.objects.get", return_value=fake_env): + with patch("features.models.Feature.objects.get", return_value=fake_feature): + result = permission.has_permission(request, view) # type: ignore[no-untyped-call] + + # THEN + assert result is True + request.user.has_environment_permission.assert_called_once_with( + permission="MANAGE_SEGMENT_OVERRIDES", + environment=fake_env, + tag_ids=[1, 2, 3], + ) diff --git a/api/tests/unit/features/feature_segments/test_unit_feature_segments_views.py b/api/tests/unit/features/feature_segments/test_unit_feature_segments_views.py index a73f6bdc7830..0e85a8a851f1 100644 --- a/api/tests/unit/features/feature_segments/test_unit_feature_segments_views.py +++ b/api/tests/unit/features/feature_segments/test_unit_feature_segments_views.py @@ -270,6 +270,71 @@ def test_create_feature_segment_staff_wrong_permission( # type: ignore[no-untyp assert response.status_code == status.HTTP_403_FORBIDDEN +def test_create_segment_override_permissions_granted( + staff_user: FFAdminUser, + project: Project, + environment: Environment, + feature: Feature, + segment: Segment, + staff_client: APIClient, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + # Give user permission to manage segment overrides + with_environment_permissions([MANAGE_SEGMENT_OVERRIDES]) # type: ignore[call-arg] + + url = reverse( + "api-v1:environments:create-segment-override", + args=[environment.api_key, feature.id], + ) + data = { + "feature_segment": {"segment": segment.id}, + "enabled": True, + "feature_state_value": { + "type": "unicode", + "string_value": "new_state", + }, + } + + # When + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED + + +def test_create_segment_override_permissions_denied( + staff_user: FFAdminUser, + project: Project, + environment: Environment, + feature: Feature, + segment: Segment, + staff_client: APIClient, +) -> None: + # No permissions given explicitly for MANAGE_SEGMENT_OVERRIDES + url = reverse( + "api-v1:environments:create-segment-override", + args=[environment.api_key, feature.id], + ) + data = { + "feature_segment": {"segment": segment.id}, + "enabled": True, + "feature_state_value": { + "type": "unicode", + "string_value": "new_state", + }, + } + + # When + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_403_FORBIDDEN + + @pytest.mark.parametrize( "client", [lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")],