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(permissions/tags): Add tags support #2685

Merged
merged 9 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 .github/workflows/api-deploy-production-ecs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ jobs:
flagsmith_saml_revision: v1.1.0
flagsmith_workflows_revision: v1.2.5
flagsmith_auth_controller_revision: v0.0.1
flagsmith_rbac_revision: v0.3.1
flagsmith_rbac_revision: v0.4.1

- name: Deploy task processor to Production
uses: ./.github/actions/task-processor-deploy-ecs
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/api-deploy-staging-ecs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ jobs:
flagsmith_saml_revision: v1.1.0
flagsmith_workflows_revision: v1.2.5
flagsmith_auth_controller_revision: v0.0.1
flagsmith_rbac_revision: v0.3.1
flagsmith_rbac_revision: v0.4.1

- name: Deploy task processor to Staging
uses: ./.github/actions/task-processor-deploy-ecs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ on:

env:
FLAGSMITH_SAML_REVISION: v1.1.0
FLAGSMITH_RBAC_REVISION: v0.3.1
FLAGSMITH_RBAC_REVISION: v0.4.1
FLAGSMITH_WORKFLOWS_REVISION: v1.2.5
FLAGSMITH_AUTH_CONTROLLER_REVISION: v0.0.1

Expand Down
25 changes: 17 additions & 8 deletions api/api_keys/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,19 @@ def is_project_admin(self, project: "Project") -> bool:
def is_environment_admin(self, environment: "Environment") -> bool:
return is_master_api_key_environment_admin(self.key, environment)

def has_project_permission(self, permission: str, project: "Project") -> bool:
return project in self.get_permitted_projects(permission)
def has_project_permission(
self, permission: str, project: "Project", tag_ids: typing.List[int] = None
) -> bool:
return project in self.get_permitted_projects(permission, tag_ids)

def has_environment_permission(
self, permission: str, environment: "Environment"
self,
permission: str,
environment: "Environment",
tag_ids: typing.List[int] = None,
) -> bool:
return environment in self.get_permitted_environments(
permission, environment.project
permission, environment.project, tag_ids
)

def has_organisation_permission(
Expand All @@ -61,12 +66,16 @@ def has_organisation_permission(
self.key, organisation, permission_key
)

def get_permitted_projects(self, permission_key: str) -> QuerySet["Project"]:
return get_permitted_projects_for_master_api_key(self.key, permission_key)
def get_permitted_projects(
self, permission_key: str, tag_ids: typing.List[int] = None
) -> QuerySet["Project"]:
return get_permitted_projects_for_master_api_key(
self.key, permission_key, tag_ids
)

def get_permitted_environments(
self, permission_key: str, project: "Project"
self, permission_key: str, project: "Project", tag_ids: typing.List[int] = None
) -> QuerySet["Environment"]:
return get_permitted_environments_for_master_api_key(
self.key, project, permission_key
self.key, project, permission_key, tag_ids
)
2 changes: 2 additions & 0 deletions api/environments/permissions/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,5 @@
"Permission to approve change requests in the given environment.",
),
]

