search

panva / openid-client

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

Reopening https://github.com/panva/node-openid-client/issues/208 #209

Closed som-nitjsr closed 4 years ago

som-nitjsr commented 4 years ago

Describe the bug

I still believe the issue is in this library only. In file https://github.com/panva/node-openid-client/blob/master/lib/passport_strategy.js at we are using if (!req.session) { throw new TypeError('authentication requires session support'); } and we are storing all the nonce variable in session. req.session[sessionKey] = pick(params, 'nonce', 'state', 'max_age', 'response_type'); If my i enable strict for my session cookies. these values will not be their when request come back from IDP. because browser will not allow the cookies id. Can you please add support fop storing these variable other than session cookie. let me know if i need to explain a bit more. To Reproduce Issuer and Client configuration: (inline or gist) - Don't forget to redact your secrets.

// Issuer configuration (issuer.metadata) and how it is constructed (discovery or manual?)
{
  // ...
}
// Client configuration (client.metadata) and how it is constructed (fromUri or manual?)
{
  // ...
}

Steps to reproduce the behaviour:

Expected behaviour A clear and concise description of what you expected to happen.

Environment:

Additional context Add any other context about the problem here.

panva commented 4 years ago

Removing the binding of nonce, state and other parameters to the browser session goes against every best current practice, i don't see why we'd remove that. Cookies are just needed since there's no other mechanism available to do that binding.

Removing state for code flow makes you csrf vulnerable. Removing nonce for id_token flow does the same.

I still believe the issue is in this library only.

It is not, by setting your session cookie to strict and therefore not receiving your cookie on a top level redirect (which you would with lax) you're removing one of the prerequisites for a secure oauth2 flow (it says so right in the error message authentication requires session support, it is INTENDED).

If you 100% want strict cookies, make your callback a self-submitting form rendering html route that takes the parameters and re-post's them to the same route only POST, on which you'll have the strategy callback.

panva commented 4 years ago

Also, do not "re-open" an issue by creating a new one. You can still comment on closed issues.