Wallet notifying the Issuer of (un)successful issuance of credential(s) #70

Sakurann · 2023-09-07T05:42:40Z

Resolves #32. migrates PR608 from bitbucket.

need to agree how the wallet specifies about which credential it is talking

proof might not work because there is no requirement for the proofs to be unique across credentials..
using format/type structure might work when combined with the identifiers in PR add identifiers for issuing credential of the same type, different content #65

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

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

Sakurann · 2023-09-28T18:30:27Z

two options on the table discussed during the call:

callback includes full credentials that the wallet successfully accepted. feels weird sending PII over the wire extra time - what is callback_endpoint is compromised...
credential response returns an identifier for this purpose (an identifier before credential is issued (PR add identifiers for issuing credential of the same type, different content #65) is different from an identifier after credential is issued - cc @pmhsfelix) , which the wallet can include in the callback.

Sakurann · 2023-09-28T18:31:29Z

personally, I like option 2 much more. it adds (yet another) identifier, but still feels better than sending an entire credential...

bc-pi · 2023-09-28T21:17:23Z

personally, I like option 2 much more. it adds (yet another) identifier, but still feels better than sending an entire credential...

I like option 2 too. As long as confusion about identifiers can be avoided.

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

Sakurann · 2023-09-29T02:28:39Z

thinking more about it, I think there is a confusion which state we are talking about. I was talking about issuer_state that the issuer passes in the credential_offer, not the state that the wallet sends in the authorization request.

There is no guarantee that the issuer_state generated by the credential Issuer is known by the AS, so the issuer_state passed in the credential offer by the credential issuer should be passed not in the authorization request, but the credential request going to the credential issuer.

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

…et-callback

Sakurann · 2023-09-29T05:58:18Z

to solve how to identify the status of each credential when more than one credential was issued, I added an option to return callback_id from credential endpoint and batch credential endpoint, which wallet needs to put in the callback. so callback depends on the issuer returning a callback_id, which should be fine.

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

This reverts commit 0f50200.

…into wallet-callback

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

peppelinux · 2023-12-21T08:46:18Z

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

@@ -1191,29 +1193,115 @@ Cache-Control: no-store
 }
 ```

+# Notification Endpoint {#notification_endpoint}
+
+This endpoint is used by the Wallet to notify the Credential Issuer of certain events for issued credentials. These events enable the Credential Issuer to take subsequent actions after issuance. The Credential Issuer needs to return the `notification_id` in the Credential Response or a Batch Credential Response for the Wallet to be able to use this Endpoint. Support for this endpoint is OPTIONAL.


Suggested change

This endpoint is used by the Wallet to notify the Credential Issuer of certain events for issued credentials. These events enable the Credential Issuer to take subsequent actions after issuance. The Credential Issuer needs to return the `notification_id` in the Credential Response or a Batch Credential Response for the Wallet to be able to use this Endpoint. Support for this endpoint is OPTIONAL.

This endpoint is used by the Wallet to notify the Credential Issuer of certain events for issued Credentials. These events enable the Credential Issuer to take subsequent actions after issuance. The Credential Issuer needs to return the `notification_id` in the Credential Response or a Batch Credential Response for the Wallet to be able to use this Endpoint. Support for this endpoint is OPTIONAL.

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

peppelinux

Few editorials still to be applied, overall approved. Thx!

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

jogu · 2023-12-21T11:53:12Z

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

+  * `invalid_notification_id`: The `notification_id` in the Notification Request was invalid.
+  * `invalid_notification_request`: The Notification Request is missing a required parameter, includes an unsupported parameter or parameter value, repeats the same parameter, or is otherwise malformed.
+
+It is at the discretion of the Wallet whether to retry the request or not.


We're now saying this in two different places (line 1206 as well). We should only say it in one place.

I do not think we should encourage or even allow the wallet to retry for a permanent failure, for example I don't think invalid_notification_id or invalid_notification_request should ever be retried.

tlodderstedt

looks good now, I just requested two small changes

tlodderstedt · 2023-12-21T14:01:05Z

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

@@ -1191,29 +1193,115 @@ Cache-Control: no-store
 }
 ```

+# Notification Endpoint {#notification_endpoint}
+
+This endpoint is used by the Wallet to notify the Credential Issuer of certain events for issued credentials. These events enable the Credential Issuer to take subsequent actions after issuance. The Credential Issuer needs to return the `notification_id` in the Credential Response or a Batch Credential Response for the Wallet to be able to use this Endpoint. Support for this endpoint is OPTIONAL.


Suggested change

This endpoint is used by the Wallet to notify the Credential Issuer of certain events for issued credentials. These events enable the Credential Issuer to take subsequent actions after issuance. The Credential Issuer needs to return the `notification_id` in the Credential Response or a Batch Credential Response for the Wallet to be able to use this Endpoint. Support for this endpoint is OPTIONAL.

This endpoint is used by the Wallet to notify the Credential Issuer of certain events for issued credentials. These events enable the Credential Issuer to take subsequent actions after issuance. The Credential Issuer needs to return one or more notification ids in a Credential Response or a Batch Credential Response for the Wallet to be able to use this Endpoint. Every notification id is related to exactly one issued Credential. Support for this endpoint is OPTIONAL.

Doesn't this belong in the definition of "notification_id" itself?

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

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

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

tlodderstedt

lgtm - please approve my editorial suggestion before merging

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

migrate PR608 from bitbucket

2d05938

Sakurann requested review from tlodderstedt and tplooker September 7, 2023 05:42

Sakurann commented Sep 27, 2023

View reviewed changes

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

Sakurann requested review from jogu, awoie and bc-pi September 27, 2023 23:40

peppelinux requested changes Sep 28, 2023

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

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

peppelinux requested changes Sep 28, 2023

View reviewed changes

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

Apply suggestions from code review

dd001bf

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

Sakurann requested a review from peppelinux September 28, 2023 18:24

peppelinux requested changes Sep 28, 2023

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

Kristina and others added 5 commits September 28, 2023 20:07

modify issuer_state

0f50200

Apply suggestions from Giuseppe's code review

f4159a5

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

Apply suggestions from Giuseppe's code review

789280f

add callback_id

ec74329

pull from remote

c412351

Sakurann requested a review from peppelinux September 29, 2023 05:21

Kristina added 2 commits September 28, 2023 22:56

align batch to credential endpoint

f8199af

Merge branch 'main' of https://github.com/openid/OpenID4VCI into wall…

2e49036

…et-callback

Sakurann commented Sep 29, 2023

View reviewed changes

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

Sakurann and others added 2 commits September 28, 2023 23:03

Apply suggestions from code review

a9b395a

Revert "modify issuer_state"

28ef6d5

This reverts commit 0f50200.

Sakurann requested a review from tplooker December 21, 2023 00:59

Apply suggestions from code review

50ec2b4

tplooker approved these changes Dec 21, 2023

View reviewed changes

Sakurann and others added 4 commits December 20, 2023 17:29

Merge branch 'main' into wallet-callback

6943429

add document history

458ab77

Merge branch 'wallet-callback' of https://github.com/openid/OpenID4VCI …

43f9cfc

…into wallet-callback

fix typo to make file compile

4d95136

peppelinux requested changes Dec 21, 2023

View reviewed changes

peppelinux approved these changes Dec 21, 2023

View reviewed changes

Sakurann added this to the ID-1 milestone Dec 21, 2023

jogu requested changes Dec 21, 2023

View reviewed changes

tlodderstedt requested changes Dec 21, 2023

View reviewed changes

Sakurann commented Dec 21, 2023

View reviewed changes

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

Sakurann commented Dec 21, 2023

View reviewed changes

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

Sakurann commented Dec 21, 2023

View reviewed changes

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

Apply suggestions from code review

f3a0d7b

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

Sakurann commented Dec 21, 2023

View reviewed changes

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

Sakurann commented Dec 21, 2023

View reviewed changes

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

Sakurann commented Dec 21, 2023

View reviewed changes

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

Apply suggestions from code review

1ffd4d9

David-Chadwick self-requested a review December 21, 2023 16:16

networking error can cause request not received

33a451c

Sakurann requested a review from tlodderstedt December 21, 2023 17:09

tlodderstedt approved these changes Dec 22, 2023

View reviewed changes

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

clarify this endpoint is optional for the wallet

fe4892d

Sakurann requested a review from jogu December 22, 2023 22:58

Merge branch 'main' into wallet-callback

b9caf63

jogu approved these changes Dec 22, 2023

View reviewed changes

Merge branch 'main' into wallet-callback

3085621

Sakurann merged commit 7210c6b into main Dec 22, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wallet notifying the Issuer of (un)successful issuance of credential(s) #70

Wallet notifying the Issuer of (un)successful issuance of credential(s) #70

Sakurann commented Sep 7, 2023 •

edited

Loading

Sakurann commented Sep 28, 2023 •

edited

Loading

Sakurann commented Sep 28, 2023

bc-pi commented Sep 28, 2023

Sakurann commented Sep 29, 2023 •

edited

Loading

Sakurann commented Sep 29, 2023 •

edited

Loading

peppelinux Dec 21, 2023

peppelinux left a comment

jogu Dec 21, 2023

tlodderstedt left a comment

tlodderstedt Dec 21, 2023

Sakurann Dec 21, 2023

tlodderstedt left a comment

	This endpoint is used by the Wallet to notify the Credential Issuer of certain events for issued credentials. These events enable the Credential Issuer to take subsequent actions after issuance. The Credential Issuer needs to return the `notification_id` in the Credential Response or a Batch Credential Response for the Wallet to be able to use this Endpoint. Support for this endpoint is OPTIONAL.
	This endpoint is used by the Wallet to notify the Credential Issuer of certain events for issued Credentials. These events enable the Credential Issuer to take subsequent actions after issuance. The Credential Issuer needs to return the `notification_id` in the Credential Response or a Batch Credential Response for the Wallet to be able to use this Endpoint. Support for this endpoint is OPTIONAL.

Wallet notifying the Issuer of (un)successful issuance of credential(s) #70

Wallet notifying the Issuer of (un)successful issuance of credential(s) #70

Conversation

Sakurann commented Sep 7, 2023 • edited Loading

Sakurann commented Sep 28, 2023 • edited Loading

Sakurann commented Sep 28, 2023

bc-pi commented Sep 28, 2023

Sakurann commented Sep 29, 2023 • edited Loading

Sakurann commented Sep 29, 2023 • edited Loading

peppelinux Dec 21, 2023

Choose a reason for hiding this comment

peppelinux left a comment

Choose a reason for hiding this comment

jogu Dec 21, 2023

Choose a reason for hiding this comment

tlodderstedt left a comment

Choose a reason for hiding this comment

tlodderstedt Dec 21, 2023

Choose a reason for hiding this comment

Sakurann Dec 21, 2023

Choose a reason for hiding this comment

tlodderstedt left a comment

Choose a reason for hiding this comment

Sakurann commented Sep 7, 2023 •

edited

Loading

Sakurann commented Sep 28, 2023 •

edited

Loading

Sakurann commented Sep 29, 2023 •

edited

Loading

Sakurann commented Sep 29, 2023 •

edited

Loading