diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6a15caa2..411190af 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -51,6 +51,8 @@ repos: language: system types: [python] require_serial: true + # this is also specified in spiffworkflow-backend/pyproject.toml but we run pre-commit + # with all-files which ignores that exclude: "/migrations/" - id: trailing-whitespace files: ^spiffworkflow-backend/ diff --git a/spiffworkflow-backend/bin/tests-par b/spiffworkflow-backend/bin/tests-par index de53221a..05b34e01 100755 --- a/spiffworkflow-backend/bin/tests-par +++ b/spiffworkflow-backend/bin/tests-par @@ -18,4 +18,4 @@ if ! python -c "import xdist" &>/dev/null; then exit 1 fi -SPIFFWORKFLOW_BACKEND_DATABASE_TYPE=sqlite poet test -n auto -x --ff --random-order +SPIFFWORKFLOW_BACKEND_DATABASE_TYPE=sqlite poet test -n auto -x --random-order diff --git a/spiffworkflow-backend/pyproject.toml b/spiffworkflow-backend/pyproject.toml index c09bb4ef..f3e9ac79 100644 --- a/spiffworkflow-backend/pyproject.toml +++ b/spiffworkflow-backend/pyproject.toml @@ -197,6 +197,10 @@ line-length = 130 # target python 3.10 target-version = "py310" +exclude = [ + "migrations" +] + [tool.ruff.per-file-ignores] "migrations/versions/*.py" = ["E501"] "tests/**/*.py" = ["PLR2004", "S101"] # PLR2004 is about magic vars, S101 allows assert diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py index d7a56254..686ca4ef 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py @@ -708,7 +708,7 @@ def task_save_draft( task_model = _get_task_model_from_guid_or_raise(task_guid, process_instance_id) full_bpmn_process_id_path = TaskService.full_bpmn_process_path(task_model.bpmn_process, "id") - task_definition_id_path = f"{':'.join(map(str,full_bpmn_process_id_path))}:{task_model.task_definition_id}" + task_definition_id_path = f"{':'.join(map(str, full_bpmn_process_id_path))}:{task_model.task_definition_id}" task_draft_data_dict: TaskDraftDataDict = { "process_instance_id": process_instance.id, "task_definition_id_path": task_definition_id_path, @@ -974,7 +974,7 @@ def _prepare_form_data( wfe.add_note(f"Error in Json Form File '{form_file}'") api_error = ApiError.from_workflow_exception("instructions_error", str(wfe), exp=wfe) api_error.file_name = form_file - raise api_error + raise api_error from wfe try: # form_contents is a str diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py index 94b240e5..f5e6de26 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py @@ -32,7 +32,6 @@ from spiffworkflow_backend.services.authentication_service import TokenExpiredEr from spiffworkflow_backend.services.authentication_service import TokenInvalidError from spiffworkflow_backend.services.authentication_service import TokenNotProvidedError from spiffworkflow_backend.services.authentication_service import UserNotLoggedInError -from spiffworkflow_backend.services.group_service import GroupService from spiffworkflow_backend.services.user_service import UserService from sqlalchemy import and_ from sqlalchemy import or_ @@ -146,7 +145,7 @@ class AuthorizationService: permission: str = "all", auth_token_properties: dict | None = None, ) -> None: - guest_user = GroupService.find_or_create_guest_user(username=username, group_identifier=group_identifier) + guest_user = UserService.find_or_create_guest_user(username=username, group_identifier=group_identifier) if permission_target is not None: cls.add_permission_from_uri_or_macro(group_identifier, permission=permission, target=permission_target) g.user = guest_user @@ -508,14 +507,14 @@ class AuthorizationService: ) else: for desired_group_identifier in desired_group_identifiers: - GroupService.add_user_to_group(user_model, desired_group_identifier) + UserService.add_user_to_group_by_group_identifier(user_model, desired_group_identifier) current_group_identifiers = [g.identifier for g in user_model.groups] groups_to_remove_from_user = [ item for item in current_group_identifiers if item not in desired_group_identifiers ] for gtrfu in groups_to_remove_from_user: if gtrfu != current_app.config["SPIFFWORKFLOW_BACKEND_DEFAULT_USER_GROUP"]: - GroupService.remove_user_from_group(user_model, gtrfu) + UserService.remove_user_from_group(user_model, gtrfu) # this may eventually get too slow. # when it does, be careful about backgrounding, because @@ -768,7 +767,7 @@ class AuthorizationService: def add_permission_from_uri_or_macro( cls, group_identifier: str, permission: str, target: str ) -> list[PermissionAssignmentModel]: - group = GroupService.find_or_create_group(group_identifier) + group = UserService.find_or_create_group(group_identifier) permissions_to_assign = cls.explode_permissions(permission, target) permission_assignments = [] for permission_to_assign in permissions_to_assign: @@ -833,12 +832,12 @@ class AuthorizationService: default_group = None default_group_identifier = current_app.config["SPIFFWORKFLOW_BACKEND_DEFAULT_USER_GROUP"] if default_group_identifier: - default_group = GroupService.find_or_create_group(default_group_identifier) + default_group = UserService.find_or_create_group(default_group_identifier) unique_user_group_identifiers.add(default_group_identifier) for group in group_permissions: group_identifier = group["name"] - GroupService.find_or_create_group(group_identifier) + UserService.find_or_create_group(group_identifier) if not group_permissions_only: for username in group["users"]: if user_model and username != user_model.username: @@ -848,7 +847,7 @@ class AuthorizationService: "group_identifier": group_identifier, } user_to_group_identifiers.append(user_to_group_dict) - GroupService.add_user_to_group_or_add_to_waiting(username, group_identifier) + UserService.add_user_to_group_or_add_to_waiting(username, group_identifier) unique_user_group_identifiers.add(group_identifier) for group in group_permissions: group_identifier = group["name"] diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/group_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/group_service.py deleted file mode 100644 index 43288749..00000000 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/group_service.py +++ /dev/null @@ -1,63 +0,0 @@ -from spiffworkflow_backend.models.db import db -from spiffworkflow_backend.models.group import SPIFF_GUEST_GROUP -from spiffworkflow_backend.models.group import GroupModel -from spiffworkflow_backend.models.user import SPIFF_GUEST_USER -from spiffworkflow_backend.models.user import UserModel -from spiffworkflow_backend.models.user_group_assignment import UserGroupAssignmentModel -from spiffworkflow_backend.models.user_group_assignment import UserGroupAssignmentNotFoundError -from spiffworkflow_backend.services.user_service import UserService -from sqlalchemy import and_ - - -class GroupService: - @classmethod - def find_or_create_group(cls, group_identifier: str) -> GroupModel: - group: GroupModel | None = GroupModel.query.filter_by(identifier=group_identifier).first() - if group is None: - group = GroupModel(identifier=group_identifier) - db.session.add(group) - db.session.commit() - UserService.create_principal(group.id, id_column_name="group_id") - return group - - @classmethod - def add_user_to_group_or_add_to_waiting(cls, username: str | UserModel, group_identifier: str) -> None: - group = cls.find_or_create_group(group_identifier) - user = UserModel.query.filter_by(username=username).first() - if user: - UserService.add_user_to_group(user, group) - else: - UserService.add_waiting_group_assignment(username, group) - - @classmethod - def add_user_to_group(cls, user: UserModel, group_identifier: str) -> None: - group = cls.find_or_create_group(group_identifier) - UserService.add_user_to_group(user, group) - - @classmethod - def remove_user_from_group(cls, user: UserModel, group_identifier: str) -> None: - user_group_assignment = ( - UserGroupAssignmentModel.query.filter_by(user_id=user.id) - .join( - GroupModel, - and_(GroupModel.id == UserGroupAssignmentModel.group_id, GroupModel.identifier == group_identifier), - ) - .first() - ) - if user_group_assignment is None: - raise (UserGroupAssignmentNotFoundError(f"User ({user.username}) is not in group ({group_identifier})")) - db.session.delete(user_group_assignment) - db.session.commit() - - @classmethod - def find_or_create_guest_user( - cls, username: str = SPIFF_GUEST_USER, group_identifier: str = SPIFF_GUEST_GROUP - ) -> UserModel: - guest_user: UserModel | None = UserModel.query.filter_by( - username=username, service="spiff_guest_service", service_id="spiff_guest_service_id" - ).first() - if guest_user is None: - guest_user = UserService.create_user(username, "spiff_guest_service", "spiff_guest_service_id") - GroupService.add_user_to_group_or_add_to_waiting(guest_user.username, group_identifier) - - return guest_user diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py index 20881248..18aa811d 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py @@ -75,7 +75,6 @@ from spiffworkflow_backend.scripts.script import Script from spiffworkflow_backend.services.custom_parser import MyCustomParser from spiffworkflow_backend.services.element_units_service import ElementUnitsService from spiffworkflow_backend.services.file_system_service import FileSystemService -from spiffworkflow_backend.services.group_service import GroupService from spiffworkflow_backend.services.jinja_service import JinjaHelpers from spiffworkflow_backend.services.process_instance_queue_service import ProcessInstanceQueueService from spiffworkflow_backend.services.process_instance_tmp_service import ProcessInstanceTmpService @@ -815,7 +814,7 @@ class ProcessInstanceProcessor: lane_assignment_id = None if "allowGuest" in task.task_spec.extensions and task.task_spec.extensions["allowGuest"] == "true": - guest_user = GroupService.find_or_create_guest_user() + guest_user = UserService.find_or_create_guest_user() potential_owner_ids = [guest_user.id] elif re.match(r"(process.?)initiator", task_lane, re.IGNORECASE): potential_owner_ids = [self.process_instance_model.process_initiator_id] diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py index 1da99bf0..00dc7acb 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py @@ -594,7 +594,7 @@ class TaskService: cls, task_model: TaskModel, create_if_not_exists: bool = False ) -> TaskDraftDataModel | None: full_bpmn_process_id_path = cls.full_bpmn_process_path(task_model.bpmn_process, "id") - task_definition_id_path = f"{':'.join(map(str,full_bpmn_process_id_path))}:{task_model.task_definition_id}" + task_definition_id_path = f"{':'.join(map(str, full_bpmn_process_id_path))}:{task_model.task_definition_id}" task_draft_data: TaskDraftDataModel | None = TaskDraftDataModel.query.filter_by( process_instance_id=task_model.process_instance_id, task_definition_id_path=task_definition_id_path ).first() diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/user_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/user_service.py index 252afa48..6222d3f8 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/user_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/user_service.py @@ -4,14 +4,18 @@ from flask import current_app from flask import g from spiffworkflow_backend.exceptions.api_error import ApiError from spiffworkflow_backend.models.db import db +from spiffworkflow_backend.models.group import SPIFF_GUEST_GROUP from spiffworkflow_backend.models.group import GroupModel from spiffworkflow_backend.models.human_task import HumanTaskModel from spiffworkflow_backend.models.human_task_user import HumanTaskUserModel from spiffworkflow_backend.models.principal import MissingPrincipalError from spiffworkflow_backend.models.principal import PrincipalModel +from spiffworkflow_backend.models.user import SPIFF_GUEST_USER from spiffworkflow_backend.models.user import UserModel from spiffworkflow_backend.models.user_group_assignment import UserGroupAssignmentModel +from spiffworkflow_backend.models.user_group_assignment import UserGroupAssignmentNotFoundError from spiffworkflow_backend.models.user_group_assignment_waiting import UserGroupAssignmentWaitingModel +from sqlalchemy import and_ class UserService: @@ -57,7 +61,7 @@ class UserService: message=f"Could not add user {username}", ) from e cls.create_principal(user_model.id) - UserService().apply_waiting_group_assignments(user_model) + cls.apply_waiting_group_assignments(user_model) return user_model else: @@ -77,9 +81,9 @@ class UserService: def has_user() -> bool: return hasattr(g, "authenticated") and g.authenticated is True and "user" in g and bool(g.user) - @staticmethod - def current_user() -> Any: - if not UserService.has_user(): + @classmethod + def current_user(cls) -> Any: + if not cls.has_user(): raise ApiError("logged_out", "You are no longer logged in.", status_code=401) return g.user @@ -114,7 +118,7 @@ class UserService: @classmethod def add_user_to_group(cls, user: UserModel, group: GroupModel) -> None: - exists = UserGroupAssignmentModel().query.filter_by(user_id=user.id).filter_by(group_id=group.id).count() + exists = UserGroupAssignmentModel.query.filter_by(user_id=user.id).filter_by(group_id=group.id).count() if not exists: ugam = UserGroupAssignmentModel(user_id=user.id, group_id=group.id) db.session.add(ugam) @@ -129,6 +133,8 @@ class UserService: wugam = UserGroupAssignmentWaitingModel(username=username, group_id=group.id) db.session.add(wugam) db.session.commit() + + # to handle people who are already signed in if wugam.is_match_all(): for user in UserModel.query.all(): cls.add_user_to_group(user, group) @@ -208,3 +214,55 @@ class UserService: principals.append(group.principal) return principals + + @classmethod + def find_or_create_group(cls, group_identifier: str) -> GroupModel: + group: GroupModel | None = GroupModel.query.filter_by(identifier=group_identifier).first() + if group is None: + group = GroupModel(identifier=group_identifier) + db.session.add(group) + db.session.commit() + cls.create_principal(group.id, id_column_name="group_id") + return group + + @classmethod + def add_user_to_group_or_add_to_waiting(cls, username: str | UserModel, group_identifier: str) -> None: + group = cls.find_or_create_group(group_identifier) + user = UserModel.query.filter_by(username=username).first() + if user: + cls.add_user_to_group(user, group) + else: + cls.add_waiting_group_assignment(username, group) + + @classmethod + def add_user_to_group_by_group_identifier(cls, user: UserModel, group_identifier: str) -> None: + group = cls.find_or_create_group(group_identifier) + cls.add_user_to_group(user, group) + + @classmethod + def remove_user_from_group(cls, user: UserModel, group_identifier: str) -> None: + user_group_assignment = ( + UserGroupAssignmentModel.query.filter_by(user_id=user.id) + .join( + GroupModel, + and_(GroupModel.id == UserGroupAssignmentModel.group_id, GroupModel.identifier == group_identifier), + ) + .first() + ) + if user_group_assignment is None: + raise (UserGroupAssignmentNotFoundError(f"User ({user.username}) is not in group ({group_identifier})")) + db.session.delete(user_group_assignment) + db.session.commit() + + @classmethod + def find_or_create_guest_user( + cls, username: str = SPIFF_GUEST_USER, group_identifier: str = SPIFF_GUEST_GROUP + ) -> UserModel: + guest_user: UserModel | None = UserModel.query.filter_by( + username=username, service="spiff_guest_service", service_id="spiff_guest_service_id" + ).first() + if guest_user is None: + guest_user = cls.create_user(username, "spiff_guest_service", "spiff_guest_service_id") + cls.add_user_to_group_or_add_to_waiting(guest_user.username, group_identifier) + + return guest_user diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py index 0d56ba47..59b2c250 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py @@ -26,11 +26,11 @@ from spiffworkflow_backend.models.reference_cache import ReferenceCacheModel from spiffworkflow_backend.models.task import TaskModel # noqa: F401 from spiffworkflow_backend.models.user import UserModel from spiffworkflow_backend.services.file_system_service import FileSystemService -from spiffworkflow_backend.services.group_service import GroupService from spiffworkflow_backend.services.process_caller_service import ProcessCallerService from spiffworkflow_backend.services.process_instance_processor import ProcessInstanceProcessor from spiffworkflow_backend.services.process_instance_service import ProcessInstanceService from spiffworkflow_backend.services.process_model_service import ProcessModelService +from spiffworkflow_backend.services.user_service import UserService from tests.spiffworkflow_backend.helpers.base_test import BaseTest from tests.spiffworkflow_backend.helpers.test_data import load_test_spec @@ -102,9 +102,9 @@ class TestProcessApi(BaseTest): with_db_and_bpmn_file_cleanup: None, ) -> None: user = self.find_or_create_user() - group = GroupService.find_or_create_group("test_group") + group = UserService.find_or_create_group("test_group") principal = group.principal - GroupService.add_user_to_group(user, group.identifier) + UserService.add_user_to_group(user, group) self.add_permissions_to_principal(principal, target_uri="/v1.0/process-groups/%", permission_names=["read"]) request_body = { "requests_to_check": { @@ -1304,7 +1304,7 @@ class TestProcessApi(BaseTest): ) assert response.json is not None - assert type(response.json["updated_at_in_seconds"]) is int + assert isinstance(response.json["updated_at_in_seconds"], int) assert response.json["updated_at_in_seconds"] > 0 assert response.json["status"] == "complete" assert response.json["process_model_identifier"] == process_model.id @@ -1738,9 +1738,9 @@ class TestProcessApi(BaseTest): assert response.json["pagination"]["total"] == 1 process_instance_dict = response.json["results"][0] - assert type(process_instance_dict["id"]) is int + assert isinstance(process_instance_dict["id"], int) assert process_instance_dict["process_model_identifier"] == process_model.id - assert type(process_instance_dict["start_in_seconds"]) is int + assert isinstance(process_instance_dict["start_in_seconds"], int) assert process_instance_dict["start_in_seconds"] > 0 assert process_instance_dict["end_in_seconds"] is None assert process_instance_dict["status"] == "not_started" diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py index b57f1924..f0030928 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py @@ -5,7 +5,6 @@ from spiffworkflow_backend.models.group import GroupModel from spiffworkflow_backend.services.authorization_service import AuthorizationService from spiffworkflow_backend.services.authorization_service import GroupPermissionsDict from spiffworkflow_backend.services.authorization_service import InvalidPermissionError -from spiffworkflow_backend.services.group_service import GroupService from spiffworkflow_backend.services.process_instance_processor import ProcessInstanceProcessor from spiffworkflow_backend.services.process_instance_service import ProcessInstanceService from spiffworkflow_backend.services.user_service import UserService @@ -341,7 +340,7 @@ class TestAuthorizationService(BaseTest): with_db_and_bpmn_file_cleanup: None, ) -> None: user = self.find_or_create_user(username="user_one") - user_group = GroupService.find_or_create_group("group_one") + user_group = UserService.find_or_create_group("group_one") UserService.add_user_to_group(user, user_group) AuthorizationService.add_permission_from_uri_or_macro(user_group.identifier, "read", "PG:hey") self.assert_user_has_permission(user, "read", "/v1.0/process-groups/hey") @@ -376,10 +375,10 @@ class TestAuthorizationService(BaseTest): admin_user = self.find_or_create_user(username="testadmin1") # this group is not mentioned so it will get deleted - GroupService.find_or_create_group("group_two") + UserService.find_or_create_group("group_two") assert GroupModel.query.filter_by(identifier="group_two").first() is not None - GroupService.find_or_create_group("group_three") + UserService.find_or_create_group("group_three") assert GroupModel.query.filter_by(identifier="group_three").first() is not None group_info: list[GroupPermissionsDict] = [ diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_user_service.py b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_user_service.py index c261bdbf..1b19be67 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_user_service.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_user_service.py @@ -2,7 +2,6 @@ from flask.app import Flask from flask.testing import FlaskClient from spiffworkflow_backend.models.user_group_assignment_waiting import UserGroupAssignmentWaitingModel -from spiffworkflow_backend.services.group_service import GroupService from spiffworkflow_backend.services.user_service import UserService from tests.spiffworkflow_backend.helpers.base_test import BaseTest @@ -15,7 +14,7 @@ class TestUserService(BaseTest): client: FlaskClient, with_db_and_bpmn_file_cleanup: None, ) -> None: - a_test_group = GroupService.find_or_create_group("aTestGroup") + a_test_group = UserService.find_or_create_group("aTestGroup") UserService.add_waiting_group_assignment("initiator_user", a_test_group) initiator_user = self.find_or_create_user("initiator_user") assert initiator_user.groups[0] == a_test_group @@ -26,7 +25,7 @@ class TestUserService(BaseTest): client: FlaskClient, with_db_and_bpmn_file_cleanup: None, ) -> None: - everybody_group = GroupService.find_or_create_group("everybodyGroup") + everybody_group = UserService.find_or_create_group("everybodyGroup") UserService.add_waiting_group_assignment(UserGroupAssignmentWaitingModel.MATCH_ALL_USERS, everybody_group) initiator_user = self.find_or_create_user("initiator_user") assert initiator_user.groups[0] == everybody_group @@ -38,6 +37,6 @@ class TestUserService(BaseTest): with_db_and_bpmn_file_cleanup: None, ) -> None: initiator_user = self.find_or_create_user("initiator_user") - everybody_group = GroupService.find_or_create_group("everybodyGroup") + everybody_group = UserService.find_or_create_group("everybodyGroup") UserService.add_waiting_group_assignment(UserGroupAssignmentWaitingModel.MATCH_ALL_USERS, everybody_group) assert initiator_user.groups[0] == everybody_group