From 1627f5dd0727251a6c5992fef1a3f2f52045ae09 Mon Sep 17 00:00:00 2001 From: jasquat <2487833+jasquat@users.noreply.github.com> Date: Thu, 30 Nov 2023 13:51:01 -0500 Subject: [PATCH] bugfix/guest-login-multiple-auths (#782) * This fixes guest login with using multiple auths, removes empty items from ApiError, and raises if redirect_url given to login does not match expected frontend host w/ burnettk * get body for debug * try to get the logs from the correct place to upload w/ burnettk * mock the openid call instead of actually calling it w/ burnettk --------- Co-authored-by: jasquat Co-authored-by: burnettk --- .github/workflows/tests.yml | 3 +- .../bin/get_bpmn_json_for_process_instance | 3 +- .../src/spiffworkflow_backend/api.yml | 31 +++++++++++ .../exceptions/api_error.py | 53 +++++++++++-------- .../spiffworkflow_backend/exceptions/error.py | 4 ++ .../routes/authentication_controller.py | 7 +++ .../routes/tasks_controller.py | 10 ++++ .../services/acceptance_test_fixtures.py | 2 +- .../services/authentication_service.py | 11 ++-- .../services/authorization_service.py | 21 ++++---- .../services/background_processing_service.py | 1 + .../services/custom_parser.py | 1 + .../services/data_setup_service.py | 1 + .../services/error_handling_service.py | 1 + .../services/file_system_service.py | 1 + .../services/git_service.py | 1 + .../services/jinja_service.py | 1 + .../services/message_service.py | 1 + .../services/oauth_service.py | 1 + .../services/process_caller_service.py | 3 +- .../services/process_instance_lock_service.py | 5 +- .../services/process_instance_processor.py | 3 +- .../process_instance_report_service.py | 13 ++--- .../services/process_instance_service.py | 1 + .../services/process_instance_tmp_service.py | 1 + .../services/process_model_service.py | 1 + .../process_model_test_runner_service.py | 1 + .../services/reference_cache_service.py | 3 +- .../services/script_unit_test_runner.py | 1 + .../services/secret_service.py | 1 + .../services/service_task_service.py | 3 +- .../services/spec_file_service.py | 1 + .../services/task_service.py | 1 + .../services/user_service.py | 3 +- .../services/workflow_execution_service.py | 1 + .../services/workflow_service.py | 1 + .../integration/test_authentication.py | 41 ++++++++++++++ .../integration/test_tasks_controller.py | 2 +- spiffworkflow-frontend/src/helpers.tsx | 10 ++++ spiffworkflow-frontend/src/routes/Login.tsx | 52 ++++++++++-------- .../src/services/UserService.ts | 10 +--- 41 files changed, 226 insertions(+), 86 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index db4f37f31..bec08bcbd 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -74,7 +74,6 @@ jobs: env: FLASK_SESSION_SECRET_KEY: super_secret_key FORCE_COLOR: "1" - NOXSESSION: ${{ matrix.session }} PRE_COMMIT_COLOR: "always" SPIFFWORKFLOW_BACKEND_DATABASE_PASSWORD: password SPIFFWORKFLOW_BACKEND_DATABASE_TYPE: ${{ matrix.database }} @@ -171,7 +170,7 @@ jobs: uses: "actions/upload-artifact@v3" with: name: logs-${{matrix.python}}-${{matrix.os}}-${{matrix.database}} - path: "./log/*.log" + path: "./spiffworkflow-backend/log/*.log" # burnettk created an account at https://app.snyk.io/org/kevin-jfx # and added his SNYK_TOKEN secret under the spiff-arena repo. diff --git a/spiffworkflow-backend/bin/get_bpmn_json_for_process_instance b/spiffworkflow-backend/bin/get_bpmn_json_for_process_instance index 15b2ab873..63697a25e 100755 --- a/spiffworkflow-backend/bin/get_bpmn_json_for_process_instance +++ b/spiffworkflow-backend/bin/get_bpmn_json_for_process_instance @@ -24,8 +24,9 @@ def main(process_instance_id: str) -> None: if not process_instance: raise Exception(f"Could not find a process instance with id: {process_instance_id}") + bpmn_process_dict = ProcessInstanceProcessor._get_full_bpmn_process_dict(process_instance, {}) with open(file_path, "w", encoding="utf-8") as f: - f.write(json.dumps(ProcessInstanceProcessor._get_full_bpmn_json(process_instance))) + f.write(json.dumps(bpmn_process_dict, indent=2)) print(f"Saved to {file_path}") diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/api.yml b/spiffworkflow-backend/src/spiffworkflow_backend/api.yml index 045f451c5..c64072006 100755 --- a/spiffworkflow-backend/src/spiffworkflow_backend/api.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/api.yml @@ -2285,6 +2285,33 @@ paths: schema: $ref: "#/components/schemas/OkTrue" + /tasks/{process_instance_id}/{task_guid}/allows-guest: + parameters: + - name: task_guid + in: path + required: true + description: The unique id of an existing process group. + schema: + type: string + - name: process_instance_id + in: path + required: true + description: The unique id of an existing process instance. + schema: + type: integer + get: + tags: + - Tasks + operationId: spiffworkflow_backend.routes.tasks_controller.task_allows_guest + summary: Gets checks if the given task allows guest login + responses: + "200": + description: Whether the task can be completed by a guest + content: + application/json: + schema: + $ref: "#/components/schemas/TaskAllowsGuest" + # NOTE: this should probably be /process-instances instead /tasks/{process_instance_id}/send-user-signal-event: parameters: @@ -3013,6 +3040,10 @@ components: documentation: "# Heading 1\n\nMarkdown documentation text goes here" type: form state: ready + TaskAllowsGuest: + properties: + allows_guest: + type: boolean Task: properties: id: diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py b/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py index c610c808a..b82f7ee77 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py @@ -3,7 +3,6 @@ from __future__ import annotations import json from dataclasses import dataclass -from dataclasses import field from typing import Any import flask.wrappers @@ -38,18 +37,18 @@ class ApiError(Exception): error_code: str message: str - error_line: str | None = "" - error_type: str | None = "" - file_name: str | None = "" - line_number: int | None = 0 - offset: int | None = 0 + error_line: str | None = None + error_type: str | None = None + file_name: str | None = None + line_number: int | None = None + offset: int | None = None sentry_link: str | None = None status_code: int | None = 400 - tag: str | None = "" - task_data: dict | str | None = field(default_factory=dict) - task_id: str | None = "" - task_name: str | None = "" - task_trace: list | None = field(default_factory=list) + tag: str | None = None + task_data: dict | str | None = None + task_id: str | None = None + task_name: str | None = None + task_trace: list | None = None # these are useful if the error response cannot be json but has to be something else # such as returning content type 'text/event-stream' for the interstitial page @@ -67,6 +66,14 @@ class ApiError(Exception): msg += f"In file {self.file_name}. " return msg + def serialized(self) -> dict[str, Any]: + initial_dict = self.__dict__ + return_dict = {} + for key, value in initial_dict.items(): + if value is not None and value != "": + return_dict[key] = value + return return_dict + @classmethod def from_task( cls, @@ -74,17 +81,17 @@ class ApiError(Exception): message: str, task: Task, status_code: int = 400, - line_number: int = 0, - offset: int = 0, - error_type: str = "", - error_line: str = "", + line_number: int | None = None, + offset: int | None = None, + error_type: str | None = None, + error_line: str | None = None, task_trace: list | None = None, ) -> ApiError: """Constructs an API Error with details pulled from the current task.""" instance = cls(error_code, message, status_code=status_code) - instance.task_id = task.task_spec.name or "" - instance.task_name = task.task_spec.description or "" - instance.file_name = task.workflow.spec.file or "" + instance.task_id = task.task_spec.name + instance.task_name = task.task_spec.description + instance.file_name = task.workflow.spec.file instance.line_number = line_number instance.offset = offset instance.error_type = error_type @@ -110,17 +117,17 @@ class ApiError(Exception): message: str, task_model: TaskModel, status_code: int | None = 400, - line_number: int | None = 0, - offset: int | None = 0, - error_type: str | None = "", - error_line: str | None = "", + line_number: int | None = None, + offset: int | None = None, + error_type: str | None = None, + error_line: str | None = None, task_trace: list | None = None, ) -> ApiError: """Constructs an API Error with details pulled from the current task model.""" instance = cls(error_code, message, status_code=status_code) task_definition = task_model.task_definition instance.task_id = task_definition.bpmn_identifier - instance.task_name = task_definition.bpmn_name or "" + instance.task_name = task_definition.bpmn_name instance.line_number = line_number instance.offset = offset instance.error_type = error_type diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/error.py b/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/error.py index 43289cc11..7a7fdaad6 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/error.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/error.py @@ -51,3 +51,7 @@ class UserDoesNotHaveAccessToTaskError(Exception): class InvalidPermissionError(Exception): pass + + +class InvalidRedirectUrlError(Exception): + pass diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/authentication_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/authentication_controller.py index efa9fbad3..3a1e232dd 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/authentication_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/authentication_controller.py @@ -15,6 +15,7 @@ from flask import request from werkzeug.wrappers import Response from spiffworkflow_backend.exceptions.api_error import ApiError +from spiffworkflow_backend.exceptions.error import InvalidRedirectUrlError from spiffworkflow_backend.exceptions.error import MissingAccessTokenError from spiffworkflow_backend.exceptions.error import TokenExpiredError from spiffworkflow_backend.helpers.api_version import V1_API_PATH_PREFIX @@ -99,6 +100,12 @@ def login( process_instance_id: int | None = None, task_guid: str | None = None, ) -> Response: + frontend_url = str(current_app.config.get("SPIFFWORKFLOW_BACKEND_URL_FOR_FRONTEND")) + if not redirect_url.startswith(frontend_url): + raise InvalidRedirectUrlError( + f"Invalid redirect url was given: '{redirect_url}'. It must match the domain the frontend is running on." + ) + if current_app.config.get("SPIFFWORKFLOW_BACKEND_AUTHENTICATION_DISABLED"): AuthenticationService.create_guest_token( username=SPIFF_NO_AUTH_USER, diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py index 583e2d3b8..479f7364d 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py @@ -80,6 +80,16 @@ class ReactJsonSchemaSelectOption(TypedDict): enum: list[str] +def task_allows_guest( + process_instance_id: int, + task_guid: str, +) -> flask.wrappers.Response: + allows_guest = False + if process_instance_id and task_guid and TaskModel.task_guid_allows_guest(task_guid, process_instance_id): + allows_guest = True + return make_response(jsonify({"allows_guest": allows_guest}), 200) + + # this is currently not used by the Frontend def task_list_my_tasks( process_instance_id: int | None = None, page: int = 1, per_page: int = 100 diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/acceptance_test_fixtures.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/acceptance_test_fixtures.py index 70f883b69..56f160afc 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/acceptance_test_fixtures.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/acceptance_test_fixtures.py @@ -1,11 +1,11 @@ import time from flask import current_app + from spiffworkflow_backend.models.db import db from spiffworkflow_backend.models.process_instance import ProcessInstanceModel from spiffworkflow_backend.models.process_instance import ProcessInstanceStatus from spiffworkflow_backend.services.process_instance_service import ProcessInstanceService - from tests.spiffworkflow_backend.helpers.base_test import BaseTest diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/authentication_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/authentication_service.py index ea70cafdc..796dbe514 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/authentication_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/authentication_service.py @@ -13,6 +13,8 @@ from flask import current_app from flask import g from flask import redirect from flask import request +from werkzeug.wrappers import Response + from spiffworkflow_backend.config import HTTP_REQUEST_TIMEOUT_SECONDS from spiffworkflow_backend.exceptions.error import OpenIdConnectionError from spiffworkflow_backend.exceptions.error import RefreshTokenStorageError @@ -23,7 +25,6 @@ from spiffworkflow_backend.models.db import db from spiffworkflow_backend.models.refresh_token import RefreshTokenModel from spiffworkflow_backend.services.authorization_service import AuthorizationService from spiffworkflow_backend.services.user_service import UserService -from werkzeug.wrappers import Response class AuthenticationProviderTypes(enum.Enum): @@ -118,7 +119,9 @@ class AuthenticationService: if redirect_url is None: redirect_url = f"{self.get_backend_url()}/v1.0/logout_return" request_url = ( - self.open_id_endpoint_for_name("end_session_endpoint", authentication_identifier=authentication_identifier) + self.__class__.open_id_endpoint_for_name( + "end_session_endpoint", authentication_identifier=authentication_identifier + ) + f"?post_logout_redirect_uri={redirect_url}&" + f"id_token_hint={id_token}" ) @@ -137,7 +140,7 @@ class AuthenticationService: ) -> str: return_redirect_url = f"{self.get_backend_url()}{redirect_url}" login_redirect_url = ( - self.open_id_endpoint_for_name( + self.__class__.open_id_endpoint_for_name( "authorization_endpoint", authentication_identifier=authentication_identifier ) + f"?state={state}&" @@ -146,7 +149,6 @@ class AuthenticationService: + "scope=openid profile email&" + f"redirect_uri={return_redirect_url}" ) - print(f"login_redirect_url: {login_redirect_url}") return login_redirect_url def get_auth_token_object( @@ -173,7 +175,6 @@ class AuthenticationService: response = requests.post(request_url, data=data, headers=headers, timeout=HTTP_REQUEST_TIMEOUT_SECONDS) auth_token_object: dict = json.loads(response.text) - print(f"auth_token_object: {auth_token_object}") return auth_token_object @classmethod diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py index 225c11492..089997d3b 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py @@ -7,6 +7,11 @@ from flask import current_app from flask import g from flask import request from flask import scaffold +from sqlalchemy import and_ +from sqlalchemy import func +from sqlalchemy import literal +from sqlalchemy import or_ + from spiffworkflow_backend.exceptions.error import HumanTaskAlreadyCompletedError from spiffworkflow_backend.exceptions.error import HumanTaskNotFoundError from spiffworkflow_backend.exceptions.error import InvalidPermissionError @@ -34,10 +39,6 @@ from spiffworkflow_backend.models.user_group_assignment import UserGroupAssignme from spiffworkflow_backend.models.user_group_assignment_waiting import UserGroupAssignmentWaitingModel from spiffworkflow_backend.routes.openid_blueprint import openid_blueprint from spiffworkflow_backend.services.user_service import UserService -from sqlalchemy import and_ -from sqlalchemy import func -from sqlalchemy import literal -from sqlalchemy import or_ @dataclass @@ -228,11 +229,15 @@ class AuthorizationService: def should_disable_auth_for_request(cls) -> bool: swagger_functions = ["get_json_spec"] authentication_exclusion_list = [ - "status", - "test_raise_error", "authentication_begin", "authentication_callback", + "authentication_options", "github_webhook_receive", + "prometheus_metrics", + "status", + "task_allows_guest", + "test_raise_error", + "url_info", ] if request.method == "OPTIONS": return True @@ -250,10 +255,6 @@ class AuthorizationService: api_view_function and api_view_function.__name__.startswith("login") or api_view_function.__name__.startswith("logout") - or api_view_function.__name__.startswith("authentication_options") - or api_view_function.__name__.startswith("prom") - or api_view_function.__name__ == "url_info" - or api_view_function.__name__.startswith("metric") or api_view_function.__name__.startswith("console_ui_") or api_view_function.__name__ in authentication_exclusion_list or api_view_function.__name__ in swagger_functions diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/background_processing_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/background_processing_service.py index 3b78608c7..f8676fdaf 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/background_processing_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/background_processing_service.py @@ -1,4 +1,5 @@ import flask + from spiffworkflow_backend.models.process_instance import ProcessInstanceStatus from spiffworkflow_backend.services.message_service import MessageService from spiffworkflow_backend.services.process_instance_lock_service import ProcessInstanceLockService diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/custom_parser.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/custom_parser.py index d4cd4b474..13f2f4402 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/custom_parser.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/custom_parser.py @@ -4,6 +4,7 @@ from SpiffWorkflow.bpmn.parser.BpmnParser import full_tag # type: ignore from SpiffWorkflow.dmn.parser.BpmnDmnParser import BpmnDmnParser # type: ignore from SpiffWorkflow.spiff.parser.process import SpiffBpmnParser # type: ignore from SpiffWorkflow.spiff.parser.task_spec import ServiceTaskParser # type: ignore + from spiffworkflow_backend.data_stores import register_data_store_classes from spiffworkflow_backend.services.service_task_service import CustomServiceTask from spiffworkflow_backend.specs.start_event import StartEvent diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/data_setup_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/data_setup_service.py index 075840233..011c727f2 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/data_setup_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/data_setup_service.py @@ -1,6 +1,7 @@ import os from flask import current_app + from spiffworkflow_backend.models.db import db from spiffworkflow_backend.models.reference_cache import ReferenceCacheModel from spiffworkflow_backend.services.file_system_service import FileSystemService diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/error_handling_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/error_handling_service.py index e877a631e..55f02d6c7 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/error_handling_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/error_handling_service.py @@ -1,5 +1,6 @@ from flask import current_app from flask import g + from spiffworkflow_backend.models.db import db from spiffworkflow_backend.models.process_instance import ProcessInstanceModel from spiffworkflow_backend.models.process_instance import ProcessInstanceStatus diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/file_system_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/file_system_service.py index ad8370696..2eb0c044b 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/file_system_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/file_system_service.py @@ -7,6 +7,7 @@ from typing import Any import pytz from flask import current_app + from spiffworkflow_backend.exceptions.api_error import ApiError from spiffworkflow_backend.models.file import CONTENT_TYPES from spiffworkflow_backend.models.file import File diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py index 455bef335..e5143bf86 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py @@ -6,6 +6,7 @@ import uuid from flask import current_app from flask import g + from spiffworkflow_backend.config import ConfigurationError from spiffworkflow_backend.models.process_model import ProcessModelInfo from spiffworkflow_backend.services.data_setup_service import DataSetupService diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/jinja_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/jinja_service.py index 94a6955ab..fc62c5382 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/jinja_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/jinja_service.py @@ -5,6 +5,7 @@ import jinja2 from jinja2 import TemplateSyntaxError from SpiffWorkflow.bpmn.exceptions import WorkflowTaskException # type: ignore from SpiffWorkflow.task import Task as SpiffTask # type: ignore + from spiffworkflow_backend.exceptions.api_error import ApiError from spiffworkflow_backend.models.task import TaskModel # noqa: F401 from spiffworkflow_backend.services.task_service import TaskModelError diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/message_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/message_service.py index 685f1f4a8..4cbd3b0f0 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/message_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/message_service.py @@ -1,6 +1,7 @@ from SpiffWorkflow.bpmn.event import BpmnEvent # type: ignore from SpiffWorkflow.bpmn.specs.event_definitions.message import CorrelationProperty # type: ignore from SpiffWorkflow.spiff.specs.event_definitions import MessageEventDefinition # type: ignore + from spiffworkflow_backend.models.db import db from spiffworkflow_backend.models.message_instance import MessageInstanceModel from spiffworkflow_backend.models.message_instance import MessageStatuses diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/oauth_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/oauth_service.py index a4d0827b9..4016bc8ec 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/oauth_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/oauth_service.py @@ -5,6 +5,7 @@ from typing import Any from flask import Flask from flask import session from flask_oauthlib.client import OAuth # type: ignore + from spiffworkflow_backend.exceptions.api_error import ApiError from spiffworkflow_backend.services.configuration_service import ConfigurationService from spiffworkflow_backend.services.secret_service import SecretService diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_caller_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_caller_service.py index 73a8c2a03..69bc8f069 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_caller_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_caller_service.py @@ -1,6 +1,7 @@ +from sqlalchemy import or_ + from spiffworkflow_backend.models.db import db from spiffworkflow_backend.models.process_caller import ProcessCallerCacheModel -from sqlalchemy import or_ class ProcessCallerService: diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_lock_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_lock_service.py index 1687c7f58..fec6650cc 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_lock_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_lock_service.py @@ -3,11 +3,12 @@ import time from typing import Any from flask import current_app -from spiffworkflow_backend.models.db import db -from spiffworkflow_backend.models.process_instance_queue import ProcessInstanceQueueModel from sqlalchemy import and_ from sqlalchemy import or_ +from spiffworkflow_backend.models.db import db +from spiffworkflow_backend.models.process_instance_queue import ProcessInstanceQueueModel + class ExpectedLockNotFoundError(Exception): pass 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 edb9cf21d..69b8037fb 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py @@ -50,6 +50,8 @@ from SpiffWorkflow.task import Task as SpiffTask # type: ignore from SpiffWorkflow.util.deep_merge import DeepMerge # type: ignore from SpiffWorkflow.util.task import TaskIterator # type: ignore from SpiffWorkflow.util.task import TaskState +from sqlalchemy import and_ + from spiffworkflow_backend.data_stores.json import JSONDataStore from spiffworkflow_backend.data_stores.json import JSONDataStoreConverter from spiffworkflow_backend.data_stores.json import JSONFileDataStore @@ -102,7 +104,6 @@ from spiffworkflow_backend.services.workflow_execution_service import TaskModelS from spiffworkflow_backend.services.workflow_execution_service import WorkflowExecutionService from spiffworkflow_backend.services.workflow_execution_service import execution_strategy_named from spiffworkflow_backend.specs.start_event import StartEvent -from sqlalchemy import and_ SPIFF_CONFIG[StandardLoopTask] = StandardLoopTaskConverter diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_report_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_report_service.py index d1b55283f..87952d9fc 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_report_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_report_service.py @@ -6,6 +6,13 @@ from typing import Any import sqlalchemy from flask import current_app from flask_sqlalchemy.query import Query +from sqlalchemy import and_ +from sqlalchemy import func +from sqlalchemy import or_ +from sqlalchemy.orm import aliased +from sqlalchemy.orm import selectinload +from sqlalchemy.orm.util import AliasedClass + from spiffworkflow_backend.exceptions.api_error import ApiError from spiffworkflow_backend.models.db import SpiffworkflowBaseDBModel from spiffworkflow_backend.models.db import db @@ -22,12 +29,6 @@ from spiffworkflow_backend.models.task import TaskModel # noqa: F401 from spiffworkflow_backend.models.user import UserModel from spiffworkflow_backend.models.user_group_assignment import UserGroupAssignmentModel from spiffworkflow_backend.services.process_model_service import ProcessModelService -from sqlalchemy import and_ -from sqlalchemy import func -from sqlalchemy import or_ -from sqlalchemy.orm import aliased -from sqlalchemy.orm import selectinload -from sqlalchemy.orm.util import AliasedClass class ProcessInstanceReportNotFoundError(Exception): diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py index ca4335152..2a4f3663b 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py @@ -16,6 +16,7 @@ from SpiffWorkflow.bpmn.specs.defaults import BoundaryEvent # type: ignore from SpiffWorkflow.bpmn.specs.event_definitions.timer import TimerEventDefinition # type: ignore from SpiffWorkflow.task import Task as SpiffTask # type: ignore from SpiffWorkflow.util.task import TaskState # type: ignore + from spiffworkflow_backend import db from spiffworkflow_backend.data_migrations.process_instance_migrator import ProcessInstanceMigrator from spiffworkflow_backend.exceptions.api_error import ApiError diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_tmp_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_tmp_service.py index b8c32c90f..b59dc3ba2 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_tmp_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_tmp_service.py @@ -3,6 +3,7 @@ import traceback from flask import g from SpiffWorkflow.bpmn.exceptions import WorkflowTaskException # type: ignore + from spiffworkflow_backend.models.db import db from spiffworkflow_backend.models.process_instance import ProcessInstanceModel from spiffworkflow_backend.models.process_instance_error_detail import ProcessInstanceErrorDetailModel 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 d617c744c..e5b8b7ebb 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_service.py @@ -6,6 +6,7 @@ from json import JSONDecodeError from typing import TypeVar from flask import current_app + from spiffworkflow_backend.exceptions.api_error import ApiError from spiffworkflow_backend.exceptions.process_entity_not_found_error import ProcessEntityNotFoundError from spiffworkflow_backend.interfaces import ProcessGroupLite diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_test_runner_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_test_runner_service.py index 58dd8f850..9df1b706c 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_test_runner_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_test_runner_service.py @@ -11,6 +11,7 @@ from SpiffWorkflow.bpmn.exceptions import WorkflowTaskException # type: ignore from SpiffWorkflow.bpmn.workflow import BpmnWorkflow # type: ignore from SpiffWorkflow.task import Task as SpiffTask # type: ignore from SpiffWorkflow.util.task import TaskState # type: ignore + from spiffworkflow_backend.services.custom_parser import MyCustomParser diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/reference_cache_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/reference_cache_service.py index a72894ac9..a2987c7c4 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/reference_cache_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/reference_cache_service.py @@ -1,9 +1,10 @@ import os +from sqlalchemy import insert + from spiffworkflow_backend.models.cache_generation import CacheGenerationModel from spiffworkflow_backend.models.db import db from spiffworkflow_backend.models.reference_cache import ReferenceCacheModel -from sqlalchemy import insert class ReferenceCacheService: diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/script_unit_test_runner.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/script_unit_test_runner.py index 1569f7481..1964cef9a 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/script_unit_test_runner.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/script_unit_test_runner.py @@ -6,6 +6,7 @@ from typing import Any from SpiffWorkflow.bpmn.exceptions import WorkflowTaskException # type: ignore from SpiffWorkflow.task import Task as SpiffTask # type: ignore + from spiffworkflow_backend.services.process_instance_processor import CustomBpmnScriptEngine PythonScriptContext = dict[str, Any] diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/secret_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/secret_service.py index 6afdff527..f490ba19c 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/secret_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/secret_service.py @@ -2,6 +2,7 @@ import re import sentry_sdk from flask import current_app + from spiffworkflow_backend.exceptions.api_error import ApiError from spiffworkflow_backend.models.db import db from spiffworkflow_backend.models.secret_model import SecretModel 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 027038d59..f5c76f5fd 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/service_task_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/service_task_service.py @@ -14,12 +14,13 @@ from SpiffWorkflow.spiff.specs.event_definitions import ErrorEventDefinition # from SpiffWorkflow.spiff.specs.event_definitions import EscalationEventDefinition from SpiffWorkflow.task import Task as SpiffTask # type: ignore from SpiffWorkflow.util.task import TaskState # type: ignore +from spiffworkflow_connector_command.command_interface import CommandErrorDict + 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 from spiffworkflow_backend.services.user_service import UserService -from spiffworkflow_connector_command.command_interface import CommandErrorDict class ConnectorProxyError(Exception): diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/spec_file_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/spec_file_service.py index 1bb426107..c3cdddb77 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/spec_file_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/spec_file_service.py @@ -4,6 +4,7 @@ from datetime import datetime from lxml import etree # type: ignore from SpiffWorkflow.bpmn.parser.BpmnParser import BpmnValidator # type: ignore + from spiffworkflow_backend.exceptions.error import NotAuthorizedError from spiffworkflow_backend.models.correlation_property_cache import CorrelationPropertyCache from spiffworkflow_backend.models.db import db diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py index bca92a556..7b8c173d0 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py @@ -10,6 +10,7 @@ from SpiffWorkflow.bpmn.workflow import BpmnWorkflow # type: ignore from SpiffWorkflow.exceptions import WorkflowException # type: ignore from SpiffWorkflow.task import Task as SpiffTask # type: ignore from SpiffWorkflow.util.task import TaskState # type: ignore + from spiffworkflow_backend.models.bpmn_process import BpmnProcessModel from spiffworkflow_backend.models.bpmn_process import BpmnProcessNotFoundError from spiffworkflow_backend.models.bpmn_process_definition import BpmnProcessDefinitionModel diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/user_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/user_service.py index 543d0ea9a..d185c9ede 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/user_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/user_service.py @@ -3,6 +3,8 @@ from typing import Any from flask import current_app from flask import g +from sqlalchemy import and_ + from spiffworkflow_backend.exceptions.api_error import ApiError from spiffworkflow_backend.interfaces import UserToGroupDict from spiffworkflow_backend.models.db import db @@ -17,7 +19,6 @@ from spiffworkflow_backend.models.user import UserModel from spiffworkflow_backend.models.user_group_assignment import UserGroupAssignmentModel from spiffworkflow_backend.models.user_group_assignment import UserGroupAssignmentNotFoundError from spiffworkflow_backend.models.user_group_assignment_waiting import UserGroupAssignmentWaitingModel -from sqlalchemy import and_ class UserService: diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/workflow_execution_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/workflow_execution_service.py index 1c75d5717..a93101b4c 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/workflow_execution_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/workflow_execution_service.py @@ -17,6 +17,7 @@ from SpiffWorkflow.bpmn.workflow import BpmnWorkflow # type: ignore from SpiffWorkflow.exceptions import SpiffWorkflowException # type: ignore from SpiffWorkflow.task import Task as SpiffTask # type: ignore from SpiffWorkflow.util.task import TaskState # type: ignore + from spiffworkflow_backend.exceptions.api_error import ApiError from spiffworkflow_backend.models.db import db from spiffworkflow_backend.models.message_instance import MessageInstanceModel diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/workflow_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/workflow_service.py index 78242f2bd..a5539816c 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/workflow_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/workflow_service.py @@ -3,6 +3,7 @@ from datetime import datetime from SpiffWorkflow.bpmn.workflow import BpmnWorkflow # type: ignore from SpiffWorkflow.task import Task as SpiffTask # type: ignore from SpiffWorkflow.util.task import TaskState # type: ignore + from spiffworkflow_backend.specs.start_event import StartConfiguration from spiffworkflow_backend.specs.start_event import StartEvent diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_authentication.py b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_authentication.py index 46719c587..a362fea09 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_authentication.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_authentication.py @@ -1,6 +1,8 @@ import ast import base64 +import re import time +from typing import Any from flask.app import Flask from flask.testing import FlaskClient @@ -116,3 +118,42 @@ class TestAuthentication(BaseTest): UserService.get_permission_targets_for_user(service_account.user, check_groups=False) ) assert service_account_permissions_before == service_account_permissions_after + + def test_can_login_with_valid_user( + self, + app: Flask, + mocker: Any, + client: FlaskClient, + with_db_and_bpmn_file_cleanup: None, + ) -> None: + redirect_uri = f"{app.config['SPIFFWORKFLOW_BACKEND_URL_FOR_FRONTEND']}/test-redirect-dne" + auth_uri = app.config["SPIFFWORKFLOW_BACKEND_AUTH_CONFIGS"][0]["uri"] + login_return_uri = f"{app.config['SPIFFWORKFLOW_BACKEND_URL']}/v1.0/login_return" + + class_method_mock = mocker.patch( + "spiffworkflow_backend.services.authentication_service.AuthenticationService.open_id_endpoint_for_name", + return_value=auth_uri, + ) + + response = client.get( + f"/v1.0/login?redirect_url={redirect_uri}&authentication_identifier=default", + ) + + assert class_method_mock.call_count == 1 + assert response.status_code == 302 + assert response.location.startswith(auth_uri) + assert re.search(r"\bredirect_uri=" + re.escape(login_return_uri), response.location) is not None + + def test_raises_error_if_invalid_redirect_url( + self, + app: Flask, + client: FlaskClient, + with_db_and_bpmn_file_cleanup: None, + ) -> None: + redirect_url = "http://www.bad_url.com/test-redirect-dne" + response = client.get( + f"/v1.0/login?redirect_url={redirect_url}&authentication_identifier=DOES_NOT_MATTER", + ) + assert response.status_code == 500 + assert response.json is not None + assert response.json["message"].startswith("InvalidRedirectUrlError:") diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_tasks_controller.py b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_tasks_controller.py index 249708a98..cf7485d12 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_tasks_controller.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_tasks_controller.py @@ -486,7 +486,7 @@ class TestTasksController(BaseTest): task_guid = task_model.guid # log in a guest user to complete the tasks - redirect_url = "/test-redirect-dne" + redirect_url = f"{app.config['SPIFFWORKFLOW_BACKEND_URL_FOR_FRONTEND']}/test-redirect-dne" response = client.get( f"/v1.0/login?process_instance_id={process_instance_id}&task_guid={task_guid}&redirect_url={redirect_url}&authentication_identifier=DOES_NOT_MATTER", ) diff --git a/spiffworkflow-frontend/src/helpers.tsx b/spiffworkflow-frontend/src/helpers.tsx index d2dc0d5f4..5190c2265 100644 --- a/spiffworkflow-frontend/src/helpers.tsx +++ b/spiffworkflow-frontend/src/helpers.tsx @@ -272,3 +272,13 @@ export const getLastMilestoneFromProcessInstance = ( } return [valueToUse, truncatedValue]; }; + +export const parseTaskShowUrl = (url: string) => { + const path = pathFromFullUrl(url); + + // expected url pattern: + // /tasks/[process_instance_id]/[task_guid] + return path.match( + /^\/tasks\/(\d+)\/([0-9a-z]{8}-([0-9a-z]{4}-){3}[0-9a-z]{12})$/ + ); +}; diff --git a/spiffworkflow-frontend/src/routes/Login.tsx b/spiffworkflow-frontend/src/routes/Login.tsx index 0a1e2a617..58366c5d6 100644 --- a/spiffworkflow-frontend/src/routes/Login.tsx +++ b/spiffworkflow-frontend/src/routes/Login.tsx @@ -1,10 +1,11 @@ import { ArrowRight } from '@carbon/icons-react'; -import { useEffect, useState } from 'react'; +import { useCallback, useEffect, useState } from 'react'; import { Loading, Button, Grid, Column } from '@carbon/react'; import { useNavigate, useSearchParams } from 'react-router-dom'; import { AuthenticationOption } from '../interfaces'; import HttpService from '../services/HttpService'; import UserService from '../services/UserService'; +import { parseTaskShowUrl } from '../helpers'; export default function Login() { const navigate = useNavigate(); @@ -12,20 +13,34 @@ export default function Login() { const [authenticationOptions, setAuthenticationOptions] = useState< AuthenticationOption[] | null >(null); + const [allowsGuestLogin, setAllowsGuestLogin] = useState( + null + ); + + const originalUrl = searchParams.get('original_url'); + const getOriginalUrl = useCallback(() => { + if (originalUrl === '/login') { + return '/'; + } + return originalUrl; + }, [originalUrl]); + useEffect(() => { HttpService.makeCallToBackend({ path: '/authentication-options', successCallback: setAuthenticationOptions, }); - }, []); - - const getOriginalUrl = () => { - const originalUrl = searchParams.get('original_url'); - if (originalUrl === '/login') { - return '/'; + const pathSegments = parseTaskShowUrl(getOriginalUrl() || ''); + if (pathSegments) { + HttpService.makeCallToBackend({ + path: `/tasks/${pathSegments[1]}/${pathSegments[2]}/allows-guest`, + successCallback: (result: any) => + setAllowsGuestLogin(result.allows_guest), + }); + } else { + setAllowsGuestLogin(false); } - return originalUrl; - }; + }, [getOriginalUrl]); const authenticationOptionButtons = () => { if (!authenticationOptions) { @@ -48,15 +63,6 @@ export default function Login() { return buttons; }; - const doLoginIfAppropriate = () => { - if (!authenticationOptions) { - return; - } - if (authenticationOptions.length === 1) { - UserService.doLogin(authenticationOptions[0], getOriginalUrl()); - } - }; - const getLoadingIcon = () => { const style = { margin: '50px 0 50px 50px' }; return ( @@ -98,8 +104,7 @@ export default function Login() { return null; } - if (authenticationOptions === null || authenticationOptions.length < 2) { - doLoginIfAppropriate(); + if (authenticationOptions === null) { return (
{getLoadingIcon()} @@ -107,8 +112,11 @@ export default function Login() { ); } - if (authenticationOptions !== null) { - doLoginIfAppropriate(); + if (authenticationOptions !== null && allowsGuestLogin !== null) { + if (allowsGuestLogin || authenticationOptions.length === 1) { + UserService.doLogin(authenticationOptions[0], getOriginalUrl()); + return null; + } return loginComponments(); } diff --git a/spiffworkflow-frontend/src/services/UserService.ts b/spiffworkflow-frontend/src/services/UserService.ts index a2a54fca0..31cd29650 100644 --- a/spiffworkflow-frontend/src/services/UserService.ts +++ b/spiffworkflow-frontend/src/services/UserService.ts @@ -2,7 +2,7 @@ import jwt from 'jwt-decode'; import cookie from 'cookie'; import { BACKEND_BASE_URL } from '../config'; import { AuthenticationOption } from '../interfaces'; -import { pathFromFullUrl } from '../helpers'; +import { parseTaskShowUrl } from '../helpers'; // NOTE: this currently stores the jwt token in local storage // which is considered insecure. Server set cookies seem to be considered @@ -36,13 +36,7 @@ const getCurrentLocation = (queryParams: string = window.location.search) => { const checkPathForTaskShowParams = ( redirectUrl: string = window.location.pathname ) => { - const path = pathFromFullUrl(redirectUrl); - - // expected url pattern: - // /tasks/[process_instance_id]/[task_guid] - const pathSegments = path.match( - /^\/tasks\/(\d+)\/([0-9a-z]{8}-([0-9a-z]{4}-){3}[0-9a-z]{12})$/ - ); + const pathSegments = parseTaskShowUrl(redirectUrl); if (pathSegments) { return { process_instance_id: pathSegments[1], task_guid: pathSegments[2] }; }