From 6d426b5cb9462845d2c2d7d506318c9bee613528 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Tue, 10 Oct 2023 10:52:44 -0700 Subject: [PATCH] feat: respect cache control for access token certs (#479) --- src/AccessToken.php | 31 +++++++++++++------ tests/AccessTokenTest.php | 64 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 82 insertions(+), 13 deletions(-) diff --git a/src/AccessToken.php b/src/AccessToken.php index e1f92ee7e..0afc4ca1e 100644 --- a/src/AccessToken.php +++ b/src/AccessToken.php @@ -311,11 +311,9 @@ private function getCerts($location, $cacheKey, array $options = []) $cacheItem = $this->cache->getItem($cacheKey); $certs = $cacheItem ? $cacheItem->get() : null; - $gotNewCerts = false; + $expireTime = null; if (!$certs) { - $certs = $this->retrieveCertsFromLocation($location, $options); - - $gotNewCerts = true; + list($certs, $expireTime) = $this->retrieveCertsFromLocation($location, $options); } if (!isset($certs['keys'])) { @@ -331,8 +329,8 @@ private function getCerts($location, $cacheKey, array $options = []) // Push caching off until after verifying certs are in a valid format. // Don't want to cache bad data. - if ($gotNewCerts) { - $cacheItem->expiresAt(new DateTime('+1 hour')); + if ($expireTime) { + $cacheItem->expiresAt(new DateTime($expireTime)); $cacheItem->set($certs); $this->cache->save($cacheItem); } @@ -345,13 +343,14 @@ private function getCerts($location, $cacheKey, array $options = []) * * @param string $url location * @param array $options [optional] Configuration options. - * @return array certificates + * @return array{array, string} * @throws InvalidArgumentException If certs could not be retrieved from a local file. * @throws RuntimeException If certs could not be retrieved from a remote location. */ private function retrieveCertsFromLocation($url, array $options = []) { // If we're retrieving a local file, just grab it. + $expireTime = '+1 hour'; if (strpos($url, 'http') !== 0) { if (!file_exists($url)) { throw new InvalidArgumentException(sprintf( @@ -360,14 +359,28 @@ private function retrieveCertsFromLocation($url, array $options = []) )); } - return json_decode((string) file_get_contents($url), true); + return [ + json_decode((string) file_get_contents($url), true), + $expireTime + ]; } $httpHandler = $this->httpHandler; $response = $httpHandler(new Request('GET', $url), $options); if ($response->getStatusCode() == 200) { - return json_decode((string) $response->getBody(), true); + if ($cacheControl = $response->getHeaderLine('Cache-Control')) { + array_map(function ($value) use (&$expireTime) { + list($key, $value) = explode('=', $value) + [null, null]; + if (trim($key) == 'max-age') { + $expireTime = '+' . $value . ' seconds'; + } + }, explode(',', $cacheControl)); + } + return [ + json_decode((string) $response->getBody(), true), + $expireTime + ]; } throw new RuntimeException(sprintf( diff --git a/tests/AccessTokenTest.php b/tests/AccessTokenTest.php index f2a6f3c81..de51474b2 100644 --- a/tests/AccessTokenTest.php +++ b/tests/AccessTokenTest.php @@ -264,7 +264,10 @@ public function testEsVerifyEndToEnd() $this->assertEquals('https://cloud.google.com/iap', $payload['iss']); } - public function testGetCertsForIap() + /** + * @dataProvider provideCertsFromUrl + */ + public function testGetCertsFromUrl($certUrl) { $token = new AccessToken(); $reflector = new \ReflectionObject($token); @@ -272,14 +275,22 @@ public function testGetCertsForIap() $cacheKeyMethod->setAccessible(true); $getCertsMethod = $reflector->getMethod('getCerts'); $getCertsMethod->setAccessible(true); - $cacheKey = $cacheKeyMethod->invoke($token, AccessToken::IAP_CERT_URL); + $cacheKey = $cacheKeyMethod->invoke($token, $certUrl); $certs = $getCertsMethod->invoke( $token, - AccessToken::IAP_CERT_URL, + $certUrl, $cacheKey ); $this->assertTrue(is_array($certs)); - $this->assertEquals(5, count($certs)); + $this->assertGreaterThanOrEqual(2, count($certs)); + } + + public function provideCertsFromUrl() + { + return [ + [AccessToken::IAP_CERT_URL], + [AccessToken::FEDERATED_SIGNON_CERT_URL], + ]; } public function testRetrieveCertsFromLocationLocalFile() @@ -398,6 +409,51 @@ public function testRetrieveCertsFromLocationLocalFileInvalidFileData() ]); } + public function testRetrieveCertsFromLocationRespectsCacheControl() + { + $certsLocation = __DIR__ . '/fixtures/federated-certs.json'; + $certsJson = file_get_contents($certsLocation); + $certsData = json_decode($certsJson, true); + + $httpHandler = function (RequestInterface $request) use ($certsJson) { + return new Response(200, [ + 'cache-control' => 'public, max-age=1000', + ], $certsJson); + }; + + $phpunit = $this; + + $item = $this->prophesize('Psr\Cache\CacheItemInterface'); + $item->get() + ->shouldBeCalledTimes(1) + ->willReturn(null); + $item->set($certsData) + ->shouldBeCalledTimes(1) + ->willReturn($item->reveal()); + + // Assert date-time is set with difference of 1000 (the max-age in the Cache-Control header) + $item->expiresAt(Argument::type('\DateTime')) + ->shouldBeCalledTimes(1) + ->will(function ($value) use ($phpunit) { + $phpunit->assertEqualsWithDelta(1000, $value[0]->getTimestamp() - time(), 1); + return $this; + }); + + $this->cache->getItem('google_auth_certs_cache|federated_signon_certs_v3') + ->shouldBeCalledTimes(1) + ->willReturn($item->reveal()); + + $this->cache->save(Argument::type('Psr\Cache\CacheItemInterface')) + ->shouldBeCalledTimes(1); + + $token = new AccessTokenStub( + $httpHandler, + $this->cache->reveal() + ); + + $token->verify($this->token); + } + public function testRetrieveCertsFromLocationRemote() { $certsLocation = __DIR__ . '/fixtures/federated-certs.json';