-
Notifications
You must be signed in to change notification settings - Fork 192
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: use IamCredentials endpoint for generating ID tokens outside GDU #581
Conversation
src/Iam.php
Outdated
public function generateIdToken( | ||
string $clientEmail, | ||
string $targetAudience, | ||
string $bearerToken |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps there could be a fourth parameter array $headers = []
. This way the calling class (e.g. ImpersonatedServiceAccountCredentials
) could add a token endpoint metrics header. (The bearer token could stay as-is as it is required for the id token to be returned.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this feedback! Could you provide me an example of what that would look like? I assume it would look like adding the following in the call to Iam::generateIdToken:
$idToken = (new Iam($httpHandler, $this->getUniverseDomain()))->generateIdToken(
$this->auth->getIssuer(),
$this->auth->getAdditionalClaims()['target_audience'],
$jwt,
$this->applyTokenEndpointMetrics([], 'it') // this is the new line
);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that is exactly what I had in mind. All requests I encountered getting an acces or id token send metrics headers, so it makes sense to add them here too. The implementation would be something like this.
/**
* @param array $headers Optional headers to send to the api, defaults to an empty array
*/
public function generateIdToken(
string $clientEmail,
string $targetAudience,
string $bearerToken,
array $headers = [] // -> added optional parameter
): string {
// ....
// updated header assignment
$headers['Authorization'] = 'Bearer ' . $bearerToken;
// ....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has been added in 475bc74
Uses the
iamcredentials
service in order to generate an ID token whenuniverse_domain
is supplied and is not the default (googleapis.com
).