remove c_nonce and c_nonce_expires_in from credential endpoint response #404
Conversation
|
Using self-contained nonces, this allows the nonce endpoint to be better separated form the Credential Issuer and it makes Wallet implementaitons easier as there are less options. |
There was a problem hiding this comment.
This PR provides a nice simplification but from my understanding has 2 potential downsides:
- adds a round trip in the corner cases where a server rejects a proof because the nonce was deemed invalid
- removes the possibility to provide stateful nonces - the nonce endpoint is not protected and there is no possibility (I think?) for this right now
The only thing that worries me a bit is losing the possibility to provide stateful nonces, but I am not entirely sure if we need them - are people using that right now? If we really want/need to preserve the ability to have stateful nonces, then we could also add the option to have the nonce endpoint protected and signal that via metadata I guess?
Co-authored-by: Christian Bormann <[email protected]>
add invalid_nonce error code Co-authored-by: Christian Bormann <[email protected]>
remove reference to credential request
Another downside could be additional calls to the nonce endpoint if several credentials have to be requested. The current spec is not entirely clear whether each credential request MUST us a fresh nonce in this case. The name "nonce" indicates use only-once principle. However, I know that not everyone is so strict about this and that there are some implementations that allow the multiple use of a nonce within a certain period of time. I would prefer it if it was either clearly defined as use only-once nonce or if the nonce was renamed to challenge, which can be used multiple times within a certain period of time. |
|
If I see it correctly this PR also addresses #394, right? Also, not sure about the process, but do we need to add the notes for the next draft regarding what changed? |
|
with my chair hat on, noting that there is not enough discussion in the issue to be called an agreement to remove this feature. |
|
Neither was there enough discussion on this in the none endpoint PR, but Brian just said he doesn't see this. I think the PR helps to make people understand this and reflects our real developer experience from implementing nonce endpoint. |
|
Brian first did a PR without discussion and that one got closed, we did discuss that in the following calls and there are multiple other issuer where the topic was discussed so when the PR came up the second time, the WG was more prepared. and for clarify, I did not close this PR. I am just noting down that more discussion is needed for those looking at it |
|
@Sakurann |
I'm not entirely sure I know what that means. But there were hundreds of comments across #381, #199, & #39 discussing removal of c_nonce* from the AS's token endpoint. The issue behind this PR #393 and an issue to get rid of c_nonce_expires_in in general #394 were broken out from #381 in an attempt to keep its scope tight and allow it to proceed. The nonce endpoint was added to #381, which was an expansion of scope, but as a condition of acceptance by a few key opponents. And even that had quite a bit of discussion. |
Stateful with respect to the access token or something contained therein. But good question.
That could be signaled by a 401 at the nonce endpoint too. |
Co-authored-by: Nemanja Patrnogic <[email protected]>
Co-authored-by: Nemanja Patrnogic <[email protected]>
|
Discussed in pre-IIW hybrid meeting - agreed to proceed with this PR, merging once 1-2 more approvals |
|
@mickrau can you please add a changelog entry that this option was removed? I will merge this PR after that |
resolves #393
Simplify spec by only provide one way to get a c_nonce