From 72dcae8b063873eec8e26668dcdcab2afc01ea78 Mon Sep 17 00:00:00 2001 From: renczesstefan Date: Tue, 14 Apr 2026 15:51:28 +0200 Subject: [PATCH 1/6] [NAE-2407] Improve Permission Validation in GetData Introduced `canCallGetData` method in `TaskAuthorizationService` and applied `@PreAuthorize` annotations to secure `getData` endpoints in `TaskController` and `PublicTaskController`. Updated related method signatures to include `Authentication` parameter for proper authorization handling. --- .../engine/workflow/service/TaskAuthorizationService.java | 7 +++++++ .../service/interfaces/ITaskAuthorizationService.java | 2 ++ .../engine/workflow/web/AbstractTaskController.java | 2 +- .../engine/workflow/web/PublicTaskController.java | 5 +++-- .../application/engine/workflow/web/TaskController.java | 5 +++-- .../workflow/web/responsebodies/DataGroupsResource.java | 2 +- .../workflow/web/responsebodies/LocalisedTaskResource.java | 2 +- 7 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/netgrif/application/engine/workflow/service/TaskAuthorizationService.java b/src/main/java/com/netgrif/application/engine/workflow/service/TaskAuthorizationService.java index 3f434663c15..3e21349e824 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/service/TaskAuthorizationService.java +++ b/src/main/java/com/netgrif/application/engine/workflow/service/TaskAuthorizationService.java @@ -141,6 +141,13 @@ public boolean canCallSaveData(LoggedUser loggedUser, String taskId) { return loggedUser.getSelfOrImpersonated().isAdmin() || isAssignee(loggedUser, taskId); } + @Override + public boolean canCallGetData(LoggedUser loggedUser, String taskId) { + Boolean rolePerm = userHasAtLeastOneRolePermission(loggedUser, taskId, RolePermission.VIEW); + Boolean userPerm = userHasUserListPermission(loggedUser, taskId, RolePermission.VIEW); + return loggedUser.getSelfOrImpersonated().isAdmin() || (userPerm == null ? (rolePerm != null && rolePerm) : userPerm); + } + @Override public boolean canCallSaveFile(LoggedUser loggedUser, String taskId) { return loggedUser.getSelfOrImpersonated().isAdmin() || isAssignee(loggedUser, taskId); diff --git a/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/ITaskAuthorizationService.java b/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/ITaskAuthorizationService.java index e65e9a3d013..db79954bc09 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/ITaskAuthorizationService.java +++ b/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/ITaskAuthorizationService.java @@ -31,6 +31,8 @@ public interface ITaskAuthorizationService { boolean canCallSaveData(LoggedUser loggedUser, String taskId); + boolean canCallGetData(LoggedUser loggedUser, String taskId); + boolean canCallSaveFile(LoggedUser loggedUser, String taskId); } diff --git a/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java b/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java index 0e295542652..70862bce63a 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java +++ b/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java @@ -198,7 +198,7 @@ public CountResponse count(SingleElasticTaskSearchRequestAsList query, MergeFilt } - public EntityModel getData(String taskId, Locale locale) { + public EntityModel getData(String taskId, Authentication authentication, Locale locale) { try { GetDataGroupsEventOutcome outcome = dataService.getDataGroups(taskId, locale); return EventOutcomeWithMessageResource.successMessage("Get data groups successful", diff --git a/src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java b/src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java index 02818262191..c5e510c7d5b 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java +++ b/src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java @@ -111,10 +111,11 @@ public EntityModel cancel(@PathVariable("id") String ta } @Override + @PreAuthorize("@taskAuthorizationService.canCallGetData(#auth.getPrincipal(), #taskId)") @GetMapping(value = "/{id}/data", produces = MediaTypes.HAL_JSON_VALUE) @Operation(summary = "Get all task data") - public EntityModel getData(@PathVariable("id") String taskId, Locale locale) { - return super.getData(taskId, locale); + public EntityModel getData(@PathVariable("id") String taskId, Authentication auth, Locale locale) { + return super.getData(taskId, auth, locale); } @Override diff --git a/src/main/java/com/netgrif/application/engine/workflow/web/TaskController.java b/src/main/java/com/netgrif/application/engine/workflow/web/TaskController.java index 593f37ce4ee..251a08f9d0c 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/web/TaskController.java +++ b/src/main/java/com/netgrif/application/engine/workflow/web/TaskController.java @@ -177,10 +177,11 @@ public CountResponse count(@RequestBody SingleElasticTaskSearchRequestAsList que } @Override + @PreAuthorize("@taskAuthorizationService.canCallGetData(#auth.getPrincipal(), #taskId)") @Operation(summary = "Get all task data", security = {@SecurityRequirement(name = "BasicAuth")}) @GetMapping(value = "/{id}/data", produces = MediaTypes.HAL_JSON_VALUE) - public EntityModel getData(@PathVariable("id") String taskId, Locale locale) { - return super.getData(taskId, locale); + public EntityModel getData(@PathVariable("id") String taskId, Authentication auth, Locale locale) { + return super.getData(taskId, auth, locale); } @PreAuthorize("@taskAuthorizationService.canCallSaveData(#auth.getPrincipal(), #taskId)") diff --git a/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/DataGroupsResource.java b/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/DataGroupsResource.java index 3b37ed432d4..b90840f8ae3 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/DataGroupsResource.java +++ b/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/DataGroupsResource.java @@ -23,6 +23,6 @@ public DataGroupsResource(Collection Date: Tue, 14 Apr 2026 16:12:01 +0200 Subject: [PATCH 2/6] Refactor task data retrieval and link building logic Updated `@PreAuthorize` annotation to use anonymous user context when fetching task data. Refactored `buildLinks` method to pass task ID dynamically, ensuring proper link generation based on the parent task. --- .../engine/workflow/web/PublicTaskController.java | 2 +- .../web/responsebodies/DataGroupsResource.java | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java b/src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java index c5e510c7d5b..3e93314ae25 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java +++ b/src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java @@ -111,7 +111,7 @@ public EntityModel cancel(@PathVariable("id") String ta } @Override - @PreAuthorize("@taskAuthorizationService.canCallGetData(#auth.getPrincipal(), #taskId)") + @PreAuthorize("@taskAuthorizationService.canCallGetData(@userService.getAnonymousLogged(), #taskId)") @GetMapping(value = "/{id}/data", produces = MediaTypes.HAL_JSON_VALUE) @Operation(summary = "Get all task data") public EntityModel getData(@PathVariable("id") String taskId, Authentication auth, Locale locale) { diff --git a/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/DataGroupsResource.java b/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/DataGroupsResource.java index b90840f8ae3..08a5efb98b9 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/DataGroupsResource.java +++ b/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/DataGroupsResource.java @@ -18,11 +18,16 @@ public DataGroupsResource(Collection id != null && !id.isBlank()) + .findFirst() + .orElse(null); + buildLinks(taskId); } - private void buildLinks() { + private void buildLinks(String taskId) { add(WebMvcLinkBuilder.linkTo(WebMvcLinkBuilder.methodOn(TaskController.class) - .getData("", null, null)).withSelfRel()); + .getData(taskId, null, null)).withSelfRel()); } } From 284962344022b6f713be3792dc1eee1e920c1cd4 Mon Sep 17 00:00:00 2001 From: renczesstefan Date: Wed, 15 Apr 2026 09:03:59 +0200 Subject: [PATCH 3/6] Remove unnecessary Authentication parameter from getData methods Refactored getData methods in TaskController, PublicTaskController, and AbstractTaskController to exclude the unused Authentication parameter. This streamlines method signatures and improves code clarity while retaining functionality and security checks. --- .../engine/workflow/web/AbstractTaskController.java | 2 +- .../engine/workflow/web/PublicTaskController.java | 5 ++--- .../application/engine/workflow/web/TaskController.java | 3 +-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java b/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java index 70862bce63a..0e295542652 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java +++ b/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java @@ -198,7 +198,7 @@ public CountResponse count(SingleElasticTaskSearchRequestAsList query, MergeFilt } - public EntityModel getData(String taskId, Authentication authentication, Locale locale) { + public EntityModel getData(String taskId, Locale locale) { try { GetDataGroupsEventOutcome outcome = dataService.getDataGroups(taskId, locale); return EventOutcomeWithMessageResource.successMessage("Get data groups successful", diff --git a/src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java b/src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java index 3e93314ae25..e1f244d9d4f 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java +++ b/src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java @@ -110,12 +110,11 @@ public EntityModel cancel(@PathVariable("id") String ta return super.cancel(loggedUser, taskId, locale); } - @Override @PreAuthorize("@taskAuthorizationService.canCallGetData(@userService.getAnonymousLogged(), #taskId)") @GetMapping(value = "/{id}/data", produces = MediaTypes.HAL_JSON_VALUE) @Operation(summary = "Get all task data") - public EntityModel getData(@PathVariable("id") String taskId, Authentication auth, Locale locale) { - return super.getData(taskId, auth, locale); + public EntityModel getData(@PathVariable("id") String taskId, Locale locale) { + return super.getData(taskId, locale); } @Override diff --git a/src/main/java/com/netgrif/application/engine/workflow/web/TaskController.java b/src/main/java/com/netgrif/application/engine/workflow/web/TaskController.java index 251a08f9d0c..70509da528f 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/web/TaskController.java +++ b/src/main/java/com/netgrif/application/engine/workflow/web/TaskController.java @@ -176,12 +176,11 @@ public CountResponse count(@RequestBody SingleElasticTaskSearchRequestAsList que return super.count(query, operation, auth, locale); } - @Override @PreAuthorize("@taskAuthorizationService.canCallGetData(#auth.getPrincipal(), #taskId)") @Operation(summary = "Get all task data", security = {@SecurityRequirement(name = "BasicAuth")}) @GetMapping(value = "/{id}/data", produces = MediaTypes.HAL_JSON_VALUE) public EntityModel getData(@PathVariable("id") String taskId, Authentication auth, Locale locale) { - return super.getData(taskId, auth, locale); + return super.getData(taskId, locale); } @PreAuthorize("@taskAuthorizationService.canCallSaveData(#auth.getPrincipal(), #taskId)") From d646c6c17eb0a14036c60dbb1f962a6a695450cd Mon Sep 17 00:00:00 2001 From: renczesstefan Date: Wed, 15 Apr 2026 09:25:00 +0200 Subject: [PATCH 4/6] Add tests for view roles and userRefs in task authorization Extended the XML configuration and added new roles and userRefs for view permissions. Implemented test cases to verify task authorization behavior for positive and negative view permissions using both roles and userRefs. --- .../auth/TaskAuthorizationServiceTest.groovy | 53 +++++++++++++++++++ ...thorization_service_test_with_userRefs.xml | 40 ++++++++++++++ 2 files changed, 93 insertions(+) diff --git a/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy b/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy index 6b1271c6439..03c64e2a793 100644 --- a/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy +++ b/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy @@ -479,4 +479,57 @@ class TaskAuthorizationServiceTest { workflowService.deleteCase(case_.stringId) } + @Test + void testCanGetDataWithPosViewRole() { + ProcessRole positiveRole = this.netWithUserRefs.getRoles().values().find(v -> v.getImportId() == "view_pos_role") + userService.addRole(testUser, positiveRole.getStringId()) + Case case_ = workflowService.createCase(netWithUserRefs.getStringId(), "Test get data", "", testUser.transformToLoggedUser()).getCase() + assert taskAuthorizationService.canCallGetData(testUser.transformToLoggedUser(), (new ArrayList<>(case_.getTasks())).get(0).task) + userService.removeRole(testUser, positiveRole.getStringId()) + workflowService.deleteCase(case_.stringId) + } + + @Test + void testCannotGetDataWithNegViewRole() { + ProcessRole negativeRole = this.netWithUserRefs.getRoles().values().find(v -> v.getImportId() == "view_neg_role") + userService.addRole(testUser, negativeRole.getStringId()) + Case case_ = workflowService.createCase(netWithUserRefs.getStringId(), "Test get data", "", testUser.transformToLoggedUser()).getCase() + assert !taskAuthorizationService.canCallGetData(testUser.transformToLoggedUser(), (new ArrayList<>(case_.getTasks())).get(0).task) + userService.removeRole(testUser, negativeRole.getStringId()) + workflowService.deleteCase(case_.stringId) + } + + @Test + void testCanGetDataWithPosViewUserRef() { + Case case_ = workflowService.createCase(netWithUserRefs.getStringId(), "Test get data", "", testUser.transformToLoggedUser()).getCase() + String taskId = (new ArrayList<>(case_.getTasks())).get(0).task + case_ = dataService.setData(taskId, ImportHelper.populateDataset([ + "view_pos_ul": [ + "value": [testUser.stringId], + "type": "userList" + ] + ] as Map)).getCase() + workflowService.save(case_) + sleep(4000) + + assert taskAuthorizationService.canCallGetData(testUser.transformToLoggedUser(), taskId) + workflowService.deleteCase(case_.stringId) + } + + @Test + void testCannotGetDataWithNegViewUserRef() { + Case case_ = workflowService.createCase(netWithUserRefs.getStringId(), "Test get data", "", testUser.transformToLoggedUser()).getCase() + String taskId = (new ArrayList<>(case_.getTasks())).get(0).task + case_ = dataService.setData(taskId, ImportHelper.populateDataset([ + "view_neg_ul": [ + "value": [testUser.stringId], + "type": "userList" + ] + ] as Map)).getCase() + workflowService.save(case_) + sleep(4000) + + assert !taskAuthorizationService.canCallGetData(testUser.transformToLoggedUser(), taskId) + workflowService.deleteCase(case_.stringId) + } } diff --git a/src/test/resources/task_authorization_service_test_with_userRefs.xml b/src/test/resources/task_authorization_service_test_with_userRefs.xml index ef5c5f338d7..fa6b93beeb0 100644 --- a/src/test/resources/task_authorization_service_test_with_userRefs.xml +++ b/src/test/resources/task_authorization_service_test_with_userRefs.xml @@ -4,6 +4,10 @@ wst_usersRef WSU WorkflowAuthorizationService test + + view_pos_role + view pos role + assign_pos_role assign pos role @@ -12,6 +16,10 @@ finish_pos_role finish pos role + + view_neg_role + view neg role + assign_neg_role assign neg role @@ -20,6 +28,10 @@ finish_neg_role finish neg role + + view_pos_ul + + </data> <data type="userList"> <id>assign_pos_ul</id> <title/> @@ -40,6 +52,10 @@ <id>cancel_pos_ul</id> <title/> </data> + <data type="userList"> + <id>view_neg_ul</id> + <title/> + </data> <data type="userList"> <id>cancel_neg_ul</id> <title/> @@ -66,6 +82,18 @@ <x>1</x> <y>1</y> <label>Transition</label> + <roleRef> + <id>view_pos_role</id> + <logic> + <view>true</view> + </logic> + </roleRef> + <roleRef> + <id>view_neg_role</id> + <logic> + <view>false</view> + </logic> + </roleRef> <roleRef> <id>assign_pos_role</id> <logic> @@ -90,6 +118,18 @@ <finish>false</finish> </logic> </roleRef> + <userRef> + <id>view_pos_ul</id> + <logic> + <view>true</view> + </logic> + </userRef> + <userRef> + <id>view_neg_ul</id> + <logic> + <view>false</view> + </logic> + </userRef> <userRef> <id>assign_pos_ul</id> <logic> From 115a961a1dcceede13aa60a08c97df708f293375 Mon Sep 17 00:00:00 2001 From: renczesstefan <renczes.stefan@gmail.com> Date: Wed, 15 Apr 2026 09:32:23 +0200 Subject: [PATCH 5/6] Remove unnecessary sleep calls in TaskAuthorizationServiceTest Eliminated redundant `sleep(4000)` calls in test cases to improve execution efficiency and reduce test runtime. The changes do not affect the test logic or expected outcomes. --- .../application/engine/auth/TaskAuthorizationServiceTest.groovy | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy b/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy index 03c64e2a793..8e0ae78a8da 100644 --- a/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy +++ b/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy @@ -510,7 +510,6 @@ class TaskAuthorizationServiceTest { ] ] as Map)).getCase() workflowService.save(case_) - sleep(4000) assert taskAuthorizationService.canCallGetData(testUser.transformToLoggedUser(), taskId) workflowService.deleteCase(case_.stringId) @@ -527,7 +526,6 @@ class TaskAuthorizationServiceTest { ] ] as Map)).getCase() workflowService.save(case_) - sleep(4000) assert !taskAuthorizationService.canCallGetData(testUser.transformToLoggedUser(), taskId) workflowService.deleteCase(case_.stringId) From 81ba9422b3e39b0375b329beb2b5dc99ee6321bf Mon Sep 17 00:00:00 2001 From: renczesstefan <renczes.stefan@gmail.com> Date: Wed, 15 Apr 2026 09:33:15 +0200 Subject: [PATCH 6/6] Remove redundant workflowService.save calls in tests The unnecessary `workflowService.save` calls have been removed from `testCanGetDataWithPosViewUserRef` and `testCannotGetDataWithNegViewUserRef` in `TaskAuthorizationServiceTest.groovy`. This simplifies the code and improves test clarity without affecting functionality. --- .../engine/auth/TaskAuthorizationServiceTest.groovy | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy b/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy index 8e0ae78a8da..5e47ee44982 100644 --- a/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy +++ b/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy @@ -503,13 +503,12 @@ class TaskAuthorizationServiceTest { void testCanGetDataWithPosViewUserRef() { Case case_ = workflowService.createCase(netWithUserRefs.getStringId(), "Test get data", "", testUser.transformToLoggedUser()).getCase() String taskId = (new ArrayList<>(case_.getTasks())).get(0).task - case_ = dataService.setData(taskId, ImportHelper.populateDataset([ + dataService.setData(taskId, ImportHelper.populateDataset([ "view_pos_ul": [ "value": [testUser.stringId], "type": "userList" ] ] as Map)).getCase() - workflowService.save(case_) assert taskAuthorizationService.canCallGetData(testUser.transformToLoggedUser(), taskId) workflowService.deleteCase(case_.stringId) @@ -519,13 +518,12 @@ class TaskAuthorizationServiceTest { void testCannotGetDataWithNegViewUserRef() { Case case_ = workflowService.createCase(netWithUserRefs.getStringId(), "Test get data", "", testUser.transformToLoggedUser()).getCase() String taskId = (new ArrayList<>(case_.getTasks())).get(0).task - case_ = dataService.setData(taskId, ImportHelper.populateDataset([ + dataService.setData(taskId, ImportHelper.populateDataset([ "view_neg_ul": [ "value": [testUser.stringId], "type": "userList" ] ] as Map)).getCase() - workflowService.save(case_) assert !taskAuthorizationService.canCallGetData(testUser.transformToLoggedUser(), taskId) workflowService.deleteCase(case_.stringId)