Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add permission for manage segments overrides #2919

Merged
merged 23 commits into from
Nov 14, 2023
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
47952b7
Add django extensions to local developement
zachaysan Nov 2, 2023
dedbb09
Add MANAGE_SEGMENT_OVERRIDES permission for feature segments
zachaysan Nov 2, 2023
bc8a549
Update to include manage segment overrides
zachaysan Nov 2, 2023
68ddeb4
Move constant to old migrations to declutter application code
zachaysan Nov 2, 2023
2d92db7
Create migraiton to add manage segments overrides and apply to existi…
zachaysan Nov 2, 2023
557f335
Add test for forbidden route
zachaysan Nov 3, 2023
6f632ef
Fix permission enforcement
zachaysan Nov 3, 2023
3a824be
Add staff user and client
zachaysan Nov 3, 2023
50035c4
Add staff tests
zachaysan Nov 3, 2023
8539bee
Format
zachaysan Nov 3, 2023
211fa75
Add staff tests
zachaysan Nov 3, 2023
7501b13
Remove unnecessary arg
zachaysan Nov 6, 2023
5c3d589
Fix conflicts and merge branch 'main' into feat/add_permission_for_ma…
zachaysan Nov 7, 2023
49ca2b0
Use fixture to create staff environment permissions
zachaysan Nov 7, 2023
4a15a52
Merge branch 'main' into feat/add_permission_for_manage_segments_over…
zachaysan Nov 7, 2023
9374d51
Create with_project_permissions fixture
zachaysan Nov 8, 2023
783fa2c
Use new fixture
zachaysan Nov 8, 2023
141333a
Use staff_user fixture instead
zachaysan Nov 8, 2023
dd5fa32
Merge branch 'main' into feat/add_permission_for_manage_segments_over…
zachaysan Nov 8, 2023
c5fcd9b
Fix word error
zachaysan Nov 8, 2023
c5fb073
Add typing to tests
zachaysan Nov 8, 2023
f49a46d
Fix conflicts and merge branch 'main' into feat/add_permission_for_ma…
zachaysan Nov 13, 2023
f725248
Merge branch 'main' into feat/add_permission_for_manage_segments_over…
zachaysan Nov 14, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/app/settings/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

ALLOWED_HOSTS.extend([".ngrok.io", "127.0.0.1", "localhost"])

INSTALLED_APPS.extend(["debug_toolbar"])
INSTALLED_APPS.extend(["debug_toolbar", "django_extensions"])

MIDDLEWARE.extend(["debug_toolbar.middleware.DebugToolbarMiddleware"])

