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

sd-jwt vc profile for oid4vc #56

Merged
merged 45 commits into from
Oct 5, 2023
Merged

sd-jwt vc profile for oid4vc #56

merged 45 commits into from
Oct 5, 2023

Conversation

tlodderstedt
Copy link
Contributor

No description provided.

@TakahikoKawasaki
Copy link

"claims" or "credentialSubject"

Either of the following changes should be made for consistency.

  1. Change the claims parameter in Credential Issuer Metadata to credentialSubject.
  2. Change the credentialSubject parameter in Authorization Details and Credential Request to claims.

Additional Claims Specified by Credential Request

  • credentialSubject: OPTIONAL. A JSON object as defined in (#authorization_vc_sd-jwt). This object determines the optional claims to be added to the credential to be issued.

If this were a functionality to allow wallets to request more claims than the set of claims implied by the type, it would mean that wallets can obtain more claims than ones authorized by the user. It should not happen.

Because a credential offer contains neither claims nor credentialSubject (which implies that a fixed set of claims is determined based on type), the purpose of the credentialSubject in a credential request should be only to narrow down the set of claims.

So, either of the following changes should be made.

  1. Rephrase the description about the credentialSubject in Credential Request.
  2. Remove the credentialSubject parameter from Credential Request.

Description of Credential Response

The value of the credential claim in the Credential Response MUST be a SD-JWT VC. Credentials of this format are already a sequence of base64url-encoded values separated by period characters and MUST NOT be re-encoded.

To be exact, the SD-JWT format contains ~ in addition to base64url characters and periods. Trying to repeat to describe technical details that are written in other specifications could cause this kind of mistakes. The description here does not have to mention details about the SD-JWT format. It would be enough to say, for example, like below.

Credentials of this format are already suitable for transfer and, therefore, they need not and MUST NOT be re-encoded.

Finally, a typo: "a SD-JWT VC" -> "an SD-JWT VC"

@peppelinux
Copy link
Member

There are some considerations made by service designers and accessibility domain experts that I would like to share to you.

these are not relevant for the scopes of this PR, so this proposal should be converted in a issue if you agree.

There are cases where a credential issuer would offer periodic updates about its issued credentials, cases where the issuer should communicate the recent news related to the use of the credential to the end user and owner of that credential

this could be implemented as web uri, the wallet instance periodically gets updates by hitting this refs.
The content type could be application/json if html could not be handly

@tlodderstedt
Copy link
Contributor Author

"claims" or "credentialSubject"

Either of the following changes should be made for consistency.

  1. Change the claims parameter in Credential Issuer Metadata to credentialSubject.
  2. Change the credentialSubject parameter in Authorization Details and Credential Request to claims.

changed it to claims - credentialSubject is not used by SD-JWT VCs.

Additional Claims Specified by Credential Request

  • credentialSubject: OPTIONAL. A JSON object as defined in (#authorization_vc_sd-jwt). This object determines the optional claims to be added to the credential to be issued.

If this were a functionality to allow wallets to request more claims than the set of claims implied by the type, it would mean that wallets can obtain more claims than ones authorized by the user. It should not happen.

The set of claims MUST be a sub set of the authorized claims.

Because a credential offer contains neither claims nor credentialSubject (which implies that a fixed set of claims is determined based on type), the purpose of the credentialSubject in a credential request should be only to narrow down the set of claims.

I agree. The question is whether it would make sense to add claims to the credential offer so the issuer could tell the wallet what optional claims are available. WDYT?

So, either of the following changes should be made.

  1. Rephrase the description about the credentialSubject in Credential Request.
  2. Remove the credentialSubject parameter from Credential Request.

Description of Credential Response

The value of the credential claim in the Credential Response MUST be a SD-JWT VC. Credentials of this format are already a sequence of base64url-encoded values separated by period characters and MUST NOT be re-encoded.

To be exact, the SD-JWT format contains ~ in addition to base64url characters and periods. Trying to repeat to describe technical details that are written in other specifications could cause this kind of mistakes. The description here does not have to mention details about the SD-JWT format. It would be enough to say, for example, like below.

Credentials of this format are already suitable for transfer and, therefore, they need not and MUST NOT be re-encoded.

I incorporated the text you proposed.

Finally, a typo: "a SD-JWT VC" -> "an SD-JWT VC"

fixed

@tlodderstedt
Copy link
Contributor Author

There are some considerations made by service designers and accessibility domain experts that I would like to share to you.

these are not relevant for the scopes of this PR, so this proposal should be converted in a issue if you agree.

There are cases where a credential issuer would offer periodic updates about its issued credentials, cases where the issuer should communicate the recent news related to the use of the credential to the end user and owner of that credential

this could be implemented as web uri, the wallet instance periodically gets updates by hitting this refs. The content type could be application/json if html could not be handly

as you already noted, this topic is out of scope of this PR. please file an issue on that topic

draft-oid4vc-haip-sd-jwt-vc.md Outdated Show resolved Hide resolved
Copy link
Contributor

@Sakurann Sakurann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a parameter in issuer metadata is missing..?

draft-oid4vc-haip-sd-jwt-vc.md Outdated Show resolved Hide resolved
draft-oid4vc-haip-sd-jwt-vc.md Outdated Show resolved Hide resolved
draft-oid4vc-haip-sd-jwt-vc.md Show resolved Hide resolved
examples/credential_metadata_jwt_vc_json.json Outdated Show resolved Hide resolved
@Sakurann
Copy link
Contributor

Sakurann commented Aug 1, 2023

Also why is this approach better than adding new parameters in each of the sub sections in section 4 that already defines extensions for OpenID4VCI? With the current structure, the implementer needs to look into two places to see parameters for the Credential Offer for example. If the entire profile is about SD-JWT VC, it should be in one place, no..?

@tlodderstedt
Copy link
Contributor Author

Need to add identifier for presentation request.

Copy link
Contributor

@Sakurann Sakurann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you!

Copy link
Member

@selfissued selfissued left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tlodderstedt tlodderstedt merged commit 31d589c into main Oct 5, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants