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

TypeError: Cannot read properties of undefined (reading 'access_token') #151

Closed
imunderwater opened this issue Aug 14, 2024 · 18 comments
Closed

Comments

@imunderwater
Copy link

When working with GitHub OAuth2, everything seems to work fine initially. However, when the auth server redirects to the client callback, an error is thrown indicating: TypeError: Cannot read properties of undefined (reading 'access_token'). I tested it with other libraries, such as simple-oauth2, and it works fine. This issue appears to be related to oauth2-client and seems to be a bug that needs to be fixed.

@imunderwater
Copy link
Author

imunderwater commented Aug 14, 2024

OAuth2Token should be of type unknown, we don't know what is the response format that the provider return.

@evert can you check this issue please

@evert
Copy link
Collaborator

evert commented Aug 14, 2024

Please share your code so I can reproduce this issue! Redact any secrets. I want to see your config + it would also be helpful to see an example of the url you're parsing (also redact any tokens)

@imunderwater
Copy link
Author

  const client = new OAuth2Client({
    authorizationEndpoint,
    tokenEndpoint,
    clientSecret,
    clientId,
  });
  async function createAuthorizationUri(
    state: string,
    redirect_uri,
  ): Promise<string> {
    const authorizationUri = await client.authorizationCode.getAuthorizeUri({
      redirectUri: redirect_uri,
      state: state,
      scope: ["read:user"],
    });

    return authorizationUri;
  }

  async function validateAuthorizationCode(code: string, redirect_uri): Promise<object> {
      const token = await client.authorizationCode.getToken({ code, redirect_uri });
     return token;
}

its the same code that i got google auth working, but instead for github doesn't work (it throws an error)

@evert
Copy link
Collaborator

evert commented Aug 14, 2024

Could you share the configuration values you're using?

@imunderwater
Copy link
Author

imunderwater commented Aug 14, 2024

sorry for the late reply
@evert here it is.

const client = new OAuth2Client({
    authorizationEndpoint: "https://github.com/login/oauth/authorize",
    tokenEndpoint: "https://github.com/login/oauth/access_token",
    clientSecret,
    clientId,
  });

as the Github oauth documentation.

@evert
Copy link
Collaborator

evert commented Aug 14, 2024

I started testing this, and at the very least this line is wrong (Typescript should error on this):

  const token = await client.authorizationCode.getToken({ code, redirect_uri });

Note that redirect_uri should be redirectUri. After that fix I was able to replicate the issue.

So it looks like Github actually 2 bugs, not this library. The issue is that Github replies with a application/x-www-form-urlencoded, when it should reply with application/json. This is non-standard behavior. Luckily it seems that adding an Accept header fixes this and doesn't have any real drawbacks.

A second issue is that Github responds with 200 OK for error =(. Pretty bad! But I can add a workaround for this too.

@evert
Copy link
Collaborator

evert commented Aug 14, 2024

I also submitted a bug report here: https://github.com/orgs/community/discussions/135818

Lastly, i think the feedback in this library could be a bit better: TypeError: Cannot read properties of undefined (reading 'access_token) is not a great error for this kind of thing!

@imunderwater
Copy link
Author

imunderwater commented Aug 14, 2024

I understand. Given that it works fine with other libraries, do you think the issue lies on oauth2-client or GitHub (or both?) Should I try adding an Accept header to the request?

@evert
Copy link
Collaborator

evert commented Aug 14, 2024

It's 100% a Github issue, but adding the Accept header will fix the issue for you (although errors will not yet be correctly processed). I do intend to add these workarounds to this library as well so in the future it's not an issue.

@imunderwater
Copy link
Author

imunderwater commented Aug 14, 2024

@evert It would be great if you could add these workarounds to avoid writing extra code. the application/json should be the default for oauth2-client, by adding that this issue will be fixed.

this line should be achieved on the getToken function too

If these headers are the default, the Accept header should be included here

@imunderwater
Copy link
Author

one more thing OAuth2Token should be keys of unknown, We don't know the response format coming from the token endpoint.

@evert
Copy link
Collaborator

evert commented Aug 14, 2024

@evert It would be great if you could add these workarounds to avoid writing extra code. the application/json should be the default for oauth2-client, by adding that this issue will be fixed.

Yes, as I said I intend to add these workarounds.

one more thing OAuth2Token should be keys of unknown, We don't know the response format coming from the token endpoint.

OAuth2 is a standard format. We definitely do know what the format will be.

@imunderwater
Copy link
Author

@evert how can i get the id_token from the google oauth that the auth server return?

@evert
Copy link
Collaborator

evert commented Aug 14, 2024

OpenID Connect support is out of scope for this library. I've started thinking a bit more about dealing with arbitrary properties here #145 but not sure yet how I will deal with that. Changing a type to unknown wouldn't do anything in this case.

@imunderwater
Copy link
Author

imunderwater commented Aug 15, 2024

Obtaining the actual response form is useful. I need to use the id_token and scoop, but it's not possible to do so.
it would be great if you could achieve that as well.

@imunderwater
Copy link
Author

hey @evert, any update on extra parameters or the Accept method?, i really need this for my project.

@evert
Copy link
Collaborator

evert commented Aug 16, 2024

No update. I will get to it when I can. In the meantime you can:

  1. monkeypatch this
  2. fork and change it
  3. submit a pull request to help move this along. If no tests failed after making this change, add a test.

if you're in a hurry 1) is probably the quickest, and you can just remove that again when this is changed in a future release.

evert added a commit that referenced this issue Aug 22, 2024
By default Github will return urlencoded data when doing requests on the
token endpoint. Setting this accept header ensures we're getting JSON
back.

This is non-standard, but there's no harm.

See #151
@evert
Copy link
Collaborator

evert commented Aug 22, 2024

2 fixes are in, #152 and #153. Closing this! Thanks for the bug report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants