clarify credential error response can be authorization or credential request error #64

Sakurann · 2023-09-01T22:34:33Z

addresses issue #40.
Moved from Bitbucket: https://bitbucket.org/openid/connect/pull-requests/590

I wonder if batch credential endpoint needs additional error codes...

…request error

bc-pi

few remaining nits

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

thank you! Co-authored-by: Brian Campbell <71398439+bc-pi@users.noreply.github.com>

Co-authored-by: Brian Campbell <71398439+bc-pi@users.noreply.github.com>

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

pmhsfelix · 2023-09-28T14:50:02Z

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

-The following additional error codes are specified:
+* `error`: REQUIRED. A key at the top level of a JSON object, the value of which SHOULD be a single ASCII [@!USASCII] error code from the following:
+  * `invalid_credential_request`: The Credential Request is missing a required parameter, includes an unsupported parameter or parameter value, repeats the same parameter, or is otherwise malformed.
+  * `unsupported_credential_type`: Requested credential type is not supported.


type is specific to the *_vc_* (W3C) formats, right? So, probably it shouldn't be in these format-independent error values.

A more general error could be unsupported_credential. This could be used to target non format related credential errors of this klnd.

type is not specific to W3C formats :) it is used to refer to a claimset to be included in a credential. the definition of doctype in ISO 18013-5 is docType is the requested document type. and we use the following definition of doctype in VCI too

doctype: REQUIRED. JSON string identifying the credential type as defined in [ISO.18013-5].

so I think the current text is ok

That's a slippery slope as the spec treats any credential type information as format specific. In addition, W3C has an array of type values. I assume the issuer would like to indicate which of them is unsupported.

how should your comment be interpreted? are you agreeing with Jacob and Pedro that a generic use of a term type in the spec should be revisited? i do not disagree but that should be a separate issue.

I agree with Jacob and Pedro that types are format specific. Whether we go for the more generic error code (as sugested by Jacob) or use format specific error codes depends on whether what wallets can reasonably process. If wallets anyway treat it generically, I would be in favor of the generic error code.

I agree with @jacobideskog .

jacobideskog

My only concern is the same as @pmhsfelix 's to name the unsupported_credential_type value after a W3C specific item.

jacobideskog · 2023-09-28T18:31:02Z

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

-The following additional error codes are specified:
+* `error`: REQUIRED. A key at the top level of a JSON object, the value of which SHOULD be a single ASCII [@!USASCII] error code from the following:
+  * `invalid_credential_request`: The Credential Request is missing a required parameter, includes an unsupported parameter or parameter value, repeats the same parameter, or is otherwise malformed.
+  * `unsupported_credential_type`: Requested credential type is not supported.


A more general error could be unsupported_credential. This could be used to target non format related credential errors of this klnd.

Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>

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

Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>

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

bc-pi

authorization error response can't mandate the auth-scheme value "Bearer

Co-authored-by: Brian Campbell <71398439+bc-pi@users.noreply.github.com>

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

tlodderstedt · 2023-10-07T09:28:51Z

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

+If the Credential Request does not contain an Access Token that enables issuance of a requested credential, the Credential Endpoint returns an authorization error response such as defined in section 3 of [@!RFC6750].

