Skip to content

Commit

Permalink
feat(permissions/tags): Add tags support (#2685)
Browse files Browse the repository at this point in the history
  • Loading branch information
gagantrivedi authored Oct 30, 2023
1 parent f4185a5 commit 78e559c
Show file tree
Hide file tree
Showing 12 changed files with 176 additions and 41 deletions.
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

3 comments on commit 78e559c

@vercel
Copy link

@vercel vercel bot commented on 78e559c Oct 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vercel
Copy link

@vercel vercel bot commented on 78e559c Oct 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vercel
Copy link

@vercel vercel bot commented on 78e559c Oct 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

docs – ./docs

docs-flagsmith.vercel.app
docs-git-main-flagsmith.vercel.app
docs.bullet-train.io
docs.flagsmith.com

Please sign in to comment.