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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Support Not Requiring projectId When Not Required #1448

Merged
merged 4 commits into from
Aug 31, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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
49 changes: 38 additions & 11 deletions src/auth/googleauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,13 @@ export interface GoogleAuthOptions<T extends AuthClient = JSONClient> {
export const CLOUD_SDK_CLIENT_ID =
'764086051850-6qr4p6gpi6hn506pt8ejuq83di341hur.apps.googleusercontent.com';

const GoogleAuthExceptionMessages = {
NO_PROJECT_ID_FOUND:
'Unable to detect a Project Id in the current environment. \n' +
'To learn more about authentication and Google APIs, visit: \n' +
'https://cloud.google.com/docs/authentication/getting-started',
} as const;

export class GoogleAuth<T extends AuthClient = JSONClient> {
transporter?: Transporter;

Expand Down Expand Up @@ -191,9 +198,32 @@ export class GoogleAuth<T extends AuthClient = JSONClient> {
}
}

private getProjectIdAsync(): Promise<string | null> {
/**
* A temporary method for internal `getProjectId` usages where `null` is
* acceptable. In a future major release, `getProjectId` should return `null`
* (as the `Promise<string | null>` base signature describes) and this private
* method should be removed.
*
* @returns Promise that resolves with project id (or `null`)
*/
async #getProjectIdOptional(): Promise<string | null> {
try {
return await this.getProjectId();
} catch (e) {
if (
e instanceof Error &&
e.message === GoogleAuthExceptionMessages.NO_PROJECT_ID_FOUND
Comment on lines +214 to +215

Choose a reason for hiding this comment

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

checking for instanceof Error is quite redundant here, if it threw like an error and has a message property like an error, it's def going to be an error 😄

Suggested change
e instanceof Error &&
e.message === GoogleAuthExceptionMessages.NO_PROJECT_ID_FOUND
e.message === GoogleAuthExceptionMessages.NO_PROJECT_ID_FOUND

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understandable; the instanceof Error check is more-so a type assertion. By default, TS considers 'e' unknown

) {
return null;
} else {
throw e;
}
}
}
Comment on lines +209 to +222

Choose a reason for hiding this comment

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

Leveraging the Promises API (similar to the previous PR) 😄 might simplify it a bit here again:

Suggested change
async #getProjectIdOptional(): Promise<string | null> {
try {
return await this.getProjectId();
} catch (e) {
if (
e instanceof Error &&
e.message === GoogleAuthExceptionMessages.NO_PROJECT_ID_FOUND
) {
return null;
} else {
throw e;
}
}
}
#getProjectIdOptional(): Promise<string | null> {
return this.getProjectId()
.catch (e => {
if (e.message === GoogleAuthExceptionMessages.NO_PROJECT_ID_FOUND) {
return null;
} else {
throw e;
}
})
}


private async getProjectIdAsync(): Promise<string | null> {
if (this._cachedProjectId) {
return Promise.resolve(this._cachedProjectId);
return this._cachedProjectId;
}

// In implicit case, supports three environments. In order of precedence,
Expand All @@ -212,11 +242,7 @@ export class GoogleAuth<T extends AuthClient = JSONClient> {
(await this.getExternalAccountClientProjectId());
this._cachedProjectId = projectId;
if (!projectId) {
throw new Error(
'Unable to detect a Project Id in the current environment. \n' +
'To learn more about authentication and Google APIs, visit: \n' +
'https://cloud.google.com/docs/authentication/getting-started'
);
throw new Error(GoogleAuthExceptionMessages.NO_PROJECT_ID_FOUND);
}
return projectId;
})();
Expand Down Expand Up @@ -269,7 +295,7 @@ export class GoogleAuth<T extends AuthClient = JSONClient> {
if (this.cachedCredential) {
return {
credential: this.cachedCredential,
projectId: await this.getProjectIdAsync(),
projectId: await this.#getProjectIdOptional(),
};
}

Expand All @@ -287,7 +313,8 @@ export class GoogleAuth<T extends AuthClient = JSONClient> {
credential.scopes = this.getAnyScopes();
}
this.cachedCredential = credential;
projectId = await this.getProjectId();
projectId = await this.#getProjectIdOptional();

return {credential, projectId};
}

Expand All @@ -302,7 +329,7 @@ export class GoogleAuth<T extends AuthClient = JSONClient> {
credential.scopes = this.getAnyScopes();
}
this.cachedCredential = credential;
projectId = await this.getProjectId();
projectId = await this.#getProjectIdOptional();
return {credential, projectId};
}

Expand All @@ -329,7 +356,7 @@ export class GoogleAuth<T extends AuthClient = JSONClient> {
// the rest.
(options as ComputeOptions).scopes = this.getAnyScopes();
this.cachedCredential = new Compute(options);
projectId = await this.getProjectId();
projectId = await this.#getProjectIdOptional();
return {projectId, credential: this.cachedCredential};
}

Expand Down
23 changes: 23 additions & 0 deletions test/test.googleauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1991,6 +1991,29 @@ describe('googleauth', () => {
);
scopes.forEach(s => s.done());
});

it('should return `null` for `projectId` when on cannot be found', async () => {
// Environment variable is set up to point to external-account-cred.json
mockEnvVar(
'GOOGLE_APPLICATION_CREDENTIALS',
'./test/fixtures/external-account-cred.json'
);

const auth = new GoogleAuth();

sandbox
.stub(
auth as {} as {
getProjectIdAsync: Promise<string | null>;
},
'getProjectIdAsync'
)
.resolves(null);

const res = await auth.getApplicationDefault();

assert.equal(res.projectId, null);
});
});

describe('getApplicationCredentialsFromFilePath()', () => {
Expand Down