Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@
PluggableAuthClient,
PluggableAuthClientOptions,
} from './pluggable-auth-client';
import {isAdcWarningSuppressed} from '../util';

export type ExternalAccountClientOptions =
| IdentityPoolClientOptions

Check failure on line 38 in core/packages/google-auth-library-nodejs/src/auth/externalclient.ts

View workflow job for this annotation

GitHub Actions / lint

Replace `|·IdentityPoolClientOptions⏎··|·AwsClientOptions⏎·` with `IdentityPoolClientOptions·|·AwsClientOptions`
| AwsClientOptions
| PluggableAuthClientOptions;

Expand Down Expand Up @@ -73,6 +74,11 @@
static fromJSON(
options: ExternalAccountClientOptions,
): BaseExternalAccountClient | null {
if (!isAdcWarningSuppressed()) {
console.warn(
'The `fromJSON` method does not validate the credential configuration. A security risk occurs when a credential configuration configured with malicious URLs is used. When the credential configuration is accepted from an untrusted source, you should validate it before using it with this method. For more details, see https://cloud.google.com/docs/authentication/external/externally-sourced-credentials.',
);
}
if (options && options.type === EXTERNAL_ACCOUNT_TYPE) {
if ((options as AwsClientOptions).credential_source?.environment_id) {
return new AwsClient(options as AwsClientOptions);
Expand Down
79 changes: 66 additions & 13 deletions core/packages/google-auth-library-nodejs/src/auth/googleauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,13 @@
GDCH_SERVICE_ACCOUNT_TYPE,
GdchCredentialsInput,
} from './gdchclient';
import {originalOrCamelOptions} from '../util';
import {
isAdcWarningSuppressed,
isFromJsonWarningSuppressed,
originalOrCamelOptions,
suppressAdcWarning,
suppressFromJsonWarning,
} from '../util';
import {AnyAuthClient, AnyAuthClientConstructor} from '..';

/**
Expand Down Expand Up @@ -248,7 +254,11 @@
private _cachedProjectId?: string | null;

// To save the contents of the JSON credential file
jsonContent: JWTInput | ExternalAccountClientOptions | GdchCredentialsInput | null = null;
jsonContent:
| JWTInput

Check failure on line 258 in core/packages/google-auth-library-nodejs/src/auth/googleauth.ts

View workflow job for this annotation

GitHub Actions / lint

Replace `|·JWTInput⏎····|·ExternalAccountClientOptions⏎····|·GdchCredentialsInput⏎····|·null·=` with `JWTInput·|·ExternalAccountClientOptions·|·GdchCredentialsInput·|·null·=⏎···`
| ExternalAccountClientOptions
| GdchCredentialsInput
| null = null;
apiKey: string | null;

cachedCredential: AnyAuthClient | T | null = null;
Expand Down Expand Up @@ -281,9 +291,19 @@
this._cachedProjectId = opts.projectId || null;
this.cachedCredential = opts.authClient || null;
this.keyFilename = opts.keyFilename || opts.keyFile;
if (this.keyFilename) {
console.warn(
'The `keyFilename` option is deprecated. Please use the `auth` object constructor instead. For more details, see https://cloud.google.com/docs/authentication/external/externally-sourced-credentials.',
);
}
this.scopes = opts.scopes;
this.clientOptions = opts.clientOptions || {};
this.jsonContent = opts.credentials || null;
if (this.jsonContent) {
console.warn(
'The `credentials` option is deprecated. Please use the `auth` object constructor instead. For more details, see https://cloud.google.com/docs/authentication/external/externally-sourced-credentials.',
);
}
this.apiKey = opts.apiKey || this.clientOptions.apiKey || null;

// Cannot use both API Key + Credentials
Expand All @@ -302,9 +322,9 @@
// variables are set on the JWT client in order to indicate the type of library,
// and sign the JWT with the correct audience and scopes (if not supplied).
setGapicJWTValues(client: JWT) {
client.defaultServicePath = this.defaultServicePath;
client.useJWTAccessWithScope = this.useJWTAccessWithScope;
client.defaultScopes = this.defaultScopes;
client.defaultServicePath ??= this.defaultServicePath;
client.useJWTAccessWithScope ??= this.useJWTAccessWithScope;
client.defaultScopes ??= this.defaultScopes;
}

/**
Expand All @@ -321,7 +341,7 @@
getProjectId(callback: ProjectIdCallback): void;
getProjectId(callback?: ProjectIdCallback): Promise<string | null> | void {
if (callback) {
this.getProjectIdAsync().then(r => callback(null, r), callback);

Check warning on line 344 in core/packages/google-auth-library-nodejs/src/auth/googleauth.ts

View workflow job for this annotation

GitHub Actions / lint

Avoid calling back inside of a promise

Check failure on line 344 in core/packages/google-auth-library-nodejs/src/auth/googleauth.ts

View workflow job for this annotation

GitHub Actions / lint

Expected catch() or return
} else {
return this.getProjectIdAsync();
}
Expand Down Expand Up @@ -467,7 +487,7 @@
options = optionsOrCallback;
}
if (callback) {
this.getApplicationDefaultAsync(options).then(

Check failure on line 490 in core/packages/google-auth-library-nodejs/src/auth/googleauth.ts

View workflow job for this annotation

GitHub Actions / lint

Expected catch() or return
r => callback!(null, r.credential, r.projectId),
callback,
);
Expand Down Expand Up @@ -666,7 +686,7 @@

// Now open a read stream on the file, and parse it.
const readStream = fs.createReadStream(filePath);
return this.fromStream(readStream, options);
return suppressAdcWarning(() => this.fromStream(readStream, options));
}

/**
Expand Down Expand Up @@ -696,7 +716,9 @@
);
}

const sourceClient = this.fromJSON(json.source_credentials);
const sourceClient = suppressFromJsonWarning(() =>
this.fromJSON(json.source_credentials!),
);

if (json.service_account_impersonation_url?.length > 256) {
/**
Expand Down Expand Up @@ -774,11 +796,21 @@
* @returns JWT or UserRefresh Client with data
*/
fromJSON(
json: JWTInput | ImpersonatedJWTInput | GdchCredentialsInput | ExternalAccountClientOptions,
json:
| JWTInput
| ImpersonatedJWTInput
| GdchCredentialsInput
| ExternalAccountClientOptions,
options: AuthClientOptions = {},
): JSONClient {
let client: JSONClient;

if (!isFromJsonWarningSuppressed()) {
console.warn(
'The `fromJSON` method is deprecated. Please use the `JWT` constructor instead. For more details, see https://cloud.google.com/docs/authentication/external/externally-sourced-credentials.',
);
}

// user's preferred universe domain
const preferredUniverseDomain =
originalOrCamelOptions(options).get('universe_domain');
Expand Down Expand Up @@ -824,10 +856,14 @@
* @returns JWT or UserRefresh Client with data
*/
private _cacheClientFromJSON(
json: JWTInput | ImpersonatedJWTInput | GdchCredentialsInput | ExternalAccountClientOptions,
json:
| JWTInput
| ImpersonatedJWTInput
| GdchCredentialsInput
| ExternalAccountClientOptions,
options?: AuthClientOptions,
): JSONClient {
const client = this.fromJSON(json, options);
const client = suppressFromJsonWarning(() => this.fromJSON(json, options));

// cache both raw data used to instantiate client and client itself.
this.jsonContent = json;
Expand Down Expand Up @@ -896,25 +932,32 @@
optionsOrCallback: AuthClientOptions | CredentialCallback = {},
callback?: CredentialCallback,
): Promise<JSONClient> | void {
const isADC = isAdcWarningSuppressed();
if (!isADC) {
console.warn(
'The `fromStream` method is deprecated. Please use the `JWT` constructor with a parsed stream instead. For more details, see https://cloud.google.com/docs/authentication/external/externally-sourced-credentials.',
);
}
let options: AuthClientOptions = {};
if (typeof optionsOrCallback === 'function') {
callback = optionsOrCallback;
} else {
options = optionsOrCallback;
}
if (callback) {
this.fromStreamAsync(inputStream, options).then(
this.fromStreamAsync(inputStream, options, isADC).then(

Check failure on line 948 in core/packages/google-auth-library-nodejs/src/auth/googleauth.ts

View workflow job for this annotation

GitHub Actions / lint

Expected catch() or return
r => callback!(null, r),
callback,
);
} else {
return this.fromStreamAsync(inputStream, options);
return this.fromStreamAsync(inputStream, options, isADC);
}
}

private fromStreamAsync(
inputStream: stream.Readable,
options?: AuthClientOptions,
isADC = false,
): Promise<JSONClient> {
return new Promise((resolve, reject) => {
if (!inputStream) {
Expand All @@ -931,7 +974,11 @@
try {
try {
const data = JSON.parse(chunks.join(''));
const r = this._cacheClientFromJSON(data, options);
const r = isADC
? suppressAdcWarning(() =>
this._cacheClientFromJSON(data, options),
)
: this._cacheClientFromJSON(data, options);
return resolve(r);
} catch (err) {
// If we failed parsing this.keyFileName, assume that it
Expand Down Expand Up @@ -1095,7 +1142,7 @@
callback?: (err: Error | null, credentials?: CredentialBody) => void,
): void | Promise<CredentialBody> {
if (callback) {
this.getCredentialsAsync().then(r => callback(null, r), callback);

Check warning on line 1145 in core/packages/google-auth-library-nodejs/src/auth/googleauth.ts

View workflow job for this annotation

GitHub Actions / lint

Avoid calling back inside of a promise

Check failure on line 1145 in core/packages/google-auth-library-nodejs/src/auth/googleauth.ts

View workflow job for this annotation

GitHub Actions / lint

Expected catch() or return
} else {
return this.getCredentialsAsync();
}
Expand All @@ -1122,7 +1169,7 @@
return {
client_email: (this.jsonContent as JWTInput).client_email,
private_key: (this.jsonContent as JWTInput).private_key,
universe_domain: (this.jsonContent as any).universe_domain,

Check warning on line 1172 in core/packages/google-auth-library-nodejs/src/auth/googleauth.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
};
}

Expand All @@ -1145,6 +1192,9 @@
*/
async getClient(): Promise<AnyAuthClient | T> {
if (this.cachedCredential) {
if (this.cachedCredential instanceof JWT) {
this.setGapicJWTValues(this.cachedCredential);
}
return this.cachedCredential;
}

Expand All @@ -1154,8 +1204,11 @@

try {
const client = await this.#pendingAuthClient;
if (client instanceof JWT) {
this.setGapicJWTValues(client);
}
if (client instanceof GdchClient && !client.apiAudience) {
const opts = this.clientOptions as any;

Check warning on line 1211 in core/packages/google-auth-library-nodejs/src/auth/googleauth.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
const endpoint = opts.apiEndpoint || opts.servicePath;
if (endpoint) {
const scheme = endpoint.startsWith('http') ? '' : 'https://';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {AuthClient} from './authclient';
import {IdTokenProvider} from './idtokenclient';
import {GaxiosError} from 'gaxios';
import {SignBlobResponse} from './googleauth';
import {originalOrCamelOptions} from '../util';
import {isAdcWarningSuppressed, originalOrCamelOptions} from '../util';

export interface ImpersonatedOptions extends OAuth2ClientOptions {
/**
Expand Down Expand Up @@ -121,6 +121,11 @@ export class Impersonated extends OAuth2Client implements IdTokenProvider {
*/
constructor(options: ImpersonatedOptions = {}) {
super(options);
if (!isAdcWarningSuppressed()) {
console.warn(
'The `Impersonated` constructor does not validate the credential configuration. A security risk occurs when a credential configuration configured with malicious URLs is used. When the credential configuration is accepted from an untrusted source, you should validate it before using it with this method. For more details, see https://cloud.google.com/docs/authentication/external/externally-sourced-credentials.',
);
}
// Start with an expired refresh token, which will automatically be
// refreshed before the first API call is made.
this.credentials = {
Expand Down
45 changes: 45 additions & 0 deletions core/packages/google-auth-library-nodejs/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,3 +300,48 @@ export function getWellKnownCertificateConfigFileLocation(): string {
function _isWindows(): boolean {
return os.platform().startsWith('win');
}

let adcWarningSuppressionDepth = 0;
let fromJsonWarningSuppressionDepth = 0;

/**
* Returns true if ADC deprecation/validation warnings are currently suppressed.
* @internal
*/
export function isAdcWarningSuppressed(): boolean {
return adcWarningSuppressionDepth > 0;
}

/**
* Runs a synchronous callback while suppressing ADC deprecation/validation warnings.
* @internal
*/
export function suppressAdcWarning<T>(fn: () => T): T {
adcWarningSuppressionDepth++;
try {
return fn();
} finally {
adcWarningSuppressionDepth--;
}
}

/**
* Returns true if the `GoogleAuth.fromJSON` deprecation warning is currently suppressed.
* @internal
*/
export function isFromJsonWarningSuppressed(): boolean {
return adcWarningSuppressionDepth > 0 || fromJsonWarningSuppressionDepth > 0;
}

/**
* Runs a synchronous callback while suppressing the `GoogleAuth.fromJSON` deprecation warning.
* @internal
*/
export function suppressFromJsonWarning<T>(fn: () => T): T {
fromJsonWarningSuppressionDepth++;
try {
return fn();
} finally {
fromJsonWarningSuppressionDepth--;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import * as assert from 'assert';
import {describe, it} from 'mocha';
import * as sinon from 'sinon';
import {AwsClient} from '../src/auth/awsclient';
import {IdentityPoolClient} from '../src/auth/identitypoolclient';
import {ExternalAccountClient} from '../src/auth/externalclient';
Expand Down Expand Up @@ -272,5 +273,19 @@ describe('ExternalAccountClient', () => {
return ExternalAccountClient.fromJSON(invalidOptions);
});
});

it('should log a warning when called directly', () => {
const warnSpy = sinon.spy(console, 'warn');
try {
Comment on lines +277 to +279

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using sinon.spy on console.warn allows the warning messages to be printed to the console during test execution, which can clutter the test output and make CI/CD logs noisy. Consider using sinon.stub instead to silence the expected warnings while still allowing assertions on them.

Suggested change
it('should log a warning when called directly', () => {
const warnSpy = sinon.spy(console, 'warn');
try {
it('should log a warning when called directly', () => {
const warnSpy = sinon.stub(console, 'warn');
try {

ExternalAccountClient.fromJSON(fileSourcedOptions);
assert(
warnSpy.calledOnceWithExactly(
'The `fromJSON` method does not validate the credential configuration. A security risk occurs when a credential configuration configured with malicious URLs is used. When the credential configuration is accepted from an untrusted source, you should validate it before using it with this method. For more details, see https://cloud.google.com/docs/authentication/external/externally-sourced-credentials.',
),
);
} finally {
warnSpy.restore();
}
});
});
});
Loading
Loading