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

Add SD-JWT VC Profile #115

Merged
merged 19 commits into from
Apr 7, 2024
Merged

Add SD-JWT VC Profile #115

merged 19 commits into from
Apr 7, 2024

Conversation

javereec
Copy link
Contributor

PR to resolve #74

Copy link
Contributor

@awoie awoie left a comment

Choose a reason for hiding this comment

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

Direction generally looks good but imo, the matcher algorithm is missing. You cannot apply JSONPath using PE to the SD-JWT (i.e., issuer-signed JWT plus disclosures). But the wallet can apply the matcher using the JSONPath to the "Unsecured Payload of an SD-JWT VC" (see https://www.ietf.org/archive/id/draft-ietf-oauth-sd-jwt-vc-03.html#section-1.4). After this was added, I'm fine approving it.

You actually can apply a JSONPath to the SD-JWT VC but it would not give you the expected result since it contains all the _sd claims and not the actual disclosures in one JSON object.

@Sakurann
Copy link
Collaborator

please also review this PR in HAIP that removes sd-jwt vc profile from HAIP openid/oid4vc-haip-sd-jwt-vc#96

@Sakurann
Copy link
Collaborator

But the wallet can apply the matcher using the JSONPath to the "Unsecured Payload of an SD-JWT VC"

it might be beneficial to adopt for sd-jwt vc a similar processing requirements as in 18013-7 for mdocs: ""path is a JSON array where each entry is a JSON String containing a requested data element from the requested document type as follows: $['']['']. For example, to request a data element with an data element identifier family_name from the namespace org.iso.18013.5.1, the following JSON String is used: $['org.iso.18013.5.1']['family_name']." cc @bc-pi @danielfett

@awoie
Copy link
Contributor

awoie commented Mar 21, 2024

But the wallet can apply the matcher using the JSONPath to the "Unsecured Payload of an SD-JWT VC"

it might be beneficial to adopt for sd-jwt vc a similar processing requirements as in 18013-7 for mdocs: ""path is a JSON array where each entry is a JSON String containing a requested data element from the requested document type as follows: [″][″].Forexample,torequestadataelementwithandataelementidentifierfamilynamefromthenamespaceorg.iso.18013.5.1,thefollowingJSONStringisused:['org.iso.18013.5.1']['family_name']." cc @bc-pi @danielfett

That would work too. Or just say that the root of the JSONPath is the Unsecured SD-JWT VC Payload. I'm ok with both options as long it is defined.

@javereec
Copy link
Contributor Author

@awoie @Sakurann I like the clarity that the unsecured payload brings in terms of finding the disclosures to be added during presentation. I have made some changes, let me know what you think.

@danielfett
Copy link
Contributor

But the wallet can apply the matcher using the JSONPath to the "Unsecured Payload of an SD-JWT VC"

it might be beneficial to adopt for sd-jwt vc a similar processing requirements as in 18013-7 for mdocs: ""path is a JSON array where each entry is a JSON String containing a requested data element from the requested document type as follows: $[''][''].
For example, to request a data element with an data element identifier family_name from the namespace org.iso.18013.5.1, the following JSON String is used: $['org.iso.18013.5.1']['family_name']." cc @bc-pi @danielfett

That definition is quite confusing and took me a while to process (part of which is caused by the use of "JSON" here).
If I understood correctly, the expectation is that the JSONPath matching happens on a structure like the following:

{
"org.iso.18013.5.1": {"family_name": "Doe", "given_name": "John"},
"domestic_namespace": {"foo": "bar"}
}

Is that right?

How would that fit SD-JWT VC?

examples/credentials/sd_jwt_vc_unsecured.json Show resolved Hide resolved
openid-4-verifiable-presentations-1_0.md Outdated Show resolved Hide resolved
openid-4-verifiable-presentations-1_0.md Outdated Show resolved Hide resolved
@bc-pi
Copy link
Member

bc-pi commented Apr 2, 2024

There are a few places in the Introduction, Terminology, and Overview sections that have text along the lines of this:

Verifiable Credentials and Verifiable Presentations can be of any format, including, but not limited to W3C Verifiable Credentials Data Model [VC_DATA], ISO mdoc [ISO.18013-5], and AnonCreds [Hyperledger.Indy].

which should probably be updated to also mention SD-JWT VC with this PR that adds a profile for it.

@Sakurann
Copy link
Collaborator

Sakurann commented Apr 2, 2024

@bc-pi can that be a separate PR..?

@jogu
Copy link
Collaborator

jogu commented Apr 2, 2024

There are a few places in the Introduction, Terminology, and Overview sections that have text along the lines of this:

Verifiable Credentials and Verifiable Presentations can be of any format, including, but not limited to W3C Verifiable Credentials Data Model [VC_DATA], ISO mdoc [ISO.18013-5], and AnonCreds [Hyperledger.Indy].

which should probably be updated to also mention SD-JWT VC with this PR that adds a profile for it.

As per Kristina's comment opened an issue here to track this: #143

Copy link
Contributor

@paulbastian paulbastian left a comment

Choose a reason for hiding this comment

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

Approved when my remaining two suggestions are considered

Copy link

@tplooker tplooker left a comment

Choose a reason for hiding this comment

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

Approved on the basis we resolve the duplication of supported algorithms as described in #136 separately.

openid-4-verifiable-presentations-1_0.md Outdated Show resolved Hide resolved
openid-4-verifiable-presentations-1_0.md Outdated Show resolved Hide resolved
openid-4-verifiable-presentations-1_0.md Outdated Show resolved Hide resolved
openid-4-verifiable-presentations-1_0.md Outdated Show resolved Hide resolved
openid-4-verifiable-presentations-1_0.md Outdated Show resolved Hide resolved
Co-authored-by: Oliver Terbu <[email protected]>
javereec and others added 2 commits April 4, 2024 14:03
Co-authored-by: Oliver Terbu <[email protected]>
@Sakurann Sakurann requested a review from awoie April 4, 2024 19:12
Copy link
Contributor

@awoie awoie left a comment

Choose a reason for hiding this comment

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

Thanks

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.

Add SD-JWT VC Profile
9 participants