From 23e147793a9ef589f7764043002a7b35dbeefd8c Mon Sep 17 00:00:00 2001 From: Kevin Burnett <18027+burnettk@users.noreply.github.com> Date: Mon, 29 Apr 2024 20:03:46 +0000 Subject: [PATCH] make permissions on process group show more nuanced (#1460) * if you have access to things in a group, allow navigating through group * lint --------- Co-authored-by: burnettk --- .../routes/process_groups_controller.py | 20 +++++++++++++++++-- .../services/authorization_service.py | 9 ++++++--- .../services/process_model_service.py | 9 +++++++++ 3 files changed, 33 insertions(+), 5 deletions(-) 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 fe90d2a03..4fe037881 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_groups_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_groups_controller.py @@ -10,13 +10,16 @@ from flask import make_response from flask.wrappers import Response from spiffworkflow_backend.exceptions.api_error import ApiError +from spiffworkflow_backend.exceptions.error import NotAuthorizedError from spiffworkflow_backend.exceptions.process_entity_not_found_error import ProcessEntityNotFoundError from spiffworkflow_backend.models.process_group import ProcessGroup from spiffworkflow_backend.models.process_group import ProcessGroupSchema from spiffworkflow_backend.routes.process_api_blueprint import _commit_and_push_to_git from spiffworkflow_backend.routes.process_api_blueprint import _un_modify_modified_process_model_id +from spiffworkflow_backend.services.authorization_service import AuthorizationService from spiffworkflow_backend.services.process_model_service import ProcessModelService from spiffworkflow_backend.services.process_model_service import ProcessModelWithInstancesNotDeletableError +from spiffworkflow_backend.services.user_service import UserService def process_group_create(body: dict) -> flask.wrappers.Response: @@ -97,13 +100,26 @@ def process_group_list( return make_response(jsonify(response_json), 200) +# this action is excluded from authorization checks, so it is important that it call: +# AuthorizationService.check_permission_for_request() +# it also allows access to the process group if the user has access to read any of the process models contained in the group 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 try: - # 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) + AuthorizationService.check_permission_for_request() + except NotAuthorizedError: + has_access_to_group_without_considering_process_models = False + + try: + if has_access_to_group_without_considering_process_models: + # 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) 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 cbc5edd95..f8a503d4c 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py @@ -337,9 +337,12 @@ class AuthorizationService: @classmethod def request_is_excluded_from_permission_check(cls) -> bool: - authorization_exclusion_list = ["permissions_check"] - api_view_function = current_app.view_functions[request.endpoint] - if api_view_function and api_view_function.__name__ in authorization_exclusion_list: + authorization_exclusion_list = [ + "spiffworkflow_backend.routes.process_api_blueprint.permissions_check", + "spiffworkflow_backend.routes.process_groups_controller.process_group_show", + ] + api_function_full_path, module = cls.get_fully_qualified_api_function_from_request() + if api_function_full_path and (api_function_full_path in authorization_exclusion_list): return True return False 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 757627a17..3ee02a912 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_service.py @@ -664,3 +664,12 @@ 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)