Support empty or null access tokens (#216)

* Explicitly handle null access token * Add tests for empty and null access token * Use ternary operator
googleapis · Sep 11, 2018 · ba7bac8 · ba7bac8
1 parent fe157fe
commit ba7bac8
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 17 deletions.
diff --git a/src/ApiCore/CredentialsWrapper.php b/src/ApiCore/CredentialsWrapper.php
@@ -141,7 +141,8 @@ public static function build(array $args = [])
      */
     public function getBearerString()
     {
-        return 'Bearer ' . self::getToken($this->credentialsFetcher, $this->authHttpHandler);
+        $token = self::getToken($this->credentialsFetcher, $this->authHttpHandler);
+        return empty($token) ? '' : "Bearer $token";
     }
 
     /**
@@ -156,7 +157,8 @@ public function getAuthorizationHeaderCallback()
         // be passed into the gRPC c extension, and changes have the potential to trigger very
         // difficult-to-diagnose segmentation faults.
         return function () use ($credentialsFetcher, $authHttpHandler) {
-            return ['authorization' => ['Bearer ' . self::getToken($credentialsFetcher, $authHttpHandler)]];
+            $token = self::getToken($credentialsFetcher, $authHttpHandler);
+            return empty($token) ? [] : ['authorization' => ["Bearer $token"]];
         };
     }
 

diff --git a/src/ApiCore/GapicClientTrait.php b/src/ApiCore/GapicClientTrait.php
@@ -315,28 +315,28 @@ private function setClientOptions(array $options)
     }
 
     /**
-     * @param mixed $auth
-     * @param array $authConfig
+     * @param mixed $credentials
+     * @param array $credentialsConfig
      * @return CredentialsWrapper
      * @throws ValidationException
      */
-    private function createCredentialsWrapper($auth, array $authConfig)
+    private function createCredentialsWrapper($credentials, array $credentialsConfig)
     {
-        if (is_null($auth)) {
-            return CredentialsWrapper::build($authConfig);
-        } elseif (is_string($auth) || is_array($auth)) {
-            return CredentialsWrapper::build(['keyFile' => $auth] + $authConfig);
-        } elseif ($auth instanceof FetchAuthTokenInterface) {
-            $authHttpHandler = isset($authConfig['authHttpHandler'])
-                ? $authConfig['authHttpHandler']
+        if (is_null($credentials)) {
+            return CredentialsWrapper::build($credentialsConfig);
+        } elseif (is_string($credentials) || is_array($credentials)) {
+            return CredentialsWrapper::build(['keyFile' => $credentials] + $credentialsConfig);
+        } elseif ($credentials instanceof FetchAuthTokenInterface) {
+            $authHttpHandler = isset($credentialsConfig['authHttpHandler'])
+                ? $credentialsConfig['authHttpHandler']
                 : null;
-            return new CredentialsWrapper($auth, $authHttpHandler);
-        } elseif ($auth instanceof CredentialsWrapper) {
-            return $auth;
+            return new CredentialsWrapper($credentials, $authHttpHandler);
+        } elseif ($credentials instanceof CredentialsWrapper) {
+            return $credentials;
         } else {
             throw new ValidationException(
                 'Unexpected value in $auth option, got: ' .
-                print_r($auth, true)
+                print_r($credentials, true)
             );
         }
     }

diff --git a/src/ApiCore/Transport/HttpUnaryTransportTrait.php b/src/ApiCore/Transport/HttpUnaryTransportTrait.php
@@ -92,7 +92,10 @@ private static function buildCommonHeaders(array $options)
 
         // If not already set, add an auth header to the request
         if (!isset($headers['Authorization']) && isset($options['credentialsWrapper'])) {
-            $headers['Authorization'] = $options['credentialsWrapper']->getBearerString();
+            $bearerString = $options['credentialsWrapper']->getBearerString();
+            if (!empty($bearerString)) {
+                $headers['Authorization'] = $bearerString;
+            }
         }
 
         return $headers;

diff --git a/tests/ApiCore/Tests/Unit/CredentialsWrapperTest.php b/tests/ApiCore/Tests/Unit/CredentialsWrapperTest.php
@@ -179,9 +179,23 @@ public function getBearerStringData()
                 'access_token' => 123,
                 'expires_at' => time() + 100,
             ]);
+        $insecureFetcher = $this->prophesize(FetchAuthTokenInterface::class);
+        $insecureFetcher->getLastReceivedToken()->willReturn(null);
+        $insecureFetcher->fetchAuthToken(Argument::any())
+            ->willReturn([
+                'access_token' => '',
+            ]);
+        $nullFetcher = $this->prophesize(FetchAuthTokenInterface::class);
+        $nullFetcher->getLastReceivedToken()->willReturn(null);
+        $nullFetcher->fetchAuthToken(Argument::any())
+            ->willReturn([
+                'access_token' => null,
+            ]);
         return [
             [$expiredFetcher->reveal(), 'Bearer 456'],
             [$unexpiredFetcher->reveal(), 'Bearer 123'],
+            [$insecureFetcher->reveal(), ''],
+            [$nullFetcher->reveal(), '']
         ];
     }
 
@@ -215,9 +229,24 @@ public function getAuthorizationHeaderCallbackData()
                 'access_token' => 123,
                 'expires_at' => time() + 100,
             ]);
+
+        $insecureFetcher = $this->prophesize(FetchAuthTokenInterface::class);
+        $insecureFetcher->getLastReceivedToken()->willReturn(null);
+        $insecureFetcher->fetchAuthToken(Argument::any())
+            ->willReturn([
+                'access_token' => '',
+            ]);
+        $nullFetcher = $this->prophesize(FetchAuthTokenInterface::class);
+        $nullFetcher->getLastReceivedToken()->willReturn(null);
+        $nullFetcher->fetchAuthToken(Argument::any())
+            ->willReturn([
+                'access_token' => null,
+            ]);
         return [
             [$expiredFetcher->reveal(), ['authorization' => ['Bearer 456']]],
             [$unexpiredFetcher->reveal(), ['authorization' => ['Bearer 123']]],
+            [$insecureFetcher->reveal(), []],
+            [$nullFetcher->reveal(), []],
         ];
     }
 }