Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Requests Respect config.projectIdRequired === false #1988

Merged
merged 2 commits into from Jun 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/nodejs-common/service.ts
Expand Up @@ -28,7 +28,7 @@ import {
util,
} from './util';

const PROJECT_ID_TOKEN = '{{projectId}}';
export const DEFAULT_PROJECT_ID_TOKEN = '{{projectId}}';

export interface StreamRequestOptions extends DecorateRequestOptions {
shouldReturnStream: true;
Expand Down Expand Up @@ -106,7 +106,7 @@ export class Service {
this.globalInterceptors = arrify(options.interceptors_!);
this.interceptors = [];
this.packageJson = config.packageJson;
this.projectId = options.projectId || PROJECT_ID_TOKEN;
this.projectId = options.projectId || DEFAULT_PROJECT_ID_TOKEN;
this.projectIdRequired = config.projectIdRequired !== false;
this.providedUserAgent = options.userAgent;

Expand Down Expand Up @@ -167,7 +167,7 @@ export class Service {

protected async getProjectIdAsync(): Promise<string> {
const projectId = await this.authClient.getProjectId();
if (this.projectId === PROJECT_ID_TOKEN && projectId) {
if (this.projectId === DEFAULT_PROJECT_ID_TOKEN && projectId) {
this.projectId = projectId;
}
return this.projectId;
Expand Down
116 changes: 92 additions & 24 deletions src/nodejs-common/util.ts
Expand Up @@ -18,7 +18,10 @@
* @module common/util
*/

import {replaceProjectIdToken} from '@google-cloud/projectify';
import {
replaceProjectIdToken,
MissingProjectIdError,
} from '@google-cloud/projectify';
import * as ent from 'ent';
import * as extend from 'extend';
import {AuthClient, GoogleAuth, GoogleAuthOptions} from 'google-auth-library';
Expand All @@ -29,6 +32,8 @@ import {Duplex, DuplexOptions, Readable, Transform, Writable} from 'stream';
import {teenyRequest} from 'teeny-request';
import {Interceptor} from './service-object';
import * as uuid from 'uuid';
import {DEFAULT_PROJECT_ID_TOKEN} from './service';

const packageJson = require('../../../package.json');

// eslint-disable-next-line @typescript-eslint/no-var-requires
Expand Down Expand Up @@ -164,6 +169,11 @@ export interface MakeAuthenticatedRequestFactoryConfig
* A new will be created if this is not set.
*/
authClient?: AuthClient | GoogleAuth;

/**
* Determines if a projectId is required for authenticated requests. Defaults to `true`.
*/
projectIdRequired?: boolean;
}

export interface MakeAuthenticatedRequestOptions {
Expand Down Expand Up @@ -592,7 +602,7 @@ export class Util {
config: MakeAuthenticatedRequestFactoryConfig
) {
const googleAutoAuthConfig = extend({}, config);
if (googleAutoAuthConfig.projectId === '{{projectId}}') {
if (googleAutoAuthConfig.projectId === DEFAULT_PROJECT_ID_TOKEN) {
delete googleAutoAuthConfig.projectId;
}

Expand Down Expand Up @@ -650,7 +660,11 @@ export class Util {
const callback =
typeof optionsOrCallback === 'function' ? optionsOrCallback : undefined;

const onAuthenticated = (
async function setProjectId() {
projectId = await authClient.getProjectId();
}

const onAuthenticated = async (
err: Error | null,
authenticatedReqOpts?: DecorateRequestOptions
) => {
Expand All @@ -667,16 +681,35 @@ export class Util {

if (!err || autoAuthFailed) {
try {
// Try with existing `projectId` value
authenticatedReqOpts = util.decorateRequest(
authenticatedReqOpts!,
projectId
);

err = null;
} catch (e) {
// A projectId was required, but we don't have one.
// Re-use the "Could not load the default credentials error" if
// auto auth failed.
err = err || (e as Error);
if (e instanceof MissingProjectIdError) {
// A `projectId` was required, but we don't have one.
try {
// Attempt to get the `projectId`
await setProjectId();

authenticatedReqOpts = util.decorateRequest(
authenticatedReqOpts!,
projectId
);

err = null;
} catch (e) {
// Re-use the "Could not load the default credentials error" if
// auto auth failed.
err = err || (e as Error);
}
} else {
// Some other error unrelated to missing `projectId`
err = err || (e as Error);
}
}
}

Expand Down Expand Up @@ -715,23 +748,58 @@ export class Util {
}
};

Promise.all([
config.projectId && config.projectId !== '{{projectId}}'
? // The user provided a project ID. We don't need to check with the
// auth client, it could be incorrect.
new Promise(resolve => resolve(config.projectId))
: authClient.getProjectId(),
reqConfig.customEndpoint && reqConfig.useAuthWithCustomEndpoint !== true
? // Using a custom API override. Do not use `google-auth-library` for
// authentication. (ex: connecting to a local Datastore server)
new Promise(resolve => resolve(reqOpts))
: authClient.authorizeRequest(reqOpts),
])
.then(([_projectId, authorizedReqOpts]) => {
projectId = _projectId as string;
onAuthenticated(null, authorizedReqOpts as DecorateRequestOptions);
})
.catch(onAuthenticated);
const prepareRequest = async () => {
try {
const getProjectId = async () => {
if (
config.projectId &&
config.projectId !== DEFAULT_PROJECT_ID_TOKEN
) {
// The user provided a project ID. We don't need to check with the
// auth client, it could be incorrect.
return config.projectId;
}

if (config.projectIdRequired === false) {
// A projectId is not required. Return the default.
return DEFAULT_PROJECT_ID_TOKEN;
}

return setProjectId();
};

const authorizeRequest = async () => {
if (
reqConfig.customEndpoint &&
!reqConfig.useAuthWithCustomEndpoint
) {
// Using a custom API override. Do not use `google-auth-library` for
// authentication. (ex: connecting to a local Datastore server)
return reqOpts;
} else {
return authClient.authorizeRequest(reqOpts);
}
};

const [_projectId, authorizedReqOpts] = await Promise.all([
getProjectId(),
authorizeRequest(),
]);

if (_projectId) {
projectId = _projectId;
}

return onAuthenticated(
null,
authorizedReqOpts as DecorateRequestOptions
);
} catch (e) {
return onAuthenticated(e as Error);
}
};

prepareRequest();

if (stream!) {
return stream!;
Expand Down
8 changes: 6 additions & 2 deletions test/nodejs-common/service.ts
Expand Up @@ -22,7 +22,11 @@ import {Request} from 'teeny-request';
import {AuthClient, GoogleAuth, OAuth2Client} from 'google-auth-library';

import {Interceptor} from '../../src/nodejs-common';
import {ServiceConfig, ServiceOptions} from '../../src/nodejs-common/service';
import {
DEFAULT_PROJECT_ID_TOKEN,
ServiceConfig,
ServiceOptions,
} from '../../src/nodejs-common/service';
import {
BodyResponseCallback,
DecorateRequestOptions,
Expand Down Expand Up @@ -228,7 +232,7 @@ describe('Service', () => {

it('should default projectId with placeholder', () => {
const service = new Service(fakeCfg, {});
assert.strictEqual(service.projectId, '{{projectId}}');
assert.strictEqual(service.projectId, DEFAULT_PROJECT_ID_TOKEN);
});

it('should localize the projectIdRequired', () => {
Expand Down
73 changes: 68 additions & 5 deletions test/nodejs-common/util.ts
Expand Up @@ -14,7 +14,10 @@
* limitations under the License.
*/

import {replaceProjectIdToken} from '@google-cloud/projectify';
import {
MissingProjectIdError,
replaceProjectIdToken,
} from '@google-cloud/projectify';
import * as assert from 'assert';
import {describe, it, before, beforeEach, afterEach} from 'mocha';
import * as extend from 'extend';
Expand Down Expand Up @@ -45,6 +48,7 @@ import {
ParsedHttpRespMessage,
Util,
} from '../../src/nodejs-common/util';
import {DEFAULT_PROJECT_ID_TOKEN} from '../../src/nodejs-common/service';

// eslint-disable-next-line @typescript-eslint/no-var-requires
const duplexify: DuplexifyConstructor = require('duplexify');
Expand Down Expand Up @@ -756,7 +760,7 @@ describe('common/util', () => {
});

it('should not pass projectId token to google-auth-library', done => {
const config = {projectId: '{{projectId}}'};
const config = {projectId: DEFAULT_PROJECT_ID_TOKEN};

sandbox.stub(fakeGoogleAuth, 'GoogleAuth').callsFake(config_ => {
assert.strictEqual(config_.projectId, undefined);
Expand All @@ -768,10 +772,10 @@ describe('common/util', () => {
});

it('should not remove projectId from config object', done => {
const config = {projectId: '{{projectId}}'};
const config = {projectId: DEFAULT_PROJECT_ID_TOKEN};

sandbox.stub(fakeGoogleAuth, 'GoogleAuth').callsFake(() => {
assert.strictEqual(config.projectId, '{{projectId}}');
assert.strictEqual(config.projectId, DEFAULT_PROJECT_ID_TOKEN);
setImmediate(done);
return authClient;
});
Expand Down Expand Up @@ -901,7 +905,7 @@ describe('common/util', () => {
});
});

describe('needs authentication', () => {
describe('authentication', () => {
it('should pass correct args to authorizeRequest', done => {
const fake = extend(true, authClient, {
authorizeRequest: async (rOpts: {}) => {
Expand Down Expand Up @@ -973,6 +977,65 @@ describe('common/util', () => {
onAuthenticated: assert.ifError,
});
});

it('should use default `projectId` and not call `authClient#getProjectId` when !`projectIdRequired`', done => {
const getProjectIdSpy = sandbox.spy(authClient, 'getProjectId');

sandbox.stub(fakeGoogleAuth, 'GoogleAuth').returns(authClient);

const config = {
customEndpoint: true,
projectIdRequired: false,
};

stub('decorateRequest', (reqOpts, projectId) => {
assert.strictEqual(projectId, DEFAULT_PROJECT_ID_TOKEN);
});

const makeAuthenticatedRequest =
util.makeAuthenticatedRequestFactory(config);

makeAuthenticatedRequest(reqOpts, {
onAuthenticated: e => {
assert.ifError(e);
assert(getProjectIdSpy.notCalled);
done(e);
},
});
});

it('should fallback to checking for a `projectId` on when missing a `projectId` when !`projectIdRequired`', done => {
const getProjectIdSpy = sandbox.spy(authClient, 'getProjectId');

sandbox.stub(fakeGoogleAuth, 'GoogleAuth').returns(authClient);

const config = {
customEndpoint: true,
projectIdRequired: false,
};

const decorateRequestStub = sandbox.stub(util, 'decorateRequest');

decorateRequestStub.onFirstCall().callsFake(() => {
throw new MissingProjectIdError();
});

decorateRequestStub.onSecondCall().callsFake((reqOpts, projectId) => {
assert.strictEqual(projectId, AUTH_CLIENT_PROJECT_ID);
return reqOpts;
});

const makeAuthenticatedRequest =
util.makeAuthenticatedRequestFactory(config);

makeAuthenticatedRequest(reqOpts, {
onAuthenticated: e => {
assert.ifError(e);
assert(getProjectIdSpy.calledOnce);
done(e);
},
});
});
});

describe('authentication errors', () => {
Expand Down