From a387b78786f9109a56df948602fe9bbb85cdeaae Mon Sep 17 00:00:00 2001 From: jasquat Date: Thu, 20 Oct 2022 17:06:48 -0400 Subject: [PATCH 01/11] added some code to respect lanes in a process model w/ burnettk --- conftest.py | 3 + migrations/env.py | 2 + .../{cf862b761896_.py => 1e25676fed79_.py} | 26 ++++- .../config/development.py | 2 +- .../{staging.yml => development.yml} | 0 .../config/permissions/testing.yml | 8 +- .../load_database_models.py | 2 +- .../models/active_task.py | 24 +++- .../models/active_task_user.py | 31 ++++++ src/spiffworkflow_backend/models/user.py | 1 + .../routes/process_api_blueprint.py | 7 +- .../services/authorization_service.py | 18 +-- .../services/process_instance_processor.py | 64 ++++++++--- .../services/process_instance_service.py | 3 +- tests/data/model_with_lanes/lanes.bpmn | 103 ++++++++++++++++++ .../helpers/base_test.py | 6 +- .../unit/test_authorization_service.py | 5 + .../unit/test_process_instance_processor.py | 59 ++++++++++ 18 files changed, 319 insertions(+), 45 deletions(-) rename migrations/versions/{cf862b761896_.py => 1e25676fed79_.py} (94%) rename src/spiffworkflow_backend/config/permissions/{staging.yml => development.yml} (100%) create mode 100644 src/spiffworkflow_backend/models/active_task_user.py create mode 100644 tests/data/model_with_lanes/lanes.bpmn diff --git a/conftest.py b/conftest.py index 242f75a0..6e4703c2 100644 --- a/conftest.py +++ b/conftest.py @@ -6,6 +6,7 @@ import pytest from flask.app import Flask from flask_bpmn.models.db import db from flask_bpmn.models.db import SpiffworkflowBaseDBModel +from spiffworkflow_backend.models.active_task_user import ActiveTaskUserModel from tests.spiffworkflow_backend.helpers.base_test import BaseTest from tests.spiffworkflow_backend.helpers.test_data import load_test_spec @@ -56,6 +57,8 @@ def app() -> Flask: @pytest.fixture() def with_db_and_bpmn_file_cleanup() -> None: """Process_group_resource.""" + db.session.query(ActiveTaskUserModel).delete() + for model in SpiffworkflowBaseDBModel._all_subclasses(): db.session.query(model).delete() db.session.commit() diff --git a/migrations/env.py b/migrations/env.py index 630e381a..68feded2 100644 --- a/migrations/env.py +++ b/migrations/env.py @@ -1,3 +1,5 @@ +from __future__ import with_statement + import logging from logging.config import fileConfig diff --git a/migrations/versions/cf862b761896_.py b/migrations/versions/1e25676fed79_.py similarity index 94% rename from migrations/versions/cf862b761896_.py rename to migrations/versions/1e25676fed79_.py index c326afe5..dd2e3ccd 100644 --- a/migrations/versions/cf862b761896_.py +++ b/migrations/versions/1e25676fed79_.py @@ -1,8 +1,8 @@ """empty message -Revision ID: cf862b761896 +Revision ID: 1e25676fed79 Revises: -Create Date: 2022-10-20 11:23:58.758922 +Create Date: 2022-10-20 16:29:56.969687 """ from alembic import op @@ -10,7 +10,7 @@ import sqlalchemy as sa # revision identifiers, used by Alembic. -revision = 'cf862b761896' +revision = '1e25676fed79' down_revision = None branch_labels = None depends_on = None @@ -157,7 +157,8 @@ def upgrade(): op.create_table('active_task', sa.Column('id', sa.Integer(), nullable=False), sa.Column('process_instance_id', sa.Integer(), nullable=False), - sa.Column('assigned_principal_id', sa.Integer(), nullable=True), + sa.Column('actual_owner_id', sa.Integer(), nullable=True), + sa.Column('lane_assignment_id', sa.Integer(), nullable=True), sa.Column('form_file_name', sa.String(length=50), nullable=True), sa.Column('ui_form_file_name', sa.String(length=50), nullable=True), sa.Column('updated_at_in_seconds', sa.Integer(), nullable=True), @@ -169,7 +170,8 @@ def upgrade(): sa.Column('task_status', sa.String(length=50), nullable=True), sa.Column('process_model_display_name', sa.String(length=255), nullable=True), sa.Column('task_data', sa.Text(length=4294000000), nullable=True), - sa.ForeignKeyConstraint(['assigned_principal_id'], ['principal.id'], ), + sa.ForeignKeyConstraint(['actual_owner_id'], ['user.id'], ), + sa.ForeignKeyConstraint(['lane_assignment_id'], ['group.id'], ), sa.ForeignKeyConstraint(['process_instance_id'], ['process_instance.id'], ), sa.PrimaryKeyConstraint('id'), sa.UniqueConstraint('task_id', 'process_instance_id', name='active_task_unique') @@ -272,6 +274,17 @@ def upgrade(): sa.ForeignKeyConstraint(['user_id'], ['user.id'], ), sa.PrimaryKeyConstraint('id') ) + op.create_table('active_task_user', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('active_task_id', sa.Integer(), nullable=False), + sa.Column('user_id', sa.Integer(), nullable=False), + sa.ForeignKeyConstraint(['active_task_id'], ['active_task.id'], ), + sa.ForeignKeyConstraint(['user_id'], ['user.id'], ), + sa.PrimaryKeyConstraint('id'), + sa.UniqueConstraint('active_task_id', 'user_id', name='active_task_user_unique') + ) + op.create_index(op.f('ix_active_task_user_active_task_id'), 'active_task_user', ['active_task_id'], unique=False) + op.create_index(op.f('ix_active_task_user_user_id'), 'active_task_user', ['user_id'], unique=False) op.create_table('data_store', sa.Column('id', sa.Integer(), nullable=False), sa.Column('updated_at_in_seconds', sa.Integer(), nullable=True), @@ -305,6 +318,9 @@ def downgrade(): op.drop_index(op.f('ix_message_correlation_message_instance_message_correlation_id'), table_name='message_correlation_message_instance') op.drop_table('message_correlation_message_instance') op.drop_table('data_store') + op.drop_index(op.f('ix_active_task_user_user_id'), table_name='active_task_user') + op.drop_index(op.f('ix_active_task_user_active_task_id'), table_name='active_task_user') + op.drop_table('active_task_user') op.drop_table('task_event') op.drop_table('spiff_logging') op.drop_table('permission_assignment') diff --git a/src/spiffworkflow_backend/config/development.py b/src/spiffworkflow_backend/config/development.py index 151868e5..a7152177 100644 --- a/src/spiffworkflow_backend/config/development.py +++ b/src/spiffworkflow_backend/config/development.py @@ -2,7 +2,7 @@ from os import environ SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_NAME = environ.get( - "SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_NAME", default="staging.yml" + "SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_NAME", default="development.yml" ) SPIFFWORKFLOW_BACKEND_LOG_LEVEL = environ.get( diff --git a/src/spiffworkflow_backend/config/permissions/staging.yml b/src/spiffworkflow_backend/config/permissions/development.yml similarity index 100% rename from src/spiffworkflow_backend/config/permissions/staging.yml rename to src/spiffworkflow_backend/config/permissions/development.yml diff --git a/src/spiffworkflow_backend/config/permissions/testing.yml b/src/spiffworkflow_backend/config/permissions/testing.yml index 0d3f3806..f77dd3ee 100644 --- a/src/spiffworkflow_backend/config/permissions/testing.yml +++ b/src/spiffworkflow_backend/config/permissions/testing.yml @@ -2,8 +2,8 @@ groups: admin: users: [testadmin1, testadmin2] - finance: - users: [testuser1, testuser2] + Finance Team: + users: [testuser2] hr: users: [testuser2, testuser3, testuser4] @@ -16,13 +16,13 @@ permissions: uri: /* read-all: - groups: [finance, hr, admin] + groups: ["Finance Team", hr, admin] users: [] allowed_permissions: [read] uri: /* finance-admin: - groups: [finance] + groups: ["Finance Team"] users: [testuser4] allowed_permissions: [create, read, update, delete] uri: /v1.0/process-groups/finance/* diff --git a/src/spiffworkflow_backend/load_database_models.py b/src/spiffworkflow_backend/load_database_models.py index 33d32c1a..4427df06 100644 --- a/src/spiffworkflow_backend/load_database_models.py +++ b/src/spiffworkflow_backend/load_database_models.py @@ -15,8 +15,8 @@ from spiffworkflow_backend.models.user_group_assignment import ( UserGroupAssignmentModel, ) # noqa: F401 - from spiffworkflow_backend.models.active_task import ActiveTaskModel # noqa: F401 +from spiffworkflow_backend.models.active_task_user import (ActiveTaskUserModel) from spiffworkflow_backend.models.bpmn_process_id_lookup import ( BpmnProcessIdLookup, ) # noqa: F401 diff --git a/src/spiffworkflow_backend/models/active_task.py b/src/spiffworkflow_backend/models/active_task.py index f9daebe8..a8b898f0 100644 --- a/src/spiffworkflow_backend/models/active_task.py +++ b/src/spiffworkflow_backend/models/active_task.py @@ -3,16 +3,25 @@ from __future__ import annotations import json from dataclasses import dataclass +from typing import TYPE_CHECKING from flask_bpmn.models.db import db from flask_bpmn.models.db import SpiffworkflowBaseDBModel from sqlalchemy import ForeignKey from sqlalchemy.orm import relationship from sqlalchemy.orm import RelationshipProperty +from spiffworkflow_backend.models.group import GroupModel from spiffworkflow_backend.models.principal import PrincipalModel from spiffworkflow_backend.models.process_instance import ProcessInstanceModel from spiffworkflow_backend.models.task import Task +from spiffworkflow_backend.models.user import UserModel + + +if TYPE_CHECKING: + from spiffworkflow_backend.models.active_task_user import ( + ActiveTaskUserModel, + ) # noqa: F401 @dataclass @@ -26,14 +35,15 @@ class ActiveTaskModel(SpiffworkflowBaseDBModel): ), ) - assigned_principal: RelationshipProperty[PrincipalModel] = relationship( - PrincipalModel + actual_owner: RelationshipProperty[UserModel] = relationship( + UserModel ) id: int = db.Column(db.Integer, primary_key=True) process_instance_id: int = db.Column( ForeignKey(ProcessInstanceModel.id), nullable=False # type: ignore ) - assigned_principal_id: int = db.Column(ForeignKey(PrincipalModel.id)) + actual_owner_id: int = db.Column(ForeignKey(UserModel.id)) + lane_assignment_id: int | None = db.Column(ForeignKey(GroupModel.id)) form_file_name: str | None = db.Column(db.String(50)) ui_form_file_name: str | None = db.Column(db.String(50)) @@ -48,6 +58,14 @@ class ActiveTaskModel(SpiffworkflowBaseDBModel): process_model_display_name = db.Column(db.String(255)) task_data: str = db.Column(db.Text(4294000000)) + active_task_users = relationship("ActiveTaskUserModel", cascade="delete") + potential_owners = relationship( # type: ignore + "UserModel", + viewonly=True, + secondary="active_task_user", + overlaps="active_task_user,users", + ) + @classmethod def to_task(cls, task: ActiveTaskModel) -> Task: """To_task.""" diff --git a/src/spiffworkflow_backend/models/active_task_user.py b/src/spiffworkflow_backend/models/active_task_user.py new file mode 100644 index 00000000..92d97796 --- /dev/null +++ b/src/spiffworkflow_backend/models/active_task_user.py @@ -0,0 +1,31 @@ +from __future__ import annotations +from dataclasses import dataclass + +from flask_bpmn.models.db import db +from flask_bpmn.models.db import SpiffworkflowBaseDBModel +from sqlalchemy import ForeignKey + +from spiffworkflow_backend.models.active_task import ActiveTaskModel +from spiffworkflow_backend.models.user import UserModel + + +@dataclass +class ActiveTaskUserModel(SpiffworkflowBaseDBModel): + + __tablename__ = "active_task_user" + + __table_args__ = ( + db.UniqueConstraint( + "active_task_id", + "user_id", + name="active_task_user_unique", + ), + ) + + id = db.Column(db.Integer, primary_key=True) + active_task_id = db.Column( + ForeignKey(ActiveTaskModel.id), nullable=False, index=True # type: ignore + ) + user_id = db.Column( + ForeignKey(UserModel.id), nullable=False, index=True + ) diff --git a/src/spiffworkflow_backend/models/user.py b/src/spiffworkflow_backend/models/user.py index 30d60a6f..65daaf39 100644 --- a/src/spiffworkflow_backend/models/user.py +++ b/src/spiffworkflow_backend/models/user.py @@ -1,4 +1,5 @@ """User.""" +from __future__ import annotations from typing import Any import jwt diff --git a/src/spiffworkflow_backend/routes/process_api_blueprint.py b/src/spiffworkflow_backend/routes/process_api_blueprint.py index aa9152f7..4f62bcd1 100644 --- a/src/spiffworkflow_backend/routes/process_api_blueprint.py +++ b/src/spiffworkflow_backend/routes/process_api_blueprint.py @@ -886,7 +886,7 @@ def process_instance_report_show( def task_list_my_tasks(page: int = 1, per_page: int = 100) -> flask.wrappers.Response: """Task_list_my_tasks.""" principal = find_principal_or_raise() - + # TODO: use join table active_tasks = ( ActiveTaskModel.query.filter_by(assigned_principal_id=principal.id) .order_by(desc(ActiveTaskModel.id)) # type: ignore @@ -1089,6 +1089,7 @@ def task_submit( ProcessInstanceService.update_task_assignments(processor) + # TODO: update next_active_task_assigned_to_me = ActiveTaskModel.query.filter_by( assigned_principal_id=principal.id, process_instance_id=process_instance.id ).first() @@ -1258,10 +1259,12 @@ def find_active_task_by_id_or_raise( process_instance_id: int, task_id: str, principal_id: PrincipalModel ) -> ActiveTaskModel: """Find_active_task_by_id_or_raise.""" + + # TODO: update active_task_assigned_to_me = ActiveTaskModel.query.filter_by( process_instance_id=process_instance_id, task_id=task_id, - assigned_principal_id=principal_id, + # assigned_principal_id=principal_id, ).first() if active_task_assigned_to_me is None: message = ( diff --git a/src/spiffworkflow_backend/services/authorization_service.py b/src/spiffworkflow_backend/services/authorization_service.py index b9353686..278ea594 100644 --- a/src/spiffworkflow_backend/services/authorization_service.py +++ b/src/spiffworkflow_backend/services/authorization_service.py @@ -164,14 +164,16 @@ class AuthorizationService: ) if "users" in permission_config: for username in permission_config["users"]: - principal = ( - PrincipalModel.query.join(UserModel) - .filter(UserModel.username == username) - .first() - ) - cls.create_permission_for_principal( - principal, permission_target, allowed_permission - ) + user = UserModel.query.filter_by(username=username).first() + if user is not None: + principal = ( + PrincipalModel.query.join(UserModel) + .filter(UserModel.username == username) + .first() + ) + cls.create_permission_for_principal( + principal, permission_target, allowed_permission + ) @classmethod def create_permission_for_principal( diff --git a/src/spiffworkflow_backend/services/process_instance_processor.py b/src/spiffworkflow_backend/services/process_instance_processor.py index 38d4fe37..470eb729 100644 --- a/src/spiffworkflow_backend/services/process_instance_processor.py +++ b/src/spiffworkflow_backend/services/process_instance_processor.py @@ -4,6 +4,7 @@ import decimal import json import logging import os +import re import time from datetime import datetime from typing import Any @@ -55,9 +56,11 @@ from SpiffWorkflow.task import TaskState from SpiffWorkflow.util.deep_merge import DeepMerge # type: ignore from spiffworkflow_backend.models.active_task import ActiveTaskModel +from spiffworkflow_backend.models.active_task_user import ActiveTaskUserModel from spiffworkflow_backend.models.bpmn_process_id_lookup import BpmnProcessIdLookup from spiffworkflow_backend.models.file import File from spiffworkflow_backend.models.file import FileType +from spiffworkflow_backend.models.group import GroupModel from spiffworkflow_backend.models.message_correlation import MessageCorrelationModel from spiffworkflow_backend.models.message_correlation_message_instance import ( MessageCorrelationMessageInstanceModel, @@ -108,6 +111,10 @@ class ProcessInstanceProcessorError(Exception): """ProcessInstanceProcessorError.""" +class NoPotentialOwnersForTaskError(Exception): + pass + + class CustomBpmnScriptEngine(PythonScriptEngine): # type: ignore """This is a custom script processor that can be easily injected into Spiff Workflow. @@ -511,28 +518,47 @@ class ProcessInstanceProcessor: if self.bpmn_process_instance.is_completed(): self.process_instance_model.end_in_seconds = round(time.time()) - db.session.add(self.process_instance_model) - ActiveTaskModel.query.filter_by( + active_tasks = ActiveTaskModel.query.filter_by( process_instance_id=self.process_instance_model.id - ).delete() + ).all() + if len(active_tasks) > 0: + for at in active_tasks: + db.session.delete(at) + + db.session.add(self.process_instance_model) + db.session.commit() ready_or_waiting_tasks = self.get_all_ready_or_waiting_tasks() for ready_or_waiting_task in ready_or_waiting_tasks: # filter out non-usertasks - if not self.bpmn_process_instance._is_engine_task( - ready_or_waiting_task.task_spec - ): + task_spec = ready_or_waiting_task.task_spec + if not self.bpmn_process_instance._is_engine_task(task_spec): user_id = ready_or_waiting_task.data["current_user"]["id"] - principal = PrincipalModel.query.filter_by(user_id=user_id).first() - if principal is None: - raise ( - ApiError( - error_code="principal_not_found", - message=f"Principal not found from user id: {user_id}", - status_code=400, - ) - ) + # principal = PrincipalModel.query.filter_by(user_id=user_id).first() + # if principal is None: + # raise ( + # ApiError( + # error_code="principal_not_found", + # message=f"Principal not found from user id: {user_id}", + # status_code=400, + # ) + # ) + # import pdb; pdb.set_trace() + task_lane = 'process_initiator' + if task_spec.lane is not None: + task_lane = task_spec.lane + + potential_owner_ids = [] + lane_assignment_id = None + if re.match(r"(process.?)initiator", task_lane, re.IGNORECASE): + potential_owner_ids = [self.process_instance_model.process_initiator_id] + else: + group_model = GroupModel.query.filter_by(identifier=task_lane).first() + if group_model is None: + raise (NoPotentialOwnersForTaskError(f"Could not find a group with name matching lane: {task_lane}")) + potential_owner_ids = [i.user_id for i in group_model.user_group_assignments] + lane_assignment_id = group_model.id extensions = ready_or_waiting_task.task_spec.extensions @@ -555,7 +581,6 @@ class ProcessInstanceProcessor: active_task = ActiveTaskModel( process_instance_id=self.process_instance_model.id, process_model_display_name=process_model_display_name, - assigned_principal_id=principal.id, form_file_name=form_file_name, ui_form_file_name=ui_form_file_name, task_id=str(ready_or_waiting_task.id), @@ -564,10 +589,15 @@ class ProcessInstanceProcessor: task_type=ready_or_waiting_task.task_spec.__class__.__name__, task_status=ready_or_waiting_task.get_state_name(), task_data=json.dumps(ready_or_waiting_task.data), + lane_assignment_id=lane_assignment_id, ) db.session.add(active_task) + db.session.commit() - db.session.commit() + for potential_owner_id in potential_owner_ids: + active_task_user = ActiveTaskUserModel(user_id=potential_owner_id,active_task_id=active_task.id) + db.session.add(active_task_user) + db.session.commit() @staticmethod def get_parser() -> MyCustomParser: diff --git a/src/spiffworkflow_backend/services/process_instance_service.py b/src/spiffworkflow_backend/services/process_instance_service.py index 2d6999fc..24b0eede 100644 --- a/src/spiffworkflow_backend/services/process_instance_service.py +++ b/src/spiffworkflow_backend/services/process_instance_service.py @@ -282,8 +282,7 @@ class ProcessInstanceService: ProcessInstanceService.log_task_action( user.id, processor, spiff_task, TaskAction.COMPLETE.value ) - processor.do_engine_steps() - processor.save() + processor.do_engine_steps(save=True) @staticmethod def log_task_action( diff --git a/tests/data/model_with_lanes/lanes.bpmn b/tests/data/model_with_lanes/lanes.bpmn new file mode 100644 index 00000000..3ee43501 --- /dev/null +++ b/tests/data/model_with_lanes/lanes.bpmn @@ -0,0 +1,103 @@ + + + + + + + + + StartEvent_1 + initator_one + Event_06f4e68 + initiator_two + + + finance_approval + + + + Flow_1tbyols + + + + + + This is initiator user? + + Flow_1tbyols + Flow_16ppta1 + + + + This is finance user? + + Flow_16ppta1 + Flow_1cfcauf + + + + Flow_0x92f7d + + + + + This is initiator again? + + Flow_1cfcauf + Flow_0x92f7d + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/spiffworkflow_backend/helpers/base_test.py b/tests/spiffworkflow_backend/helpers/base_test.py index c779c819..57b26617 100644 --- a/tests/spiffworkflow_backend/helpers/base_test.py +++ b/tests/spiffworkflow_backend/helpers/base_test.py @@ -207,10 +207,12 @@ class BaseTest: # return public_access_token def create_process_instance_from_process_model( - self, process_model: ProcessModelInfo, status: Optional[str] = "not_started" + self, process_model: ProcessModelInfo, status: Optional[str] = "not_started", user: Optional[UserModel] = None ) -> ProcessInstanceModel: """Create_process_instance_from_process_model.""" - user = self.find_or_create_user() + if user is None: + user = self.find_or_create_user() + current_time = round(time.time()) process_instance = ProcessInstanceModel( status=status, diff --git a/tests/spiffworkflow_backend/unit/test_authorization_service.py b/tests/spiffworkflow_backend/unit/test_authorization_service.py index 43949d60..d8f9ce4b 100644 --- a/tests/spiffworkflow_backend/unit/test_authorization_service.py +++ b/tests/spiffworkflow_backend/unit/test_authorization_service.py @@ -19,6 +19,11 @@ class TestAuthorizationService(BaseTest): raise_if_missing_user=True ) + def test_does_not_fail_if_user_not_created( + self, app: Flask, with_db_and_bpmn_file_cleanup: None + ) -> None: + AuthorizationService.import_permissions_from_yaml_file() + def test_can_import_permissions_from_yaml( self, app: Flask, with_db_and_bpmn_file_cleanup: None ) -> None: diff --git a/tests/spiffworkflow_backend/unit/test_process_instance_processor.py b/tests/spiffworkflow_backend/unit/test_process_instance_processor.py index 009239f7..34594b6d 100644 --- a/tests/spiffworkflow_backend/unit/test_process_instance_processor.py +++ b/tests/spiffworkflow_backend/unit/test_process_instance_processor.py @@ -1,6 +1,11 @@ """Test_process_instance_processor.""" from flask.app import Flask +from spiffworkflow_backend.models.group import GroupModel +from spiffworkflow_backend.models.process_instance import ProcessInstanceModel, ProcessInstanceStatus +from spiffworkflow_backend.services.authorization_service import AuthorizationService +from spiffworkflow_backend.services.process_instance_service import ProcessInstanceService from tests.spiffworkflow_backend.helpers.base_test import BaseTest +from tests.spiffworkflow_backend.helpers.test_data import load_test_spec from spiffworkflow_backend.services.process_instance_processor import ( ProcessInstanceProcessor, @@ -34,3 +39,57 @@ class TestProcessInstanceProcessor(BaseTest): result == "Chuck Norris doesn’t read books. He stares them down until he gets the information he wants." ) + + def test_sets_permission_correctly_on_active_task( + self, + app: Flask, + with_db_and_bpmn_file_cleanup: None, + ) -> None: + testuser1 = self.find_or_create_user("testuser1") + testuser2 = self.find_or_create_user("testuser2") + assert testuser1.principal is not None + assert testuser2.principal is not None + AuthorizationService.import_permissions_from_yaml_file() + + finance_group = GroupModel.query.filter_by(identifier="Finance Team").first() + assert finance_group is not None + + process_model = load_test_spec(process_model_id="model_with_lanes", bpmn_file_name="lanes.bpmn") + process_instance = self.create_process_instance_from_process_model(process_model=process_model, user=testuser1) + processor = ProcessInstanceProcessor(process_instance) + processor.do_engine_steps(save=True) + + assert len(process_instance.active_tasks) == 1 + active_task = process_instance.active_tasks[0] + assert active_task.lane_assignment_id is None + assert len(active_task.potential_owners) == 1 + assert active_task.potential_owners[0] == testuser1 + + task = processor.__class__.get_task_by_bpmn_identifier(active_task.task_name, processor.bpmn_process_instance) + ProcessInstanceService.complete_form_task( + processor, task, {}, testuser1 + ) + + assert len(process_instance.active_tasks) == 1 + active_task = process_instance.active_tasks[0] + assert active_task.lane_assignment_id == finance_group.id + assert len(active_task.potential_owners) == 1 + assert active_task.potential_owners[0] == testuser2 + + task = processor.__class__.get_task_by_bpmn_identifier(active_task.task_name, processor.bpmn_process_instance) + ProcessInstanceService.complete_form_task( + processor, task, {}, testuser1 + ) + + assert len(process_instance.active_tasks) == 1 + active_task = process_instance.active_tasks[0] + assert active_task.lane_assignment_id is None + assert len(active_task.potential_owners) == 1 + assert active_task.potential_owners[0] == testuser1 + + task = processor.__class__.get_task_by_bpmn_identifier(active_task.task_name, processor.bpmn_process_instance) + ProcessInstanceService.complete_form_task( + processor, task, {}, testuser1 + ) + + assert process_instance.status == ProcessInstanceStatus.complete.value From be1f4bcc1ad4cbb320664cb9889e61af8a811c1d Mon Sep 17 00:00:00 2001 From: jasquat Date: Thu, 20 Oct 2022 17:23:23 -0400 Subject: [PATCH 02/11] added validation to ensure user has access to task w/ burnettk --- .../services/process_instance_service.py | 18 +++++++++++-- .../unit/test_process_instance_processor.py | 27 ++++++++++++------- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/src/spiffworkflow_backend/services/process_instance_service.py b/src/spiffworkflow_backend/services/process_instance_service.py index 24b0eede..fa0fac1e 100644 --- a/src/spiffworkflow_backend/services/process_instance_service.py +++ b/src/spiffworkflow_backend/services/process_instance_service.py @@ -9,7 +9,8 @@ from flask import current_app from flask_bpmn.api.api_error import ApiError from flask_bpmn.models.db import db from SpiffWorkflow.task import Task as SpiffTask # type: ignore -from SpiffWorkflow.util.deep_merge import DeepMerge # type: ignore +from SpiffWorkflow.util.deep_merge import DeepMerge +from spiffworkflow_backend.models.active_task import ActiveTaskModel # type: ignore from spiffworkflow_backend.models.process_instance import ProcessInstanceApi from spiffworkflow_backend.models.process_instance import ProcessInstanceModel @@ -25,7 +26,13 @@ from spiffworkflow_backend.services.process_instance_processor import ( ) from spiffworkflow_backend.services.process_model_service import ProcessModelService -# from SpiffWorkflow.task import TaskState # type: ignore + +class ActiveTaskNotFoundError(Exception): + pass + + +class UserDoesNotHaveAccessToTaskError(Exception): + pass class ProcessInstanceService: @@ -272,6 +279,13 @@ class ProcessInstanceService: Abstracted here because we need to do it multiple times when completing all tasks in a multi-instance task. """ + active_task = ActiveTaskModel.query.filter_by(task_name=spiff_task.task_spec.name, process_instance_id=processor.process_instance_model.id).first() + if active_task is None: + raise ActiveTaskNotFoundError("Could find an active task with task name '{spiff_task.task_spec.name}' for process instance '{processor.process_instance_model.id}'") + + if user not in active_task.potential_owners: + raise UserDoesNotHaveAccessToTaskError("User {user.username} does not have access to update task'{spiff_task.task_spec.name}' for process instance '{processor.process_instance_model.id}'") + dot_dct = ProcessInstanceService.create_dot_dict(data) spiff_task.update_data(dot_dct) # ProcessInstanceService.post_process_form(spiff_task) # some properties may update the data store. diff --git a/tests/spiffworkflow_backend/unit/test_process_instance_processor.py b/tests/spiffworkflow_backend/unit/test_process_instance_processor.py index 34594b6d..0a9469c9 100644 --- a/tests/spiffworkflow_backend/unit/test_process_instance_processor.py +++ b/tests/spiffworkflow_backend/unit/test_process_instance_processor.py @@ -1,9 +1,10 @@ """Test_process_instance_processor.""" +import pytest from flask.app import Flask from spiffworkflow_backend.models.group import GroupModel from spiffworkflow_backend.models.process_instance import ProcessInstanceModel, ProcessInstanceStatus from spiffworkflow_backend.services.authorization_service import AuthorizationService -from spiffworkflow_backend.services.process_instance_service import ProcessInstanceService +from spiffworkflow_backend.services.process_instance_service import ProcessInstanceService, UserDoesNotHaveAccessToTaskError from tests.spiffworkflow_backend.helpers.base_test import BaseTest from tests.spiffworkflow_backend.helpers.test_data import load_test_spec @@ -65,9 +66,13 @@ class TestProcessInstanceProcessor(BaseTest): assert len(active_task.potential_owners) == 1 assert active_task.potential_owners[0] == testuser1 - task = processor.__class__.get_task_by_bpmn_identifier(active_task.task_name, processor.bpmn_process_instance) + spiff_task = processor.__class__.get_task_by_bpmn_identifier(active_task.task_name, processor.bpmn_process_instance) + with pytest.raises(UserDoesNotHaveAccessToTaskError): + ProcessInstanceService.complete_form_task( + processor, spiff_task, {}, testuser2 + ) ProcessInstanceService.complete_form_task( - processor, task, {}, testuser1 + processor, spiff_task, {}, testuser1 ) assert len(process_instance.active_tasks) == 1 @@ -76,20 +81,24 @@ class TestProcessInstanceProcessor(BaseTest): assert len(active_task.potential_owners) == 1 assert active_task.potential_owners[0] == testuser2 - task = processor.__class__.get_task_by_bpmn_identifier(active_task.task_name, processor.bpmn_process_instance) - ProcessInstanceService.complete_form_task( - processor, task, {}, testuser1 - ) + spiff_task = processor.__class__.get_task_by_bpmn_identifier(active_task.task_name, processor.bpmn_process_instance) + with pytest.raises(UserDoesNotHaveAccessToTaskError): + ProcessInstanceService.complete_form_task( + processor, spiff_task, {}, testuser1 + ) + ProcessInstanceService.complete_form_task( + processor, spiff_task, {}, testuser2 + ) assert len(process_instance.active_tasks) == 1 active_task = process_instance.active_tasks[0] assert active_task.lane_assignment_id is None assert len(active_task.potential_owners) == 1 assert active_task.potential_owners[0] == testuser1 - task = processor.__class__.get_task_by_bpmn_identifier(active_task.task_name, processor.bpmn_process_instance) + spiff_task = processor.__class__.get_task_by_bpmn_identifier(active_task.task_name, processor.bpmn_process_instance) ProcessInstanceService.complete_form_task( - processor, task, {}, testuser1 + processor, spiff_task, {}, testuser1 ) assert process_instance.status == ProcessInstanceStatus.complete.value From 47bdb99fba429e063f9c94e5e98cb31a1e5e85ab Mon Sep 17 00:00:00 2001 From: jasquat Date: Fri, 21 Oct 2022 10:11:25 -0400 Subject: [PATCH 03/11] fixed swagger ui w/ burnettk --- src/spiffworkflow_backend/services/authorization_service.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/spiffworkflow_backend/services/authorization_service.py b/src/spiffworkflow_backend/services/authorization_service.py index 278ea594..6a423e0a 100644 --- a/src/spiffworkflow_backend/services/authorization_service.py +++ b/src/spiffworkflow_backend/services/authorization_service.py @@ -204,6 +204,7 @@ class AuthorizationService: @classmethod def should_disable_auth_for_request(cls) -> bool: """Should_disable_auth_for_request.""" + swagger_functions = ["get_json_spec"] authentication_exclusion_list = ["status", "authentication_callback"] if request.method == "OPTIONS": return True @@ -220,7 +221,9 @@ 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("console_ui_") or api_view_function.__name__ in authentication_exclusion_list + or api_view_function.__name__ in swagger_functions ): return True From 49eefc561eb9da13431dc75351a8a1d49ae2118d Mon Sep 17 00:00:00 2001 From: jasquat Date: Fri, 21 Oct 2022 11:00:31 -0400 Subject: [PATCH 04/11] some precommit stuff w/ burnettk --- conftest.py | 2 +- migrations/env.py | 2 - .../load_database_models.py | 1 - .../models/active_task.py | 11 ++--- .../models/active_task_user.py | 7 +-- src/spiffworkflow_backend/models/user.py | 1 + .../routes/process_api_blueprint.py | 1 - .../services/process_instance_processor.py | 31 ++++++++----- .../services/process_instance_service.py | 21 ++++++--- .../helpers/base_test.py | 5 ++- .../unit/test_authorization_service.py | 1 + .../unit/test_process_instance_processor.py | 44 ++++++++++++------- 12 files changed, 77 insertions(+), 50 deletions(-) diff --git a/conftest.py b/conftest.py index 6e4703c2..17daaaa0 100644 --- a/conftest.py +++ b/conftest.py @@ -6,10 +6,10 @@ import pytest from flask.app import Flask from flask_bpmn.models.db import db from flask_bpmn.models.db import SpiffworkflowBaseDBModel -from spiffworkflow_backend.models.active_task_user import ActiveTaskUserModel from tests.spiffworkflow_backend.helpers.base_test import BaseTest from tests.spiffworkflow_backend.helpers.test_data import load_test_spec +from spiffworkflow_backend.models.active_task_user import ActiveTaskUserModel from spiffworkflow_backend.models.process_instance import ProcessInstanceModel from spiffworkflow_backend.models.user import UserModel from spiffworkflow_backend.services.process_instance_processor import ( diff --git a/migrations/env.py b/migrations/env.py index 68feded2..630e381a 100644 --- a/migrations/env.py +++ b/migrations/env.py @@ -1,5 +1,3 @@ -from __future__ import with_statement - import logging from logging.config import fileConfig diff --git a/src/spiffworkflow_backend/load_database_models.py b/src/spiffworkflow_backend/load_database_models.py index 0f987cba..ef5113d0 100644 --- a/src/spiffworkflow_backend/load_database_models.py +++ b/src/spiffworkflow_backend/load_database_models.py @@ -16,7 +16,6 @@ from spiffworkflow_backend.models.user_group_assignment import ( ) # noqa: F401 from spiffworkflow_backend.models.active_task import ActiveTaskModel # noqa: F401 -from spiffworkflow_backend.models.active_task_user import (ActiveTaskUserModel) from spiffworkflow_backend.models.bpmn_process_id_lookup import ( BpmnProcessIdLookup, ) # noqa: F401 diff --git a/src/spiffworkflow_backend/models/active_task.py b/src/spiffworkflow_backend/models/active_task.py index 4e5a139e..c576f39e 100644 --- a/src/spiffworkflow_backend/models/active_task.py +++ b/src/spiffworkflow_backend/models/active_task.py @@ -9,18 +9,17 @@ from flask_bpmn.models.db import SpiffworkflowBaseDBModel from sqlalchemy import ForeignKey from sqlalchemy.orm import relationship from sqlalchemy.orm import RelationshipProperty -from spiffworkflow_backend.models.group import GroupModel -from spiffworkflow_backend.models.principal import PrincipalModel +from spiffworkflow_backend.models.group import GroupModel from spiffworkflow_backend.models.process_instance import ProcessInstanceModel from spiffworkflow_backend.models.task import Task from spiffworkflow_backend.models.user import UserModel if TYPE_CHECKING: - from spiffworkflow_backend.models.active_task_user import ( + from spiffworkflow_backend.models.active_task_user import ( # noqa: F401 ActiveTaskUserModel, - ) # noqa: F401 + ) @dataclass @@ -34,9 +33,7 @@ class ActiveTaskModel(SpiffworkflowBaseDBModel): ), ) - actual_owner: RelationshipProperty[UserModel] = relationship( - UserModel - ) + actual_owner: RelationshipProperty[UserModel] = relationship(UserModel) id: int = db.Column(db.Integer, primary_key=True) process_instance_id: int = db.Column( ForeignKey(ProcessInstanceModel.id), nullable=False # type: ignore diff --git a/src/spiffworkflow_backend/models/active_task_user.py b/src/spiffworkflow_backend/models/active_task_user.py index 92d97796..f194c38e 100644 --- a/src/spiffworkflow_backend/models/active_task_user.py +++ b/src/spiffworkflow_backend/models/active_task_user.py @@ -1,4 +1,6 @@ +"""Active_task_user.""" from __future__ import annotations + from dataclasses import dataclass from flask_bpmn.models.db import db @@ -11,6 +13,7 @@ from spiffworkflow_backend.models.user import UserModel @dataclass class ActiveTaskUserModel(SpiffworkflowBaseDBModel): + """ActiveTaskUserModel.""" __tablename__ = "active_task_user" @@ -26,6 +29,4 @@ class ActiveTaskUserModel(SpiffworkflowBaseDBModel): active_task_id = db.Column( ForeignKey(ActiveTaskModel.id), nullable=False, index=True # type: ignore ) - user_id = db.Column( - ForeignKey(UserModel.id), nullable=False, index=True - ) + user_id = db.Column(ForeignKey(UserModel.id), nullable=False, index=True) diff --git a/src/spiffworkflow_backend/models/user.py b/src/spiffworkflow_backend/models/user.py index 65daaf39..22cdfb69 100644 --- a/src/spiffworkflow_backend/models/user.py +++ b/src/spiffworkflow_backend/models/user.py @@ -1,5 +1,6 @@ """User.""" from __future__ import annotations + from typing import Any import jwt diff --git a/src/spiffworkflow_backend/routes/process_api_blueprint.py b/src/spiffworkflow_backend/routes/process_api_blueprint.py index 0fe89edc..00a6460a 100644 --- a/src/spiffworkflow_backend/routes/process_api_blueprint.py +++ b/src/spiffworkflow_backend/routes/process_api_blueprint.py @@ -1298,7 +1298,6 @@ def find_active_task_by_id_or_raise( process_instance_id: int, task_id: str, principal_id: PrincipalModel ) -> ActiveTaskModel: """Find_active_task_by_id_or_raise.""" - # TODO: update active_task_assigned_to_me = ActiveTaskModel.query.filter_by( process_instance_id=process_instance_id, diff --git a/src/spiffworkflow_backend/services/process_instance_processor.py b/src/spiffworkflow_backend/services/process_instance_processor.py index 5375bcba..dedf4396 100644 --- a/src/spiffworkflow_backend/services/process_instance_processor.py +++ b/src/spiffworkflow_backend/services/process_instance_processor.py @@ -70,7 +70,6 @@ from spiffworkflow_backend.models.message_correlation_property import ( ) from spiffworkflow_backend.models.message_instance import MessageInstanceModel from spiffworkflow_backend.models.message_instance import MessageModel -from spiffworkflow_backend.models.principal import PrincipalModel from spiffworkflow_backend.models.process_instance import ProcessInstanceModel from spiffworkflow_backend.models.process_instance import ProcessInstanceStatus from spiffworkflow_backend.models.process_model import ProcessModelInfo @@ -112,7 +111,7 @@ class ProcessInstanceProcessorError(Exception): class NoPotentialOwnersForTaskError(Exception): - pass + """NoPotentialOwnersForTaskError.""" class CustomBpmnScriptEngine(PythonScriptEngine): # type: ignore @@ -518,7 +517,6 @@ class ProcessInstanceProcessor: if self.bpmn_process_instance.is_completed(): self.process_instance_model.end_in_seconds = round(time.time()) - active_tasks = ActiveTaskModel.query.filter_by( process_instance_id=self.process_instance_model.id ).all() @@ -534,7 +532,7 @@ class ProcessInstanceProcessor: # filter out non-usertasks task_spec = ready_or_waiting_task.task_spec if not self.bpmn_process_instance._is_engine_task(task_spec): - user_id = ready_or_waiting_task.data["current_user"]["id"] + ready_or_waiting_task.data["current_user"]["id"] # principal = PrincipalModel.query.filter_by(user_id=user_id).first() # if principal is None: # raise ( @@ -545,19 +543,29 @@ class ProcessInstanceProcessor: # ) # ) # import pdb; pdb.set_trace() - task_lane = 'process_initiator' + task_lane = "process_initiator" if task_spec.lane is not None: task_lane = task_spec.lane potential_owner_ids = [] lane_assignment_id = None if re.match(r"(process.?)initiator", task_lane, re.IGNORECASE): - potential_owner_ids = [self.process_instance_model.process_initiator_id] + potential_owner_ids = [ + self.process_instance_model.process_initiator_id + ] else: - group_model = GroupModel.query.filter_by(identifier=task_lane).first() + group_model = GroupModel.query.filter_by( + identifier=task_lane + ).first() if group_model is None: - raise (NoPotentialOwnersForTaskError(f"Could not find a group with name matching lane: {task_lane}")) - potential_owner_ids = [i.user_id for i in group_model.user_group_assignments] + raise ( + NoPotentialOwnersForTaskError( + f"Could not find a group with name matching lane: {task_lane}" + ) + ) + potential_owner_ids = [ + i.user_id for i in group_model.user_group_assignments + ] lane_assignment_id = group_model.id extensions = ready_or_waiting_task.task_spec.extensions @@ -588,14 +596,15 @@ class ProcessInstanceProcessor: task_title=ready_or_waiting_task.task_spec.description, task_type=ready_or_waiting_task.task_spec.__class__.__name__, task_status=ready_or_waiting_task.get_state_name(), - task_data=json.dumps(ready_or_waiting_task.data), lane_assignment_id=lane_assignment_id, ) db.session.add(active_task) db.session.commit() for potential_owner_id in potential_owner_ids: - active_task_user = ActiveTaskUserModel(user_id=potential_owner_id,active_task_id=active_task.id) + active_task_user = ActiveTaskUserModel( + user_id=potential_owner_id, active_task_id=active_task.id + ) db.session.add(active_task_user) db.session.commit() diff --git a/src/spiffworkflow_backend/services/process_instance_service.py b/src/spiffworkflow_backend/services/process_instance_service.py index fa0fac1e..429acbbc 100644 --- a/src/spiffworkflow_backend/services/process_instance_service.py +++ b/src/spiffworkflow_backend/services/process_instance_service.py @@ -10,8 +10,8 @@ from flask_bpmn.api.api_error import ApiError from flask_bpmn.models.db import db from SpiffWorkflow.task import Task as SpiffTask # type: ignore from SpiffWorkflow.util.deep_merge import DeepMerge -from spiffworkflow_backend.models.active_task import ActiveTaskModel # type: ignore +from spiffworkflow_backend.models.active_task import ActiveTaskModel # type: ignore from spiffworkflow_backend.models.process_instance import ProcessInstanceApi from spiffworkflow_backend.models.process_instance import ProcessInstanceModel from spiffworkflow_backend.models.process_instance import ProcessInstanceStatus @@ -28,11 +28,11 @@ from spiffworkflow_backend.services.process_model_service import ProcessModelSer class ActiveTaskNotFoundError(Exception): - pass + """ActiveTaskNotFoundError.""" class UserDoesNotHaveAccessToTaskError(Exception): - pass + """UserDoesNotHaveAccessToTaskError.""" class ProcessInstanceService: @@ -279,12 +279,21 @@ class ProcessInstanceService: Abstracted here because we need to do it multiple times when completing all tasks in a multi-instance task. """ - active_task = ActiveTaskModel.query.filter_by(task_name=spiff_task.task_spec.name, process_instance_id=processor.process_instance_model.id).first() + active_task = ActiveTaskModel.query.filter_by( + task_name=spiff_task.task_spec.name, + process_instance_id=processor.process_instance_model.id, + ).first() if active_task is None: - raise ActiveTaskNotFoundError("Could find an active task with task name '{spiff_task.task_spec.name}' for process instance '{processor.process_instance_model.id}'") + raise ActiveTaskNotFoundError( + f"Could find an active task with task name '{spiff_task.task_spec.name}'" + f" for process instance '{processor.process_instance_model.id}'" + ) if user not in active_task.potential_owners: - raise UserDoesNotHaveAccessToTaskError("User {user.username} does not have access to update task'{spiff_task.task_spec.name}' for process instance '{processor.process_instance_model.id}'") + raise UserDoesNotHaveAccessToTaskError( + f"User {user.username} does not have access to update task'{spiff_task.task_spec.name}'" + f" for process instance '{processor.process_instance_model.id}'" + ) dot_dct = ProcessInstanceService.create_dot_dict(data) spiff_task.update_data(dot_dct) diff --git a/tests/spiffworkflow_backend/helpers/base_test.py b/tests/spiffworkflow_backend/helpers/base_test.py index 1a207db8..b7d2c5d7 100644 --- a/tests/spiffworkflow_backend/helpers/base_test.py +++ b/tests/spiffworkflow_backend/helpers/base_test.py @@ -207,7 +207,10 @@ class BaseTest: # return public_access_token def create_process_instance_from_process_model( - self, process_model: ProcessModelInfo, status: Optional[str] = "not_started", user: Optional[UserModel] = None + self, + process_model: ProcessModelInfo, + status: Optional[str] = "not_started", + user: Optional[UserModel] = None, ) -> ProcessInstanceModel: """Create_process_instance_from_process_model.""" if user is None: diff --git a/tests/spiffworkflow_backend/unit/test_authorization_service.py b/tests/spiffworkflow_backend/unit/test_authorization_service.py index d8f9ce4b..6d7cd5e9 100644 --- a/tests/spiffworkflow_backend/unit/test_authorization_service.py +++ b/tests/spiffworkflow_backend/unit/test_authorization_service.py @@ -22,6 +22,7 @@ class TestAuthorizationService(BaseTest): def test_does_not_fail_if_user_not_created( self, app: Flask, with_db_and_bpmn_file_cleanup: None ) -> None: + """Test_does_not_fail_if_user_not_created.""" AuthorizationService.import_permissions_from_yaml_file() def test_can_import_permissions_from_yaml( diff --git a/tests/spiffworkflow_backend/unit/test_process_instance_processor.py b/tests/spiffworkflow_backend/unit/test_process_instance_processor.py index 0a9469c9..f1a15436 100644 --- a/tests/spiffworkflow_backend/unit/test_process_instance_processor.py +++ b/tests/spiffworkflow_backend/unit/test_process_instance_processor.py @@ -1,16 +1,21 @@ """Test_process_instance_processor.""" import pytest from flask.app import Flask -from spiffworkflow_backend.models.group import GroupModel -from spiffworkflow_backend.models.process_instance import ProcessInstanceModel, ProcessInstanceStatus -from spiffworkflow_backend.services.authorization_service import AuthorizationService -from spiffworkflow_backend.services.process_instance_service import ProcessInstanceService, UserDoesNotHaveAccessToTaskError from tests.spiffworkflow_backend.helpers.base_test import BaseTest from tests.spiffworkflow_backend.helpers.test_data import load_test_spec +from spiffworkflow_backend.models.group import GroupModel +from spiffworkflow_backend.models.process_instance import ProcessInstanceStatus +from spiffworkflow_backend.services.authorization_service import AuthorizationService from spiffworkflow_backend.services.process_instance_processor import ( ProcessInstanceProcessor, ) +from spiffworkflow_backend.services.process_instance_service import ( + ProcessInstanceService, +) +from spiffworkflow_backend.services.process_instance_service import ( + UserDoesNotHaveAccessToTaskError, +) class TestProcessInstanceProcessor(BaseTest): @@ -46,6 +51,7 @@ class TestProcessInstanceProcessor(BaseTest): app: Flask, with_db_and_bpmn_file_cleanup: None, ) -> None: + """Test_sets_permission_correctly_on_active_task.""" testuser1 = self.find_or_create_user("testuser1") testuser2 = self.find_or_create_user("testuser2") assert testuser1.principal is not None @@ -55,8 +61,12 @@ class TestProcessInstanceProcessor(BaseTest): finance_group = GroupModel.query.filter_by(identifier="Finance Team").first() assert finance_group is not None - process_model = load_test_spec(process_model_id="model_with_lanes", bpmn_file_name="lanes.bpmn") - process_instance = self.create_process_instance_from_process_model(process_model=process_model, user=testuser1) + process_model = load_test_spec( + process_model_id="model_with_lanes", bpmn_file_name="lanes.bpmn" + ) + process_instance = self.create_process_instance_from_process_model( + process_model=process_model, user=testuser1 + ) processor = ProcessInstanceProcessor(process_instance) processor.do_engine_steps(save=True) @@ -66,14 +76,14 @@ class TestProcessInstanceProcessor(BaseTest): assert len(active_task.potential_owners) == 1 assert active_task.potential_owners[0] == testuser1 - spiff_task = processor.__class__.get_task_by_bpmn_identifier(active_task.task_name, processor.bpmn_process_instance) + spiff_task = processor.__class__.get_task_by_bpmn_identifier( + active_task.task_name, processor.bpmn_process_instance + ) with pytest.raises(UserDoesNotHaveAccessToTaskError): ProcessInstanceService.complete_form_task( processor, spiff_task, {}, testuser2 ) - ProcessInstanceService.complete_form_task( - processor, spiff_task, {}, testuser1 - ) + ProcessInstanceService.complete_form_task(processor, spiff_task, {}, testuser1) assert len(process_instance.active_tasks) == 1 active_task = process_instance.active_tasks[0] @@ -81,24 +91,24 @@ class TestProcessInstanceProcessor(BaseTest): assert len(active_task.potential_owners) == 1 assert active_task.potential_owners[0] == testuser2 - spiff_task = processor.__class__.get_task_by_bpmn_identifier(active_task.task_name, processor.bpmn_process_instance) + spiff_task = processor.__class__.get_task_by_bpmn_identifier( + active_task.task_name, processor.bpmn_process_instance + ) with pytest.raises(UserDoesNotHaveAccessToTaskError): ProcessInstanceService.complete_form_task( processor, spiff_task, {}, testuser1 ) - ProcessInstanceService.complete_form_task( - processor, spiff_task, {}, testuser2 - ) + ProcessInstanceService.complete_form_task(processor, spiff_task, {}, testuser2) assert len(process_instance.active_tasks) == 1 active_task = process_instance.active_tasks[0] assert active_task.lane_assignment_id is None assert len(active_task.potential_owners) == 1 assert active_task.potential_owners[0] == testuser1 - spiff_task = processor.__class__.get_task_by_bpmn_identifier(active_task.task_name, processor.bpmn_process_instance) - ProcessInstanceService.complete_form_task( - processor, spiff_task, {}, testuser1 + spiff_task = processor.__class__.get_task_by_bpmn_identifier( + active_task.task_name, processor.bpmn_process_instance ) + ProcessInstanceService.complete_form_task(processor, spiff_task, {}, testuser1) assert process_instance.status == ProcessInstanceStatus.complete.value From 63796d50e65fe59445a35172412dc5c5ccc11532 Mon Sep 17 00:00:00 2001 From: jasquat Date: Fri, 21 Oct 2022 11:05:54 -0400 Subject: [PATCH 05/11] fixed model load issue w/ burnettk --- src/spiffworkflow_backend/load_database_models.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/spiffworkflow_backend/load_database_models.py b/src/spiffworkflow_backend/load_database_models.py index ef5113d0..12b33a7e 100644 --- a/src/spiffworkflow_backend/load_database_models.py +++ b/src/spiffworkflow_backend/load_database_models.py @@ -10,10 +10,12 @@ avoid circular imports from flask_bpmn.models.db import add_listeners -# must load this before UserModel and GroupModel for relationships +# must load these before UserModel and GroupModel for relationships from spiffworkflow_backend.models.user_group_assignment import ( UserGroupAssignmentModel, ) # noqa: F401 +from spiffworkflow_backend.models.principal import PrincipalModel # noqa: F401 + from spiffworkflow_backend.models.active_task import ActiveTaskModel # noqa: F401 from spiffworkflow_backend.models.bpmn_process_id_lookup import ( @@ -37,7 +39,6 @@ from spiffworkflow_backend.models.permission_assignment import ( from spiffworkflow_backend.models.permission_target import ( PermissionTargetModel, ) # noqa: F401 -from spiffworkflow_backend.models.principal import PrincipalModel # noqa: F401 from spiffworkflow_backend.models.process_instance import ( ProcessInstanceModel, ) # noqa: F401 From c9b086d96925f0f173aa82bc145c13df458b332e Mon Sep 17 00:00:00 2001 From: jasquat Date: Fri, 21 Oct 2022 11:09:28 -0400 Subject: [PATCH 06/11] ignore all null-ls w/ burnettk --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 22fea4c9..58cb1434 100644 --- a/.gitignore +++ b/.gitignore @@ -18,4 +18,4 @@ node_modules /tests/spiffworkflow_backend/files /bin/import_secrets.py /src/spiffworkflow_backend/config/secrets.py -_null-ls_* +*null-ls_* From 57248eab760d11cbc450b2a10f1c6389a95895f0 Mon Sep 17 00:00:00 2001 From: jasquat Date: Fri, 21 Oct 2022 11:57:27 -0400 Subject: [PATCH 07/11] fixed submitting and getting user tasks w/ burnettk --- src/spiffworkflow_backend/models/principal.py | 4 + .../routes/process_api_blueprint.py | 59 ++--- .../services/authorization_service.py | 224 +++--------------- .../services/process_instance_service.py | 28 +-- .../unit/test_process_instance_processor.py | 6 +- 5 files changed, 62 insertions(+), 259 deletions(-) diff --git a/src/spiffworkflow_backend/models/principal.py b/src/spiffworkflow_backend/models/principal.py index fbe05930..c7efa860 100644 --- a/src/spiffworkflow_backend/models/principal.py +++ b/src/spiffworkflow_backend/models/principal.py @@ -4,6 +4,7 @@ from dataclasses import dataclass from flask_bpmn.models.db import db from flask_bpmn.models.db import SpiffworkflowBaseDBModel from sqlalchemy import ForeignKey +from sqlalchemy.orm import relationship from sqlalchemy.schema import CheckConstraint from spiffworkflow_backend.models.group import GroupModel @@ -28,3 +29,6 @@ class PrincipalModel(SpiffworkflowBaseDBModel): id = db.Column(db.Integer, primary_key=True) user_id = db.Column(ForeignKey(UserModel.id), nullable=True, unique=True) group_id = db.Column(ForeignKey(GroupModel.id), nullable=True, unique=True) + + user = relationship("UserModel", viewonly=True) + group = relationship("GroupModel", viewonly=True) diff --git a/src/spiffworkflow_backend/routes/process_api_blueprint.py b/src/spiffworkflow_backend/routes/process_api_blueprint.py index 00a6460a..af73c3cd 100644 --- a/src/spiffworkflow_backend/routes/process_api_blueprint.py +++ b/src/spiffworkflow_backend/routes/process_api_blueprint.py @@ -28,12 +28,14 @@ from lxml import etree # type: ignore from lxml.builder import ElementMaker # type: ignore from SpiffWorkflow.task import Task as SpiffTask # type: ignore from SpiffWorkflow.task import TaskState +from sqlalchemy import asc from sqlalchemy import desc from spiffworkflow_backend.exceptions.process_entity_not_found_error import ( ProcessEntityNotFoundError, ) from spiffworkflow_backend.models.active_task import ActiveTaskModel +from spiffworkflow_backend.models.active_task_user import ActiveTaskUserModel from spiffworkflow_backend.models.file import FileSchema from spiffworkflow_backend.models.message_instance import MessageInstanceModel from spiffworkflow_backend.models.message_model import MessageModel @@ -918,11 +920,11 @@ def process_instance_report_show( def task_list_my_tasks(page: int = 1, per_page: int = 100) -> flask.wrappers.Response: """Task_list_my_tasks.""" principal = find_principal_or_raise() - # TODO: use join table active_tasks = ( - ActiveTaskModel.query.filter_by(assigned_principal_id=principal.id) - .order_by(desc(ActiveTaskModel.id)) # type: ignore + ActiveTaskModel.query.order_by(desc(ActiveTaskModel.id)) # type: ignore .join(ProcessInstanceModel) + .join(ActiveTaskUserModel) + .filter_by(user_id=principal.user_id) # just need this add_columns to add the process_model_identifier. Then add everything back that was removed. .add_columns( ProcessInstanceModel.process_model_identifier, @@ -1085,18 +1087,15 @@ def task_submit( ) -> flask.wrappers.Response: """Task_submit_user_data.""" principal = find_principal_or_raise() - active_task_assigned_to_me = find_active_task_by_id_or_raise( - process_instance_id, task_id, principal.id - ) - - process_instance = find_process_instance_by_id_or_raise( - active_task_assigned_to_me.process_instance_id - ) + process_instance = find_process_instance_by_id_or_raise(process_instance_id) processor = ProcessInstanceProcessor(process_instance) spiff_task = get_spiff_task_from_process_instance( task_id, process_instance, processor=processor ) + AuthorizationService.assert_user_can_complete_spiff_task( + processor, spiff_task, principal.user + ) if spiff_task.state != TaskState.READY: raise ( @@ -1110,10 +1109,6 @@ def task_submit( if terminate_loop and spiff_task.is_looping(): spiff_task.terminate_loop() - # TODO: support repeating fields - # Extract the details specific to the form submitted - # form_data = WorkflowService().extract_form_data(body, spiff_task) - ProcessInstanceService.complete_form_task(processor, spiff_task, body, g.user) # If we need to update all tasks, then get the next ready task and if it a multi-instance with the same @@ -1128,10 +1123,13 @@ def task_submit( ProcessInstanceService.update_task_assignments(processor) - # TODO: update - next_active_task_assigned_to_me = ActiveTaskModel.query.filter_by( - assigned_principal_id=principal.id, process_instance_id=process_instance.id - ).first() + next_active_task_assigned_to_me = ( + ActiveTaskModel.query.filter_by(process_instance_id=process_instance_id) + .order_by(asc(ActiveTaskModel.id)) # type: ignore + .join(ActiveTaskUserModel) + .filter_by(user_id=principal.user_id) + .first() + ) if next_active_task_assigned_to_me: return make_response( jsonify(ActiveTaskModel.to_task(next_active_task_assigned_to_me)), 200 @@ -1294,31 +1292,6 @@ def find_principal_or_raise() -> PrincipalModel: return principal # type: ignore -def find_active_task_by_id_or_raise( - process_instance_id: int, task_id: str, principal_id: PrincipalModel -) -> ActiveTaskModel: - """Find_active_task_by_id_or_raise.""" - # TODO: update - active_task_assigned_to_me = ActiveTaskModel.query.filter_by( - process_instance_id=process_instance_id, - task_id=task_id, - # assigned_principal_id=principal_id, - ).first() - if active_task_assigned_to_me is None: - message = ( - f"Task not found for principal user {principal_id} " - f"process_instance_id: {process_instance_id}, task_id: {task_id}" - ) - raise ( - ApiError( - error_code="task_not_found", - message=message, - status_code=400, - ) - ) - return active_task_assigned_to_me # type: ignore - - def find_process_instance_by_id_or_raise( process_instance_id: int, ) -> ProcessInstanceModel: diff --git a/src/spiffworkflow_backend/services/authorization_service.py b/src/spiffworkflow_backend/services/authorization_service.py index 6a423e0a..f777bb36 100644 --- a/src/spiffworkflow_backend/services/authorization_service.py +++ b/src/spiffworkflow_backend/services/authorization_service.py @@ -10,8 +10,10 @@ from flask import g from flask import request from flask_bpmn.api.api_error import ApiError from flask_bpmn.models.db import db +from SpiffWorkflow.task import Task as SpiffTask from sqlalchemy import text +from spiffworkflow_backend.models.active_task import ActiveTaskModel # type: ignore from spiffworkflow_backend.models.group import GroupModel from spiffworkflow_backend.models.permission_assignment import PermissionAssignmentModel from spiffworkflow_backend.models.permission_target import PermissionTargetModel @@ -20,6 +22,9 @@ from spiffworkflow_backend.models.principal import PrincipalModel from spiffworkflow_backend.models.user import UserModel from spiffworkflow_backend.models.user import UserNotFoundError from spiffworkflow_backend.models.user_group_assignment import UserGroupAssignmentModel +from spiffworkflow_backend.services.process_instance_processor import ( + ProcessInstanceProcessor, +) from spiffworkflow_backend.services.user_service import UserService @@ -27,6 +32,14 @@ class PermissionsFileNotSetError(Exception): """PermissionsFileNotSetError.""" +class ActiveTaskNotFoundError(Exception): + """ActiveTaskNotFoundError.""" + + +class UserDoesNotHaveAccessToTaskError(Exception): + """UserDoesNotHaveAccessToTaskError.""" + + class AuthorizationService: """Determine whether a user has permission to perform their request.""" @@ -364,196 +377,29 @@ class AuthorizationService: "The Authentication token you provided is invalid. You need a new token. ", ) from exception - # def get_bearer_token_from_internal_token(self, internal_token): - # """Get_bearer_token_from_internal_token.""" - # self.decode_auth_token(internal_token) - # print(f"get_user_by_internal_token: {internal_token}") + @staticmethod + def assert_user_can_complete_spiff_task( + processor: ProcessInstanceProcessor, + spiff_task: SpiffTask, + user: UserModel, + ) -> bool: + """Assert_user_can_complete_spiff_task.""" + active_task = ActiveTaskModel.query.filter_by( + task_name=spiff_task.task_spec.name, + process_instance_id=processor.process_instance_model.id, + ).first() + if active_task is None: + raise ActiveTaskNotFoundError( + f"Could find an active task with task name '{spiff_task.task_spec.name}'" + f" for process instance '{processor.process_instance_model.id}'" + ) - # def introspect_token(self, basic_token: str) -> dict: - # """Introspect_token.""" - # ( - # open_id_server_url, - # open_id_client_id, - # open_id_realm_name, - # open_id_client_secret_key, - # ) = AuthorizationService.get_open_id_args() - # - # bearer_token = AuthorizationService().get_bearer_token(basic_token) - # auth_bearer_string = f"Bearer {bearer_token['access_token']}" - # - # headers = { - # "Content-Type": "application/x-www-form-urlencoded", - # "Authorization": auth_bearer_string, - # } - # data = { - # "client_id": open_id_client_id, - # "client_secret": open_id_client_secret_key, - # "token": basic_token, - # } - # request_url = f"{open_id_server_url}/realms/{open_id_realm_name}/protocol/openid-connect/token/introspect" - # - # introspect_response = requests.post(request_url, headers=headers, data=data) - # introspection = json.loads(introspect_response.text) - # - # return introspection - - # def get_permission_by_basic_token(self, basic_token: dict) -> list: - # """Get_permission_by_basic_token.""" - # ( - # open_id_server_url, - # open_id_client_id, - # open_id_realm_name, - # open_id_client_secret_key, - # ) = AuthorizationService.get_open_id_args() - # - # # basic_token = AuthorizationService().refresh_token(basic_token) - # # bearer_token = AuthorizationService().get_bearer_token(basic_token['access_token']) - # bearer_token = AuthorizationService().get_bearer_token(basic_token) - # # auth_bearer_string = f"Bearer {bearer_token['access_token']}" - # auth_bearer_string = f"Bearer {bearer_token}" - # - # headers = { - # "Content-Type": "application/x-www-form-urlencoded", - # "Authorization": auth_bearer_string, - # } - # data = { - # "client_id": open_id_client_id, - # "client_secret": open_id_client_secret_key, - # "grant_type": "urn:ietf:params:oauth:grant-type:uma-ticket", - # "response_mode": "permissions", - # "audience": open_id_client_id, - # "response_include_resource_name": True, - # } - # request_url = f"{open_id_server_url}/realms/{open_id_realm_name}/protocol/openid-connect/token" - # permission_response = requests.post(request_url, headers=headers, data=data) - # permission = json.loads(permission_response.text) - # return permission - - # def get_auth_status_for_resource_and_scope_by_token( - # self, basic_token: dict, resource: str, scope: str - # ) -> str: - # """Get_auth_status_for_resource_and_scope_by_token.""" - # ( - # open_id_server_url, - # open_id_client_id, - # open_id_realm_name, - # open_id_client_secret_key, - # ) = AuthorizationService.get_open_id_args() - # - # # basic_token = AuthorizationService().refresh_token(basic_token) - # bearer_token = AuthorizationService().get_bearer_token(basic_token) - # auth_bearer_string = f"Bearer {bearer_token['access_token']}" - # - # headers = { - # "Content-Type": "application/x-www-form-urlencoded", - # "Authorization": auth_bearer_string, - # } - # data = { - # "client_id": open_id_client_id, - # "client_secret": open_id_client_secret_key, - # "grant_type": "urn:ietf:params:oauth:grant-type:uma-ticket", - # "permission": f"{resource}#{scope}", - # "response_mode": "permissions", - # "audience": open_id_client_id, - # } - # request_url = f"{open_id_server_url}/realms/{open_id_realm_name}/protocol/openid-connect/token" - # auth_response = requests.post(request_url, headers=headers, data=data) - # - # print("get_auth_status_for_resource_and_scope_by_token") - # auth_status: str = json.loads(auth_response.text) - # return auth_status - - # def get_permissions_by_token_for_resource_and_scope( - # self, basic_token: str, resource: str|None=None, scope: str|None=None - # ) -> str: - # """Get_permissions_by_token_for_resource_and_scope.""" - # ( - # open_id_server_url, - # open_id_client_id, - # open_id_realm_name, - # open_id_client_secret_key, - # ) = AuthorizationService.get_open_id_args() - # - # # basic_token = AuthorizationService().refresh_token(basic_token) - # # bearer_token = AuthorizationService().get_bearer_token(basic_token['access_token']) - # bearer_token = AuthorizationService().get_bearer_token(basic_token) - # auth_bearer_string = f"Bearer {bearer_token['access_token']}" - # - # headers = { - # "Content-Type": "application/x-www-form-urlencoded", - # "Authorization": auth_bearer_string, - # } - # permision = "" - # if resource is not None and resource != '': - # permision += resource - # if scope is not None and scope != '': - # permision += "#" + scope - # data = { - # "client_id": open_id_client_id, - # "client_secret": open_id_client_secret_key, - # "grant_type": "urn:ietf:params:oauth:grant-type:uma-ticket", - # "response_mode": "permissions", - # "permission": permision, - # "audience": open_id_client_id, - # "response_include_resource_name": True, - # } - # request_url = f"{open_id_server_url}/realms/{open_id_realm_name}/protocol/openid-connect/token" - # permission_response = requests.post(request_url, headers=headers, data=data) - # permission: str = json.loads(permission_response.text) - # return permission - - # def get_resource_set(self, public_access_token, uri): - # """Get_resource_set.""" - # ( - # open_id_server_url, - # open_id_client_id, - # open_id_realm_name, - # open_id_client_secret_key, - # ) = AuthorizationService.get_open_id_args() - # bearer_token = AuthorizationService().get_bearer_token(public_access_token) - # auth_bearer_string = f"Bearer {bearer_token['access_token']}" - # headers = { - # "Content-Type": "application/json", - # "Authorization": auth_bearer_string, - # } - # data = { - # "matchingUri": "true", - # "deep": "true", - # "max": "-1", - # "exactName": "false", - # "uri": uri, - # } - # - # # f"matchingUri=true&deep=true&max=-1&exactName=false&uri={URI_TO_TEST_AGAINST}" - # request_url = f"{open_id_server_url}/realms/{open_id_realm_name}/authz/protection/resource_set" - # response = requests.get(request_url, headers=headers, data=data) - # - # print("get_resource_set") - - # def get_permission_by_token(self, public_access_token: str) -> dict: - # """Get_permission_by_token.""" - # # TODO: Write a test for this - # ( - # open_id_server_url, - # open_id_client_id, - # open_id_realm_name, - # open_id_client_secret_key, - # ) = AuthorizationService.get_open_id_args() - # bearer_token = AuthorizationService().get_bearer_token(public_access_token) - # auth_bearer_string = f"Bearer {bearer_token['access_token']}" - # headers = { - # "Content-Type": "application/x-www-form-urlencoded", - # "Authorization": auth_bearer_string, - # } - # data = { - # "grant_type": "urn:ietf:params:oauth:grant-type:uma-ticket", - # "audience": open_id_client_id, - # } - # request_url = f"{open_id_server_url}/realms/{open_id_realm_name}/protocol/openid-connect/token" - # permission_response = requests.post(request_url, headers=headers, data=data) - # permission: dict = json.loads(permission_response.text) - # - # return permission + if user not in active_task.potential_owners: + raise UserDoesNotHaveAccessToTaskError( + f"User {user.username} does not have access to update task'{spiff_task.task_spec.name}'" + f" for process instance '{processor.process_instance_model.id}'" + ) + return True class KeycloakAuthorization: diff --git a/src/spiffworkflow_backend/services/process_instance_service.py b/src/spiffworkflow_backend/services/process_instance_service.py index 429acbbc..5eeb09e9 100644 --- a/src/spiffworkflow_backend/services/process_instance_service.py +++ b/src/spiffworkflow_backend/services/process_instance_service.py @@ -11,7 +11,6 @@ from flask_bpmn.models.db import db from SpiffWorkflow.task import Task as SpiffTask # type: ignore from SpiffWorkflow.util.deep_merge import DeepMerge -from spiffworkflow_backend.models.active_task import ActiveTaskModel # type: ignore from spiffworkflow_backend.models.process_instance import ProcessInstanceApi from spiffworkflow_backend.models.process_instance import ProcessInstanceModel from spiffworkflow_backend.models.process_instance import ProcessInstanceStatus @@ -20,6 +19,7 @@ from spiffworkflow_backend.models.task import Task from spiffworkflow_backend.models.task_event import TaskAction from spiffworkflow_backend.models.task_event import TaskEventModel from spiffworkflow_backend.models.user import UserModel +from spiffworkflow_backend.services.authorization_service import AuthorizationService from spiffworkflow_backend.services.git_service import GitService from spiffworkflow_backend.services.process_instance_processor import ( ProcessInstanceProcessor, @@ -27,14 +27,6 @@ from spiffworkflow_backend.services.process_instance_processor import ( from spiffworkflow_backend.services.process_model_service import ProcessModelService -class ActiveTaskNotFoundError(Exception): - """ActiveTaskNotFoundError.""" - - -class UserDoesNotHaveAccessToTaskError(Exception): - """UserDoesNotHaveAccessToTaskError.""" - - class ProcessInstanceService: """ProcessInstanceService.""" @@ -279,21 +271,9 @@ class ProcessInstanceService: Abstracted here because we need to do it multiple times when completing all tasks in a multi-instance task. """ - active_task = ActiveTaskModel.query.filter_by( - task_name=spiff_task.task_spec.name, - process_instance_id=processor.process_instance_model.id, - ).first() - if active_task is None: - raise ActiveTaskNotFoundError( - f"Could find an active task with task name '{spiff_task.task_spec.name}'" - f" for process instance '{processor.process_instance_model.id}'" - ) - - if user not in active_task.potential_owners: - raise UserDoesNotHaveAccessToTaskError( - f"User {user.username} does not have access to update task'{spiff_task.task_spec.name}'" - f" for process instance '{processor.process_instance_model.id}'" - ) + AuthorizationService.assert_user_can_complete_spiff_task( + processor, spiff_task, user + ) dot_dct = ProcessInstanceService.create_dot_dict(data) spiff_task.update_data(dot_dct) diff --git a/tests/spiffworkflow_backend/unit/test_process_instance_processor.py b/tests/spiffworkflow_backend/unit/test_process_instance_processor.py index f1a15436..dcd29016 100644 --- a/tests/spiffworkflow_backend/unit/test_process_instance_processor.py +++ b/tests/spiffworkflow_backend/unit/test_process_instance_processor.py @@ -7,15 +7,15 @@ from tests.spiffworkflow_backend.helpers.test_data import load_test_spec from spiffworkflow_backend.models.group import GroupModel from spiffworkflow_backend.models.process_instance import ProcessInstanceStatus from spiffworkflow_backend.services.authorization_service import AuthorizationService +from spiffworkflow_backend.services.authorization_service import ( + UserDoesNotHaveAccessToTaskError, +) from spiffworkflow_backend.services.process_instance_processor import ( ProcessInstanceProcessor, ) from spiffworkflow_backend.services.process_instance_service import ( ProcessInstanceService, ) -from spiffworkflow_backend.services.process_instance_service import ( - UserDoesNotHaveAccessToTaskError, -) class TestProcessInstanceProcessor(BaseTest): From 645e4d8fb6e51f9b98115978e526c4d61fdec6fc Mon Sep 17 00:00:00 2001 From: jasquat Date: Fri, 21 Oct 2022 13:15:17 -0400 Subject: [PATCH 08/11] updated flask-bpmn for sentry and fixed for pyl w/ burnettk --- poetry.lock | 51 +++++++------------ pyproject.toml | 2 +- src/spiffworkflow_backend/__init__.py | 3 -- .../config/permissions/testing.yml | 2 +- .../services/authorization_service.py | 4 +- .../services/process_instance_service.py | 2 +- .../unit/test_authorization_service.py | 2 +- .../unit/test_process_instance_processor.py | 26 +++++----- 8 files changed, 38 insertions(+), 54 deletions(-) diff --git a/poetry.lock b/poetry.lock index f487a2c1..152bfb43 100644 --- a/poetry.lock +++ b/poetry.lock @@ -95,7 +95,7 @@ python-versions = ">=3.5" dev = ["cloudpickle", "coverage[toml] (>=5.0.2)", "furo", "hypothesis", "mypy (>=0.900,!=0.940)", "pre-commit", "pympler", "pytest (>=4.3.0)", "pytest-mypy-plugins", "sphinx", "sphinx-notfound-page", "zope.interface"] docs = ["furo", "sphinx", "sphinx-notfound-page", "zope.interface"] tests = ["cloudpickle", "coverage[toml] (>=5.0.2)", "hypothesis", "mypy (>=0.900,!=0.940)", "pympler", "pytest (>=4.3.0)", "pytest-mypy-plugins", "zope.interface"] -tests_no_zope = ["cloudpickle", "coverage[toml] (>=5.0.2)", "hypothesis", "mypy (>=0.900,!=0.940)", "pympler", "pytest (>=4.3.0)", "pytest-mypy-plugins"] +tests-no-zope = ["cloudpickle", "coverage[toml] (>=5.0.2)", "hypothesis", "mypy (>=0.900,!=0.940)", "pympler", "pytest (>=4.3.0)", "pytest-mypy-plugins"] [[package]] name = "Babel" @@ -268,7 +268,7 @@ optional = false python-versions = ">=3.6.0" [package.extras] -unicode_backport = ["unicodedata2"] +unicode-backport = ["unicodedata2"] [[package]] name = "classify-imports" @@ -618,7 +618,7 @@ description = "Flask Bpmn" category = "main" optional = false python-versions = "^3.7" -develop = false +develop = true [package.dependencies] click = "^8.0.1" @@ -636,10 +636,8 @@ spiffworkflow = "*" werkzeug = "*" [package.source] -type = "git" -url = "https://github.com/sartography/flask-bpmn" -reference = "main" -resolved_reference = "c8fd01df47518749a074772fec383256c482139f" +type = "directory" +url = "../flask-bpmn" [[package]] name = "Flask-Cors" @@ -1512,7 +1510,7 @@ urllib3 = ">=1.21.1,<1.27" [package.extras] socks = ["PySocks (>=1.5.6,!=1.5.7)"] -use_chardet_on_py3 = ["chardet (>=3.0.2,<6)"] +use-chardet-on-py3 = ["chardet (>=3.0.2,<6)"] [[package]] name = "requests-toolbelt" @@ -1604,7 +1602,7 @@ gitlab = ["python-gitlab (>=1.3.0)"] [[package]] name = "sentry-sdk" -version = "1.9.10" +version = "1.10.1" description = "Python client for Sentry (https://sentry.io)" category = "main" optional = false @@ -1625,7 +1623,7 @@ falcon = ["falcon (>=1.4)"] fastapi = ["fastapi (>=0.79.0)"] flask = ["blinker (>=1.1)", "flask (>=0.11)"] httpx = ["httpx (>=0.16.0)"] -pure_eval = ["asttokens", "executing", "pure-eval"] +pure-eval = ["asttokens", "executing", "pure-eval"] pyspark = ["pyspark (>=2.4.4)"] quart = ["blinker (>=1.1)", "quart (>=0.16.1)"] rq = ["rq (>=0.6)"] @@ -1883,19 +1881,19 @@ aiomysql = ["aiomysql", "greenlet (!=0.4.17)"] aiosqlite = ["aiosqlite", "greenlet (!=0.4.17)", "typing_extensions (!=3.10.0.1)"] asyncio = ["greenlet (!=0.4.17)"] asyncmy = ["asyncmy (>=0.2.3,!=0.2.4)", "greenlet (!=0.4.17)"] -mariadb_connector = ["mariadb (>=1.0.1,!=1.1.2)"] +mariadb-connector = ["mariadb (>=1.0.1,!=1.1.2)"] mssql = ["pyodbc"] -mssql_pymssql = ["pymssql"] -mssql_pyodbc = ["pyodbc"] +mssql-pymssql = ["pymssql"] +mssql-pyodbc = ["pyodbc"] mypy = ["mypy (>=0.910)", "sqlalchemy2-stubs"] mysql = ["mysqlclient (>=1.4.0)", "mysqlclient (>=1.4.0,<2)"] -mysql_connector = ["mysql-connector-python"] +mysql-connector = ["mysql-connector-python"] oracle = ["cx_oracle (>=7)", "cx_oracle (>=7,<8)"] postgresql = ["psycopg2 (>=2.7)"] -postgresql_asyncpg = ["asyncpg", "greenlet (!=0.4.17)"] -postgresql_pg8000 = ["pg8000 (>=1.16.6,!=1.29.0)"] -postgresql_psycopg2binary = ["psycopg2-binary"] -postgresql_psycopg2cffi = ["psycopg2cffi"] +postgresql-asyncpg = ["asyncpg", "greenlet (!=0.4.17)"] +postgresql-pg8000 = ["pg8000 (>=1.16.6,!=1.29.0)"] +postgresql-psycopg2binary = ["psycopg2-binary"] +postgresql-psycopg2cffi = ["psycopg2cffi"] pymysql = ["pymysql", "pymysql (<1)"] sqlcipher = ["sqlcipher3_binary"] @@ -2240,7 +2238,7 @@ testing = ["flake8 (<5)", "func-timeout", "jaraco.functools", "jaraco.itertools" [metadata] lock-version = "1.1" python-versions = ">=3.9,<3.11" -content-hash = "29b4ba7f1afdf87ad0a336216ef71d2e2659cd1bd3baecb009efdaac2937737a" +content-hash = "ce05bf155acb17a9dbd941e5e280a7ab001f73a1852cefbb2dd5862db9096f66" [metadata.files] alabaster = [ @@ -3043,18 +3041,7 @@ py = [ {file = "py-1.11.0.tar.gz", hash = "sha256:51c75c4126074b472f746a24399ad32f6053d1b34b68d2fa41e558e6f4a98719"}, ] pyasn1 = [ - {file = "pyasn1-0.4.8-py2.4.egg", hash = "sha256:fec3e9d8e36808a28efb59b489e4528c10ad0f480e57dcc32b4de5c9d8c9fdf3"}, - {file = "pyasn1-0.4.8-py2.5.egg", hash = "sha256:0458773cfe65b153891ac249bcf1b5f8f320b7c2ce462151f8fa74de8934becf"}, - {file = "pyasn1-0.4.8-py2.6.egg", hash = "sha256:5c9414dcfede6e441f7e8f81b43b34e834731003427e5b09e4e00e3172a10f00"}, - {file = "pyasn1-0.4.8-py2.7.egg", hash = "sha256:6e7545f1a61025a4e58bb336952c5061697da694db1cae97b116e9c46abcf7c8"}, {file = "pyasn1-0.4.8-py2.py3-none-any.whl", hash = "sha256:39c7e2ec30515947ff4e87fb6f456dfc6e84857d34be479c9d4a4ba4bf46aa5d"}, - {file = "pyasn1-0.4.8-py3.1.egg", hash = "sha256:78fa6da68ed2727915c4767bb386ab32cdba863caa7dbe473eaae45f9959da86"}, - {file = "pyasn1-0.4.8-py3.2.egg", hash = "sha256:08c3c53b75eaa48d71cf8c710312316392ed40899cb34710d092e96745a358b7"}, - {file = "pyasn1-0.4.8-py3.3.egg", hash = "sha256:03840c999ba71680a131cfaee6fab142e1ed9bbd9c693e285cc6aca0d555e576"}, - {file = "pyasn1-0.4.8-py3.4.egg", hash = "sha256:7ab8a544af125fb704feadb008c99a88805126fb525280b2270bb25cc1d78a12"}, - {file = "pyasn1-0.4.8-py3.5.egg", hash = "sha256:e89bf84b5437b532b0803ba5c9a5e054d21fec423a89952a74f87fa2c9b7bce2"}, - {file = "pyasn1-0.4.8-py3.6.egg", hash = "sha256:014c0e9976956a08139dc0712ae195324a75e142284d5f87f1a87ee1b068a359"}, - {file = "pyasn1-0.4.8-py3.7.egg", hash = "sha256:99fcc3c8d804d1bc6d9a099921e39d827026409a58f2a720dcdb89374ea0c776"}, {file = "pyasn1-0.4.8.tar.gz", hash = "sha256:aef77c9fb94a3ac588e87841208bdec464471d9871bd5050a287cc9a475cd0ba"}, ] pycodestyle = [ @@ -3326,8 +3313,8 @@ safety = [ {file = "safety-2.3.1.tar.gz", hash = "sha256:6e6fcb7d4e8321098cf289f59b65051cafd3467f089c6e57c9f894ae32c23b71"}, ] sentry-sdk = [ - {file = "sentry-sdk-1.9.10.tar.gz", hash = "sha256:4fbace9a763285b608c06f01a807b51acb35f6059da6a01236654e08b0ee81ff"}, - {file = "sentry_sdk-1.9.10-py2.py3-none-any.whl", hash = "sha256:2469240f6190aaebcb453033519eae69cfe8cc602065b4667e18ee14fc1e35dc"}, + {file = "sentry-sdk-1.10.1.tar.gz", hash = "sha256:105faf7bd7b7fa25653404619ee261527266b14103fe1389e0ce077bd23a9691"}, + {file = "sentry_sdk-1.10.1-py2.py3-none-any.whl", hash = "sha256:06c0fa9ccfdc80d7e3b5d2021978d6eb9351fa49db9b5847cf4d1f2a473414ad"}, ] setuptools = [ {file = "setuptools-65.5.0-py3-none-any.whl", hash = "sha256:f62ea9da9ed6289bfe868cd6845968a2c854d1427f8548d52cae02a42b4f0356"}, diff --git a/pyproject.toml b/pyproject.toml index 0ff379e5..9c86efb5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,7 +31,7 @@ werkzeug = "*" SpiffWorkflow = {git = "https://github.com/sartography/SpiffWorkflow", rev = "main"} # SpiffWorkflow = {develop = true, path = "/Users/kevin/projects/github/sartography/SpiffWorkflow"} # SpiffWorkflow = {develop = true, path = "/home/jason/projects/github/sartography/SpiffWorkflow"} -sentry-sdk = "^1.9.10" +sentry-sdk = "^1.10" sphinx-autoapi = "^1.8.4" # flask-bpmn = {develop = true, path = "/home/jason/projects/github/sartography/flask-bpmn"} # flask-bpmn = {develop = true, path = "/Users/kevin/projects/github/sartography/flask-bpmn"} diff --git a/src/spiffworkflow_backend/__init__.py b/src/spiffworkflow_backend/__init__.py index 1358c06a..9a090330 100644 --- a/src/spiffworkflow_backend/__init__.py +++ b/src/spiffworkflow_backend/__init__.py @@ -145,7 +145,6 @@ def get_hacked_up_app_for_script() -> flask.app.Flask: def configure_sentry(app: flask.app.Flask) -> None: """Configure_sentry.""" import sentry_sdk - from flask import Flask from sentry_sdk.integrations.flask import FlaskIntegration def before_send(event: Any, hint: Any) -> Any: @@ -172,5 +171,3 @@ def configure_sentry(app: flask.app.Flask) -> None: traces_sample_rate=float(sentry_sample_rate), before_send=before_send, ) - - app = Flask(__name__) diff --git a/src/spiffworkflow_backend/config/permissions/testing.yml b/src/spiffworkflow_backend/config/permissions/testing.yml index f77dd3ee..a26bab59 100644 --- a/src/spiffworkflow_backend/config/permissions/testing.yml +++ b/src/spiffworkflow_backend/config/permissions/testing.yml @@ -3,7 +3,7 @@ groups: users: [testadmin1, testadmin2] Finance Team: - users: [testuser2] + users: [testuser1, testuser2] hr: users: [testuser2, testuser3, testuser4] diff --git a/src/spiffworkflow_backend/services/authorization_service.py b/src/spiffworkflow_backend/services/authorization_service.py index f777bb36..b32d1945 100644 --- a/src/spiffworkflow_backend/services/authorization_service.py +++ b/src/spiffworkflow_backend/services/authorization_service.py @@ -10,10 +10,10 @@ from flask import g from flask import request from flask_bpmn.api.api_error import ApiError from flask_bpmn.models.db import db -from SpiffWorkflow.task import Task as SpiffTask +from SpiffWorkflow.task import Task as SpiffTask # type: ignore from sqlalchemy import text -from spiffworkflow_backend.models.active_task import ActiveTaskModel # type: ignore +from spiffworkflow_backend.models.active_task import ActiveTaskModel from spiffworkflow_backend.models.group import GroupModel from spiffworkflow_backend.models.permission_assignment import PermissionAssignmentModel from spiffworkflow_backend.models.permission_target import PermissionTargetModel diff --git a/src/spiffworkflow_backend/services/process_instance_service.py b/src/spiffworkflow_backend/services/process_instance_service.py index 5eeb09e9..5244cba6 100644 --- a/src/spiffworkflow_backend/services/process_instance_service.py +++ b/src/spiffworkflow_backend/services/process_instance_service.py @@ -9,7 +9,7 @@ from flask import current_app from flask_bpmn.api.api_error import ApiError from flask_bpmn.models.db import db from SpiffWorkflow.task import Task as SpiffTask # type: ignore -from SpiffWorkflow.util.deep_merge import DeepMerge +from SpiffWorkflow.util.deep_merge import DeepMerge # type: ignore from spiffworkflow_backend.models.process_instance import ProcessInstanceApi from spiffworkflow_backend.models.process_instance import ProcessInstanceModel diff --git a/tests/spiffworkflow_backend/unit/test_authorization_service.py b/tests/spiffworkflow_backend/unit/test_authorization_service.py index 6d7cd5e9..44ea6d07 100644 --- a/tests/spiffworkflow_backend/unit/test_authorization_service.py +++ b/tests/spiffworkflow_backend/unit/test_authorization_service.py @@ -46,7 +46,7 @@ class TestAuthorizationService(BaseTest): assert len(users["testadmin1"].groups) == 1 assert users["testadmin1"].groups[0].identifier == "admin" assert len(users["testuser1"].groups) == 1 - assert users["testuser1"].groups[0].identifier == "finance" + assert users["testuser1"].groups[0].identifier == "Finance Team" assert len(users["testuser2"].groups) == 2 self.assert_user_has_permission( diff --git a/tests/spiffworkflow_backend/unit/test_process_instance_processor.py b/tests/spiffworkflow_backend/unit/test_process_instance_processor.py index dcd29016..453f21d2 100644 --- a/tests/spiffworkflow_backend/unit/test_process_instance_processor.py +++ b/tests/spiffworkflow_backend/unit/test_process_instance_processor.py @@ -52,10 +52,10 @@ class TestProcessInstanceProcessor(BaseTest): with_db_and_bpmn_file_cleanup: None, ) -> None: """Test_sets_permission_correctly_on_active_task.""" - testuser1 = self.find_or_create_user("testuser1") - testuser2 = self.find_or_create_user("testuser2") - assert testuser1.principal is not None - assert testuser2.principal is not None + initiator_user = self.find_or_create_user("initiator_user") + finance_user = self.find_or_create_user("testuser2") + assert initiator_user.principal is not None + assert finance_user.principal is not None AuthorizationService.import_permissions_from_yaml_file() finance_group = GroupModel.query.filter_by(identifier="Finance Team").first() @@ -65,7 +65,7 @@ class TestProcessInstanceProcessor(BaseTest): process_model_id="model_with_lanes", bpmn_file_name="lanes.bpmn" ) process_instance = self.create_process_instance_from_process_model( - process_model=process_model, user=testuser1 + process_model=process_model, user=initiator_user ) processor = ProcessInstanceProcessor(process_instance) processor.do_engine_steps(save=True) @@ -74,41 +74,41 @@ class TestProcessInstanceProcessor(BaseTest): active_task = process_instance.active_tasks[0] assert active_task.lane_assignment_id is None assert len(active_task.potential_owners) == 1 - assert active_task.potential_owners[0] == testuser1 + assert active_task.potential_owners[0] == initiator_user spiff_task = processor.__class__.get_task_by_bpmn_identifier( active_task.task_name, processor.bpmn_process_instance ) with pytest.raises(UserDoesNotHaveAccessToTaskError): ProcessInstanceService.complete_form_task( - processor, spiff_task, {}, testuser2 + processor, spiff_task, {}, finance_user ) - ProcessInstanceService.complete_form_task(processor, spiff_task, {}, testuser1) + ProcessInstanceService.complete_form_task(processor, spiff_task, {}, initiator_user) assert len(process_instance.active_tasks) == 1 active_task = process_instance.active_tasks[0] assert active_task.lane_assignment_id == finance_group.id assert len(active_task.potential_owners) == 1 - assert active_task.potential_owners[0] == testuser2 + assert active_task.potential_owners[0] == finance_user spiff_task = processor.__class__.get_task_by_bpmn_identifier( active_task.task_name, processor.bpmn_process_instance ) with pytest.raises(UserDoesNotHaveAccessToTaskError): ProcessInstanceService.complete_form_task( - processor, spiff_task, {}, testuser1 + processor, spiff_task, {}, initiator_user ) - ProcessInstanceService.complete_form_task(processor, spiff_task, {}, testuser2) + ProcessInstanceService.complete_form_task(processor, spiff_task, {}, finance_user) assert len(process_instance.active_tasks) == 1 active_task = process_instance.active_tasks[0] assert active_task.lane_assignment_id is None assert len(active_task.potential_owners) == 1 - assert active_task.potential_owners[0] == testuser1 + assert active_task.potential_owners[0] == initiator_user spiff_task = processor.__class__.get_task_by_bpmn_identifier( active_task.task_name, processor.bpmn_process_instance ) - ProcessInstanceService.complete_form_task(processor, spiff_task, {}, testuser1) + ProcessInstanceService.complete_form_task(processor, spiff_task, {}, initiator_user) assert process_instance.status == ProcessInstanceStatus.complete.value From afdf81a0316cfcf6168bc7899a6119297efd26f8 Mon Sep 17 00:00:00 2001 From: jasquat Date: Fri, 21 Oct 2022 15:19:51 -0400 Subject: [PATCH 09/11] added test to ensure users can update their own task w/ burnettk --- .../config/permissions/testing.yml | 17 +++- .../services/authorization_service.py | 43 ++++++--- .../integration/test_process_api.py | 90 +++++++++++++++++++ .../unit/test_authorization_service.py | 13 +-- .../unit/test_process_instance_processor.py | 12 ++- 5 files changed, 156 insertions(+), 19 deletions(-) diff --git a/src/spiffworkflow_backend/config/permissions/testing.yml b/src/spiffworkflow_backend/config/permissions/testing.yml index a26bab59..333ced14 100644 --- a/src/spiffworkflow_backend/config/permissions/testing.yml +++ b/src/spiffworkflow_backend/config/permissions/testing.yml @@ -1,3 +1,5 @@ +default_group: everybody + groups: admin: users: [testadmin1, testadmin2] @@ -21,8 +23,21 @@ permissions: allowed_permissions: [read] uri: /* - finance-admin: + tasks-crud: + groups: [everybody] + users: [] + allowed_permissions: [create, read, update, delete] + uri: /v1.0/tasks/* + + # TODO: all uris should really have the same structure + finance-admin-group: groups: ["Finance Team"] users: [testuser4] allowed_permissions: [create, read, update, delete] uri: /v1.0/process-groups/finance/* + + finance-admin-model: + groups: ["Finance Team"] + users: [testuser4] + allowed_permissions: [create, read, update, delete] + uri: /v1.0/process-models/finance/* diff --git a/src/spiffworkflow_backend/services/authorization_service.py b/src/spiffworkflow_backend/services/authorization_service.py index b32d1945..eeb244a1 100644 --- a/src/spiffworkflow_backend/services/authorization_service.py +++ b/src/spiffworkflow_backend/services/authorization_service.py @@ -106,6 +106,19 @@ class AuthorizationService: db.session.commit() cls.import_permissions_from_yaml_file() + @classmethod + def associate_user_with_group(cls, user: UserModel, group: GroupModel) -> None: + """Associate_user_with_group.""" + user_group_assignemnt = UserGroupAssignmentModel.query.filter_by( + user_id=user.id, group_id=group.id + ).first() + if user_group_assignemnt is None: + user_group_assignemnt = UserGroupAssignmentModel( + user_id=user.id, group_id=group.id + ) + db.session.add(user_group_assignemnt) + db.session.commit() + @classmethod def import_permissions_from_yaml_file( cls, raise_if_missing_user: bool = False @@ -122,6 +135,20 @@ class AuthorizationService: with open(current_app.config["PERMISSIONS_FILE_FULLPATH"]) as file: permission_configs = yaml.safe_load(file) + default_group = None + if "default_group" in permission_configs: + default_group_identifier = permission_configs["default_group"] + default_group = GroupModel.query.filter_by( + identifier=default_group_identifier + ).first() + if default_group is None: + default_group = GroupModel(identifier=default_group_identifier) + db.session.add(default_group) + db.session.commit() + UserService.create_principal( + default_group.id, id_column_name="group_id" + ) + if "groups" in permission_configs: for group_identifier, group_config in permission_configs["groups"].items(): group = GroupModel.query.filter_by(identifier=group_identifier).first() @@ -140,15 +167,7 @@ class AuthorizationService: ) ) continue - user_group_assignemnt = UserGroupAssignmentModel.query.filter_by( - user_id=user.id, group_id=group.id - ).first() - if user_group_assignemnt is None: - user_group_assignemnt = UserGroupAssignmentModel( - user_id=user.id, group_id=group.id - ) - db.session.add(user_group_assignemnt) - db.session.commit() + cls.associate_user_with_group(user, group) if "permissions" in permission_configs: for _permission_identifier, permission_config in permission_configs[ @@ -188,6 +207,10 @@ class AuthorizationService: principal, permission_target, allowed_permission ) + if default_group is not None: + for user in UserModel.query.all(): + cls.associate_user_with_group(user, default_group) + @classmethod def create_permission_for_principal( cls, @@ -295,7 +318,7 @@ class AuthorizationService: raise ApiError( error_code="unauthorized", - message="User is not authorized to perform requested action.", + message=f"User {g.user.username} is not authorized to perform requested action: {permission_string} - {request.path}", status_code=403, ) diff --git a/tests/spiffworkflow_backend/integration/test_process_api.py b/tests/spiffworkflow_backend/integration/test_process_api.py index 36bcf4ac..2d79b652 100644 --- a/tests/spiffworkflow_backend/integration/test_process_api.py +++ b/tests/spiffworkflow_backend/integration/test_process_api.py @@ -15,6 +15,7 @@ from spiffworkflow_backend.exceptions.process_entity_not_found_error import ( ProcessEntityNotFoundError, ) from spiffworkflow_backend.models.active_task import ActiveTaskModel +from spiffworkflow_backend.models.group import GroupModel from spiffworkflow_backend.models.process_group import ProcessGroup from spiffworkflow_backend.models.process_group import ProcessGroupSchema from spiffworkflow_backend.models.process_instance import ProcessInstanceModel @@ -26,6 +27,7 @@ from spiffworkflow_backend.models.process_model import NotificationType from spiffworkflow_backend.models.process_model import ProcessModelInfoSchema from spiffworkflow_backend.models.task_event import TaskEventModel from spiffworkflow_backend.models.user import UserModel +from spiffworkflow_backend.services.authorization_service import AuthorizationService from spiffworkflow_backend.services.file_system_service import FileSystemService from spiffworkflow_backend.services.process_instance_processor import ( ProcessInstanceProcessor, @@ -1772,6 +1774,94 @@ class TestProcessApi(BaseTest): assert response.json is not None assert len(response.json["results"]) == 2 + def test_correct_user_can_get_and_update_a_task( + self, + app: Flask, + client: FlaskClient, + with_db_and_bpmn_file_cleanup: None, + with_super_admin_user: UserModel, + ) -> None: + """Test_correct_user_can_get_and_update_a_task.""" + initiator_user = self.find_or_create_user("testuser4") + finance_user = self.find_or_create_user("testuser2") + assert initiator_user.principal is not None + assert finance_user.principal is not None + AuthorizationService.import_permissions_from_yaml_file() + + finance_group = GroupModel.query.filter_by(identifier="Finance Team").first() + assert finance_group is not None + + process_model = load_test_spec( + process_model_id="model_with_lanes", + bpmn_file_name="lanes.bpmn", + process_group_id="finance", + ) + + response = self.create_process_instance( + client, + process_model.process_group_id, + process_model.id, + headers=self.logged_in_headers(initiator_user), + ) + assert response.status_code == 201 + + assert response.json is not None + process_instance_id = response.json["id"] + response = client.post( + f"/v1.0/process-models/{process_model.process_group_id}/{process_model.id}/process-instances/{process_instance_id}/run", + headers=self.logged_in_headers(initiator_user), + ) + assert response.status_code == 200 + + response = client.get( + "/v1.0/tasks", + headers=self.logged_in_headers(finance_user), + ) + assert response.status_code == 200 + assert response.json is not None + assert len(response.json["results"]) == 0 + + response = client.get( + "/v1.0/tasks", + headers=self.logged_in_headers(initiator_user), + ) + assert response.status_code == 200 + assert response.json is not None + assert len(response.json["results"]) == 1 + + task_id = response.json["results"][0]["id"] + assert task_id is not None + + response = client.put( + f"/v1.0/tasks/{process_instance_id}/{task_id}", + headers=self.logged_in_headers(finance_user), + ) + assert response.status_code == 500 + assert response.json + assert "UserDoesNotHaveAccessToTaskError" in response.json["message"] + + response = client.put( + f"/v1.0/tasks/{process_instance_id}/{task_id}", + headers=self.logged_in_headers(initiator_user), + ) + assert response.status_code == 202 + + response = client.get( + "/v1.0/tasks", + headers=self.logged_in_headers(initiator_user), + ) + assert response.status_code == 200 + assert response.json is not None + assert len(response.json["results"]) == 0 + + response = client.get( + "/v1.0/tasks", + headers=self.logged_in_headers(finance_user), + ) + assert response.status_code == 200 + assert response.json is not None + assert len(response.json["results"]) == 1 + # TODO: test the auth callback endpoint # def test_can_store_authentication_secret( # self, app: Flask, client: FlaskClient, with_db_and_bpmn_file_cleanup: None diff --git a/tests/spiffworkflow_backend/unit/test_authorization_service.py b/tests/spiffworkflow_backend/unit/test_authorization_service.py index 44ea6d07..5df7eaf2 100644 --- a/tests/spiffworkflow_backend/unit/test_authorization_service.py +++ b/tests/spiffworkflow_backend/unit/test_authorization_service.py @@ -43,11 +43,13 @@ class TestAuthorizationService(BaseTest): users[username] = user AuthorizationService.import_permissions_from_yaml_file() - assert len(users["testadmin1"].groups) == 1 - assert users["testadmin1"].groups[0].identifier == "admin" - assert len(users["testuser1"].groups) == 1 - assert users["testuser1"].groups[0].identifier == "Finance Team" - assert len(users["testuser2"].groups) == 2 + assert len(users["testadmin1"].groups) == 2 + testadmin1_group_identifiers = sorted([g.identifier for g in users["testadmin1"].groups]) + assert testadmin1_group_identifiers == ["admin", "everybody"] + assert len(users["testuser1"].groups) == 2 + testuser1_group_identifiers = sorted([g.identifier for g in users["testuser1"].groups]) + assert testuser1_group_identifiers == ["Finance Team", "everybody"] + assert len(users["testuser2"].groups) == 3 self.assert_user_has_permission( users["testuser1"], "update", "/v1.0/process-groups/finance/model1" @@ -61,6 +63,7 @@ class TestAuthorizationService(BaseTest): self.assert_user_has_permission( users["testuser4"], "update", "/v1.0/process-groups/finance/model1" ) + # via the user, not the group self.assert_user_has_permission( users["testuser4"], "read", "/v1.0/process-groups/finance/model1" ) diff --git a/tests/spiffworkflow_backend/unit/test_process_instance_processor.py b/tests/spiffworkflow_backend/unit/test_process_instance_processor.py index 453f21d2..b7dffc4b 100644 --- a/tests/spiffworkflow_backend/unit/test_process_instance_processor.py +++ b/tests/spiffworkflow_backend/unit/test_process_instance_processor.py @@ -83,7 +83,9 @@ class TestProcessInstanceProcessor(BaseTest): ProcessInstanceService.complete_form_task( processor, spiff_task, {}, finance_user ) - ProcessInstanceService.complete_form_task(processor, spiff_task, {}, initiator_user) + ProcessInstanceService.complete_form_task( + processor, spiff_task, {}, initiator_user + ) assert len(process_instance.active_tasks) == 1 active_task = process_instance.active_tasks[0] @@ -99,7 +101,9 @@ class TestProcessInstanceProcessor(BaseTest): processor, spiff_task, {}, initiator_user ) - ProcessInstanceService.complete_form_task(processor, spiff_task, {}, finance_user) + ProcessInstanceService.complete_form_task( + processor, spiff_task, {}, finance_user + ) assert len(process_instance.active_tasks) == 1 active_task = process_instance.active_tasks[0] assert active_task.lane_assignment_id is None @@ -109,6 +113,8 @@ class TestProcessInstanceProcessor(BaseTest): spiff_task = processor.__class__.get_task_by_bpmn_identifier( active_task.task_name, processor.bpmn_process_instance ) - ProcessInstanceService.complete_form_task(processor, spiff_task, {}, initiator_user) + ProcessInstanceService.complete_form_task( + processor, spiff_task, {}, initiator_user + ) assert process_instance.status == ProcessInstanceStatus.complete.value From d1571c582a7c1a147daf7661cd39bb38949da9e4 Mon Sep 17 00:00:00 2001 From: jasquat Date: Fri, 21 Oct 2022 15:22:51 -0400 Subject: [PATCH 10/11] added default group to demo and development w/ burnettk --- src/spiffworkflow_backend/config/permissions/demo.yml | 2 ++ src/spiffworkflow_backend/config/permissions/development.yml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/spiffworkflow_backend/config/permissions/demo.yml b/src/spiffworkflow_backend/config/permissions/demo.yml index 283a4cb9..c0cacf33 100644 --- a/src/spiffworkflow_backend/config/permissions/demo.yml +++ b/src/spiffworkflow_backend/config/permissions/demo.yml @@ -1,3 +1,5 @@ +default_group: everybody + groups: admin: users: diff --git a/src/spiffworkflow_backend/config/permissions/development.yml b/src/spiffworkflow_backend/config/permissions/development.yml index ed71a503..bdf8b02e 100644 --- a/src/spiffworkflow_backend/config/permissions/development.yml +++ b/src/spiffworkflow_backend/config/permissions/development.yml @@ -1,3 +1,5 @@ +default_group: everybody + groups: admin: users: From 3a382b4085bf95832204d3fc1a2c4e7a77b66140 Mon Sep 17 00:00:00 2001 From: jasquat Date: Fri, 21 Oct 2022 16:13:30 -0400 Subject: [PATCH 11/11] add new user to active task if appropriate w/ burnettk --- src/spiffworkflow_backend/routes/user.py | 45 ++--------------- .../services/authorization_service.py | 44 ++++++++++++++++ .../services/process_instance_processor.py | 10 ---- .../services/user_service.py | 16 ++++++ .../unit/test_authorization_service.py | 50 ++++++++++++++++++- 5 files changed, 113 insertions(+), 52 deletions(-) diff --git a/src/spiffworkflow_backend/routes/user.py b/src/spiffworkflow_backend/routes/user.py index 60e1814b..aa5bcdd8 100644 --- a/src/spiffworkflow_backend/routes/user.py +++ b/src/spiffworkflow_backend/routes/user.py @@ -203,7 +203,6 @@ def login_return(code: str, state: str, session_state: str) -> Optional[Response """Login_return.""" state_dict = ast.literal_eval(base64.b64decode(state).decode("utf-8")) state_redirect_url = state_dict["redirect_url"] - auth_token_object = AuthenticationService().get_auth_token_object(code) if "id_token" in auth_token_object: id_token = auth_token_object["id_token"] @@ -213,46 +212,12 @@ def login_return(code: str, state: str, session_state: str) -> Optional[Response auth_token_object["access_token"] ) if user_info and "error" not in user_info: - user_model = ( - UserModel.query.filter(UserModel.service == "open_id") - .filter(UserModel.service_id == user_info["sub"]) - .first() + user_model = AuthorizationService.create_user_from_sign_in(user_info) + g.user = user_model.id + g.token = auth_token_object["id_token"] + AuthenticationService.store_refresh_token( + user_model.id, auth_token_object["refresh_token"] ) - - if user_model is None: - current_app.logger.debug("create_user in login_return") - name = username = email = "" - if "name" in user_info: - name = user_info["name"] - if "username" in user_info: - username = user_info["username"] - elif "preferred_username" in user_info: - username = user_info["preferred_username"] - if "email" in user_info: - email = user_info["email"] - user_model = UserService().create_user( - service="open_id", - service_id=user_info["sub"], - name=name, - username=username, - email=email, - ) - - if user_model: - g.user = user_model.id - g.token = auth_token_object["id_token"] - AuthenticationService.store_refresh_token( - user_model.id, auth_token_object["refresh_token"] - ) - - # this may eventually get too slow. - # when it does, be careful about backgrounding, because - # the user will immediately need permissions to use the site. - # we are also a little apprehensive about pre-creating users - # before the user signs in, because we won't know things like - # the external service user identifier. - AuthorizationService.import_permissions_from_yaml_file() - redirect_url = ( f"{state_redirect_url}?" + f"access_token={auth_token_object['access_token']}&" diff --git a/src/spiffworkflow_backend/services/authorization_service.py b/src/spiffworkflow_backend/services/authorization_service.py index eeb244a1..8796fa99 100644 --- a/src/spiffworkflow_backend/services/authorization_service.py +++ b/src/spiffworkflow_backend/services/authorization_service.py @@ -424,6 +424,50 @@ class AuthorizationService: ) return True + @classmethod + def create_user_from_sign_in(cls, user_info: dict) -> UserModel: + """Create_user_from_sign_in.""" + is_new_user = False + user_model = ( + UserModel.query.filter(UserModel.service == "open_id") + .filter(UserModel.service_id == user_info["sub"]) + .first() + ) + + if user_model is None: + current_app.logger.debug("create_user in login_return") + is_new_user = True + name = username = email = "" + if "name" in user_info: + name = user_info["name"] + if "username" in user_info: + username = user_info["username"] + elif "preferred_username" in user_info: + username = user_info["preferred_username"] + if "email" in user_info: + email = user_info["email"] + user_model = UserService().create_user( + service="open_id", + service_id=user_info["sub"], + name=name, + username=username, + email=email, + ) + + # this may eventually get too slow. + # when it does, be careful about backgrounding, because + # the user will immediately need permissions to use the site. + # we are also a little apprehensive about pre-creating users + # before the user signs in, because we won't know things like + # the external service user identifier. + cls.import_permissions_from_yaml_file() + + if is_new_user: + UserService.add_user_to_active_tasks_if_appropriate(user_model) + + # this cannot be None so ignore mypy + return user_model # type: ignore + class KeycloakAuthorization: """Interface with Keycloak server.""" diff --git a/src/spiffworkflow_backend/services/process_instance_processor.py b/src/spiffworkflow_backend/services/process_instance_processor.py index dedf4396..2aca1ac6 100644 --- a/src/spiffworkflow_backend/services/process_instance_processor.py +++ b/src/spiffworkflow_backend/services/process_instance_processor.py @@ -533,16 +533,6 @@ class ProcessInstanceProcessor: task_spec = ready_or_waiting_task.task_spec if not self.bpmn_process_instance._is_engine_task(task_spec): ready_or_waiting_task.data["current_user"]["id"] - # principal = PrincipalModel.query.filter_by(user_id=user_id).first() - # if principal is None: - # raise ( - # ApiError( - # error_code="principal_not_found", - # message=f"Principal not found from user id: {user_id}", - # status_code=400, - # ) - # ) - # import pdb; pdb.set_trace() task_lane = "process_initiator" if task_spec.lane is not None: task_lane = task_spec.lane diff --git a/src/spiffworkflow_backend/services/user_service.py b/src/spiffworkflow_backend/services/user_service.py index 3ae4755c..6480fb0d 100644 --- a/src/spiffworkflow_backend/services/user_service.py +++ b/src/spiffworkflow_backend/services/user_service.py @@ -7,6 +7,8 @@ from flask import g from flask_bpmn.api.api_error import ApiError from flask_bpmn.models.db import db +from spiffworkflow_backend.models.active_task import ActiveTaskModel +from spiffworkflow_backend.models.active_task_user import ActiveTaskUserModel from spiffworkflow_backend.models.group import GroupModel from spiffworkflow_backend.models.principal import PrincipalModel from spiffworkflow_backend.models.user import AdminSessionModel @@ -313,3 +315,17 @@ class UserService: if user: return user return None + + @classmethod + def add_user_to_active_tasks_if_appropriate(cls, user: UserModel) -> None: + """Add_user_to_active_tasks_if_appropriate.""" + group_ids = [g.id for g in user.groups] + active_tasks = ActiveTaskModel.query.filter( + ActiveTaskModel.lane_assignment_id.in_(group_ids) # type: ignore + ).all() + for active_task in active_tasks: + active_task_user = ActiveTaskUserModel( + user_id=user.id, active_task_id=active_task.id + ) + db.session.add(active_task_user) + db.session.commit() diff --git a/tests/spiffworkflow_backend/unit/test_authorization_service.py b/tests/spiffworkflow_backend/unit/test_authorization_service.py index 5df7eaf2..fe54686d 100644 --- a/tests/spiffworkflow_backend/unit/test_authorization_service.py +++ b/tests/spiffworkflow_backend/unit/test_authorization_service.py @@ -2,9 +2,16 @@ import pytest from flask import Flask from tests.spiffworkflow_backend.helpers.base_test import BaseTest +from tests.spiffworkflow_backend.helpers.test_data import load_test_spec from spiffworkflow_backend.models.user import UserNotFoundError from spiffworkflow_backend.services.authorization_service import AuthorizationService +from spiffworkflow_backend.services.process_instance_processor import ( + ProcessInstanceProcessor, +) +from spiffworkflow_backend.services.process_instance_service import ( + ProcessInstanceService, +) class TestAuthorizationService(BaseTest): @@ -44,10 +51,14 @@ class TestAuthorizationService(BaseTest): AuthorizationService.import_permissions_from_yaml_file() assert len(users["testadmin1"].groups) == 2 - testadmin1_group_identifiers = sorted([g.identifier for g in users["testadmin1"].groups]) + testadmin1_group_identifiers = sorted( + [g.identifier for g in users["testadmin1"].groups] + ) assert testadmin1_group_identifiers == ["admin", "everybody"] assert len(users["testuser1"].groups) == 2 - testuser1_group_identifiers = sorted([g.identifier for g in users["testuser1"].groups]) + testuser1_group_identifiers = sorted( + [g.identifier for g in users["testuser1"].groups] + ) assert testuser1_group_identifiers == ["Finance Team", "everybody"] assert len(users["testuser2"].groups) == 3 @@ -76,3 +87,38 @@ class TestAuthorizationService(BaseTest): self.assert_user_has_permission( users["testuser2"], "read", "/v1.0/process-groups/" ) + + def test_user_can_be_added_to_active_task_on_first_login( + self, app: Flask, with_db_and_bpmn_file_cleanup: None + ) -> None: + """Test_user_can_be_added_to_active_task_on_first_login.""" + initiator_user = self.find_or_create_user("initiator_user") + assert initiator_user.principal is not None + AuthorizationService.import_permissions_from_yaml_file() + + process_model = load_test_spec( + process_model_id="model_with_lanes", bpmn_file_name="lanes.bpmn" + ) + process_instance = self.create_process_instance_from_process_model( + process_model=process_model, user=initiator_user + ) + processor = ProcessInstanceProcessor(process_instance) + processor.do_engine_steps(save=True) + active_task = process_instance.active_tasks[0] + spiff_task = processor.__class__.get_task_by_bpmn_identifier( + active_task.task_name, processor.bpmn_process_instance + ) + ProcessInstanceService.complete_form_task( + processor, spiff_task, {}, initiator_user + ) + + active_task = process_instance.active_tasks[0] + spiff_task = processor.__class__.get_task_by_bpmn_identifier( + active_task.task_name, processor.bpmn_process_instance + ) + finance_user = AuthorizationService.create_user_from_sign_in( + {"username": "testuser2", "sub": "open_id"} + ) + ProcessInstanceService.complete_form_task( + processor, spiff_task, {}, finance_user + )