Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider removing the c_nonce_expires_in parameter #394

Closed
tplooker opened this issue Sep 17, 2024 · 6 comments · Fixed by #419
Closed

Consider removing the c_nonce_expires_in parameter #394

tplooker opened this issue Sep 17, 2024 · 6 comments · Fixed by #419
Labels

Comments

@tplooker
Copy link
Contributor

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
Copy link
Member

bc-pi commented Sep 18, 2024

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
Copy link
Member

bc-pi commented Nov 7, 2024

Now that #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
Copy link
Contributor

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
Copy link
Contributor Author

Would others agree?

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

@jogu
Copy link
Contributor

jogu commented Nov 19, 2024

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
Copy link
Contributor Author

tplooker commented Nov 19, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants