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

feat: allow ServiceAccountJwtAccessCredentials to sign scopes #341

Merged
merged 7 commits into from
Jun 22, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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
70 changes: 62 additions & 8 deletions src/Credentials/ServiceAccountCredentials.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,16 @@ class ServiceAccountCredentials extends CredentialsLoader implements
*/
private $lastReceivedJwtAccessToken;

/*
* @var bool
*/
private $useJwtAccessWithScope;
bshaffer marked this conversation as resolved.
Show resolved Hide resolved

/*
* @var ServiceAccountJwtAccessCredentials|null
*/
private $jwtAccessCredentials;

/**
* Create a new ServiceAccountCredentials.
*
Expand Down Expand Up @@ -153,6 +163,18 @@ public function __construct(
: null;
}

/**
* When called, the ServiceAccountCredentials will use an instance of
* ServiceAccountJwtAccessCredentials to fetch (self-sign) an access token
* even when only scopes are supplied. Otherwise,
* ServiceAccountJwtAccessCredentials is only called when no scopes and an
* authUrl (audience) is suppled.
*/
public function useJwtAccessWithScope()
{
$this->useJwtAccessWithScope = true;
}

/**
* @param callable $httpHandler
*
Expand All @@ -164,6 +186,18 @@ public function __construct(
*/
public function fetchAuthToken(callable $httpHandler = null)
{
if ($this->useJwtAccessWithScope) {
$jwtCreds = $this->createJwtAccessCredentials();

$accessToken = $jwtCreds->fetchAuthToken($httpHandler);

if ($lastReceivedToken = $jwtCreds->getLastReceivedToken()) {
// Keep self-signed JWTs in memory as the last received token
$this->lastReceivedJwtAccessToken = $lastReceivedToken;
}

return $accessToken;
}
return $this->auth->fetchAuthToken($httpHandler);
}

Expand Down Expand Up @@ -223,14 +257,13 @@ public function updateMetadata(
return parent::updateMetadata($metadata, $authUri, $httpHandler);
}

// no scope found. create jwt with the auth uri
$credJson = array(
'private_key' => $this->auth->getSigningKey(),
'client_email' => $this->auth->getIssuer(),
);
$jwtCreds = new ServiceAccountJwtAccessCredentials($credJson);

$updatedMetadata = $jwtCreds->updateMetadata($metadata, $authUri, $httpHandler);
$jwtCreds = $this->createJwtAccessCredentials();
if ($this->auth->getScope()) {
// Prefer user-provided "scope" to "audience"
$updatedMetadata = $jwtCreds->updateMetadata($metadata, null, $httpHandler);
} else {
$updatedMetadata = $jwtCreds->updateMetadata($metadata, $authUri, $httpHandler);
}

