Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support empty or null access tokens #216

Merged
merged 3 commits into from
Sep 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/ApiCore/CredentialsWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}

/**
Expand All @@ -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"]];
};
}

Expand Down
28 changes: 14 additions & 14 deletions src/ApiCore/GapicClientTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
}
}
Expand Down
5 changes: 4 additions & 1 deletion src/ApiCore/Transport/HttpUnaryTransportTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
29 changes: 29 additions & 0 deletions tests/ApiCore/Tests/Unit/CredentialsWrapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(), '']
];
}

Expand Down Expand Up @@ -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(), []],
];
}
}