diff --git a/.github/workflows/conventional-commit.yml b/.github/workflows/conventional-commit.yml index 225ece385b55..6dcac8d62781 100644 --- a/.github/workflows/conventional-commit.yml +++ b/.github/workflows/conventional-commit.yml @@ -2,7 +2,9 @@ name: Conventional Commit on: pull_request: - types: [opened, synchronize, reopened, ready_for_review] + types: + - edited + - opened jobs: conventional-commit: diff --git a/api/features/feature_segments/permissions.py b/api/features/feature_segments/permissions.py index f2f81021329b..54a64897acad 100644 --- a/api/features/feature_segments/permissions.py +++ b/api/features/feature_segments/permissions.py @@ -1,5 +1,3 @@ -from contextlib import suppress - from common.environments.permissions import ( MANAGE_SEGMENT_OVERRIDES, ) @@ -22,13 +20,20 @@ def has_permission(self, request, view): # type: ignore[no-untyped-def] return True if view.action == "create": - with suppress(Environment.DoesNotExist, ValueError): - environment = request.data.get("environment") - environment = Environment.objects.get(id=int(environment)) - - return request.user.has_environment_permission( - MANAGE_SEGMENT_OVERRIDES, environment - ) + try: + # Validate here because this evaluates prior to the serializer layer + environment_pk = int(request.data.get("environment")) + except (TypeError, ValueError): + return False + + try: + environment = Environment.objects.get(pk=environment_pk) + except Environment.DoesNotExist: + return False + + return request.user.has_environment_permission( + MANAGE_SEGMENT_OVERRIDES, environment + ) return False 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..d64e71a4945d --- /dev/null +++ b/api/tests/unit/features/feature_segments/test_unit_feature_segments_permissions.py @@ -0,0 +1,37 @@ +from typing import Any, Callable + +import pytest +from pytest_mock import MockerFixture + +from environments.models import Environment +from features.feature_segments import permissions + + +@pytest.mark.parametrize( + "get_payload, expected", + [ + (lambda environment: {}, False), + (lambda environment: {"environment": "invalid"}, False), + (lambda environment: {"environment": 10_000_000}, False), # Please don't exist + (lambda environment: {"environment": environment.pk}, True), + ], +) +def test_FeatureSegmentPermissions_has_permission__create_action__handles_environment_arg( + environment: Environment, + expected: bool, + mocker: MockerFixture, + get_payload: Callable[[Environment], dict[str, Any]], +) -> None: + # Given + view = mocker.Mock(action="create", detail=False) + request = mocker.Mock(data=get_payload(environment)) + request.user.has_environment_permission.return_value = True + + # When + permission = mocker.Mock(spec=permissions.FeatureSegmentPermissions) + result = permissions.FeatureSegmentPermissions.has_permission( # type: ignore[no-untyped-call] + permission, request, view + ) + + # Then + assert result is expected