From 5b793d5a817fcfa328ad1fbd7d9928bc3b37d839 Mon Sep 17 00:00:00 2001 From: jasquat Date: Thu, 18 May 2023 10:02:07 -0400 Subject: [PATCH 1/6] added parse method to turn the yaml into the same format as the incoming perms from the dmn tables w/ burnettk --- .../scripts/refresh_permissions.py | 1 - .../services/authorization_service.py | 63 ++++++++++++++++--- .../unit/test_authorization_service.py | 4 +- 3 files changed, 58 insertions(+), 10 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/refresh_permissions.py b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/refresh_permissions.py index 4981af93..4a49d7b5 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/refresh_permissions.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/refresh_permissions.py @@ -34,6 +34,5 @@ class RefreshPermissions(Script): *args: Any, **kwargs: Any, ) -> Any: - """Run.""" group_info = args[0] AuthorizationService.refresh_permissions(group_info) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py index 761f14a2..ccf51cd8 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py @@ -94,13 +94,22 @@ class UserToGroupDict(TypedDict): class DesiredPermissionDict(TypedDict): - """DesiredPermissionDict.""" - group_identifiers: Set[str] permission_assignments: list[PermissionAssignmentModel] user_to_group_identifiers: list[UserToGroupDict] +class DesiredGroupPermissionDict(TypedDict): + actions: list[str] + uri: str + + +class GroupPermissionsDict(TypedDict): + users: list[str] + name: str + permissions: list[DesiredGroupPermissionDict] + + class AuthorizationService: """Determine whether a user has permission to perform their request.""" @@ -699,17 +708,57 @@ class AuthorizationService: return permission_assignments @classmethod - def refresh_permissions(cls, group_info: list[dict[str, Any]]) -> None: + def parse_permissions_yaml_into_group_info(cls) -> list[GroupPermissionsDict]: + if current_app.config["SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_NAME"] is None: + raise ( + PermissionsFileNotSetError( + "SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_NAME needs to be set in order to import permissions" + ) + ) + + permission_configs = None + with open(current_app.config["SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_ABSOLUTE_PATH"]) as file: + permission_configs = yaml.safe_load(file) + + group_permissions: list[GroupPermissionsDict] = [] + group_permissions_by_group: dict[str, GroupPermissionsDict] = {} + if "default_group" in permission_configs: + default_group_identifier = permission_configs["default_group"] + # group_permissions.append({"name": default_group_identifier, "users": [], "permissions": []}) + group_permissions_by_group[default_group_identifier] = {"name": default_group_identifier, "users": [], "permissions": []} + + if "groups" in permission_configs: + for group_identifier, group_config in permission_configs["groups"].items(): + group_info: GroupPermissionsDict = {"name": group_identifier, "users": [], "permissions": []} + for username in group_config["users"]: + group_info['users'].append(username) + group_permissions_by_group[group_identifier] = group_info + + if "permissions" in permission_configs: + for _permission_identifier, permission_config in permission_configs["permissions"].items(): + uri = permission_config["uri"] + for group_identifier in permission_config["groups"]: + group_permissions_by_group[group_identifier]['permissions'].append({'actions': permission_config["allowed_permissions"], "uri": uri}) + + for _group_identifier, group_permission in group_permissions_by_group.items(): + group_permissions.append(group_permission) + + return group_permissions + + @classmethod + def refresh_permissions(cls, group_info: list[GroupPermissionsDict]) -> None: """Adds new permission assignments and deletes old ones.""" initial_permission_assignments = PermissionAssignmentModel.query.all() initial_user_to_group_assignments = UserGroupAssignmentModel.query.all() - result = cls.import_permissions_from_yaml_file() - desired_permission_assignments = result["permission_assignments"] - desired_group_identifiers = result["group_identifiers"] - desired_user_to_group_identifiers = result["user_to_group_identifiers"] + group_info = group_info + cls.parse_permissions_yaml_into_group_info() + + desired_group_identifiers: Set[str] = set() + desired_permission_assignments: list[PermissionAssignmentModel] = [] + desired_user_to_group_identifiers: list[UserToGroupDict] = [] for group in group_info: group_identifier = group["name"] + GroupService.find_or_create_group(group_identifier) for username in group["users"]: user_to_group_dict: UserToGroupDict = { "username": username, 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 b742c463..5712cb62 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py @@ -7,7 +7,7 @@ from tests.spiffworkflow_backend.helpers.base_test import BaseTest from spiffworkflow_backend.models.group import GroupModel from spiffworkflow_backend.models.user import UserModel from spiffworkflow_backend.models.user import UserNotFoundError -from spiffworkflow_backend.services.authorization_service import AuthorizationService +from spiffworkflow_backend.services.authorization_service import AuthorizationService, 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 ( @@ -399,7 +399,7 @@ class TestAuthorizationService(BaseTest): GroupService.find_or_create_group("group_three") assert GroupModel.query.filter_by(identifier="group_three").first() is not None - group_info = [ + group_info: list[GroupPermissionsDict] = [ { "users": ["user_one", "user_two"], "name": "group_one", From 84f3847c507f3d28165b4f4ccf932adf7ffe0b8c Mon Sep 17 00:00:00 2001 From: jasquat Date: Thu, 18 May 2023 11:29:15 -0400 Subject: [PATCH 2/6] refactored import perms from yaml and from dmn tables to do the same thing w/ burnettk --- .../spiffworkflow_backend/config/default.py | 1 + .../config/permissions/unit_testing.yml | 10 -- .../scripts/refresh_permissions.py | 1 + .../services/authorization_service.py | 163 ++++++------------ .../services/group_service.py | 5 - .../unit/test_authorization_service.py | 18 +- 6 files changed, 59 insertions(+), 139 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/config/default.py b/spiffworkflow-backend/src/spiffworkflow_backend/config/default.py index 88a34faf..08187f7e 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/config/default.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/default.py @@ -143,6 +143,7 @@ SPIFFWORKFLOW_BACKEND_ALLOW_CONFISCATING_LOCK_AFTER_SECONDS = int( environ.get("SPIFFWORKFLOW_BACKEND_ALLOW_CONFISCATING_LOCK_AFTER_SECONDS", default="600") ) +# FIXME: do not default this but we will need to coordinate release of it since it is a breaking change SPIFFWORKFLOW_BACKEND_DEFAULT_USER_GROUP = environ.get("SPIFFWORKFLOW_BACKEND_DEFAULT_USER_GROUP", default="everybody") SPIFFWORKFLOW_BACKEND_ENGINE_STEP_DEFAULT_STRATEGY_BACKGROUND = environ.get( diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/unit_testing.yml b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/unit_testing.yml index d3edf0a8..6cabb4b0 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/unit_testing.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/unit_testing.yml @@ -1,5 +1,3 @@ -default_group: everybody - users: testadmin1: service: https://testing/openid/thing @@ -20,49 +18,41 @@ groups: permissions: admin: groups: [admin] - users: [] allowed_permissions: [create, read, update, delete] uri: /* read-all: groups: ["Finance Team", hr, admin] - users: [] allowed_permissions: [read] uri: /* process-instances-find-by-id: groups: [everybody] - users: [] allowed_permissions: [read] uri: /process-instances/find-by-id/* tasks-crud: groups: [everybody] - users: [] allowed_permissions: [create, read, update, delete] uri: /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: /process-groups/finance/* finance-admin-model: groups: ["Finance Team"] - users: [testuser4] allowed_permissions: [create, read, update, delete] uri: /process-models/finance/* finance-admin-model-lanes: groups: ["Finance Team"] - users: [testuser4] allowed_permissions: [create, read, update, delete] uri: /process-models/finance:model_with_lanes/* finance-admin-instance-run: groups: ["Finance Team"] - users: [testuser4] allowed_permissions: [create, read, update, delete] uri: /process-instances/* diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/refresh_permissions.py b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/refresh_permissions.py index 4a49d7b5..c8192574 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/refresh_permissions.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/refresh_permissions.py @@ -35,4 +35,5 @@ class RefreshPermissions(Script): **kwargs: Any, ) -> Any: group_info = args[0] + import pdb; pdb.set_trace() AuthorizationService.refresh_permissions(group_info) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py index ccf51cd8..61e4d25c 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py @@ -21,6 +21,7 @@ from sqlalchemy import or_ from sqlalchemy import text from spiffworkflow_backend.helpers.api_version import V1_API_PATH_PREFIX +from spiffworkflow_backend.models import permission_assignment from spiffworkflow_backend.models.db import db from spiffworkflow_backend.models.group import GroupModel from spiffworkflow_backend.models.human_task import HumanTaskModel @@ -196,88 +197,13 @@ class AuthorizationService: db.session.commit() @classmethod - def import_permissions_from_yaml_file(cls, raise_if_missing_user: bool = False) -> DesiredPermissionDict: - """Import_permissions_from_yaml_file.""" - if current_app.config["SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_NAME"] is None: - raise ( - PermissionsFileNotSetError( - "SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_NAME needs to be set in order to import permissions" - ) - ) - - permission_configs = None - with open(current_app.config["SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_ABSOLUTE_PATH"]) as file: - permission_configs = yaml.safe_load(file) - - default_group = None - unique_user_group_identifiers: Set[str] = set() - user_to_group_identifiers: list[UserToGroupDict] = [] - if "default_group" in permission_configs: - default_group_identifier = permission_configs["default_group"] - default_group = GroupService.find_or_create_group(default_group_identifier) - unique_user_group_identifiers.add(default_group_identifier) - - if "groups" in permission_configs: - for group_identifier, group_config in permission_configs["groups"].items(): - group = GroupService.find_or_create_group(group_identifier) - unique_user_group_identifiers.add(group_identifier) - for username in group_config["users"]: - user = UserModel.query.filter_by(username=username).first() - if user is None: - if raise_if_missing_user: - raise (UserNotFoundError(f"Could not find a user with name: {username}")) - continue - user_to_group_dict: UserToGroupDict = { - "username": user.username, - "group_identifier": group_identifier, - } - user_to_group_identifiers.append(user_to_group_dict) - cls.associate_user_with_group(user, group) - - permission_assignments = [] - if "permissions" in permission_configs: - for _permission_identifier, permission_config in permission_configs["permissions"].items(): - uri = permission_config["uri"] - permission_target = cls.find_or_create_permission_target(uri) - - for allowed_permission in permission_config["allowed_permissions"]: - if "groups" in permission_config: - for group_identifier in permission_config["groups"]: - group = GroupService.find_or_create_group(group_identifier) - unique_user_group_identifiers.add(group_identifier) - permission_assignments.append( - cls.create_permission_for_principal( - group.principal, - permission_target, - allowed_permission, - ) - ) - if "users" in permission_config: - for username in permission_config["users"]: - user = UserModel.query.filter_by(username=username).first() - if user is not None: - principal = ( - PrincipalModel.query.join(UserModel).filter(UserModel.username == username).first() - ) - permission_assignments.append( - 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) - - return { - "group_identifiers": unique_user_group_identifiers, - "permission_assignments": permission_assignments, - "user_to_group_identifiers": user_to_group_identifiers, - } + def import_permissions_from_yaml_file(cls, user_model: Optional[UserModel] = None) -> DesiredPermissionDict: + group_permissions = cls.parse_permissions_yaml_into_group_info() + result = cls.add_permissions_from_group_permissions(group_permissions, user_model) + return result @classmethod def find_or_create_permission_target(cls, uri: str) -> PermissionTargetModel: - """Find_or_create_permission_target.""" uri_with_percent = re.sub(r"\*", "%", uri) target_uri_normalized = uri_with_percent.removeprefix(V1_API_PATH_PREFIX) permission_target: Optional[PermissionTargetModel] = PermissionTargetModel.query.filter_by( @@ -296,7 +222,6 @@ class AuthorizationService: permission_target: PermissionTargetModel, permission: str, ) -> PermissionAssignmentModel: - """Create_permission_for_principal.""" permission_assignment: Optional[PermissionAssignmentModel] = PermissionAssignmentModel.query.filter_by( principal_id=principal.id, permission_target_id=permission_target.id, @@ -315,7 +240,6 @@ 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", @@ -353,7 +277,6 @@ class AuthorizationService: @classmethod def get_permission_from_http_method(cls, http_method: str) -> Optional[str]: - """Get_permission_from_request_method.""" request_method_mapper = { "POST": "create", "GET": "read", @@ -515,7 +438,7 @@ class AuthorizationService: # 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() + cls.import_permissions_from_yaml_file(user_model) if is_new_user: UserService.add_user_to_human_tasks_if_appropriate(user_model) @@ -720,11 +643,9 @@ class AuthorizationService: with open(current_app.config["SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_ABSOLUTE_PATH"]) as file: permission_configs = yaml.safe_load(file) - group_permissions: list[GroupPermissionsDict] = [] group_permissions_by_group: dict[str, GroupPermissionsDict] = {} - if "default_group" in permission_configs: - default_group_identifier = permission_configs["default_group"] - # group_permissions.append({"name": default_group_identifier, "users": [], "permissions": []}) + if current_app.config['SPIFFWORKFLOW_BACKEND_DEFAULT_USER_GROUP']: + default_group_identifier = current_app.config['SPIFFWORKFLOW_BACKEND_DEFAULT_USER_GROUP'] group_permissions_by_group[default_group_identifier] = {"name": default_group_identifier, "users": [], "permissions": []} if "groups" in permission_configs: @@ -738,25 +659,25 @@ class AuthorizationService: for _permission_identifier, permission_config in permission_configs["permissions"].items(): uri = permission_config["uri"] for group_identifier in permission_config["groups"]: - group_permissions_by_group[group_identifier]['permissions'].append({'actions': permission_config["allowed_permissions"], "uri": uri}) + group_permissions_by_group[group_identifier]['permissions'].append( + {'actions': permission_config["allowed_permissions"], "uri": uri} + ) - for _group_identifier, group_permission in group_permissions_by_group.items(): - group_permissions.append(group_permission) - - return group_permissions + return list(group_permissions_by_group.values()) @classmethod - def refresh_permissions(cls, group_info: list[GroupPermissionsDict]) -> None: - """Adds new permission assignments and deletes old ones.""" - initial_permission_assignments = PermissionAssignmentModel.query.all() - initial_user_to_group_assignments = UserGroupAssignmentModel.query.all() - group_info = group_info + cls.parse_permissions_yaml_into_group_info() + def add_permissions_from_group_permissions(cls, group_permissions: list[GroupPermissionsDict], user_model: Optional[UserModel] = None) -> DesiredPermissionDict: + unique_user_group_identifiers: Set[str] = set() + user_to_group_identifiers: list[UserToGroupDict] = [] + permission_assignments = [] - desired_group_identifiers: Set[str] = set() - desired_permission_assignments: list[PermissionAssignmentModel] = [] - desired_user_to_group_identifiers: list[UserToGroupDict] = [] + 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) + unique_user_group_identifiers.add(default_group_identifier) - for group in group_info: + for group in group_permissions: group_identifier = group["name"] GroupService.find_or_create_group(group_identifier) for username in group["users"]: @@ -764,19 +685,44 @@ class AuthorizationService: "username": username, "group_identifier": group_identifier, } - desired_user_to_group_identifiers.append(user_to_group_dict) + user_to_group_identifiers.append(user_to_group_dict) GroupService.add_user_to_group_or_add_to_waiting(username, group_identifier) - desired_group_identifiers.add(group_identifier) + unique_user_group_identifiers.add(group_identifier) for permission in group["permissions"]: for crud_op in permission["actions"]: - desired_permission_assignments.extend( + permission_assignments.extend( cls.add_permission_from_uri_or_macro( group_identifier=group_identifier, target=permission["uri"], permission=crud_op, ) ) - desired_group_identifiers.add(group_identifier) + unique_user_group_identifiers.add(group_identifier) + + if default_group is not None: + if user_model: + cls.associate_user_with_group(user_model, default_group) + else: + for user in UserModel.query.all(): + cls.associate_user_with_group(user, default_group) + + return { + "group_identifiers": unique_user_group_identifiers, + "permission_assignments": permission_assignments, + "user_to_group_identifiers": user_to_group_identifiers, + } + + @classmethod + def refresh_permissions(cls, group_permissions: list[GroupPermissionsDict]) -> None: + """Adds new permission assignments and deletes old ones.""" + initial_permission_assignments = PermissionAssignmentModel.query.all() + initial_user_to_group_assignments = UserGroupAssignmentModel.query.all() + group_permissions = group_permissions + cls.parse_permissions_yaml_into_group_info() + + result = cls.add_permissions_from_group_permissions(group_permissions) + desired_permission_assignments = result["permission_assignments"] + desired_group_identifiers = result["group_identifiers"] + desired_user_to_group_identifiers = result["user_to_group_identifiers"] for ipa in initial_permission_assignments: if ipa not in desired_permission_assignments: @@ -801,10 +747,3 @@ class AuthorizationService: for gtd in groups_to_delete: db.session.delete(gtd) db.session.commit() - - -class KeycloakAuthorization: - """Interface with Keycloak server.""" - - -# class KeycloakClient: diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/group_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/group_service.py index eee47bc6..c1938cfb 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/group_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/group_service.py @@ -1,4 +1,3 @@ -"""Group_service.""" from typing import Optional from spiffworkflow_backend.models.db import db @@ -8,11 +7,8 @@ from spiffworkflow_backend.services.user_service import UserService class GroupService: - """GroupService.""" - @classmethod def find_or_create_group(cls, group_identifier: str) -> GroupModel: - """Find_or_create_group.""" group: Optional[GroupModel] = GroupModel.query.filter_by(identifier=group_identifier).first() if group is None: group = GroupModel(identifier=group_identifier) @@ -23,7 +19,6 @@ class GroupService: @classmethod def add_user_to_group_or_add_to_waiting(cls, username: str, group_identifier: str) -> None: - """Add_user_to_group_or_add_to_waiting.""" group = cls.find_or_create_group(group_identifier) user = UserModel.query.filter_by(username=username).first() if user: 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 5712cb62..380677ae 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py @@ -21,19 +21,10 @@ from spiffworkflow_backend.services.user_service import UserService class TestAuthorizationService(BaseTest): - """TestAuthorizationService.""" - - def test_can_raise_if_missing_user(self, app: Flask, with_db_and_bpmn_file_cleanup: None) -> None: - """Test_can_raise_if_missing_user.""" - with pytest.raises(UserNotFoundError): - AuthorizationService.import_permissions_from_yaml_file(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: - """Test_can_import_permissions_from_yaml.""" usernames = [ "testadmin1", "testadmin2", @@ -59,8 +50,6 @@ class TestAuthorizationService(BaseTest): self.assert_user_has_permission(users["testuser1"], "update", "/v1.0/process-groups/finance/model1") self.assert_user_has_permission(users["testuser1"], "update", "/v1.0/process-groups/finance/") self.assert_user_has_permission(users["testuser1"], "update", "/v1.0/process-groups/", expected_result=False) - 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") self.assert_user_has_permission(users["testuser2"], "update", "/v1.0/process-groups/finance/model1") self.assert_user_has_permission(users["testuser2"], "update", "/v1.0/process-groups/", expected_result=False) @@ -387,7 +376,6 @@ class TestAuthorizationService(BaseTest): client: FlaskClient, with_db_and_bpmn_file_cleanup: None, ) -> None: - """Test_can_refresh_permissions.""" user = self.find_or_create_user(username="user_one") user_two = self.find_or_create_user(username="user_two") admin_user = self.find_or_create_user(username="testadmin1") @@ -410,6 +398,11 @@ class TestAuthorizationService(BaseTest): "name": "group_three", "permissions": [{"actions": ["create", "read"], "uri": "PG:hey2"}], }, + { + "users": [], + "name": "everybody", + "permissions": [{"actions": ["read"], "uri": "PG:hey2everybody"}], + }, ] AuthorizationService.refresh_permissions(group_info) assert GroupModel.query.filter_by(identifier="group_two").first() is None @@ -418,6 +411,7 @@ class TestAuthorizationService(BaseTest): self.assert_user_has_permission(user, "read", "/v1.0/process-groups/hey") self.assert_user_has_permission(user, "read", "/v1.0/process-groups/hey:yo") self.assert_user_has_permission(user, "create", "/v1.0/process-groups/hey:yo") + self.assert_user_has_permission(user, "read", "/v1.0/process-groups/hey2everybody:yo") self.assert_user_has_permission(user_two, "read", "/v1.0/process-groups/hey") self.assert_user_has_permission(user_two, "read", "/v1.0/process-groups/hey:yo") From 40b3246eb70c77a2e9edd30426a5e9fbdbfd7f53 Mon Sep 17 00:00:00 2001 From: jasquat Date: Thu, 18 May 2023 12:11:40 -0400 Subject: [PATCH 3/6] support macros in perm yml and pyl --- .../config/permissions/acceptance_tests.yml | 1 - .../config/permissions/demo.yml | 3 --- .../config/permissions/example.yml | 10 ------- .../config/permissions/example_read_only.yml | 14 ---------- .../config/permissions/local_development.yml | 2 -- .../terraform_deployed_environment.yml | 2 -- .../config/permissions/unit_testing.yml | 13 +++++----- .../scripts/refresh_permissions.py | 1 - .../services/authorization_service.py | 26 ++++++++++--------- .../integration/test_process_api.py | 8 ------ .../unit/test_authorization_service.py | 17 ++++++------ 11 files changed, 28 insertions(+), 69 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/acceptance_tests.yml b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/acceptance_tests.yml index 0382f389..f3f84773 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/acceptance_tests.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/acceptance_tests.yml @@ -12,6 +12,5 @@ groups: permissions: admin: groups: [admin] - users: [] allowed_permissions: [create, read, update, delete] uri: /* diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/demo.yml b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/demo.yml index dbded55b..1b1d161e 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/demo.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/demo.yml @@ -1,5 +1,3 @@ -default_group: everybody - groups: admin: users: @@ -19,6 +17,5 @@ groups: permissions: admin: groups: [admin, tech_writers] - users: [] allowed_permissions: [create, read, update, delete] uri: /* diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/example.yml b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/example.yml index a11578bd..0684ef55 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/example.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/example.yml @@ -1,4 +1,3 @@ -default_group: everybody users: admin: @@ -41,52 +40,43 @@ permissions: # Admins have access to everything. admin: groups: [admin] - users: [] allowed_permissions: [create, read, update, delete] uri: /* # Everybody can participate in tasks assigned to them. tasks-crud: groups: [everybody] - users: [] allowed_permissions: [create, read, update, delete] uri: /tasks/* # Everybody can start all intstances create-test-instances: groups: [ everybody ] - users: [ ] allowed_permissions: [ create ] uri: /process-instances/* # Everyone can see everything (all groups, and processes are visible) read-all-process-groups: groups: [ everybody ] - users: [ ] allowed_permissions: [ read ] uri: /process-groups/* read-all-process-models: groups: [ everybody ] - users: [ ] allowed_permissions: [ read ] uri: /process-models/* read-all-process-instance: groups: [ everybody ] - users: [ ] allowed_permissions: [ read ] uri: /process-instances/* read-process-instance-reports: groups: [ everybody ] - users: [ ] allowed_permissions: [ read ] uri: /process-instances/reports/* processes-read: groups: [ everybody ] - users: [ ] allowed_permissions: [ read ] uri: /processes groups-everybody: groups: [everybody] - users: [] allowed_permissions: [create, read] uri: /v1.0/user-groups/for-current-user diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/example_read_only.yml b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/example_read_only.yml index d201a555..b6facc64 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/example_read_only.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/example_read_only.yml @@ -1,4 +1,3 @@ -default_group: everybody groups: admin: @@ -7,78 +6,65 @@ groups: permissions: admin: groups: [admin] - users: [] allowed_permissions: [read] uri: /* tasks-crud: groups: [admin] - users: [] allowed_permissions: [create, update, delete] uri: /tasks/* process-instances-crud: groups: [ admin ] - users: [ ] allowed_permissions: [create, update, delete] uri: /process-instances/* suspend: groups: [admin] - users: [] allowed_permissions: [create] uri: /v1.0/process-instance-suspend terminate: groups: [admin] - users: [] allowed_permissions: [create] uri: /v1.0/process-instance-terminate resume: groups: [admin] - users: [] allowed_permissions: [create] uri: /v1.0/process-instance-resume reset: groups: [admin] - users: [] allowed_permissions: [create] uri: /v1.0/process-instance-reset users-exist: groups: [admin] - users: [] allowed_permissions: [create] uri: /v1.0/users/exists/by-username send-event: groups: [admin] - users: [] allowed_permissions: [create] uri: /v1.0/send-event/* task-complete: groups: [admin] - users: [] allowed_permissions: [create] uri: /v1.0/task-complete/* messages: groups: [admin] - users: [] allowed_permissions: [create] uri: /v1.0/messages/* secrets: groups: [admin] - users: [] allowed_permissions: [create, update, delete] uri: /v1.0/secrets/* task-data: groups: [admin] - users: [] allowed_permissions: [update] uri: /v1.0/task-data/* diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/local_development.yml b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/local_development.yml index eb9ce4b7..4b596568 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/local_development.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/local_development.yml @@ -1,4 +1,3 @@ -default_group: everybody groups: admin: @@ -11,6 +10,5 @@ groups: permissions: admin: groups: [admin, group1, group2] - users: [] allowed_permissions: [create, read, update, delete] uri: /* diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/terraform_deployed_environment.yml b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/terraform_deployed_environment.yml index 049c991e..14876bf7 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/terraform_deployed_environment.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/terraform_deployed_environment.yml @@ -1,4 +1,3 @@ -default_group: everybody groups: admin: @@ -7,6 +6,5 @@ groups: permissions: admin: groups: [admin] - users: [] allowed_permissions: [create, read, update, delete] uri: /* diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/unit_testing.yml b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/unit_testing.yml index 6cabb4b0..10980462 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/unit_testing.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/unit_testing.yml @@ -36,16 +36,15 @@ permissions: allowed_permissions: [create, read, update, delete] uri: /tasks/* - # TODO: all uris should really have the same structure finance-admin-group: groups: ["Finance Team"] - allowed_permissions: [create, read, update, delete] - uri: /process-groups/finance/* + allowed_permissions: [all] + uri: PG:finance - finance-admin-model: - groups: ["Finance Team"] - allowed_permissions: [create, read, update, delete] - uri: /process-models/finance/* + finance-hr-start: + groups: ["hr"] + allowed_permissions: [start] + uri: PG:finance finance-admin-model-lanes: groups: ["Finance Team"] diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/refresh_permissions.py b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/refresh_permissions.py index c8192574..4a49d7b5 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/refresh_permissions.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/refresh_permissions.py @@ -35,5 +35,4 @@ class RefreshPermissions(Script): **kwargs: Any, ) -> Any: group_info = args[0] - import pdb; pdb.set_trace() AuthorizationService.refresh_permissions(group_info) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py index 61e4d25c..32efe6d8 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py @@ -5,7 +5,6 @@ from dataclasses import dataclass from hashlib import sha256 from hmac import compare_digest from hmac import HMAC -from typing import Any from typing import Optional from typing import Set from typing import TypedDict @@ -21,7 +20,6 @@ from sqlalchemy import or_ from sqlalchemy import text from spiffworkflow_backend.helpers.api_version import V1_API_PATH_PREFIX -from spiffworkflow_backend.models import permission_assignment from spiffworkflow_backend.models.db import db from spiffworkflow_backend.models.group import GroupModel from spiffworkflow_backend.models.human_task import HumanTaskModel @@ -30,7 +28,6 @@ from spiffworkflow_backend.models.permission_target import PermissionTargetModel from spiffworkflow_backend.models.principal import MissingPrincipalError 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.routes.openid_blueprint import openid_blueprint from spiffworkflow_backend.services.authentication_service import NotAuthorizedError @@ -617,7 +614,6 @@ class AuthorizationService: def add_permission_from_uri_or_macro( cls, group_identifier: str, permission: str, target: str ) -> list[PermissionAssignmentModel]: - """Add_permission_from_uri_or_macro.""" group = GroupService.find_or_create_group(group_identifier) permissions_to_assign = cls.explode_permissions(permission, target) permission_assignments = [] @@ -644,35 +640,41 @@ class AuthorizationService: permission_configs = yaml.safe_load(file) group_permissions_by_group: dict[str, GroupPermissionsDict] = {} - if current_app.config['SPIFFWORKFLOW_BACKEND_DEFAULT_USER_GROUP']: - default_group_identifier = current_app.config['SPIFFWORKFLOW_BACKEND_DEFAULT_USER_GROUP'] - group_permissions_by_group[default_group_identifier] = {"name": default_group_identifier, "users": [], "permissions": []} + if current_app.config["SPIFFWORKFLOW_BACKEND_DEFAULT_USER_GROUP"]: + default_group_identifier = current_app.config["SPIFFWORKFLOW_BACKEND_DEFAULT_USER_GROUP"] + group_permissions_by_group[default_group_identifier] = { + "name": default_group_identifier, + "users": [], + "permissions": [], + } if "groups" in permission_configs: for group_identifier, group_config in permission_configs["groups"].items(): group_info: GroupPermissionsDict = {"name": group_identifier, "users": [], "permissions": []} for username in group_config["users"]: - group_info['users'].append(username) + group_info["users"].append(username) group_permissions_by_group[group_identifier] = group_info if "permissions" in permission_configs: for _permission_identifier, permission_config in permission_configs["permissions"].items(): uri = permission_config["uri"] for group_identifier in permission_config["groups"]: - group_permissions_by_group[group_identifier]['permissions'].append( - {'actions': permission_config["allowed_permissions"], "uri": uri} + group_permissions_by_group[group_identifier]["permissions"].append( + {"actions": permission_config["allowed_permissions"], "uri": uri} ) return list(group_permissions_by_group.values()) @classmethod - def add_permissions_from_group_permissions(cls, group_permissions: list[GroupPermissionsDict], user_model: Optional[UserModel] = None) -> DesiredPermissionDict: + def add_permissions_from_group_permissions( + cls, group_permissions: list[GroupPermissionsDict], user_model: Optional[UserModel] = None + ) -> DesiredPermissionDict: unique_user_group_identifiers: Set[str] = set() user_to_group_identifiers: list[UserToGroupDict] = [] permission_assignments = [] default_group = None - default_group_identifier = current_app.config['SPIFFWORKFLOW_BACKEND_DEFAULT_USER_GROUP'] + 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) unique_user_group_identifiers.add(default_group_identifier) 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 2cd16063..54d67848 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py @@ -2349,7 +2349,6 @@ class TestProcessApi(BaseTest): 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 @@ -2372,15 +2371,8 @@ class TestProcessApi(BaseTest): bpmn_file_location=bpmn_file_location, ) - # 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_from_process_model_id_with_api( client, - # process_model.process_group_id, process_model_identifier, headers=self.logged_in_headers(initiator_user), ) 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 380677ae..a1f41d3e 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py @@ -6,8 +6,8 @@ from tests.spiffworkflow_backend.helpers.base_test import BaseTest from spiffworkflow_backend.models.group import GroupModel from spiffworkflow_backend.models.user import UserModel -from spiffworkflow_backend.models.user import UserNotFoundError -from spiffworkflow_backend.services.authorization_service import AuthorizationService, GroupPermissionsDict +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 ( @@ -47,13 +47,13 @@ class TestAuthorizationService(BaseTest): 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") - self.assert_user_has_permission(users["testuser1"], "update", "/v1.0/process-groups/finance/") + self.assert_user_has_permission(users["testuser1"], "update", "/v1.0/process-groups/finance:model1") + self.assert_user_has_permission(users["testuser1"], "update", "/v1.0/process-groups/finance") self.assert_user_has_permission(users["testuser1"], "update", "/v1.0/process-groups/", expected_result=False) - self.assert_user_has_permission(users["testuser4"], "read", "/v1.0/process-groups/finance/model1") - self.assert_user_has_permission(users["testuser2"], "update", "/v1.0/process-groups/finance/model1") - self.assert_user_has_permission(users["testuser2"], "update", "/v1.0/process-groups/", expected_result=False) - self.assert_user_has_permission(users["testuser2"], "read", "/v1.0/process-groups/") + self.assert_user_has_permission(users["testuser4"], "read", "/v1.0/process-groups/finance:model1") + self.assert_user_has_permission(users["testuser2"], "update", "/v1.0/process-groups/finance:model1") + self.assert_user_has_permission(users["testuser2"], "update", "/v1.0/process-groups", expected_result=False) + self.assert_user_has_permission(users["testuser2"], "read", "/v1.0/process-groups") def test_user_can_be_added_to_human_task_on_first_login( self, @@ -110,7 +110,6 @@ class TestAuthorizationService(BaseTest): client: FlaskClient, with_db_and_bpmn_file_cleanup: None, ) -> None: - """Test_explode_permissions_all_on_process_group.""" expected_permissions = sorted( [ ("/event-error-details/some-process-group:some-process-model:*", "read"), From a445badcd16a85ab6abc7fe2a1944206b957ecc7 Mon Sep 17 00:00:00 2001 From: jasquat Date: Thu, 18 May 2023 12:35:23 -0400 Subject: [PATCH 4/6] moved remove permission code to own method and some cleanup --- .../services/authorization_service.py | 78 +++++++++---------- 1 file changed, 36 insertions(+), 42 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py index 32efe6d8..1ff58394 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py @@ -1,4 +1,3 @@ -"""Authorization_service.""" import inspect import re from dataclasses import dataclass @@ -40,25 +39,23 @@ from spiffworkflow_backend.services.user_service import UserService class PermissionsFileNotSetError(Exception): - """PermissionsFileNotSetError.""" + pass class HumanTaskNotFoundError(Exception): - """HumanTaskNotFoundError.""" + pass class UserDoesNotHaveAccessToTaskError(Exception): - """UserDoesNotHaveAccessToTaskError.""" + pass class InvalidPermissionError(Exception): - """InvalidPermissionError.""" + pass @dataclass class PermissionToAssign: - """PermissionToAssign.""" - permission: str target_uri: str @@ -91,7 +88,7 @@ class UserToGroupDict(TypedDict): group_identifier: str -class DesiredPermissionDict(TypedDict): +class AddedPermissionDict(TypedDict): group_identifiers: Set[str] permission_assignments: list[PermissionAssignmentModel] user_to_group_identifiers: list[UserToGroupDict] @@ -114,7 +111,6 @@ class AuthorizationService: # https://stackoverflow.com/a/71320673/6090676 @classmethod def verify_sha256_token(cls, auth_header: Optional[str]) -> None: - """Verify_sha256_token.""" if auth_header is None: raise TokenNotProvidedError( "unauthorized", @@ -130,7 +126,6 @@ class AuthorizationService: @classmethod def has_permission(cls, principals: list[PrincipalModel], permission: str, target_uri: str) -> bool: - """Has_permission.""" principal_ids = [p.id for p in principals] target_uri_normalized = target_uri.removeprefix(V1_API_PATH_PREFIX) @@ -160,7 +155,6 @@ class AuthorizationService: @classmethod def user_has_permission(cls, user: UserModel, permission: str, target_uri: str) -> bool: - """User_has_permission.""" if user.principal is None: raise MissingPrincipalError(f"Missing principal for user with id: {user.id}") @@ -186,7 +180,6 @@ class AuthorizationService: @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) @@ -194,7 +187,7 @@ class AuthorizationService: db.session.commit() @classmethod - def import_permissions_from_yaml_file(cls, user_model: Optional[UserModel] = None) -> DesiredPermissionDict: + def import_permissions_from_yaml_file(cls, user_model: Optional[UserModel] = None) -> AddedPermissionDict: group_permissions = cls.parse_permissions_yaml_into_group_info() result = cls.add_permissions_from_group_permissions(group_permissions, user_model) return result @@ -292,7 +285,6 @@ class AuthorizationService: @classmethod def check_for_permission(cls) -> None: - """Check_for_permission.""" if cls.should_disable_auth_for_request(): return None @@ -326,11 +318,6 @@ class AuthorizationService: @staticmethod def decode_auth_token(auth_token: str) -> dict[str, Union[str, None]]: - """Decode the auth token. - - :param auth_token: - :return: integer|string - """ secret_key = current_app.config.get("SECRET_KEY") if secret_key is None: raise KeyError("we need current_app.config to have a SECRET_KEY") @@ -374,10 +361,11 @@ class AuthorizationService: @classmethod def create_user_from_sign_in(cls, user_info: dict) -> UserModel: - """Create_user_from_sign_in.""" - """Name, family_name, given_name, middle_name, nickname, preferred_username,""" - """Profile, picture, website, gender, birthdate, zoneinfo, locale, and updated_at. """ - """Email.""" + """Fields from user_info. + + name, family_name, given_name, middle_name, nickname, preferred_username, + profile, picture, website, gender, birthdate, zoneinfo, locale,updated_at, email. + """ is_new_user = False user_attributes = {} @@ -450,7 +438,6 @@ class AuthorizationService: process_related_path_segment: str, target_uris: list[str], ) -> list[PermissionToAssign]: - """Get_permissions_to_assign.""" permissions = permission_set.split(",") if permission_set == "all": permissions = ["create", "read", "update", "delete"] @@ -500,7 +487,6 @@ class AuthorizationService: @classmethod def set_basic_permissions(cls) -> list[PermissionToAssign]: - """Set_basic_permissions.""" permissions_to_assign: list[PermissionToAssign] = [] permissions_to_assign.append(PermissionToAssign(permission="create", target_uri="/process-instances/for-me")) permissions_to_assign.append( @@ -528,7 +514,6 @@ class AuthorizationService: @classmethod def set_process_group_permissions(cls, target: str, permission_set: str) -> list[PermissionToAssign]: - """Set_process_group_permissions.""" permissions_to_assign: list[PermissionToAssign] = [] process_group_identifier = target.removeprefix("PG:").replace("/", ":").removeprefix(":") process_related_path_segment = f"{process_group_identifier}:*" @@ -545,7 +530,6 @@ class AuthorizationService: @classmethod def set_process_model_permissions(cls, target: str, permission_set: str) -> list[PermissionToAssign]: - """Set_process_model_permissions.""" permissions_to_assign: list[PermissionToAssign] = [] process_model_identifier = target.removeprefix("PM:").replace("/", ":").removeprefix(":") process_related_path_segment = f"{process_model_identifier}/*" @@ -668,7 +652,7 @@ class AuthorizationService: @classmethod def add_permissions_from_group_permissions( cls, group_permissions: list[GroupPermissionsDict], user_model: Optional[UserModel] = None - ) -> DesiredPermissionDict: + ) -> AddedPermissionDict: unique_user_group_identifiers: Set[str] = set() user_to_group_identifiers: list[UserToGroupDict] = [] permission_assignments = [] @@ -715,19 +699,18 @@ class AuthorizationService: } @classmethod - def refresh_permissions(cls, group_permissions: list[GroupPermissionsDict]) -> None: - """Adds new permission assignments and deletes old ones.""" - initial_permission_assignments = PermissionAssignmentModel.query.all() - initial_user_to_group_assignments = UserGroupAssignmentModel.query.all() - group_permissions = group_permissions + cls.parse_permissions_yaml_into_group_info() - - result = cls.add_permissions_from_group_permissions(group_permissions) - desired_permission_assignments = result["permission_assignments"] - desired_group_identifiers = result["group_identifiers"] - desired_user_to_group_identifiers = result["user_to_group_identifiers"] + def remove_old_permissions_from_added_permissions( + cls, + added_permissions: AddedPermissionDict, + initial_permission_assignments: list[PermissionAssignmentModel], + initial_user_to_group_assignments: list[UserGroupAssignmentModel], + ) -> None: + added_permission_assignments = added_permissions["permission_assignments"] + added_group_identifiers = added_permissions["group_identifiers"] + added_user_to_group_identifiers = added_permissions["user_to_group_identifiers"] for ipa in initial_permission_assignments: - if ipa not in desired_permission_assignments: + if ipa not in added_permission_assignments: db.session.delete(ipa) for iutga in initial_user_to_group_assignments: @@ -740,12 +723,23 @@ class AuthorizationService: "username": iutga.user.username, "group_identifier": iutga.group.identifier, } - if current_user_dict not in desired_user_to_group_identifiers: + if current_user_dict not in added_user_to_group_identifiers: db.session.delete(iutga) # do not remove the default user group - desired_group_identifiers.add(current_app.config["SPIFFWORKFLOW_BACKEND_DEFAULT_USER_GROUP"]) - groups_to_delete = GroupModel.query.filter(GroupModel.identifier.not_in(desired_group_identifiers)).all() + added_group_identifiers.add(current_app.config["SPIFFWORKFLOW_BACKEND_DEFAULT_USER_GROUP"]) + groups_to_delete = GroupModel.query.filter(GroupModel.identifier.not_in(added_group_identifiers)).all() for gtd in groups_to_delete: db.session.delete(gtd) db.session.commit() + + @classmethod + def refresh_permissions(cls, group_permissions: list[GroupPermissionsDict]) -> None: + """Adds new permission assignments and deletes old ones.""" + initial_permission_assignments = PermissionAssignmentModel.query.all() + initial_user_to_group_assignments = UserGroupAssignmentModel.query.all() + group_permissions = group_permissions + cls.parse_permissions_yaml_into_group_info() + added_permissions = cls.add_permissions_from_group_permissions(group_permissions) + cls.remove_old_permissions_from_added_permissions( + added_permissions, initial_permission_assignments, initial_user_to_group_assignments + ) From a285037505fb2cf14b10830a913035990fabf66f Mon Sep 17 00:00:00 2001 From: jasquat Date: Fri, 19 May 2023 10:50:55 -0400 Subject: [PATCH 5/6] added elevated permission macro --- .../src/spiffworkflow_backend/api.yml | 2 +- .../config/permissions/example_read_only.yml | 69 +++---------------- .../services/authorization_service.py | 33 +++++++-- .../unit/test_authorization_service.py | 28 ++++++++ 4 files changed, 66 insertions(+), 66 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/api.yml b/spiffworkflow-backend/src/spiffworkflow_backend/api.yml index 7dcb81ce..a4483f6b 100755 --- a/spiffworkflow-backend/src/spiffworkflow_backend/api.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/api.yml @@ -1122,7 +1122,7 @@ paths: - Process Instances responses: "200": - description: Empty ok true response on successful resume. + description: Empty ok true response on successful reset. content: application/json: schema: diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/example_read_only.yml b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/example_read_only.yml index b6facc64..10a22088 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/example_read_only.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/example_read_only.yml @@ -1,70 +1,17 @@ - groups: admin: users: [admin@spiffworkflow.org] permissions: - admin: + process-groups-ro: groups: [admin] allowed_permissions: [read] - uri: /* - - tasks-crud: + uri: PG:ALL + basic: groups: [admin] - allowed_permissions: [create, update, delete] - uri: /tasks/* - - process-instances-crud: - groups: [ admin ] - allowed_permissions: [create, update, delete] - uri: /process-instances/* - - suspend: + allowed_permissions: [ALL] + uri: BASIC + elevated-operations: groups: [admin] - allowed_permissions: [create] - uri: /v1.0/process-instance-suspend - - terminate: - groups: [admin] - allowed_permissions: [create] - uri: /v1.0/process-instance-terminate - - resume: - groups: [admin] - allowed_permissions: [create] - uri: /v1.0/process-instance-resume - - reset: - groups: [admin] - allowed_permissions: [create] - uri: /v1.0/process-instance-reset - - users-exist: - groups: [admin] - allowed_permissions: [create] - uri: /v1.0/users/exists/by-username - - send-event: - groups: [admin] - allowed_permissions: [create] - uri: /v1.0/send-event/* - - task-complete: - groups: [admin] - allowed_permissions: [create] - uri: /v1.0/task-complete/* - - messages: - groups: [admin] - allowed_permissions: [create] - uri: /v1.0/messages/* - - secrets: - groups: [admin] - allowed_permissions: [create, update, delete] - uri: /v1.0/secrets/* - - task-data: - groups: [admin] - allowed_permissions: [update] - uri: /v1.0/task-data/* + allowed_permissions: [ALL] + uri: ELEVATED diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py index 1ff58394..0f63531c 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py @@ -438,10 +438,6 @@ class AuthorizationService: process_related_path_segment: str, target_uris: list[str], ) -> list[PermissionToAssign]: - permissions = permission_set.split(",") - if permission_set == "all": - permissions = ["create", "read", "update", "delete"] - permissions_to_assign: list[PermissionToAssign] = [] # we were thinking that if you can start an instance, you ought to be able to: @@ -472,7 +468,9 @@ class AuthorizationService: ]: permissions_to_assign.append(PermissionToAssign(permission="read", target_uri=target_uri)) else: + permissions = permission_set.split(",") if permission_set == "all": + permissions = ["create", "read", "update", "delete"] for path_segment_dict in PATH_SEGMENTS_FOR_PERMISSION_ALL: target_uri = f"{path_segment_dict['path']}/{process_related_path_segment}" relevant_permissions = path_segment_dict["relevant_permissions"] @@ -512,6 +510,29 @@ class AuthorizationService: permissions_to_assign.append(PermissionToAssign(permission=permission, target_uri="/tasks/*")) return permissions_to_assign + @classmethod + def set_elevated_permissions(cls) -> list[PermissionToAssign]: + permissions_to_assign: list[PermissionToAssign] = [] + for process_instance_action in ["resume", "terminate", "suspend", "reset"]: + permissions_to_assign.append( + PermissionToAssign(permission="create", target_uri=f"/process-instances-{process_instance_action}/*") + ) + + # FIXME: we need to fix so that user that can start a process-model + # can also start through messages as well + permissions_to_assign.append(PermissionToAssign(permission="create", target_uri="/messages/*")) + + permissions_to_assign.append(PermissionToAssign(permission="create", target_uri="/send-event/*")) + permissions_to_assign.append(PermissionToAssign(permission="create", target_uri="/task-complete/*")) + + # read comes from PG and PM permissions + permissions_to_assign.append(PermissionToAssign(permission="update", target_uri="/task-data/*")) + + for permission in ["create", "read", "update", "delete"]: + permissions_to_assign.append(PermissionToAssign(permission=permission, target_uri="/process-instances/*")) + permissions_to_assign.append(PermissionToAssign(permission=permission, target_uri="/secrets/*")) + return permissions_to_assign + @classmethod def set_process_group_permissions(cls, target: str, permission_set: str) -> list[PermissionToAssign]: permissions_to_assign: list[PermissionToAssign] = [] @@ -557,6 +578,8 @@ class AuthorizationService: * affects given process-model BASIC * Basic access to complete tasks and use the site + ELEVATED + * Operations that require elevated permissions Permission Macros: all @@ -579,6 +602,8 @@ class AuthorizationService: elif target.startswith("BASIC"): permissions_to_assign += cls.set_basic_permissions() + elif target.startswith("ELEVATED"): + permissions_to_assign += cls.set_elevated_permissions() elif target == "ALL": for permission in permissions: permissions_to_assign.append(PermissionToAssign(permission=permission, target_uri="/*")) 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 a1f41d3e..a96b3017 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py @@ -301,6 +301,34 @@ class TestAuthorizationService(BaseTest): permissions_to_assign_tuples = sorted([(p.target_uri, p.permission) for p in permissions_to_assign]) assert permissions_to_assign_tuples == expected_permissions + def test_explode_permissions_elevated( + self, + app: Flask, + client: FlaskClient, + with_db_and_bpmn_file_cleanup: None, + ) -> None: + expected_permissions = [ + ("/messages/*", "create"), + ("/process-instances-reset/*", "create"), + ("/process-instances-resume/*", "create"), + ("/process-instances-suspend/*", "create"), + ("/process-instances-terminate/*", "create"), + ("/process-instances/*", "create"), + ("/process-instances/*", "delete"), + ("/process-instances/*", "read"), + ("/process-instances/*", "update"), + ("/secrets/*", "create"), + ("/secrets/*", "delete"), + ("/secrets/*", "read"), + ("/secrets/*", "update"), + ("/send-event/*", "create"), + ("/task-complete/*", "create"), + ("/task-data/*", "update"), + ] + permissions_to_assign = AuthorizationService.explode_permissions("all", "ELEVATED") + permissions_to_assign_tuples = sorted([(p.target_uri, p.permission) for p in permissions_to_assign]) + assert permissions_to_assign_tuples == expected_permissions + def test_explode_permissions_all( self, app: Flask, From d6829fcb6d9e447ecc4836b6fb7c4469d52bba4b Mon Sep 17 00:00:00 2001 From: jasquat Date: Fri, 19 May 2023 11:23:44 -0400 Subject: [PATCH 6/6] merged in main and fixed acceptance tests --- spiffworkflow-frontend/cypress/support/commands.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spiffworkflow-frontend/cypress/support/commands.js b/spiffworkflow-frontend/cypress/support/commands.js index 80383593..e7b6ca6d 100644 --- a/spiffworkflow-frontend/cypress/support/commands.js +++ b/spiffworkflow-frontend/cypress/support/commands.js @@ -1,4 +1,3 @@ -import { string } from 'prop-types'; import { modifyProcessIdentifierForPathParam } from '../../src/helpers'; import { miscDisplayName } from './helpers'; import 'cypress-file-upload'; @@ -115,7 +114,10 @@ Cypress.Commands.add( cy.contains('Task: ', { timeout: 30000 }); } else { cy.url().should('include', `/interstitial`); - cy.contains('Status: Completed'); + // cy.contains('Status: Completed'); + cy.contains( + 'There are no additional instructions or information for this task.' + ); if (returnToProcessModelShow) { cy.getBySel('process-model-breadcrumb-link').click(); cy.getBySel('process-model-show-permissions-loaded').should('exist');