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 <[email protected]> Co-authored-by: burnettk <[email protected]>
sartography · Nov 30, 2023 · 68fa15d · 68fa15d
1 parent a7ab3e2
commit 68fa15d
Show file tree

Hide file tree

Showing 41 changed files with 226 additions and 86 deletions.
diff --git 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
@@ -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
@@ -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
@@ -3,7 +3,6 @@
 
 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,24 +66,32 @@ def __str__(self) -> str:
             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,
         error_code: str,
         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 @@ def from_task_model(
         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
@@ -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
@@ -15,6 +15,7 @@
 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
@@ -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
@@ -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
@@ -13,6 +13,8 @@
 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.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 @@ def logout(self, id_token: str, authentication_identifier: str, redirect_url: st
         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 @@ def get_login_redirect_url(
     ) -> 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 @@ def get_login_redirect_url(
             + "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 @@ def get_auth_token_object(
 
         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
@@ -7,6 +7,11 @@
 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_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 @@ def create_permission_for_principal(
     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 @@ def should_disable_auth_for_request(cls) -> bool:
             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
@@ -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
@@ -4,6 +4,7 @@
 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
@@ -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
@@ -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
@@ -7,6 +7,7 @@
 
 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
@@ -6,6 +6,7 @@
 
 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
@@ -5,6 +5,7 @@
 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
@@ -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
-Original file line number
+Diff line change
@@ Expand Up / @@ -51,3 +51,7 @@ class UserDoesNotHaveAccessToTaskError(Exception): @@
     class InvalidPermissionError(Exception):
         pass
+    class InvalidRedirectUrlError(Exception):
+        pass