Skip to content

FOUR-24957: Collection Record Control does not work to users different to super admin #8367

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.

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
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

eiresendez
Copy link
Contributor

@eiresendez eiresendez commented Jul 11, 2025

Issue & Reproduction Steps

This PR proposes the removal of a screen access check within the viewScreen policy for tasks.

Currently, the policy performs a validation using:

in_array($screen->id, $task->getScreenAndNestedIds())

This check fails to account for screens that are part of FormCollectionRecordControl components. As a result, users with valid task access cannot load certain screens that are dynamically embedded in the process task.

To reproduce:

  1. Create a process with a task that references a screen with a FormCollectionRecordControl.
  2. Attempt to fetch a the screen via GET /api/1.0/tasks/{task}/screens/{screen}.
  3. Receive a 403 error despite the user being correctly assigned to the task and with updateTask permission.

Solution

  • Removed the validation from the viewScreen policy, which was introduced 5 years ago and is likely no longer sufficient or necessary in the current architecture.
  • Now the policy simply checks whether the user has update permissions on the task.

⚠️ Notes & Alternative Solutions

This PR is a proposal for simplification. If there are concerns about removing the screen ID validation entirely, alternative approaches include:

  • Extending backend context: making the task aware of related screens in collection components
  • Discussing with Product for a more advanced policy (e.g., based on “Collection Permissions”)

While alternatives could work, they would likely add complexity and go beyond the scope of what this PR aims to solve.

How to Test

  1. Run ./vendor/bin/phpunit --filter=ProcessRequestTokenPolicyTest.
  2. It should pass.
  3. Follow the reproduction steps for a manual review.

Related Tickets & Packages

Code Review Checklist

  • I have pulled this code locally and tested it on my instance, along with any associated packages.
  • This code adheres to ProcessMaker Coding Guidelines.
  • This code includes a unit test or an E2E test that tests its functionality, or is covered by an existing test.
  • This solution fixes the bug reported in the original ticket.
  • This solution does not alter the expected output of a component in a way that would break existing Processes.
  • This solution does not implement any breaking changes that would invalidate documentation or cause existing Processes to fail.
  • This solution has been tested with enterprise packages that rely on its functionality and does not introduce bugs in those packages.
  • This code does not duplicate functionality that already exists in the framework or in ProcessMaker.
  • This ticket conforms to the PRD associated with this part of ProcessMaker.

- Removed validation, which blocked access to valid nested and collection screens.
- Simplified viewScreen policy to rely solely on task update permissions.
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants