feat: add S256 PKCE, carried by the caller rather than the provider

CHANGELOG.md

@@ -23,6 +23,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Mutation testing with [Infection](https://infection.github.io/)
   (`task test:mutation`), run in CI and reported to the Stryker dashboard
   (mutation score badge in README)
+- PKCE support (RFC 7636), S256 only. `generatePkceVerifier()` returns a
+  128-character verifier and `getPkceChallenge()` derives its challenge;
+  `getIdToken()` takes the verifier as an optional second argument and sends
+  `code_verifier` when given. PKCE is opt-in with no configuration flag —
+  passing a `code_challenge` to `getAuthorizationUrl()` turns it on, and
+  `code_challenge_method=S256` is filled in automatically, since omitting it
+  makes the server assume `plain` (RFC 7636 §4.3) and treat the challenge as a
+  cleartext value. Neither the verifier nor the challenge is stored on the
+  provider: `league/oauth2-client` keeps its verifier on the provider object,
+  which on an instance shared between requests would let one request's verifier
+  be sent for another request's token exchange, so the verifier is carried by
+  the caller in the session exactly like the state and the nonce
 - `phpstan-lowest` CI job and the matching `task analyze:php:lowest`, analysing
   the declared dependency floor with current dev tooling. Only the packages
   `require` names are lowered, so findings are about the runtime dependencies

README.md

@@ -217,6 +217,47 @@ normative sentence names access tokens, so it does not literally cover a bare
 `id_token` response — but `code` is the flow it points at, and the one
 [openid-connect-bundle](https://github.com/itk-dev/openid-connect-bundle) uses.
 
+##### PKCE (RFC 7636)
+
+PKCE is opt-in, and there is no configuration flag: passing a `code_challenge`
+turns it on, omitting it changes nothing.
+
+The verifier is a secret and belongs in the session alongside the state and the
+nonce. Only its challenge may reach the authorization request.
+
+```php
+// Authorization request
+$verifier = $provider->generatePkceVerifier();
+$session->set('oauth2pkce', $verifier);
+
+$authUrl = $provider->getAuthorizationUrl([
+    'state' => $state,
+    'nonce' => $nonce,
+    'response_type' => 'code',
+    'code_challenge' => $provider->getPkceChallenge($verifier),
+]);
+```
+
+`code_challenge_method=S256` is filled in for you. Omitting it would make the
+server assume `plain` (RFC 7636 §4.3) and treat the challenge as a value sent in
+the clear, so it is not left to the caller to remember.
+
+On the way back, hand the verifier to the code exchange:
+
+```php
+$verifier = $session->get('oauth2pkce');
+$session->remove('oauth2pkce');
+
+$idToken = $provider->getIdToken($request->query->get('code'), $verifier);
+$claims = $provider->validateIdToken($idToken, $session->get('oauth2nonce'));
+```
+
+`generatePkceVerifier()` stores nothing on the provider, which is deliberate.
+`league/oauth2-client` keeps its own verifier on the provider object; on an
+instance shared between requests that would let one request's verifier be sent
+for another request's exchange — the same hazard as reading `getState()` back.
+The session is the only place the verifier should live.
+
 #### Verify authorized requests
 
 The authorization service will redirect the user back to the `redirectUri`. This

src/Security/OpenIdConfigurationProvider.php

@@ -54,6 +54,17 @@ class OpenIdConfigurationProvider extends AbstractProvider
     // decode and cache.
     private const int MAX_JSON_RESOURCE_BYTES = 1048576;
 
+    // PKCE (RFC 7636). S256 is the only challenge method offered: "plain" is a
+    // downgrade with no reason to exist where sha256 is available.
+    //
+    // RFC 7636 §4.1 allows a verifier of 43 to 128 characters and servers must
+    // support the whole range, so the verifier is always generated at the
+    // maximum — a shorter one has nothing to recommend it. 96 random bytes
+    // base64url-encode to exactly 128 characters with no padding, which is why
+    // that byte count and no other.
+    private const string PKCE_CHALLENGE_METHOD = 'S256';
+    private const int PKCE_VERIFIER_BYTES = 96;
+
     // @see https://openid.net/specs/openid-connect-rpinitiated-1_0.html#RPLogout
     private const string POST_LOGOUT_REDIRECT_URI = 'post_logout_redirect_uri';
     private const string ID_TOKEN_HINT = 'id_token_hint';
@@ -168,6 +179,14 @@ public function getAuthorizationUrl(array $options = []): string
             throw new MissingParameterException('Required parameter "nonce" missing');
         }
 
+        // PKCE (RFC 7636) is opt-in: passing a code_challenge turns it on. The
+        // method is filled in here because omitting it makes the server assume
+        // "plain" (RFC 7636 §4.3), which would silently downgrade an S256
+        // challenge to a value sent in the clear.
+        if (!empty($options['code_challenge'])) {
+            $options += ['code_challenge_method' => self::PKCE_CHALLENGE_METHOD];
+        }
+
         // Add default response_type and response_mode. The `scope` default is
         // supplied by getDefaultScopes() via league's
         // getAuthorizationParameters(), so it is not repeated here.
@@ -385,18 +404,27 @@ private static function requireStringClaim(object $claims, string $name): string
      *
      * @throws OpenIdConnectExceptionInterface
      */
-    public function getIdToken(string $code): string
+    public function getIdToken(string $code, ?string $codeVerifier = null): string
     {
         try {
             $endpoint = $this->getSecureEndpoint('token_endpoint');
+
+            $params = [
+                'client_id' => $this->clientId,
+                'client_secret' => $this->clientSecret,
+                'redirect_uri' => $this->redirectUri,
+                'grant_type' => 'authorization_code',
+                'code' => $code,
+            ];
+
+            // Sent only when the caller holds one. An unsolicited code_verifier
+            // is an error to a server that issued no challenge.
+            if (null !== $codeVerifier) {
+                $params['code_verifier'] = $codeVerifier;
+            }
+
             $response = $this->getHttpClient()->request('POST', $endpoint, [
-                'form_params' => [
-                    'client_id' => $this->clientId,
-                    'client_secret' => $this->clientSecret,
-                    'redirect_uri' => $this->redirectUri,
-                    'grant_type' => 'authorization_code',
-                    'code' => $code,
-                ],
+                'form_params' => $params,
             ]);
 
             $payload = json_decode((string) $response->getBody(), true, 512, JSON_THROW_ON_ERROR);
@@ -466,6 +494,43 @@ public function generateNonce(int $length = 32): string
      * @throws JsonException
      * @throws MetadataException
      */
+    /**
+     * Generate a PKCE code verifier (RFC 7636 §4.1).
+     *
+     * Nothing is stored on the provider. As with `generateNonce()`, the caller
+     * is the only holder — and here that is not merely tidy. league's own PKCE
+     * support keeps the verifier on the provider instance, which on an instance
+     * shared between requests (a long-running worker, or a container that
+     * memoizes the service) would let one request's verifier be sent for
+     * another's token exchange. Persist this in the session next to the state
+     * and the nonce, and hand it back to `getIdToken()`.
+     *
+     * The length is not configurable: RFC 7636 caps the verifier at 128
+     * characters and requires servers to accept that, so there is no reason to
+     * ask for less entropy.
+     *
+     * @return string A 128-character code verifier
+     */
+    public function generatePkceVerifier(): string
+    {
+        return self::base64urlEncode(random_bytes(self::PKCE_VERIFIER_BYTES));
+    }
+
+    /**
+     * Derive the S256 code challenge for a verifier (RFC 7636 §4.2).
+     *
+     * A pure function of its argument: no provider state is read or written, so
+     * the verifier never comes to rest on this object.
+     *
+     * @param string $verifier The code verifier to derive a challenge for
+     *
+     * @return string The S256 challenge, for the `code_challenge` authorization parameter
+     */
+    public function getPkceChallenge(string $verifier): string
+    {
+        return self::base64urlEncode(hash('sha256', $verifier, true));
+    }
+
     public function getBaseAccessTokenUrl(array $params): string
     {
         return $this->getSecureEndpoint('token_endpoint');
@@ -620,6 +685,14 @@ private function getJwksDocument(): array
         }
     }
 
+    /**
+     * Encode base 64 url, without padding (RFC 7515 §2).
+     */
+    private static function base64urlEncode(string $input): string
+    {
+        return rtrim(strtr(base64_encode($input), '+/', '-_'), '=');
+    }
+
     /**
      * Decode base 64 url.
      *

tests/Security/OpenIdConfigurationProviderTest.php

@@ -238,6 +238,84 @@ public function testGetAuthorizationUrl(): void
         $this->assertSame($nonce, $query['nonce']);
     }
 
+    /**
+     * RFC 7636 Appendix B publishes a verifier/challenge pair; deriving anything
+     * else means the challenge would not match at the token endpoint.
+     */
+    public function testGetPkceChallengeMatchesTheRfcTestVector(): void
+    {
+        $this->assertSame(
+            'E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM',
+            $this->provider->getPkceChallenge('dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk')
+        );
+    }
+
+    public function testGeneratePkceVerifierProducesAMaximumLengthVerifier(): void
+    {
+        $verifier = $this->provider->generatePkceVerifier();
+
+        // RFC 7636 §4.1 caps the verifier at 128 characters and restricts it to
+        // the unreserved set.
+        $this->assertSame(128, strlen($verifier));
+        $this->assertMatchesRegularExpression('/^[A-Za-z0-9\-._~]+$/', $verifier);
+        $this->assertNotSame($verifier, $this->provider->generatePkceVerifier(), 'Each call must produce a fresh verifier');
+    }
+
+    /**
+     * Omitting code_challenge_method makes the server assume "plain"
+     * (RFC 7636 §4.3), so passing a challenge must carry S256 with it.
+     */
+    public function testGetAuthorizationUrlAddsS256MethodWithAChallenge(): void
+    {
+        $verifier = $this->provider->generatePkceVerifier();
+        $challenge = $this->provider->getPkceChallenge($verifier);
+
+        $query = $this->authorizationUrlQuery(['code_challenge' => $challenge]);
+
+        $this->assertSame($challenge, $query['code_challenge']);
+        $this->assertSame('S256', $query['code_challenge_method']);
+    }
+
+    /**
+     * The verifier is the secret half of the exchange. It is carried in the
+     * caller's session and must never reach the authorization request.
+     */
+    public function testGetAuthorizationUrlNeverCarriesTheVerifier(): void
+    {
+        $verifier = $this->provider->generatePkceVerifier();
+
+        $authUrl = $this->provider->getAuthorizationUrl([
+            'state' => '12345678',
+            'nonce' => 'abcdefghij',
+            'code_challenge' => $this->provider->getPkceChallenge($verifier),
+        ]);
+
+        $this->assertStringNotContainsString($verifier, $authUrl);
+        $this->assertStringNotContainsString('code_verifier', $authUrl);
+    }
+
+    public function testGetAuthorizationUrlOmitsPkceWithoutAChallenge(): void
+    {
+        $query = $this->authorizationUrlQuery([]);
+
+        $this->assertArrayNotHasKey('code_challenge', $query);
+        $this->assertArrayNotHasKey('code_challenge_method', $query);
+    }
+
+    /**
+     * A caller who names the method keeps it, so a provider needing "plain"
+     * is not locked out by the default.
+     */
+    public function testGetAuthorizationUrlKeepsAnExplicitChallengeMethod(): void
+    {
+        $query = $this->authorizationUrlQuery([
+            'code_challenge' => 'challenge-value',
+            'code_challenge_method' => 'plain',
+        ]);
+
+        $this->assertSame('plain', $query['code_challenge_method']);
+    }
+
     public function testGetAuthorizationUrlStateException(): void
     {
         $this->expectException(MissingParameterException::class);
@@ -917,6 +995,61 @@ public function testGetIdTokenSuccess(): void
         $this->assertSame('the-id-token', $idToken);
     }
 
+    /**
+     * With a verifier the exchange carries `code_verifier`, and only then: the
+     * request map below matches on exact form parameters, so an unconditionally
+     * added key would fail to match and the stub would return null. That is what
+     * pins both halves of the condition.
+     */
+    public function testGetIdTokenSendsCodeVerifierWhenGiven(): void
+    {
+        $tokenEndpoint = 'https://azure_b2c_test.b2clogin.com/azure_b2c_test.onmicrosoft.com/oauth2/v2.0/token?p=test-policy';
+        $openIDConnectMetadataUrl = 'https://provider.example.org/openid-configuration';
+        $verifier = 'dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk';
+
+        $mockConfigResponse = $this->getMockHttpSuccessResponse('/../MockData/mockOpenIDConfiguration.json');
+
+        $tokenResponseBody = json_encode(['id_token' => 'the-id-token']);
+        $mockTokenStream = $this->createStub(StreamInterface::class);
+        $mockTokenStream->method('getContents')->willReturn($tokenResponseBody);
+        $mockTokenStream->method('__toString')->willReturn($tokenResponseBody);
+
+        $mockTokenResponse = $this->createStub(ResponseInterface::class);
+        $mockTokenResponse->method('getStatusCode')->willReturn(200);
+        $mockTokenResponse->method('getBody')->willReturn($mockTokenStream);
+
+        $mockHttpClient = $this->createStub(ClientInterface::class);
+        $mockHttpClient->method('request')->willReturnMap([
+            ['GET', $openIDConnectMetadataUrl, [], $mockConfigResponse],
+            ['POST', $tokenEndpoint, ['form_params' => [
+                'client_id' => self::CLIENT_ID,
+                'client_secret' => self::CLIENT_SECRET,
+                'redirect_uri' => self::REDIRECT_URI,
+                'grant_type' => 'authorization_code',
+                'code' => 'test-code',
+                'code_verifier' => $verifier,
+            ]], $mockTokenResponse],
+        ]);
+
+        $mockCacheItem = $this->createStub(CacheItemInterface::class);
+        $mockCacheItem->method('isHit')->willReturn(false);
+
+        $mockCacheItemPool = $this->createStub(CacheItemPoolInterface::class);
+        $mockCacheItemPool->method('getItem')->willReturn($mockCacheItem);
+
+        $provider = new OpenIdConfigurationProvider([
+            'openIDConnectMetadataUrl' => $openIDConnectMetadataUrl,
+            'cacheItemPool' => $mockCacheItemPool,
+            'clientId' => self::CLIENT_ID,
+            'clientSecret' => self::CLIENT_SECRET,
+            'redirectUri' => self::REDIRECT_URI,
+        ], [
+            'httpClient' => $mockHttpClient,
+        ]);
+
+        $this->assertSame('the-id-token', $provider->getIdToken('test-code', $verifier));
+    }
+
     public function testGetIdTokenFailure(): void
     {
         $openIDConnectMetadataUrl = 'https://provider.example.org/openid-configuration';
@@ -1978,6 +2111,30 @@ private function discoveryDocumentOfExactByteSize(int $bytes): string
         return $json;
     }
 
+    /**
+     * Build an authorization URL with state and nonce supplied, and return its
+     * query parameters.
+     *
+     * @param array<string, string> $options extra authorization parameters
+     *
+     * @return array<int|string, array<mixed>|string> as parse_str() populates it
+     */
+    private function authorizationUrlQuery(array $options): array
+    {
+        $authUrl = $this->provider->getAuthorizationUrl($options + [
+            'state' => '12345678',
+            'nonce' => 'abcdefghij',
+        ]);
+
+        $queryString = parse_url($authUrl, PHP_URL_QUERY);
+        $this->assertIsString($queryString, 'Generated authorization URL must have a query string');
+
+        $query = [];
+        parse_str($queryString, $query);
+
+        return $query;
+    }
+
     private function loadMockFixture(string $filename): array
     {
         $path = __DIR__.'/../MockData/'.$filename;