From f0b608789b6cdc3ef4303efac053746c98571a48 Mon Sep 17 00:00:00 2001 From: jasquat Date: Fri, 21 Oct 2022 16:28:09 -0400 Subject: [PATCH] Squashed 'spiffworkflow-backend/' changes from 22766521..b978f502 b978f502 Merge pull request #148 from sartography/feature/respect_lanes e7f2d23e merged in main and resolved conflicts w/ burnettk 3a382b40 add new user to active task if appropriate w/ burnettk d1571c58 added default group to demo and development w/ burnettk afdf81a0 added test to ensure users can update their own task w/ burnettk 6e13ee4e Merge remote-tracking branch 'origin/main' into feature/respect_lanes 645e4d8f updated flask-bpmn for sentry and fixed for pyl w/ burnettk 57248eab fixed submitting and getting user tasks w/ burnettk c9b086d9 ignore all null-ls w/ burnettk 63796d50 fixed model load issue w/ burnettk 49eefc56 some precommit stuff w/ burnettk 579f0902 Merge remote-tracking branch 'origin/main' into feature/respect_lanes 47bdb99f fixed swagger ui w/ burnettk 5128f752 merged in main and resolved conflicts be1f4bcc added validation to ensure user has access to task w/ burnettk a387b787 added some code to respect lanes in a process model w/ burnettk git-subtree-dir: spiffworkflow-backend git-subtree-split: b978f502a0f840554940125c6b729b9b99086001 --- .gitignore | 2 +- conftest.py | 3 + .../{e12e98d4e7e4_.py => 4ba2ed52a63a_.py} | 26 +- poetry.lock | 38 +-- pyproject.toml | 2 +- src/spiffworkflow_backend/__init__.py | 3 - .../config/development.py | 2 +- .../config/permissions/demo.yml | 2 + .../{staging.yml => development.yml} | 2 + .../config/permissions/testing.yml | 23 +- .../load_database_models.py | 4 +- .../models/active_task.py | 25 +- .../models/active_task_user.py | 32 ++ src/spiffworkflow_backend/models/principal.py | 4 + src/spiffworkflow_backend/models/user.py | 2 + .../routes/process_api_blueprint.py | 57 +--- src/spiffworkflow_backend/routes/user.py | 45 +-- .../services/authorization_service.py | 322 +++++++----------- .../services/process_instance_processor.py | 68 ++-- .../services/process_instance_service.py | 10 +- .../services/user_service.py | 16 + tests/data/model_with_lanes/lanes.bpmn | 103 ++++++ .../helpers/base_test.py | 9 +- .../integration/test_process_api.py | 90 +++++ .../unit/test_authorization_service.py | 65 +++- .../unit/test_process_instance_processor.py | 84 +++++ 26 files changed, 684 insertions(+), 355 deletions(-) rename migrations/versions/{e12e98d4e7e4_.py => 4ba2ed52a63a_.py} (94%) rename src/spiffworkflow_backend/config/permissions/{staging.yml => development.yml} (96%) create mode 100644 src/spiffworkflow_backend/models/active_task_user.py create mode 100644 tests/data/model_with_lanes/lanes.bpmn diff --git a/.gitignore b/.gitignore index 22fea4c91..58cb14347 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_* diff --git a/conftest.py b/conftest.py index 242f75a0f..17daaaa0d 100644 --- a/conftest.py +++ b/conftest.py @@ -9,6 +9,7 @@ from flask_bpmn.models.db import SpiffworkflowBaseDBModel 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 ( @@ -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/versions/e12e98d4e7e4_.py b/migrations/versions/4ba2ed52a63a_.py similarity index 94% rename from migrations/versions/e12e98d4e7e4_.py rename to migrations/versions/4ba2ed52a63a_.py index bf7fff66f..e0578a3a9 100644 --- a/migrations/versions/e12e98d4e7e4_.py +++ b/migrations/versions/4ba2ed52a63a_.py @@ -1,8 +1,8 @@ """empty message -Revision ID: e12e98d4e7e4 +Revision ID: 4ba2ed52a63a Revises: -Create Date: 2022-10-21 08:53:52.815491 +Create Date: 2022-10-21 09:31:30.520942 """ from alembic import op @@ -10,7 +10,7 @@ import sqlalchemy as sa # revision identifiers, used by Alembic. -revision = 'e12e98d4e7e4' +revision = '4ba2ed52a63a' down_revision = None branch_labels = None depends_on = None @@ -165,7 +165,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), @@ -176,7 +177,8 @@ def upgrade(): sa.Column('task_type', sa.String(length=50), nullable=True), sa.Column('task_status', sa.String(length=50), nullable=True), sa.Column('process_model_display_name', sa.String(length=255), 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') @@ -279,6 +281,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), @@ -312,6 +325,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/poetry.lock b/poetry.lock index 844b2d1a7..2d1802051 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)"] @@ -1891,19 +1889,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"] @@ -3323,8 +3321,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 f932da68c..dcbecc58d 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 1358c06a0..9a0903305 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/development.py b/src/spiffworkflow_backend/config/development.py index 151868e51..a7152177d 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/demo.yml b/src/spiffworkflow_backend/config/permissions/demo.yml index 283a4cb91..c0cacf33b 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/staging.yml b/src/spiffworkflow_backend/config/permissions/development.yml similarity index 96% rename from src/spiffworkflow_backend/config/permissions/staging.yml rename to src/spiffworkflow_backend/config/permissions/development.yml index ed71a5038..bdf8b02e7 100644 --- a/src/spiffworkflow_backend/config/permissions/staging.yml +++ b/src/spiffworkflow_backend/config/permissions/development.yml @@ -1,3 +1,5 @@ +default_group: everybody + groups: admin: users: diff --git a/src/spiffworkflow_backend/config/permissions/testing.yml b/src/spiffworkflow_backend/config/permissions/testing.yml index 0d3f3806e..333ced14b 100644 --- a/src/spiffworkflow_backend/config/permissions/testing.yml +++ b/src/spiffworkflow_backend/config/permissions/testing.yml @@ -1,8 +1,10 @@ +default_group: everybody + groups: admin: users: [testadmin1, testadmin2] - finance: + Finance Team: users: [testuser1, testuser2] hr: @@ -16,13 +18,26 @@ permissions: uri: /* read-all: - groups: [finance, hr, admin] + groups: ["Finance Team", hr, admin] users: [] allowed_permissions: [read] uri: /* - finance-admin: - groups: [finance] + 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/load_database_models.py b/src/spiffworkflow_backend/load_database_models.py index c064613af..12b33a7e5 100644 --- a/src/spiffworkflow_backend/load_database_models.py +++ b/src/spiffworkflow_backend/load_database_models.py @@ -10,10 +10,11 @@ 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 @@ -38,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 diff --git a/src/spiffworkflow_backend/models/active_task.py b/src/spiffworkflow_backend/models/active_task.py index cdd79aa6c..c576f39e9 100644 --- a/src/spiffworkflow_backend/models/active_task.py +++ b/src/spiffworkflow_backend/models/active_task.py @@ -2,6 +2,7 @@ from __future__ import annotations from dataclasses import dataclass +from typing import TYPE_CHECKING from flask_bpmn.models.db import db from flask_bpmn.models.db import SpiffworkflowBaseDBModel @@ -9,9 +10,16 @@ from sqlalchemy import ForeignKey from sqlalchemy.orm import relationship from sqlalchemy.orm import RelationshipProperty -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 ( # noqa: F401 + ActiveTaskUserModel, + ) @dataclass @@ -25,14 +33,13 @@ 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)) @@ -46,6 +53,14 @@ class ActiveTaskModel(SpiffworkflowBaseDBModel): task_status = db.Column(db.String(50)) process_model_display_name = db.Column(db.String(255)) + 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 000000000..f194c38e4 --- /dev/null +++ b/src/spiffworkflow_backend/models/active_task_user.py @@ -0,0 +1,32 @@ +"""Active_task_user.""" +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): + """ActiveTaskUserModel.""" + + __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/principal.py b/src/spiffworkflow_backend/models/principal.py index fbe059306..c7efa8609 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/models/user.py b/src/spiffworkflow_backend/models/user.py index 30d60a6f9..22cdfb69f 100644 --- a/src/spiffworkflow_backend/models/user.py +++ b/src/spiffworkflow_backend/models/user.py @@ -1,4 +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 ad7e9b374..af73c3cd0 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() - 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,9 +1123,13 @@ def task_submit( ProcessInstanceService.update_task_assignments(processor) - 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 @@ -1293,30 +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.""" - 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/routes/user.py b/src/spiffworkflow_backend/routes/user.py index 60e1814b1..aa5bcdd8b 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 b9353686e..8796fa992 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 # type: ignore from sqlalchemy import text +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 @@ -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.""" @@ -93,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 @@ -109,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() @@ -127,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[ @@ -164,14 +196,20 @@ 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 + ) + + 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( @@ -202,6 +240,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 @@ -218,7 +257,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 @@ -277,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, ) @@ -359,196 +400,73 @@ 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 + 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 - # 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 + @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() + ) - # 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 + 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, + ) - # 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 + # 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() - # 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") + if is_new_user: + UserService.add_user_to_active_tasks_if_appropriate(user_model) - # 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 + # this cannot be None so ignore mypy + return user_model # type: ignore class KeycloakAuthorization: diff --git a/src/spiffworkflow_backend/services/process_instance_processor.py b/src/spiffworkflow_backend/services/process_instance_processor.py index 087a4b49a..2aca1ac67 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, @@ -67,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 @@ -108,6 +110,10 @@ class ProcessInstanceProcessorError(Exception): """ProcessInstanceProcessorError.""" +class NoPotentialOwnersForTaskError(Exception): + """NoPotentialOwnersForTaskError.""" + + class CustomBpmnScriptEngine(PythonScriptEngine): # type: ignore """This is a custom script processor that can be easily injected into Spiff Workflow. @@ -511,28 +517,46 @@ 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 - ): - 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, + 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"] + 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 +579,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), @@ -563,10 +586,17 @@ 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(), + 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 2d6999fce..5244cba69 100644 --- a/src/spiffworkflow_backend/services/process_instance_service.py +++ b/src/spiffworkflow_backend/services/process_instance_service.py @@ -19,14 +19,13 @@ 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, ) from spiffworkflow_backend.services.process_model_service import ProcessModelService -# from SpiffWorkflow.task import TaskState # type: ignore - class ProcessInstanceService: """ProcessInstanceService.""" @@ -272,6 +271,10 @@ class ProcessInstanceService: Abstracted here because we need to do it multiple times when completing all tasks in a multi-instance task. """ + AuthorizationService.assert_user_can_complete_spiff_task( + processor, spiff_task, user + ) + 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. @@ -282,8 +285,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/src/spiffworkflow_backend/services/user_service.py b/src/spiffworkflow_backend/services/user_service.py index 3ae4755cd..6480fb0d2 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/data/model_with_lanes/lanes.bpmn b/tests/data/model_with_lanes/lanes.bpmn new file mode 100644 index 000000000..3ee435013 --- /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 b586a2b0f..b7d2c5d7e 100644 --- a/tests/spiffworkflow_backend/helpers/base_test.py +++ b/tests/spiffworkflow_backend/helpers/base_test.py @@ -207,10 +207,15 @@ 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/integration/test_process_api.py b/tests/spiffworkflow_backend/integration/test_process_api.py index 36bcf4acc..2d79b6528 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 43949d602..fe54686d1 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): @@ -19,6 +26,12 @@ 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: + """Test_does_not_fail_if_user_not_created.""" + AuthorizationService.import_permissions_from_yaml_file() + def test_can_import_permissions_from_yaml( self, app: Flask, with_db_and_bpmn_file_cleanup: None ) -> None: @@ -37,11 +50,17 @@ 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" - 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" @@ -55,6 +74,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" ) @@ -67,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 + ) diff --git a/tests/spiffworkflow_backend/unit/test_process_instance_processor.py b/tests/spiffworkflow_backend/unit/test_process_instance_processor.py index 009239f7a..b7dffc4b0 100644 --- a/tests/spiffworkflow_backend/unit/test_process_instance_processor.py +++ b/tests/spiffworkflow_backend/unit/test_process_instance_processor.py @@ -1,10 +1,21 @@ """Test_process_instance_processor.""" +import pytest from flask.app 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.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, +) class TestProcessInstanceProcessor(BaseTest): @@ -34,3 +45,76 @@ 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: + """Test_sets_permission_correctly_on_active_task.""" + 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() + 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=initiator_user + ) + 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] == 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, {}, finance_user + ) + 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] == 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, {}, initiator_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 + assert len(active_task.potential_owners) == 1 + 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, {}, initiator_user + ) + + assert process_instance.status == ProcessInstanceStatus.complete.value