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 property 'includes' of undefined #312

Closed
codebling opened this issue Nov 25, 2020 · 3 comments
Closed

TypeError: Cannot read property 'includes' of undefined #312

codebling opened this issue Nov 25, 2020 · 3 comments
Labels

Comments

@codebling
Copy link

codebling commented Nov 25, 2020

Describe the bug

UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'includes' of undefined
    at new OpenIDConnectStrategy (/project/node_modules/openid-client/lib/passport_strategy.js:60:101)
    at init (/project/lib/my-code.js:50:20)

To Reproduce
Initialise a Passport.js Strategy using openid-client.
When creating the new Client, include more than one response type in response_types

  const client = new issuer.Client({
    response_types: [
      'code',
      'another response type',
    ],
  });

When creating the new Strategy, do not specify response_type in options.params, or specify no options.

  const strategy = new Strategy({client}, verifyCallback);

Expected behaviour
No TypeError.

Environment:

  • openid-client version: v4.2.1
  • node version: v14

Additional context
Seems to be due to the code in resolveResponseType() in lib/helpers/client.js. If there is more than one code in response_types, it returns undefined. The same is true of resolveRedirectUri().

passport_strategy.js seems to want this._params.response_type to be an array, since it is calling .includes() everywhere. Or perhaps it is by design that it is using the string method .includes()? This would seem strange to me. Or perhaps I should never be adding additional response_types in Client metadata without specifying response_type in params when constructing a new Strategy, which would avoid the call to resolve the response type from the client ?

@panva
Copy link
Owner

panva commented Nov 25, 2020

Or perhaps I should never be adding additional response_types in Client metadata without specifying response_type in params when constructing a new Strategy, which would avoid the call to resolve the response type from the client ?

Either specify a configuration value for usePKCE, which when not provided tries to default get enabled based on the response_type. But response_type is only only "resolved" when there's a single one.

So, either just

  • set usePKCE to a value
  • provide a single response type
  • provide the params object with a response type

Either way you'll have to be providing it at authenticate time...

I think the fix here is resolving usePKCE at request time, in which case it would throw for you at that time if you didn't specify a concrete response type.

Not sure when i'll get to this, seems not that critical as there are clear ways for you to move forward.

@panva panva added bug and removed triage labels Nov 25, 2020
@codebling
Copy link
Author

Thanks for the quick response. There's absolutely no pressure on my end, I am just poking around a little. I've done OpenID before, but not in as detailed a fashion and never with this library, so many of the terms are new to me. I'm really just reporting as the behaviour seemed somewhat incorrect, I'm not blocked in any way, feel free to close.

@panva panva closed this as completed in 1970af4 Nov 30, 2020
@panva
Copy link
Owner

panva commented Nov 30, 2020

Fixed in v4.2.2

Please consider supporting the library if it provides value to you or your company and this support was of help to you. Supporting the library means, amongst other things, that such support will be available in the future.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
2 participants