cooldown for failed lookups/responses

Author: bshaffer

src/TrustBoundaryTrait.php

@@ -67,10 +67,6 @@ private function getTrustBoundary(
             return null;
         }
 
-        if (!array_key_exists('encodedLocations', $trustBoundary)) {
-            throw new \LogicException('Trust boundary lookup failed to return \'encodedLocations\'');
-        }
-
         // Save to cache
         $tbLifetime = 6 * 60 * 60; // 6-hour cache TTL
         $this->setCachedValue($this->getCacheKey() . ':trustboundary', $trustBoundary, $tbLifetime);
@@ -156,13 +152,35 @@ private function lookupTrustBoundary(
         $request = $request->withHeader('authorization', $authHeader);
         try {
             $response = $httpHandler($request);
-            return json_decode((string) $response->getBody(), true);
         } catch (RequestException $e) {
-            // We swallow all errors here - a failed trust boundary lookup
+            // An HTTP error occurred while requesting the trust boundary lookup
+            // We swallow all errors here as a failed trust boundary lookup
+            // should not disrupt client authentication.
+            //@TODO Add debug logging
+            $this->initiateCooldown();
+            return null;
+        }
+
+        $trustBoundary = json_decode((string) $response->getBody(), true);
+        if (null === $trustBoundary) {
+            // An error occurred during the JSON parsing of the request body
+            // We swallow all errors here as a failed trust boundary lookup
             // should not disrupt client authentication.
+            //@TODO Add debug logging
             $this->initiateCooldown();
+            return null;
         }
-        return null;
+
+        if (!array_key_exists('encodedLocations', $trustBoundary)) {
+            // The JSON response did not contain expected "allowLocations"
+            // We swallow all errors here as a failed trust boundary lookup
+            // should not disrupt client authentication.
+            //@TODO Add debug logging
+            $this->initiateCooldown();
+            return null;
+        }
+
+        return $trustBoundary;
     }
 
     private function initiateCooldown(): void

tests/ApplicationDefaultCredentialsTest.php

@@ -908,10 +908,7 @@ public function testUniverseDomainInGceCredentials()
     public function testTrustBoundaryLookupIntegration()
     {
         if ('true' !== getenv('RUN_TRUST_BOUNDARY_TESTS')) {
-            $this->markTestSkipped(
-                'This test requires RUN_TRUST_BOUNDARY_TESTS=true and a set of credentials with ' .
-                'trust boundaries enabled'
-            );
+            $this->markTestSkipped('This test requires RUN_TRUST_BOUNDARY_TESTS=true');
         }
 
         $creds = ApplicationDefaultCredentials::getCredentials(

tests/TrustBoundaryTraitTest.php

@@ -224,7 +224,7 @@ public function provideCooldown()
     /**
      * @dataProvider provideCooldown
      */
-    public function testCooldown(int $attempt, int $expectedExpiry)
+    public function testInitiateCooldown(int $attempt, int $expectedExpiry)
     {
         $cache = $this->prophesize(CacheItemPoolInterface::class);
 
@@ -274,6 +274,36 @@ public function testCooldown(int $attempt, int $expectedExpiry)
 
         $this->assertNull($result1);
     }
+
+    public function provideMalformedResponseFromAllowLocationsLookup()
+    {
+        return [
+            [200, '{"locations": ["us-west1"]}'], // missing allowLocations
+            [200, '{"locations": ["us-west1"]'],  // invalid JSON
+            [401, ''],                            // 4xx error
+            [500, ''],                            // 5xx error
+        ];
+    }
+
+    /**
+     * @dataProvider provideMalformedResponseFromAllowLocationsLookup
+     */
+    public function testMalformedResponseFromAllowLocationsLookup(int $statusCode, string $responseBody)
+    {
+        $this->impl->setCache(new MemoryCacheItemPool());
+        $handler = getHandler([
+            new Response($statusCode, [], $responseBody),
+        ]);
+        $result = $this->impl->getTrustBoundary(
+            GetUniverseDomainInterface::DEFAULT_UNIVERSE_DOMAIN,
+            $handler,
+            'default',
+            ['authorization' => ['xyz']]
+        );
+
+        $this->assertNull($result);
+        $this->assertTrue($this->impl->cooldownIsActive());
+    }
 }
 
 class TrustBoundaryTraitImpl
@@ -305,4 +335,9 @@ public function setCache($cache)
     {
         $this->cache = $cache;
     }
+
+    public function cooldownIsActive(): bool
+    {
+        return (bool) $this->getCachedValue($this->getCacheKey() . ':trustboundary:cooldown');
+    }
 }