[NAE-2387] Fix permission evaluation order between ActorRef and proce…

[renczesstefan]

Description

Fix permission evaluation order between ActorRef and processRole with implementing check for undefined permissions on both role refs and actor refs.

Fixes NAE-2387

Dependencies

No new dependencies were introduced

Third party dependencies

No new dependencies were introduced

Blocking Pull requests

There are no dependencies on other PR

How Has Been This Tested?

This was tested manually and with unit tests.

Test Configuration

Name	Tested on
OS	macOS Tahoe 26.3
Runtime	Java 21
Dependency Manager	Maven 3.9.9n
Framework version	Spring Boot 3.4.4
Run parameters
Other configuration

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• My changes have been checked, personally or remotely, with @machacjozef @Retoocs @tuplle
• I have commented my code, particularly in hard-to-understand areas
• I have resolved all conflicts with the target branch of the PR
• I have updated and synced my code with the target branch
• I have added tests that prove my fix is effective or that my feature works
• New and existing tests pass locally with my changes:
  • Lint test
  • Unit tests
  • Integration tests
• I have checked my contribution with code analysis tools:
  • SonarCloud
  • Snyk
• I have made corresponding changes to the documentation:
  • Developer documentation
  • User Guides
  • Migration Guides

Summary by CodeRabbit

• Tests

  • Added tests expanding authorization coverage for task assignment and workflow deletion, validating role-based and actor-list/user-list permission combinations.

• Refactor

  • Consolidated permission-evaluation logic across authorization services to centralize and standardize how role and user-list permissions are checked.

[coderabbitai]

Warning

Rate limit exceeded

@renczesstefan has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minutes and 2 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 68189658-7e09-4a52-bc0e-ffc83cb146ea

📥 Commits

Reviewing files that changed from the base of the PR and between 9874fd1 and 531a2ac.

📒 Files selected for processing (2)

• application-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovy
• application-engine/src/test/resources/task_authorization_service_test_with_userRefs.xml

Walkthrough

Introduces a centralized protected method checkPermissions(Map<String, Boolean>, List<String>) in AbstractAuthorizationService and updates TaskAuthorizationService and WorkflowAuthorizationService to use it. Adds four new tests and corresponding XML test resources to cover role and actor-list permission scenarios.

Changes

Cohort / File(s)	Summary
Authorization Service Refactoring application-engine/src/main/java/com/netgrif/application/engine/workflow/service/AbstractAuthorizationService.java, application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskAuthorizationService.java, application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowAuthorizationService.java	Added protected checkPermissions(Map<String, Boolean>, List<String>) to AbstractAuthorizationService; TaskAuthorizationService and WorkflowAuthorizationService now delegate permission evaluation to this method. Imports in AbstractAuthorizationService switched to java.util.*.
Task authorization tests & resources application-engine/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy, application-engine/src/test/resources/task_authorization_service_test_with_userRefs.xml	Added two test methods to TaskAuthorizationServiceTest covering assign permission with role/actorRef combinations; extended XML with view_pos_role role and view_pos_ul actorList.
Workflow authorization tests & resources application-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovy, application-engine/src/test/resources/workflow_authorization_service_test_with_userRefs.xml	Added two test methods to WorkflowAuthorizationServiceTest covering delete permission with role/actorRef combinations; extended XML with view_pos_role roleRef and view_actor_list actorList entries.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

refactor, test

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title references NAE-2387 and indicates a fix for permission evaluation order between ActorRef and processRole, which aligns with the core changes (centralized checkPermissions method and its usage across authorization services).

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowAuthorizationService.java (1)

53-53: ⚠️ Potential issue | 🟠 Major

Potential NPE: userHasAtLeastOneRolePermission can return null.

The method canCallCreate returns a primitive boolean, but userHasAtLeastOneRolePermission returns Boolean which can be null (when the user has no matching roles in the permissions map). This will cause a NullPointerException during unboxing.

🐛 Proposed fix to handle null return

     `@Override`
     public boolean canCallCreate(LoggedUser user, String netId) {
         // TODO: impersonation
         if (user.isAdmin()) {
             return true;
         }

         PetriNet net = petriNetService.getPetriNet(netId);
         // TODO: impersonation
-        return userHasAtLeastOneRolePermission(user, net, ProcessRolePermission.CREATE);
+        Boolean rolePerm = userHasAtLeastOneRolePermission(user, net, ProcessRolePermission.CREATE);
+        return rolePerm != null && rolePerm;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowAuthorizationService.java`
at line 53, canCallCreate currently returns the primitive boolean but delegates
to userHasAtLeastOneRolePermission which returns a Boolean that may be null;
change canCallCreate to guard against null by returning
Boolean.TRUE.equals(userHasAtLeastOneRolePermission(user, net,
ProcessRolePermission.CREATE)) (or use Optional.ofNullable(...).orElse(false))
so a null result is treated as false and no NPE occurs.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@application-engine/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy`:
- Around line 593-633: Both new tests
(testCanAssignWithRoleAssignTrueAndWithActorRefAssignUndefined and
testCanAssignWithRoleAssignUndefinedAndWithActorRefAssignTrue) are missing the
explicit cleanup used elsewhere; after asserting canCallAssign, call
userService.removeRole(testUser, positiveRole.get_id()) and
workflowService.deleteCase(case_.get_id()) (or the equivalent deleteCase
invocation used in this test class) to remove the added role and case to match
the pattern in testCanAssign/testCanFinish and keep tests consistent.

---

Outside diff comments:
In
`@application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowAuthorizationService.java`:
- Line 53: canCallCreate currently returns the primitive boolean but delegates
to userHasAtLeastOneRolePermission which returns a Boolean that may be null;
change canCallCreate to guard against null by returning
Boolean.TRUE.equals(userHasAtLeastOneRolePermission(user, net,
ProcessRolePermission.CREATE)) (or use Optional.ofNullable(...).orElse(false))
so a null result is treated as false and no NPE occurs.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c830f451-05d6-4b3b-a962-2d80a0330b9b

📥 Commits

Reviewing files that changed from the base of the PR and between 92f4890 and 6cf550e.

📒 Files selected for processing (5)

• application-engine/src/main/java/com/netgrif/application/engine/workflow/service/AbstractAuthorizationService.java
• application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskAuthorizationService.java
• application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowAuthorizationService.java
• application-engine/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy
• application-engine/src/test/resources/task_authorization_service_test_with_userRefs.xml

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@application-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovy`:
- Around line 334-352: The variable positiveDeleteRole in
testCanCallDeleteWithRoleDeleteUndefinedAndActorRefDeleteTrue is misleading
because it actually loads the "view_pos_role"; rename it to a clearer identifier
(e.g., viewPosRole or roleWithViewPermission) and update its declaration and
subsequent use in userService.addRole(...) so the variable name reflects the
role semantics returned by netWithUserRefs.getRoles().values().find(...
"view_pos_role").

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f3b3f441-4ff0-4ce5-9687-aa486d7ebc05

📥 Commits

Reviewing files that changed from the base of the PR and between 6cf550e and 9874fd1.

📒 Files selected for processing (2)

• application-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovy
• application-engine/src/test/resources/workflow_authorization_service_test_with_userRefs.xml