Skip to content

fix: allow authentication with custom endpoint #698

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
ddelgrosso1 merged 2 commits into from
Oct 6, 2021
Merged

fix: allow authentication with custom endpoint #698

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2650c41
fix: allow authentication with custom endpoint
ddelgrosso1 Oct 6, 2021
02091b8
Merge branch 'main' into ddelgrosso/custom-endpoint-auth
ddelgrosso1 Oct 6, 2021
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
1 change: 1 addition & 0 deletions src/service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export interface ServiceOptions extends GoogleAuthOptions {
token?: string;
timeout?: number; // http.request.options.timeout
userAgent?: string;
useAuthWithCustomEndpoint?: boolean;
}

export class Service {
Expand Down
8 changes: 7 additions & 1 deletion src/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@ export interface MakeAuthenticatedRequestFactoryConfig
*/
customEndpoint?: boolean;

/**
* If true, will authenticate when using a custom endpoint. Default: false.
*/
useAuthWithCustomEndpoint?: boolean;

/**
* Account email address, required for PEM/P12 usage.
*/
Expand Down Expand Up @@ -572,6 +577,7 @@ export class Util {
* (default: true)
* @param {object=} config.credentials - Credentials object.
* @param {boolean=} config.customEndpoint - If true, just return the provided request options. Default: false.
* @param {boolean=} config.useAuthWithCustomEndpoint - If true, will authenticate when using a custom endpoint. Default: false.
* @param {string=} config.email - Account email address, required for PEM/P12 usage.
* @param {number=} config.maxRetries - Maximum number of automatic retries attempted before returning the error. (default: 3)
* @param {string=} config.keyFile - Path to a .json, .pem, or .p12 keyfile.
Expand Down Expand Up @@ -697,7 +703,7 @@ export class Util {
// auth client, it could be incorrect.
new Promise(resolve => resolve(config.projectId))
: authClient.getProjectId(),
reqConfig.customEndpoint
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))
Expand Down
27 changes: 27 additions & 0 deletions test/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,33 @@ describe('common/util', () => {
});
});

describe('customEndpoint (authentication attempted)', () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let makeAuthenticatedRequest: any;
const config = {customEndpoint: true, useAuthWithCustomEndpoint: true};
Copy link
Contributor

@shaffeeullah shaffeeullah Oct 6, 2021

Choose a reason for hiding this comment

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

is the case where reqConfig.useAuthWithCustomEndpoint === undefined implicitly covered in existing tests? (guessing it is, but just want to double check). recommend adding an assert somewhere or a note verifying that auth is bypassed.

Copy link
Contributor Author

@ddelgrosso1 ddelgrosso1 Oct 6, 2021

Choose a reason for hiding this comment

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

Yes, there are existing tests that implicitly pass undefined and test. that auth is bypassed (

nodejs-common/test/util.ts

Line 826 in 2e400c6

it('should not authenticate requests with a custom API', done => {
)

Copy link
Contributor

@shaffeeullah shaffeeullah Oct 6, 2021

Choose a reason for hiding this comment

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

amazing


beforeEach(() => {
sandbox.stub(fakeGoogleAuth, 'GoogleAuth').returns(authClient);
makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory(config);
});

it('should authenticate requests with a custom API', done => {
const reqOpts = {a: 'b', c: 'd'};

stub('makeRequest', rOpts => {
assert.deepStrictEqual(rOpts, reqOpts);
done();
});

authClient.authorizeRequest = async (opts: {}) => {
assert.strictEqual(opts, reqOpts);
done();
};

makeAuthenticatedRequest(reqOpts, assert.ifError);
});
});

describe('needs authentication', () => {
it('should pass correct args to authorizeRequest', done => {
const fake = extend(true, authClient, {
Expand Down