From fcc79e639a9eb0c202b4b11c108fc02a301ca357 Mon Sep 17 00:00:00 2001 From: jasquat Date: Wed, 21 Dec 2022 11:24:38 -0500 Subject: [PATCH] added permission to run privileged scripts w/ burnettk --- .../config/permissions/development.yml | 6 +-- .../src/spiffworkflow_backend/routes/user.py | 2 +- .../scripts/add_permission.py | 4 ++ .../spiffworkflow_backend/scripts/script.py | 47 ++++++++++++++++--- .../services/process_model_service.py | 4 +- .../script_add_permission/add_permission.bpmn | 39 +++++++++++++++ .../scripts/test_add_permission.py | 38 +++++++++++++++ .../src/routes/ProcessModelEditDiagram.tsx | 2 +- 8 files changed, 128 insertions(+), 14 deletions(-) create mode 100644 spiffworkflow-backend/tests/data/script_add_permission/add_permission.bpmn diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/development.yml b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/development.yml index 6ab24699..d8c43d4f 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/development.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/development.yml @@ -16,7 +16,7 @@ groups: alex, dan, mike, - jason, + jason@sartography.com, jarrad, elizabeth, jon, @@ -29,7 +29,7 @@ groups: alex, dan, mike, - jason, + jason@sartography.com, amir, jarrad, elizabeth, @@ -46,7 +46,7 @@ groups: fin, fin1, harmeet, - jason, + jason@sartography.com, sasha, manuchehr, lead, diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/user.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/user.py index a690a160..48e30eed 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/user.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/user.py @@ -105,7 +105,7 @@ def verify_token( ) from e if ( - user_info is not None and "error" not in user_info + user_info is not None and "error" not in user_info and 'iss' in user_info ): # not sure what to test yet user_model = ( UserModel.query.filter(UserModel.service == user_info["iss"]) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/add_permission.py b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/add_permission.py index 806fd991..de78b524 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/add_permission.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/add_permission.py @@ -14,6 +14,10 @@ from spiffworkflow_backend.services.group_service import GroupService class AddPermission(Script): """AddUserToGroup.""" + @staticmethod + def requires_privileged_permissions() -> bool: + return True + def get_description(self) -> str: """Get_description.""" return """Add a permission to a group. ex: add_permission("read", "test/*", "Editors") """ diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/script.py b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/script.py index b744694a..0da73764 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/script.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/script.py @@ -1,5 +1,7 @@ """Script.""" from __future__ import annotations +from spiffworkflow_backend.models.process_instance import ProcessInstanceModel, ProcessInstanceNotFoundError +from spiffworkflow_backend.services.authorization_service import AuthorizationService import importlib import os @@ -20,6 +22,10 @@ from spiffworkflow_backend.models.script_attributes_context import ( SCRIPT_SUB_CLASSES = None +class ScriptUnauthorizedForUserError(Exception): + pass + + class Script: """Provides an abstract class that defines how scripts should work, this must be extended in all Script Tasks.""" @@ -43,6 +49,10 @@ class Script: + "does not properly implement the run function.", ) + @staticmethod + def requires_privileged_permissions() -> bool: + return True + @staticmethod def generate_augmented_list( script_attributes_context: ScriptAttributesContext, @@ -71,18 +81,41 @@ class Script: that we created. """ instance = subclass() - return lambda *ar, **kw: subclass.run( - instance, - script_attributes_context, - *ar, - **kw, - ) + + def run_subclass(*ar: Any, **kw: Any) -> Any: + if subclass.requires_privileged_permissions(): + script_function_name = get_script_function_name(subclass) + uri = f"/v1.0/can-run-privileged-script/{script_function_name}" + process_instance = ProcessInstanceModel.query.filter_by(id=script_attributes_context.process_instance_id).first() + if process_instance is None: + raise ProcessInstanceNotFoundError( + f"Could not find a process instance with id '{script_attributes_context.process_instance_id}' " + f"when running script '{script_function_name}'" + ) + user = process_instance.process_initiator + has_permission = AuthorizationService.user_has_permission( + user=user, permission="create", target_uri=uri + ) + if not has_permission: + raise ScriptUnauthorizedForUserError( + f"User {user.username} does not have access to run privileged script '{script_function_name}'" + ) + return subclass.run( + instance, + script_attributes_context, + *ar, + **kw, + ) + return run_subclass + + def get_script_function_name(subclass: type[Script]) -> str: + return subclass.__module__.split(".")[-1] execlist = {} subclasses = Script.get_all_subclasses() for x in range(len(subclasses)): subclass = subclasses[x] - execlist[subclass.__module__.split(".")[-1]] = make_closure( + execlist[get_script_function_name(subclass)] = make_closure( subclass, script_attributes_context=script_attributes_context ) return execlist 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 67be986e..714cd799 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_service.py @@ -224,10 +224,10 @@ class ProcessModelService(FileSystemService): new_process_model_list = [] for process_model in process_models: uri = f"/v1.0/process-instances/{process_model.id.replace('/', ':')}" - result = AuthorizationService.user_has_permission( + has_permission = AuthorizationService.user_has_permission( user=user, permission="create", target_uri=uri ) - if result: + if has_permission: new_process_model_list.append(process_model) return new_process_model_list diff --git a/spiffworkflow-backend/tests/data/script_add_permission/add_permission.bpmn b/spiffworkflow-backend/tests/data/script_add_permission/add_permission.bpmn new file mode 100644 index 00000000..73070f72 --- /dev/null +++ b/spiffworkflow-backend/tests/data/script_add_permission/add_permission.bpmn @@ -0,0 +1,39 @@ + + + + + Flow_01cweoc + + + + Flow_1xle2yo + + + + Flow_01cweoc + Flow_1xle2yo + add_permission('read', '/v1.0/test_permission_uri', "test_group") + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/scripts/test_add_permission.py b/spiffworkflow-backend/tests/spiffworkflow_backend/scripts/test_add_permission.py index cb70c4bc..a9e7bd26 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/scripts/test_add_permission.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/scripts/test_add_permission.py @@ -1,6 +1,11 @@ """Test_get_localtime.""" from flask.app import Flask +from flask_bpmn.api.api_error import ApiError +import pytest +from spiffworkflow_backend.scripts.script import ScriptUnauthorizedForUserError +from tests.spiffworkflow_backend.helpers.test_data import load_test_spec from flask.testing import FlaskClient +from spiffworkflow_backend.services.process_instance_processor import ProcessInstanceProcessor from tests.spiffworkflow_backend.helpers.base_test import BaseTest from spiffworkflow_backend.models.group import GroupModel @@ -58,3 +63,36 @@ class TestAddPermission(BaseTest): assert group is not None assert permission_target is not None assert len(permission_assignments) == 1 + + def test_add_permission_script_through_bpmn( + self, + app: Flask, + client: FlaskClient, + with_db_and_bpmn_file_cleanup: None, + ) -> None: + basic_user = self.find_or_create_user("basic_user") + privileged_user = self.find_or_create_user("privileged_user") + self.add_permissions_to_user( + privileged_user, + target_uri="/v1.0/can-run-privileged-script/*", + permission_names=["create"], + ) + process_model = load_test_spec( + process_model_id="add_permission", + process_model_source_directory="script_add_permission", + ) + process_instance = self.create_process_instance_from_process_model( + process_model=process_model, user=basic_user + ) + processor = ProcessInstanceProcessor(process_instance) + + with pytest.raises(ApiError) as exception: + processor.do_engine_steps(save=True) + assert "ScriptUnauthorizedForUserError" in str(exception) + + process_instance = self.create_process_instance_from_process_model( + process_model=process_model, user=privileged_user + ) + processor = ProcessInstanceProcessor(process_instance) + processor.do_engine_steps(save=True) + assert process_instance.status == "complete" diff --git a/spiffworkflow-frontend/src/routes/ProcessModelEditDiagram.tsx b/spiffworkflow-frontend/src/routes/ProcessModelEditDiagram.tsx index 563d7303..ad99ee81 100644 --- a/spiffworkflow-frontend/src/routes/ProcessModelEditDiagram.tsx +++ b/spiffworkflow-frontend/src/routes/ProcessModelEditDiagram.tsx @@ -5,7 +5,6 @@ import { useParams, useSearchParams, } from 'react-router-dom'; -// @ts-ignore import { Button, Modal, @@ -15,6 +14,7 @@ import { Tab, TabPanels, TabPanel, +// @ts-ignore } from '@carbon/react'; import Row from 'react-bootstrap/Row'; import Col from 'react-bootstrap/Col';