- Credential Request was malformed. One or more of the parameters (i.e., `format`, `proof`) are missing or malformed.
+For the errors specific to the Credential Request such as those caused by `type`, `format`, `proof`, or encryption parameters in the request, the error codes values defined in (#credential-request-errors) SHOULD be used instead of a generic `invalid_request` parameter defined in section 3.1 of [@!RFC6750].


I find this paragraph underneath #### Authorization Errors {#authorization-errors} misleading. Suggest to remove the section heading and just have two paragraphs or to move the second section into the credential issuance errors section.

I think it would be more misleading to have two separate paragraphs talking about errors for credential endpoint. we can change section titles (credential error response; authorization errors; credential request errors) if they are misleading, but I really do not think we need to change the structure.

Ok. Let's try the current text.

discussed in the WG call, agreed to move line 889 under 891.

tlodderstedt · 2023-10-07T09:32:05Z

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

-The following additional error codes are specified:
+* `error`: REQUIRED. A key at the top level of a JSON object, the value of which SHOULD be a single ASCII [@!USASCII] error code from the following:
+  * `invalid_credential_request`: The Credential Request is missing a required parameter, includes an unsupported parameter or parameter value, repeats the same parameter, or is otherwise malformed.
+  * `unsupported_credential_type`: Requested credential type is not supported.


That's a slippery slope as the spec treats any credential type information as format specific. In addition, W3C has an array of type values. I assume the issuer would like to indicate which of them is unsupported.

Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>

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

Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>

…ential-error-response

Sakurann · 2023-10-26T20:18:08Z

@tlodderstedt we restructured as you suggested, please re-review. the WG was not clear on your second comment regarding "type"

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

tlodderstedt · 2023-10-18T07:33:52Z

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

-The following additional error codes are specified:
+* `error`: REQUIRED. A key at the top level of a JSON object, the value of which SHOULD be a single ASCII [@!USASCII] error code from the following:
+  * `invalid_credential_request`: The Credential Request is missing a required parameter, includes an unsupported parameter or parameter value, repeats the same parameter, or is otherwise malformed.
+  * `unsupported_credential_type`: Requested credential type is not supported.


I agree with Jacob and Pedro that types are format specific. Whether we go for the more generic error code (as sugested by Jacob) or use format specific error codes depends on whether what wallets can reasonably process. If wallets anyway treat it generically, I would be in favor of the generic error code.

tlodderstedt · 2023-10-25T18:59:01Z

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

-The following additional error codes are specified:
+* `error`: REQUIRED. A key at the top level of a JSON object, the value of which SHOULD be a single ASCII [@!USASCII] error code from the following:
+  * `invalid_credential_request`: The Credential Request is missing a required parameter, includes an unsupported parameter or parameter value, repeats the same parameter, or is otherwise malformed.
+  * `unsupported_credential_type`: Requested credential type is not supported.


I agree with @jacobideskog .

tlodderstedt · 2023-10-25T19:01:17Z

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

+If the Credential Request does not contain an Access Token that enables issuance of a requested credential, the Credential Endpoint returns an authorization error response such as defined in section 3 of [@!RFC6750].

- Credential Request was malformed. One or more of the parameters (i.e., `format`, `proof`) are missing or malformed.
+For the errors specific to the Credential Request such as those caused by `type`, `format`, `proof`, or encryption parameters in the request, the error codes values defined in (#credential-request-errors) SHOULD be used instead of a generic `invalid_request` parameter defined in section 3.1 of [@!RFC6750].


Ok. Let's try the current text.

Sakurann · 2023-10-31T17:39:13Z

agreed with torsten to open an issue to rename unsupported_credential_type to unsupported_credential and merge this PR for now.

Sakurann · 2023-10-31T17:44:42Z

6 approvals. open for more than a week. no objection to merge during the WG call.

clarify credential error response can be authorization or credential …

32a4b4a

…request error

Sakurann requested review from bc-pi, jogu, tlodderstedt and tplooker September 1, 2023 22:34

bc-pi requested changes Sep 6, 2023

View reviewed changes

Sakurann and others added 2 commits September 6, 2023 21:54

Apply suggestions from Brian's code review

ba7193e

thank you! Co-authored-by: Brian Campbell <71398439+bc-pi@users.noreply.github.com>

Apply suggestions from Brian's code review

5ceb8e5

Co-authored-by: Brian Campbell <71398439+bc-pi@users.noreply.github.com>

Sakurann requested a review from bc-pi September 7, 2023 15:28

bc-pi approved these changes Sep 7, 2023

View reviewed changes

Sakurann requested a review from pmhsfelix September 28, 2023 14:46

peppelinux requested changes Sep 28, 2023

View reviewed changes

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

pmhsfelix reviewed Sep 28, 2023

View reviewed changes

jacobideskog reviewed Sep 28, 2023

View reviewed changes

Apply suggestions from Giuseppe's code review

bcf5dc8

Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>

Sakurann requested review from jacobideskog, peppelinux and pmhsfelix October 2, 2023 17:28

jogu approved these changes Oct 2, 2023

View reviewed changes

Sakurann mentioned this pull request Oct 4, 2023

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

Merged

Sakurann added the priority label Oct 4, 2023

peppelinux requested changes Oct 4, 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

Clean-up based on the suggestions from Giuseppes's code review

6ecf1a5

Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>

Sakurann requested a review from peppelinux October 5, 2023 06:01

bc-pi reviewed Oct 5, 2023

View reviewed changes

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

bc-pi requested changes Oct 5, 2023

View reviewed changes

removing a MUST in authz error response

659b724

Co-authored-by: Brian Campbell <71398439+bc-pi@users.noreply.github.com>

Sakurann commented Oct 5, 2023

View reviewed changes

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

Sakurann commented Oct 5, 2023

View reviewed changes

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

fixing another MUST

a9386fd

pmhsfelix approved these changes Oct 5, 2023

View reviewed changes

peppelinux approved these changes Oct 5, 2023

View reviewed changes

Sakurann requested a review from bc-pi October 5, 2023 18:02

bc-pi approved these changes Oct 5, 2023

View reviewed changes

tlodderstedt requested changes Oct 7, 2023

View reviewed changes

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

e37c82c

Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>

peppelinux requested changes Oct 26, 2023

View reviewed changes

peppelinux approved these changes Oct 26, 2023

View reviewed changes

c2bo approved these changes Oct 26, 2023

View reviewed changes

Sakurann and others added 3 commits October 26, 2023 13:14

capitalize Credential

64905ed

Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>

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

cbb854b

…ential-error-response

restructure per torsten's comment

8da4b54

Sakurann requested a review from tlodderstedt October 26, 2023 20:17

Sakurann commented Oct 31, 2023

View reviewed changes

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

clarify credential request errors

ed0e98f

tlodderstedt approved these changes Oct 31, 2023

View reviewed changes

Sakurann merged commit af2ee50 into main Oct 31, 2023

Sakurann deleted the credential-error-response branch October 31, 2023 17:42

clarify credential error response can be authorization or credential request error #64

clarify credential error response can be authorization or credential request error #64

Uh oh!

Conversation

Sakurann commented Sep 1, 2023

Uh oh!

bc-pi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jacobideskog left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bc-pi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Sakurann commented Oct 26, 2023

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sakurann commented Oct 31, 2023

Uh oh!

Sakurann commented Oct 31, 2023

Uh oh!

Reviewers