From 3d68b6d5161900abb873f252f4106f9a6aa2a230 Mon Sep 17 00:00:00 2001 From: Yash Sahu <54198301+yash30201@users.noreply.github.com> Date: Wed, 15 Nov 2023 08:47:17 +0000 Subject: [PATCH] chore: refactor AuthTokenMiddleware logic (#492) --- src/Middleware/AuthTokenMiddleware.php | 47 ++++---- tests/FetchAuthTokenTest.php | 17 ++- tests/Middleware/AuthTokenMiddlewareTest.php | 109 +++++++++++++------ 3 files changed, 119 insertions(+), 54 deletions(-) diff --git a/src/Middleware/AuthTokenMiddleware.php b/src/Middleware/AuthTokenMiddleware.php index 1e2f7fb6d..b10cf9bff 100644 --- a/src/Middleware/AuthTokenMiddleware.php +++ b/src/Middleware/AuthTokenMiddleware.php @@ -17,8 +17,11 @@ namespace Google\Auth\Middleware; +use Google\Auth\FetchAuthTokenCache; use Google\Auth\FetchAuthTokenInterface; use Google\Auth\GetQuotaProjectInterface; +use Google\Auth\UpdateMetadataInterface; +use GuzzleHttp\Psr7\Utils; use Psr\Http\Message\RequestInterface; /** @@ -40,6 +43,9 @@ class AuthTokenMiddleware private $httpHandler; /** + * It must be an implementation of FetchAuthTokenInterface. + * It may also implement UpdateMetadataInterface allowing direct + * retrieval of auth related headers * @var FetchAuthTokenInterface */ private $fetcher; @@ -99,7 +105,7 @@ public function __invoke(callable $handler) return $handler($request, $options); } - $request = $request->withHeader('authorization', 'Bearer ' . $this->fetchToken()); + $request = $this->addAuthHeaders($request); if ($quotaProject = $this->getQuotaProject()) { $request = $request->withHeader( @@ -113,32 +119,33 @@ public function __invoke(callable $handler) } /** - * Call fetcher to fetch the token. + * Adds auth related headers to the request. * - * @return string|null + * @param RequestInterface $request + * @return RequestInterface */ - private function fetchToken() + private function addAuthHeaders(RequestInterface $request) { - $auth_tokens = (array) $this->fetcher->fetchAuthToken($this->httpHandler); - - if (array_key_exists('access_token', $auth_tokens)) { - // notify the callback if applicable - if ($this->tokenCallback) { - call_user_func( - $this->tokenCallback, - $this->fetcher->getCacheKey(), - $auth_tokens['access_token'] - ); - } - - return $auth_tokens['access_token']; + if (!$this->fetcher instanceof UpdateMetadataInterface || + ($this->fetcher instanceof FetchAuthTokenCache && + !$this->fetcher->getFetcher() instanceof UpdateMetadataInterface) + ) { + $token = $this->fetcher->fetchAuthToken(); + $request = $request->withHeader( + 'authorization', 'Bearer ' . ($token['access_token'] ?? $token['id_token']) + ); + } else { + $headers = $this->fetcher->updateMetadata($request->getHeaders(), null, $this->httpHandler); + $request = Utils::modifyRequest($request, ['set_headers' => $headers]); } - if (array_key_exists('id_token', $auth_tokens)) { - return $auth_tokens['id_token']; + if ($this->tokenCallback && ($token = $this->fetcher->getLastReceivedToken())) { + if (array_key_exists('access_token', $token)) { + call_user_func($this->tokenCallback, $this->fetcher->getCacheKey(), $token['access_token']); + } } - return null; + return $request; } /** diff --git a/tests/FetchAuthTokenTest.php b/tests/FetchAuthTokenTest.php index 5b7badbe0..6fe7df242 100644 --- a/tests/FetchAuthTokenTest.php +++ b/tests/FetchAuthTokenTest.php @@ -25,6 +25,7 @@ use Google\Auth\CredentialsLoader; use Google\Auth\FetchAuthTokenInterface; use Google\Auth\OAuth2; +use Google\Auth\UpdateMetadataInterface; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; @@ -53,10 +54,20 @@ class_implements($fetcherClass) )) { $mockFetcher->getQuotaProject()->shouldBeCalledTimes(1); } - $mockFetcher->fetchAuthToken(Argument::any()) - ->shouldBeCalledTimes(1) - ->will($httpHandler); + + if (is_a($fetcherClass, UpdateMetadataInterface::class, true)) { + $mockFetcher->updateMetadata(Argument::cetera()) + ->shouldBeCalledTimes(1)->will(function () use (&$httpHandlerCalled) { + $httpHandlerCalled = true; + return ['authorization' => ['Bearer xyz']]; + }); + } else { + $mockFetcher->fetchAuthToken(Argument::any()) + ->shouldBeCalledTimes(1) + ->will($httpHandler); + } $mockFetcher->getCacheKey()->willReturn(''); + $mockFetcher->getLastReceivedToken()->willReturn(['access_token' => 'xyz']); $tokenCallbackCalled = false; $tokenCallback = function ($cacheKey, $accessToken) use (&$tokenCallbackCalled) { diff --git a/tests/Middleware/AuthTokenMiddlewareTest.php b/tests/Middleware/AuthTokenMiddlewareTest.php index 06c6ee485..cf0eb4182 100644 --- a/tests/Middleware/AuthTokenMiddlewareTest.php +++ b/tests/Middleware/AuthTokenMiddlewareTest.php @@ -20,7 +20,9 @@ use Google\Auth\FetchAuthTokenCache; use Google\Auth\Middleware\AuthTokenMiddleware; use Google\Auth\Tests\BaseTest; +use Google\Auth\UpdateMetadataInterface; use GuzzleHttp\Handler\MockHandler; +use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; @@ -64,11 +66,7 @@ public function testAddsTheTokenAsAnAuthorizationHeader() ->shouldBeCalledTimes(1) ->willReturn($this->mockRequest->reveal()); - // Run the test. - $middleware = new AuthTokenMiddleware($this->mockFetcher->reveal()); - $mock = new MockHandler([new Response(200)]); - $callable = $middleware($mock); - $callable($this->mockRequest->reveal(), ['auth' => 'google_auth']); + $this->runTestCase($this->mockFetcher->reveal()); } public function testDoesNotAddAnAuthorizationHeaderOnNoAccessToken() @@ -80,11 +78,7 @@ public function testDoesNotAddAnAuthorizationHeaderOnNoAccessToken() $this->mockRequest->withHeader('authorization', 'Bearer ') ->willReturn($this->mockRequest->reveal()); - // Run the test. - $middleware = new AuthTokenMiddleware($this->mockFetcher->reveal()); - $mock = new MockHandler([new Response(200)]); - $callable = $middleware($mock); - $callable($this->mockRequest->reveal(), ['auth' => 'google_auth']); + $this->runTestCase($this->mockFetcher->reveal()); } public function testUsesIdTokenWhenAccessTokenDoesNotExist() @@ -96,12 +90,10 @@ public function testUsesIdTokenWhenAccessTokenDoesNotExist() ->willReturn($authResult); $this->mockRequest->withHeader('authorization', 'Bearer ' . $token) ->shouldBeCalledTimes(1) - ->willReturn($this->mockRequest); + ->willReturn($this->mockRequest->reveal()); + + $this->runTestCase($this->mockFetcher->reveal()); - $middleware = new AuthTokenMiddleware($this->mockFetcher->reveal()); - $mock = new MockHandler([new Response(200)]); - $callable = $middleware($mock); - $callable($this->mockRequest->reveal(), ['auth' => 'google_auth']); } public function testUsesCachedAccessToken() @@ -133,10 +125,7 @@ public function testUsesCachedAccessToken() null, $this->mockCache->reveal() ); - $middleware = new AuthTokenMiddleware($cachedFetcher); - $mock = new MockHandler([new Response(200)]); - $callable = $middleware($mock); - $callable($this->mockRequest->reveal(), ['auth' => 'google_auth']); + $this->runTestCase($cachedFetcher); } public function testUsesCachedIdToken() @@ -168,10 +157,7 @@ public function testUsesCachedIdToken() null, $this->mockCache->reveal() ); - $middleware = new AuthTokenMiddleware($cachedFetcher); - $mock = new MockHandler([new Response(200)]); - $callable = $middleware($mock); - $callable($this->mockRequest->reveal(), ['auth' => 'google_auth']); + $this->runTestCase($cachedFetcher); } public function testGetsCachedAuthTokenUsingCacheOptions() @@ -204,10 +190,7 @@ public function testGetsCachedAuthTokenUsingCacheOptions() ['prefix' => $prefix], $this->mockCache->reveal() ); - $middleware = new AuthTokenMiddleware($cachedFetcher); - $mock = new MockHandler([new Response(200)]); - $callable = $middleware($mock); - $callable($this->mockRequest->reveal(), ['auth' => 'google_auth']); + $this->runTestCase($cachedFetcher); } public function testShouldSaveValueInCacheWithSpecifiedPrefix() @@ -248,10 +231,7 @@ public function testShouldSaveValueInCacheWithSpecifiedPrefix() ['prefix' => $prefix, 'lifetime' => $lifetime], $this->mockCache->reveal() ); - $middleware = new AuthTokenMiddleware($cachedFetcher); - $mock = new MockHandler([new Response(200)]); - $callable = $middleware($mock); - $callable($this->mockRequest->reveal(), ['auth' => 'google_auth']); + $this->runTestCase($cachedFetcher); } /** @@ -282,6 +262,8 @@ public function testShouldNotifyTokenCallback(callable $tokenCallback) $this->mockFetcher->fetchAuthToken(Argument::any()) ->shouldBeCalledTimes(1) ->willReturn($cachedValue); + $this->mockFetcher->getLastReceivedToken() + ->willReturn($cachedValue); $this->mockRequest->withHeader(Argument::any(), Argument::any()) ->willReturn($this->mockRequest->reveal()); @@ -306,6 +288,71 @@ public function testShouldNotifyTokenCallback(callable $tokenCallback) $this->assertTrue(MiddlewareCallback::$called); } + public function testAddAuthHeadersFromUpdateMetadata() + { + $authResult = [ + 'authorization' => 'Bearer 1/abcdef1234567890', + ]; + + $this->mockFetcher->willImplement(UpdateMetadataInterface::class); + $this->mockFetcher->updateMetadata(Argument::cetera()) + ->shouldBeCalledTimes(1) + ->willReturn($authResult); + $this->mockFetcher->getLastReceivedToken() + ->willReturn(['access_token' => '1/abcdef1234567890']); + + $request = new Request('GET', 'http://foo.com'); + + $middleware = new AuthTokenMiddleware($this->mockFetcher->reveal()); + $mockHandlerCalled = false; + $mock = new MockHandler([function ($request, $options) use ($authResult, &$mockHandlerCalled) { + $this->assertEquals($authResult['authorization'], $request->getHeaderLine('authorization')); + $mockHandlerCalled = true; + return new Response(200); + }]); + $callable = $middleware($mock); + $callable($request, ['auth' => 'google_auth']); + $this->assertTrue($mockHandlerCalled); + } + + public function testOverlappingAddAuthHeadersFromUpdateMetadata() + { + $authHeaders = [ + 'authorization' => 'Bearer 1/abcdef1234567890', + 'x-goog-api-client' => 'extra-value' + ]; + + $request = new Request('GET', 'http://foo.com'); + + $this->mockFetcher->willImplement(UpdateMetadataInterface::class); + $this->mockFetcher->updateMetadata(Argument::cetera()) + ->shouldBeCalledTimes(1) + ->willReturn($authHeaders); + $this->mockFetcher->getLastReceivedToken() + ->willReturn(['access_token' => '1/abcdef1234567890']); + + $middleware = new AuthTokenMiddleware($this->mockFetcher->reveal()); + + $mockHandlerCalled = false; + $mock = new MockHandler([function ($request, $options) use ($authHeaders, &$mockHandlerCalled) { + $this->assertEquals($authHeaders['authorization'], $request->getHeaderLine('authorization')); + $this->assertArrayHasKey('x-goog-api-client', $request->getHeaders()); + $mockHandlerCalled = true; + return new Response(200); + }]); + $callable = $middleware($mock); + $callable($request, ['auth' => 'google_auth']); + $this->assertTrue($mockHandlerCalled); + } + + private function runTestCase($fetcher) + { + $middleware = new AuthTokenMiddleware($fetcher); + $mock = new MockHandler([new Response(200)]); + $callable = $middleware($mock); + $callable($this->mockRequest->reveal(), ['auth' => 'google_auth']); + } + public function provideShouldNotifyTokenCallback() { MiddlewareCallback::$phpunit = $this;