panva / openid-client

OAuth 2 / OpenID Connect Client API for JavaScript Runtimes
MIT License
1.83k stars 392 forks source link

next() is not called after redirect when using passport strategy #217

Closed nolaexe closed 4 years ago

nolaexe commented 4 years ago

Describe the bug Passport.authenticate receives 3 parameters: req, res, and next. Next is a callback to call the next middleware. When using the OpenId strategy with passport using openid-client package, the next callback is never called after the redirect, thus never returning and in case running asynchronously the code written after passport.authenticate with openId strategy will be never reached.

To Reproduce

Steps to reproduce the behaviour:

  1. Initialize a new passport instance
  2. Create and configure an OpenId strategy using openid-client
  3. Call passport.authenticate, then call the returned function with req, res and next that calls console.log('test')
  4. This console.log will never be called

Expected behaviour next() to be called

Environment:

Additional context Might be because OpenIDConnectStrategy.prototype.authenticate never calls pass() after the redirect

panva commented 4 years ago

You might wanna ask/clarify this in passport itself, not this strategy.

As you can see here the code doesn't work with res nor next and it always either .success, .fail or .error as the passport "framework" wants. It also calls the strategy prototype's .redirect which means .pass is not warranted.

nolaexe commented 4 years ago

Thank you for your quick response.

nextshould receive an error as a parameter if any error occurred in the function. If next is never called, how can I check for these errors after authenticating with openid strategy?

panva commented 4 years ago

@nolaexe please consult passport, this module is merely implementing the interface passport requires, that's it. I cannot provide any insight or help to passport itself.

edit: https://github.com/jaredhanson/passport/issues/224 edit2: https://github.com/passport/www.passportjs.org/pull/51/files