From a278d19a0b211b13e5cf5176d40128e704b55780 Mon Sep 17 00:00:00 2001 From: aeitzman <12433791+aeitzman@users.noreply.github.com> Date: Fri, 3 Feb 2023 13:42:59 -0800 Subject: [PATCH] fix: Removing 3pi config URL validation (#1517) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Removing url validation * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix: minor change to language in documentation * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot --- .readme-partials.yaml | 3 + README.md | 3 + src/auth/baseexternalclient.ts | 80 ------------------- test/test.baseexternalclient.ts | 133 -------------------------------- 4 files changed, 6 insertions(+), 213 deletions(-) diff --git a/.readme-partials.yaml b/.readme-partials.yaml index f73558b1..3284dc89 100644 --- a/.readme-partials.yaml +++ b/.readme-partials.yaml @@ -888,6 +888,9 @@ body: |- } ``` + #### Security Considerations + Note that this library does not perform any validation on the token_url, token_info_url, or service_account_impersonation_url fields of the credential configuration. It is not recommended to use a credential configuration that you did not generate with the gcloud CLI unless you verify that the URL fields point to a googleapis.com domain. + ## Working with ID Tokens ### Fetching ID Tokens If your application is running on Cloud Run or Cloud Functions, or using Cloud Identity-Aware diff --git a/README.md b/README.md index b6264233..13539195 100644 --- a/README.md +++ b/README.md @@ -932,6 +932,9 @@ async function main() { } ``` +#### Security Considerations +Note that this library does not perform any validation on the token_url, token_info_url, or service_account_impersonation_url fields of the credential configuration. It is not recommended to use a credential configuration that you did not generate with the gcloud CLI unless you verify that the URL fields point to a googleapis.com domain. + ## Working with ID Tokens ### Fetching ID Tokens If your application is running on Cloud Run or Cloud Functions, or using Cloud Identity-Aware diff --git a/src/auth/baseexternalclient.ts b/src/auth/baseexternalclient.ts index 76841280..ddd79d8e 100644 --- a/src/auth/baseexternalclient.ts +++ b/src/auth/baseexternalclient.ts @@ -37,10 +37,6 @@ const STS_GRANT_TYPE = 'urn:ietf:params:oauth:grant-type:token-exchange'; const STS_REQUEST_TOKEN_TYPE = 'urn:ietf:params:oauth:token-type:access_token'; /** The default OAuth scope to request when none is provided. */ const DEFAULT_OAUTH_SCOPE = 'https://www.googleapis.com/auth/cloud-platform'; -/** The google apis domain pattern. */ -const GOOGLE_APIS_DOMAIN_PATTERN = '\\.googleapis\\.com$'; -/** The variable portion pattern in a Google APIs domain. */ -const VARIABLE_PORTION_PATTERN = '[^\\.\\s\\/\\\\]+'; /** Default impersonated token lifespan in seconds.*/ const DEFAULT_TOKEN_LIFESPAN = 3600; @@ -171,9 +167,6 @@ export abstract class BaseExternalAccountClient extends AuthClient { clientSecret: options.client_secret, } as ClientAuthentication) : undefined; - if (!this.validateGoogleAPIsUrl('sts', options.token_url)) { - throw new Error(`"${options.token_url}" is not a valid token url.`); - } this.stsCredential = new sts.StsCredentials( options.token_url, this.clientAuth @@ -195,18 +188,6 @@ export abstract class BaseExternalAccountClient extends AuthClient { 'credentials.' ); } - if ( - typeof options.service_account_impersonation_url !== 'undefined' && - !this.validateGoogleAPIsUrl( - 'iamcredentials', - options.service_account_impersonation_url - ) - ) { - throw new Error( - `"${options.service_account_impersonation_url}" is ` + - 'not a valid service account impersonation url.' - ); - } this.serviceAccountImpersonationUrl = options.service_account_impersonation_url; this.serviceAccountImpersonationLifetime = @@ -561,65 +542,4 @@ export abstract class BaseExternalAccountClient extends AuthClient { return this.scopes; } } - - /** - * Checks whether Google APIs URL is valid. - * @param apiName The apiName of url. - * @param url The Google API URL to validate. - * @return Whether the URL is valid or not. - */ - private validateGoogleAPIsUrl(apiName: string, url: string): boolean { - let parsedUrl; - // Return false if error is thrown during parsing URL. - try { - parsedUrl = new URL(url); - } catch (e) { - return false; - } - - const urlDomain = parsedUrl.hostname; - // Check the protocol is https. - if (parsedUrl.protocol !== 'https:') { - return false; - } - - const googleAPIsDomainPatterns: RegExp[] = [ - new RegExp( - '^' + - VARIABLE_PORTION_PATTERN + - '\\.' + - apiName + - GOOGLE_APIS_DOMAIN_PATTERN - ), - new RegExp('^' + apiName + GOOGLE_APIS_DOMAIN_PATTERN), - new RegExp( - '^' + - apiName + - '\\.' + - VARIABLE_PORTION_PATTERN + - GOOGLE_APIS_DOMAIN_PATTERN - ), - new RegExp( - '^' + - VARIABLE_PORTION_PATTERN + - '\\-' + - apiName + - GOOGLE_APIS_DOMAIN_PATTERN - ), - new RegExp( - '^' + - apiName + - '\\-' + - VARIABLE_PORTION_PATTERN + - '\\.p' + - GOOGLE_APIS_DOMAIN_PATTERN - ), - ]; - for (const googleAPIsDomainPattern of googleAPIsDomainPatterns) { - if (urlDomain.match(googleAPIsDomainPattern)) { - return true; - } - } - return false; - } } diff --git a/test/test.baseexternalclient.ts b/test/test.baseexternalclient.ts index 899015b8..6c98df7d 100644 --- a/test/test.baseexternalclient.ts +++ b/test/test.baseexternalclient.ts @@ -160,139 +160,6 @@ describe('BaseExternalAccountClient', () => { }, expectedError); }); - const invalidTokenUrls = [ - 'http://sts.googleapis.com', - 'https://', - 'https://sts.google.com', - 'https://sts.googleapis.net', - 'https://sts.googleapis.comevil.com', - 'https://sts.googleapis.com.evil.com', - 'https://sts.googleapis.com.evil.com/path/to/example', - 'https://sts..googleapis.com', - 'https://-sts.googleapis.com', - 'https://evilsts.googleapis.com', - 'https://us.east.1.sts.googleapis.com', - 'https://us east 1.sts.googleapis.com', - 'https://us-east- 1.sts.googleapis.com', - 'https://us/.east/.1.sts.googleapis.com', - 'https://us.ea\\st.1.sts.googleapis.com', - 'https://sts.pgoogleapis.com', - 'https://p.googleapis.com', - 'https://sts.p.com', - 'http://sts.p.googleapis.com', - 'https://xyz-sts.p.googleapis.com', - 'https://sts-xyz.123.p.googleapis.com', - 'https://sts-xyz.p1.googleapis.com', - 'https://sts-xyz.p.foo.com', - 'https://sts-xyz.p.foo.googleapis.com', - ]; - invalidTokenUrls.forEach(invalidTokenUrl => { - it(`should throw on invalid token url: ${invalidTokenUrl}`, () => { - const invalidOptions = Object.assign({}, externalAccountOptions); - invalidOptions.token_url = invalidTokenUrl; - const expectedError = new Error( - `"${invalidTokenUrl}" is not a valid token url.` - ); - assert.throws(() => { - return new TestExternalAccountClient(invalidOptions); - }, expectedError); - }); - }); - - it('should not throw on valid token urls', () => { - const validTokenUrls = [ - 'https://sts.googleapis.com', - 'https://sts.us-west-1.googleapis.com', - 'https://sts.google.googleapis.com', - 'https://sts.googleapis.com/path/to/example', - 'https://us-west-1.sts.googleapis.com', - 'https://us-west-1-sts.googleapis.com', - 'https://exmaple.sts.googleapis.com', - 'https://example-sts.googleapis.com', - 'https://sts-xyz123.p.googleapis.com', - 'https://sts-xyz-123.p.googleapis.com', - 'https://sts-xys123.p.googleapis.com/path/to/example', - ]; - const validOptions = Object.assign({}, externalAccountOptions); - for (const validTokenUrl of validTokenUrls) { - validOptions.token_url = validTokenUrl; - assert.doesNotThrow(() => { - return new TestExternalAccountClient(validOptions); - }); - } - }); - - const invalidServiceAccountImpersonationUrls = [ - 'http://iamcredentials.googleapis.com', - 'https://', - 'https://iamcredentials.google.com', - 'https://iamcredentials.googleapis.net', - 'https://iamcredentials.googleapis.comevil.com', - 'https://iamcredentials.googleapis.com.evil.com', - 'https://iamcredentials.googleapis.com.evil.com/path/to/example', - 'https://iamcredentials..googleapis.com', - 'https://-iamcredentials.googleapis.com', - 'https://eviliamcredentials.googleapis.com', - 'https://evil.eviliamcredentials.googleapis.com', - 'https://us.east.1.iamcredentials.googleapis.com', - 'https://us east 1.iamcredentials.googleapis.com', - 'https://us-east- 1.iamcredentials.googleapis.com', - 'https://us/.east/.1.iamcredentials.googleapis.com', - 'https://us.ea\\st.1.iamcredentials.googleapis.com', - 'https://iamcredentials.pgoogleapis.com', - 'https://p.googleapis.com', - 'https://iamcredentials.p.com', - 'http://iamcredentials.p.googleapis.com', - 'https://xyz-iamcredentials.p.googleapis.com', - 'https://iamcredentials-xyz.123.p.googleapis.com', - 'https://iamcredentials-xyz.p1.googleapis.com', - 'https://iamcredentials-xyz.p.foo.com', - 'https://iamcredentials-xyz.p.foo.googleapis.com', - ]; - invalidServiceAccountImpersonationUrls.forEach( - invalidServiceAccountImpersonationUrl => { - it(`should throw on invalid service account impersonation url: ${invalidServiceAccountImpersonationUrl}`, () => { - const invalidOptions = Object.assign( - {}, - externalAccountOptionsWithSA - ); - invalidOptions.service_account_impersonation_url = - invalidServiceAccountImpersonationUrl; - const expectedError = new Error( - `"${invalidServiceAccountImpersonationUrl}" is ` + - 'not a valid service account impersonation url.' - ); - assert.throws(() => { - return new TestExternalAccountClient(invalidOptions); - }, expectedError); - }); - } - ); - - it('should not throw on valid service account impersonation url', () => { - const validServiceAccountImpersonationUrls = [ - 'https://iamcredentials.googleapis.com', - 'https://iamcredentials.us-west-1.googleapis.com', - 'https://iamcredentials.google.googleapis.com', - 'https://iamcredentials.googleapis.com/path/to/example', - 'https://us-west-1.iamcredentials.googleapis.com', - 'https://us-west-1-iamcredentials.googleapis.com', - 'https://example.iamcredentials.googleapis.com', - 'https://example-iamcredentials.googleapis.com', - 'https://iamcredentials-xyz123.p.googleapis.com', - 'https://iamcredentials-xyz-123.p.googleapis.com', - 'https://iamcredentials-xys123.p.googleapis.com/path/to/example', - ]; - const validOptions = Object.assign({}, externalAccountOptionsWithSA); - for (const validServiceAccountImpersonationUrl of validServiceAccountImpersonationUrls) { - validOptions.service_account_impersonation_url = - validServiceAccountImpersonationUrl; - assert.doesNotThrow(() => { - return new TestExternalAccountClient(validOptions); - }); - } - }); - const invalidWorkforceAudiences = [ '//iam.googleapis.com/locations/global/workloadIdentityPools/pool/providers/provider', '//iam.googleapis.com/locations/global/workforcepools/pool/providers/provider',