Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,24 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
unchanged. **Consumer follow-up (phlix-server `RelayConsumer::buildRequest()`):**
stop trusting `x-phlix-relay-user` from the envelope and strip forbidden headers
via `isForbiddenHeader()` — that paired PR is the actual auth-bypass fix.
- **`Auth\JwtClaims::fromPayloadStrict(array): self`** (finding S4) — strict variant
of `fromPayload()` that **throws** `InvalidArgumentException` when the `aud` claim is
absent, instead of silently defaulting it to `AUD_SERVER`. `fromPayload()` keeps the
v0.10.x backward-compat default for legacy tokens (existing behaviour unchanged);
consumers can migrate to the strict variant once every issuer emits `aud`. Additive,
BC-safe.

### Documentation
- **`Auth\JwtClaims` security & round-trip docs** (findings S4/B4):
- Class docblock now prominently states `JwtClaims` performs **no signature
verification** — it is a typed view over an already-decoded/verified payload;
verifying the JWT signature (and rejecting `alg: none`) is the caller's
responsibility (server/hub `JwtHandler`).
- Documents the deliberate `toPayload()` asymmetry: null/empty optionals
(`nbf`/`jti`/`scope`/`serverId`) are omitted from the array for legacy-decoder
wire-compat, yet `fromPayload(toPayload($claims)) == $claims` remains lossless
because `fromPayload()` re-applies the defaults. New round-trip object-equality
tests cover both the all-fields and minimal-claims cases. No behaviour change.

## [0.10.1] - 2026-06-23

Expand Down
99 changes: 88 additions & 11 deletions src/Auth/JwtClaims.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,38 @@
* decoded payload into this DTO so server and hub share one definition
* of "what's in a Phlix JWT".
*
* ## SECURITY: this class performs NO signature verification.
*
* `JwtClaims` is purely a typed *view* over an array payload that has
* **already been decoded and cryptographically verified** by the caller.
* Neither {@see self::fromPayload()} nor {@see self::fromPayloadStrict()}
* checks the JWT signature, the `alg` header, key identity, or any HMAC —
* they only validate field presence and PHP types. Verifying the token
* signature (and rejecting `alg: none`) is the **caller's responsibility**:
* server and hub do this in their respective `JwtHandler` before passing
* the decoded payload here. Constructing a `JwtClaims` from an unverified
* payload conveys **no** authenticity guarantee.
*
* ## Audience (`aud`) handling
*
* {@see self::fromPayload()} defaults a missing `aud` to {@see self::AUD_SERVER}
* as a deliberate v0.10.x backward-compat shim for legacy tokens minted by
* `JwtHandler` before the field existed. Once every issuer emits `aud`,
* consumers should migrate to {@see self::fromPayloadStrict()}, which throws
* on a missing `aud` instead of defaulting.
*
* ## Round-trip / `toPayload()` asymmetry (deliberate)
*
* {@see self::toPayload()} intentionally **omits** null/empty optional claims
* (`nbf`, `jti`, `scope`, `serverId`) from its output so that legacy decoders
* predating those fields never see unexpected keys — this is a wire-compat
* requirement and must NOT change. The serialization is therefore
* asymmetric at the array level (a minimal claim set produces fewer keys than
* the full constructor), yet `fromPayload(toPayload($claims)) == $claims`
* still holds: `fromPayload()` re-applies the same null/empty defaults for the
* omitted keys, so the object round-trip is lossless. See the round-trip tests
* in `JwtClaimsTest` for both the all-fields and minimal-claims cases.
*
* @package Phlix\Shared\Auth
* @since 0.2.0
*/
Expand Down Expand Up @@ -60,25 +92,24 @@ public function __construct(
* Build from the array shape `JwtHandler::validateToken()` returns today.
*
* Tolerant of missing optional fields (`nbf`, `jti`, `scope`,
* `serverId`). Throws {@see InvalidArgumentException} when any of
* the RFC 7519 required fields is missing or has the wrong type.
* `serverId`) and of a missing `aud` (defaulted to {@see self::AUD_SERVER}
* for legacy tokens — see the class docblock). Throws
* {@see InvalidArgumentException} when any of the RFC 7519 required fields
* is missing or has the wrong type.
*
* Performs **no** signature verification — see the class docblock.
*
* @param array<string, mixed> $payload Decoded token payload.
*
* @throws InvalidArgumentException When a required field is missing or wrong-typed.
*/
public static function fromPayload(array $payload): self
{
$iss = self::requireString($payload, 'iss');
$sub = self::requireString($payload, 'sub');
$iat = self::requireInt($payload, 'iat');
$exp = self::requireInt($payload, 'exp');
$type = self::requireString($payload, 'type');

// `aud` is required per the shared shape; default to AUD_SERVER
// for tokens emitted by the legacy `JwtHandler` that pre-date
// the field. This keeps `fromPayload()` tolerant of v0.10.x
// payloads but still surfaces wrong-typed values.
// payloads but still surfaces wrong-typed values. Migrate to
// fromPayloadStrict() once every issuer emits `aud`.
$aud = self::AUD_SERVER;
if (array_key_exists('aud', $payload)) {
if (!is_string($payload['aud'])) {
Expand All @@ -87,6 +118,47 @@ public static function fromPayload(array $payload): self
$aud = $payload['aud'];
}

return self::build($payload, $aud);
}

/**
* Strict variant of {@see self::fromPayload()} that requires an explicit
* `aud` claim and throws when it is absent, instead of defaulting it to
* {@see self::AUD_SERVER}.
*
* Use this once all token issuers (server and hub `JwtHandler`) emit `aud`
* so a missing audience is treated as a malformed token rather than
* silently accepted as a legacy server-audience token.
*
* Performs **no** signature verification — see the class docblock.
*
* @param array<string, mixed> $payload Decoded token payload.
*
* @throws InvalidArgumentException When a required field (including `aud`)
* is missing or wrong-typed.
*/
public static function fromPayloadStrict(array $payload): self
{
$aud = self::requireString($payload, 'aud');

return self::build($payload, $aud);
}

/**
* Build the DTO from a payload using a pre-resolved `aud`.
*
* @param array<string, mixed> $payload Decoded token payload.
*
* @throws InvalidArgumentException When a required field is missing or wrong-typed.
*/
private static function build(array $payload, string $aud): self
{
$iss = self::requireString($payload, 'iss');
$sub = self::requireString($payload, 'sub');
$iat = self::requireInt($payload, 'iat');
$exp = self::requireInt($payload, 'exp');
$type = self::requireString($payload, 'type');

$nbf = null;
if (array_key_exists('nbf', $payload) && $payload['nbf'] !== null) {
if (!is_int($payload['nbf'])) {
Expand Down Expand Up @@ -141,8 +213,13 @@ public static function fromPayload(array $payload): self
/**
* Serialize to the array shape `JwtHandler::encode()` expects today.
*
* Optional fields are omitted when null/empty so v0.10.x decoders
* that pre-date them don't see unexpected keys.
* Optional fields (`nbf`, `jti`, `scope`, `serverId`) are **deliberately
* omitted** when null/empty so v0.10.x decoders that pre-date them don't
* see unexpected keys (wire-compat — must not change). This makes the
* array output asymmetric versus the constructor, but the object
* round-trip remains lossless: `fromPayload(toPayload($claims)) == $claims`
* because `fromPayload()` re-applies the same defaults. See the class
* docblock and the round-trip tests.
*
* @return array<string, mixed>
*/
Expand Down
86 changes: 86 additions & 0 deletions tests/Auth/JwtClaimsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -156,4 +156,90 @@ public function test_toPayload_omits_optional_null_fields(): void
$this->assertArrayNotHasKey('scope', $payload);
$this->assertArrayNotHasKey('serverId', $payload);
}

// --- S4: fromPayloadStrict() audience hardening -----------------------

public function test_fromPayloadStrict_with_explicit_aud_succeeds(): void
{
$claims = JwtClaims::fromPayloadStrict([
'iss' => JwtClaims::ISS_PHLIX_HUB,
'aud' => JwtClaims::AUD_CLIENT,
'sub' => 'user-uuid',
'iat' => 1700000000,
'exp' => 1700003600,
'type' => JwtClaims::TYPE_ACCESS,
]);

$this->assertSame(JwtClaims::AUD_CLIENT, $claims->aud);
$this->assertSame(JwtClaims::ISS_PHLIX_HUB, $claims->iss);
}

public function test_fromPayloadStrict_missing_aud_throws(): void
{
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('JWT claim "aud" is required.');
JwtClaims::fromPayloadStrict([
'iss' => 'phlix', 'sub' => 'u', 'iat' => 1, 'exp' => 2, 'type' => 'access',
]);
}

public function test_fromPayloadStrict_wrong_type_aud_throws(): void
{
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('JWT claim "aud" must be a string.');
JwtClaims::fromPayloadStrict([
'iss' => 'phlix', 'aud' => 42, 'sub' => 'u', 'iat' => 1, 'exp' => 2, 'type' => 'access',
]);
}

public function test_fromPayloadStrict_still_validates_other_required_fields(): void
{
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('JWT claim "iss" is required.');
JwtClaims::fromPayloadStrict([
'aud' => JwtClaims::AUD_SERVER, 'sub' => 'u', 'iat' => 1, 'exp' => 2, 'type' => 'access',
]);
}

public function test_fromPayload_still_defaults_missing_aud_for_legacy_tokens(): void
{
// BC: the lenient variant must keep defaulting a missing `aud`.
$claims = JwtClaims::fromPayload([
'iss' => 'phlix', 'sub' => 'u', 'iat' => 1, 'exp' => 2, 'type' => 'access',
]);

$this->assertSame(JwtClaims::AUD_SERVER, $claims->aud);
}

// --- B4: object round-trip symmetry -----------------------------------

public function test_roundtrip_object_equality_full_claims(): void
{
$claims = JwtClaims::fromPayload([
'iss' => JwtClaims::ISS_PHLIX_HUB,
'aud' => JwtClaims::AUD_CLIENT,
'sub' => 'user-uuid',
'iat' => 1700000000,
'exp' => 1700003600,
'type' => JwtClaims::TYPE_ACCESS,
'nbf' => 1700000000,
'jti' => 'token-id',
'scope' => ['library:read', 'playback:write'],
'serverId' => 'server-uuid',
]);

$this->assertEquals($claims, JwtClaims::fromPayload($claims->toPayload()));
}

public function test_roundtrip_object_equality_minimal_claims(): void
{
// toPayload() omits the null/empty optionals; fromPayload() re-defaults
// them, so the reconstructed object must equal the original despite the
// asymmetric array serialization.
$claims = JwtClaims::fromPayload([
'iss' => 'phlix', 'sub' => 'u', 'iat' => 1, 'exp' => 2, 'type' => 'access',
]);

$this->assertEquals($claims, JwtClaims::fromPayload($claims->toPayload()));
}
}
Loading