remove `c_nonce` from the token endpoint response #381

bc-pi · 2024-08-27T11:20:39Z

remove c_nonce and c_nonce_expires_in from the token endpoint response to fix #39

again

…ponse to fix #39

tlodderstedt

I suggest to add a dedicated nonce endpoint to the credential issuer as replacement for the c_nonce token response parameter. Just removing the c_nonce will force wallets to try the credential issuance request in order to get a nonce in order to be able to create a proper nonce-bound proof of possession. That's complicated.

jogu · 2024-08-27T21:00:13Z

I think this is the previous issue about a nonce endpoint for wallet attestation that people were asking about on today's call:

#71

bc-pi · 2024-08-29T17:29:47Z

There has been a lot of discussion about this (and roughly mostly in favor of it) in recent months in #39 and also #331 and maybe elsewhere too. [edit: like https://github.com//issues/357 ]

peppelinux · 2024-09-03T19:33:01Z

I suggest to add a dedicated nonce endpoint to the credential issuer as replacement for the c_nonce token response parameter.

@tlodderstedt that's exactly what I had in mind when I started writing this draft: https://github.com/peppelinux/draft-demarco-oauth-nonce-endpoint?tab=readme-ov-file

@jogu many of the conversations and points raised within the issue #71 was filtered and mentioned in the nonce endpoint draft

…ue without the overhead of a full Credential Request

bc-pi · 2024-09-04T21:09:35Z

as requested, 074065b adds a dedicated nonce endpoint to the credential issuer as replacement for the c_nonce token response parameter

paulbastian

I would like to mandate nonce endpoint if the issuer requires the use of c_nonce, i.e. the presence of the endpoint enforces its usage, therefore we could get rid of sending fake ugly Credential requests to fetch a c_nonce completely.

Having this endpoint optional and no other indication of the issuer whether he requires c_nonce for proofs seems inefficient.

bc-pi · 2024-09-06T22:57:51Z

I would like to mandate nonce endpoint if the issuer requires the use of c_nonce

48a3678 updates the proposed text to do that.

openid-4-verifiable-credential-issuance-1_0.md

Sakurann · 2024-09-10T13:50:45Z

openid-4-verifiable-credential-issuance-1_0.md

@@ -710,6 +706,45 @@ Cache-Control: no-store
 }
 ```

+# Nonce Endpoint {#nonce-endpoint}
+
+A Credential Issuer that requires `c_nonce` values to be incorporated into proofs in the Credential Request (see (#credential-request)) MUST offer a Nonce Endpoint. This endpoint allows a Client to acquire a fresh `c_nonce` value without the overhead of a full Credential Request.


improve readability

Suggested change

A Credential Issuer that requires `c_nonce` values to be incorporated into proofs in the Credential Request (see (#credential-request)) MUST offer a Nonce Endpoint. This endpoint allows a Client to acquire a fresh `c_nonce` value without the overhead of a full Credential Request.

This endpoint allows a Client to acquire a fresh `c_nonce` value without the overhead of a full Credential Request. A Credential Issuer that requires `c_nonce` values to be incorporated into proofs in the Credential Request (see (#credential-request)) MUST offer a Nonce Endpoint.

but also do we really want to mandate nonce endpoint for all issuers that require c_nonce? than why keep an option to return c_nonce in credential response?

was trying to keep the scope of the changes to something that had been agreed on while also accommodating Paul's request for changes

while also keeping my own frustration in check about the fact that these are all things that were only hidden by the nonce from token endpoint but have existed all along so why do they always and only come up in the context of trying to make an improvement

Also removing the ability to have send a c_nonce in the credential response would probably be a breaking change by most measures and it's unclear if the WG has the appetite for that.

openid-4-verifiable-credential-issuance-1_0.md

Sakurann

overall looks good. I almost approved, but I think we should discuss if we really want to mandate nonce endpoint for all issuers that require c_nonce? than why keep an option to return c_nonce in credential response?

Co-authored-by: Kristina <[email protected]>

bc-pi · 2024-09-10T16:33:20Z

overall looks good. I almost approved, but I think we should discuss if we really want to mandate nonce endpoint for all issuers that require c_nonce? than why keep an option to return c_nonce in credential response?

fair questions that i guess should be discussed in the WG

openid-4-verifiable-credential-issuance-1_0.md

paulbastian

I like the current direction.

If we keep things as-is, we can completely delete the chapter 7.3.2 as the nonce endpoint is doing the job and all other details are already covered in the main section of Credential Response

openid-4-verifiable-credential-issuance-1_0.md

bc-pi · 2024-09-16T21:22:34Z

overall looks good. I almost approved, but I think we should discuss if we really want to mandate nonce endpoint for all issuers that require c_nonce? than why keep an option to return c_nonce in credential response?

fair questions that i guess should be discussed in the WG

Noting that removing the ability to have send a c_nonce in the credential response would probably be a breaking change by most measures. Which, I think, would be okay by me. But should have some buy-in from the WG.

openid-4-verifiable-credential-issuance-1_0.md

Sakurann · 2024-09-17T19:53:35Z

Discussed on the WG call, direction on removing section 7.3.2 and consolidating its content in Nonce Endpoint definition.
https://openid.github.io/OpenID4VCI/openid-4-verifiable-credential-issuance-wg-draft.html#section-7.3.2

also in this PR, add a little more explanation that issuer can invalidate c_nonce any time. please open an issue on removing c_nonce_expires_in parameter (@tplooker).

also agreed to open a separate issue on removing an option to return c_nonce from the credential error response, so that we can merge this PR as-is (@bc-pi).

paulbastian · 2024-09-24T16:50:06Z

As I would read this PR, it recommends the wallet to use the nonce endpoint if it's present and the parameter in credential response is kind of an alternative/fallback. Therefore this paragraph does not fit any longer.

bc-pi · 2024-09-24T17:08:15Z

aadb2bb is what I was able to summon the appetite for

Co-authored-by: Giuseppe De Marco <[email protected]>

tlodderstedt · 2024-09-26T15:50:51Z

openid-4-verifiable-credential-issuance-1_0.md

+Below is a non-normative example of a Nonce Request:
+
+```
+GET /nonce HTTP/1.1


Wouldn't a POST request be more appropriate? I mean the endpoint is supposed to provide a new/different value with every response and the result should not be cached (in my opinion).

+1, I tried to push in this direction as well: #381 (comment)

I didn't realize this was GET. my suggestion has been POST. In german wallet project we have been prototyping with POST and it works well https://bmi.usercontent.opencode.de/eudi-wallet/eidas-2.0-architekturkonzept/flows/PID-IssuerSigned-cloud/#issuer-session-endpoint-at-the-pid-provider

Other than describing the media type of content that doesn't exist as JSON, which is semantically invalid and maybe syntactically wrong too, what value does the German wallet project get from using POST?

=== Screenshot of the previously cited german wallet project ===

Wouldn't a POST request be more appropriate?

No.

I mean the endpoint is supposed to provide a new/different value with every response and the result should not be cached (in my opinion).

That's why it was made not cacheable in the example https://github.com/openid/OpenID4VCI/pull/381/files#diff-1f424614b35a9899813079f1b1f6218631a2aedd993368ccb89bb81a9eda0289R741 and text https://github.com/openid/OpenID4VCI/pull/381/files#diff-1f424614b35a9899813079f1b1f6218631a2aedd993368ccb89bb81a9eda0289R734 using the Cache-Control construct that HTTP gives for control of caching.

My 2 cents for what they are worth,

If we asked the question "is this call safe (i.e. is it read-only)" I think the answer would be "maybe". Safe calls shouldn't alter the state of the server in any way but without knowing how the server implements the nonce endpoint we would not know the answer to the question.

If we asked the question "is this call idempotent" I think we would also get a "maybe". Client could do this call just to "fetch a value" which might not assume any intention of a side-effect, and from the perspective of the server it would depend on the implementation.

I do see the point in making it a GET because it is technically just fetching a random value, so semantically if fits, but there might be an embedded assumption here that this random value is a prerequisite for some other steps, and that getting a nonce can be viewed as setting up a state that needs to be validated by the server.

One could argue that it might be safer to make this a POST because this would communicate that the call might not be safe or idempotent and that subsequent calls could cause load, burden, and side-effects for the server.

But this is just philosophizing, I personally do not have a clear preference, I see arguments for both GET and POST.

I can see arguments for both as well but was really trying to avoid them entirely with a simple (maybe simplistic) approach.

but avoidance didn't work and so f7b4dc8 Removed the HTTP method specificity from the Nonce Request text and change the example from GET to POST

After reading the comments and listening to the discussions, I'm ok with GET or POST, but we need to pick one.

as discussed on the call - it's POST now 55c8388

openid-4-verifiable-credential-issuance-1_0.md

Sakurann

i did not realize this was GET, approved assuming this was POST, sorry about that...

…hange the example from GET to POST

openid-4-verifiable-credential-issuance-1_0.md

