Allow the nonce to be empty

[DrDaveD]

I ran into a nonce mismatch error with the new oidc provider and when writing the refresh_token through the API. This prevents the error.

[DrDaveD]

I did a force-push after running goimports

[impl]

Hmm, I'm not sure about this change. The nonce is a relatively important security mechanism to prevent MITM/replay attacks. If the server is sending back a nonce, it should match something you have recorded somewhere. The spec is pretty clear about this: "If present in the ID Token, Clients MUST verify that the nonce Claim Value is equal to the value of the nonce parameter sent in the Authentication Request."

That said, it feels unusual for a nonce to come through on a refresh in the ID token. I'm wondering if it's actually set to a valid value, or if we're just doing a bad job of detecting when it's absent in the ID token?

[DrDaveD]

Ok then this check should only be being done on the initial exchange, not in a refresh nor in an RFC8693 exchange (which is not yet in this code but is in the features/token-exchange branch). I will try to change it so it is only done there.

[impl]

I found a bug in the current implementation that is causing the nonce to never be sent through that our test wasn't catching. Let me try fixing that first and let's see if that resolves the problem.

FWIW, the spec also doesn't seem to think that you should change the behavior for refreshing, saying "otherwise, the same rules apply as apply when issuing an ID Token at the time of the original authentication." Is it possibly a bug in the IdP you're using that's sending this nonce through in the ID Token on refresh?

[DrDaveD]

ID Tokens are not required on refresh as the spec says. I don't think my token issuer sends it.

Remember that in my case I'm not doing the code flow at all through this plugin, that's done in the auth-jwt plugin. I'm only passing in the refresh_token in the credsUpdateOperation() and there is no nonce.

[DrDaveD]

I don't think my token issuer sends it.

I must be wrong about that if I'm reading this plugin's code correctly, because it seems to be requiring an id token.

I do know the nonce that was passed to the auth-jwt plugin. Maybe I just need to have a nonce parameter on the credsUpdateOperation to pass it there along with the refresh_token.

[DrDaveD]

I do know the nonce that was passed to the auth-jwt plugin.

No, actually I don't. The auth-jwt plugin supports an optional "client_nonce" to exchange between the plugin and the client, but that's different than the nonce that that the plugin uses between itself and token issuer.

So anyway, since the ID token is not required to be sent in response to a refresh request, I don't see why it has to be required to be checked again at that point if it is present.

[impl]

Oh interesting. I overlooked that a refresh might not return an ID token. I agree that if it is missing from the response in that case it shouldn't be checked. I'll open a PR to address that as well. If it is returned, though, it seems like the spec expects it to be revalidated?

[DrDaveD]

I think that the spec is not clear about that. Section 7.5 point 8 talks about nonces being used to avoid replay attacks. I don't see how that's relevant to validating a response to a refresh token request which may be long after the original request, even months if refresh tokens are updated. It's also only a "should" and not a must. If a nonce were needed for refresh requests, to me the only thing that makes sense would be to create a new nonce for each request.

Anyway, in my case there isn't any practical way to implement it.

[impl]

Yeah, I think you're right -- a valid interpretation of this would be that it is up to the user of this plugin to decide whether a nonce "was sent in the Authentication Request," i.e., a server could also return an impossible-to-satisfy nonce for fun and it should be ignored if the plugin's user doesn't direct the plugin to validate the nonce. So in that sense this is indeed the correct change to make. I'm going to go ahead and merge this as-is, thanks!