From 6cf550e974902047239614b478c3609949e9fc83 Mon Sep 17 00:00:00 2001 From: renczesstefan Date: Thu, 5 Mar 2026 13:40:20 +0100 Subject: [PATCH 1/4] [NAE-2387] Fix permission evaluation order between ActorRef and processRole - Introduced `checkPermissions` method in `AbstractAuthorizationService` to streamline permission verification logic across services. - Updated `TaskAuthorizationService` and `WorkflowAuthorizationService` to utilize the new `checkPermissions` method. - Added roles and actor references to XML test configuration for improved test scenarios. - Expanded `TaskAuthorizationServiceTest` with new unit tests to cover additional role and actor reference assignment scenarios. --- .../service/AbstractAuthorizationService.java | 13 ++++-- .../service/TaskAuthorizationService.java | 7 ++-- .../service/WorkflowAuthorizationService.java | 6 +-- .../auth/TaskAuthorizationServiceTest.groovy | 41 +++++++++++++++++++ ...thorization_service_test_with_userRefs.xml | 20 +++++++++ 5 files changed, 75 insertions(+), 12 deletions(-) diff --git a/application-engine/src/main/java/com/netgrif/application/engine/workflow/service/AbstractAuthorizationService.java b/application-engine/src/main/java/com/netgrif/application/engine/workflow/service/AbstractAuthorizationService.java index d6a9479369..2a691c8f30 100644 --- a/application-engine/src/main/java/com/netgrif/application/engine/workflow/service/AbstractAuthorizationService.java +++ b/application-engine/src/main/java/com/netgrif/application/engine/workflow/service/AbstractAuthorizationService.java @@ -2,14 +2,19 @@ import com.netgrif.application.engine.objects.auth.domain.AbstractUser; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.stream.Collectors; public abstract class AbstractAuthorizationService { + protected Boolean checkPermissions(Map providedPermissions, List requiredPermissions) { + if (requiredPermissions.stream().allMatch(permission -> providedPermissions.get(permission) == null)) { + return null; + } + return requiredPermissions.stream() + .anyMatch(permission -> hasPermission(providedPermissions.get(permission))); + } + protected boolean hasPermission(Boolean permissionValue) { return permissionValue != null && permissionValue; } diff --git a/application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskAuthorizationService.java b/application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskAuthorizationService.java index fa2be2cce0..0a3d0d12ff 100644 --- a/application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskAuthorizationService.java +++ b/application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskAuthorizationService.java @@ -36,8 +36,7 @@ public Boolean userHasAtLeastOneRolePermission(AbstractUser user, Task task, Rol } } - return Arrays.stream(permissions) - .anyMatch(permission -> hasPermission(aggregatePermissions.get(permission.toString()))); + return checkPermissions(aggregatePermissions, Arrays.stream(permissions).map(RolePermission::toString).toList()); } @Override @@ -62,8 +61,8 @@ public Boolean userHasUserListPermission(AbstractUser user, Task task, RolePermi return false; } } - return Arrays.stream(permissions) - .anyMatch(permission -> hasPermission(userPermissions.get(permission.toString()))); + + return checkPermissions(userPermissions, Arrays.stream(permissions).map(RolePermission::toString).toList()); } @Override diff --git a/application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowAuthorizationService.java b/application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowAuthorizationService.java index d3111107a5..ae4f4aadd7 100644 --- a/application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowAuthorizationService.java +++ b/application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowAuthorizationService.java @@ -63,8 +63,7 @@ public Boolean userHasAtLeastOneRolePermission(AbstractUser user, PetriNet net, } } - return Arrays.stream(permissions) - .anyMatch(permission -> hasPermission(aggregatePermissions.get(permission.toString()))); + return checkPermissions(aggregatePermissions, Arrays.stream(permissions).map(ProcessRolePermission::toString).toList()); } @Override @@ -84,8 +83,7 @@ public Boolean userHasUserListPermission(AbstractUser user, Case useCase, Proces return false; } } - return Arrays.stream(permissions) - .anyMatch(permission -> hasPermission(userPermissions.get(permission.toString()))); + return checkPermissions(userPermissions, Arrays.stream(permissions).map(ProcessRolePermission::toString).toList()); } private Map findUserPermissions(Case useCase, AbstractUser user) { diff --git a/application-engine/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy b/application-engine/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy index b68de6738a..e7cadf5f54 100644 --- a/application-engine/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy +++ b/application-engine/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy @@ -590,4 +590,45 @@ class TaskAuthorizationServiceTest { workflowService.deleteCase(new DeleteCaseParams(case_.stringId)) } + @Test + void testCanAssignWithRoleAssignTrueAndWithActorRefAssignUndefined() { + ProcessRole positiveRole = this.netWithUserRefs.getRoles().values().find(v -> v.getImportId() == "assign_pos_role") + userService.addRole(testUser, positiveRole.get_id()) + Case case_ = workflowService.createCase(CreateCaseParams.with() + .process(netWithUserRefs) + .title("Test assign") + .color("") + .author(ActorTransformer.toLoggedUser(testUser)) + .build()).getCase() + + String taskId = (new ArrayList<>(case_.getTasks())).get(0).task + case_ = dataService.setData(taskId, ImportHelper.populateDataset([ + "view_pos_ul": [ + "value": [testUser.stringId], + "type": "actorList" + ] + ] as Map)).getCase() + assert taskAuthorizationService.canCallAssign(ActorTransformer.toLoggedUser(testUser), (new ArrayList<>(case_.getTasks())).get(0).task) + } + + @Test + void testCanAssignWithRoleAssignUndefinedAndWithActorRefAssignTrue() { + ProcessRole positiveRole = this.netWithUserRefs.getRoles().values().find(v -> v.getImportId() == "view_pos_role") + userService.addRole(testUser, positiveRole.get_id()) + Case case_ = workflowService.createCase(CreateCaseParams.with() + .process(netWithUserRefs) + .title("Test assign") + .color("") + .author(ActorTransformer.toLoggedUser(testUser)) + .build()).getCase() + + String taskId = (new ArrayList<>(case_.getTasks())).get(0).task + case_ = dataService.setData(taskId, ImportHelper.populateDataset([ + "assign_pos_ul": [ + "value": [testUser.stringId], + "type": "actorList" + ] + ] as Map)).getCase() + assert taskAuthorizationService.canCallAssign(ActorTransformer.toLoggedUser(testUser), (new ArrayList<>(case_.getTasks())).get(0).task) + } } diff --git a/application-engine/src/test/resources/task_authorization_service_test_with_userRefs.xml b/application-engine/src/test/resources/task_authorization_service_test_with_userRefs.xml index 1171a5c9f7..22808453ae 100644 --- a/application-engine/src/test/resources/task_authorization_service_test_with_userRefs.xml +++ b/application-engine/src/test/resources/task_authorization_service_test_with_userRefs.xml @@ -20,6 +20,10 @@ finish_neg_role finish neg role + + view_pos_role + view pos role + assign_pos_ul @@ -52,6 +56,10 @@ <id>finish_neg_ul</id> <title/> </data> + <data type="actorList"> + <id>view_pos_ul</id> + <title/> + </data> <data type="text"> <id>text</id> <title>Text @@ -90,6 +98,12 @@ false + + view_pos_role + + true + + assign_pos_ul @@ -138,6 +152,12 @@ false + + view_pos_ul + + false + + text From 9874fd1f016d3b6674be1cb37389f9c8ee0c448a Mon Sep 17 00:00:00 2001 From: renczesstefan Date: Thu, 5 Mar 2026 13:53:09 +0100 Subject: [PATCH 2/4] [NAE-2387] Fix permission evaluation order between ActorRef and processRole Expanded XML test config with additional roles and actor references to support new scenarios. Introduced two new unit tests in `WorkflowAuthorizationServiceTest` to validate delete permissions based on role and actor reference combinations. --- .../WorkflowAuthorizationServiceTest.groovy | 40 +++++++++++++++++++ ...thorization_service_test_with_userRefs.xml | 20 ++++++++++ 2 files changed, 60 insertions(+) diff --git a/application-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovy b/application-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovy index 9872f6c511..9f13e8cad7 100644 --- a/application-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovy +++ b/application-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovy @@ -311,6 +311,46 @@ class WorkflowAuthorizationServiceTest { userService.removeRole(testUser, negDeleteRole.getStringId()) } + @Test + void testCanCallDeleteWithRoleDeleteTrueAndActorRefDeleteUndefined() { + ProcessRole positiveDeleteRole = this.netWithUserRefs.getRoles().values().find(v -> v.getImportId() == "delete_pos_role") + userService.addRole(testUser, positiveDeleteRole.getStringId()) + Case case_ = workflowService.createCase(CreateCaseParams.with() + .process(netWithUserRefs) + .title("Test delete") + .color("") + .author(ActorTransformer.toLoggedUser(testUser)) + .build()).getCase() + String taskId = (new ArrayList<>(case_.getTasks())).get(0).task + case_ = dataService.setData(taskId, ImportHelper.populateDataset([ + "view_actor_list": [ + "value": [testUser.stringId], + "type": "actorList" + ] + ] as Map)).getCase() + assert workflowAuthorizationService.canCallDelete(ActorTransformer.toLoggedUser(testUser), case_.getStringId()) + } + + @Test + void testCanCallDeleteWithRoleDeleteUndefinedAndActorRefDeleteTrue() { + ProcessRole positiveDeleteRole = this.netWithUserRefs.getRoles().values().find(v -> v.getImportId() == "view_pos_role") + userService.addRole(testUser, positiveDeleteRole.getStringId()) + Case case_ = workflowService.createCase(CreateCaseParams.with() + .process(netWithUserRefs) + .title("Test delete") + .color("") + .author(ActorTransformer.toLoggedUser(testUser)) + .build()).getCase() + String taskId = (new ArrayList<>(case_.getTasks())).get(0).task + case_ = dataService.setData(taskId, ImportHelper.populateDataset([ + "pos_user_list": [ + "value": [testUser.stringId], + "type": "actorList" + ] + ] as Map)).getCase() + assert workflowAuthorizationService.canCallDelete(ActorTransformer.toLoggedUser(testUser), case_.getStringId()) + } + @SuppressWarnings("GrMethodMayBeStatic") private def parseResult(MvcResult result) { return (new JsonSlurper()).parseText(result.response.contentAsString) diff --git a/application-engine/src/test/resources/workflow_authorization_service_test_with_userRefs.xml b/application-engine/src/test/resources/workflow_authorization_service_test_with_userRefs.xml index 634e1e300f..5df1c89ecc 100644 --- a/application-engine/src/test/resources/workflow_authorization_service_test_with_userRefs.xml +++ b/application-engine/src/test/resources/workflow_authorization_service_test_with_userRefs.xml @@ -28,6 +28,12 @@ false + + view_pos_role + + true + + neg_user_list @@ -40,6 +46,12 @@ true + + view_actor_list + + true + + delete_pos_role delete role @@ -56,6 +68,10 @@ create_neg_role create role + + view_pos_role + view role + pos_user_list @@ -64,6 +80,10 @@ <id>neg_user_list</id> <title/> </data> + <data type="actorList"> + <id>view_actor_list</id> + <title/> + </data> <data type="text"> <id>text</id> <title>Text From 8d5535ad6ec89a98078e3825d314df5eb2e5523c Mon Sep 17 00:00:00 2001 From: renczesstefan Date: Thu, 5 Mar 2026 14:07:43 +0100 Subject: [PATCH 3/4] - renamed variable --- .../engine/auth/WorkflowAuthorizationServiceTest.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/application-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovy b/application-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovy index 9f13e8cad7..dd9517e20a 100644 --- a/application-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovy +++ b/application-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovy @@ -333,8 +333,8 @@ class WorkflowAuthorizationServiceTest { @Test void testCanCallDeleteWithRoleDeleteUndefinedAndActorRefDeleteTrue() { - ProcessRole positiveDeleteRole = this.netWithUserRefs.getRoles().values().find(v -> v.getImportId() == "view_pos_role") - userService.addRole(testUser, positiveDeleteRole.getStringId()) + ProcessRole positiveViewRole = this.netWithUserRefs.getRoles().values().find(v -> v.getImportId() == "view_pos_role") + userService.addRole(testUser, positiveViewRole.getStringId()) Case case_ = workflowService.createCase(CreateCaseParams.with() .process(netWithUserRefs) .title("Test delete") From 531a2acb5e678e24dd4f60409e59ded69ba78fab Mon Sep 17 00:00:00 2001 From: renczesstefan Date: Thu, 5 Mar 2026 14:13:31 +0100 Subject: [PATCH 4/4] Enable view permission in task authorization tests Updated the actorRef logic to set the "view" permission to true in the test XML. This change ensures the test reflects the updated authorization configuration. --- .../resources/task_authorization_service_test_with_userRefs.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application-engine/src/test/resources/task_authorization_service_test_with_userRefs.xml b/application-engine/src/test/resources/task_authorization_service_test_with_userRefs.xml index 22808453ae..47b24c6841 100644 --- a/application-engine/src/test/resources/task_authorization_service_test_with_userRefs.xml +++ b/application-engine/src/test/resources/task_authorization_service_test_with_userRefs.xml @@ -155,7 +155,7 @@ view_pos_ul - false + true