Skip to content

re #96 migrate JWT adapter to lcobucci/jwt 5.x + fix content negotiation#109

Merged
tonydspaniard merged 2 commits into
masterfrom
feat/96-jwt-lcobucci5-migration
May 28, 2026
Merged

re #96 migrate JWT adapter to lcobucci/jwt 5.x + fix content negotiation#109
tonydspaniard merged 2 commits into
masterfrom
feat/96-jwt-lcobucci5-migration

Conversation

@tonydspaniard
Copy link
Copy Markdown
Member

@tonydspaniard tonydspaniard commented May 28, 2026

Summary

Part of the #96 burn-down. The baseline entries that looked like "optional-dep class-not-found" were actually real API-version-mismatch bugs: the Altair\Http\Jwt\* adapters targeted the lcobucci/jwt 3.x API while the framework requires ^5.3, so the JWT feature could not run against the installed library. Fixed properly (no ignoreErrors).

JWT — migrated to lcobucci/jwt 5.x

  • LcobucciTokenGenerator / LcobucciTokenParser now derive a fresh Lcobucci\JWT\Configuration from the framework TokenConfigurationInterface instead of injecting shared Builder/Parser primitives. (The v5 builder is immutable; the old shared-builder loop also silently discarded withClaim() results — a latent bug.)
  • Parser now performs full validation: signature (SignedWith, algorithm fixed to the configured signer → alg=none/algorithm-confusion rejected), issuer (IssuedBy), and time window (LooseValidAt). The old validateToken() was dead code, so token expiry was never enforced — that security gap is now closed and covered by tests.
  • Added PSR-20 Altair\Http\Jwt\SystemClock (injectable; frozen in tests for deterministic expiry) and declared psr/clock as a direct dependency.

Security hardening (from independent security review)

  • Reject the unconfigured TOKEN_PUBLIC_KEY placeholder at runtime (fail fast) instead of silently issuing unverifiable tokens.
  • Stop interpolating the raw token into parse-failure exception messages (log-injection / oracle risk).
  • Finding 4 (stable issuer): iss is now a configurable TOKEN_ISSUER (required, fail-fast) on TokenConfigurationInterface rather than the per-request URI — a token is no longer valid only at the exact endpoint that minted it. The generator/parser no longer depend on ServerRequest.
  • Finding 5 (audience): optional TOKEN_AUDIENCE — when set, the generator adds permittedFor() and the parser asserts PermittedFor; absent audience keeps the single-service default.

Content negotiation

  • FormatNegotiator: narrow Negotiator::getBest() (typed as the empty AcceptHeader marker interface) to BaseAccept before getValue(); and flatten mime priorities with array_merge(...) — the previous call_user_func('array_merge', …) passed an array-of-arrays as one argument, so header negotiation never worked.

PHPStan

  • Regenerated phpstan-baseline.neon: 14 Lcobucci/Negotiation findings drop out, none added. 65 → 51 errors.

Test plan

  • composer cs — clean
  • composer stan[OK] No errors
  • vendor/bin/rector process --dry-run (full codebase) — clean
  • composer test — JWT round-trip + tampered/foreign-key/expired/wrong-issuer/malformed rejection + audience accept/reject + Accept-header negotiation all pass; only pre-existing local MongoSessionHandlerTest env errors (CI loads ext-mongodb)

Antonio Ramirez added 2 commits May 28, 2026 07:39
…ation

The Altair\Http\Jwt adapters targeted the lcobucci/jwt 3.x API (removed
Jose\Parsing namespace, Constraint\ValidAt, Validation\InvalidTokenException,
new Key() on an interface, Lcobucci\Clock\SystemClock) while the framework
requires ^5.3 — so the code could not run against the installed library.

JWT (lcobucci 5.x):
- Generator and parser now derive a fresh Lcobucci\JWT\Configuration from the
  framework TokenConfigurationInterface instead of injecting shared Builder/Parser
  primitives (the builder is immutable in v5; the old shared-builder loop also
  silently discarded withClaim() results).
- Parser performs full validation: signature (SignedWith, algorithm fixed to the
  configured signer so alg=none / alg-confusion is rejected), issuer (IssuedBy),
  and time window (LooseValidAt). The old validateToken() was dead code, so expiry
  was never enforced — that gap is now closed.
- Add a PSR-20 Altair\Http\Jwt\SystemClock (injectable, frozen in tests) and
  declare psr/clock as a direct dependency.
- Security hardening: reject the unconfigured TOKEN_PUBLIC_KEY placeholder at
  runtime (fail fast) and stop interpolating the raw token into parse-failure
  exception messages (log-injection / oracle risk).

Content negotiation:
- FormatNegotiator: narrow Negotiator::getBest() (typed as the empty AcceptHeader
  marker interface) to BaseAccept before calling getValue(), and flatten the mime
  priorities with array_merge(...) — the previous call_user_func('array_merge', ...)
  passed an array-of-arrays as one argument, so header negotiation never worked.

Tests: round-trip, tampered-signature / foreign-key / expired / wrong-issuer /
malformed rejection for JWT; Accept-header negotiation for FormatNegotiator.

Regenerate phpstan-baseline.neon: 14 Lcobucci/Negotiation findings drop out;
no entries added. Baseline 65 -> 51 errors.
Address the JWT security-review follow-ups (Findings 4 & 5):

- iss is now a stable, configurable issuer (TOKEN_ISSUER) on
  TokenConfigurationInterface instead of the per-request URI. The old
  request-URI issuer made a token valid only at the exact endpoint that
  minted it; the generator and parser no longer depend on ServerRequest.
  TOKEN_ISSUER is required (fail fast) like TOKEN_PUBLIC_KEY.
- Optional audience (aud): when TOKEN_AUDIENCE is configured the generator
  adds permittedFor() and the parser asserts PermittedFor; absent audience
  keeps the single-service default (no aud).

Also add #[\Override] to the JWT test helpers/fixtures (FrozenClock::now,
setUpBeforeClass) so the full-codebase `rector process --dry-run` is clean —
these were missed because the migration commit only ran rector against src/.

Tests extended for the configured issuer and for audience accept/reject.
@tonydspaniard tonydspaniard merged commit 32311c1 into master May 28, 2026
3 checks passed
@tonydspaniard tonydspaniard deleted the feat/96-jwt-lcobucci5-migration branch May 28, 2026 05:50
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