Expand Down
27 changes: 26 additions & 1 deletion api/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,10 @@ def test_user_client(api_client, test_user):
def staff_user(django_user_model):
"""
A non-admin user fixture.

To add to an environment with permissions use the fixture
with_environment_permissions.
with_environment_permissions, or similar with the fixture


This fixture is attached to the organisation fixture.
"""
Expand Down Expand Up @@ -199,6 +201,29 @@ def _with_environment_permissions(
return _with_environment_permissions


@pytest.fixture()
def with_project_permissions(
project: Project, staff_user: FFAdminUser
) -> typing.Callable:
"""
Add project permissions to the staff_user fixture.
Defaults to associating to the project fixture.
"""

def _with_project_permissions(
permission_keys: list[str], project_id: typing.Optional[int] = None
) -> UserProjectPermission:
project_id = project_id or project.id
upp, __ = UserProjectPermission.objects.get_or_create(
project_id=project_id, user=staff_user
)
upp.permissions.add(*permission_keys)

return upp

return _with_project_permissions


@pytest.fixture()
def identity(environment):
return Identity.objects.create(identifier="test_identity", environment=environment)
Expand Down
17 changes: 14 additions & 3 deletions api/environments/migrations/0010_auto_20200219_2343.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,20 @@
# Generated by Django 2.2.10 on 2020-02-19 23:43

from django.db import migrations

from environments.permissions.constants import ENVIRONMENT_PERMISSIONS

from environments.permissions.constants import VIEW_ENVIRONMENT, UPDATE_FEATURE_STATE, MANAGE_IDENTITIES, CREATE_CHANGE_REQUEST, APPROVE_CHANGE_REQUEST
ENVIRONMENT_PERMISSIONS = [
(VIEW_ENVIRONMENT, "View permission for the given environment."),
(UPDATE_FEATURE_STATE, "Update the state or value for a given feature state."),
(MANAGE_IDENTITIES, "Manage identities in the given environment."),
(
CREATE_CHANGE_REQUEST,
"Permission to create change requests in the given environment.",
),
(
APPROVE_CHANGE_REQUEST,
"Permission to approve change requests in the given environment.",
),
]

def create_default_permissions(apps, schema_editor):
EnvironmentPermission = apps.get_model('environments', 'EnvironmentPermission')
Expand Down
15 changes: 1 addition & 14 deletions api/environments/permissions/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,6 @@
VIEW_IDENTITIES = "VIEW_IDENTITIES"
CREATE_CHANGE_REQUEST = "CREATE_CHANGE_REQUEST"
APPROVE_CHANGE_REQUEST = "APPROVE_CHANGE_REQUEST"

ENVIRONMENT_PERMISSIONS = [
(VIEW_ENVIRONMENT, "View permission for the given environment."),
(UPDATE_FEATURE_STATE, "Update the state or value for a given feature state."),
(MANAGE_IDENTITIES, "Manage identities in the given environment."),
(
CREATE_CHANGE_REQUEST,
"Permission to create change requests in the given environment.",
),
(
APPROVE_CHANGE_REQUEST,
"Permission to approve change requests in the given environment.",
),
]
MANAGE_SEGMENT_OVERRIDES = "MANAGE_SEGMENT_OVERRIDES"

TAG_SUPPORTED_PERMISSIONS = [UPDATE_FEATURE_STATE]
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# Generated by Django 3.2.20 on 2023-11-01 19:54

from django.db import migrations
from environments.permissions.constants import MANAGE_SEGMENT_OVERRIDES, UPDATE_FEATURE_STATE
from core.migration_helpers import create_new_environment_permissions

from permissions.models import ENVIRONMENT_PERMISSION_TYPE

def add_manage_segment_overrides_permission(apps, schema_editor):
PermissionModel = apps.get_model("permissions", "PermissionModel")
UserEnvironmentPermission = apps.get_model(
"environment_permissions",
"UserEnvironmentPermission",
)
UserPermissionGroupEnvironmentPermission = apps.get_model(
"environment_permissions",
"UserPermissionGroupEnvironmentPermission",
)
manage_segment_overrides_permission, _ = PermissionModel.objects.get_or_create(
key=MANAGE_SEGMENT_OVERRIDES,
description="Permission to manage segment overrides in the given environment",
type=ENVIRONMENT_PERMISSION_TYPE,
)

# Add MANAGE_SEGMENT_OVERRIDES to existing UPDATE_FEATURE_STATE holders
create_new_environment_permissions(
UPDATE_FEATURE_STATE,
UserEnvironmentPermission,
"userenvironmentpermission",
[manage_segment_overrides_permission],
)
create_new_environment_permissions(
UPDATE_FEATURE_STATE,
UserPermissionGroupEnvironmentPermission,
"userpermissiongroupenvironmentpermission",
[manage_segment_overrides_permission],
)


def remove_manage_segment_overrides_permission(apps, schema_editor):
PermissionModel = apps.get_model("permissions", "PermissionModel")
PermissionModel.objects.filter(key=MANAGE_SEGMENT_OVERRIDES).delete()

class Migration(migrations.Migration):

dependencies = [
('environment_permissions', '0007_add_unique_permission_constraint'),
]

operations = [
migrations.RunPython(
add_manage_segment_overrides_permission,
reverse_code=remove_manage_segment_overrides_permission,
)
]
4 changes: 1 addition & 3 deletions api/environments/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -780,9 +780,7 @@ def test_user_can_list_environment_permission(client, environment):

# Then
assert response.status_code == status.HTTP_200_OK
assert (
len(response.json()) == 6
) # hard code how many permissions we expect there to be
assert len(response.json()) == 7


def test_environment_my_permissions_reruns_400_for_master_api_key(
Expand Down
6 changes: 3 additions & 3 deletions api/features/feature_segments/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from rest_framework.permissions import IsAuthenticated

from environments.models import Environment
from environments.permissions.constants import UPDATE_FEATURE_STATE
from environments.permissions.constants import MANAGE_SEGMENT_OVERRIDES


class FeatureSegmentPermissions(IsAuthenticated):
Expand All @@ -25,12 +25,12 @@ def has_permission(self, request, view):
environment = Environment.objects.get(id=int(environment))

return request.user.has_environment_permission(
UPDATE_FEATURE_STATE, environment
MANAGE_SEGMENT_OVERRIDES, environment
)

return False

def has_object_permission(self, request, view, obj):
return request.user.has_environment_permission(
UPDATE_FEATURE_STATE, environment=obj.environment
MANAGE_SEGMENT_OVERRIDES, environment=obj.environment
)
4 changes: 2 additions & 2 deletions api/features/feature_segments/serializers.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from rest_framework import serializers
from rest_framework.exceptions import PermissionDenied

from environments.permissions.constants import UPDATE_FEATURE_STATE
from environments.permissions.constants import MANAGE_SEGMENT_OVERRIDES
from features.models import FeatureSegment


Expand Down Expand Up @@ -115,7 +115,7 @@ def validate(self, attrs):
environment = environments.pop()

if not self.context["request"].user.has_environment_permission(
UPDATE_FEATURE_STATE, environment
MANAGE_SEGMENT_OVERRIDES, environment
):
raise PermissionDenied("You do not have permission to perform this action.")

Expand Down
143 changes: 143 additions & 0 deletions api/features/feature_segments/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,21 @@
import json
from typing import Callable

import pytest
from django.urls import reverse
from pytest_lazyfixture import lazy_fixture
from rest_framework import status

from environments.models import Environment
from environments.permissions.constants import (
MANAGE_SEGMENT_OVERRIDES,
UPDATE_FEATURE_STATE,
)
from features.models import Feature, FeatureSegment
from projects.models import Project, UserProjectPermission
from projects.permissions import VIEW_PROJECT
from segments.models import Segment
from users.models import FFAdminUser


@pytest.mark.parametrize(
Expand Down Expand Up @@ -138,6 +146,59 @@ def test_create_feature_segment_without_permission_returns_403(
assert response.status_code == status.HTTP_403_FORBIDDEN


def test_create_feature_segment_staff_with_permission(
segment: Segment,
feature: Feature,
environment: Environment,
staff_client: FFAdminUser,
staff_user: FFAdminUser,
with_environment_permissions: Callable,
) -> None:
# Given
data = {
"feature": feature.id,
"segment": segment.id,
"environment": environment.id,
}
url = reverse("api-v1:features:feature-segment-list")
with_environment_permissions([MANAGE_SEGMENT_OVERRIDES])

# 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_feature_segment_staff_wrong_permission(
segment: Segment,
feature: Feature,
environment: Environment,
staff_client: FFAdminUser,
staff_user: FFAdminUser,
with_environment_permissions: Callable,
):
# Given
data = {
"feature": feature.id,
"segment": segment.id,
"environment": environment.id,
}
url = reverse("api-v1:features:feature-segment-list")
# Former permission; no longer authorizes.
with_environment_permissions([UPDATE_FEATURE_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")],
Expand Down Expand Up @@ -197,6 +258,33 @@ def test_update_priority_of_multiple_feature_segments(
assert json_response[1]["id"] == another_feature_segment.id


def test_update_priority_for_staff(
feature_segment: FeatureSegment,
project: Project,
environment: Environment,
feature: Feature,
staff_client: FFAdminUser,
staff_user: FFAdminUser,
with_environment_permissions: Callable,
) -> None:
# Given
url = reverse("api-v1:features:feature-segment-update-priorities")

data = [
{"id": feature_segment.id, "priority": 1},
]

with_environment_permissions([MANAGE_SEGMENT_OVERRIDES])

# When
response = staff_client.post(
url, data=json.dumps(data), content_type="application/json"
)

# Then
assert response.status_code == status.HTTP_200_OK


def test_update_priority_returns_403_if_user_does_not_have_permission(
feature_segment,
project,
Expand Down Expand Up @@ -258,6 +346,33 @@ def test_get_feature_segment_by_uuid(
assert json_response["uuid"] == str(feature_segment.uuid)


def test_get_feature_segment_by_uuid_for_staff(
feature_segment: FeatureSegment,
project: Project,
staff_client: FFAdminUser,
staff_user: FFAdminUser,
environment: Environment,
feature: Feature,
with_environment_permissions: Callable,
with_project_permissions: Callable,
) -> None:
# Given
url = reverse(
"api-v1:features:feature-segment-get-by-uuid", args=[feature_segment.uuid]
)
with_environment_permissions([UPDATE_FEATURE_STATE])
with_project_permissions([VIEW_PROJECT])

# When
response = staff_client.get(url)

# Then
assert response.status_code == status.HTTP_200_OK
json_response = response.json()
assert json_response["id"] == feature_segment.id
assert json_response["uuid"] == str(feature_segment.uuid)


def test_get_feature_segment_by_uuid_returns_404_if_user_does_not_have_access(
feature_segment, project, test_user_client, environment, feature
):
Expand Down Expand Up @@ -293,6 +408,34 @@ def test_get_feature_segment_by_id(
assert json_response["uuid"] == str(feature_segment.uuid)


def test_get_feature_segment_by_id_for_staff(
feature_segment: FeatureSegment,
project: Project,
staff_client: FFAdminUser,
staff_user: FFAdminUser,
environment: Environment,
feature: Feature,
with_environment_permissions: Callable,
):
# Given
url = reverse("api-v1:features:feature-segment-detail", args=[feature_segment.id])

with_environment_permissions([MANAGE_SEGMENT_OVERRIDES])
user_project_permission = UserProjectPermission.objects.create(
user=staff_user, project=project
)
user_project_permission.add_permission(VIEW_PROJECT)

# When
response = staff_client.get(url)

# Then
assert response.status_code == status.HTTP_200_OK
json_response = response.json()
assert json_response["id"] == feature_segment.id
assert json_response["uuid"] == str(feature_segment.uuid)


@pytest.mark.parametrize(
"client",
[lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")],
Expand Down
1 change: 1 addition & 0 deletions api/features/feature_segments/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ def get_queryset(self):
permitted_projects = self.request.user.get_permitted_projects(
permission_key=VIEW_PROJECT
)

queryset = FeatureSegment.objects.filter(
feature__project__in=permitted_projects
)
Expand Down
Loading