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

'openid-client' doesn't play well with @nestjs/passport #141

Closed
hegelstad opened this issue Nov 23, 2019 · 4 comments
Closed

'openid-client' doesn't play well with @nestjs/passport #141

hegelstad opened this issue Nov 23, 2019 · 4 comments

Comments

@hegelstad
Copy link

hegelstad commented Nov 23, 2019

I'm submitting a...

[ ] Regression 
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

The strategy delivered by openid-client does not have access to all params such as userinfo-object because the library openid-client does a check on the number of parameters sent into the callback function. Because the callback function uses destructuring of the params, the number of parameters equals 0 and thus the userinfo-object of openid-client is not appended in the parameters. This is very unexpected behavior, as it "works" when putting the verify function directly in the constructor super call where the parameters are declared separately (tokenset, userinfo, done) => {}. This is unwanted as I would really like to stay idiomatic and use the framework as it is meant to be used.

Expected behavior

Ideally, I would be able to declare the params (tokenset, userinfo) in the validate function, as I am trying to do, and still have the openid-client correctly interpret the correct amount of parameters.

Minimal reproduction of the problem with instructions

In the below screenshot we see the check that adds the userinfo object if the length of the parameters of the verify/validate function is large enough. In our case when using the Nest PassportStrategy validate function, the destructuring of params results in this._verify.length // => 0.
Screenshot 2019-11-23 at 18 00 06

Here the destructuring happens:
Screenshot 2019-11-23 at 18 00 32

What is the motivation / use case for changing the behavior?

PassportModule should be able to support openid-client (OpenID Connect) which covers strategies for a wide range of IdPs such as Okta and OneLogin.

Environment


Nest version: 6.10.1
@kamilmysliwiec
Copy link
Member

Do you want to create a PR for this issue?

@MilkWangStudio
Copy link

I got the problem too

@hegelstad
Copy link
Author

@kamilmysliwiec I forgot to reply, my issue is that I don't really understand how I would fix this. I made an attempt but seemed to get nowhere close.

@kamilmysliwiec
Copy link
Member

The only thing I can suggest is to simply copy & paste the passport.strategy.ts file into your project and adjust the callback call to your requirements. There are tons of various passport strategies and adapting all of them with one class is simply impossible (not enough flexible).

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

No branches or pull requests

3 participants