refactor: contain the JWT::$leeway static, and restore it after each decode

CHANGELOG.md

@@ -50,6 +50,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   newly escaped mutant fails the build rather than being absorbed by headroom,
   and pull requests stay free of `--logger-github` annotations unless they
   genuinely regress the score
+- The write to `firebase/php-jwt`'s process-global `JWT::$leeway` is contained in
+  a single private `decodeWithLeeway()` method, which does nothing but set the
+  static and decode. Behaviour is unchanged; the point is that the constraint
+  — nothing that could suspend may come between the write and the decode — now
+  has one home and a stated rationale instead of being an ordering that happened
+  to hold. Under PHP-FPM this is a formality, but it is what a fibre-based or
+  worker runtime would depend on
+- The previous `JWT::$leeway` value is restored after each decode. Under PHP-FPM
+  the mutated static died with the request; in a worker process it persisted for
+  the life of the process and silently applied to any other `firebase/php-jwt`
+  consumer that never set its own leeway. Writing a process-global is
+  unavoidable given the upstream API, but leaving it written is not
 - The discovery document and the JWKS are capped at 1 MiB each, raising
   `HttpException` when a response exceeds it. Both are a few kilobytes in
   practice, so a hostile or misconfigured endpoint could previously hand over an

src/Security/OpenIdConfigurationProvider.php

@@ -234,10 +234,9 @@ public function getEndSessionUrl(?string $postLogoutRedirectUri = null, ?string
      * constant-time: it is the only claim compared against a value the caller
      * holds, so a timing signal would leak that secret.
      *
-     * Note: JWT::$leeway is a static property, so in environments with multiple
-     * OpenIdConfigurationProvider instances (e.g. multi-tenant setups in long-running
-     * processes), the leeway value set by the last provider to call validateIdToken()
-     * will apply globally until overwritten.
+     * Note: the leeway is applied through a process-global static in
+     * firebase/php-jwt, so with several providers configured differently the
+     * value belongs to whichever called last. See decodeWithLeeway().
      *
      * @param string $idToken Raw id token
      * @param string $nonce   Nonce
@@ -257,10 +256,7 @@ public function validateIdToken(string $idToken, string $nonce): object
     {
         try {
             $keys = $this->getJwtVerificationKeys();
-            // NB: JWT::$leeway is a static property shared across all instances.
-            // Always set it immediately before decode to ensure the correct value.
-            JWT::$leeway = $this->leeway;
-            $claims = JWT::decode($idToken, $keys);
+            $claims = $this->decodeWithLeeway($idToken, $keys);
 
             // "exp" and "iat" are REQUIRED by OIDC Core §2, but
             // firebase/php-jwt only validates them when they are present — a
@@ -298,6 +294,46 @@ public function validateIdToken(string $idToken, string $nonce): object
         }
     }
 
+    /**
+     * Decode an ID token, applying the configured leeway.
+     *
+     * INVARIANT: firebase/php-jwt exposes leeway only as `JWT::$leeway`, a
+     * process-global static with no per-call alternative, so this method is the
+     * single place that writes it. Nothing may come between the write and
+     * `JWT::decode()` that could suspend — no HTTP, no cache read, no I/O of any
+     * kind. That is why the verification keys are resolved by the caller before
+     * this is entered, and why this method does nothing else.
+     *
+     * Under PHP-FPM, where a request owns its process, that ordering is a
+     * formality. Under cooperative concurrency it is what stops a fiber from
+     * decoding with a sibling provider's leeway, and under a preemptive model it
+     * would not be sufficient at all. Revisit before adopting FrankenPHP
+     * workers or any other long-lived concurrent runtime.
+     *
+     * The previous value is restored afterwards. Under FPM the mutated static
+     * dies with the request either way, but in a worker it would persist for the
+     * life of the process and silently apply to any other firebase/php-jwt
+     * consumer in it that never sets its own leeway. Writing a process-global is
+     * unavoidable here; leaving it written is not.
+     *
+     * @param array<string, Key> $keys Verification keys, indexed by JWK `kid`
+     *
+     * @return \stdClass The token's payload, as JWT::decode() returns it
+     *
+     * @throws \UnexpectedValueException from JWT::decode(), wrapped by the caller
+     */
+    private function decodeWithLeeway(string $idToken, array $keys): \stdClass
+    {
+        $previousLeeway = JWT::$leeway;
+        JWT::$leeway = $this->leeway;
+
+        try {
+            return JWT::decode($idToken, $keys);
+        } finally {
+            JWT::$leeway = $previousLeeway;
+        }
+    }
+
     /**
      * Assert that a claim is present and numeric.
      *

tests/Security/OpenIdConfigurationProviderTest.php

@@ -306,6 +306,54 @@ public function testValidateIdTokenSuccess(): void
         $this->assertEquals(self::CLIENT_ID, $claims->aud);
     }
 
+    /**
+     * The configured leeway must reach firebase/php-jwt. It travels through the
+     * process-global JWT::$leeway, which is why MockJWT declares that static —
+     * asserting it here is what pins decodeWithLeeway() to writing the
+     * provider's value rather than leaving whatever was there before.
+     */
+    public function testValidateIdTokenAppliesConfiguredLeeway(): void
+    {
+        MockJWT::$leeway = 5;
+
+        $observedDuringDecode = null;
+        $mockClaims = $this->getMockClaims();
+
+        $mockJWT = $this->overloadJwt();
+        $mockJWT->shouldReceive('decode')->andReturnUsing(
+            function () use (&$observedDuringDecode, $mockClaims): \stdClass {
+                $observedDuringDecode = MockJWT::$leeway;
+
+                return $mockClaims;
+            }
+        );
+
+        $this->provider->validateIdToken('token', self::NONCE);
+
+        // 30 is the leeway the provider is constructed with in setUp().
+        $this->assertSame(30, $observedDuringDecode, 'Configured leeway must be in effect for the decode');
+        $this->assertSame(5, MockJWT::$leeway, 'Any pre-existing leeway must be restored afterwards');
+    }
+
+    /**
+     * The static is restored even when the decode throws, so a rejected token
+     * cannot leave this library's leeway applied to the rest of the process.
+     */
+    public function testValidateIdTokenRestoresLeewayWhenDecodeFails(): void
+    {
+        MockJWT::$leeway = 5;
+
+        $mockJWT = $this->overloadJwt();
+        $mockJWT->shouldReceive('decode')->andThrow(SignatureInvalidException::class, 'Signature verification failed');
+
+        try {
+            $this->provider->validateIdToken('token', self::NONCE);
+            $this->fail('Expected ValidationException was not thrown');
+        } catch (ValidationException) {
+            $this->assertSame(5, MockJWT::$leeway, 'Leeway must be restored on the failure path too');
+        }
+    }
+
     public function testValidateIdTokenFailure(): void
     {
         $mockJWT = $this->overloadJwt();