Skip to content
This repository was archived by the owner on May 29, 2026. It is now read-only.

fix: ZgwController base class + AC scope-gate + consumerHasScope fail-closed (C2/C3/H2/M3)#660

Merged
rubenvdlinde merged 1 commit into
developmentfrom
fix/critical-zgw-auth-scope-gate
May 27, 2026
Merged

fix: ZgwController base class + AC scope-gate + consumerHasScope fail-closed (C2/C3/H2/M3)#660
rubenvdlinde merged 1 commit into
developmentfrom
fix/critical-zgw-auth-scope-gate

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Summary

Fixes four security findings from the deep review pass.

C3 — ZgwAuthMiddleware dead (abstract base class missing):
ZgwAuthMiddleware::beforeController used instanceof ZgwController but ZgwController never existed — the middleware always returned immediately without enforcing any scope check. Creates lib/Controller/ZgwController.php as an abstract marker base and makes all six ZGW controllers (AcController, ZrcController, ZtcController, DrcController, BrcController, NrcController) extend it.

C2 — AC controller no scope-gating on writes:
Any valid JWT could POST /api/ac/applicaties with heeftAlleAutorisaties: true to mint a superuser consumer. Write operations (create, update, destroy) now require ac.aanmaken / ac.bijwerken / ac.verwijderen scope respectively. Setting heeftAlleAutorisaties=true additionally requires ac.superuser scope (only granted to existing superusers).

H2 — consumerHasScope / getConsumerAuthorisaties fail-permissive:
Multiple non-exception paths returned true (scope granted) or null (unrestricted) — null ConsumerMapper, malformed JWT, missing client_id/iss, empty consumer set. All now return false / [] (deny/restricted).

M3 — validateJwtAuth leaks internal JWT error messages:
Both ZgwService::validateJwtAuth and ZgwAuthMiddleware::beforeController echoed raw $e->getMessage() in 403 responses, exposing algorithm/issuer enumeration signals. Now logs detail server-side and returns only a generic message to the caller.

Test plan

  • 10 PHPUnit tests pass (ZgwAuthMiddlewareTest: 8 existing + 2 new C3 coverage tests)
  • testZgwControllerIsAbstract — verifies the marker class is abstract
  • testZgwControllersExtendBase — verifies all 6 ZGW controllers extend ZgwController
  • Existing confidentiality-level tests unchanged

Closes #635, #636, #644, #652

…-closed (C2/C3/H2/M3)

- Create abstract ZgwController so ZgwAuthMiddleware instanceof guard actually fires (C3)
- Make AcController, ZrcController, ZtcController, DrcController, BrcController, NrcController
  extend ZgwController so all ZGW endpoints are covered by the middleware
- Gate AC write ops (create/update/destroy) on ac.aanmaken/bijwerken/verwijderen scopes (C2)
- Block heeftAlleAutorisaties=true unless requesting consumer is a superuser (C2)
- Fail consumerHasScope/getConsumerAuthorisaties closed on null mapper, malformed JWT,
  missing client_id, and empty consumer — all previously returned permissive true/null (H2)
- Remove internal JWT exception detail from HTTP response; log server-side only (M3)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant