panva / openid-client

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

Device flow auth nonce check #271

Closed katsanva closed 4 years ago

katsanva commented 4 years ago

Hello.

I've encountered a weird behavior with polling the TokenSet for device auth: Mu OpenId server gives the id token that has nonce in JWT payload, but unfortunately the handle.poll() rejects with

RPError: nonce mismatch, expected undefined, got: 4ab20ceb73b2624074f30aa7e9284173
    at Client.validateIdToken

This happens because of id token check in device handle fails due to the actual check returns true.

So I'm just curious - is this behavior intended and there should be no nonce is the JWT (is there any RFC on that) or it's an issue in openid-client?

Just tested that manually:

> const payload = {nonce: 'fff'}
undefined
> nonce = undefined
undefined
> nonce !== null && (payload.nonce || nonce !== undefined) && payload.nonce !== nonce
true
panva commented 4 years ago

There should be no server generated nonce in the id token unless you’re sending one in. Using nonce for decice flow makes no sense anyway.

katsanva commented 4 years ago

Thanks! Isn't that better to ignore nonce in the JWT for this case instead of throwing an error?

panva commented 4 years ago

Nop, it’s better if the server doesn’t fill the id token with random values the client should be generating (not for device flow, as i said, it’s useless for that flow). But it won’t be ignored as you still may send it in. But mind you there’s no RFC defining how OIDC interacts with Device Flow.

panva commented 4 years ago

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.

panva commented 4 years ago

Think about it this way - if this was about code flow, for which you're not required to send in a nonce as a client but you got one back anyway (generated by the server) - the client would throw because it's possible a mixup attack happened. At no point in time should the server be filling in a nonce, that's a client generated value.

katsanva commented 4 years ago

Thank you for such a detailed response!