diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/local_development.yml b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/local_development.yml index 606989020..ac0d7e6ee 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/local_development.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/local_development.yml @@ -41,10 +41,6 @@ permissions: groups: [group3] actions: [read] uri: PM:site-administration:set-permissions - pg-read: - groups: [group3] - actions: [read] - uri: PG:misc public_access: groups: [spiff_public] diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/permission_assignment.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/permission_assignment.py index d9b6485e7..839cfbbd8 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/permission_assignment.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/permission_assignment.py @@ -52,3 +52,10 @@ class PermissionAssignmentModel(SpiffworkflowBaseDBModel): @validates("permission") def validate_permission(self, key: str, value: str) -> Any: return self.validate_enum_field(key, value, Permission) + + def __repr__(self) -> str: + value = ( + f"PermissionAssignmentModel(id={self.id}, target={self.permission_target.uri}, " + f"permission={self.permission}, grant_type={self.grant_type})" + ) + return value diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_groups_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_groups_controller.py index 4fe037881..ee9598c79 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_groups_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_groups_controller.py @@ -107,19 +107,22 @@ def process_group_show( modified_process_group_id: str, ) -> Any: process_group_id = _un_modify_modified_process_model_id(modified_process_group_id) - has_access_to_group_without_considering_process_models = True + has_access_to_group_without_considering_subgroups_and_models = True try: AuthorizationService.check_permission_for_request() except NotAuthorizedError: - has_access_to_group_without_considering_process_models = False + has_access_to_group_without_considering_subgroups_and_models = False try: - if has_access_to_group_without_considering_process_models: + user = UserService.current_user() + if ( + has_access_to_group_without_considering_subgroups_and_models + or AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, process_group_id) + ): # do not return child models and groups here since this call does not check permissions of them process_group = ProcessModelService.get_process_group(process_group_id, find_direct_nested_items=False) else: - user = UserService.current_user() - process_group = ProcessModelService.get_process_group_with_permission_check(process_group_id, user) + raise ProcessEntityNotFoundError("viewing this process group is not authorized") except ProcessEntityNotFoundError as exception: raise ( ApiError( diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py index f8a503d4c..d9fab53a4 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py @@ -31,6 +31,7 @@ from spiffworkflow_backend.models.human_task import HumanTaskModel from spiffworkflow_backend.models.permission_assignment import PermissionAssignmentModel from spiffworkflow_backend.models.permission_target import PermissionTargetModel from spiffworkflow_backend.models.principal import PrincipalModel +from spiffworkflow_backend.models.process_model import ProcessModelInfo from spiffworkflow_backend.models.service_account import SPIFF_SERVICE_ACCOUNT_AUTH_SERVICE from spiffworkflow_backend.models.task import TaskModel # noqa: F401 from spiffworkflow_backend.models.user import SPIFF_GUEST_USER @@ -175,14 +176,18 @@ class AuthorizationService: permission_assignment.permission_target.uri, target_uri_normalized ): matching_permission_assignments.append(permission_assignment) - if len(matching_permission_assignments) == 0: + + return cls.has_permissions_and_all_permissions_permit(matching_permission_assignments) + + @classmethod + def has_permissions_and_all_permissions_permit(cls, permission_assignments: list[PermissionAssignmentModel]) -> bool: + if len(permission_assignments) == 0: return False - all_permissions_permit = True - for permission_assignment in matching_permission_assignments: + for permission_assignment in permission_assignments: if permission_assignment.grant_type == "deny": - all_permissions_permit = False - return all_permissions_permit + return False + return True @classmethod def target_uri_matches_actual_uri(cls, target_uri: str, actual_uri: str) -> bool: @@ -205,6 +210,36 @@ class AuthorizationService: db.session.delete(group) db.session.commit() + # if you have access to PG:hey:%, you should be able to see PG hey, obviously. + # if you have access to PG:hey:yo:%, you should ALSO be able to see PG hey, because that allows you to navigate to hey:yo. + # if you have access to PG:hey:%, you should be able to see PG hey:yo, but that is handled by the normal permissions system, + # and we will never ask that question of this method. + @classmethod + def is_user_allowed_to_view_process_group_with_id(cls, user: UserModel, group_identifier: str) -> bool: + modified_group_identifier = ProcessModelInfo.modify_process_identifier_for_path_param(group_identifier) + has_permission = cls.user_has_permission( + user=user, + permission="read", + target_uri=f"/process-groups/{modified_group_identifier}", + ) + if has_permission: + return True + all_permission_assignments = cls.all_permission_assignments_for_user(user) + matching_permission_assignments = [] + for permission_assignment in all_permission_assignments: + uri = permission_assignment.permission_target.uri + uri_as_pg_identifier = uri.removeprefix("/process-groups/").removesuffix(":%") + if permission_assignment.permission == "read" and ( + permission_assignment.grant_type == "permit" + and ( + uri.startswith(f"/process-groups/{modified_group_identifier}") + or uri.startswith(f"/process-models/{modified_group_identifier}") + ) + or (permission_assignment.grant_type == "deny" and modified_group_identifier.startswith(uri_as_pg_identifier)) + ): + matching_permission_assignments.append(permission_assignment) + return cls.has_permissions_and_all_permissions_permit(matching_permission_assignments) + @classmethod def associate_user_with_group(cls, user: UserModel, group: GroupModel) -> None: user_group_assignemnt = UserGroupAssignmentModel.query.filter_by(user_id=user.id, group_id=group.id).first() diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_service.py index e9fd93951..55067fb7a 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_service.py @@ -671,12 +671,3 @@ class ProcessModelService(FileSystemService): # we don't store `id` in the json files, so we add it in here process_model_info.id = name return process_model_info - - @classmethod - def get_process_group_with_permission_check(cls, process_group_id: str, user: UserModel) -> ProcessGroup: - process_models = cls.get_process_models_for_api(user=user, process_group_id=process_group_id, recursive=False) - if not process_models: - raise ProcessEntityNotFoundError("User has no access to contained models, and therefore no access to group.") - - # do not return child models and groups here since this call does not check permissions of them - return cls.get_process_group(process_group_id, find_direct_nested_items=False) diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py index 0b68eb961..d6c0a0dd2 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py @@ -335,6 +335,41 @@ class TestAuthorizationService(BaseTest): self.assert_user_has_permission(user, "read", "/v1.0/process-groups/hey") self.assert_user_has_permission(user, "read", "/v1.0/process-groups/hey:yo") + # https://github.com/sartography/spiff-arena/issues/1090 describes why we need access to process_group_show for parents + def test_granting_access_to_subgroup_gives_access_to_subgroup_its_subgroups_and_even_show_for_its_parents( + self, + app: Flask, + client: FlaskClient, + with_db_and_bpmn_file_cleanup: None, + ) -> None: + user = self.find_or_create_user(username="user_one") + user_group = UserService.find_or_create_group("group_one") + UserService.add_user_to_group(user, user_group) + AuthorizationService.add_permission_from_uri_or_macro(user_group.identifier, "read", "PG:anotherprefix:yo") + assert AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, "hey") is False + + AuthorizationService.add_permission_from_uri_or_macro(user_group.identifier, "read", "PG:/hey/yo") + AuthorizationService.add_permission_from_uri_or_macro(user_group.identifier, "DENY:read", "PG:hey:yo:who") + assert AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, "hey") + assert AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, "hey/yo") + assert AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, "hey:yo") + assert AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, "hey/yo/who") is False + assert AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, "hey/yo/who/what") is False + + def test_granting_access_to_model_gives_access_to_process_group_show_for_parent_groups_to_allow_navigating_to_model( + self, + app: Flask, + client: FlaskClient, + with_db_and_bpmn_file_cleanup: None, + ) -> None: + user = self.find_or_create_user(username="user_one") + user_group = UserService.find_or_create_group("group_one") + UserService.add_user_to_group(user, user_group) + AuthorizationService.add_permission_from_uri_or_macro(user_group.identifier, "read", "PM:hey:yo:wow:hot") + assert AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, "hey") + assert AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, "hey/yo") + assert AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, "hey/yo/wow") + def test_explode_permissions_with_invalid_target_uri( self, app: Flask,