tlodderstedt

Happy to accept if Joseph's proposal is incorporated.
https://github.com/openid/OpenID4VCI/pull/381/files#r1787382536

…the client mandatory if it's present and a nonce is required" Co-authored-by: Joseph Heenan <[email protected]>

bc-pi · 2024-10-08T12:19:50Z

Happy to accept if Joseph's proposal is incorporated. https://github.com/openid/OpenID4VCI/pull/381/files#r1787382536

it's done with 9e0d059

tlodderstedt · 2024-10-08T13:12:17Z

Happy to accept if Joseph's proposal is incorporated. https://github.com/openid/OpenID4VCI/pull/381/files#r1787382536

it's done with 9e0d059

Great, thanks!

jogu · 2024-10-08T21:17:23Z

As discussed on today's WG call - we now have an astounding 12 approvals and no remaining objections/comments - merging! Thank you everyone!

remove c_nonce and c_nonce_expires_in from the token endpoint res…

ce28a10

…ponse to fix #39

tlodderstedt requested changes Aug 27, 2024

View reviewed changes

added a Nonce Endpoint where a Client can acquire a fresh c_nonce val…

074065b

…ue without the overhead of a full Credential Request

bc-pi requested a review from tlodderstedt September 4, 2024 21:07

bc-pi requested review from tplooker, Sakurann and jogu September 4, 2024 21:10

paulbastian requested changes Sep 5, 2024

View reviewed changes

mandate nonce endpoint if the issuer requires the use of c_nonce

48a3678

bc-pi requested a review from paulbastian September 6, 2024 22:57

Sakurann reviewed Sep 10, 2024

View reviewed changes

openid-4-verifiable-credential-issuance-1_0.md Show resolved Hide resolved

Sakurann reviewed Sep 10, 2024

View reviewed changes

openid-4-verifiable-credential-issuance-1_0.md Show resolved Hide resolved

Sakurann reviewed Sep 10, 2024

View reviewed changes

openid-4-verifiable-credential-issuance-1_0.md Show resolved Hide resolved

Sakurann requested changes Sep 10, 2024

View reviewed changes

improve readability via Sakurann

58bbf81

Co-authored-by: Kristina <[email protected]>

paulbastian reviewed Sep 10, 2024

View reviewed changes

openid-4-verifiable-credential-issuance-1_0.md Outdated Show resolved Hide resolved

paulbastian reviewed Sep 10, 2024

View reviewed changes

pmhsfelix reviewed Sep 12, 2024

View reviewed changes

openid-4-verifiable-credential-issuance-1_0.md Show resolved Hide resolved

openid-4-verifiable-credential-issuance-1_0.md Show resolved Hide resolved

make nonce response uncacheable

0e0d188

jogu added this to the Final 1.0 milestone Sep 16, 2024

c2bo reviewed Sep 17, 2024

View reviewed changes

openid-4-verifiable-credential-issuance-1_0.md Outdated Show resolved Hide resolved

openid-4-verifiable-credential-issuance-1_0.md Outdated Show resolved Hide resolved

big MUST to little can

aadb2bb

bc-pi requested a review from peppelinux September 24, 2024 16:59

tplooker approved these changes Sep 24, 2024

View reviewed changes

peppelinux approved these changes Sep 24, 2024

View reviewed changes

peppelinux mentioned this pull request Sep 24, 2024

remove c_nonce from token response, we have the nonce endpoint italia/eudi-wallet-it-docs#422

Open

big C

55c109e

Co-authored-by: Giuseppe De Marco <[email protected]>

paulbastian approved these changes Sep 26, 2024

View reviewed changes

tlodderstedt reviewed Sep 26, 2024

View reviewed changes

Sakurann requested changes Sep 26, 2024

View reviewed changes

Removed the HTTP method specificity from the Nonce Request text and c…

f7b4dc8

…hange the example from GET to POST

bc-pi requested review from tlodderstedt and Sakurann September 30, 2024 20:15

bc-pi commented Oct 1, 2024

View reviewed changes

openid-4-verifiable-credential-issuance-1_0.md Outdated Show resolved Hide resolved

HTTP is as HTTP does

55c8388

OR13 approved these changes Oct 1, 2024

View reviewed changes

Sakurann approved these changes Oct 3, 2024

View reviewed changes

danielfett approved these changes Oct 3, 2024

View reviewed changes

jogu mentioned this pull request Oct 4, 2024

Complete IANA Considerations section #401

Merged

javereec mentioned this pull request Oct 4, 2024

Clean Up Outdated Files in Repo #402

Merged

tlodderstedt requested changes Oct 8, 2024

View reviewed changes

joseph's suggested text for "making the use of the nonce_endpoint by …

9e0d059

…the client mandatory if it's present and a nonce is required" Co-authored-by: Joseph Heenan <[email protected]>

jogu approved these changes Oct 8, 2024

View reviewed changes

tlodderstedt approved these changes Oct 8, 2024

View reviewed changes

jogu merged commit 5bbb11a into main Oct 8, 2024
2 checks passed

bc-pi mentioned this pull request Oct 21, 2024

remove c_nonce and c_nonce_expires_in from credential endpoint response #404

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove `c_nonce` from the token endpoint response #381

remove `c_nonce` from the token endpoint response #381

bc-pi commented Aug 27, 2024 •

edited

Loading

tlodderstedt left a comment

jogu commented Aug 27, 2024

bc-pi commented Aug 29, 2024 •

edited

Loading

peppelinux commented Sep 3, 2024 •

edited

Loading

bc-pi commented Sep 4, 2024

paulbastian left a comment

bc-pi commented Sep 6, 2024

Sakurann Sep 10, 2024

bc-pi Sep 10, 2024

bc-pi Sep 10, 2024

bc-pi Sep 16, 2024

Sakurann left a comment

bc-pi commented Sep 10, 2024

paulbastian left a comment •

edited

Loading

bc-pi commented Sep 16, 2024

Sakurann commented Sep 17, 2024

paulbastian commented Sep 24, 2024

bc-pi commented Sep 24, 2024

tlodderstedt Sep 26, 2024 •

edited

Loading

paulbastian Sep 26, 2024

Sakurann Sep 26, 2024

bc-pi Sep 26, 2024 •

edited

Loading

bc-pi Sep 26, 2024 •

edited

Loading

nemqe Sep 27, 2024 •

edited

Loading

bc-pi Sep 30, 2024

bc-pi Sep 30, 2024

tlodderstedt Oct 1, 2024

bc-pi Oct 1, 2024

Sakurann left a comment

tlodderstedt left a comment

bc-pi commented Oct 8, 2024

tlodderstedt commented Oct 8, 2024

jogu commented Oct 8, 2024

	A Credential Issuer that requires `c_nonce` values to be incorporated into proofs in the Credential Request (see (#credential-request)) MUST offer a Nonce Endpoint. This endpoint allows a Client to acquire a fresh `c_nonce` value without the overhead of a full Credential Request.
	This endpoint allows a Client to acquire a fresh `c_nonce` value without the overhead of a full Credential Request. A Credential Issuer that requires `c_nonce` values to be incorporated into proofs in the Credential Request (see (#credential-request)) MUST offer a Nonce Endpoint.

remove c_nonce from the token endpoint response #381

remove c_nonce from the token endpoint response #381

Conversation

bc-pi commented Aug 27, 2024 • edited Loading

tlodderstedt left a comment

Choose a reason for hiding this comment

jogu commented Aug 27, 2024

bc-pi commented Aug 29, 2024 • edited Loading

peppelinux commented Sep 3, 2024 • edited Loading

bc-pi commented Sep 4, 2024

paulbastian left a comment

Choose a reason for hiding this comment

bc-pi commented Sep 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sakurann left a comment

Choose a reason for hiding this comment

bc-pi commented Sep 10, 2024

paulbastian left a comment • edited Loading

Choose a reason for hiding this comment

bc-pi commented Sep 16, 2024

Sakurann commented Sep 17, 2024

paulbastian commented Sep 24, 2024

bc-pi commented Sep 24, 2024

tlodderstedt Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bc-pi Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

bc-pi Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

nemqe Sep 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sakurann left a comment

Choose a reason for hiding this comment

tlodderstedt left a comment

Choose a reason for hiding this comment

bc-pi commented Oct 8, 2024

tlodderstedt commented Oct 8, 2024

jogu commented Oct 8, 2024

remove `c_nonce` from the token endpoint response #381

remove `c_nonce` from the token endpoint response #381

bc-pi commented Aug 27, 2024 •

edited

Loading

bc-pi commented Aug 29, 2024 •

edited

Loading

peppelinux commented Sep 3, 2024 •

edited

Loading

paulbastian left a comment •

edited

Loading

tlodderstedt Sep 26, 2024 •

edited

Loading

bc-pi Sep 26, 2024 •

edited

Loading

bc-pi Sep 26, 2024 •

edited

Loading

nemqe Sep 27, 2024 •

edited

Loading