From 1a43960792c072cc1661cf1891d9dec4d86ca09e Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Fri, 22 Nov 2024 16:17:10 -0500 Subject: [PATCH 01/21] feat: add debug logging support --- package.json | 1 + src/auth/baseexternalclient.ts | 28 +++++++-- .../defaultawssecuritycredentialssupplier.ts | 39 +++++++++--- .../externalAccountAuthorizedUserClient.ts | 11 ++++ src/auth/oauth2client.ts | 60 +++++++++++++++---- src/auth/refreshclient.ts | 15 ++++- src/auth/stscredentials.ts | 17 +++++- src/auth/urlsubjecttokensupplier.ts | 14 ++++- 8 files changed, 151 insertions(+), 34 deletions(-) diff --git a/package.json b/package.json index 2bc32c04..14965eac 100644 --- a/package.json +++ b/package.json @@ -21,6 +21,7 @@ "ecdsa-sig-formatter": "^1.0.11", "gaxios": "^6.1.1", "gcp-metadata": "^6.1.0", + "google-logging-utils": "next", "gtoken": "^7.0.0", "jws": "^4.0.0" }, diff --git a/src/auth/baseexternalclient.ts b/src/auth/baseexternalclient.ts index 26436979..03bf2f43 100644 --- a/src/auth/baseexternalclient.ts +++ b/src/auth/baseexternalclient.ts @@ -20,6 +20,8 @@ import { GaxiosResponse, } from 'gaxios'; import * as stream from 'stream'; +import {log as makeLog} from 'google-logging-utils'; +const log = makeLog('auth'); import {Credentials} from './credentials'; import {AuthClient, AuthClientOptions} from './authclient'; @@ -478,13 +480,19 @@ export abstract class BaseExternalAccountClient extends AuthClient { } else if (projectNumber) { // Preferable not to use request() to avoid retrial policies. const headers = await this.getRequestHeaders(); + const url = `${this.cloudResourceManagerURL.toString()}${projectNumber}`; + const request = { + headers, + url, + }; + log.info('getProjectId %j', request); const response = await this.transporter.request({ + ...request, ...BaseExternalAccountClient.RETRY_CONFIG, - headers, - url: `${this.cloudResourceManagerURL.toString()}${projectNumber}`, responseType: 'json', }); this.projectId = response.data.projectId; + log.info('getProjectId, id %s', this.projectId); return this.projectId; } return null; @@ -665,10 +673,8 @@ export abstract class BaseExternalAccountClient extends AuthClient { private async getImpersonatedAccessToken( token: string ): Promise { - const opts: GaxiosOptions = { - ...BaseExternalAccountClient.RETRY_CONFIG, + const request = { url: this.serviceAccountImpersonationUrl!, - method: 'POST', headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${token}`, @@ -677,11 +683,23 @@ export abstract class BaseExternalAccountClient extends AuthClient { scope: this.getScopesArray(), lifetime: this.serviceAccountImpersonationLifetime + 's', }, + }; + log.info('getImpersonatedAccessToken %j', request); + const opts: GaxiosOptions = { + ...request, + ...BaseExternalAccountClient.RETRY_CONFIG, + method: 'POST', responseType: 'json', }; const response = await this.transporter.request(opts); const successResponse = response.data; + log.info( + 'getImpersonatedAccessToken success: %s, %s, %s', + successResponse.accessToken, + successResponse.expireTime, + response + ); return { access_token: successResponse.accessToken, // Convert from ISO format to timestamp. diff --git a/src/auth/defaultawssecuritycredentialssupplier.ts b/src/auth/defaultawssecuritycredentialssupplier.ts index 12d48d3f..7a413801 100644 --- a/src/auth/defaultawssecuritycredentialssupplier.ts +++ b/src/auth/defaultawssecuritycredentialssupplier.ts @@ -12,6 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +import {log as makeLog} from 'google-logging-utils'; +const log = makeLog('auth'); + import {ExternalAccountSupplierContext} from './baseexternalclient'; import {Gaxios, GaxiosOptions} from 'gaxios'; import {Transporter} from '../transporters'; @@ -122,14 +125,19 @@ export class DefaultAwsSecurityCredentialsSupplier '"options.credential_source.region_url"' ); } + const request = { + url: this.regionUrl, + headers: metadataHeaders, + }; + log.info('getAwsRegion %j', request); const opts: GaxiosOptions = { + ...request, ...this.additionalGaxiosOptions, - url: this.regionUrl, method: 'GET', responseType: 'text', - headers: metadataHeaders, }; const response = await context.transporter.request(opts); + log.info('getAwsRegion is %s', response.data); // Remove last character. For example, if us-east-2b is returned, // the region would be us-east-2. return response.data.substr(0, response.data.length - 1); @@ -186,14 +194,19 @@ export class DefaultAwsSecurityCredentialsSupplier async #getImdsV2SessionToken( transporter: Transporter | Gaxios ): Promise { + const request = { + url: this.imdsV2SessionTokenUrl, + headers: {'x-aws-ec2-metadata-token-ttl-seconds': '300'}, + }; const opts: GaxiosOptions = { + ...request, ...this.additionalGaxiosOptions, - url: this.imdsV2SessionTokenUrl, method: 'PUT', responseType: 'text', - headers: {'x-aws-ec2-metadata-token-ttl-seconds': '300'}, }; + log.info('#getImdsV2SessionToken %j', request); const response = await transporter.request(opts); + log.info('#getImdsV2SessionToken is %s', response.data); return response.data; } @@ -213,14 +226,19 @@ export class DefaultAwsSecurityCredentialsSupplier '"options.credential_source.url"' ); } + const request = { + url: this.securityCredentialsUrl, + headers: headers, + }; + log.info('#getAwsRoleName %j', request); const opts: GaxiosOptions = { + ...request, ...this.additionalGaxiosOptions, - url: this.securityCredentialsUrl, method: 'GET', responseType: 'text', - headers: headers, }; const response = await transporter.request(opts); + log.info('#getAwsRoleName name is %s', response.data); return response.data; } @@ -238,12 +256,17 @@ export class DefaultAwsSecurityCredentialsSupplier headers: Headers, transporter: Transporter | Gaxios ): Promise { + const request = { + url: `${this.securityCredentialsUrl}/${roleName}`, + headers: headers, + }; + log.info('#retrieveAwsSecurityCredentials %j', request); const response = await transporter.request({ + ...request, ...this.additionalGaxiosOptions, - url: `${this.securityCredentialsUrl}/${roleName}`, responseType: 'json', - headers: headers, }); + log.info('#retrieveAwsSecurityCredentials %s', response.data); return response.data; } diff --git a/src/auth/externalAccountAuthorizedUserClient.ts b/src/auth/externalAccountAuthorizedUserClient.ts index 24a480c0..b80135c1 100644 --- a/src/auth/externalAccountAuthorizedUserClient.ts +++ b/src/auth/externalAccountAuthorizedUserClient.ts @@ -12,6 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +import {log as makeLog} from 'google-logging-utils'; +const log = makeLog('auth'); + import {AuthClient, AuthClientOptions} from './authclient'; import {Headers} from './oauth2client'; import { @@ -124,16 +127,24 @@ class ExternalAccountAuthorizedUserHandler extends OAuthClientAuthHandler { // Apply OAuth client authentication. this.applyClientAuthenticationOptions(opts); + log.info('refreshToken %j', { + url: opts.url, + headers: opts.headers, + data: opts.data, + }); + try { const response = await this.transporter.request(opts); // Successful response. const tokenRefreshResponse = response.data; tokenRefreshResponse.res = response; + log.info('refreshToken response %j', tokenRefreshResponse); return tokenRefreshResponse; } catch (error) { // Translate error to OAuthError. if (error instanceof GaxiosError && error.response) { + log.error('refreshToken failed %j', error.response?.data); throw getErrorFromOAuthErrorResponse( error.response.data as OAuthErrorResponse, // Preserve other fields from the original error. diff --git a/src/auth/oauth2client.ts b/src/auth/oauth2client.ts index fb813a7f..44af6780 100644 --- a/src/auth/oauth2client.ts +++ b/src/auth/oauth2client.ts @@ -21,6 +21,8 @@ import { import * as querystring from 'querystring'; import * as stream from 'stream'; import * as formatEcdsa from 'ecdsa-sig-formatter'; +import {log as makeLog} from 'google-logging-utils'; +const log = makeLog('auth'); import {createCrypto, JwkCertificate, hasBrowserCrypto} from '../crypto/crypto'; import {BodyResponseCallback} from '../transporters'; @@ -708,14 +710,21 @@ export class OAuth2Client extends AuthClient { if (this.clientAuthentication === ClientAuthentication.ClientSecretPost) { values.client_secret = this._clientSecret; } - const res = await this.transporter.request({ - ...OAuth2Client.RETRY_CONFIG, - method: 'POST', + + const request = { url, data: querystring.stringify(values), headers, + }; + log.info('getTokenAsync %j', request); + + const res = await this.transporter.request({ + ...request, + ...OAuth2Client.RETRY_CONFIG, + method: 'POST', }); const tokens = res.data as Credentials; + log.info('getTokenAsync success %j', tokens); if (res.data && res.data.expires_in) { tokens.expiry_date = new Date().getTime() + res.data.expires_in * 1000; delete (tokens as CredentialRequest).expires_in; @@ -769,18 +778,24 @@ export class OAuth2Client extends AuthClient { grant_type: 'refresh_token', }; + const request = { + url, + data: querystring.stringify(data), + headers: {'Content-Type': 'application/x-www-form-urlencoded'}, + }; + log.info('refreshTokenNoCache %j', request); + let res: GaxiosResponse; try { // request for new token res = await this.transporter.request({ + ...request, ...OAuth2Client.RETRY_CONFIG, method: 'POST', - url, - data: querystring.stringify(data), - headers: {'Content-Type': 'application/x-www-form-urlencoded'}, }); - } catch (e) { + } catch (exc) { + const e = exc as Error; if ( e instanceof GaxiosError && e.message === 'invalid_grant' && @@ -789,11 +804,13 @@ export class OAuth2Client extends AuthClient { ) { e.message = JSON.stringify(e.response.data); } + log.error('refreshTokenNoCache failure %j', e.message); throw e; } const tokens = res.data as Credentials; + log.info('refreshTokenNoCache success %j', tokens); // TODO: de-duplicate this code from a few spots if (res.data && res.data.expires_in) { tokens.expiry_date = new Date().getTime() + res.data.expires_in * 1000; @@ -1002,12 +1019,17 @@ export class OAuth2Client extends AuthClient { url: this.getRevokeTokenURL(token).toString(), method: 'POST', }; + log.info('revokeToken %s', opts.url); if (callback) { - this.transporter - .request(opts) - .then(r => callback(null, r), callback); + this.transporter.request(opts).then(r => { + log.info('revokeToken success %s', r.data ?? ''); + callback(null, r); + }, callback); } else { - return this.transporter.request(opts); + return this.transporter.request(opts).then(r => { + log.info('revokeToken success %j', r.data); + return r; + }); } } @@ -1271,14 +1293,22 @@ export class OAuth2Client extends AuthClient { throw new Error(`Unsupported certificate format ${format}`); } try { + log.info('getFederatedSignonCertsAsync %s', url); res = await this.transporter.request({ ...OAuth2Client.RETRY_CONFIG, url, }); - } catch (e) { + log.info( + 'getFederatedSignonCertsAsync success %j %j', + res?.data, + res?.headers + ); + } catch (err) { + const e = err as Error; if (e instanceof Error) { e.message = `Failed to retrieve verification certificates: ${e.message}`; } + log.error('getFederatedSignonCertsAsync failed', e?.message); throw e; } @@ -1342,14 +1372,18 @@ export class OAuth2Client extends AuthClient { const url = this.endpoints.oauth2IapPublicKeyUrl.toString(); try { + log.info('getIapPublicKeysAsync %s', url); res = await this.transporter.request({ ...OAuth2Client.RETRY_CONFIG, url, }); - } catch (e) { + log.info('getIapPublicKeysAsync success %j', res.data); + } catch (err) { + const e = err as Error; if (e instanceof Error) { e.message = `Failed to retrieve verification certificates: ${e.message}`; } + log.error('getIapPublicKeysAsync failed', e?.message); throw e; } diff --git a/src/auth/refreshclient.ts b/src/auth/refreshclient.ts index 93c07d49..fe225e9f 100644 --- a/src/auth/refreshclient.ts +++ b/src/auth/refreshclient.ts @@ -20,6 +20,8 @@ import { OAuth2ClientOptions, } from './oauth2client'; import {stringify} from 'querystring'; +import {log as makeLog} from 'google-logging-utils'; +const log = makeLog('auth'); export const USER_REFRESH_ACCOUNT_TYPE = 'authorized_user'; @@ -80,13 +82,11 @@ export class UserRefreshClient extends OAuth2Client { } async fetchIdToken(targetAudience: string): Promise { - const res = await this.transporter.request({ - ...UserRefreshClient.RETRY_CONFIG, + const request = { url: this.endpoints.oauth2TokenUrl, headers: { 'Content-Type': 'application/x-www-form-urlencoded', }, - method: 'POST', data: stringify({ client_id: this._clientId, client_secret: this._clientSecret, @@ -94,8 +94,17 @@ export class UserRefreshClient extends OAuth2Client { refresh_token: this._refreshToken, target_audience: targetAudience, }), + }; + log.info('fetchIdToken %j', request); + + const res = await this.transporter.request({ + ...request, + ...UserRefreshClient.RETRY_CONFIG, + method: 'POST', }); + log.info('fetchIdToken success %s', res?.data?.id_token); + return res.data.id_token!; } diff --git a/src/auth/stscredentials.ts b/src/auth/stscredentials.ts index 291da246..597b5020 100644 --- a/src/auth/stscredentials.ts +++ b/src/auth/stscredentials.ts @@ -14,6 +14,8 @@ import {GaxiosError, GaxiosOptions, GaxiosResponse} from 'gaxios'; import * as querystring from 'querystring'; +import {log as makeLog} from 'google-logging-utils'; +const log = makeLog('auth'); import {DefaultTransporter, Transporter} from '../transporters'; import {Headers} from './oauth2client'; @@ -194,14 +196,20 @@ export class StsCredentials extends OAuthClientAuthHandler { // Inject additional STS headers if available. Object.assign(headers, additionalHeaders || {}); - const opts: GaxiosOptions = { - ...StsCredentials.RETRY_CONFIG, + const request = { url: this.tokenExchangeEndpoint.toString(), - method: 'POST', headers, data: querystring.stringify( values as unknown as querystring.ParsedUrlQueryInput ), + }; + + log.info('exchangeToken %j', request); + + const opts: GaxiosOptions = { + ...request, + ...StsCredentials.RETRY_CONFIG, + method: 'POST', responseType: 'json', }; // Apply OAuth client authentication. @@ -211,10 +219,13 @@ export class StsCredentials extends OAuthClientAuthHandler { const response = await this.transporter.request(opts); // Successful response. + log.info('exchangeToken success %j', response.data); const stsSuccessfulResponse = response.data; stsSuccessfulResponse.res = response; return stsSuccessfulResponse; } catch (error) { + log.error('exchangeToken failure %j', error); + // Translate error to OAuthError. if (error instanceof GaxiosError && error.response) { throw getErrorFromOAuthErrorResponse( diff --git a/src/auth/urlsubjecttokensupplier.ts b/src/auth/urlsubjecttokensupplier.ts index 9ae01f75..361ccad0 100644 --- a/src/auth/urlsubjecttokensupplier.ts +++ b/src/auth/urlsubjecttokensupplier.ts @@ -14,6 +14,8 @@ import {ExternalAccountSupplierContext} from './baseexternalclient'; import {GaxiosOptions} from 'gaxios'; +import {log as makeLog} from 'google-logging-utils'; +const log = makeLog('auth'); import { SubjectTokenFormatType, SubjectTokenJsonResponse, @@ -82,11 +84,16 @@ export class UrlSubjectTokenSupplier implements SubjectTokenSupplier { async getSubjectToken( context: ExternalAccountSupplierContext ): Promise { + const request = { + url: this.url, + headers: this.headers, + }; + log.info('getSubjectToken %j', request); + const opts: GaxiosOptions = { + ...request, ...this.additionalGaxiosOptions, - url: this.url, method: 'GET', - headers: this.headers, responseType: this.formatType, }; let subjectToken: string | undefined; @@ -99,10 +106,13 @@ export class UrlSubjectTokenSupplier implements SubjectTokenSupplier { subjectToken = response.data[this.subjectTokenFieldName]; } if (!subjectToken) { + log.error('getSubjectToken failed'); throw new Error( 'Unable to parse the subject_token from the credential_source URL' ); } + + log.info('getSubjectToken success %s', subjectToken); return subjectToken; } } From d0c9dff589aa9fba659a375f4ceb40324e1904ba Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Tue, 26 Nov 2024 15:45:29 -0500 Subject: [PATCH 02/21] build: add google-logging-utils in a temporary fashion --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 14965eac..a7e47c9a 100644 --- a/package.json +++ b/package.json @@ -28,6 +28,7 @@ "devDependencies": { "@types/base64-js": "^1.2.5", "@types/chai": "^4.1.7", + "@types/debug": "^4.1.12", "@types/jws": "^3.1.0", "@types/mocha": "^9.0.0", "@types/mv": "^2.1.0", @@ -60,7 +61,6 @@ "nock": "^13.0.0", "null-loader": "^4.0.0", "pdfmake": "0.2.12", - "puppeteer": "^21.0.0", "sinon": "^18.0.0", "ts-loader": "^8.0.0", "typescript": "^5.1.6", From 37893f7462c2247e6af9a736767720e21665dbdb Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Tue, 26 Nov 2024 15:47:03 -0500 Subject: [PATCH 03/21] build: undo unintentional package.json changes --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a7e47c9a..14965eac 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,6 @@ "devDependencies": { "@types/base64-js": "^1.2.5", "@types/chai": "^4.1.7", - "@types/debug": "^4.1.12", "@types/jws": "^3.1.0", "@types/mocha": "^9.0.0", "@types/mv": "^2.1.0", @@ -61,6 +60,7 @@ "nock": "^13.0.0", "null-loader": "^4.0.0", "pdfmake": "0.2.12", + "puppeteer": "^21.0.0", "sinon": "^18.0.0", "ts-loader": "^8.0.0", "typescript": "^5.1.6", From bb258a02bec4b5731eb06b13be7fdc537d2859b1 Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Wed, 11 Dec 2024 14:50:01 -0500 Subject: [PATCH 04/21] fix: move makeLog calls into shared classes for the most part --- src/auth/authclient.ts | 2 ++ src/auth/baseexternalclient.ts | 10 +++---- .../defaultawssecuritycredentialssupplier.ts | 22 +++++++------- .../externalAccountAuthorizedUserClient.ts | 9 ++---- src/auth/oauth2client.ts | 30 +++++++++---------- src/auth/oauth2common.ts | 2 ++ src/auth/refreshclient.ts | 6 ++-- src/auth/stscredentials.ts | 8 ++--- src/auth/urlsubjecttokensupplier.ts | 8 ++--- 9 files changed, 45 insertions(+), 52 deletions(-) diff --git a/src/auth/authclient.ts b/src/auth/authclient.ts index b9de5c95..b56207fb 100644 --- a/src/auth/authclient.ts +++ b/src/auth/authclient.ts @@ -19,6 +19,7 @@ import {DefaultTransporter, Transporter} from '../transporters'; import {Credentials} from './credentials'; import {GetAccessTokenResponse, Headers} from './oauth2client'; import {OriginalAndCamel, originalOrCamelOptions} from '../util'; +import {log as makeLog} from 'google-logging-utils'; /** * Base auth configurations (e.g. from JWT or `.json` files) with conventional @@ -189,6 +190,7 @@ export abstract class AuthClient eagerRefreshThresholdMillis = DEFAULT_EAGER_REFRESH_THRESHOLD_MILLIS; forceRefreshOnFailure = false; universeDomain = DEFAULT_UNIVERSE; + log = makeLog('auth'); constructor(opts: AuthClientOptions = {}) { super(); diff --git a/src/auth/baseexternalclient.ts b/src/auth/baseexternalclient.ts index 03bf2f43..023af9f1 100644 --- a/src/auth/baseexternalclient.ts +++ b/src/auth/baseexternalclient.ts @@ -20,8 +20,6 @@ import { GaxiosResponse, } from 'gaxios'; import * as stream from 'stream'; -import {log as makeLog} from 'google-logging-utils'; -const log = makeLog('auth'); import {Credentials} from './credentials'; import {AuthClient, AuthClientOptions} from './authclient'; @@ -485,14 +483,14 @@ export abstract class BaseExternalAccountClient extends AuthClient { headers, url, }; - log.info('getProjectId %j', request); + this.log.info('getProjectId %j', request); const response = await this.transporter.request({ ...request, ...BaseExternalAccountClient.RETRY_CONFIG, responseType: 'json', }); this.projectId = response.data.projectId; - log.info('getProjectId, id %s', this.projectId); + this.log.info('getProjectId, id %s', this.projectId); return this.projectId; } return null; @@ -684,7 +682,7 @@ export abstract class BaseExternalAccountClient extends AuthClient { lifetime: this.serviceAccountImpersonationLifetime + 's', }, }; - log.info('getImpersonatedAccessToken %j', request); + this.log.info('getImpersonatedAccessToken %j', request); const opts: GaxiosOptions = { ...request, ...BaseExternalAccountClient.RETRY_CONFIG, @@ -694,7 +692,7 @@ export abstract class BaseExternalAccountClient extends AuthClient { const response = await this.transporter.request(opts); const successResponse = response.data; - log.info( + this.log.info( 'getImpersonatedAccessToken success: %s, %s, %s', successResponse.accessToken, successResponse.expireTime, diff --git a/src/auth/defaultawssecuritycredentialssupplier.ts b/src/auth/defaultawssecuritycredentialssupplier.ts index 7a413801..559d0001 100644 --- a/src/auth/defaultawssecuritycredentialssupplier.ts +++ b/src/auth/defaultawssecuritycredentialssupplier.ts @@ -12,15 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {log as makeLog} from 'google-logging-utils'; -const log = makeLog('auth'); - import {ExternalAccountSupplierContext} from './baseexternalclient'; import {Gaxios, GaxiosOptions} from 'gaxios'; import {Transporter} from '../transporters'; import {AwsSecurityCredentialsSupplier} from './awsclient'; import {AwsSecurityCredentials} from './awsrequestsigner'; import {Headers} from './oauth2client'; +import {log as makeLog} from 'google-logging-utils'; /** * Interface defining the AWS security-credentials endpoint response. @@ -85,6 +83,8 @@ export class DefaultAwsSecurityCredentialsSupplier private readonly imdsV2SessionTokenUrl?: string; private readonly additionalGaxiosOptions?: GaxiosOptions; + private log = makeLog('auth'); + /** * Instantiates a new DefaultAwsSecurityCredentialsSupplier using information * from the credential_source stored in the ADC file. @@ -129,7 +129,7 @@ export class DefaultAwsSecurityCredentialsSupplier url: this.regionUrl, headers: metadataHeaders, }; - log.info('getAwsRegion %j', request); + this.log.info('getAwsRegion %j', request); const opts: GaxiosOptions = { ...request, ...this.additionalGaxiosOptions, @@ -137,7 +137,7 @@ export class DefaultAwsSecurityCredentialsSupplier responseType: 'text', }; const response = await context.transporter.request(opts); - log.info('getAwsRegion is %s', response.data); + this.log.info('getAwsRegion is %s', response.data); // Remove last character. For example, if us-east-2b is returned, // the region would be us-east-2. return response.data.substr(0, response.data.length - 1); @@ -204,9 +204,9 @@ export class DefaultAwsSecurityCredentialsSupplier method: 'PUT', responseType: 'text', }; - log.info('#getImdsV2SessionToken %j', request); + this.log.info('#getImdsV2SessionToken %j', request); const response = await transporter.request(opts); - log.info('#getImdsV2SessionToken is %s', response.data); + this.log.info('#getImdsV2SessionToken is %s', response.data); return response.data; } @@ -230,7 +230,7 @@ export class DefaultAwsSecurityCredentialsSupplier url: this.securityCredentialsUrl, headers: headers, }; - log.info('#getAwsRoleName %j', request); + this.log.info('#getAwsRoleName %j', request); const opts: GaxiosOptions = { ...request, ...this.additionalGaxiosOptions, @@ -238,7 +238,7 @@ export class DefaultAwsSecurityCredentialsSupplier responseType: 'text', }; const response = await transporter.request(opts); - log.info('#getAwsRoleName name is %s', response.data); + this.log.info('#getAwsRoleName name is %s', response.data); return response.data; } @@ -260,13 +260,13 @@ export class DefaultAwsSecurityCredentialsSupplier url: `${this.securityCredentialsUrl}/${roleName}`, headers: headers, }; - log.info('#retrieveAwsSecurityCredentials %j', request); + this.log.info('#retrieveAwsSecurityCredentials %j', request); const response = await transporter.request({ ...request, ...this.additionalGaxiosOptions, responseType: 'json', }); - log.info('#retrieveAwsSecurityCredentials %s', response.data); + this.log.info('#retrieveAwsSecurityCredentials %s', response.data); return response.data; } diff --git a/src/auth/externalAccountAuthorizedUserClient.ts b/src/auth/externalAccountAuthorizedUserClient.ts index b80135c1..18b6cad4 100644 --- a/src/auth/externalAccountAuthorizedUserClient.ts +++ b/src/auth/externalAccountAuthorizedUserClient.ts @@ -12,9 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {log as makeLog} from 'google-logging-utils'; -const log = makeLog('auth'); - import {AuthClient, AuthClientOptions} from './authclient'; import {Headers} from './oauth2client'; import { @@ -127,7 +124,7 @@ class ExternalAccountAuthorizedUserHandler extends OAuthClientAuthHandler { // Apply OAuth client authentication. this.applyClientAuthenticationOptions(opts); - log.info('refreshToken %j', { + this.log.info('refreshToken %j', { url: opts.url, headers: opts.headers, data: opts.data, @@ -139,12 +136,12 @@ class ExternalAccountAuthorizedUserHandler extends OAuthClientAuthHandler { // Successful response. const tokenRefreshResponse = response.data; tokenRefreshResponse.res = response; - log.info('refreshToken response %j', tokenRefreshResponse); + this.log.info('refreshToken response %j', tokenRefreshResponse); return tokenRefreshResponse; } catch (error) { // Translate error to OAuthError. if (error instanceof GaxiosError && error.response) { - log.error('refreshToken failed %j', error.response?.data); + this.log.error('refreshToken failed %j', error.response?.data); throw getErrorFromOAuthErrorResponse( error.response.data as OAuthErrorResponse, // Preserve other fields from the original error. diff --git a/src/auth/oauth2client.ts b/src/auth/oauth2client.ts index 44af6780..5e8f40e7 100644 --- a/src/auth/oauth2client.ts +++ b/src/auth/oauth2client.ts @@ -21,8 +21,6 @@ import { import * as querystring from 'querystring'; import * as stream from 'stream'; import * as formatEcdsa from 'ecdsa-sig-formatter'; -import {log as makeLog} from 'google-logging-utils'; -const log = makeLog('auth'); import {createCrypto, JwkCertificate, hasBrowserCrypto} from '../crypto/crypto'; import {BodyResponseCallback} from '../transporters'; @@ -716,7 +714,7 @@ export class OAuth2Client extends AuthClient { data: querystring.stringify(values), headers, }; - log.info('getTokenAsync %j', request); + this.log.info('getTokenAsync %j', request); const res = await this.transporter.request({ ...request, @@ -724,7 +722,7 @@ export class OAuth2Client extends AuthClient { method: 'POST', }); const tokens = res.data as Credentials; - log.info('getTokenAsync success %j', tokens); + this.log.info('getTokenAsync success %j', tokens); if (res.data && res.data.expires_in) { tokens.expiry_date = new Date().getTime() + res.data.expires_in * 1000; delete (tokens as CredentialRequest).expires_in; @@ -783,7 +781,7 @@ export class OAuth2Client extends AuthClient { data: querystring.stringify(data), headers: {'Content-Type': 'application/x-www-form-urlencoded'}, }; - log.info('refreshTokenNoCache %j', request); + this.log.info('refreshTokenNoCache %j', request); let res: GaxiosResponse; @@ -804,13 +802,13 @@ export class OAuth2Client extends AuthClient { ) { e.message = JSON.stringify(e.response.data); } - log.error('refreshTokenNoCache failure %j', e.message); + this.log.error('refreshTokenNoCache failure %j', e.message); throw e; } const tokens = res.data as Credentials; - log.info('refreshTokenNoCache success %j', tokens); + this.log.info('refreshTokenNoCache success %j', tokens); // TODO: de-duplicate this code from a few spots if (res.data && res.data.expires_in) { tokens.expiry_date = new Date().getTime() + res.data.expires_in * 1000; @@ -1019,15 +1017,15 @@ export class OAuth2Client extends AuthClient { url: this.getRevokeTokenURL(token).toString(), method: 'POST', }; - log.info('revokeToken %s', opts.url); + this.log.info('revokeToken %s', opts.url); if (callback) { this.transporter.request(opts).then(r => { - log.info('revokeToken success %s', r.data ?? ''); + this.log.info('revokeToken success %s', r.data ?? ''); callback(null, r); }, callback); } else { return this.transporter.request(opts).then(r => { - log.info('revokeToken success %j', r.data); + this.log.info('revokeToken success %j', r.data); return r; }); } @@ -1293,12 +1291,12 @@ export class OAuth2Client extends AuthClient { throw new Error(`Unsupported certificate format ${format}`); } try { - log.info('getFederatedSignonCertsAsync %s', url); + this.log.info('getFederatedSignonCertsAsync %s', url); res = await this.transporter.request({ ...OAuth2Client.RETRY_CONFIG, url, }); - log.info( + this.log.info( 'getFederatedSignonCertsAsync success %j %j', res?.data, res?.headers @@ -1308,7 +1306,7 @@ export class OAuth2Client extends AuthClient { if (e instanceof Error) { e.message = `Failed to retrieve verification certificates: ${e.message}`; } - log.error('getFederatedSignonCertsAsync failed', e?.message); + this.log.error('getFederatedSignonCertsAsync failed', e?.message); throw e; } @@ -1372,18 +1370,18 @@ export class OAuth2Client extends AuthClient { const url = this.endpoints.oauth2IapPublicKeyUrl.toString(); try { - log.info('getIapPublicKeysAsync %s', url); + this.log.info('getIapPublicKeysAsync %s', url); res = await this.transporter.request({ ...OAuth2Client.RETRY_CONFIG, url, }); - log.info('getIapPublicKeysAsync success %j', res.data); + this.log.info('getIapPublicKeysAsync success %j', res.data); } catch (err) { const e = err as Error; if (e instanceof Error) { e.message = `Failed to retrieve verification certificates: ${e.message}`; } - log.error('getIapPublicKeysAsync failed', e?.message); + this.log.error('getIapPublicKeysAsync failed', e?.message); throw e; } diff --git a/src/auth/oauth2common.ts b/src/auth/oauth2common.ts index 19f4fe8e..0d02e754 100644 --- a/src/auth/oauth2common.ts +++ b/src/auth/oauth2common.ts @@ -14,6 +14,7 @@ import {GaxiosOptions} from 'gaxios'; import * as querystring from 'querystring'; +import {log as makeLog} from 'google-logging-utils'; import {Crypto, createCrypto} from '../crypto/crypto'; @@ -69,6 +70,7 @@ export interface ClientAuthentication { */ export abstract class OAuthClientAuthHandler { private crypto: Crypto; + log = makeLog('auth'); /** * Instantiates an OAuth client authentication handler. diff --git a/src/auth/refreshclient.ts b/src/auth/refreshclient.ts index fe225e9f..998b4391 100644 --- a/src/auth/refreshclient.ts +++ b/src/auth/refreshclient.ts @@ -20,8 +20,6 @@ import { OAuth2ClientOptions, } from './oauth2client'; import {stringify} from 'querystring'; -import {log as makeLog} from 'google-logging-utils'; -const log = makeLog('auth'); export const USER_REFRESH_ACCOUNT_TYPE = 'authorized_user'; @@ -95,7 +93,7 @@ export class UserRefreshClient extends OAuth2Client { target_audience: targetAudience, }), }; - log.info('fetchIdToken %j', request); + this.log.info('fetchIdToken %j', request); const res = await this.transporter.request({ ...request, @@ -103,7 +101,7 @@ export class UserRefreshClient extends OAuth2Client { method: 'POST', }); - log.info('fetchIdToken success %s', res?.data?.id_token); + this.log.info('fetchIdToken success %s', res?.data?.id_token); return res.data.id_token!; } diff --git a/src/auth/stscredentials.ts b/src/auth/stscredentials.ts index 597b5020..a6aff436 100644 --- a/src/auth/stscredentials.ts +++ b/src/auth/stscredentials.ts @@ -14,8 +14,6 @@ import {GaxiosError, GaxiosOptions, GaxiosResponse} from 'gaxios'; import * as querystring from 'querystring'; -import {log as makeLog} from 'google-logging-utils'; -const log = makeLog('auth'); import {DefaultTransporter, Transporter} from '../transporters'; import {Headers} from './oauth2client'; @@ -204,7 +202,7 @@ export class StsCredentials extends OAuthClientAuthHandler { ), }; - log.info('exchangeToken %j', request); + this.log.info('exchangeToken %j', request); const opts: GaxiosOptions = { ...request, @@ -219,12 +217,12 @@ export class StsCredentials extends OAuthClientAuthHandler { const response = await this.transporter.request(opts); // Successful response. - log.info('exchangeToken success %j', response.data); + this.log.info('exchangeToken success %j', response.data); const stsSuccessfulResponse = response.data; stsSuccessfulResponse.res = response; return stsSuccessfulResponse; } catch (error) { - log.error('exchangeToken failure %j', error); + this.log.error('exchangeToken failure %j', error); // Translate error to OAuthError. if (error instanceof GaxiosError && error.response) { diff --git a/src/auth/urlsubjecttokensupplier.ts b/src/auth/urlsubjecttokensupplier.ts index 361ccad0..6771488e 100644 --- a/src/auth/urlsubjecttokensupplier.ts +++ b/src/auth/urlsubjecttokensupplier.ts @@ -15,7 +15,6 @@ import {ExternalAccountSupplierContext} from './baseexternalclient'; import {GaxiosOptions} from 'gaxios'; import {log as makeLog} from 'google-logging-utils'; -const log = makeLog('auth'); import { SubjectTokenFormatType, SubjectTokenJsonResponse, @@ -61,6 +60,7 @@ export class UrlSubjectTokenSupplier implements SubjectTokenSupplier { private readonly formatType: SubjectTokenFormatType; private readonly subjectTokenFieldName?: string; private readonly additionalGaxiosOptions?: GaxiosOptions; + private readonly log = makeLog('auth'); /** * Instantiates a URL subject token supplier. @@ -88,7 +88,7 @@ export class UrlSubjectTokenSupplier implements SubjectTokenSupplier { url: this.url, headers: this.headers, }; - log.info('getSubjectToken %j', request); + this.log.info('getSubjectToken %j', request); const opts: GaxiosOptions = { ...request, @@ -106,13 +106,13 @@ export class UrlSubjectTokenSupplier implements SubjectTokenSupplier { subjectToken = response.data[this.subjectTokenFieldName]; } if (!subjectToken) { - log.error('getSubjectToken failed'); + this.log.error('getSubjectToken failed'); throw new Error( 'Unable to parse the subject_token from the credential_source URL' ); } - log.info('getSubjectToken success %s', subjectToken); + this.log.info('getSubjectToken success %s', subjectToken); return subjectToken; } } From 3a35055bb1b1e93f3b4d470d5eeb94656f84207f Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Thu, 19 Dec 2024 16:30:08 -0500 Subject: [PATCH 05/21] fix: update to anticipated release version of logger --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 14965eac..2b7db342 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "ecdsa-sig-formatter": "^1.0.11", "gaxios": "^6.1.1", "gcp-metadata": "^6.1.0", - "google-logging-utils": "next", + "google-logging-utils": "^1.0.0", "gtoken": "^7.0.0", "jws": "^4.0.0" }, From 690ee5f8322d1200786ac3cb35fc31202e9783ba Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Wed, 19 Feb 2025 17:33:48 -0500 Subject: [PATCH 06/21] fix: fix possible merging errors from earlier --- src/auth/baseexternalclient.ts | 2 -- src/auth/defaultawssecuritycredentialssupplier.ts | 4 ---- src/auth/oauth2client.ts | 3 --- src/auth/refreshclient.ts | 3 --- src/auth/stscredentials.ts | 9 +-------- src/auth/urlsubjecttokensupplier.ts | 1 - 6 files changed, 1 insertion(+), 21 deletions(-) diff --git a/src/auth/baseexternalclient.ts b/src/auth/baseexternalclient.ts index e8fec228..cd5f7f64 100644 --- a/src/auth/baseexternalclient.ts +++ b/src/auth/baseexternalclient.ts @@ -484,7 +484,6 @@ export abstract class BaseExternalAccountClient extends AuthClient { const response = await this.transporter.request({ ...request, ...BaseExternalAccountClient.RETRY_CONFIG, - responseType: 'json', }); this.projectId = response.data.projectId; this.log.info('getProjectId, id %s', this.projectId); @@ -680,7 +679,6 @@ export abstract class BaseExternalAccountClient extends AuthClient { ...request, ...BaseExternalAccountClient.RETRY_CONFIG, method: 'POST', - responseType: 'json', }; const response = await this.transporter.request(opts); diff --git a/src/auth/defaultawssecuritycredentialssupplier.ts b/src/auth/defaultawssecuritycredentialssupplier.ts index 6bb3e9d7..acd10a9e 100644 --- a/src/auth/defaultawssecuritycredentialssupplier.ts +++ b/src/auth/defaultawssecuritycredentialssupplier.ts @@ -134,7 +134,6 @@ export class DefaultAwsSecurityCredentialsSupplier ...request, ...this.additionalGaxiosOptions, method: 'GET', - responseType: 'text', }; const response = await context.transporter.request(opts); this.log.info('getAwsRegion is %s', response.data); @@ -204,7 +203,6 @@ export class DefaultAwsSecurityCredentialsSupplier ...request, ...this.additionalGaxiosOptions, method: 'PUT', - responseType: 'text', }; this.log.info('#getImdsV2SessionToken %j', request); const response = await transporter.request(opts); @@ -237,7 +235,6 @@ export class DefaultAwsSecurityCredentialsSupplier ...request, ...this.additionalGaxiosOptions, method: 'GET', - responseType: 'text', }; const response = await transporter.request(opts); this.log.info('#getAwsRoleName name is %s', response.data); @@ -266,7 +263,6 @@ export class DefaultAwsSecurityCredentialsSupplier const response = await transporter.request({ ...request, ...this.additionalGaxiosOptions, - responseType: 'json', }); this.log.info('#retrieveAwsSecurityCredentials %s', response.data); return response.data; diff --git a/src/auth/oauth2client.ts b/src/auth/oauth2client.ts index a32035ef..60b01f2d 100644 --- a/src/auth/oauth2client.ts +++ b/src/auth/oauth2client.ts @@ -820,7 +820,6 @@ export class OAuth2Client extends AuthClient { const request = { url, data: querystring.stringify(data), - headers: {'Content-Type': 'application/x-www-form-urlencoded'}, }; this.log.info('refreshTokenNoCache %j', request); @@ -832,8 +831,6 @@ export class OAuth2Client extends AuthClient { ...request, ...OAuth2Client.RETRY_CONFIG, method: 'POST', - url, - data: new URLSearchParams(data), }); } catch (exc) { const e = exc as Error; diff --git a/src/auth/refreshclient.ts b/src/auth/refreshclient.ts index 0d157fe8..80fddef3 100644 --- a/src/auth/refreshclient.ts +++ b/src/auth/refreshclient.ts @@ -102,9 +102,6 @@ export class UserRefreshClient extends OAuth2Client { async fetchIdToken(targetAudience: string): Promise { const request = { url: this.endpoints.oauth2TokenUrl, - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - }, method: 'POST', data: new URLSearchParams({ client_id: this._clientId, diff --git a/src/auth/stscredentials.ts b/src/auth/stscredentials.ts index 798dc19e..a34a4711 100644 --- a/src/auth/stscredentials.ts +++ b/src/auth/stscredentials.ts @@ -211,15 +211,9 @@ export class StsCredentials extends OAuthClientAuthHandler { } }); - const requestHeaders = { - 'Content-Type': 'application/x-www-form-urlencoded', - }; - // Inject additional STS headers if available. - Object.assign(requestHeaders, headers || {}); - const request = { url: this.#tokenExchangeEndpoint.toString(), - requestHeaders, + headers, data: new URLSearchParams(payload), }; @@ -229,7 +223,6 @@ export class StsCredentials extends OAuthClientAuthHandler { ...request, ...StsCredentials.RETRY_CONFIG, method: 'POST', - responseType: 'json', }; // Apply OAuth client authentication. this.applyClientAuthenticationOptions(opts); diff --git a/src/auth/urlsubjecttokensupplier.ts b/src/auth/urlsubjecttokensupplier.ts index 6771488e..503b6d39 100644 --- a/src/auth/urlsubjecttokensupplier.ts +++ b/src/auth/urlsubjecttokensupplier.ts @@ -94,7 +94,6 @@ export class UrlSubjectTokenSupplier implements SubjectTokenSupplier { ...request, ...this.additionalGaxiosOptions, method: 'GET', - responseType: this.formatType, }; let subjectToken: string | undefined; if (this.formatType === 'text') { From 609c2d233ab76300761ae9dbfe85d85e3df2c1ab Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Wed, 19 Feb 2025 17:51:23 -0500 Subject: [PATCH 07/21] fix: more potential merging issues --- src/auth/defaultawssecuritycredentialssupplier.ts | 3 +-- src/auth/oauth2client.ts | 6 +++--- src/auth/refreshclient.ts | 3 +-- src/auth/stscredentials.ts | 2 +- src/auth/urlsubjecttokensupplier.ts | 2 +- 5 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/auth/defaultawssecuritycredentialssupplier.ts b/src/auth/defaultawssecuritycredentialssupplier.ts index acd10a9e..8a84ac17 100644 --- a/src/auth/defaultawssecuritycredentialssupplier.ts +++ b/src/auth/defaultawssecuritycredentialssupplier.ts @@ -194,7 +194,6 @@ export class DefaultAwsSecurityCredentialsSupplier */ async #getImdsV2SessionToken(transporter: Gaxios): Promise { const request = { - ...this.additionalGaxiosOptions, url: this.imdsV2SessionTokenUrl, method: 'PUT', headers: {'x-aws-ec2-metadata-token-ttl-seconds': '300'}, @@ -261,8 +260,8 @@ export class DefaultAwsSecurityCredentialsSupplier }; this.log.info('#retrieveAwsSecurityCredentials %j', request); const response = await transporter.request({ - ...request, ...this.additionalGaxiosOptions, + ...request, }); this.log.info('#retrieveAwsSecurityCredentials %s', response.data); return response.data; diff --git a/src/auth/oauth2client.ts b/src/auth/oauth2client.ts index 60b01f2d..27e36377 100644 --- a/src/auth/oauth2client.ts +++ b/src/auth/oauth2client.ts @@ -758,9 +758,9 @@ export class OAuth2Client extends AuthClient { this.log.info('getTokenAsync %j', request); const res = await this.transporter.request({ - ...request, ...OAuth2Client.RETRY_CONFIG, method: 'POST', + ...request, }); const tokens = res.data as Credentials; this.log.info('getTokenAsync success %j', tokens); @@ -828,9 +828,9 @@ export class OAuth2Client extends AuthClient { try { // request for new token res = await this.transporter.request({ - ...request, ...OAuth2Client.RETRY_CONFIG, method: 'POST', + ...request, }); } catch (exc) { const e = exc as Error; @@ -1065,7 +1065,7 @@ export class OAuth2Client extends AuthClient { }, callback); } else { return this.transporter.request(opts).then(r => { - this.log.info('revokeToken success %j', r.data); + this.log.info('revokeToken success %s', r.data ?? ''); return r; }); } diff --git a/src/auth/refreshclient.ts b/src/auth/refreshclient.ts index 80fddef3..3cc9a5c8 100644 --- a/src/auth/refreshclient.ts +++ b/src/auth/refreshclient.ts @@ -102,7 +102,6 @@ export class UserRefreshClient extends OAuth2Client { async fetchIdToken(targetAudience: string): Promise { const request = { url: this.endpoints.oauth2TokenUrl, - method: 'POST', data: new URLSearchParams({ client_id: this._clientId, client_secret: this._clientSecret, @@ -114,9 +113,9 @@ export class UserRefreshClient extends OAuth2Client { this.log.info('fetchIdToken %j', request); const res = await this.transporter.request({ - ...request, ...UserRefreshClient.RETRY_CONFIG, method: 'POST', + ...request, }); this.log.info('fetchIdToken success %s', res?.data?.id_token); diff --git a/src/auth/stscredentials.ts b/src/auth/stscredentials.ts index a34a4711..025ef96d 100644 --- a/src/auth/stscredentials.ts +++ b/src/auth/stscredentials.ts @@ -220,9 +220,9 @@ export class StsCredentials extends OAuthClientAuthHandler { this.log.info('exchangeToken %j', request); const opts: GaxiosOptions = { - ...request, ...StsCredentials.RETRY_CONFIG, method: 'POST', + ...request, }; // Apply OAuth client authentication. this.applyClientAuthenticationOptions(opts); diff --git a/src/auth/urlsubjecttokensupplier.ts b/src/auth/urlsubjecttokensupplier.ts index 503b6d39..02eff686 100644 --- a/src/auth/urlsubjecttokensupplier.ts +++ b/src/auth/urlsubjecttokensupplier.ts @@ -91,9 +91,9 @@ export class UrlSubjectTokenSupplier implements SubjectTokenSupplier { this.log.info('getSubjectToken %j', request); const opts: GaxiosOptions = { - ...request, ...this.additionalGaxiosOptions, method: 'GET', + ...request, }; let subjectToken: string | undefined; if (this.formatType === 'text') { From 730a8ad735dc292f80e260c587a9c46f7630ca45 Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Thu, 20 Feb 2025 15:06:49 -0500 Subject: [PATCH 08/21] fix: more rearranging to match the older ordering --- src/auth/baseexternalclient.ts | 2 +- src/auth/defaultawssecuritycredentialssupplier.ts | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/auth/baseexternalclient.ts b/src/auth/baseexternalclient.ts index cd5f7f64..04d65db1 100644 --- a/src/auth/baseexternalclient.ts +++ b/src/auth/baseexternalclient.ts @@ -482,8 +482,8 @@ export abstract class BaseExternalAccountClient extends AuthClient { }; this.log.info('getProjectId %j', request); const response = await this.transporter.request({ - ...request, ...BaseExternalAccountClient.RETRY_CONFIG, + ...request, }); this.projectId = response.data.projectId; this.log.info('getProjectId, id %s', this.projectId); diff --git a/src/auth/defaultawssecuritycredentialssupplier.ts b/src/auth/defaultawssecuritycredentialssupplier.ts index 8a84ac17..fdd27d50 100644 --- a/src/auth/defaultawssecuritycredentialssupplier.ts +++ b/src/auth/defaultawssecuritycredentialssupplier.ts @@ -131,9 +131,9 @@ export class DefaultAwsSecurityCredentialsSupplier }; this.log.info('getAwsRegion %j', request); const opts: GaxiosOptions = { - ...request, ...this.additionalGaxiosOptions, method: 'GET', + ...request, }; const response = await context.transporter.request(opts); this.log.info('getAwsRegion is %s', response.data); @@ -195,13 +195,12 @@ export class DefaultAwsSecurityCredentialsSupplier async #getImdsV2SessionToken(transporter: Gaxios): Promise { const request = { url: this.imdsV2SessionTokenUrl, - method: 'PUT', headers: {'x-aws-ec2-metadata-token-ttl-seconds': '300'}, }; const opts: GaxiosOptions = { - ...request, ...this.additionalGaxiosOptions, method: 'PUT', + ...request, }; this.log.info('#getImdsV2SessionToken %j', request); const response = await transporter.request(opts); @@ -231,9 +230,9 @@ export class DefaultAwsSecurityCredentialsSupplier }; this.log.info('#getAwsRoleName %j', request); const opts: GaxiosOptions = { - ...request, ...this.additionalGaxiosOptions, method: 'GET', + ...request, }; const response = await transporter.request(opts); this.log.info('#getAwsRoleName name is %s', response.data); From 6c487ce72a465b20b90d2ef4cd26623dc1798a38 Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Thu, 20 Feb 2025 15:21:13 -0500 Subject: [PATCH 09/21] fix: merging two months of changes is sometimes trying --- src/auth/baseexternalclient.ts | 2 +- src/auth/oauth2client.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/auth/baseexternalclient.ts b/src/auth/baseexternalclient.ts index 04d65db1..1fd1d6fe 100644 --- a/src/auth/baseexternalclient.ts +++ b/src/auth/baseexternalclient.ts @@ -676,8 +676,8 @@ export abstract class BaseExternalAccountClient extends AuthClient { }; this.log.info('getImpersonatedAccessToken %j', request); const opts: GaxiosOptions = { - ...request, ...BaseExternalAccountClient.RETRY_CONFIG, + ...request, method: 'POST', }; const response = diff --git a/src/auth/oauth2client.ts b/src/auth/oauth2client.ts index 27e36377..53f2c02b 100644 --- a/src/auth/oauth2client.ts +++ b/src/auth/oauth2client.ts @@ -819,7 +819,7 @@ export class OAuth2Client extends AuthClient { const request = { url, - data: querystring.stringify(data), + data: new URLSearchParams(data), }; this.log.info('refreshTokenNoCache %j', request); From 87fb037aecd1a4b606574aec0032fcee99a00132 Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Fri, 21 Feb 2025 18:13:14 -0500 Subject: [PATCH 10/21] fix: very work in progress for switching to using interceptor and symbol based logs --- src/auth/authclient.ts | 96 ++++++++++++++++++- src/auth/baseexternalclient.ts | 11 +-- .../defaultawssecuritycredentialssupplier.ts | 33 +++---- .../externalAccountAuthorizedUserClient.ts | 13 +-- src/auth/oauth2client.ts | 18 ++-- 5 files changed, 118 insertions(+), 53 deletions(-) diff --git a/src/auth/authclient.ts b/src/auth/authclient.ts index 19403291..703bb558 100644 --- a/src/auth/authclient.ts +++ b/src/auth/authclient.ts @@ -21,6 +21,13 @@ import {log as makeLog} from 'google-logging-utils'; import {PRODUCT_NAME, USER_AGENT} from '../shared.cjs'; +/** + * Easy access to symbol-indexed strings on config objects. + */ +export type SymbolIndexString = { + [key: symbol]: string | undefined; +}; + /** * Base auth configurations (e.g. from JWT or `.json` files) with conventional * camelCased options. @@ -212,6 +219,16 @@ export abstract class AuthClient universeDomain = DEFAULT_UNIVERSE; log = makeLog('auth'); + /** + * Symbols that can be added to GaxiosOptions to specify the method name that is + * making an RPC call, for logging purposes, as well as a string ID that can be + * used to correlate calls and responses. + */ + static readonly RequestMethodNameSymbol: unique symbol = Symbol( + 'request method name' + ); + static readonly RequestLogIdSymbol: unique symbol = Symbol('request log id'); + constructor(opts: AuthClientOptions = {}) { super(); @@ -229,7 +246,10 @@ export abstract class AuthClient if (options.get('useAuthRequestParameters') !== false) { this.transporter.interceptors.request.add( - AuthClient.DEFAULT_REQUEST_INTERCEPTOR, + AuthClient.DEFAULT_REQUEST_INTERCEPTOR + ); + this.transporter.interceptors.response.add( + AuthClient.DEFAULT_RESPONSE_INTERCEPTOR ); } @@ -308,7 +328,7 @@ export abstract class AuthClient */ protected addUserProjectAndAuthHeaders( target: T, - source: Headers, + source: Headers ): T { const xGoogUserProject = source.get('x-goog-user-project'); const authorizationHeader = source.get('authorization'); @@ -324,6 +344,7 @@ export abstract class AuthClient return target; } + static log = makeLog('auth:interceptor'); static readonly DEFAULT_REQUEST_INTERCEPTOR: Parameters< Gaxios['interceptors']['request']['add'] >[0] = { @@ -342,10 +363,81 @@ export abstract class AuthClient config.headers.set('User-Agent', `${userAgent} ${USER_AGENT}`); } + const symbols: SymbolIndexString = config as unknown as SymbolIndexString; + const methodName = symbols[AuthClient.RequestMethodNameSymbol]; + + // This doesn't need to be very unique or interesting, it's just an aid for + // matching requests to responses. + const logId = `${Math.floor(Math.random() * 1000)}`; + symbols[AuthClient.RequestLogIdSymbol] = logId; + + // Boil down the object we're printing out. + const logObject = { + url: config.url, + headers: config.headers, + }; + if (methodName) { + AuthClient.log.info('%s [%s] request %j', methodName, logId, logObject); + } else { + AuthClient.log.info('[%s] request %j', logId, logObject); + } + return config; }, }; + static readonly DEFAULT_RESPONSE_INTERCEPTOR: Parameters< + Gaxios['interceptors']['response']['add'] + >[0] = { + resolved: async response => { + const symbols: SymbolIndexString = + response.config as unknown as SymbolIndexString; + const methodName = symbols[AuthClient.RequestMethodNameSymbol]; + const logId = symbols[AuthClient.RequestLogIdSymbol]; + if (methodName) { + AuthClient.log.info( + '%s [%s] response %j', + methodName, + logId, + response.data + ); + } else { + AuthClient.log.info('[%s] response %j', logId, response.data); + } + + return response; + }, + rejected: async error => { + const symbols: SymbolIndexString = + error.config as unknown as SymbolIndexString; + const methodName = symbols[AuthClient.RequestMethodNameSymbol]; + const logId = symbols[AuthClient.RequestLogIdSymbol]; + if (methodName) { + AuthClient.log.info( + '%s [%s] error %j', + methodName, + logId, + error.response?.data + ); + } else { + AuthClient.log.error('[%s] error %j', logId, error.response?.data); + } + + return error; + }, + }; + + /** + * Sets the method name that is making a Gaxios request, so that logging may tag + * log lines with the operation. + * @param config A Gaxios request config + * @param methodName The method name making the call + */ + static setMethodName(config: GaxiosOptions, methodName: string) { + const symbols: SymbolIndexString = config as unknown as SymbolIndexString; + symbols[AuthClient.RequestMethodNameSymbol] = methodName; + } + /** * Retry config for Auth-related requests. * diff --git a/src/auth/baseexternalclient.ts b/src/auth/baseexternalclient.ts index cbabc6a0..7b73689b 100644 --- a/src/auth/baseexternalclient.ts +++ b/src/auth/baseexternalclient.ts @@ -477,13 +477,12 @@ export abstract class BaseExternalAccountClient extends AuthClient { headers, url, }; - this.log.info('getProjectId %j', request); + AuthClient.setMethodName(request, 'getProjectId'); const response = await this.transporter.request({ ...BaseExternalAccountClient.RETRY_CONFIG, ...request, }); this.projectId = response.data.projectId; - this.log.info('getProjectId, id %s', this.projectId); return this.projectId; } return null; @@ -671,21 +670,15 @@ export abstract class BaseExternalAccountClient extends AuthClient { lifetime: this.serviceAccountImpersonationLifetime + 's', }, }; - this.log.info('getImpersonatedAccessToken %j', request); const opts: GaxiosOptions = { ...BaseExternalAccountClient.RETRY_CONFIG, ...request, method: 'POST', }; + AuthClient.setMethodName(opts, 'getImpersonatedAccessToken'); const response = await this.transporter.request(opts); const successResponse = response.data; - this.log.info( - 'getImpersonatedAccessToken success: %s, %s, %s', - successResponse.accessToken, - successResponse.expireTime, - response - ); return { access_token: successResponse.accessToken, // Convert from ISO format to timestamp. diff --git a/src/auth/defaultawssecuritycredentialssupplier.ts b/src/auth/defaultawssecuritycredentialssupplier.ts index 05ba4a00..680f9b85 100644 --- a/src/auth/defaultawssecuritycredentialssupplier.ts +++ b/src/auth/defaultawssecuritycredentialssupplier.ts @@ -17,6 +17,7 @@ import {Gaxios, GaxiosOptions} from 'gaxios'; import {AwsSecurityCredentialsSupplier} from './awsclient'; import {AwsSecurityCredentials} from './awsrequestsigner'; import {log as makeLog} from 'google-logging-utils'; +import {AuthClient} from './authclient'; /** * Interface defining the AWS security-credentials endpoint response. @@ -129,14 +130,13 @@ export class DefaultAwsSecurityCredentialsSupplier url: this.regionUrl, headers: metadataHeaders, }; - this.log.info('getAwsRegion %j', request); const opts: GaxiosOptions = { ...this.additionalGaxiosOptions, method: 'GET', ...request, }; + AuthClient.setMethodName(opts, 'getAwsRegion'); const response = await context.transporter.request(opts); - this.log.info('getAwsRegion is %s', response.data); // Remove last character. For example, if us-east-2b is returned, // the region would be us-east-2. return response.data.substr(0, response.data.length - 1); @@ -202,9 +202,8 @@ export class DefaultAwsSecurityCredentialsSupplier method: 'PUT', ...request, }; - this.log.info('#getImdsV2SessionToken %j', request); + AuthClient.setMethodName(opts, '#getImdsV2SessionToken'); const response = await transporter.request(opts); - this.log.info('#getImdsV2SessionToken is %s', response.data); return response.data; } @@ -224,18 +223,14 @@ export class DefaultAwsSecurityCredentialsSupplier '"options.credential_source.url"', ); } - const request = { - url: this.securityCredentialsUrl, - headers: headers, - }; - this.log.info('#getAwsRoleName %j', request); const opts: GaxiosOptions = { ...this.additionalGaxiosOptions, + url: this.securityCredentialsUrl, method: 'GET', - ...request, + headers: headers, }; + AuthClient.setMethodName(opts, '#getAwsRoleName'); const response = await transporter.request(opts); - this.log.info('#getAwsRoleName name is %s', response.data); return response.data; } @@ -251,18 +246,16 @@ export class DefaultAwsSecurityCredentialsSupplier async #retrieveAwsSecurityCredentials( roleName: string, headers: Headers, - transporter: Gaxios, + transporter: Gaxios ): Promise { - const request = { + const opts = { + ...this.additionalGaxiosOptions, url: `${this.securityCredentialsUrl}/${roleName}`, headers: headers, - }; - this.log.info('#retrieveAwsSecurityCredentials %j', request); - const response = await transporter.request({ - ...this.additionalGaxiosOptions, - ...request, - }); - this.log.info('#retrieveAwsSecurityCredentials %s', response.data); + } as GaxiosOptions; + AuthClient.setMethodName(opts, '#retrieveAwsSecurityCredentials'); + const response = + await transporter.request(opts); return response.data; } diff --git a/src/auth/externalAccountAuthorizedUserClient.ts b/src/auth/externalAccountAuthorizedUserClient.ts index 954dcdef..b7246bad 100644 --- a/src/auth/externalAccountAuthorizedUserClient.ts +++ b/src/auth/externalAccountAuthorizedUserClient.ts @@ -109,7 +109,7 @@ class ExternalAccountAuthorizedUserHandler extends OAuthClientAuthHandler { */ async refreshToken( refreshToken: string, - headers?: HeadersInit, + headers?: HeadersInit ): Promise { const opts: GaxiosOptions = { ...ExternalAccountAuthorizedUserHandler.RETRY_CONFIG, @@ -121,32 +121,25 @@ class ExternalAccountAuthorizedUserHandler extends OAuthClientAuthHandler { refresh_token: refreshToken, }), }; + AuthClient.setMethodName(opts, 'refreshToken'); // Apply OAuth client authentication. this.applyClientAuthenticationOptions(opts); - this.log.info('refreshToken %j', { - url: opts.url, - headers: opts.headers, - data: opts.data, - }); - try { const response = await this.transporter.request(opts); // Successful response. const tokenRefreshResponse = response.data; tokenRefreshResponse.res = response; - this.log.info('refreshToken response %j', tokenRefreshResponse); return tokenRefreshResponse; } catch (error) { // Translate error to OAuthError. if (error instanceof GaxiosError && error.response) { - this.log.error('refreshToken failed %j', error.response?.data); throw getErrorFromOAuthErrorResponse( error.response.data as OAuthErrorResponse, // Preserve other fields from the original error. - error, + error ); } // Request could fail before the server responds. diff --git a/src/auth/oauth2client.ts b/src/auth/oauth2client.ts index 97294846..bd9c510c 100644 --- a/src/auth/oauth2client.ts +++ b/src/auth/oauth2client.ts @@ -750,20 +750,16 @@ export class OAuth2Client extends AuthClient { values.client_secret = this._clientSecret; } - const request = { + const opts = { + ...OAuth2Client.RETRY_CONFIG, url, + method: 'POST', data: new URLSearchParams(values as {}), headers, }; - this.log.info('getTokenAsync %j', request); - - const res = await this.transporter.request({ - ...OAuth2Client.RETRY_CONFIG, - method: 'POST', - ...request, - }); + AuthClient.setMethodName(opts, 'getTokenAsync'); + const res = await this.transporter.request(opts); const tokens = res.data as Credentials; - this.log.info('getTokenAsync success %j', tokens); if (res.data && res.data.expires_in) { tokens.expiry_date = new Date().getTime() + res.data.expires_in * 1000; delete (tokens as CredentialRequest).expires_in; @@ -821,7 +817,7 @@ export class OAuth2Client extends AuthClient { url, data: new URLSearchParams(data), }; - this.log.info('refreshTokenNoCache %j', request); + AuthClient.setMethodName(request, 'refreshTokenNoCache'); let res: GaxiosResponse; @@ -842,13 +838,11 @@ export class OAuth2Client extends AuthClient { ) { e.message = JSON.stringify(e.response.data); } - this.log.error('refreshTokenNoCache failure %j', e.message); throw e; } const tokens = res.data as Credentials; - this.log.info('refreshTokenNoCache success %j', tokens); // TODO: de-duplicate this code from a few spots if (res.data && res.data.expires_in) { tokens.expiry_date = new Date().getTime() + res.data.expires_in * 1000; From 5599ae6ed500b4742c324999c37083276bb2491e Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Wed, 26 Feb 2025 16:11:11 -0500 Subject: [PATCH 11/21] fix: catch all exceptions, finish piping in method names --- src/auth/authclient.ts | 98 ++++++++++++++++------------- src/auth/oauth2client.ts | 33 ++++------ src/auth/refreshclient.ts | 14 ++--- src/auth/stscredentials.ts | 17 ++--- src/auth/urlsubjecttokensupplier.ts | 14 ++--- 5 files changed, 81 insertions(+), 95 deletions(-) diff --git a/src/auth/authclient.ts b/src/auth/authclient.ts index 703bb558..d0ae5dba 100644 --- a/src/auth/authclient.ts +++ b/src/auth/authclient.ts @@ -363,23 +363,27 @@ export abstract class AuthClient config.headers.set('User-Agent', `${userAgent} ${USER_AGENT}`); } - const symbols: SymbolIndexString = config as unknown as SymbolIndexString; - const methodName = symbols[AuthClient.RequestMethodNameSymbol]; - - // This doesn't need to be very unique or interesting, it's just an aid for - // matching requests to responses. - const logId = `${Math.floor(Math.random() * 1000)}`; - symbols[AuthClient.RequestLogIdSymbol] = logId; - - // Boil down the object we're printing out. - const logObject = { - url: config.url, - headers: config.headers, - }; - if (methodName) { - AuthClient.log.info('%s [%s] request %j', methodName, logId, logObject); - } else { - AuthClient.log.info('[%s] request %j', logId, logObject); + try { + const symbols: SymbolIndexString = config as unknown as SymbolIndexString; + const methodName = symbols[AuthClient.RequestMethodNameSymbol]; + + // This doesn't need to be very unique or interesting, it's just an aid for + // matching requests to responses. + const logId = `${Math.floor(Math.random() * 1000)}`; + symbols[AuthClient.RequestLogIdSymbol] = logId; + + // Boil down the object we're printing out. + const logObject = { + url: config.url, + headers: config.headers, + }; + if (methodName) { + AuthClient.log.info('%s [%s] request %j', methodName, logId, logObject); + } else { + AuthClient.log.info('[%s] request %j', logId, logObject); + } + } catch (e) { + // Logging must not create new errors; swallow them all. } return config; @@ -390,37 +394,45 @@ export abstract class AuthClient Gaxios['interceptors']['response']['add'] >[0] = { resolved: async response => { - const symbols: SymbolIndexString = - response.config as unknown as SymbolIndexString; - const methodName = symbols[AuthClient.RequestMethodNameSymbol]; - const logId = symbols[AuthClient.RequestLogIdSymbol]; - if (methodName) { - AuthClient.log.info( - '%s [%s] response %j', - methodName, - logId, - response.data - ); - } else { - AuthClient.log.info('[%s] response %j', logId, response.data); + try { + const symbols: SymbolIndexString = + response.config as unknown as SymbolIndexString; + const methodName = symbols[AuthClient.RequestMethodNameSymbol]; + const logId = symbols[AuthClient.RequestLogIdSymbol]; + if (methodName) { + AuthClient.log.info( + '%s [%s] response %j', + methodName, + logId, + response.data + ); + } else { + AuthClient.log.info('[%s] response %j', logId, response.data); + } + } catch (e) { + // Logging must not create new errors; swallow them all. } return response; }, rejected: async error => { - const symbols: SymbolIndexString = - error.config as unknown as SymbolIndexString; - const methodName = symbols[AuthClient.RequestMethodNameSymbol]; - const logId = symbols[AuthClient.RequestLogIdSymbol]; - if (methodName) { - AuthClient.log.info( - '%s [%s] error %j', - methodName, - logId, - error.response?.data - ); - } else { - AuthClient.log.error('[%s] error %j', logId, error.response?.data); + try { + const symbols: SymbolIndexString = + error.config as unknown as SymbolIndexString; + const methodName = symbols[AuthClient.RequestMethodNameSymbol]; + const logId = symbols[AuthClient.RequestLogIdSymbol]; + if (methodName) { + AuthClient.log.info( + '%s [%s] error %j', + methodName, + logId, + error.response?.data + ); + } else { + AuthClient.log.error('[%s] error %j', logId, error.response?.data); + } + } catch (e) { + // Logging must not create new errors; swallow them all. } return error; diff --git a/src/auth/oauth2client.ts b/src/auth/oauth2client.ts index bd9c510c..07570a38 100644 --- a/src/auth/oauth2client.ts +++ b/src/auth/oauth2client.ts @@ -1050,17 +1050,13 @@ export class OAuth2Client extends AuthClient { url: this.getRevokeTokenURL(token).toString(), method: 'POST', }; - this.log.info('revokeToken %s', opts.url); + AuthClient.setMethodName(opts, 'revokeToken'); if (callback) { this.transporter.request(opts).then(r => { - this.log.info('revokeToken success %s', r.data ?? ''); callback(null, r); }, callback); } else { - return this.transporter.request(opts).then(r => { - this.log.info('revokeToken success %s', r.data ?? ''); - return r; - }); + return this.transporter.request(opts); } } @@ -1320,22 +1316,17 @@ export class OAuth2Client extends AuthClient { throw new Error(`Unsupported certificate format ${format}`); } try { - this.log.info('getFederatedSignonCertsAsync %s', url); - res = await this.transporter.request({ + const opts = { ...OAuth2Client.RETRY_CONFIG, url, - }); - this.log.info( - 'getFederatedSignonCertsAsync success %j %j', - res?.data, - res?.headers - ); + }; + AuthClient.setMethodName(opts, 'getFederatedSignonCertsAsync'); + res = await this.transporter.request(opts); } catch (err) { const e = err as Error; if (e instanceof Error) { e.message = `Failed to retrieve verification certificates: ${e.message}`; } - this.log.error('getFederatedSignonCertsAsync failed', e?.message); throw e; } @@ -1399,18 +1390,16 @@ export class OAuth2Client extends AuthClient { const url = this.endpoints.oauth2IapPublicKeyUrl.toString(); try { - this.log.info('getIapPublicKeysAsync %s', url); - res = await this.transporter.request({ + const opts = { ...OAuth2Client.RETRY_CONFIG, url, - }); - this.log.info('getIapPublicKeysAsync success %j', res.data); - } catch (err) { - const e = err as Error; + }; + AuthClient.setMethodName(opts, 'getIapPublicKeysAsync'); + res = await this.transporter.request(opts); + } catch (e) { if (e instanceof Error) { e.message = `Failed to retrieve verification certificates: ${e.message}`; } - this.log.error('getIapPublicKeysAsync failed', e?.message); throw e; } diff --git a/src/auth/refreshclient.ts b/src/auth/refreshclient.ts index affe0c02..ecc5c685 100644 --- a/src/auth/refreshclient.ts +++ b/src/auth/refreshclient.ts @@ -19,6 +19,7 @@ import { OAuth2Client, OAuth2ClientOptions, } from './oauth2client'; +import {AuthClient} from './authclient'; export const USER_REFRESH_ACCOUNT_TYPE = 'authorized_user'; @@ -97,7 +98,9 @@ export class UserRefreshClient extends OAuth2Client { async fetchIdToken(targetAudience: string): Promise { const request = { + ...UserRefreshClient.RETRY_CONFIG, url: this.endpoints.oauth2TokenUrl, + method: 'POST', data: new URLSearchParams({ client_id: this._clientId, client_secret: this._clientSecret, @@ -106,16 +109,9 @@ export class UserRefreshClient extends OAuth2Client { target_audience: targetAudience, } as {}), }; - this.log.info('fetchIdToken %j', request); - - const res = await this.transporter.request({ - ...UserRefreshClient.RETRY_CONFIG, - method: 'POST', - ...request, - }); - - this.log.info('fetchIdToken success %s', res?.data?.id_token); + AuthClient.setMethodName(request, 'fetchIdToken'); + const res = await this.transporter.request(request); return res.data.id_token!; } diff --git a/src/auth/stscredentials.ts b/src/auth/stscredentials.ts index 8fcdc239..65175f1f 100644 --- a/src/auth/stscredentials.ts +++ b/src/auth/stscredentials.ts @@ -13,7 +13,7 @@ // limitations under the License. import {GaxiosError, GaxiosOptions, GaxiosResponse} from 'gaxios'; -import {HeadersInit} from './authclient'; +import {AuthClient, HeadersInit} from './authclient'; import { ClientAuthentication, OAuthClientAuthHandler, @@ -211,19 +211,15 @@ export class StsCredentials extends OAuthClientAuthHandler { } }); - const request = { + const opts: GaxiosOptions = { + ...StsCredentials.RETRY_CONFIG, url: this.#tokenExchangeEndpoint.toString(), + method: 'POST', headers, data: new URLSearchParams(payload), }; + AuthClient.setMethodName(opts, 'exchangeToken'); - this.log.info('exchangeToken %j', request); - - const opts: GaxiosOptions = { - ...StsCredentials.RETRY_CONFIG, - method: 'POST', - ...request, - }; // Apply OAuth client authentication. this.applyClientAuthenticationOptions(opts); @@ -231,13 +227,10 @@ export class StsCredentials extends OAuthClientAuthHandler { const response = await this.transporter.request(opts); // Successful response. - this.log.info('exchangeToken success %j', response.data); const stsSuccessfulResponse = response.data; stsSuccessfulResponse.res = response; return stsSuccessfulResponse; } catch (error) { - this.log.error('exchangeToken failure %j', error); - // Translate error to OAuthError. if (error instanceof GaxiosError && error.response) { throw getErrorFromOAuthErrorResponse( diff --git a/src/auth/urlsubjecttokensupplier.ts b/src/auth/urlsubjecttokensupplier.ts index a8e4bfad..fce9a51a 100644 --- a/src/auth/urlsubjecttokensupplier.ts +++ b/src/auth/urlsubjecttokensupplier.ts @@ -20,6 +20,7 @@ import { SubjectTokenJsonResponse, SubjectTokenSupplier, } from './identitypoolclient'; +import {AuthClient} from './authclient'; /** * Interface that defines options used to build a {@link UrlSubjectTokenSupplier} @@ -84,17 +85,14 @@ export class UrlSubjectTokenSupplier implements SubjectTokenSupplier { async getSubjectToken( context: ExternalAccountSupplierContext, ): Promise { - const request = { - url: this.url, - headers: this.headers, - }; - this.log.info('getSubjectToken %j', request); - const opts: GaxiosOptions = { ...this.additionalGaxiosOptions, + url: this.url, method: 'GET', - ...request, + headers: this.headers, }; + AuthClient.setMethodName(opts, 'getSubjectToken'); + let subjectToken: string | undefined; if (this.formatType === 'text') { const response = await context.transporter.request(opts); @@ -105,13 +103,11 @@ export class UrlSubjectTokenSupplier implements SubjectTokenSupplier { subjectToken = response.data[this.subjectTokenFieldName]; } if (!subjectToken) { - this.log.error('getSubjectToken failed'); throw new Error( 'Unable to parse the subject_token from the credential_source URL', ); } - this.log.info('getSubjectToken success %s', subjectToken); return subjectToken; } } From 1415718bfc9653497881160853fdc24dab36bab8 Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Wed, 26 Feb 2025 16:32:50 -0500 Subject: [PATCH 12/21] fix: various self-review updates --- src/auth/authclient.ts | 8 ++++-- src/auth/baseexternalclient.ts | 19 +++++-------- .../defaultawssecuritycredentialssupplier.ts | 19 ++++--------- .../externalAccountAuthorizedUserClient.ts | 4 +-- src/auth/oauth2client.ts | 28 +++++++++---------- src/auth/oauth2common.ts | 2 -- src/auth/refreshclient.ts | 7 +++-- src/auth/urlsubjecttokensupplier.ts | 2 -- 8 files changed, 37 insertions(+), 52 deletions(-) diff --git a/src/auth/authclient.ts b/src/auth/authclient.ts index d0ae5dba..0f6aeba6 100644 --- a/src/auth/authclient.ts +++ b/src/auth/authclient.ts @@ -446,8 +446,12 @@ export abstract class AuthClient * @param methodName The method name making the call */ static setMethodName(config: GaxiosOptions, methodName: string) { - const symbols: SymbolIndexString = config as unknown as SymbolIndexString; - symbols[AuthClient.RequestMethodNameSymbol] = methodName; + try { + const symbols: SymbolIndexString = config as unknown as SymbolIndexString; + symbols[AuthClient.RequestMethodNameSymbol] = methodName; + } catch (e) { + // Logging must not create new errors; swallow them all. + } } /** diff --git a/src/auth/baseexternalclient.ts b/src/auth/baseexternalclient.ts index 7b73689b..c4556df1 100644 --- a/src/auth/baseexternalclient.ts +++ b/src/auth/baseexternalclient.ts @@ -473,15 +473,13 @@ export abstract class BaseExternalAccountClient extends AuthClient { // Preferable not to use request() to avoid retrial policies. const headers = await this.getRequestHeaders(); const url = `${this.cloudResourceManagerURL.toString()}${projectNumber}`; - const request = { + const opts: GaxiosOptions = { + ...BaseExternalAccountClient.RETRY_CONFIG, headers, url, }; - AuthClient.setMethodName(request, 'getProjectId'); - const response = await this.transporter.request({ - ...BaseExternalAccountClient.RETRY_CONFIG, - ...request, - }); + AuthClient.setMethodName(opts, 'getProjectId'); + const response = await this.transporter.request(opts); this.projectId = response.data.projectId; return this.projectId; } @@ -659,8 +657,10 @@ export abstract class BaseExternalAccountClient extends AuthClient { private async getImpersonatedAccessToken( token: string, ): Promise { - const request = { + const opts: GaxiosOptions = { + ...BaseExternalAccountClient.RETRY_CONFIG, url: this.serviceAccountImpersonationUrl!, + method: 'POST', headers: { 'content-type': 'application/json', authorization: `Bearer ${token}`, @@ -670,11 +670,6 @@ export abstract class BaseExternalAccountClient extends AuthClient { lifetime: this.serviceAccountImpersonationLifetime + 's', }, }; - const opts: GaxiosOptions = { - ...BaseExternalAccountClient.RETRY_CONFIG, - ...request, - method: 'POST', - }; AuthClient.setMethodName(opts, 'getImpersonatedAccessToken'); const response = await this.transporter.request(opts); diff --git a/src/auth/defaultawssecuritycredentialssupplier.ts b/src/auth/defaultawssecuritycredentialssupplier.ts index 680f9b85..63d41407 100644 --- a/src/auth/defaultawssecuritycredentialssupplier.ts +++ b/src/auth/defaultawssecuritycredentialssupplier.ts @@ -16,7 +16,6 @@ import {ExternalAccountSupplierContext} from './baseexternalclient'; import {Gaxios, GaxiosOptions} from 'gaxios'; import {AwsSecurityCredentialsSupplier} from './awsclient'; import {AwsSecurityCredentials} from './awsrequestsigner'; -import {log as makeLog} from 'google-logging-utils'; import {AuthClient} from './authclient'; /** @@ -82,8 +81,6 @@ export class DefaultAwsSecurityCredentialsSupplier private readonly imdsV2SessionTokenUrl?: string; private readonly additionalGaxiosOptions?: GaxiosOptions; - private log = makeLog('auth'); - /** * Instantiates a new DefaultAwsSecurityCredentialsSupplier using information * from the credential_source stored in the ADC file. @@ -126,14 +123,11 @@ export class DefaultAwsSecurityCredentialsSupplier '"options.credential_source.region_url"', ); } - const request = { - url: this.regionUrl, - headers: metadataHeaders, - }; const opts: GaxiosOptions = { ...this.additionalGaxiosOptions, + url: this.regionUrl, method: 'GET', - ...request, + headers: metadataHeaders, }; AuthClient.setMethodName(opts, 'getAwsRegion'); const response = await context.transporter.request(opts); @@ -193,15 +187,12 @@ export class DefaultAwsSecurityCredentialsSupplier * @return A promise that resolves with the IMDSv2 Session Token. */ async #getImdsV2SessionToken(transporter: Gaxios): Promise { - const request = { - url: this.imdsV2SessionTokenUrl, - headers: {'x-aws-ec2-metadata-token-ttl-seconds': '300'}, - }; const opts: GaxiosOptions = { ...this.additionalGaxiosOptions, + url: this.imdsV2SessionTokenUrl, method: 'PUT', - ...request, - }; + headers: {'x-aws-ec2-metadata-token-ttl-seconds': '300'}, + }; AuthClient.setMethodName(opts, '#getImdsV2SessionToken'); const response = await transporter.request(opts); return response.data; diff --git a/src/auth/externalAccountAuthorizedUserClient.ts b/src/auth/externalAccountAuthorizedUserClient.ts index b7246bad..d473e4dc 100644 --- a/src/auth/externalAccountAuthorizedUserClient.ts +++ b/src/auth/externalAccountAuthorizedUserClient.ts @@ -109,7 +109,7 @@ class ExternalAccountAuthorizedUserHandler extends OAuthClientAuthHandler { */ async refreshToken( refreshToken: string, - headers?: HeadersInit + headers?: HeadersInit, ): Promise { const opts: GaxiosOptions = { ...ExternalAccountAuthorizedUserHandler.RETRY_CONFIG, @@ -139,7 +139,7 @@ class ExternalAccountAuthorizedUserHandler extends OAuthClientAuthHandler { throw getErrorFromOAuthErrorResponse( error.response.data as OAuthErrorResponse, // Preserve other fields from the original error. - error + error, ); } // Request could fail before the server responds. diff --git a/src/auth/oauth2client.ts b/src/auth/oauth2client.ts index 07570a38..5c99bf3e 100644 --- a/src/auth/oauth2client.ts +++ b/src/auth/oauth2client.ts @@ -813,23 +813,20 @@ export class OAuth2Client extends AuthClient { grant_type: 'refresh_token', }; - const request = { - url, + const opts: GaxiosOptions = { + ...OAuth2Client.RETRY_CONFIG, + method: 'POST', + url, data: new URLSearchParams(data), }; - AuthClient.setMethodName(request, 'refreshTokenNoCache'); + AuthClient.setMethodName(opts, 'refreshTokenNoCache'); let res: GaxiosResponse; try { // request for new token - res = await this.transporter.request({ - ...OAuth2Client.RETRY_CONFIG, - method: 'POST', - ...request, - }); - } catch (exc) { - const e = exc as Error; + res = await this.transporter.request(opts); + } catch (e) { if ( e instanceof GaxiosError && e.message === 'invalid_grant' && @@ -1052,9 +1049,11 @@ export class OAuth2Client extends AuthClient { }; AuthClient.setMethodName(opts, 'revokeToken'); if (callback) { - this.transporter.request(opts).then(r => { - callback(null, r); - }, callback); + this.transporter + .request(opts) + .then(r => { + callback(null, r); + }, callback); } else { return this.transporter.request(opts); } @@ -1322,8 +1321,7 @@ export class OAuth2Client extends AuthClient { }; AuthClient.setMethodName(opts, 'getFederatedSignonCertsAsync'); res = await this.transporter.request(opts); - } catch (err) { - const e = err as Error; + } catch (e) { if (e instanceof Error) { e.message = `Failed to retrieve verification certificates: ${e.message}`; } diff --git a/src/auth/oauth2common.ts b/src/auth/oauth2common.ts index 367a8927..3b1de441 100644 --- a/src/auth/oauth2common.ts +++ b/src/auth/oauth2common.ts @@ -13,7 +13,6 @@ // limitations under the License. import {Gaxios, GaxiosOptions} from 'gaxios'; -import {log as makeLog} from 'google-logging-utils'; import {createCrypto} from '../crypto/crypto'; @@ -80,7 +79,6 @@ export interface OAuthClientAuthHandlerOptions { * request bodies are supported. */ export abstract class OAuthClientAuthHandler { - log = makeLog('auth'); #crypto = createCrypto(); #clientAuthentication?: ClientAuthentication; protected transporter: Gaxios; diff --git a/src/auth/refreshclient.ts b/src/auth/refreshclient.ts index ecc5c685..41abcaea 100644 --- a/src/auth/refreshclient.ts +++ b/src/auth/refreshclient.ts @@ -20,6 +20,7 @@ import { OAuth2ClientOptions, } from './oauth2client'; import {AuthClient} from './authclient'; +import {GaxiosOptions} from 'gaxios'; export const USER_REFRESH_ACCOUNT_TYPE = 'authorized_user'; @@ -97,7 +98,7 @@ export class UserRefreshClient extends OAuth2Client { } async fetchIdToken(targetAudience: string): Promise { - const request = { + const opts: GaxiosOptions = { ...UserRefreshClient.RETRY_CONFIG, url: this.endpoints.oauth2TokenUrl, method: 'POST', @@ -109,9 +110,9 @@ export class UserRefreshClient extends OAuth2Client { target_audience: targetAudience, } as {}), }; - AuthClient.setMethodName(request, 'fetchIdToken'); + AuthClient.setMethodName(opts, 'fetchIdToken'); - const res = await this.transporter.request(request); + const res = await this.transporter.request(opts); return res.data.id_token!; } diff --git a/src/auth/urlsubjecttokensupplier.ts b/src/auth/urlsubjecttokensupplier.ts index fce9a51a..75a0f45f 100644 --- a/src/auth/urlsubjecttokensupplier.ts +++ b/src/auth/urlsubjecttokensupplier.ts @@ -14,7 +14,6 @@ import {ExternalAccountSupplierContext} from './baseexternalclient'; import {GaxiosOptions} from 'gaxios'; -import {log as makeLog} from 'google-logging-utils'; import { SubjectTokenFormatType, SubjectTokenJsonResponse, @@ -61,7 +60,6 @@ export class UrlSubjectTokenSupplier implements SubjectTokenSupplier { private readonly formatType: SubjectTokenFormatType; private readonly subjectTokenFieldName?: string; private readonly additionalGaxiosOptions?: GaxiosOptions; - private readonly log = makeLog('auth'); /** * Instantiates a URL subject token supplier. From fc027a420e0606580c620643599e5a72ad09d7e3 Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Wed, 26 Feb 2025 16:34:14 -0500 Subject: [PATCH 13/21] chore: put back missing comma --- src/auth/authclient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/auth/authclient.ts b/src/auth/authclient.ts index 0f6aeba6..6385ebe4 100644 --- a/src/auth/authclient.ts +++ b/src/auth/authclient.ts @@ -328,7 +328,7 @@ export abstract class AuthClient */ protected addUserProjectAndAuthHeaders( target: T, - source: Headers + source: Headers, ): T { const xGoogUserProject = source.get('x-goog-user-project'); const authorizationHeader = source.get('authorization'); From 144bde8d58bb88b24b3b749217898930d2ad4b0a Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Wed, 26 Feb 2025 16:34:57 -0500 Subject: [PATCH 14/21] fix: update log name back to the original --- src/auth/authclient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/auth/authclient.ts b/src/auth/authclient.ts index 6385ebe4..752edf64 100644 --- a/src/auth/authclient.ts +++ b/src/auth/authclient.ts @@ -344,7 +344,7 @@ export abstract class AuthClient return target; } - static log = makeLog('auth:interceptor'); + static log = makeLog('auth'); static readonly DEFAULT_REQUEST_INTERCEPTOR: Parameters< Gaxios['interceptors']['request']['add'] >[0] = { From fa72c1b474ada53a16355ec7640bf881c196c54c Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Wed, 26 Feb 2025 16:44:01 -0500 Subject: [PATCH 15/21] fix: rejection interceptors must re-throw errors --- src/auth/authclient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/auth/authclient.ts b/src/auth/authclient.ts index 752edf64..7df76439 100644 --- a/src/auth/authclient.ts +++ b/src/auth/authclient.ts @@ -435,7 +435,7 @@ export abstract class AuthClient // Logging must not create new errors; swallow them all. } - return error; + throw error; }, }; From f71a4a0694988b6b40e60882f9224aa5815614da Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Wed, 26 Feb 2025 16:50:12 -0500 Subject: [PATCH 16/21] chore: fix various reversions --- src/auth/baseexternalclient.ts | 3 +-- .../defaultawssecuritycredentialssupplier.ts | 2 +- src/auth/oauth2client.ts | 22 +++++++++---------- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/auth/baseexternalclient.ts b/src/auth/baseexternalclient.ts index c4556df1..b1e7d61f 100644 --- a/src/auth/baseexternalclient.ts +++ b/src/auth/baseexternalclient.ts @@ -472,11 +472,10 @@ export abstract class BaseExternalAccountClient extends AuthClient { } else if (projectNumber) { // Preferable not to use request() to avoid retrial policies. const headers = await this.getRequestHeaders(); - const url = `${this.cloudResourceManagerURL.toString()}${projectNumber}`; const opts: GaxiosOptions = { ...BaseExternalAccountClient.RETRY_CONFIG, headers, - url, + url: `${this.cloudResourceManagerURL.toString()}${projectNumber}`, }; AuthClient.setMethodName(opts, 'getProjectId'); const response = await this.transporter.request(opts); diff --git a/src/auth/defaultawssecuritycredentialssupplier.ts b/src/auth/defaultawssecuritycredentialssupplier.ts index 63d41407..a2bc6248 100644 --- a/src/auth/defaultawssecuritycredentialssupplier.ts +++ b/src/auth/defaultawssecuritycredentialssupplier.ts @@ -237,7 +237,7 @@ export class DefaultAwsSecurityCredentialsSupplier async #retrieveAwsSecurityCredentials( roleName: string, headers: Headers, - transporter: Gaxios + transporter: Gaxios, ): Promise { const opts = { ...this.additionalGaxiosOptions, diff --git a/src/auth/oauth2client.ts b/src/auth/oauth2client.ts index 5c99bf3e..5cff7273 100644 --- a/src/auth/oauth2client.ts +++ b/src/auth/oauth2client.ts @@ -752,8 +752,8 @@ export class OAuth2Client extends AuthClient { const opts = { ...OAuth2Client.RETRY_CONFIG, - url, method: 'POST', + url, data: new URLSearchParams(values as {}), headers, }; @@ -813,17 +813,17 @@ export class OAuth2Client extends AuthClient { grant_type: 'refresh_token', }; - const opts: GaxiosOptions = { - ...OAuth2Client.RETRY_CONFIG, - method: 'POST', - url, - data: new URLSearchParams(data), - }; - AuthClient.setMethodName(opts, 'refreshTokenNoCache'); - let res: GaxiosResponse; try { + const opts: GaxiosOptions = { + ...OAuth2Client.RETRY_CONFIG, + method: 'POST', + url, + data: new URLSearchParams(data), + }; + AuthClient.setMethodName(opts, 'refreshTokenNoCache'); + // request for new token res = await this.transporter.request(opts); } catch (e) { @@ -1051,9 +1051,7 @@ export class OAuth2Client extends AuthClient { if (callback) { this.transporter .request(opts) - .then(r => { - callback(null, r); - }, callback); + .then(r => callback(null, r), callback); } else { return this.transporter.request(opts); } From f1e590618f30f1fd6b8250d0bc9fc287190c6533 Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Thu, 27 Feb 2025 13:04:20 -0500 Subject: [PATCH 17/21] chore: fix more linter issues --- src/auth/authclient.ts | 20 ++++++++++++------- .../defaultawssecuritycredentialssupplier.ts | 2 +- src/auth/oauth2client.ts | 2 +- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/auth/authclient.ts b/src/auth/authclient.ts index 7df76439..94e20309 100644 --- a/src/auth/authclient.ts +++ b/src/auth/authclient.ts @@ -225,7 +225,7 @@ export abstract class AuthClient * used to correlate calls and responses. */ static readonly RequestMethodNameSymbol: unique symbol = Symbol( - 'request method name' + 'request method name', ); static readonly RequestLogIdSymbol: unique symbol = Symbol('request log id'); @@ -246,10 +246,10 @@ export abstract class AuthClient if (options.get('useAuthRequestParameters') !== false) { this.transporter.interceptors.request.add( - AuthClient.DEFAULT_REQUEST_INTERCEPTOR + AuthClient.DEFAULT_REQUEST_INTERCEPTOR, ); this.transporter.interceptors.response.add( - AuthClient.DEFAULT_RESPONSE_INTERCEPTOR + AuthClient.DEFAULT_RESPONSE_INTERCEPTOR, ); } @@ -364,7 +364,8 @@ export abstract class AuthClient } try { - const symbols: SymbolIndexString = config as unknown as SymbolIndexString; + const symbols: SymbolIndexString = + config as unknown as SymbolIndexString; const methodName = symbols[AuthClient.RequestMethodNameSymbol]; // This doesn't need to be very unique or interesting, it's just an aid for @@ -378,7 +379,12 @@ export abstract class AuthClient headers: config.headers, }; if (methodName) { - AuthClient.log.info('%s [%s] request %j', methodName, logId, logObject); + AuthClient.log.info( + '%s [%s] request %j', + methodName, + logId, + logObject, + ); } else { AuthClient.log.info('[%s] request %j', logId, logObject); } @@ -404,7 +410,7 @@ export abstract class AuthClient '%s [%s] response %j', methodName, logId, - response.data + response.data, ); } else { AuthClient.log.info('[%s] response %j', logId, response.data); @@ -426,7 +432,7 @@ export abstract class AuthClient '%s [%s] error %j', methodName, logId, - error.response?.data + error.response?.data, ); } else { AuthClient.log.error('[%s] error %j', logId, error.response?.data); diff --git a/src/auth/defaultawssecuritycredentialssupplier.ts b/src/auth/defaultawssecuritycredentialssupplier.ts index a2bc6248..a59818be 100644 --- a/src/auth/defaultawssecuritycredentialssupplier.ts +++ b/src/auth/defaultawssecuritycredentialssupplier.ts @@ -192,7 +192,7 @@ export class DefaultAwsSecurityCredentialsSupplier url: this.imdsV2SessionTokenUrl, method: 'PUT', headers: {'x-aws-ec2-metadata-token-ttl-seconds': '300'}, - }; + }; AuthClient.setMethodName(opts, '#getImdsV2SessionToken'); const response = await transporter.request(opts); return response.data; diff --git a/src/auth/oauth2client.ts b/src/auth/oauth2client.ts index 5cff7273..2c70a8ce 100644 --- a/src/auth/oauth2client.ts +++ b/src/auth/oauth2client.ts @@ -819,7 +819,7 @@ export class OAuth2Client extends AuthClient { const opts: GaxiosOptions = { ...OAuth2Client.RETRY_CONFIG, method: 'POST', - url, + url, data: new URLSearchParams(data), }; AuthClient.setMethodName(opts, 'refreshTokenNoCache'); From e2407f64f56247ac3df523233a2c9f6ba2fad00c Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Thu, 27 Feb 2025 13:11:11 -0500 Subject: [PATCH 18/21] chore: remove extraneous makeLog --- src/auth/authclient.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/auth/authclient.ts b/src/auth/authclient.ts index 94e20309..ffc011fb 100644 --- a/src/auth/authclient.ts +++ b/src/auth/authclient.ts @@ -217,7 +217,6 @@ export abstract class AuthClient eagerRefreshThresholdMillis = DEFAULT_EAGER_REFRESH_THRESHOLD_MILLIS; forceRefreshOnFailure = false; universeDomain = DEFAULT_UNIVERSE; - log = makeLog('auth'); /** * Symbols that can be added to GaxiosOptions to specify the method name that is From c737b47c6ce590fe515b839007bed0cb8b7b1ed9 Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Tue, 4 Mar 2025 13:32:30 -0500 Subject: [PATCH 19/21] tests: add interceptor response shared tests --- test/test.authclient.ts | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/test/test.authclient.ts b/test/test.authclient.ts index f82d8644..9b7a9196 100644 --- a/test/test.authclient.ts +++ b/test/test.authclient.ts @@ -42,8 +42,8 @@ describe('AuthClient', () => { } }); - describe('shared auth interceptor', () => { - it('should use the default interceptor', () => { + describe('shared auth interceptors', () => { + it('should use the default interceptors', () => { const gaxios = new Gaxios(); new PassThroughClient({transporter: gaxios}); @@ -51,11 +51,15 @@ describe('AuthClient', () => { assert( gaxios.interceptors.request.has(AuthClient.DEFAULT_REQUEST_INTERCEPTOR), ); + assert( + gaxios.interceptors.response.has(AuthClient.DEFAULT_RESPONSE_INTERCEPTOR), + ); }); it('should allow disabling of the default interceptor', () => { const gaxios = new Gaxios(); - const originalInterceptorCount = gaxios.interceptors.request.size; + const originalRequestInterceptorCount = gaxios.interceptors.request.size; + const originalResponseInterceptorCount = gaxios.interceptors.response.size; const authClient = new PassThroughClient({ transporter: gaxios, @@ -65,19 +69,26 @@ describe('AuthClient', () => { assert.equal(authClient.transporter, gaxios); assert.equal( authClient.transporter.interceptors.request.size, - originalInterceptorCount, + originalRequestInterceptorCount, + ); + assert.equal( + authClient.transporter.interceptors.response.size, + originalResponseInterceptorCount, ); }); it('should add the default interceptor exactly once between instances', () => { const gaxios = new Gaxios(); - const originalInterceptorCount = gaxios.interceptors.request.size; - const expectedInterceptorCount = originalInterceptorCount + 1; + const originalRequestInterceptorCount = gaxios.interceptors.request.size; + const expectedRequestInterceptorCount = originalRequestInterceptorCount + 1; + const originalResponseInterceptorCount = gaxios.interceptors.response.size; + const expectedResponseInterceptorCount = originalResponseInterceptorCount + 1; new PassThroughClient({transporter: gaxios}); new PassThroughClient({transporter: gaxios}); - assert.equal(gaxios.interceptors.request.size, expectedInterceptorCount); + assert.equal(gaxios.interceptors.request.size, expectedRequestInterceptorCount); + assert.equal(gaxios.interceptors.response.size, expectedResponseInterceptorCount); }); describe('User-Agent', () => { From 3ece9523d574ea8b83f5058eebadc4d0bf5a6502 Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Tue, 4 Mar 2025 16:03:29 -0500 Subject: [PATCH 20/21] tests: add logging tests to interceptors --- src/auth/authclient.ts | 1 + test/test.authclient.ts | 121 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 121 insertions(+), 1 deletion(-) diff --git a/src/auth/authclient.ts b/src/auth/authclient.ts index ffc011fb..820f1495 100644 --- a/src/auth/authclient.ts +++ b/src/auth/authclient.ts @@ -440,6 +440,7 @@ export abstract class AuthClient // Logging must not create new errors; swallow them all. } + // Re-throw the error. throw error; }, }; diff --git a/test/test.authclient.ts b/test/test.authclient.ts index 9b7a9196..083ade30 100644 --- a/test/test.authclient.ts +++ b/test/test.authclient.ts @@ -14,11 +14,36 @@ import {strict as assert} from 'assert'; -import {Gaxios, GaxiosOptionsPrepared} from 'gaxios'; +import {Gaxios, GaxiosError, GaxiosOptionsPrepared, GaxiosResponse} from 'gaxios'; import {AuthClient, PassThroughClient} from '../src'; import {snakeToCamel} from '../src/util'; import {PRODUCT_NAME, USER_AGENT} from '../src/shared.cjs'; +import * as logging from 'google-logging-utils'; + +// Fakes for the logger, to capture logs that would've happened. +interface TestLog { + namespace: string; + fields: logging.LogFields; + args: unknown[]; +} + +class TestLogSink extends logging.DebugLogBackendBase { + logs: TestLog[] = []; + + makeLogger(namespace: string): logging.AdhocDebugLogCallable { + return (fields: logging.LogFields, ...args: unknown[]) => { + this.logs.push({namespace, fields, args}); + }; + } + + setFilters(): void {} + + reset() { + this.filters = []; + this.logs = []; + } +} describe('AuthClient', () => { it('should accept and normalize snake case options to camel case', () => { @@ -162,5 +187,99 @@ describe('AuthClient', () => { assert.equal(options.headers.get('x-goog-api-client'), expected); }); }); + + describe('logging', () => { + // Enable and capture any log lines that happen during these tests. + let testLogSink: TestLogSink; + let replacementLogger: logging.AdhocDebugLogFunction; + beforeEach(() => { + process.env[logging.env.nodeEnables] = 'auth'; + testLogSink = new TestLogSink(); + logging.setBackend(testLogSink); + replacementLogger = logging.log('auth'); + }); + after(() => { + delete process.env[logging.env.nodeEnables]; + logging.setBackend(null); + }) + + it('logs requests', async () => { + const options: GaxiosOptionsPrepared = { + headers: new Headers({ + 'x-goog-api-client': 'something', + }), + url: new URL('https://google.com'), + }; + AuthClient.setMethodName(options, 'testMethod'); + + // This will become nicer with the 1.1.0 release of google-logging-utils. + AuthClient.log = replacementLogger; + const returned = await AuthClient.DEFAULT_REQUEST_INTERCEPTOR?.resolved?.(options); + assert.strictEqual(returned, options); + + // Unfortunately, there is a fair amount of entropy and changeable formatting in the + // actual logs, so this mostly validates that a few key pieces of info are in there. + assert.deepStrictEqual(testLogSink.logs.length, 1); + assert.deepStrictEqual(testLogSink.logs[0].namespace, 'auth'); + assert.deepStrictEqual(testLogSink.logs[0].args.length, 4); + assert.strictEqual((testLogSink.logs[0].args[0] as string).includes('request'), true); + assert.deepStrictEqual(testLogSink.logs[0].args[1], 'testMethod'); + assert.deepStrictEqual((testLogSink.logs[0].args[3] as GaxiosOptionsPrepared).headers.get('x-goog-api-client'), 'something'); + assert.deepStrictEqual((testLogSink.logs[0].args[3] as GaxiosOptionsPrepared).url.href, 'https://google.com/'); + }); + + it('logs responses', async () => { + const response = { + config: { + headers: new Headers({ + 'x-goog-api-client': 'something', + }), + url: new URL('https://google.com'), + } as GaxiosOptionsPrepared, + headers: new Headers({ + 'x-goog-api-client': 'something', + }), + url: new URL('https://google.com'), + data: { + test:'test!' + }, + } as unknown as GaxiosResponse<{test: string}>; + AuthClient.setMethodName(response.config, 'testMethod'); + + // This will become nicer with the 1.1.0 release of google-logging-utils. + AuthClient.log = replacementLogger; + const resolvedReturned = await AuthClient.DEFAULT_RESPONSE_INTERCEPTOR?.resolved?.(response); + assert.strictEqual(resolvedReturned, response); + + // Unfortunately, there is a fair amount of entropy and changeable formatting in the + // actual logs, so this mostly validates that a few key pieces of info are in there. + assert.deepStrictEqual(testLogSink.logs.length, 1); + assert.deepStrictEqual(testLogSink.logs[0].namespace, 'auth'); + assert.deepStrictEqual(testLogSink.logs[0].args.length, 4); + assert.strictEqual((testLogSink.logs[0].args[0] as string).includes('response'), true); + assert.deepStrictEqual(testLogSink.logs[0].args[1], 'testMethod'); + assert.deepStrictEqual((testLogSink.logs[0].args[3] as {test: string}), {test: 'test!'}); + + const error = { + config: response.config, + response: { + data: { + message: 'boo!', + } + } + } as unknown as GaxiosError<{test: string}>; + testLogSink.reset(); + AuthClient.DEFAULT_RESPONSE_INTERCEPTOR?.rejected?.(error); + + // Unfortunately, there is a fair amount of entropy and changeable formatting in the + // actual logs, so this mostly validates that a few key pieces of info are in there. + assert.deepStrictEqual(testLogSink.logs.length, 1); + assert.deepStrictEqual(testLogSink.logs[0].namespace, 'auth'); + assert.deepStrictEqual(testLogSink.logs[0].args.length, 4); + assert.strictEqual((testLogSink.logs[0].args[0] as string).includes('error'), true); + assert.deepStrictEqual(testLogSink.logs[0].args[1], 'testMethod'); + assert.deepStrictEqual((testLogSink.logs[0].args[3] as {test: string}), {message: 'boo!'}); + }); + }); }); }); From fbc06233548cd45f2b2a3fc1072030d822591bd0 Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Tue, 4 Mar 2025 16:06:35 -0500 Subject: [PATCH 21/21] tests: lint fixes --- test/test.authclient.ts | 82 ++++++++++++++++++++++++++++++----------- 1 file changed, 61 insertions(+), 21 deletions(-) diff --git a/test/test.authclient.ts b/test/test.authclient.ts index 083ade30..235bfc9b 100644 --- a/test/test.authclient.ts +++ b/test/test.authclient.ts @@ -14,7 +14,12 @@ import {strict as assert} from 'assert'; -import {Gaxios, GaxiosError, GaxiosOptionsPrepared, GaxiosResponse} from 'gaxios'; +import { + Gaxios, + GaxiosError, + GaxiosOptionsPrepared, + GaxiosResponse, +} from 'gaxios'; import {AuthClient, PassThroughClient} from '../src'; import {snakeToCamel} from '../src/util'; @@ -77,14 +82,17 @@ describe('AuthClient', () => { gaxios.interceptors.request.has(AuthClient.DEFAULT_REQUEST_INTERCEPTOR), ); assert( - gaxios.interceptors.response.has(AuthClient.DEFAULT_RESPONSE_INTERCEPTOR), + gaxios.interceptors.response.has( + AuthClient.DEFAULT_RESPONSE_INTERCEPTOR, + ), ); }); it('should allow disabling of the default interceptor', () => { const gaxios = new Gaxios(); const originalRequestInterceptorCount = gaxios.interceptors.request.size; - const originalResponseInterceptorCount = gaxios.interceptors.response.size; + const originalResponseInterceptorCount = + gaxios.interceptors.response.size; const authClient = new PassThroughClient({ transporter: gaxios, @@ -105,15 +113,24 @@ describe('AuthClient', () => { it('should add the default interceptor exactly once between instances', () => { const gaxios = new Gaxios(); const originalRequestInterceptorCount = gaxios.interceptors.request.size; - const expectedRequestInterceptorCount = originalRequestInterceptorCount + 1; - const originalResponseInterceptorCount = gaxios.interceptors.response.size; - const expectedResponseInterceptorCount = originalResponseInterceptorCount + 1; + const expectedRequestInterceptorCount = + originalRequestInterceptorCount + 1; + const originalResponseInterceptorCount = + gaxios.interceptors.response.size; + const expectedResponseInterceptorCount = + originalResponseInterceptorCount + 1; new PassThroughClient({transporter: gaxios}); new PassThroughClient({transporter: gaxios}); - assert.equal(gaxios.interceptors.request.size, expectedRequestInterceptorCount); - assert.equal(gaxios.interceptors.response.size, expectedResponseInterceptorCount); + assert.equal( + gaxios.interceptors.request.size, + expectedRequestInterceptorCount, + ); + assert.equal( + gaxios.interceptors.response.size, + expectedResponseInterceptorCount, + ); }); describe('User-Agent', () => { @@ -201,7 +218,7 @@ describe('AuthClient', () => { after(() => { delete process.env[logging.env.nodeEnables]; logging.setBackend(null); - }) + }); it('logs requests', async () => { const options: GaxiosOptionsPrepared = { @@ -214,7 +231,8 @@ describe('AuthClient', () => { // This will become nicer with the 1.1.0 release of google-logging-utils. AuthClient.log = replacementLogger; - const returned = await AuthClient.DEFAULT_REQUEST_INTERCEPTOR?.resolved?.(options); + const returned = + await AuthClient.DEFAULT_REQUEST_INTERCEPTOR?.resolved?.(options); assert.strictEqual(returned, options); // Unfortunately, there is a fair amount of entropy and changeable formatting in the @@ -222,10 +240,21 @@ describe('AuthClient', () => { assert.deepStrictEqual(testLogSink.logs.length, 1); assert.deepStrictEqual(testLogSink.logs[0].namespace, 'auth'); assert.deepStrictEqual(testLogSink.logs[0].args.length, 4); - assert.strictEqual((testLogSink.logs[0].args[0] as string).includes('request'), true); + assert.strictEqual( + (testLogSink.logs[0].args[0] as string).includes('request'), + true, + ); assert.deepStrictEqual(testLogSink.logs[0].args[1], 'testMethod'); - assert.deepStrictEqual((testLogSink.logs[0].args[3] as GaxiosOptionsPrepared).headers.get('x-goog-api-client'), 'something'); - assert.deepStrictEqual((testLogSink.logs[0].args[3] as GaxiosOptionsPrepared).url.href, 'https://google.com/'); + assert.deepStrictEqual( + (testLogSink.logs[0].args[3] as GaxiosOptionsPrepared).headers.get( + 'x-goog-api-client', + ), + 'something', + ); + assert.deepStrictEqual( + (testLogSink.logs[0].args[3] as GaxiosOptionsPrepared).url.href, + 'https://google.com/', + ); }); it('logs responses', async () => { @@ -241,14 +270,15 @@ describe('AuthClient', () => { }), url: new URL('https://google.com'), data: { - test:'test!' + test: 'test!', }, } as unknown as GaxiosResponse<{test: string}>; AuthClient.setMethodName(response.config, 'testMethod'); // This will become nicer with the 1.1.0 release of google-logging-utils. AuthClient.log = replacementLogger; - const resolvedReturned = await AuthClient.DEFAULT_RESPONSE_INTERCEPTOR?.resolved?.(response); + const resolvedReturned = + await AuthClient.DEFAULT_RESPONSE_INTERCEPTOR?.resolved?.(response); assert.strictEqual(resolvedReturned, response); // Unfortunately, there is a fair amount of entropy and changeable formatting in the @@ -256,17 +286,22 @@ describe('AuthClient', () => { assert.deepStrictEqual(testLogSink.logs.length, 1); assert.deepStrictEqual(testLogSink.logs[0].namespace, 'auth'); assert.deepStrictEqual(testLogSink.logs[0].args.length, 4); - assert.strictEqual((testLogSink.logs[0].args[0] as string).includes('response'), true); + assert.strictEqual( + (testLogSink.logs[0].args[0] as string).includes('response'), + true, + ); assert.deepStrictEqual(testLogSink.logs[0].args[1], 'testMethod'); - assert.deepStrictEqual((testLogSink.logs[0].args[3] as {test: string}), {test: 'test!'}); + assert.deepStrictEqual(testLogSink.logs[0].args[3] as {test: string}, { + test: 'test!', + }); const error = { config: response.config, response: { data: { message: 'boo!', - } - } + }, + }, } as unknown as GaxiosError<{test: string}>; testLogSink.reset(); AuthClient.DEFAULT_RESPONSE_INTERCEPTOR?.rejected?.(error); @@ -276,9 +311,14 @@ describe('AuthClient', () => { assert.deepStrictEqual(testLogSink.logs.length, 1); assert.deepStrictEqual(testLogSink.logs[0].namespace, 'auth'); assert.deepStrictEqual(testLogSink.logs[0].args.length, 4); - assert.strictEqual((testLogSink.logs[0].args[0] as string).includes('error'), true); + assert.strictEqual( + (testLogSink.logs[0].args[0] as string).includes('error'), + true, + ); assert.deepStrictEqual(testLogSink.logs[0].args[1], 'testMethod'); - assert.deepStrictEqual((testLogSink.logs[0].args[3] as {test: string}), {message: 'boo!'}); + assert.deepStrictEqual(testLogSink.logs[0].args[3] as {test: string}, { + message: 'boo!', + }); }); }); });