if ($lastReceivedToken = $jwtCreds->getLastReceivedToken()) {
// Keep self-signed JWTs in memory as the last received token
Expand All @@ -240,6 +273,23 @@ public function updateMetadata(
return $updatedMetadata;
}

private function createJwtAccessCredentials()
{
if (!$this->jwtAccessCredentials) {
// Create credentials for self-signing a JWT (JwtAccess)
$credJson = array(
'private_key' => $this->auth->getSigningKey(),
'client_email' => $this->auth->getIssuer(),
);
$this->jwtAccessCredentials = new ServiceAccountJwtAccessCredentials(
$credJson,
$this->auth->getScope()
);
}

return $this->jwtAccessCredentials;
}

/**
* @param string $sub an email address account to impersonate, in situations when
* the service account has been delegated domain wide access.
Expand Down Expand Up @@ -274,6 +324,10 @@ public function getQuotaProject()

private function useSelfSignedJwt()
{
// When true, ServiceAccountCredentials will always use JwtAccess
if ($this->useJwtAccessWithScope) {
return true;
}
return is_null($this->auth->getScope());
}
}
17 changes: 14 additions & 3 deletions src/Credentials/ServiceAccountJwtAccessCredentials.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,10 @@ class ServiceAccountJwtAccessCredentials extends CredentialsLoader implements
*
* @param string|array $jsonKey JSON credential file path or JSON credentials
* as an associative array
* @param string|array $scope the scope of the access request, expressed
* either as an Array or as a space-delimited String.
*/
public function __construct($jsonKey)
public function __construct($jsonKey, $scope = null)
{
if (is_string($jsonKey)) {
if (!file_exists($jsonKey)) {
Expand Down Expand Up @@ -87,6 +89,7 @@ public function __construct($jsonKey)
'sub' => $jsonKey['client_email'],
'signingAlgorithm' => 'RS256',
'signingKey' => $jsonKey['private_key'],
'scope' => $scope,
]);

$this->projectId = isset($jsonKey['project_id'])
Expand All @@ -107,7 +110,8 @@ public function updateMetadata(
$authUri = null,
callable $httpHandler = null
) {
if (empty($authUri)) {
$scope = $this->auth->getScope();
if (empty($authUri) && empty($scope)) {
return $metadata;
}

Expand All @@ -128,10 +132,17 @@ public function updateMetadata(
public function fetchAuthToken(callable $httpHandler = null)
{
$audience = $this->auth->getAudience();
if (empty($audience)) {
$scope = $this->auth->getScope();
if (empty($audience) && empty($scope)) {
return null;
}

if (!empty($audience) && !empty($scope)) {
throw new UnexpectedValueException(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
throw new UnexpectedValueException(
throw new \UnexpectedValueException(

Would we be able to document this being thrown? Wondering if we need a test for this as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch and yes, I added a test!

I'm happy to add @throws. In this library in general (not that it's the right thing to do), we haven't been documenting thrown exceptions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I trust your judgement 👍

'Cannot sign both audience and scope in JwtAccess'
);
}

$access_token = $this->auth->toJwt();

// Set the self-signed access token in OAuth2 for getLastReceivedToken
Expand Down
10 changes: 9 additions & 1 deletion src/OAuth2.php
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,6 @@ public function toJwt(array $config = [])

$assertion = [
'iss' => $this->getIssuer(),
'aud' => $this->getAudience(),
'exp' => ($now + $this->getExpiry()),
'iat' => ($now - $opts['skew']),
];
Expand All @@ -437,9 +436,18 @@ public function toJwt(array $config = [])
throw new \DomainException($k . ' should not be null');
}
}
if (!(is_null($this->getAudience()))) {
$assertion['aud'] = $this->getAudience();
}

if (!(is_null($this->getScope()))) {
$assertion['scope'] = $this->getScope();
}

if (empty($assertion['scope']) && empty($assertion['aud'])) {
throw new \DomainException('one of scope or aud should not be null');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a test to exercise this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, added!

}

if (!(is_null($this->getSub()))) {
$assertion['sub'] = $this->getSub();
}
Expand Down
142 changes: 142 additions & 0 deletions tests/Credentials/ServiceAccountCredentialsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,148 @@ public function testNoScopeUseJwtAccess()
$this->assertGreaterThan(30, strlen($bearer_token));
}

public function testUpdateMetadataWithScopeAndUseJwtAccessWithScopeParameter()
{
$testJson = $this->createTestJson();
// jwt access should be used even when scopes are supplied, no outbound
// call should be made
$scope = 'scope1 scope2';
$sa = new ServiceAccountCredentials(
$scope,
$testJson
);
$sa->useJwtAccessWithScope();

$actual_metadata = $sa->updateMetadata(
$metadata = array('foo' => 'bar'),
$authUri = 'https://example.com/service'
);

$this->assertArrayHasKey(
CredentialsLoader::AUTH_METADATA_KEY,
$actual_metadata
);

$authorization = $actual_metadata[CredentialsLoader::AUTH_METADATA_KEY];
$this->assertInternalType('array', $authorization);

$bearer_token = current($authorization);
$this->assertInternalType('string', $bearer_token);
$this->assertEquals(0, strpos($bearer_token, 'Bearer '));

// Ensure scopes are signed inside
$token = substr($bearer_token, strlen('Bearer '));
$this->assertEquals(2, substr_count($token, '.'));
list($header, $payload, $sig) = explode('.', $bearer_token);
$json = json_decode(base64_decode($payload), true);
$this->assertInternalType('array', $json);
$this->assertArrayHasKey('scope', $json);
$this->assertEquals($json['scope'], $scope);
}

public function testUpdateMetadataWithScopeAndUseJwtAccessWithScopeParameterAndArrayScopes()
{
$testJson = $this->createTestJson();
// jwt access should be used even when scopes are supplied, no outbound
// call should be made
$scope = ['scope1', 'scope2'];
$sa = new ServiceAccountCredentials(
$scope,
$testJson
);
$sa->useJwtAccessWithScope();

$actual_metadata = $sa->updateMetadata(
$metadata = array('foo' => 'bar'),
$authUri = 'https://example.com/service'
);

$this->assertArrayHasKey(
CredentialsLoader::AUTH_METADATA_KEY,
$actual_metadata
);

$authorization = $actual_metadata[CredentialsLoader::AUTH_METADATA_KEY];
$this->assertInternalType('array', $authorization);

$bearer_token = current($authorization);
$this->assertInternalType('string', $bearer_token);
$this->assertEquals(0, strpos($bearer_token, 'Bearer '));

// Ensure scopes are signed inside
$token = substr($bearer_token, strlen('Bearer '));
$this->assertEquals(2, substr_count($token, '.'));
list($header, $payload, $sig) = explode('.', $bearer_token);
$json = json_decode(base64_decode($payload), true);
$this->assertInternalType('array', $json);
$this->assertArrayHasKey('scope', $json);
$this->assertEquals($json['scope'], implode(' ', $scope));

// Test last received token
$cachedToken = $sa->getLastReceivedToken();
$this->assertInternalType('array', $cachedToken);
$this->assertArrayHasKey('access_token', $cachedToken);
$this->assertEquals($token, $cachedToken['access_token']);
}

public function testFetchAuthTokenWithScopeAndUseJwtAccessWithScopeParameter()
{
$testJson = $this->createTestJson();
// jwt access should be used even when scopes are supplied, no outbound
// call should be made
$scope = 'scope1 scope2';
$sa = new ServiceAccountCredentials(
$scope,
$testJson
);
$sa->useJwtAccessWithScope();

$access_token = $sa->fetchAuthToken();
$this->assertInternalType('array', $access_token);
$this->assertArrayHasKey('access_token', $access_token);
$token = $access_token['access_token'];

// Ensure scopes are signed inside
$this->assertEquals(2, substr_count($token, '.'));
list($header, $payload, $sig) = explode('.', $token);
$json = json_decode(base64_decode($payload), true);
$this->assertInternalType('array', $json);
$this->assertArrayHasKey('scope', $json);
$this->assertEquals($json['scope'], $scope);
}

public function testFetchAuthTokenWithScopeAndUseJwtAccessWithScopeParameterAndArrayScopes()
{
$testJson = $this->createTestJson();
// jwt access should be used even when scopes are supplied, no outbound
// call should be made
$scope = ['scope1', 'scope2'];
$sa = new ServiceAccountCredentials(
$scope,
$testJson
);
$sa->useJwtAccessWithScope();

$access_token = $sa->fetchAuthToken();
$this->assertInternalType('array', $access_token);
$this->assertArrayHasKey('access_token', $access_token);
$token = $access_token['access_token'];

// Ensure scopes are signed inside
$this->assertEquals(2, substr_count($token, '.'));
list($header, $payload, $sig) = explode('.', $token);
$json = json_decode(base64_decode($payload), true);
$this->assertInternalType('array', $json);
$this->assertArrayHasKey('scope', $json);
$this->assertEquals($json['scope'], implode(' ', $scope));

// Test last received token
$cachedToken = $sa->getLastReceivedToken();
$this->assertInternalType('array', $cachedToken);
$this->assertArrayHasKey('access_token', $cachedToken);
$this->assertEquals($token, $cachedToken['access_token']);
}

/** @runInSeparateProcess */
public function testJwtAccessFromApplicationDefault()
{
Expand Down
1 change: 1 addition & 0 deletions tests/OAuth2Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@ public function testFailsWithMissingAudience()
{
$testConfig = $this->signingMinimal;
unset($testConfig['audience']);
unset($testConfig['scope']);
$o = new OAuth2($testConfig);
$o->toJwt();
}
Expand Down