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

fix: headers were incorrectly set to application/x-www-form-urlencoded #362

Closed
wants to merge 1 commit into from
Closed

fix: headers were incorrectly set to application/x-www-form-urlencoded #362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ export class GoogleToken {
grant_type: 'urn:ietf:params:oauth:grant-type:jwt-bearer',
assertion: signedJWT,
},
headers: {'Content-Type': 'application/x-www-form-urlencoded'},
Copy link
Contributor

Choose a reason for hiding this comment

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

I am worried this is not correct. I'm reading this doc:
https://developers.google.com/identity/protocols/oauth2/service-account#httprest

It's pretty specific:

POST /token HTTP/1.1
Host: oauth2.googleapis.com
Content-Type: application/x-www-form-urlencoded

grant_type=urn%3Aietf%3Aparams%3Aoauth%3Agrant-type%3Ajwt-bearer&assertion=eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiI3NjEzMjY3OTgwNjktcjVtbGpsbG4xcmQ0bHJiaGc3NWVmZ2lncDM2bTc4ajVAZGV2ZWxvcGVyLmdzZXJ2aWNlYWNjb3VudC5jb20iLCJzY29wZSI6Imh0dHBzOi8vd3d3Lmdvb2dsZWFwaXMuY29tL2F1dGgvcHJlZGljdGlvbiIsImF1ZCI6Imh0dHBzOi8vYWNjb3VudHMuZ29vZ2xlLmNvbS9vL29hdXRoMi90b2tlbiIsImV4cCI6MTMyODU3MzM4MSwiaWF0IjoxMzI4NTY5NzgxfQ.ixOUGehweEVX_UKXv5BbbwVEdcz6AYS-6uQV6fGorGKrHf3LIJnyREw9evE-gs2bmMaQI5_UbabvI4k-mQE4kBqtmSpTzxYBL1TCd7Kv5nTZoUC1CmwmWCFqT9RE6D7XSgPUh_jF1qskLa2w0rxMSjwruNKbysgRNctZPln7cqQ

Do we have an end to end test checking this out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If gaxios sees an object it turns it into application/json for content-type, I'm 99% sure that we were never setting the form encoded header.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I totally believe you. I'm specifically scared that POST-ing JSON happens to work on the endpoint you're using, but that it isn't fully supported.

headers: {'Content-Type': 'application/json'},
responseType: 'json',
});
this.rawToken = r.data;
Expand Down
16 changes: 7 additions & 9 deletions test/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -610,16 +610,14 @@ describe('.getToken()', () => {
});

function createGetTokenMock(code = 200, body?: {}) {
return nock(GOOGLE_TOKEN_URLS[0])
return nock(GOOGLE_TOKEN_URLS[0], {
reqheaders: {'Content-Type': 'application/json'},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we had reqheaders in the wrong place, and it was not actually being asserted against.

application/x-www-form-urlencoded was already being replaced by gaxios with application/json.

Copy link
Contributor

@JustinBeckwith JustinBeckwith Feb 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I see that the docs do call out that headers are specified in this outer function, but doesn't it have to have some way to define header assertions on a per-route basis? Like it makes sense to be that these could be defined at the top level for the host, but I would expect .get and .post to accept this parameter as well. I am le nervous here. If nock isn't validating headers here, when the types accept Options in InterceptorFunction, that means there's a bug in their types as well :(

})
.replyContentLength()
.post(
GOOGLE_TOKEN_URLS[1],
{
grant_type: 'urn:ietf:params:oauth:grant-type:jwt-bearer',
assertion: /.?/,
},
{reqheaders: {'Content-Type': 'application/x-www-form-urlencoded'}}
)
.post(GOOGLE_TOKEN_URLS[1], {
grant_type: 'urn:ietf:params:oauth:grant-type:jwt-bearer',
assertion: /.?/,
})
.reply(code, body);
}

Expand Down