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.

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

Jump to bottom

feat: set x-goog-user-project header, with quota_project from default credentials #829

Merged
merged 3 commits into from
Nov 20, 2019
Merged

feat: set x-goog-user-project header, with quota_project from default credentials #829

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ed99191
feat: set x-goog-user-project header, with quota_project from default…
bcoe Nov 20, 2019
ef43af7
chore: fix up second test
bcoe Nov 20, 2019
2b4c84f
chore: couldn't help myself, let's try out optional chain
bcoe Nov 20, 2019
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
1 change: 1 addition & 0 deletions src/auth/credentials.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export interface JWTInput {
client_id?: string;
client_secret?: string;
refresh_token?: string;
quota_project?: string;
bcoe marked this conversation as resolved.
Show resolved Hide resolved
}

export interface CredentialBody {
Expand Down
11 changes: 10 additions & 1 deletion src/auth/googleauth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,16 @@ export class GoogleAuth {
*/
async getRequestHeaders(url?: string) {
const client = await this.getClient();
return client.getRequestHeaders(url);
let headers = client.getRequestHeaders(url);
// quota_project, stored in application_default_credentials.json, is set in
// the x-goog-user-project header, to indicate an alternate account for
// billing and quota:
if (this.jsonContent && this.jsonContent.quota_project) {
broady marked this conversation as resolved.
Show resolved Hide resolved
headers = Object.assign({}, headers, {
'x-goog-user-project': this.jsonContent.quota_project,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

A comment about why it's not the other way around, i.e.
Object.assign({}, ..., headers)

Copy link
Contributor

Choose a reason for hiding this comment

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

If client.getRequestHeaders() always returns an object, the whole Object.assign can be removed and replaced with a simple assignment :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tend to agree that headers should take precedence if set explicitly, I've updated the PR and added a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've noticed a few bugs in our auth library in my time working on it, caused by passing an object by reference and than modifying the object, e.g., deleting a key.

I've been being defensive by creating shallow copies with Object.assign({}), something worth thinking about in API design would be starting to Object.freeze objects that we don't wish for users to modify, at which point it forces people to make copies, but you can pass the object around if you do not need to mutate it.

}
return headers;
}

/**
Expand Down
8 changes: 8 additions & 0 deletions test/fixtures/.config/gcloud/application_default_credentials.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"client_id": "764086051850-6qr4p6gpi6hn506pt8ejuq83di341hur.apps.googleusercontent.com",
"client_secret": "privatekey",
"refresh_token": "refreshtoken",
"type": "authorized_user",
"project_id": "my-project",
"quota_project": "my-quota-project"
}
38 changes: 38 additions & 0 deletions test/test.googleauth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1444,4 +1444,42 @@ describe('googleauth', () => {
/Passing options to getClient is forbidden in v5.0.0/
);
});

it('getRequestHeaders() populates x-goog-user-project with quota_project if present', async () => {
// Fake a home directory in our fixtures path.
mockEnvVar('GCLOUD_PROJECT', 'my-fake-project');
mockEnvVar('HOME', './test/fixtures');
mockEnvVar('APPDATA', './test/fixtures/.config');
// The first time auth.getClient() is called /token endpoint is used to
// fetch a JWT.
const req = nock('https://oauth2.googleapis.com')
.post('/token')
.reply(200, {});

const auth = new GoogleAuth();
const headers = await auth.getRequestHeaders();
assert.strictEqual(headers['x-goog-user-project'], 'my-quota-project');
req.done();
});

it('getRequestHeaders() does not populate x-goog-user-project if quota_project is not present', async () => {
// Fake a home directory in our fixtures path.
mockEnvVar('GCLOUD_PROJECT', 'my-fake-project');
mockEnvVar('HOME', './test/fixtures');
mockEnvVar('APPDATA', './test/fixtures/.config');
// The first time auth.getClient() is called /token endpoint is used to
// fetch a JWT.
const req = nock('https://oauth2.googleapis.com')
.post('/token')
.reply(200, {});

const auth = new GoogleAuth();
// Force jsonContent to load, and then remove the quota_project parameter.
await auth.getClient();
delete auth.jsonContent!.quota_project;

const headers = await auth.getRequestHeaders();
assert.strictEqual(headers['x-goog-user-project'], undefined);
req.done();
});
});