Skip to content

[NAE-2407] Improve Permission Validation in GetData #431

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
machacjozef merged 6 commits into from
Apr 16, 2026
Merged

[NAE-2407] Improve Permission Validation in GetData #431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
72dcae8
[NAE-2407] Improve Permission Validation in GetData
renczesstefan Apr 14, 2026
e4b431a
Refactor task data retrieval and link building logic
renczesstefan Apr 14, 2026
2849623
Remove unnecessary Authentication parameter from getData methods
renczesstefan Apr 15, 2026
d646c6c
Add tests for view roles and userRefs in task authorization
renczesstefan Apr 15, 2026
115a961
Remove unnecessary sleep calls in TaskAuthorizationServiceTest
renczesstefan Apr 15, 2026
81ba942
Remove redundant workflowService.save calls in tests
renczesstefan Apr 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/main/java/com/netgrif/application/engine/workflow/service/TaskAuthorizationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Comment thread
Retoocs marked this conversation as resolved.
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);
Expand Down
2 changes: 2 additions & 0 deletions ...com/netgrif/application/engine/workflow/service/interfaces/ITaskAuthorizationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

}
2 changes: 1 addition & 1 deletion src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public EntityModel<EventOutcomeWithMessage> 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<EventOutcomeWithMessage> getData(@PathVariable("id") String taskId, Locale locale) {
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/netgrif/application/engine/workflow/web/TaskController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,10 @@ 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<EventOutcomeWithMessage> getData(@PathVariable("id") String taskId, Locale locale) {
public EntityModel<EventOutcomeWithMessage> getData(@PathVariable("id") String taskId, Authentication auth, Locale locale) {
return super.getData(taskId, locale);
}

Expand Down
11 changes: 8 additions & 3 deletions ...n/java/com/netgrif/application/engine/workflow/web/responsebodies/DataGroupsResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,16 @@ public DataGroupsResource(Collection<com.netgrif.application.engine.petrinet.dom
return dataGroup;
})
.collect(Collectors.toList()));
buildLinks();
String taskId = content.stream()
.map(com.netgrif.application.engine.petrinet.domain.DataGroup::getParentTaskId)
.filter(id -> 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)).withSelfRel());
.getData(taskId, null, null)).withSelfRel());
}
}
2 changes: 1 addition & 1 deletion ...ava/com/netgrif/application/engine/workflow/web/responsebodies/LocalisedTaskResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ private void buildLinks() {
add(WebMvcLinkBuilder.linkTo(WebMvcLinkBuilder.methodOn(TaskController.class)
.cancel((Authentication) null, task.getStringId(), null)).withRel("cancel"));
add(WebMvcLinkBuilder.linkTo(WebMvcLinkBuilder.methodOn(TaskController.class)
.getData(task.getStringId(), null)).withRel("data"));
.getData(task.getStringId(), null, null)).withRel("data"));
add(WebMvcLinkBuilder.linkTo(WebMvcLinkBuilder.methodOn(TaskController.class)
.setData(task.getStringId(), null, null)).withRel("data-edit"));
try {
Expand Down
49 changes: 49 additions & 0 deletions src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,4 +479,53 @@ 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
dataService.setData(taskId, ImportHelper.populateDataset([
"view_pos_ul": [
"value": [testUser.stringId],
"type": "userList"
]
] as Map)).getCase()

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
dataService.setData(taskId, ImportHelper.populateDataset([
"view_neg_ul": [
"value": [testUser.stringId],
"type": "userList"
]
] as Map)).getCase()

assert !taskAuthorizationService.canCallGetData(testUser.transformToLoggedUser(), taskId)
workflowService.deleteCase(case_.stringId)
}
}
40 changes: 40 additions & 0 deletions src/test/resources/task_authorization_service_test_with_userRefs.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
<id>wst_usersRef</id>
<initials>WSU</initials>
<title>WorkflowAuthorizationService test</title>
<role>
<id>view_pos_role</id>
<name>view pos role</name>
</role>
<role>
<id>assign_pos_role</id>
<name>assign pos role</name>
Expand All @@ -12,6 +16,10 @@
<id>finish_pos_role</id>
<name>finish pos role</name>
</role>
<role>
<id>view_neg_role</id>
<name>view neg role</name>
</role>
<role>
<id>assign_neg_role</id>
<name>assign neg role</name>
Expand All @@ -20,6 +28,10 @@
<id>finish_neg_role</id>
<name>finish neg role</name>
</role>
<data type="userList">
<id>view_pos_ul</id>
<title/>
</data>
<data type="userList">
<id>assign_pos_ul</id>
<title/>
Expand All @@ -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/>
Expand All @@ -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>
Expand All @@ -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>
Expand Down
Loading