From 9b6ac3ab92218393bf178a439843c537d50b28d2 Mon Sep 17 00:00:00 2001 From: Ralf Lang Date: Mon, 15 Jun 2026 09:15:35 +0200 Subject: [PATCH 1/3] style: whitespace --- .gitignore | 6 +++ test/Client/OAuth2ClientTest.php | 86 ++++++++++++++++---------------- 2 files changed, 49 insertions(+), 43 deletions(-) diff --git a/.gitignore b/.gitignore index db11d2e..d27b0bf 100644 --- a/.gitignore +++ b/.gitignore @@ -53,3 +53,9 @@ run-tests.log /phpstan.neon # PHPStan cache directory /.phpstan.cache/ + +# Added by horde-components QC --fix-qc-issues +# Horde installer plugin runtime data +/var/ +# Horde installer plugin web-accessible directory +/web/ diff --git a/test/Client/OAuth2ClientTest.php b/test/Client/OAuth2ClientTest.php index b12433a..81f2e32 100644 --- a/test/Client/OAuth2ClientTest.php +++ b/test/Client/OAuth2ClientTest.php @@ -40,7 +40,7 @@ protected function setUp(): void 'authorization_endpoint' => 'https://idp.example.org/authorize', 'token_endpoint' => 'https://idp.example.org/token', 'revocation_endpoint' => 'https://idp.example.org/revoke', - 'token_endpoint_auth_methods_supported' => ['client_secret_post'], + 'token_endpoint_auth_methods_supported' => ['client_secret_post'], ]); $this->stream = $this->createStub(StreamInterface::class); @@ -59,13 +59,13 @@ private function makeClient( ?ProviderConfig $provider = null, ): OAuth2Client { return new OAuth2Client( - provider: $provider ?? $this->provider, - clientId: 'test-client', - clientSecret: $clientSecret, - redirectUri: 'https://horde.example.org/callback', - httpClient: $httpClient, + provider: $provider ?? $this->provider, + clientId: 'test-client', + clientSecret: $clientSecret, + redirectUri: 'https://horde.example.org/callback', + httpClient: $httpClient, requestFactory: $requestFactory, - streamFactory: $this->streamFactory, + streamFactory: $this->streamFactory, ); } @@ -110,13 +110,13 @@ public function testRevokeTokenIncludesTokenInBodyWhenPostConfigured(): void $httpClient->method('sendRequest')->willReturn($response); $client = new OAuth2Client( - provider: $this->provider, - clientId: 'test-client', - clientSecret: 'secret', - redirectUri: 'https://horde.example.org/callback', - httpClient: $httpClient, + provider: $this->provider, + clientId: 'test-client', + clientSecret: 'secret', + redirectUri: 'https://horde.example.org/callback', + httpClient: $httpClient, requestFactory: $requestFactory, - streamFactory: $streamFactory, + streamFactory: $streamFactory, ); $client->revokeToken('my-access-token', 'access_token'); @@ -147,13 +147,13 @@ public function testRevokeTokenWithDefaultHintIsAccessToken(): void $httpClient->method('sendRequest')->willReturn($response); $client = new OAuth2Client( - provider: $this->provider, - clientId: 'test-client', - clientSecret: 'secret', - redirectUri: 'https://horde.example.org/callback', - httpClient: $httpClient, + provider: $this->provider, + clientId: 'test-client', + clientSecret: 'secret', + redirectUri: 'https://horde.example.org/callback', + httpClient: $httpClient, requestFactory: $requestFactory, - streamFactory: $streamFactory, + streamFactory: $streamFactory, ); $client->revokeToken('my-token'); @@ -181,13 +181,13 @@ public function testRevokeTokenWithoutClientSecretOmitsIt(): void $httpClient->method('sendRequest')->willReturn($response); $client = new OAuth2Client( - provider: $this->provider, - clientId: 'test-client', - clientSecret: null, - redirectUri: 'https://horde.example.org/callback', - httpClient: $httpClient, + provider: $this->provider, + clientId: 'test-client', + clientSecret: null, + redirectUri: 'https://horde.example.org/callback', + httpClient: $httpClient, requestFactory: $requestFactory, - streamFactory: $streamFactory, + streamFactory: $streamFactory, ); $client->revokeToken('my-token'); @@ -207,13 +207,13 @@ public function testRevokeTokenThrowsWhenNoRevocationEndpoint(): void $requestFactory = $this->createStub(RequestFactoryInterface::class); $client = new OAuth2Client( - provider: $provider, - clientId: 'test-client', - clientSecret: 'secret', - redirectUri: 'https://horde.example.org/callback', - httpClient: $httpClient, + provider: $provider, + clientId: 'test-client', + clientSecret: 'secret', + redirectUri: 'https://horde.example.org/callback', + httpClient: $httpClient, requestFactory: $requestFactory, - streamFactory: $this->streamFactory, + streamFactory: $this->streamFactory, ); $this->expectException(OAuthException::class); @@ -276,13 +276,13 @@ public function testRevokeTokenUsesBasicAuthWhenConfigured(): void $httpClient->method('sendRequest')->willReturn($response); $client = new OAuth2Client( - provider: $provider, - clientId: 'test-client', - clientSecret: 'secret', - redirectUri: 'https://horde.example.org/callback', - httpClient: $httpClient, + provider: $provider, + clientId: 'test-client', + clientSecret: 'secret', + redirectUri: 'https://horde.example.org/callback', + httpClient: $httpClient, requestFactory: $requestFactory, - streamFactory: $streamFactory, + streamFactory: $streamFactory, ); $client->revokeToken('my-token'); @@ -323,13 +323,13 @@ public function testRevokeTokenUsesPostWhenBothMethodsSupported(): void $httpClient->method('sendRequest')->willReturn($response); $client = new OAuth2Client( - provider: $provider, - clientId: 'test-client', - clientSecret: 'secret', - redirectUri: 'https://horde.example.org/callback', - httpClient: $httpClient, + provider: $provider, + clientId: 'test-client', + clientSecret: 'secret', + redirectUri: 'https://horde.example.org/callback', + httpClient: $httpClient, requestFactory: $requestFactory, - streamFactory: $streamFactory, + streamFactory: $streamFactory, ); $client->revokeToken('my-token'); From 25ccccfa0c9ea93f50d6a3061a0006b991ce715d Mon Sep 17 00:00:00 2001 From: Ralf Lang Date: Mon, 15 Jun 2026 10:45:22 +0200 Subject: [PATCH 2/3] refactor(test): factor capture helpers in OAuth2ClientTest Adds two private helpers, captureStreamBody() and captureAuthHeader() that build stub factories returning a callback-based capture. Five existing tests use them, dropping ~30 lines of duplicated stub setup. --- test/Client/OAuth2ClientTest.php | 112 +++++++++++++++++-------------- 1 file changed, 61 insertions(+), 51 deletions(-) diff --git a/test/Client/OAuth2ClientTest.php b/test/Client/OAuth2ClientTest.php index 81f2e32..61f7185 100644 --- a/test/Client/OAuth2ClientTest.php +++ b/test/Client/OAuth2ClientTest.php @@ -36,10 +36,10 @@ final class OAuth2ClientTest extends TestCase protected function setUp(): void { $this->provider = ProviderConfig::fromArray([ - 'issuer' => 'https://idp.example.org', - 'authorization_endpoint' => 'https://idp.example.org/authorize', - 'token_endpoint' => 'https://idp.example.org/token', - 'revocation_endpoint' => 'https://idp.example.org/revoke', + 'issuer' => 'https://idp.example.org', + 'authorization_endpoint' => 'https://idp.example.org/authorize', + 'token_endpoint' => 'https://idp.example.org/token', + 'revocation_endpoint' => 'https://idp.example.org/revoke', 'token_endpoint_auth_methods_supported' => ['client_secret_post'], ]); @@ -69,6 +69,47 @@ private function makeClient( ); } + /** + * Returns a stream factory whose createStream() captures the body string + * passed in, then returns the shared $this->stream stub. Use when a test + * needs to assert on the wire body without touching a real HTTP layer. + */ + private function captureStreamBody(?string &$captured): StreamFactoryInterface + { + $factory = $this->createStub(StreamFactoryInterface::class); + $factory->method('createStream') + ->willReturnCallback(function (string $body) use (&$captured): StreamInterface { + $captured = $body; + return $this->stream; + }); + return $factory; + } + + /** + * Returns a fresh RequestInterface stub (NOT $this->request) whose + * withHeader() captures any "Authorization" value into $captured and + * returns itself. withBody() also returns itself so the production + * code's fluent chain (withHeader()->withHeader()->withBody()->...) + * resolves correctly. + * + * Using a fresh stub here — instead of re-stubbing $this->request from + * setUp() — avoids PHPUnit's matcher-fallthrough behaviour when the + * same method is configured twice on the same stub. + */ + private function captureAuthHeader(?string &$captured): RequestInterface + { + $request = $this->createStub(RequestInterface::class); + $request->method('withBody')->willReturn($request); + $request->method('withHeader') + ->willReturnCallback(function (string $name, string $value) use (&$captured, $request): RequestInterface { + if ($name === 'Authorization') { + $captured = $value; + } + return $request; + }); + return $request; + } + public function testRevokeTokenSendsPostToRevocationEndpoint(): void { $requestFactory = $this->createMock(RequestFactoryInterface::class); @@ -96,12 +137,7 @@ public function testRevokeTokenIncludesTokenInBodyWhenPostConfigured(): void $requestFactory->method('createRequest')->willReturn($this->request); $capturedBody = null; - $streamFactory = $this->createStub(StreamFactoryInterface::class); - $streamFactory->method('createStream') - ->willReturnCallback(function (string $body) use (&$capturedBody): StreamInterface { - $capturedBody = $body; - return $this->stream; - }); + $streamFactory = $this->captureStreamBody($capturedBody); $response = $this->createStub(ResponseInterface::class); $response->method('getStatusCode')->willReturn(200); @@ -133,12 +169,7 @@ public function testRevokeTokenWithDefaultHintIsAccessToken(): void $requestFactory->method('createRequest')->willReturn($this->request); $capturedBody = null; - $streamFactory = $this->createStub(StreamFactoryInterface::class); - $streamFactory->method('createStream') - ->willReturnCallback(function (string $body) use (&$capturedBody): StreamInterface { - $capturedBody = $body; - return $this->stream; - }); + $streamFactory = $this->captureStreamBody($capturedBody); $response = $this->createStub(ResponseInterface::class); $response->method('getStatusCode')->willReturn(200); @@ -167,12 +198,7 @@ public function testRevokeTokenWithoutClientSecretOmitsIt(): void $requestFactory->method('createRequest')->willReturn($this->request); $capturedBody = null; - $streamFactory = $this->createStub(StreamFactoryInterface::class); - $streamFactory->method('createStream') - ->willReturnCallback(function (string $body) use (&$capturedBody): StreamInterface { - $capturedBody = $body; - return $this->stream; - }); + $streamFactory = $this->captureStreamBody($capturedBody); $response = $this->createStub(ResponseInterface::class); $response->method('getStatusCode')->willReturn(200); @@ -242,32 +268,21 @@ public function testRevokeTokenThrowsOnErrorResponse(): void public function testRevokeTokenUsesBasicAuthWhenConfigured(): void { $provider = ProviderConfig::fromArray([ - 'issuer' => 'https://idp.example.org', - 'authorization_endpoint' => 'https://idp.example.org/authorize', - 'token_endpoint' => 'https://idp.example.org/token', - 'revocation_endpoint' => 'https://idp.example.org/revoke', - 'token_endpoint_auth_methods_supported' => ['client_secret_basic'], + 'issuer' => 'https://idp.example.org', + 'authorization_endpoint' => 'https://idp.example.org/authorize', + 'token_endpoint' => 'https://idp.example.org/token', + 'revocation_endpoint' => 'https://idp.example.org/revoke', + 'token_endpoint_auth_methods_supported' => ['client_secret_basic'], ]); - $requestFactory = $this->createStub(RequestFactoryInterface::class); - $requestFactory->method('createRequest')->willReturn($this->request); - - $capturedBody = null; $capturedAuth = null; - $this->request->method('withHeader') - ->willReturnCallback(function (string $name, string $value) use (&$capturedAuth): RequestInterface { - if ($name === 'Authorization') { - $capturedAuth = $value; - } - return $this->request; - }); + $request = $this->captureAuthHeader($capturedAuth); - $streamFactory = $this->createStub(StreamFactoryInterface::class); - $streamFactory->method('createStream') - ->willReturnCallback(function (string $body) use (&$capturedBody): StreamInterface { - $capturedBody = $body; - return $this->stream; - }); + $requestFactory = $this->createStub(RequestFactoryInterface::class); + $requestFactory->method('createRequest')->willReturn($request); + + $capturedBody = null; + $streamFactory = $this->captureStreamBody($capturedBody); $response = $this->createStub(ResponseInterface::class); $response->method('getStatusCode')->willReturn(200); @@ -308,13 +323,8 @@ public function testRevokeTokenUsesPostWhenBothMethodsSupported(): void $requestFactory = $this->createStub(RequestFactoryInterface::class); $requestFactory->method('createRequest')->willReturn($this->request); - $capturedBody = null; - $streamFactory = $this->createStub(StreamFactoryInterface::class); - $streamFactory->method('createStream') - ->willReturnCallback(function (string $body) use (&$capturedBody): StreamInterface { - $capturedBody = $body; - return $this->stream; - }); + $capturedBody = null; + $streamFactory = $this->captureStreamBody($capturedBody); $response = $this->createStub(ResponseInterface::class); $response->method('getStatusCode')->willReturn(200); From ae4025ce0221a2d120259bb7772b081448c877c1 Mon Sep 17 00:00:00 2001 From: Ralf Lang Date: Mon, 15 Jun 2026 11:28:55 +0200 Subject: [PATCH 3/3] feat(client): honour revocation_endpoint_auth_methods_supported override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New optional revocationEndpointAuthMethodsSupported field on ProviderConfig. As of RFC 8414 §2 OAuth2Client::revokeToken() uses it for client authentication selection when present or falls back to tokenEndpointAuthMethodsSupported per the spec's specified default. If both are missing, implies POST method. --- src/Client/OAuth2Client.php | 11 +++- src/Client/ProviderConfig.php | 19 +++++-- test/Client/OAuth2ClientTest.php | 87 ++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 7 deletions(-) diff --git a/src/Client/OAuth2Client.php b/src/Client/OAuth2Client.php index 53fc10f..32f3f9e 100644 --- a/src/Client/OAuth2Client.php +++ b/src/Client/OAuth2Client.php @@ -219,6 +219,10 @@ private function tokenRequest(array $params): TokenSet /** * Revoke a token at the provider's revocation endpoint (RFC 7009). * + * Client authentication uses revocation_endpoint_auth_methods_supported + * when advertised, falling back to token_endpoint_auth_methods_supported + * per RFC 8414 §2. + * * @param string $token The token to revoke (access or refresh). * @param string $tokenTypeHint 'access_token' or 'refresh_token'. * @throws OAuthException If the provider has no revocation endpoint @@ -236,10 +240,13 @@ public function revokeToken(string $token, string $tokenTypeHint = 'access_token 'client_id' => $this->clientId, ]; + $authMethods = $this->provider->revocationEndpointAuthMethodsSupported + ?? $this->provider->tokenEndpointAuthMethodsSupported; + $useBasic = false; if ($this->clientSecret !== null) { - $useBasic = in_array('client_secret_basic', $this->provider->tokenEndpointAuthMethodsSupported, true) - && !in_array('client_secret_post', $this->provider->tokenEndpointAuthMethodsSupported, true); + $useBasic = in_array('client_secret_basic', $authMethods, true) + && !in_array('client_secret_post', $authMethods, true); if (!$useBasic) { $params['client_secret'] = $this->clientSecret; diff --git a/src/Client/ProviderConfig.php b/src/Client/ProviderConfig.php index 2c46331..007c57f 100644 --- a/src/Client/ProviderConfig.php +++ b/src/Client/ProviderConfig.php @@ -16,11 +16,15 @@ final class ProviderConfig { /** - * @param string[] $scopesSupported - * @param string[] $responseTypesSupported - * @param string[] $grantTypesSupported - * @param string[] $tokenEndpointAuthMethodsSupported - * @param string[] $idTokenSigningAlgValuesSupported + * @param string[] $scopesSupported + * @param string[] $responseTypesSupported + * @param string[] $grantTypesSupported + * @param string[] $tokenEndpointAuthMethodsSupported + * @param string[]|null $revocationEndpointAuthMethodsSupported RFC 8414 + * §2 override for the revocation endpoint. Null means "not + * advertised; fall back to tokenEndpointAuthMethodsSupported per + * RFC 8414 §2's specified default." + * @param string[] $idTokenSigningAlgValuesSupported */ public function __construct( public readonly string $issuer, @@ -34,6 +38,7 @@ public function __construct( public readonly array $responseTypesSupported = ['code'], public readonly array $grantTypesSupported = [], public readonly array $tokenEndpointAuthMethodsSupported = ['client_secret_basic'], + public readonly ?array $revocationEndpointAuthMethodsSupported = null, public readonly array $idTokenSigningAlgValuesSupported = [], ) {} @@ -54,6 +59,9 @@ public static function fromArray(array $data): self responseTypesSupported: (array) ($data['response_types_supported'] ?? ['code']), grantTypesSupported: (array) ($data['grant_types_supported'] ?? []), tokenEndpointAuthMethodsSupported: (array) ($data['token_endpoint_auth_methods_supported'] ?? ['client_secret_basic']), + revocationEndpointAuthMethodsSupported: isset($data['revocation_endpoint_auth_methods_supported']) + ? (array) $data['revocation_endpoint_auth_methods_supported'] + : null, idTokenSigningAlgValuesSupported: (array) ($data['id_token_signing_alg_values_supported'] ?? []), ); } @@ -75,6 +83,7 @@ public function toArray(): array 'response_types_supported' => $this->responseTypesSupported, 'grant_types_supported' => $this->grantTypesSupported, 'token_endpoint_auth_methods_supported' => $this->tokenEndpointAuthMethodsSupported, + 'revocation_endpoint_auth_methods_supported' => $this->revocationEndpointAuthMethodsSupported, 'id_token_signing_alg_values_supported' => $this->idTokenSigningAlgValuesSupported, ], static fn($v) => $v !== null && $v !== []); } diff --git a/test/Client/OAuth2ClientTest.php b/test/Client/OAuth2ClientTest.php index 61f7185..49097c7 100644 --- a/test/Client/OAuth2ClientTest.php +++ b/test/Client/OAuth2ClientTest.php @@ -346,4 +346,91 @@ public function testRevokeTokenUsesPostWhenBothMethodsSupported(): void self::assertStringContainsString('client_secret=secret', $capturedBody); } + + public function testRevokeTokenPrefersRevocationAuthMethodsWhenAdvertised(): void + { + // Token endpoint advertises post; revocation endpoint advertises + // basic only. The override must win for revokeToken(). + $provider = ProviderConfig::fromArray([ + 'issuer' => 'https://idp.example.org', + 'authorization_endpoint' => 'https://idp.example.org/authorize', + 'token_endpoint' => 'https://idp.example.org/token', + 'revocation_endpoint' => 'https://idp.example.org/revoke', + 'token_endpoint_auth_methods_supported' => ['client_secret_post'], + 'revocation_endpoint_auth_methods_supported' => ['client_secret_basic'], + ]); + + $capturedAuth = null; + $request = $this->captureAuthHeader($capturedAuth); + + $requestFactory = $this->createStub(RequestFactoryInterface::class); + $requestFactory->method('createRequest')->willReturn($request); + + $capturedBody = null; + $streamFactory = $this->captureStreamBody($capturedBody); + + $response = $this->createStub(ResponseInterface::class); + $response->method('getStatusCode')->willReturn(200); + + $httpClient = $this->createStub(ClientInterface::class); + $httpClient->method('sendRequest')->willReturn($response); + + $client = new OAuth2Client( + provider: $provider, + clientId: 'test-client', + clientSecret: 'secret', + redirectUri: 'https://horde.example.org/callback', + httpClient: $httpClient, + requestFactory: $requestFactory, + streamFactory: $streamFactory, + ); + + $client->revokeToken('my-token'); + + self::assertStringNotContainsString('client_secret', $capturedBody); + self::assertStringStartsWith('Basic ', $capturedAuth); + } + + public function testRevokeTokenFallsBackToTokenAuthMethodsWhenRevocationNotAdvertised(): void + { + // Revocation endpoint advertises no auth methods. Per RFC 8414 §2, + // fall back to the token endpoint's advertised methods. + $provider = ProviderConfig::fromArray([ + 'issuer' => 'https://idp.example.org', + 'authorization_endpoint' => 'https://idp.example.org/authorize', + 'token_endpoint' => 'https://idp.example.org/token', + 'revocation_endpoint' => 'https://idp.example.org/revoke', + 'token_endpoint_auth_methods_supported' => ['client_secret_basic'], + ]); + + $capturedAuth = null; + $request = $this->captureAuthHeader($capturedAuth); + + $requestFactory = $this->createStub(RequestFactoryInterface::class); + $requestFactory->method('createRequest')->willReturn($request); + + $capturedBody = null; + $streamFactory = $this->captureStreamBody($capturedBody); + + $response = $this->createStub(ResponseInterface::class); + $response->method('getStatusCode')->willReturn(200); + + $httpClient = $this->createStub(ClientInterface::class); + $httpClient->method('sendRequest')->willReturn($response); + + $client = new OAuth2Client( + provider: $provider, + clientId: 'test-client', + clientSecret: 'secret', + redirectUri: 'https://horde.example.org/callback', + httpClient: $httpClient, + requestFactory: $requestFactory, + streamFactory: $streamFactory, + ); + + $client->revokeToken('my-token'); + + self::assertStringNotContainsString('client_secret', $capturedBody); + self::assertStringStartsWith('Basic ', $capturedAuth); + } }