Closed trombonekenny closed 1 year ago
The draft, now RFC9207 doesn't talk about authentication requests, only responses. That leads me to believe this is a bug.
Exactly, because this is an authorization response parameter it makes iss
listed as "callback" parameter, which means when present in the query the strategy flows to the "callback" code path.
That leads me to believe this is a bug.
Why?
That leads me to believe this is a bug.
Why?
Because iss
can also come in via an authentication request, per the LTI spec I posted. The presence of iss
doesn't necessarily mean it is an authentication response. I believe in a response, it should validate the iss like the RFC recommends. But it's presence shouldn't be used by the passport strategy to determine if the .authenticate()
is processing a request or response.
Thanks for the quick response! I didn't see that you'd made a patch so quickly when this closed. 😃
5.0.0 introduced OAuth 2.0 authorization server issuer checking which has a side effect of causing the passport strategy to process authentication requests with
iss
in them as if they were responses instead of requests.This errors with a
did not find expected authorization request details in session, req.session['foo'] is undefined
I believe this is happening because
iss
is (as of 5.0.0) listed here: https://github.com/panva/node-openid-client/blob/363c2152d125580897b394841bfc785b0cdcb054/lib/client.js#L53which causes the
if
here always fails and we pass into authentication response. https://github.com/panva/node-openid-client/blob/363c2152d125580897b394841bfc785b0cdcb054/lib/passport_strategy.js#L88To Reproduce Steps to reproduce the behaviour:
iss
property presentExpected behaviour I'm currently working around this by doing a
delete req.body.iss
in my authentication request route before callingpassport.authenticate
. Then it behaves like the 4.9.1 version and processes the authentication request properly.Environment:
Additional context The draft, now RFC9207 doesn't talk about authentication requests, only responses. That leads me to believe this is a bug.
The LTI 1.3 security framework is an example of a spec that says
iss
is required in the third-party initiated login authentication request.