search

openid / OpenID4VCI

68 stars 20 forks source link

Consider removing the `c_nonce_expires_in` parameter #394

Open tplooker opened 2 months ago

tplooker commented 2 months ago

The c_nonce_expires_in parameter is intended to signal to the client/wallet when the c_nonce expires and therefore when a client/wallet should obtain a new one. However, given a credential issuer can invalidate a c_nonce at any stage this parameter is a bit of an un-reliable signal to clients/wallets. Practically this means clients/wallets will have to account for an error at the credential response endpoint due to an invalid nonce even if it isn't expired. For that reason I think we should remove the c_nonce_expires_in parameter and therefore simplify the nonce handling.

bc-pi commented 2 months ago

Concur with the removal of c_nonce_expires_in for the reasons @tplooker mentions and others like just general aesthetics. I was wanting to drop it with the work on #381 but decided to try and keep the scope of changes there narrow in hopes of landing[1] the PR in a timely manner.

[1] I think that's what the cool kids say nowadays

bc-pi commented 2 weeks ago

Now that https://github.com/openid/OpenID4VCI/pull/404 has been merged, it seem like a good time to consider doing this. I think it's just the nonce endpoint now.

Would others agree?

I don't think the "has-PR" label is correct BTW.

babisRoutis commented 3 days ago

Hi @tplooker & @bc-pi

Totally agree.

IMHO, it is easier for a wallet to handle an invalid_proof by obtaining a new c_nonce, re-calculating the proofs and repeating the call, than having to track the c_nonce_expires_in

tplooker commented 3 days ago

Would others agree?

I do and would be happy to do the PR if we have consensus.

jogu commented 2 days ago

I think the practical outcome of removing c_nonce_expires might well be that many wallets treat nonce as single use and fetch a new one immediately prior to generating each set of proofs. (Maybe that's fine but I thought I'd mention it.)

Basically the "well I don't know if this one might still be valid so I might as well get a new one before I do a bunch of potentially 'expensive' generation of proofs" approach.

tplooker commented 2 days ago

I think the practical outcome of removing c_nonce_expires might well be that many wallets treat nonce as single use and fetch a new one immediately prior to generating each set of proofs. (Maybe that's fine but I thought I'd mention it.)

If we are concerned that this sort of implementation behaviour might set in, we could always add some implementation guidance that makes it clear that a wallet should continue to use an obtained nonce until its given a new one, for instance DPoP has the following to say on this topic.

"It is up to the authorization server when to supply a new nonce value for the client to use. The client is expected to use the existing supplied nonce in DPoP proofs until the server supplies a new nonce value."

This won't prevent a wallet electing to use a nonce only once, however I'm not sure that is preventable even with the c_nonce_expires parameter defines as a wallet could always choose to ignore this.