Skip to content

[NAE 2424] UserRefs negative view permissions aren't resolved #442

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

Open
SamuelPalaj wants to merge 6 commits into
base: release/6.5.0
Choose a base branch
from
Open

[NAE 2424] UserRefs negative view permissions aren't resolved #442

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
6361d3b
[NAE-2424] UserRefs negative view permissions aren't resolved
SamuelPalaj May 13, 2026
f094011
[NAE-2424]
mazarijuraj May 13, 2026
05593d0
[NAE-2424] UserRefs negative view permissions aren't resolved
SamuelPalaj May 25, 2026
b22a27d
Merge branch 'release/6.5.0' into NAE-2424
SamuelPalaj May 25, 2026
cb6f7b8
[NAE-2424] UserRefs negative view permissions aren't resolved
SamuelPalaj May 25, 2026
87794e5
[NAE-2424] UserRefs negative view permissions aren't resolved
SamuelPalaj May 25, 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
12 changes: 0 additions & 12 deletions src/main/java/com/netgrif/application/engine/elastic/service/ElasticTaskService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ protected BoolQueryBuilder buildSingleQuery(ElasticTaskSearchRequest request, Lo
if (request == null) {
throw new IllegalArgumentException("Request can not be null!");
}
addRolesQueryConstraint(request, user);

BoolQueryBuilder query = boolQuery();
buildViewPermissionQuery(query, user);
Expand All @@ -198,17 +197,6 @@ protected BoolQueryBuilder buildSingleQuery(ElasticTaskSearchRequest request, Lo
return query;
}

protected void addRolesQueryConstraint(ElasticTaskSearchRequest request, LoggedUser user) {
if (request.role != null && !request.role.isEmpty()) {
Set<String> roles = new HashSet<>(request.role);
roles.addAll(user.getProcessRoles());
request.role = new ArrayList<>(roles);
} else {
request.role = new ArrayList<>(user.getProcessRoles());
}
}


/**
* Tasks of case with id "5cb07b6ff05be15f0b972c4d"
* {
Expand Down
83 changes: 27 additions & 56 deletions ...ain/java/com/netgrif/application/engine/elastic/service/ElasticViewPermissionService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,93 +8,64 @@
public abstract class ElasticViewPermissionService {

protected void buildViewPermissionQuery(BoolQueryBuilder query, LoggedUser user) {
// Check if viewRoles or viewUserRefs exist
BoolQueryBuilder viewPermsExists = boolQuery()
.should(existsQuery("viewRoles"))
.should(existsQuery("viewUserRefs"));
// Condition where these attributes do NOT exist
BoolQueryBuilder viewPermNotExists = boolQuery()
.mustNot(viewPermsExists);

/* Build positive view role query */
BoolQueryBuilder positiveViewRole = buildPositiveViewRoleQuery(viewPermNotExists, user);
// (Rp!=0 & Rn = 0)
BoolQueryBuilder roleViewQuery = boolQuery()
.filter(buildPositiveViewRoleQuery(user))
.mustNot(buildNegativeViewRoleQuery(user));

/* Build negative view role query */
BoolQueryBuilder negativeViewRole = buildNegativeViewRoleQuery(user);

/* Positive view role set-minus negative view role */
BoolQueryBuilder positiveRoleSetMinusNegativeRole = setMinus(positiveViewRole, negativeViewRole);

/* Build positive view userList query */
BoolQueryBuilder positiveViewUser = buildPositiveViewUser(viewPermNotExists, user);

/* Role query union positive view userList */
BoolQueryBuilder roleSetMinusPositiveUserList = union(positiveRoleSetMinusNegativeRole, positiveViewUser);

/* Build negative view userList query */
BoolQueryBuilder negativeViewUser = buildNegativeViewUser(user);

/* Role-UserListPositive set-minus negative view userList */
BoolQueryBuilder permissionQuery = setMinus(roleSetMinusPositiveUserList, negativeViewUser);
// ((Rp!=0 & Rn = 0) or Up!=0)
BoolQueryBuilder roleOrPositiveUserQuery = boolQuery().should(roleViewQuery)
.should(buildPositiveViewUser(user))
.minimumShouldMatch(1);

query.filter(permissionQuery);
// (((Rp!=0 & Rn = 0) or Up!=0) & Un=0) == 1
query.filter(roleOrPositiveUserQuery)
.mustNot(buildNegativeViewUser(user));
Comment on lines +18 to +24
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="src/main/java/com/netgrif/application/engine/elastic/service/ElasticViewPermissionService.java"

# Basic sanity: show file with line numbers (but avoid huge output)
echo "== File: $FILE =="
wc -l "$FILE"

# Show the relevant line ranges mentioned in the review comment
echo
echo "== Lines 1-120 =="
sed -n '1,120p' "$FILE" | nl -ba | sed -n '1,120p'

# Show the specific ranges more precisely
echo
echo "== Lines 1-80 (detailed) =="
sed -n '1,80p' "$FILE" | nl -ba

echo
echo "== Lines 80-200 (possible helpers) =="
sed -n '80,200p' "$FILE" | nl -ba

echo
echo "== Remaining lines (if any) =="
sed -n '200,500p' "$FILE" | nl -ba

# Find definitions of helper methods used in the snippet
echo
echo "== Search for builder methods =="
rg -n "buildPositiveViewUser|buildNegativeViewUser|buildViewPermissionQuery|roleViewQuery|viewRoles|viewUsers|not exists|missing" "$FILE" || true

# Show call sites to see whether any migration/backfill logic exists nearby
echo
echo "== Call sites for buildViewPermissionQuery usage =="
rg -n "buildViewPermissionQuery" src/main/java/com/netgrif/application/engine/elastic/service | head -n 50

Repository: netgrif/application-engine

Length of output: 356

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="src/main/java/com/netgrif/application/engine/elastic/service/ElasticViewPermissionService.java"

echo "== File: $FILE =="
wc -l "$FILE"

echo
echo "== Lines 1-120 =="
awk 'NR>=1 && NR<=120 {printf "%4d\t%s\n", NR, $0}' "$FILE"

echo
echo "== Grep helper/query components =="
rg -n "buildPositiveViewUser|buildNegativeViewUser|buildViewPermissionQuery|roleViewQuery|viewRoles|viewUsers|mustNot|minimumShouldMatch|not exists|missing" "$FILE" || true

echo
echo "== Find usages of buildViewPermissionQuery =="
rg -n "buildViewPermissionQuery" src/main/java/com/netgrif/application/engine/elastic/service

echo
echo "== Locate positive/negative helper method implementations =="
rg -n "buildPositiveViewUser|buildNegativeViewUser" "$FILE"

Repository: netgrif/application-engine

Length of output: 5084

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for any legacy "no permission fields" / not-exists logic references across Java sources
rg -n --hidden --glob '!**/target/**' \
  "viewPermNotExists|view perm(ission)? not exists|permission not exists|not exists|missing.*view|viewRoles.*missing|viewUsers.*missing" \
  src/main/java || true

# Search for query parts touching those fields
rg -n --hidden --glob '!**/target/**' \
  "viewRoles|viewUsers|negativeViewRoles|negativeViewUsers" \
  src/main/java/com/netgrif/application/engine/elastic/service || true

# Specifically check other permission-related services for compatibility paths
rg -n --hidden --glob '!**/target/**' \
  "ElasticViewPermissionService|buildViewPermissionQuery|buildPositiveViewRoleQuery|buildPositiveViewUser" \
  src/main/java || true

Repository: netgrif/application-engine

Length of output: 3064

🏁 Script executed:

#!/bin/bash
set -euo pipefail

TASK_FILE="src/main/java/com/netgrif/application/engine/elastic/service/ElasticTaskService.java"
CASE_FILE="src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java"

echo "== ElasticTaskService: show around buildViewPermissionQuery usage (line ~183) =="
awk 'NR>=150 && NR<=230 {printf "%4d\t%s\n", NR, $0}' "$TASK_FILE" | sed -n '1,120p'

echo
echo "== ElasticCaseService: show around buildViewPermissionQuery usage (line ~205) =="
awk 'NR>=170 && NR<=260 {printf "%4d\t%s\n", NR, $0}' "$CASE_FILE" | sed -n '1,120p'

echo
echo "== Check for other view permission logic in those services =="
rg -n "buildViewPermissionQuery|viewRoles|viewUsers|negativeViewRoles|negativeViewUsers|mustNot\\(buildNegativeView|minimumShouldMatch" "$TASK_FILE" "$CASE_FILE" || true

Repository: netgrif/application-engine

Length of output: 7725

Elastic view permission query now excludes documents without explicit viewRoles/viewUsers matches

In ElasticViewPermissionService.buildViewPermissionQuery (lines 18-24, helpers 31-59), roleOrPositiveUserQuery uses minimumShouldMatch(1) across roleViewQuery (positive viewRoles) and buildPositiveViewUser (positive viewUsers), with no “field missing => allowed” branch. As a result, documents that omit viewRoles and viewUsers will not match, impacting both task and case searches since both ElasticTaskService and ElasticCaseService call this shared helper.

Restore the intended legacy “no permission fields” behavior or document and implement a migration/reindex/backfill that populates explicit positive defaults for existing documents.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/main/java/com/netgrif/application/engine/elastic/service/ElasticViewPermissionService.java`
around lines 18 - 24, The current buildViewPermissionQuery uses a
BoolQueryBuilder roleOrPositiveUserQuery that requires either roleViewQuery or
buildPositiveViewUser to match, which excludes documents that lack both
viewRoles and viewUsers; change roleOrPositiveUserQuery to also allow documents
where both permission fields are missing by adding an additional should clause
that matches "viewRoles not exists AND viewUsers not exists" (use exists queries
for viewRoles/viewUsers and combine them with mustNot as a should option) so
minimumShouldMatch(1) still applies, and leave the existing
mustNot(buildNegativeViewUser(user)) in place; update
ElasticViewPermissionService.buildViewPermissionQuery (and any helper usage of
roleViewQuery, buildPositiveViewUser, buildNegativeViewUser) to implement this
or alternatively document and implement a migration/backfill that populates
explicit positive defaults for existing documents.

}

/**
* Build a positive view role query using termsQuery for efficiency.
* This reduces the number of clauses by sending all roles at once.
*/
private BoolQueryBuilder buildPositiveViewRoleQuery(BoolQueryBuilder viewPermNotExists, LoggedUser user) {
private BoolQueryBuilder buildPositiveViewRoleQuery(LoggedUser user) {
BoolQueryBuilder positiveViewRole = boolQuery();
if (!user.getProcessRoles().isEmpty()) {
positiveViewRole.should(termsQuery("viewRoles", user.getProcessRoles()));
}
positiveViewRole.should(viewPermNotExists);
positiveViewRole.minimumShouldMatch(1);
return positiveViewRole;
}

/**
* Build a negative view role query by excluding negative roles.
* Build a negative view role query.
*/
private BoolQueryBuilder buildNegativeViewRoleQuery(LoggedUser user) {
BoolQueryBuilder negativeViewRole = boolQuery();
if (!user.getProcessRoles().isEmpty()) {
negativeViewRole.mustNot(termsQuery("negativeViewRoles", user.getProcessRoles()));
negativeViewRole.should(termsQuery("negativeViewRoles", user.getProcessRoles()));
}
negativeViewRole.minimumShouldMatch(1);
return negativeViewRole;
}

/**
* Build a positive view user query using filter (as score is not needed).
* Build a positive view user query.
*/
private BoolQueryBuilder buildPositiveViewUser(BoolQueryBuilder viewPermNotExists, LoggedUser user) {
return boolQuery()
.should(viewPermNotExists)
.filter(termQuery("viewUsers", user.getId()));
private BoolQueryBuilder buildPositiveViewUser(LoggedUser user) {
BoolQueryBuilder positiveViewUser = boolQuery();
positiveViewUser.should(termQuery("viewUsers", user.getId()));
positiveViewUser.minimumShouldMatch(1);
return positiveViewUser;
}

/**
* Build a negative view user query to exclude the specified user.
* Build a negative view user query.
*/
private BoolQueryBuilder buildNegativeViewUser(LoggedUser user) {
return boolQuery()
.mustNot(termQuery("negativeViewUsers", user.getId()));
}

private BoolQueryBuilder setMinus(BoolQueryBuilder positiveSet, BoolQueryBuilder negativeSet) {
BoolQueryBuilder positiveSetMinusNegativeSet = boolQuery();
positiveSetMinusNegativeSet.must(positiveSet);
positiveSetMinusNegativeSet.must(negativeSet);
return positiveSetMinusNegativeSet;
}

/**
* Unions two queries using OR with a minimum_should_match of 1.
*/
private BoolQueryBuilder union(BoolQueryBuilder setA, BoolQueryBuilder setB) {
return boolQuery()
.should(setA)
.should(setB)
.minimumShouldMatch(1);
BoolQueryBuilder negativeViewUser = boolQuery();
negativeViewUser.should(termQuery("negativeViewUsers", user.getId()));
negativeViewUser.minimumShouldMatch(1);
return negativeViewUser;
}
}
22 changes: 5 additions & 17 deletions src/main/java/com/netgrif/application/engine/importer/service/Importer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1156,32 +1156,20 @@ protected void addPredefinedRolesWithDefaultPermissions(com.netgrif.application.
return;
}
}
// Don't add if positive roles or triggers or positive user refs
if ((importTransition.getRoleRef() != null && importTransition.getRoleRef().stream().anyMatch(this::hasPositivePermission))
|| (importTransition.getTrigger() != null && !importTransition.getTrigger().isEmpty())
|| (importTransition.getUsersRef() != null && importTransition.getUsersRef().stream().anyMatch(this::hasPositivePermission))
|| (importTransition.getUserRef() != null && importTransition.getUserRef().stream().anyMatch(this::hasPositivePermission))) {

if ((importTransition.getRoleRef() != null && !importTransition.getRoleRef().isEmpty()) ||
(importTransition.getUsersRef() != null && !importTransition.getUsersRef().isEmpty()) ||
(importTransition.getUserRef() != null && !importTransition.getUserRef().isEmpty())) {
return;
}

addDefaultRole(transition);
addAnonymousRole(transition);
}

protected boolean hasPositivePermission(PermissionRef permissionRef) {
return (permissionRef.getLogic().isPerform() != null && permissionRef.getLogic().isPerform())
|| (permissionRef.getLogic().isCancel() != null && permissionRef.getLogic().isCancel())
|| (permissionRef.getLogic().isView() != null && permissionRef.getLogic().isView())
|| (permissionRef.getLogic().isAssign() != null && permissionRef.getLogic().isAssign())
|| (permissionRef.getLogic().isAssigned() != null && permissionRef.getLogic().isAssigned())
|| (permissionRef.getLogic().isFinish() != null && permissionRef.getLogic().isFinish())
|| (permissionRef.getLogic().isDelegate() != null && permissionRef.getLogic().isDelegate());
}

protected void addPredefinedRolesWithDefaultPermissions() {
// only if no positive role associations and no positive user ref associations
if (net.getPermissions().values().stream().anyMatch(perms -> perms.containsValue(true))
|| net.getUserRefs().values().stream().anyMatch(perms -> perms.containsValue(true))) {
if (!net.getPermissions().isEmpty() || !net.getUserRefs().isEmpty()) {
return;
}

Expand Down
9 changes: 8 additions & 1 deletion src/main/java/com/netgrif/application/engine/workflow/domain/Case.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,9 +354,16 @@ public void resolveViewUserRefs() {
public void resolveViewUsers() {
getViewUsers();
this.viewUsers.clear();
this.negativeViewUsers.clear();
this.users.forEach((user, perms) -> {
if (perms.containsKey(RolePermission.VIEW.getValue()) && perms.get(RolePermission.VIEW.getValue())) {
if (!perms.containsKey(RolePermission.VIEW.getValue())) {
return;
}
boolean viewPermission = perms.get(RolePermission.VIEW.getValue());
if(viewPermission){
viewUsers.add(user);
} else {
negativeViewUsers.add(user);
}
});
}
Expand Down
9 changes: 8 additions & 1 deletion src/main/java/com/netgrif/application/engine/workflow/domain/Task.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,9 +329,16 @@ public void resolveViewUserRefs() {
public void resolveViewUsers() {
getViewUsers();
this.viewUsers.clear();
this.negativeViewUsers.clear();
this.users.forEach((role, perms) -> {
if (perms.containsKey(RolePermission.VIEW.getValue()) && perms.get(RolePermission.VIEW.getValue())) {
if (!perms.containsKey(RolePermission.VIEW.getValue())) {
return;
}
boolean viewPermission = perms.get(RolePermission.VIEW.getValue());
if (viewPermission) {
viewUsers.add(role);
} else {
negativeViewUsers.add(role);
}
});
}
Expand Down
25 changes: 13 additions & 12 deletions src/main/java/com/netgrif/application/engine/workflow/service/TaskSearchService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,17 @@ public Predicate buildQuery(List<TaskSearchRequest> requests, LoggedUser user, L

BooleanBuilder builder = constructPredicateTree(singleQueries, isIntersection ? BooleanBuilder::and : BooleanBuilder::or);

BooleanBuilder constraints = new BooleanBuilder(buildRolesQueryConstraint(loggedOrImpersonated));
constraints.or(buildUserRefQueryConstraint(loggedOrImpersonated));
builder.and(constraints);

BooleanBuilder permissionConstraints = new BooleanBuilder(buildViewRoleQueryConstraint(loggedOrImpersonated));
permissionConstraints.andNot(buildNegativeViewRoleQueryConstraint(loggedOrImpersonated));
permissionConstraints.or(buildViewUserQueryConstraint(loggedOrImpersonated));
permissionConstraints.andNot(buildNegativeViewUsersQueryConstraint(loggedOrImpersonated));
builder.and(permissionConstraints);
return builder;
// (Rp!=0 & Rn = 0)
BooleanBuilder constraints = new BooleanBuilder(buildViewRoleQueryConstraint(loggedOrImpersonated))
.andNot(buildNegativeViewRoleQueryConstraint(loggedOrImpersonated));

// ((Rp!=0 & Rn = 0) or Up!=0)
constraints.or(buildViewUserQueryConstraint(loggedOrImpersonated));

// (((Rp!=0 & Rn = 0) or Up!=0) & Un=0) == 1
constraints.andNot(buildNegativeViewUsersQueryConstraint(loggedOrImpersonated));

return builder.and(constraints);
}

protected Predicate buildRolesQueryConstraint(LoggedUser user) {
Expand All @@ -69,7 +70,7 @@ protected Predicate buildViewRoleQueryConstraint(LoggedUser user) {
}

public Predicate viewRoleQuery(String role) {
return QTask.task.viewUserRefs.isEmpty().and(QTask.task.viewRoles.isEmpty()).or(QTask.task.viewRoles.contains(role));
return QTask.task.viewRoles.contains(role);
}

protected Predicate buildViewUserQueryConstraint(LoggedUser user) {
Expand All @@ -78,7 +79,7 @@ protected Predicate buildViewUserQueryConstraint(LoggedUser user) {
}

public Predicate viewUsersQuery(String userId) {
return QTask.task.negativeViewRoles.isEmpty().and(QTask.task.viewUserRefs.isEmpty()).and(QTask.task.viewRoles.isEmpty()).or(QTask.task.viewUsers.contains(userId));
return QTask.task.viewUsers.contains(userId);
}

protected Predicate buildNegativeViewRoleQueryConstraint(LoggedUser user) {
Expand Down
5 changes: 1 addition & 4 deletions src/main/java/com/netgrif/application/engine/workflow/service/TaskService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -780,12 +780,9 @@ public List<Task> resolveUserRef(Case useCase) {
@Override
public Task resolveUserRef(Task task, Case useCase) {
task.getUsers().clear();
task.getNegativeViewUsers().clear();
task.getUserRefs().forEach((id, permission) -> {
List<String> userIds = getExistingUsers((UserListFieldValue) useCase.getDataSet().get(id).getValue());
if (userIds != null && !userIds.isEmpty() && permission.containsKey("view") && !permission.get("view")) {
task.getNegativeViewUsers().addAll(userIds);
} else if (userIds != null && !userIds.isEmpty()) {
if (userIds != null && !userIds.isEmpty()) {
task.addUsers(new HashSet<>(userIds), permission);
}
});
Expand Down
7 changes: 1 addition & 6 deletions src/main/java/com/netgrif/application/engine/workflow/service/WorkflowService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@ public long count(Map<String, Object> request, LoggedUser user, Locale locale) {
@Override
public Case resolveUserRef(Case useCase) {
useCase.getUsers().clear();
useCase.getNegativeViewUsers().clear();
useCase.getUserRefs().forEach((id, permission) -> {
resolveUserRefPermissions(useCase, id, permission);
});
Expand All @@ -230,11 +229,7 @@ public Case resolveUserRef(Case useCase) {
private void resolveUserRefPermissions(Case useCase, String userListId, Map<String, Boolean> permission) {
List<String> userIds = getExistingUsers((UserListFieldValue) useCase.getDataSet().get(userListId).getValue());
if (userIds != null && !userIds.isEmpty()) {
if (permission.containsKey("view") && !permission.get("view")) {
useCase.getNegativeViewUsers().addAll(userIds);
} else {
useCase.addUsers(new HashSet<>(userIds), permission);
}
useCase.addUsers(new HashSet<>(userIds), permission);
}
}

Expand Down
Loading
Loading