Skip to content

JwtClaims: strict-aud variant + verification/round-trip docs (S4+B4)#15

Merged
detain merged 1 commit into
masterfrom
fix/shared-s4-b4
Jun 28, 2026
Merged

JwtClaims: strict-aud variant + verification/round-trip docs (S4+B4)#15
detain merged 1 commit into
masterfrom
fix/shared-s4-b4

Conversation

@detain

@detain detain commented Jun 28, 2026

Copy link
Copy Markdown
Owner

Steps

Implements S4 and B4 from findings/plan_phlix-shared.md (combined per the plan: both touch src/Auth/JwtClaims.php + its tests). No Version.php bump — that is the batch-final release step.

S4 — JwtClaims audience-default hardening + verification docs (finding S4, LOW)

  • Add fromPayloadStrict(array $payload): self that throws InvalidArgumentException when aud is absent, rather than defaulting it.
  • fromPayload() keeps defaulting a missing audAUD_SERVER for legacy v0.10.x tokens (BC; existing tests unchanged).
  • Add a prominent class-level docblock stating JwtClaims performs no signature verification — it is a typed view over an already-decoded/verified payload; signature verification (and rejecting alg: none) is the caller's responsibility (server/hub JwtHandler).
  • Internal refactor: fromPayload()/fromPayloadStrict() share a private build() helper.

B4 — round-trip symmetry note (finding B4, LOW; no behaviour change)

  • Document the deliberate toPayload() null/empty-optional omission asymmetry (nbf/jti/scope/serverId omitted for legacy-decoder wire-compat) and why fromPayload(toPayload($claims)) == $claims still holds (re-defaulting on input).
  • toPayload() behaviour unchanged (wire compat).
  • Add round-trip object-equality tests for both all-fields and minimal-claims cases.

How verified (full gate, green, not weakened)

  • composer test — PHPUnit 349 tests, 2838 assertions, OK (9 new tests added).
  • composer stan — PHPStan Level 9, no baseline, No errors.
  • composer cs — phpcs PSR-12, clean.
  • composer psalmNo errors found.

BC / consumer impact

Additive, BC-safe (minor bump at release). Optional later migration of phlix-server/phlix-hub JwtHandler to fromPayloadStrict() once all issuers set aud — not required by this PR.

🤖 Generated with Claude Code

S4 — audience-default hardening + verification docs:
- Add `fromPayloadStrict(array): self` that THROWS when `aud` is absent,
  instead of defaulting to AUD_SERVER. `fromPayload()` keeps the v0.10.x
  legacy default (existing tests/behaviour unchanged) — BC-safe.
- Add a prominent class docblock stating JwtClaims performs NO signature
  verification: it is a typed view over an already-decoded/verified payload;
  signature verification is the caller's responsibility (server/hub JwtHandler).

B4 — round-trip symmetry note (no behaviour change):
- Document the deliberate `toPayload()` null/empty-optional omission asymmetry
  (wire-compat for legacy decoders) and why the object round-trip stays lossless.
- Add round-trip object-equality tests asserting
  fromPayload(toPayload($claims)) == $claims for both the all-fields and
  minimal-claims cases.

Refactor fromPayload/fromPayloadStrict to share a private build() helper.
CHANGELOG [Unreleased] updated. No Version bump (batch-final release step).

Gate: PHPUnit 349 tests / 2838 assertions OK; PHPStan L9 (no baseline) no
errors; phpcs PSR-12 clean; Psalm no errors.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@codecov

codecov Bot commented Jun 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.81%. Comparing base (b9073f2) to head (943f828).

Additional details and impacted files
@@             Coverage Diff              @@
##             master      #15      +/-   ##
============================================
+ Coverage     58.57%   58.81%   +0.24%     
- Complexity      422      424       +2     
============================================
  Files            38       38              
  Lines          1260     1265       +5     
============================================
+ Hits            738      744       +6     
+ Misses          522      521       -1     
Flag Coverage Δ
phpunit 58.81% <100.00%> (+0.24%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codacy-production

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 7 medium · 9 minor

Alerts:
⚠ 16 issues (≤ 0 issues of at least minor severity)

Results:
16 new issues

Category Results
BestPractice 3 medium
CodeStyle 9 minor
Complexity 4 medium

View in Codacy

🟢 Metrics 7 complexity · 0 duplication

Metric Results
Complexity 7
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@detain detain merged commit 17ed149 into master Jun 28, 2026
8 of 9 checks passed
@detain detain deleted the fix/shared-s4-b4 branch June 28, 2026 16:28
detain added a commit that referenced this pull request Jun 28, 2026
Bump Version::VERSION to 0.11.0 and promote the [Unreleased] CHANGELOG
section to [0.11.0] - 2026-06-28, rolling up the four changes merged
since v0.10.1:

- #14 S1: harden RelayHttpRequest with path/method gate + forbidden-header
  helpers (isForbiddenHeader/withoutForbiddenHeaders/assertSafe)
- #15 JwtClaims: strict-aud variant (fromPayloadStrict) + verification/
  round-trip docs (S4+B4)
- #16 refactor(arr): extract AbstractArrClient to dedup the four *arr
  clients (F2a)
- #17 feat(arr): inject async HTTP transport via ArrTransportInterface
  (F2b/B1/P1)

composer.json carries no hardcoded version field (tags drive version),
so it is left unchanged. The git tag is applied by the coordinator
after merge.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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