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

/token: Authorization header is not set #70

Closed
bbigras opened this issue Jul 4, 2022 · 1 comment · Fixed by #72
Closed

/token: Authorization header is not set #70

bbigras opened this issue Jul 4, 2022 · 1 comment · Fixed by #72
Assignees
Labels
bug Something isn't working

Comments

@bbigras
Copy link
Contributor

bbigras commented Jul 4, 2022

I'm using this and the Authorization header doesn't seem to be set in the request:

clientSecret is set.

  const oauth2Token = await client.authorizationCode.getToken({
    code: code,
    redirectUri: "http://localhost:3000/callback"
  });

I'm wondering if this condition should be body.grant_type === 'authorization_code':

oauth2-client/src/client.ts

Lines 268 to 271 in 00f1cd3

if (body.grant_type !== 'authorization_code' && this.settings.clientSecret) {
const basicAuthStr = btoa(this.settings.clientId + ':' + this.settings.clientSecret);
headers.Authorization = 'Basic ' + basicAuthStr;
}

@evert
Copy link
Collaborator

evert commented Jul 4, 2022

I was under the assumption that the secret shouldn't be specified, because this request is usually done by browsers and a clientSecret would be kinda pointless there, but it appears from the specification that this incorrect; which makes sense now given that the request to the /token endpoint could also be done by another server.

So this is a gap in the library, need to fix this!

@evert evert added the bug Something isn't working label Jul 4, 2022
@evert evert self-assigned this Jul 4, 2022
evert added a commit that referenced this issue Jul 4, 2022
Also added a bunch of tests for this.

Fixes #70
@evert evert closed this as completed in #72 Jul 4, 2022
bradjones1 pushed a commit to bradjones1/fetch-mw-oauth2 that referenced this issue Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants