From 2631fa5f95d3ac6a20488950161ab4ea8816b985 Mon Sep 17 00:00:00 2001 From: jasquat <2487833+jasquat@users.noreply.github.com> Date: Fri, 9 Jun 2023 14:19:02 -0400 Subject: [PATCH] some fixes and updates to help with running an acceptance test model (#323) Co-authored-by: jasquat --- .../spiffworkflow_backend/config/__init__.py | 1 + .../services/process_instance_processor.py | 4 +- .../services/service_task_service.py | 37 +++++++++++++++---- .../unit/test_service_task_delegate.py | 20 ++++++++-- .../src/components/ProcessInstanceRun.tsx | 22 +++++++++-- 5 files changed, 70 insertions(+), 14 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/config/__init__.py b/spiffworkflow-backend/src/spiffworkflow_backend/config/__init__.py index d742c182..e1087ef4 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/config/__init__.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/__init__.py @@ -10,6 +10,7 @@ from werkzeug.utils import ImportStringError from spiffworkflow_backend.services.logging_service import setup_logger HTTP_REQUEST_TIMEOUT_SECONDS = 15 +CONNECTOR_PROXY_COMMAND_TIMEOUT = 30 class ConfigurationError(Exception): diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py index 9871c320..c491b192 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py @@ -350,7 +350,9 @@ class CustomBpmnScriptEngine(PythonScriptEngine): # type: ignore methods = self.__get_augment_methods(task) if external_methods: methods.update(external_methods) - super().execute(task, script, methods) + # do not run script if it is blank + if script: + super().execute(task, script, methods) return True except WorkflowException as e: raise e diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/service_task_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/service_task_service.py index d0161497..b5e1fd95 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/service_task_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/service_task_service.py @@ -1,10 +1,12 @@ import json +import re from typing import Any import requests import sentry_sdk from flask import current_app from flask import g +from spiffworkflow_backend.config import CONNECTOR_PROXY_COMMAND_TIMEOUT from spiffworkflow_backend.config import HTTP_REQUEST_TIMEOUT_SECONDS from spiffworkflow_backend.services.file_system_service import FileSystemService from spiffworkflow_backend.services.secret_service import SecretService @@ -21,8 +23,8 @@ def connector_proxy_url() -> Any: class ServiceTaskDelegate: - @staticmethod - def check_prefixes(value: Any) -> Any: + @classmethod + def handle_template_substitutions(cls, value: Any) -> Any: if isinstance(value, str): secret_prefix = "secret:" # noqa: S105 if value.startswith(secret_prefix): @@ -38,6 +40,24 @@ class ServiceTaskDelegate: with open(full_path) as f: return f.read() + if "SPIFF_SECRET:" in value: + spiff_secret_match = re.match(r".*SPIFF_SECRET:(?P\w+).*", value) + if spiff_secret_match is not None: + spiff_variable_name = spiff_secret_match.group("variable_name") + secret = SecretService.get_secret(spiff_variable_name) + with sentry_sdk.start_span(op="task", description="decrypt_secret"): + decrypted_value = SecretService._decrypt(secret.value) + return re.sub(r"\bSPIFF_SECRET:\w+", decrypted_value, value) + + return value + + @classmethod + def value_with_secrets_replaced(cls, value: Any) -> Any: + if isinstance(value, str): + return cls.handle_template_substitutions(value) + elif isinstance(value, dict): + for key, v in value.items(): + value[key] = cls.value_with_secrets_replaced(v) return value @staticmethod @@ -65,17 +85,17 @@ class ServiceTaskDelegate: ) return msg - @staticmethod - def call_connector(name: str, bpmn_params: Any, task_data: Any) -> str: + @classmethod + def call_connector(cls, name: str, bpmn_params: Any, task_data: Any) -> str: """Calls a connector via the configured proxy.""" call_url = f"{connector_proxy_url()}/v1/do/{name}" current_app.logger.info(f"Calling connector proxy using connector: {name}") with sentry_sdk.start_span(op="connector_by_name", description=name): with sentry_sdk.start_span(op="call-connector", description=call_url): - params = {k: ServiceTaskDelegate.check_prefixes(v["value"]) for k, v in bpmn_params.items()} + params = {k: cls.value_with_secrets_replaced(v["value"]) for k, v in bpmn_params.items()} params["spiff__task_data"] = task_data - proxied_response = requests.post(call_url, json=params, timeout=HTTP_REQUEST_TIMEOUT_SECONDS) + proxied_response = requests.post(call_url, json=params, timeout=CONNECTOR_PROXY_COMMAND_TIMEOUT) response_text = proxied_response.text json_parse_error = None @@ -98,7 +118,10 @@ class ServiceTaskDelegate: message = ServiceTaskDelegate.get_message_for_status(proxied_response.status_code) error = f"Received an unexpected response from service {name} : {message}" if "error" in parsed_response: - error += parsed_response["error"] + error_response = parsed_response["error"] + if isinstance(error_response, list | dict): + error_response = json.dumps(parsed_response["error"]) + error += error_response if json_parse_error: error += "A critical component (The connector proxy) is not responding correctly." raise ConnectorProxyError(error) diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_service_task_delegate.py b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_service_task_delegate.py index 0cae53bc..6c1b5ecd 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_service_task_delegate.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_service_task_delegate.py @@ -11,19 +11,33 @@ from tests.spiffworkflow_backend.helpers.base_test import BaseTest class TestServiceTaskDelegate(BaseTest): def test_check_prefixes_without_secret(self, app: Flask, with_db_and_bpmn_file_cleanup: None) -> None: - result = ServiceTaskDelegate.check_prefixes("hey") + result = ServiceTaskDelegate.value_with_secrets_replaced("hey") assert result == "hey" def test_check_prefixes_with_int(self, app: Flask, with_db_and_bpmn_file_cleanup: None) -> None: - result = ServiceTaskDelegate.check_prefixes(1) + result = ServiceTaskDelegate.value_with_secrets_replaced(1) assert result == 1 def test_check_prefixes_with_secret(self, app: Flask, with_db_and_bpmn_file_cleanup: None) -> None: user = self.find_or_create_user("test_user") SecretService().add_secret("hot_secret", "my_secret_value", user.id) - result = ServiceTaskDelegate.check_prefixes("secret:hot_secret") + result = ServiceTaskDelegate.value_with_secrets_replaced("secret:hot_secret") assert result == "my_secret_value" + def test_check_prefixes_with_spiff_secret(self, app: Flask, with_db_and_bpmn_file_cleanup: None) -> None: + user = self.find_or_create_user("test_user") + SecretService().add_secret("hot_secret", "my_secret_value", user.id) + result = ServiceTaskDelegate.value_with_secrets_replaced("TOKEN SPIFF_SECRET:hot_secret-haha") + assert result == "TOKEN my_secret_value-haha" + + def test_check_prefixes_with_spiff_secret_in_dict(self, app: Flask, with_db_and_bpmn_file_cleanup: None) -> None: + user = self.find_or_create_user("test_user") + SecretService().add_secret("hot_secret", "my_secret_value", user.id) + result = ServiceTaskDelegate.value_with_secrets_replaced( + {"Authorization": "TOKEN SPIFF_SECRET:hot_secret-haha"} + ) + assert result == {"Authorization": "TOKEN my_secret_value-haha"} + def test_invalid_call_returns_good_error_message(self, app: Flask, with_db_and_bpmn_file_cleanup: None) -> None: with patch("requests.post") as mock_post: mock_post.return_value.status_code = 404 diff --git a/spiffworkflow-frontend/src/components/ProcessInstanceRun.tsx b/spiffworkflow-frontend/src/components/ProcessInstanceRun.tsx index 0f61e128..af8a010d 100644 --- a/spiffworkflow-frontend/src/components/ProcessInstanceRun.tsx +++ b/spiffworkflow-frontend/src/components/ProcessInstanceRun.tsx @@ -4,6 +4,7 @@ import { // @ts-ignore } from '@carbon/react'; import { Can } from '@casl/react'; +import { useState } from 'react'; import { PermissionsToCheck, ProcessModel, @@ -78,6 +79,7 @@ export default function ProcessInstanceRun({ }: OwnProps) { const navigate = useNavigate(); const { addError, removeError } = useAPIError(); + const [disableStartButton, setDisableStartButton] = useState(false); const modifiedProcessModelId = modifyProcessIdentifierForPathParam( processModel.id ); @@ -109,20 +111,29 @@ export default function ProcessInstanceRun({ HttpService.makeCallToBackend({ path: `/process-instances/${modifiedProcessModelId}/${processInstance.id}/run`, successCallback: onProcessInstanceRun, - failureCallback: addError, + failureCallback: (result: any) => { + addError(result); + setDisableStartButton(false); + }, httpMethod: 'POST', }); }; const processInstanceCreateAndRun = () => { removeError(); + setDisableStartButton(true); HttpService.makeCallToBackend({ path: processInstanceCreatePath, successCallback: processModelRun, - failureCallback: addError, + failureCallback: (result: any) => { + addError(result); + setDisableStartButton(false); + }, httpMethod: 'POST', }); }; + + // if checkPermissions is false then assume the page using this component has already checked the permissions if (checkPermissions) { return ( @@ -130,6 +141,7 @@ export default function ProcessInstanceRun({ data-qa="start-process-instance" onClick={processInstanceCreateAndRun} className={className} + disabled={disableStartButton} > Start @@ -137,7 +149,11 @@ export default function ProcessInstanceRun({ ); } return ( - );