Skip to content

FOUR-34196 Review the issue Unauthorized - #9117

Open
devmiguelangel wants to merge 5 commits into
developfrom
FOUR-34196
Open

devmiguelangel wants to merge 5 commits into
developfrom
FOUR-34196

Conversation

@devmiguelangel

@devmiguelangel devmiguelangel commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Issue & Reproduction Steps

Review the issue Unauthorized

The performance2 environment generated a high volume of OAuth authorization errors when requests contained malformed bearer values such as Bearer undefined, Bearer null, or placeholder JWTs.

EnsureAccountAllowsAccess was registered in the web middleware group but unconditionally evaluated both the web and api guards. As a result, Passport parsed and reported malformed bearer tokens even for requests authenticated through a web session, producing large stack traces and unnecessary log/disk overhead.

To reproduce before this change:

  1. Authenticate through the web guard.
  2. Send a web request with Authorization: Bearer undefined.
  3. Observe Passport resolving the API guard and reporting an OAuth/JWT parsing exception.

Solution

  • Scope EnsureAccountAllowsAccess to explicitly supplied guards.
  • Check only the web guard when the middleware runs in the web group.
  • Pass the authenticated guard list from ProcessMakerAuthenticate, so auth:api checks only api.
  • Add regression coverage for active, blocked, and inactive users across web and API guard contexts.
  • Harden the k6 ETag tests so they require PM_K6_BASE_URL and a valid PM_K6_TOKEN instead of sending nullish or placeholder bearer values.
  • Document k6 ETag test configuration.

This change preserves normal 401 behavior for malformed credentials sent directly to authenticated API routes; it prevents unrelated API guard evaluation on web routes.

How to Test

Run:

./vendor/bin/phpunit \
  tests/Feature/Http/Middleware/EnsureAccountAllowsAccessTest.php \
  tests/Feature/AuthTest.php \
  tests/unit/ProcessMaker/Auth/PassportTokenGuardFactoryTest.php

Result: 13 tests, 58 assertions, all passing.

Related Tickets & Packages

FOUR-34196

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.

@cursor

cursor Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes authentication middleware guard selection and deny/logout paths, which can affect who gets blocked and how sessions are cleared, though behavior for intentional API auth failures is preserved.

Overview
Fixes spurious OAuth/JWT errors on web requests that carry junk Authorization headers (e.g. Bearer undefined) by stopping EnsureAccountAllowsAccess from always probing both web and api.

Guard scoping: blockingResponseForRequest now takes an explicit guard list (defaulting to the configured default guard). The web middleware group passes ['web'] only; ProcessMakerAuthenticate passes the same $guards used for auth:api, so blocked/inactive checks no longer touch Passport on unrelated routes.

Logout behavior: Session invalidation and logout run only when the resolved guard implements StatefulGuard, avoiding inappropriate session teardown on stateless API guards.

Tests & k6: New feature tests cover malformed bearer on web, denied statuses on web/API, and guard isolation via ProcessMakerAuthenticate. ETag k6 scripts require PM_K6_BASE_URL and a real PM_K6_TOKEN (with README), instead of defaulting to placeholder tokens that hit the API.

Reviewed by Cursor Bugbot for commit b9a7090. Bugbot is set up for automated code reviews on this repo. Configure here.

@devmiguelangel devmiguelangel changed the title FOUR-34196: Prevent malformed bearer token processing on web routes FOUR-34196 Review the issue Unauthorized Oct 29, 2026
@devmiguelangel devmiguelangel self-assigned this Oct 29, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 29ba1da. Configure here.

@decisions-sonarqube

Copy link
Copy Markdown

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant