Perms for group navigate (#1470)

* wip related to giving access to groups when you have subgroups

* handle deny permissions and check normal permissions if pg parent method as well w/ burnettk

* reverted local dev perm file w/ burnettk

* a little test cleanup

---------

Co-authored-by: burnettk <burnettk@users.noreply.github.com>
Co-authored-by: jasquat <jasquat@users.noreply.github.com>

spiffworkflow-backend/src/spiffworkflow_backend/config/permissions/local_development.yml

@@ -41,10 +41,6 @@ permissions:
     groups: [group3]
     actions: [read]
     uri: PM:site-administration:set-permissions
-  pg-read:
-    groups: [group3]
-    actions: [read]
-    uri: PG:misc
 
   public_access:
     groups: [spiff_public]

spiffworkflow-backend/src/spiffworkflow_backend/models/permission_assignment.py

@@ -52,3 +52,10 @@ class PermissionAssignmentModel(SpiffworkflowBaseDBModel):
     @validates("permission")
     def validate_permission(self, key: str, value: str) -> Any:
         return self.validate_enum_field(key, value, Permission)
+
+    def __repr__(self) -> str:
+        value = (
+            f"PermissionAssignmentModel(id={self.id}, target={self.permission_target.uri}, "
+            f"permission={self.permission}, grant_type={self.grant_type})"
+        )
+        return value

spiffworkflow-backend/src/spiffworkflow_backend/routes/process_groups_controller.py

@@ -107,19 +107,22 @@ def process_group_show(
     modified_process_group_id: str,
 ) -> Any:
     process_group_id = _un_modify_modified_process_model_id(modified_process_group_id)
-    has_access_to_group_without_considering_process_models = True
+    has_access_to_group_without_considering_subgroups_and_models = True
     try:
         AuthorizationService.check_permission_for_request()
     except NotAuthorizedError:
-        has_access_to_group_without_considering_process_models = False
+        has_access_to_group_without_considering_subgroups_and_models = False
 
     try:
-        if has_access_to_group_without_considering_process_models:
+        user = UserService.current_user()
+        if (
+            has_access_to_group_without_considering_subgroups_and_models
+            or AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, process_group_id)
+        ):
             # do not return child models and groups here since this call does not check permissions of them
             process_group = ProcessModelService.get_process_group(process_group_id, find_direct_nested_items=False)
         else:
-            user = UserService.current_user()
-            process_group = ProcessModelService.get_process_group_with_permission_check(process_group_id, user)
+            raise ProcessEntityNotFoundError("viewing this process group is not authorized")
     except ProcessEntityNotFoundError as exception:
         raise (
             ApiError(

spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py

@@ -31,6 +31,7 @@ from spiffworkflow_backend.models.human_task import HumanTaskModel
 from spiffworkflow_backend.models.permission_assignment import PermissionAssignmentModel
 from spiffworkflow_backend.models.permission_target import PermissionTargetModel
 from spiffworkflow_backend.models.principal import PrincipalModel
+from spiffworkflow_backend.models.process_model import ProcessModelInfo
 from spiffworkflow_backend.models.service_account import SPIFF_SERVICE_ACCOUNT_AUTH_SERVICE
 from spiffworkflow_backend.models.task import TaskModel  # noqa: F401
 from spiffworkflow_backend.models.user import SPIFF_GUEST_USER
@@ -175,14 +176,18 @@ class AuthorizationService:
                 permission_assignment.permission_target.uri, target_uri_normalized
             ):
                 matching_permission_assignments.append(permission_assignment)
-        if len(matching_permission_assignments) == 0:
+
+        return cls.has_permissions_and_all_permissions_permit(matching_permission_assignments)
+
+    @classmethod
+    def has_permissions_and_all_permissions_permit(cls, permission_assignments: list[PermissionAssignmentModel]) -> bool:
+        if len(permission_assignments) == 0:
             return False
 
-        all_permissions_permit = True
-        for permission_assignment in matching_permission_assignments:
+        for permission_assignment in permission_assignments:
             if permission_assignment.grant_type == "deny":
-                all_permissions_permit = False
-        return all_permissions_permit
+                return False
+        return True
 
     @classmethod
     def target_uri_matches_actual_uri(cls, target_uri: str, actual_uri: str) -> bool:
@@ -205,6 +210,36 @@ class AuthorizationService:
             db.session.delete(group)
         db.session.commit()
 
+    # if you have access to PG:hey:%, you should be able to see PG hey, obviously.
+    # if you have access to PG:hey:yo:%, you should ALSO be able to see PG hey, because that allows you to navigate to hey:yo.
+    # if you have access to PG:hey:%, you should be able to see PG hey:yo, but that is handled by the normal permissions system,
+    # and we will never ask that question of this method.
+    @classmethod
+    def is_user_allowed_to_view_process_group_with_id(cls, user: UserModel, group_identifier: str) -> bool:
+        modified_group_identifier = ProcessModelInfo.modify_process_identifier_for_path_param(group_identifier)
+        has_permission = cls.user_has_permission(
+            user=user,
+            permission="read",
+            target_uri=f"/process-groups/{modified_group_identifier}",
+        )
+        if has_permission:
+            return True
+        all_permission_assignments = cls.all_permission_assignments_for_user(user)
+        matching_permission_assignments = []
+        for permission_assignment in all_permission_assignments:
+            uri = permission_assignment.permission_target.uri
+            uri_as_pg_identifier = uri.removeprefix("/process-groups/").removesuffix(":%")
+            if permission_assignment.permission == "read" and (
+                permission_assignment.grant_type == "permit"
+                and (
+                    uri.startswith(f"/process-groups/{modified_group_identifier}")
+                    or uri.startswith(f"/process-models/{modified_group_identifier}")
+                )
+                or (permission_assignment.grant_type == "deny" and modified_group_identifier.startswith(uri_as_pg_identifier))
+            ):
+                matching_permission_assignments.append(permission_assignment)
+        return cls.has_permissions_and_all_permissions_permit(matching_permission_assignments)
+
     @classmethod
     def associate_user_with_group(cls, user: UserModel, group: GroupModel) -> None:
         user_group_assignemnt = UserGroupAssignmentModel.query.filter_by(user_id=user.id, group_id=group.id).first()

spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_service.py

@@ -671,12 +671,3 @@ class ProcessModelService(FileSystemService):
             # we don't store `id` in the json files, so we add it in here
             process_model_info.id = name
         return process_model_info
-
-    @classmethod
-    def get_process_group_with_permission_check(cls, process_group_id: str, user: UserModel) -> ProcessGroup:
-        process_models = cls.get_process_models_for_api(user=user, process_group_id=process_group_id, recursive=False)
-        if not process_models:
-            raise ProcessEntityNotFoundError("User has no access to contained models, and therefore no access to group.")
-
-        # do not return child models and groups here since this call does not check permissions of them
-        return cls.get_process_group(process_group_id, find_direct_nested_items=False)

spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py

@@ -335,6 +335,41 @@ 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")
 
+    # https://github.com/sartography/spiff-arena/issues/1090 describes why we need access to process_group_show for parents
+    def test_granting_access_to_subgroup_gives_access_to_subgroup_its_subgroups_and_even_show_for_its_parents(
+        self,
+        app: Flask,
+        client: FlaskClient,
+        with_db_and_bpmn_file_cleanup: None,
+    ) -> None:
+        user = self.find_or_create_user(username="user_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:anotherprefix:yo")
+        assert AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, "hey") is False
+
+        AuthorizationService.add_permission_from_uri_or_macro(user_group.identifier, "read", "PG:/hey/yo")
+        AuthorizationService.add_permission_from_uri_or_macro(user_group.identifier, "DENY:read", "PG:hey:yo:who")
+        assert AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, "hey")
+        assert AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, "hey/yo")
+        assert AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, "hey:yo")
+        assert AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, "hey/yo/who") is False
+        assert AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, "hey/yo/who/what") is False
+
+    def test_granting_access_to_model_gives_access_to_process_group_show_for_parent_groups_to_allow_navigating_to_model(
+        self,
+        app: Flask,
+        client: FlaskClient,
+        with_db_and_bpmn_file_cleanup: None,
+    ) -> None:
+        user = self.find_or_create_user(username="user_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", "PM:hey:yo:wow:hot")
+        assert AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, "hey")
+        assert AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, "hey/yo")
+        assert AuthorizationService.is_user_allowed_to_view_process_group_with_id(user, "hey/yo/wow")
+
     def test_explode_permissions_with_invalid_target_uri(
         self,
         app: Flask,