diff --git a/src/auth/authclient.ts b/src/auth/authclient.ts index 29e1602a..8e004d63 100644 --- a/src/auth/authclient.ts +++ b/src/auth/authclient.ts @@ -259,4 +259,22 @@ export abstract class AuthClient } return headers; } + + /** + * Retry config for Auth-related requests. + * + * @remarks + * + * This is not a part of the default {@link AuthClient.transporter transporter/gaxios} + * config as some downstream APIs would prefer if customers explicitly enable retries, + * such as GCS. + */ + protected static get RETRY_CONFIG(): GaxiosOptions { + return { + retry: true, + retryConfig: { + httpMethodsToRetry: ['GET', 'PUT', 'POST', 'HEAD', 'OPTIONS', 'DELETE'], + }, + }; + } } diff --git a/src/auth/awsclient.ts b/src/auth/awsclient.ts index dc2e0eee..f9f899d9 100644 --- a/src/auth/awsclient.ts +++ b/src/auth/awsclient.ts @@ -192,6 +192,7 @@ export class AwsClient extends BaseExternalAccountClient { // Generate signed request to AWS STS GetCallerIdentity API. // Use the required regional endpoint. Otherwise, the request will fail. const options = await this.awsRequestSigner.getRequestOptions({ + ...AwsClient.RETRY_CONFIG, url: this.regionalCredVerificationUrl.replace('{region}', this.region), method: 'POST', }); @@ -240,6 +241,7 @@ export class AwsClient extends BaseExternalAccountClient { */ private async getImdsV2SessionToken(): Promise { const opts: GaxiosOptions = { + ...AwsClient.RETRY_CONFIG, url: this.imdsV2SessionTokenUrl, method: 'PUT', responseType: 'text', @@ -266,6 +268,7 @@ export class AwsClient extends BaseExternalAccountClient { ); } const opts: GaxiosOptions = { + ...AwsClient.RETRY_CONFIG, url: this.regionUrl, method: 'GET', responseType: 'text', @@ -290,6 +293,7 @@ export class AwsClient extends BaseExternalAccountClient { ); } const opts: GaxiosOptions = { + ...AwsClient.RETRY_CONFIG, url: this.securityCredentialsUrl, method: 'GET', responseType: 'text', @@ -313,6 +317,7 @@ export class AwsClient extends BaseExternalAccountClient { ): Promise { const response = await this.transporter.request({ + ...AwsClient.RETRY_CONFIG, url: `${this.securityCredentialsUrl}/${roleName}`, responseType: 'json', headers: headers, diff --git a/src/auth/baseexternalclient.ts b/src/auth/baseexternalclient.ts index 45ff17ff..94789556 100644 --- a/src/auth/baseexternalclient.ts +++ b/src/auth/baseexternalclient.ts @@ -381,6 +381,7 @@ export abstract class BaseExternalAccountClient extends AuthClient { // Preferable not to use request() to avoid retrial policies. const headers = await this.getRequestHeaders(); const response = await this.transporter.request({ + ...BaseExternalAccountClient.RETRY_CONFIG, headers, url: `${this.cloudResourceManagerURL.toString()}${projectNumber}`, responseType: 'json', @@ -395,12 +396,12 @@ export abstract class BaseExternalAccountClient extends AuthClient { * Authenticates the provided HTTP request, processes it and resolves with the * returned response. * @param opts The HTTP request options. - * @param retry Whether the current attempt is a retry after a failed attempt. + * @param reAuthRetried Whether the current attempt is a retry after a failed attempt due to an auth failure. * @return A promise that resolves with the successful response. */ protected async requestAsync( opts: GaxiosOptions, - retry = false + reAuthRetried = false ): Promise> { let response: GaxiosResponse; try { @@ -426,7 +427,7 @@ export abstract class BaseExternalAccountClient extends AuthClient { const isReadableStream = res.config.data instanceof stream.Readable; const isAuthErr = statusCode === 401 || statusCode === 403; if ( - !retry && + !reAuthRetried && isAuthErr && !isReadableStream && this.forceRefreshOnFailure @@ -554,6 +555,7 @@ export abstract class BaseExternalAccountClient extends AuthClient { token: string ): Promise { const opts: GaxiosOptions = { + ...BaseExternalAccountClient.RETRY_CONFIG, url: this.serviceAccountImpersonationUrl!, method: 'POST', headers: { diff --git a/src/auth/downscopedclient.ts b/src/auth/downscopedclient.ts index 2e2716f1..3005e7cc 100644 --- a/src/auth/downscopedclient.ts +++ b/src/auth/downscopedclient.ts @@ -250,12 +250,12 @@ export class DownscopedClient extends AuthClient { * Authenticates the provided HTTP request, processes it and resolves with the * returned response. * @param opts The HTTP request options. - * @param retry Whether the current attempt is a retry after a failed attempt. + * @param reAuthRetried Whether the current attempt is a retry after a failed attempt due to an auth failure * @return A promise that resolves with the successful response. */ protected async requestAsync( opts: GaxiosOptions, - retry = false + reAuthRetried = false ): Promise> { let response: GaxiosResponse; try { @@ -281,7 +281,7 @@ export class DownscopedClient extends AuthClient { const isReadableStream = res.config.data instanceof stream.Readable; const isAuthErr = statusCode === 401 || statusCode === 403; if ( - !retry && + !reAuthRetried && isAuthErr && !isReadableStream && this.forceRefreshOnFailure diff --git a/src/auth/externalAccountAuthorizedUserClient.ts b/src/auth/externalAccountAuthorizedUserClient.ts index c9534440..e338164c 100644 --- a/src/auth/externalAccountAuthorizedUserClient.ts +++ b/src/auth/externalAccountAuthorizedUserClient.ts @@ -113,6 +113,7 @@ class ExternalAccountAuthorizedUserHandler extends OAuthClientAuthHandler { }; const opts: GaxiosOptions = { + ...ExternalAccountAuthorizedUserHandler.RETRY_CONFIG, url: this.url, method: 'POST', headers, @@ -248,12 +249,12 @@ export class ExternalAccountAuthorizedUserClient extends AuthClient { * Authenticates the provided HTTP request, processes it and resolves with the * returned response. * @param opts The HTTP request options. - * @param retry Whether the current attempt is a retry after a failed attempt. + * @param reAuthRetried Whether the current attempt is a retry after a failed attempt due to an auth failure. * @return A promise that resolves with the successful response. */ protected async requestAsync( opts: GaxiosOptions, - retry = false + reAuthRetried = false ): Promise> { let response: GaxiosResponse; try { @@ -279,7 +280,7 @@ export class ExternalAccountAuthorizedUserClient extends AuthClient { const isReadableStream = res.config.data instanceof stream.Readable; const isAuthErr = statusCode === 401 || statusCode === 403; if ( - !retry && + !reAuthRetried && isAuthErr && !isReadableStream && this.forceRefreshOnFailure diff --git a/src/auth/googleauth.ts b/src/auth/googleauth.ts index 472ae789..f3faedac 100644 --- a/src/auth/googleauth.ts +++ b/src/auth/googleauth.ts @@ -1130,6 +1130,10 @@ export class GoogleAuth { data: { payload: crypto.encodeBase64StringUtf8(data), }, + retry: true, + retryConfig: { + httpMethodsToRetry: ['POST'], + }, }); return res.data.signedBlob; diff --git a/src/auth/identitypoolclient.ts b/src/auth/identitypoolclient.ts index 2ed66b11..0e89f544 100644 --- a/src/auth/identitypoolclient.ts +++ b/src/auth/identitypoolclient.ts @@ -225,6 +225,7 @@ export class IdentityPoolClient extends BaseExternalAccountClient { headers?: {[key: string]: string} ): Promise { const opts: GaxiosOptions = { + ...IdentityPoolClient.RETRY_CONFIG, url, method: 'GET', headers, diff --git a/src/auth/impersonated.ts b/src/auth/impersonated.ts index ac24fd7a..2b4a2555 100644 --- a/src/auth/impersonated.ts +++ b/src/auth/impersonated.ts @@ -143,6 +143,7 @@ export class Impersonated extends OAuth2Client implements IdTokenProvider { payload: Buffer.from(blobToSign).toString('base64'), }; const res = await this.sourceClient.request({ + ...Impersonated.RETRY_CONFIG, url: u, data: body, method: 'POST', @@ -157,11 +158,8 @@ export class Impersonated extends OAuth2Client implements IdTokenProvider { /** * Refreshes the access token. - * @param refreshToken Unused parameter */ - protected async refreshToken( - refreshToken?: string | null - ): Promise { + protected async refreshToken(): Promise { try { await this.sourceClient.getAccessToken(); const name = 'projects/-/serviceAccounts/' + this.targetPrincipal; @@ -172,6 +170,7 @@ export class Impersonated extends OAuth2Client implements IdTokenProvider { lifetime: this.lifetime + 's', }; const res = await this.sourceClient.request({ + ...Impersonated.RETRY_CONFIG, url: u, data: body, method: 'POST', @@ -227,6 +226,7 @@ export class Impersonated extends OAuth2Client implements IdTokenProvider { includeEmail: options?.includeEmail ?? true, }; const res = await this.sourceClient.request({ + ...Impersonated.RETRY_CONFIG, url: u, data: body, method: 'POST', diff --git a/src/auth/oauth2client.ts b/src/auth/oauth2client.ts index c9f87d56..ce1fb829 100644 --- a/src/auth/oauth2client.ts +++ b/src/auth/oauth2client.ts @@ -669,6 +669,7 @@ export class OAuth2Client extends AuthClient { code_verifier: options.codeVerifier, }; const res = await this.transporter.request({ + ...OAuth2Client.RETRY_CONFIG, method: 'POST', url, data: querystring.stringify(values), @@ -733,6 +734,7 @@ export class OAuth2Client extends AuthClient { try { // request for new token res = await this.transporter.request({ + ...OAuth2Client.RETRY_CONFIG, method: 'POST', url, data: querystring.stringify(data), @@ -956,6 +958,7 @@ export class OAuth2Client extends AuthClient { callback?: BodyResponseCallback ): GaxiosPromise | void { const opts: GaxiosOptions = { + ...OAuth2Client.RETRY_CONFIG, url: this.getRevokeTokenURL(token).toString(), method: 'POST', }; @@ -1024,7 +1027,7 @@ export class OAuth2Client extends AuthClient { protected async requestAsync( opts: GaxiosOptions, - retry = false + reAuthRetried = false ): Promise> { let r2: GaxiosResponse; try { @@ -1078,11 +1081,16 @@ export class OAuth2Client extends AuthClient { this.refreshHandler; const isReadableStream = res.config.data instanceof stream.Readable; const isAuthErr = statusCode === 401 || statusCode === 403; - if (!retry && isAuthErr && !isReadableStream && mayRequireRefresh) { + if ( + !reAuthRetried && + isAuthErr && + !isReadableStream && + mayRequireRefresh + ) { await this.refreshAccessTokenAsync(); return this.requestAsync(opts, true); } else if ( - !retry && + !reAuthRetried && isAuthErr && !isReadableStream && mayRequireRefreshWithNoRefreshToken @@ -1157,6 +1165,7 @@ export class OAuth2Client extends AuthClient { */ async getTokenInfo(accessToken: string): Promise { const {data} = await this.transporter.request({ + ...OAuth2Client.RETRY_CONFIG, method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded', @@ -1222,7 +1231,10 @@ export class OAuth2Client extends AuthClient { throw new Error(`Unsupported certificate format ${format}`); } try { - res = await this.transporter.request({url}); + res = await this.transporter.request({ + ...OAuth2Client.RETRY_CONFIG, + url, + }); } catch (e) { if (e instanceof Error) { e.message = `Failed to retrieve verification certificates: ${e.message}`; @@ -1290,7 +1302,10 @@ export class OAuth2Client extends AuthClient { const url = this.endpoints.oauth2IapPublicKeyUrl.toString(); try { - res = await this.transporter.request({url}); + res = await this.transporter.request({ + ...OAuth2Client.RETRY_CONFIG, + url, + }); } 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 34d1bb6d..19f4fe8e 100644 --- a/src/auth/oauth2common.ts +++ b/src/auth/oauth2common.ts @@ -181,6 +181,24 @@ export abstract class OAuthClientAuthHandler { } } } + + /** + * Retry config for Auth-related requests. + * + * @remarks + * + * This is not a part of the default {@link AuthClient.transporter transporter/gaxios} + * config as some downstream APIs would prefer if customers explicitly enable retries, + * such as GCS. + */ + protected static get RETRY_CONFIG(): GaxiosOptions { + return { + retry: true, + retryConfig: { + httpMethodsToRetry: ['GET', 'PUT', 'POST', 'HEAD', 'OPTIONS', 'DELETE'], + }, + }; + } } /** diff --git a/src/auth/stscredentials.ts b/src/auth/stscredentials.ts index a075eae1..291da246 100644 --- a/src/auth/stscredentials.ts +++ b/src/auth/stscredentials.ts @@ -195,6 +195,7 @@ export class StsCredentials extends OAuthClientAuthHandler { Object.assign(headers, additionalHeaders || {}); const opts: GaxiosOptions = { + ...StsCredentials.RETRY_CONFIG, url: this.tokenExchangeEndpoint.toString(), method: 'POST', headers, diff --git a/test/test.awsclient.ts b/test/test.awsclient.ts index 800f670b..fb00cadd 100644 --- a/test/test.awsclient.ts +++ b/test/test.awsclient.ts @@ -401,12 +401,12 @@ describe('AwsClient', () => { // Simulate error during region retrieval. const scope = nock(metadataBaseUrl) .get('/latest/meta-data/placement/availability-zone') - .reply(500); + .reply(400); const client = new AwsClient(awsOptions); await assert.rejects(client.retrieveSubjectToken(), { - status: 500, + status: 400, }); scope.done(); }); @@ -435,12 +435,12 @@ describe('AwsClient', () => { .get('/latest/meta-data/iam/security-credentials') .reply(200, awsRole) .get(`/latest/meta-data/iam/security-credentials/${awsRole}`) - .reply(408); + .reply(404); const client = new AwsClient(awsOptions); await assert.rejects(client.retrieveSubjectToken(), { - status: 408, + status: 404, }); scope.done(); }); @@ -602,12 +602,12 @@ describe('AwsClient', () => { .get('/latest/meta-data/placement/availability-zone') .reply(200, `${awsRegion}b`) .get('/latest/meta-data/iam/security-credentials') - .reply(500); + .reply(400); const client = new AwsClient(awsOptions); await assert.rejects(client.getAccessToken(), { - status: 500, + status: 400, }); scope.done(); }); @@ -704,12 +704,12 @@ describe('AwsClient', () => { // Simulate error during region retrieval. const scope = nock(metadataBaseUrl) .get('/latest/meta-data/placement/availability-zone') - .reply(500); + .reply(400); const client = new AwsClient(awsOptions); await assert.rejects(client.retrieveSubjectToken(), { - status: 500, + status: 400, }); scope.done(); }); @@ -982,12 +982,12 @@ describe('AwsClient', () => { const scope = nock(metadataBaseUrl) .get('/latest/meta-data/placement/availability-zone') - .reply(500); + .reply(400); const client = new AwsClient(awsOptions); await assert.rejects(client.getAccessToken(), { - status: 500, + status: 400, }); scope.done(); }); diff --git a/test/test.downscopedclient.ts b/test/test.downscopedclient.ts index b808be72..067155b3 100644 --- a/test/test.downscopedclient.ts +++ b/test/test.downscopedclient.ts @@ -17,7 +17,7 @@ import {describe, it, beforeEach, afterEach} from 'mocha'; import * as nock from 'nock'; import * as sinon from 'sinon'; -import {GaxiosError, GaxiosOptions, GaxiosPromise} from 'gaxios'; +import {GaxiosError, GaxiosPromise} from 'gaxios'; import {Credentials} from '../src/auth/credentials'; import {StsSuccessfulResponse} from '../src/auth/stscredentials'; import { diff --git a/test/test.externalaccountauthorizeduserclient.ts b/test/test.externalaccountauthorizeduserclient.ts index c97e4b03..5d0e7920 100644 --- a/test/test.externalaccountauthorizeduserclient.ts +++ b/test/test.externalaccountauthorizeduserclient.ts @@ -227,7 +227,10 @@ describe('ExternalAccountAuthorizedUserClient', () => { scope.done(); }); - it('should handle refresh timeout', async () => { + it('should handle and retry on timeout', async () => { + // we need timers/`setTimeout` for this test + clock.restore(); + const expectedRequest = new URLSearchParams({ grant_type: 'refresh_token', refresh_token: 'refreshToken', @@ -239,13 +242,19 @@ describe('ExternalAccountAuthorizedUserClient', () => { }, }) .post(REFRESH_PATH, expectedRequest.toString()) - .replyWithError({code: 'ETIMEDOUT'}); + .replyWithError({code: 'ETIMEDOUT'}) + .post(REFRESH_PATH, expectedRequest.toString()) + .reply(200, successfulRefreshResponse); const client = new ExternalAccountAuthorizedUserClient( externalAccountAuthorizedUserCredentialOptions ); - await assert.rejects(client.getAccessToken(), { - code: 'ETIMEDOUT', + + const actualResponse = await client.getAccessToken(); + assertGaxiosResponsePresent(actualResponse); + delete actualResponse.res; + assert.deepStrictEqual(actualResponse, { + token: successfulRefreshResponse.access_token, }); scope.done(); }); diff --git a/test/test.jwt.ts b/test/test.jwt.ts index 13b4695e..1d1ee013 100644 --- a/test/test.jwt.ts +++ b/test/test.jwt.ts @@ -200,7 +200,9 @@ describe('jwt', () => { const got = await jwt.getRequestHeaders(testUri); assert.notStrictEqual(null, got, 'the creds should be present'); const decoded = jws.decode(got.Authorization.replace('Bearer ', '')); - assert.deepStrictEqual({alg: 'RS256', typ: 'JWT'}, decoded.header); + assert(decoded); + assert.strictEqual(decoded.header.alg, 'RS256'); + assert.strictEqual(decoded.header.typ, 'JWT'); const payload = decoded.payload; assert.strictEqual(email, payload.iss); assert.strictEqual(email, payload.sub); @@ -221,10 +223,12 @@ describe('jwt', () => { const got = await jwt.getRequestHeaders(testUri); assert.notStrictEqual(null, got, 'the creds should be present'); const decoded = jws.decode(got.Authorization.replace('Bearer ', '')); - assert.deepStrictEqual( - {alg: 'RS256', typ: 'JWT', kid: '101'}, - decoded.header - ); + assert(decoded); + assert.deepStrictEqual(decoded.header, { + alg: 'RS256', + typ: 'JWT', + kid: '101', + }); }); it('should accept additionalClaims', async () => { @@ -244,6 +248,7 @@ describe('jwt', () => { const got = await jwt.getRequestHeaders(testUri); assert.notStrictEqual(null, got, 'the creds should be present'); const decoded = jws.decode(got.Authorization.replace('Bearer ', '')); + assert(decoded); const payload = decoded.payload; assert.strictEqual(testDefault, payload.aud); assert.strictEqual(someClaim, payload.someClaim); diff --git a/test/test.jwtaccess.ts b/test/test.jwtaccess.ts index 1434f256..d441e3b0 100644 --- a/test/test.jwtaccess.ts +++ b/test/test.jwtaccess.ts @@ -49,7 +49,8 @@ describe('jwtaccess', () => { const headers = client.getRequestHeaders(testUri); assert.notStrictEqual(null, headers, 'an creds object should be present'); const decoded = jws.decode(headers.Authorization.replace('Bearer ', '')); - assert.deepStrictEqual({alg: 'RS256', typ: 'JWT'}, decoded.header); + assert(decoded); + assert.deepStrictEqual(decoded.header, {alg: 'RS256', typ: 'JWT'}); const payload = decoded.payload; assert.strictEqual(email, payload.iss); assert.strictEqual(email, payload.sub); @@ -60,6 +61,7 @@ describe('jwtaccess', () => { const client = new JWTAccess(email, keys.private); const headers = client.getRequestHeaders(testUri, undefined, 'myfakescope'); const decoded = jws.decode(headers.Authorization.replace('Bearer ', '')); + assert(decoded); const payload = decoded.payload; assert.strictEqual('myfakescope', payload.scope); }); @@ -68,6 +70,7 @@ describe('jwtaccess', () => { const client = new JWTAccess(email, keys.private); const headers = client.getRequestHeaders(testUri); const decoded = jws.decode(headers.Authorization.replace('Bearer ', '')); + assert(decoded); const payload = decoded.payload; assert.strictEqual(testUri, payload.aud); }); @@ -76,10 +79,12 @@ describe('jwtaccess', () => { const client = new JWTAccess(email, keys.private, '101'); const headers = client.getRequestHeaders(testUri); const decoded = jws.decode(headers.Authorization.replace('Bearer ', '')); - assert.deepStrictEqual( - {alg: 'RS256', typ: 'JWT', kid: '101'}, - decoded.header - ); + assert(decoded); + assert.deepStrictEqual(decoded.header, { + alg: 'RS256', + typ: 'JWT', + kid: '101', + }); }); it('getRequestHeaders should not allow overriding with additionalClaims', () => { diff --git a/test/test.oauth2.ts b/test/test.oauth2.ts index 6156b8aa..b836656f 100644 --- a/test/test.oauth2.ts +++ b/test/test.oauth2.ts @@ -992,7 +992,7 @@ describe('oauth2', () => { reqheaders: {'content-type': 'application/x-www-form-urlencoded'}, }) .post('/token') - .reply(500) + .reply(400) .post('/token') .reply(200, {access_token: 'abc123', expires_in: 100000}), nock('http://example.com').get('/').reply(200), @@ -1020,7 +1020,7 @@ describe('oauth2', () => { reqheaders: {'content-type': 'application/x-www-form-urlencoded'}, }) .post('/token') - .reply(500, reAuthErrorBody), + .reply(400, reAuthErrorBody), ]; client.credentials = {refresh_token: 'refresh-token-placeholder'}; diff --git a/test/test.stscredentials.ts b/test/test.stscredentials.ts index 72e17119..4aafd20d 100644 --- a/test/test.stscredentials.ts +++ b/test/test.stscredentials.ts @@ -216,26 +216,31 @@ describe('StsCredentials', () => { scope.done(); }); - it('should handle request timeout', async () => { + it('should handle and retry on timeout', async () => { const scope = nock(baseUrl) .post(path, qs.stringify(expectedRequest), { reqheaders: { 'content-type': 'application/x-www-form-urlencoded', }, }) - .replyWithError({code: 'ETIMEDOUT'}); + .replyWithError({code: 'ETIMEDOUT'}) + .post(path, qs.stringify(expectedRequest), { + reqheaders: { + 'content-type': 'application/x-www-form-urlencoded', + }, + }) + .reply(200, stsSuccessfulResponse); const stsCredentials = new StsCredentials(tokenExchangeEndpoint); - await assert.rejects( - stsCredentials.exchangeToken( - stsCredentialsOptions, - additionalHeaders, - options - ), - { - code: 'ETIMEDOUT', - } + const resp = await stsCredentials.exchangeToken( + stsCredentialsOptions, + additionalHeaders, + options ); + + assertGaxiosResponsePresent(resp); + delete resp.res; + assert.deepStrictEqual(resp, stsSuccessfulResponse); scope.done(); }); });