From 1554cbd37c6817443a123435f75120a17f87f40b Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Tue, 5 Nov 2024 16:12:14 -0500 Subject: [PATCH 1/3] fix: Set quota project ID for ADC human accounts --- src/app/credential-internal.ts | 9 +++++++++ src/utils/api-request.ts | 12 ++++++++---- test/integration/setup.ts | 5 +++-- .../app-check/app-check-api-client-internal.spec.ts | 1 - .../data-connect-api-client-internal.spec.ts | 1 - .../functions/functions-api-client-internal.spec.ts | 1 - .../machine-learning-api-client.spec.ts | 1 - .../remote-config/remote-config-api-client.spec.ts | 1 - .../security-rules/security-rules-api-client.spec.ts | 1 - 9 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/app/credential-internal.ts b/src/app/credential-internal.ts index 2d240cc301..31e319de3f 100644 --- a/src/app/credential-internal.ts +++ b/src/app/credential-internal.ts @@ -39,6 +39,7 @@ export class ApplicationDefaultCredential implements Credential { private readonly googleAuth: GoogleAuth; private authClient: AnyAuthClient; private projectId?: string; + private quotaProjectId?: string; private accountId?: string; constructor(httpAgent?: Agent) { @@ -58,6 +59,7 @@ export class ApplicationDefaultCredential implements Credential { } await this.authClient.getAccessToken(); const credentials = this.authClient.credentials; + this.quotaProjectId = this.authClient.quotaProjectId; return populateCredential(credentials); } @@ -68,6 +70,13 @@ export class ApplicationDefaultCredential implements Credential { return Promise.resolve(this.projectId); } + public getQuotaProjectId(): string | undefined { + if (!this.quotaProjectId) { + this.quotaProjectId = this.authClient.quotaProjectId; + } + return this.quotaProjectId; + } + public async isComputeEngineCredential(): Promise { if (!this.authClient) { this.authClient = await this.googleAuth.getClient(); diff --git a/src/utils/api-request.ts b/src/utils/api-request.ts index d64e0a2762..b5d9737a1f 100644 --- a/src/utils/api-request.ts +++ b/src/utils/api-request.ts @@ -26,6 +26,7 @@ import url = require('url'); import { EventEmitter } from 'events'; import { Readable } from 'stream'; import * as zlibmod from 'zlib'; +import { ApplicationDefaultCredential } from '../app/credential-internal'; import { getMetricsHeader } from '../utils/index'; /** Http method type definition. */ @@ -1077,10 +1078,13 @@ export class AuthorizedHttpClient extends HttpClient { const authHeader = 'Authorization'; requestCopy.headers[authHeader] = `Bearer ${token}`; - // Fix issue where firebase-admin does not specify quota project that is - // necessary for use when utilizing human account with ADC (RSDF) - if (!requestCopy.headers['x-goog-user-project'] && this.app.options.projectId) { - requestCopy.headers['x-goog-user-project'] = this.app.options.projectId + let quotaProjectId: string | undefined; + if (this.app.options.credential instanceof ApplicationDefaultCredential){ + quotaProjectId = this.app.options.credential.getQuotaProjectId(); + } + quotaProjectId = process.env.GOOGLE_CLOUD_QUOTA_PROJECT || undefined; + if (!requestCopy.headers['x-goog-user-project'] && validator.isNonEmptyString(quotaProjectId)) { + requestCopy.headers['x-goog-user-project'] = quotaProjectId; } if (!requestCopy.httpAgent && this.app.options.httpAgent) { diff --git a/test/integration/setup.ts b/test/integration/setup.ts index 8c700d4c41..b17188f476 100644 --- a/test/integration/setup.ts +++ b/test/integration/setup.ts @@ -19,7 +19,7 @@ import minimist = require('minimist'); import path = require('path'); import { random } from 'lodash'; import { - App, Credential, GoogleOAuthAccessToken, cert, deleteApp, initializeApp, + App, Credential, GoogleOAuthAccessToken, applicationDefault, cert, deleteApp, initializeApp, } from '../../lib/app/index' // eslint-disable-next-line @typescript-eslint/no-var-requires @@ -87,7 +87,8 @@ before(() => { storageBucket = projectId + '.appspot.com'; defaultApp = initializeApp({ - ...getCredential(), + //...getCredential(), + credential: applicationDefault(), projectId, databaseURL: databaseUrl, storageBucket, diff --git a/test/unit/app-check/app-check-api-client-internal.spec.ts b/test/unit/app-check/app-check-api-client-internal.spec.ts index 3f2746cf59..743f6ed89f 100644 --- a/test/unit/app-check/app-check-api-client-internal.spec.ts +++ b/test/unit/app-check/app-check-api-client-internal.spec.ts @@ -45,7 +45,6 @@ describe('AppCheckApiClient', () => { const EXPECTED_HEADERS = { 'Authorization': 'Bearer mock-token', 'X-Firebase-Client': `fire-admin-node/${getSdkVersion()}`, - 'x-goog-user-project': 'test-project', 'X-Goog-Api-Client': getMetricsHeader(), }; diff --git a/test/unit/data-connect/data-connect-api-client-internal.spec.ts b/test/unit/data-connect/data-connect-api-client-internal.spec.ts index 0815515204..93b7120703 100644 --- a/test/unit/data-connect/data-connect-api-client-internal.spec.ts +++ b/test/unit/data-connect/data-connect-api-client-internal.spec.ts @@ -43,7 +43,6 @@ describe('DataConnectApiClient', () => { const EXPECTED_HEADERS = { 'Authorization': 'Bearer mock-token', 'X-Firebase-Client': `fire-admin-node/${getSdkVersion()}`, - 'x-goog-user-project': 'test-project', 'X-Goog-Api-Client': getMetricsHeader(), }; diff --git a/test/unit/functions/functions-api-client-internal.spec.ts b/test/unit/functions/functions-api-client-internal.spec.ts index 40e24b1fea..15862d57b2 100644 --- a/test/unit/functions/functions-api-client-internal.spec.ts +++ b/test/unit/functions/functions-api-client-internal.spec.ts @@ -46,7 +46,6 @@ describe('FunctionsApiClient', () => { const EXPECTED_HEADERS = { 'X-Firebase-Client': `fire-admin-node/${getSdkVersion()}`, 'Authorization': 'Bearer mock-token', - 'x-goog-user-project': 'test-project', 'X-Goog-Api-Client': getMetricsHeader(), }; diff --git a/test/unit/machine-learning/machine-learning-api-client.spec.ts b/test/unit/machine-learning/machine-learning-api-client.spec.ts index c77380f2f3..2f4447c2da 100644 --- a/test/unit/machine-learning/machine-learning-api-client.spec.ts +++ b/test/unit/machine-learning/machine-learning-api-client.spec.ts @@ -117,7 +117,6 @@ describe('MachineLearningApiClient', () => { const EXPECTED_HEADERS = { 'Authorization': 'Bearer mock-token', 'X-Firebase-Client': `fire-admin-node/${getSdkVersion()}`, - 'x-goog-user-project': 'test-project', 'X-Goog-Api-Client': getMetricsHeader(), }; const noProjectId = 'Failed to determine project ID. Initialize the SDK with service ' diff --git a/test/unit/remote-config/remote-config-api-client.spec.ts b/test/unit/remote-config/remote-config-api-client.spec.ts index 9b6b8709bf..91783e9088 100644 --- a/test/unit/remote-config/remote-config-api-client.spec.ts +++ b/test/unit/remote-config/remote-config-api-client.spec.ts @@ -57,7 +57,6 @@ describe('RemoteConfigApiClient', () => { 'Authorization': 'Bearer mock-token', 'X-Firebase-Client': `fire-admin-node/${getSdkVersion()}`, 'Accept-Encoding': 'gzip', - 'x-goog-user-project': 'test-project', 'X-Goog-Api-Client': getMetricsHeader(), }; diff --git a/test/unit/security-rules/security-rules-api-client.spec.ts b/test/unit/security-rules/security-rules-api-client.spec.ts index 2c1c8d9ec0..c4176540a6 100644 --- a/test/unit/security-rules/security-rules-api-client.spec.ts +++ b/test/unit/security-rules/security-rules-api-client.spec.ts @@ -44,7 +44,6 @@ describe('SecurityRulesApiClient', () => { const EXPECTED_HEADERS = { 'Authorization': 'Bearer mock-token', 'X-Firebase-Client': `fire-admin-node/${getSdkVersion()}`, - 'x-goog-user-project': 'test-project', 'X-Goog-Api-Client': getMetricsHeader(), }; const noProjectId = 'Failed to determine project ID. Initialize the SDK with service ' From 9fc3448d73ec389e62182a553cf8087e2aaad4cb Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Thu, 7 Nov 2024 14:21:28 -0500 Subject: [PATCH 2/3] Add header to http/2 calls --- src/app/credential-internal.ts | 2 +- src/utils/api-request.ts | 7 +++++-- test/unit/utils/api-request.spec.ts | 3 +++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/app/credential-internal.ts b/src/app/credential-internal.ts index 31e319de3f..c8ce25b246 100644 --- a/src/app/credential-internal.ts +++ b/src/app/credential-internal.ts @@ -72,7 +72,7 @@ export class ApplicationDefaultCredential implements Credential { public getQuotaProjectId(): string | undefined { if (!this.quotaProjectId) { - this.quotaProjectId = this.authClient.quotaProjectId; + this.quotaProjectId = this.authClient?.quotaProjectId; } return this.quotaProjectId; } diff --git a/src/utils/api-request.ts b/src/utils/api-request.ts index b5d9737a1f..f2f8d56956 100644 --- a/src/utils/api-request.ts +++ b/src/utils/api-request.ts @@ -1079,10 +1079,13 @@ export class AuthorizedHttpClient extends HttpClient { requestCopy.headers[authHeader] = `Bearer ${token}`; let quotaProjectId: string | undefined; - if (this.app.options.credential instanceof ApplicationDefaultCredential){ + if (process.env.GOOGLE_CLOUD_QUOTA_PROJECT) { + quotaProjectId = process.env.GOOGLE_CLOUD_QUOTA_PROJECT; + } + else if (this.app.options.credential instanceof ApplicationDefaultCredential){ quotaProjectId = this.app.options.credential.getQuotaProjectId(); } - quotaProjectId = process.env.GOOGLE_CLOUD_QUOTA_PROJECT || undefined; + if (!requestCopy.headers['x-goog-user-project'] && validator.isNonEmptyString(quotaProjectId)) { requestCopy.headers['x-goog-user-project'] = quotaProjectId; } diff --git a/test/unit/utils/api-request.spec.ts b/test/unit/utils/api-request.spec.ts index 4fcb5a397b..720040f06d 100644 --- a/test/unit/utils/api-request.spec.ts +++ b/test/unit/utils/api-request.spec.ts @@ -2569,6 +2569,9 @@ describe('AuthorizedHttpClient', () => { afterEach(() => { transportSpy!.restore(); transportSpy = null; + if (process.env.GOOGLE_CLOUD_QUOTA_PROJECT) { + delete process.env.GOOGLE_CLOUD_QUOTA_PROJECT; + } return mockAppWithAgent.delete(); }); From c10036f301a29a246e847df7e2e8389d4e03e26f Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Thu, 7 Nov 2024 17:32:59 -0500 Subject: [PATCH 3/3] add unit tests --- src/utils/api-request.ts | 16 ++++--- test/integration/setup.ts | 5 +-- .../extensions-api-client-internal.spec.ts | 1 - test/unit/utils/api-request.spec.ts | 44 +++++++++++++++++-- 4 files changed, 54 insertions(+), 12 deletions(-) diff --git a/src/utils/api-request.ts b/src/utils/api-request.ts index f2f8d56956..c5e284e414 100644 --- a/src/utils/api-request.ts +++ b/src/utils/api-request.ts @@ -1079,13 +1079,10 @@ export class AuthorizedHttpClient extends HttpClient { requestCopy.headers[authHeader] = `Bearer ${token}`; let quotaProjectId: string | undefined; - if (process.env.GOOGLE_CLOUD_QUOTA_PROJECT) { - quotaProjectId = process.env.GOOGLE_CLOUD_QUOTA_PROJECT; - } - else if (this.app.options.credential instanceof ApplicationDefaultCredential){ + if (this.app.options.credential instanceof ApplicationDefaultCredential) { quotaProjectId = this.app.options.credential.getQuotaProjectId(); } - + quotaProjectId = process.env.GOOGLE_CLOUD_QUOTA_PROJECT || quotaProjectId; if (!requestCopy.headers['x-goog-user-project'] && validator.isNonEmptyString(quotaProjectId)) { requestCopy.headers['x-goog-user-project'] = quotaProjectId; } @@ -1119,6 +1116,15 @@ export class AuthorizedHttp2Client extends Http2Client { const authHeader = 'Authorization'; requestCopy.headers[authHeader] = `Bearer ${token}`; + let quotaProjectId: string | undefined; + if (this.app.options.credential instanceof ApplicationDefaultCredential) { + quotaProjectId = this.app.options.credential.getQuotaProjectId(); + } + quotaProjectId = process.env.GOOGLE_CLOUD_QUOTA_PROJECT || quotaProjectId; + if (!requestCopy.headers['x-goog-user-project'] && validator.isNonEmptyString(quotaProjectId)) { + requestCopy.headers['x-goog-user-project'] = quotaProjectId; + } + requestCopy.headers['X-Goog-Api-Client'] = getMetricsHeader() return super.send(requestCopy); diff --git a/test/integration/setup.ts b/test/integration/setup.ts index b17188f476..8c700d4c41 100644 --- a/test/integration/setup.ts +++ b/test/integration/setup.ts @@ -19,7 +19,7 @@ import minimist = require('minimist'); import path = require('path'); import { random } from 'lodash'; import { - App, Credential, GoogleOAuthAccessToken, applicationDefault, cert, deleteApp, initializeApp, + App, Credential, GoogleOAuthAccessToken, cert, deleteApp, initializeApp, } from '../../lib/app/index' // eslint-disable-next-line @typescript-eslint/no-var-requires @@ -87,8 +87,7 @@ before(() => { storageBucket = projectId + '.appspot.com'; defaultApp = initializeApp({ - //...getCredential(), - credential: applicationDefault(), + ...getCredential(), projectId, databaseURL: databaseUrl, storageBucket, diff --git a/test/unit/extensions/extensions-api-client-internal.spec.ts b/test/unit/extensions/extensions-api-client-internal.spec.ts index 32213b8068..66f2e8198b 100644 --- a/test/unit/extensions/extensions-api-client-internal.spec.ts +++ b/test/unit/extensions/extensions-api-client-internal.spec.ts @@ -43,7 +43,6 @@ describe('Extension API client', () => { const EXPECTED_HEADERS = { 'Authorization': 'Bearer mock-token', 'X-Firebase-Client': `fire-admin-node/${getSdkVersion()}`, - 'x-goog-user-project': 'test-project', 'X-Goog-Api-Client': getMetricsHeader(), } diff --git a/test/unit/utils/api-request.spec.ts b/test/unit/utils/api-request.spec.ts index 720040f06d..d64589f8c9 100644 --- a/test/unit/utils/api-request.spec.ts +++ b/test/unit/utils/api-request.spec.ts @@ -17,6 +17,7 @@ 'use strict'; +import * as _ from 'lodash'; import * as chai from 'chai'; import * as nock from 'nock'; import * as sinon from 'sinon'; @@ -35,6 +36,7 @@ import { import { deepCopy } from '../../../src/utils/deep-copy'; import { Agent } from 'http'; import * as zlib from 'zlib'; +import { getMetricsHeader } from '../../../src/utils'; chai.should(); chai.use(sinonChai); @@ -2569,9 +2571,6 @@ describe('AuthorizedHttpClient', () => { afterEach(() => { transportSpy!.restore(); transportSpy = null; - if (process.env.GOOGLE_CLOUD_QUOTA_PROJECT) { - delete process.env.GOOGLE_CLOUD_QUOTA_PROJECT; - } return mockAppWithAgent.delete(); }); @@ -2651,6 +2650,45 @@ describe('AuthorizedHttpClient', () => { }); }); + describe('Quota Project', () => { + let stubs: sinon.SinonStub[] = []; + + afterEach(() => { + _.forEach(stubs, (stub) => stub.restore()); + stubs = []; + if (process.env.GOOGLE_CLOUD_QUOTA_PROJECT) { + delete process.env.GOOGLE_CLOUD_QUOTA_PROJECT; + } + }); + + it('should include quota project id in headers when GOOGLE_CLOUD_QUOTA_PROJECT is set', () => { + const reqData = { request: 'data' }; + const stub = sinon + .stub(HttpClient.prototype, 'send') + .resolves(utils.responseFrom({}, 200)); + stubs.push(stub); + process.env.GOOGLE_CLOUD_QUOTA_PROJECT = 'test-project-id'; + const client = new AuthorizedHttpClient(mockApp); + return client.send({ + method: 'POST', + url: mockUrl, + data: reqData, + }) + .then(() => { + expect(stub).to.have.been.calledOnce.and.calledWith({ + method: 'POST', + url: mockUrl, + headers: { + ...requestHeaders.reqheaders, + 'x-goog-user-project': 'test-project-id', + 'X-Goog-Api-Client': getMetricsHeader(), + }, + data: reqData + }); + }); + }); + }); + it('should not mutate the arguments', () => { const reqData = { request: 'data' }; const options = {