TAG_SUPPORTED_PERMISSIONS = [UPDATE_FEATURE_STATE]
14 changes: 9 additions & 5 deletions api/features/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from django_lifecycle import (
AFTER_CREATE,
BEFORE_CREATE,
BEFORE_SAVE,
LifecycleModelMixin,
hook,
)
Expand Down Expand Up @@ -673,18 +674,21 @@ def get_multivariate_feature_state_value(
return getattr(self, "feature_state_value", None)

@hook(BEFORE_CREATE)
def check_for_existing_feature_state(self):
# prevent duplicate feature states being created for an environment
@hook(BEFORE_SAVE, when="deleted", is_not=True)
def check_for_duplicate_feature_state(self):
if self.version is None:
return

if FeatureState.objects.filter(
filter_ = Q(
environment=self.environment,
feature=self.feature,
version=self.version,
feature_segment=self.feature_segment,
identity=self.identity,
).exists():
)
if self.id:
filter_ &= ~Q(id=self.id)

if FeatureState.objects.filter(filter_).exists():
raise ValidationError(
"Feature state already exists for this environment, feature, "
"version, segment & identity combination"
Expand Down
40 changes: 35 additions & 5 deletions api/features/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,20 @@
from rest_framework.permissions import IsAuthenticated

from environments.models import Environment
from environments.permissions.constants import (
TAG_SUPPORTED_PERMISSIONS as TAG_SUPPORTED_ENVIRONMENT_PERMISSIONS,
)
from environments.permissions.constants import (
UPDATE_FEATURE_STATE,
VIEW_ENVIRONMENT,
)
from features.models import Feature
from projects.models import Project
from projects.permissions import CREATE_FEATURE, DELETE_FEATURE, VIEW_PROJECT
from projects.permissions import CREATE_FEATURE, DELETE_FEATURE
from projects.permissions import (
TAG_SUPPORTED_PERMISSIONS as TAG_SUPPORTED_PROJECT_PERMISSIONS,
)
from projects.permissions import VIEW_PROJECT

ACTION_PERMISSIONS_MAP = {
"retrieve": VIEW_PROJECT,
Expand All @@ -28,6 +36,10 @@ def has_permission(self, request, view):
if not super().has_permission(request, view):
return False

if view.detail:
# handled by has_object_permission
return True

try:
project_id = view.kwargs.get("project_pk") or request.data.get("project")
project = Project.objects.get(id=project_id)
Expand All @@ -46,8 +58,13 @@ def has_permission(self, request, view):
def has_object_permission(self, request, view, obj):
# map of actions and their required permission
if view.action in ACTION_PERMISSIONS_MAP:
tag_ids = []
required_permission = ACTION_PERMISSIONS_MAP.get(view.action)
if required_permission in TAG_SUPPORTED_PROJECT_PERMISSIONS:
tag_ids = list(obj.tags.values_list("id", flat=True))

return request.user.has_project_permission(
ACTION_PERMISSIONS_MAP[view.action], obj.project
ACTION_PERMISSIONS_MAP[view.action], obj.project, tag_ids=tag_ids
)

if view.action == "segments":
Expand Down Expand Up @@ -76,17 +93,30 @@ def has_permission(self, request, view):

if environment and (isinstance(environment, int) or environment.isdigit()):
environment = Environment.objects.get(id=int(environment))

tag_ids = None
required_permission = action_permission_map.get(view.action)
if required_permission in TAG_SUPPORTED_ENVIRONMENT_PERMISSIONS:
feature_id = request.data.get("feature")
feature = Feature.objects.get(id=feature_id)

tag_ids = list(feature.tags.values_list("id", flat=True))

return request.user.has_environment_permission(
action_permission_map.get(view.action), environment
required_permission, environment, tag_ids=tag_ids
)
return False

except Environment.DoesNotExist:
except (Environment.DoesNotExist, Feature.DoesNotExist):
return False

def has_object_permission(self, request, view, obj):
tag_ids = None
if UPDATE_FEATURE_STATE in TAG_SUPPORTED_ENVIRONMENT_PERMISSIONS:
tag_ids = list(obj.feature.tags.values_list("id", flat=True))

return request.user.has_environment_permission(
UPDATE_FEATURE_STATE, environment=obj.environment
UPDATE_FEATURE_STATE, environment=obj.environment, tag_ids=tag_ids
)


Expand Down
33 changes: 25 additions & 8 deletions api/permissions/permission_service.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import TYPE_CHECKING, Union
from typing import TYPE_CHECKING, List, Union

from django.db.models import Q, QuerySet

Expand Down Expand Up @@ -59,7 +59,7 @@ def is_master_api_key_environment_admin(


def get_permitted_projects_for_user(
user: "FFAdminUser", permission_key: str
user: "FFAdminUser", permission_key: str, tag_ids: List[int] = None
) -> QuerySet[Project]:
"""
Get all projects that the user has the given permissions for.
Expand All @@ -70,8 +70,15 @@ def get_permitted_projects_for_user(
- User is an admin for the organisation the project belongs to
- User has a role attached with the required permissions(if rbac is enabled)
- User is in a UserPermissionGroup that has a role attached with the required permissions
NOTE:
- If `tag_ids` is None, tags filter will not be applied
- If `tag_ids` is an empty list, only project with no tags will be returned
- If `tag_ids` is a list of tag IDs, only project with one of those tags will
be returned
"""
base_filter = get_base_permission_filter(user, Project, permission_key)
base_filter = get_base_permission_filter(
user, Project, permission_key, tag_ids=tag_ids
)

organisation_filter = Q(
organisation__userorganisation__user=user,
Expand All @@ -82,20 +89,21 @@ def get_permitted_projects_for_user(


def get_permitted_projects_for_master_api_key(
master_api_key: "MasterAPIKey", permission_key: str
master_api_key: "MasterAPIKey", permission_key: str, tag_ids: List[int] = None
) -> QuerySet[Project]:
if master_api_key.is_admin:
return Project.objects.filter(organisation_id=master_api_key.organisation_id)

return get_permitted_projects_for_master_api_key_using_roles(
master_api_key, permission_key
master_api_key, permission_key, tag_ids
)


def get_permitted_environments_for_user(
user: "FFAdminUser",
project: Project,
permission_key: str,
tag_ids: List[int] = None,
) -> QuerySet[Environment]:
"""
Get all environments that the user has the given permissions for.
Expand All @@ -107,12 +115,19 @@ def get_permitted_environments_for_user(
- User is an admin for the organisation the environment belongs to
- User has a role attached with the required permissions(if rbac is enabled)
- User is in a UserPermissionGroup that has a role attached with the required permissions(if rbac is enabled)
NOTE:
- If `tag_ids` is None, tags filter will not be applied
- If `tag_ids` is an empty list, only environments with no tags will be returned
- If `tag_ids` is a list of tag IDs, only environments with one of those tags will
be returned
"""

if is_user_project_admin(user, project):
return project.environments.all()

base_filter = get_base_permission_filter(user, Environment, permission_key)
base_filter = get_base_permission_filter(
user, Environment, permission_key, tag_ids=tag_ids
)
filter_ = base_filter & Q(project=project)

return Environment.objects.filter(filter_).distinct().defer("description")
Expand All @@ -122,12 +137,13 @@ def get_permitted_environments_for_master_api_key(
master_api_key: "MasterAPIKey",
project: Project,
permission_key: str,
tag_ids: List[int] = None,
) -> QuerySet[Environment]:
if is_master_api_key_project_admin(master_api_key, project):
return project.environments.all()

return get_permitted_environments_for_master_api_key_using_roles(
master_api_key, project, permission_key
master_api_key, project, permission_key, tag_ids
)


Expand Down Expand Up @@ -173,12 +189,13 @@ def get_base_permission_filter(
for_model: Union[Organisation, Project, Environment] = None,
permission_key: str = None,
allow_admin: bool = True,
tag_ids=None,
) -> Q:
user_filter = get_user_permission_filter(user, permission_key, allow_admin)
group_filter = get_group_permission_filter(user, permission_key, allow_admin)

role_filter = get_role_permission_filter(
user, for_model, permission_key, allow_admin
user, for_model, permission_key, allow_admin, tag_ids
)

return user_filter | group_filter | role_filter
Expand Down
11 changes: 7 additions & 4 deletions api/permissions/rbac_wrapper.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Union
from typing import List, Union

from django.conf import settings
from django.db.models import Q, QuerySet
Expand Down Expand Up @@ -46,25 +46,28 @@ def is_master_api_key_object_admin(


def get_permitted_projects_for_master_api_key_using_roles(
master_api_key: "MasterAPIKey", permission_key: str
master_api_key: "MasterAPIKey", permission_key: str, tag_ids=None
) -> QuerySet[Project]:
if not settings.IS_RBAC_INSTALLED:
return Project.objects.none()

filter_ = get_role_permission_filter(master_api_key, Project, permission_key)
filter_ = get_role_permission_filter(
master_api_key, Project, permission_key, tag_ids=tag_ids
)
return Project.objects.filter(filter_).distinct()


def get_permitted_environments_for_master_api_key_using_roles(
master_api_key: "MasterAPIKey",
project: Project,
permission_key: str,
tag_ids: List[int] = None,
) -> QuerySet[Environment]:
if not settings.IS_RBAC_INSTALLED:
return Environment.objects.none()

base_filter = get_role_permission_filter(
master_api_key, Environment, permission_key
master_api_key, Environment, permission_key, tag_ids=tag_ids
)

filter_ = base_filter & Q(project=project)
Expand Down
2 changes: 2 additions & 0 deletions api/projects/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
EDIT_FEATURE = "EDIT_FEATURE"
MANAGE_SEGMENTS = "MANAGE_SEGMENTS"

TAG_SUPPORTED_PERMISSIONS = [DELETE_FEATURE]

PROJECT_PERMISSIONS = [
(VIEW_PROJECT, "View permission for the given project."),
(CREATE_ENVIRONMENT, "Ability to create an environment in the given project."),
Expand Down
Loading