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.

Sign up for GitHub

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

Jump to bottom

Add interoperable way for holder-asserted claims in a VP #1186

Merged
merged 18 commits into from
Jul 23, 2023
Merged

Add interoperable way for holder-asserted claims in a VP #1186

Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
7490cfb
add section on holder-derived creds in VP
brentzundel Jul 5, 2023
e85b745
Apply suggestions from code review
brentzundel Jul 10, 2023
93696db
Update index.html
brentzundel Jul 10, 2023
5bf51b2
soften requirement for 'type', clarify two methods for VC.
brentzundel Jul 10, 2023
37c32cb
Apply suggestions from code review
brentzundel Jul 10, 2023
47a67ef
Update index.html
brentzundel Jul 10, 2023
14fb906
add paragraph clarifying subjects of self-asserted VCs
brentzundel Jul 12, 2023
9446c62
Apply suggestions from code review
brentzundel Jul 14, 2023
bef11ba
re-phrase normative considerations, add example
brentzundel Jul 14, 2023
56965c2
Merge branch 'issue-860' of github.com:brentzundel/vc-data-model into…
brentzundel Jul 14, 2023
8eca073
Update title for holder-asserted example.
msporny Jul 14, 2023
b4e16a6
remove controversial recommended 'type'
brentzundel Jul 17, 2023
6d943aa
Update index.html
brentzundel Jul 17, 2023
8590bc1
change example title
brentzundel Jul 17, 2023
31dc79c
Merge branch 'issue-860' of github.com:brentzundel/vc-data-model into…
brentzundel Jul 17, 2023
3c308a4
Apply suggestions from code review
brentzundel Jul 17, 2023
9dec2f3
Apply suggestions from code review
brentzundel Jul 17, 2023
9559c36
modify example 2 with Joe's language
brentzundel Jul 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 38 additions & 14 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2059,29 +2059,24 @@ <h4>Presentations Including Holder Claims</h4>
<a>property</a>.
</p>
<p>
A self-asserted <a>verifiable credential</a> that has been included in the value
of the <code>verifiableCredential</code> <a>property</a> of a
<a>verifiable presentation</a> MUST include the following properties:
All of the normative requirements defined for <a>verifiable credentials</a>
apply to self-asserted <a>verifiable credentials</a>. Some additional
considerations follow:
</p>

<dl>
<dt><var>type</var></dt>
<dd>
The <code>type</code> <a>property</a> is required. One value MUST be
<code>VerifiableCredential</code>, and it is RECOMMENDED that another
value be <code>SelfAssertedCredential</code>. Additional types MAY also
be included. For example,<br>
It is RECOMMENDED that <code>SelfAssertedCredential</code> be included as an
Copy link
Member

Choose a reason for hiding this comment

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

I agree with @selfissued and @dlongley that this is not necessary. You can just check to see if the key used to sign a VC belongs to the holder. For Data Integrity, you check to see if the proof.verificationMethod is associated with holder using the algorithm defined in https://w3c.github.io/vc-data-integrity/#retrieve-verification-method. I expect VC COSE JOSE would define the same sort of mapping. IOW, just because someone asserts this, doesn't mean it's true, you need to verify it... and if only a subset of the ecosystem is doing it, you can't really count on it as a dependable mechansim.

This feedback is non-blocking, just thinking out loud.

Copy link
Contributor

Choose a reason for hiding this comment

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

I expect VC COSE JOSE would define the same sort of mapping. IOW, just because someone asserts this, doesn't mean it's true, you need to verify it... and if only a subset of the ecosystem is doing it, you can't really count on it as a dependable mechanism.

This describes "Verifiable Presentations" as a whole... And I agree... its not very helpful.

We seem to have to do a lot of work for "holder" and "VerifiablePresentation"... when they are really just special cases of "issuer" and "VerifiableCredential"....

I continue to assert there is a better solution to this, but I don't think we can turn this boat at this point.

additional <code>type</code> value. For example,<br>
<code>"type": ["VerifiableCredential", "SelfAssertedCredential", "ExampleTypeCredential"]</code>
</dd>
<dt><var>issuer</var></dt>
<dd>
The <code>issuer</code> <a>property</a> is required. For self-asserted
<a>verifiable credentials</a> that are secured using the same mechanism as the
<a>verifiable presentation</a>, the value MUST be identical to the
<code>holder</code> <a>property</a> of the <a>verifiable presentation</a>.
Self-asserted <a>verifiable credentials</a> that use their own securing
mechanism MUST follow the normative guidance for the <code>issuer</code>
<a>property</a> in Section <a href="#issuer"></a>.
For self-asserted <a>verifiable credentials</a> that are secured using the same
mechanism as the <a>verifiable presentation</a>, the value of the
<code>issuer</code> <a>property</a> MUST be identical to the <code>holder</code>
<a>property</a> of the <a>verifiable presentation</a>.
brentzundel marked this conversation as resolved.
Show resolved Hide resolved
</dd>
</dl>
<p>
Expand All @@ -2108,6 +2103,35 @@ <h4>Presentations Including Holder Claims</h4>
{ <span class="comment">...</span> }
}],
"proof": [{ <span class="comment">...</span> }]</span>
OR13 marked this conversation as resolved.
Show resolved Hide resolved
}
</pre>
<p>
The example below shows a <a>verifiable presentation</a> that embeds a
self-asserted <a>verifiable credential</a> that holds <a>claims</a> about the
<a>verifiable presentation</a>. It is secured using the same mechanism as the
<a>verifiable presentation</a>.
</p>

<pre class="example nohighlight" title="Verifiable presentation with a self-asserted verifiable credential about the verifiable presentation">
Copy link
Contributor

@OR13 OR13 Jul 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
<pre class="example nohighlight" title="Verifiable presentation with a self-asserted verifiable credential about the verifiable presentation">
<pre class="example nohighlight" title="A secured verifiable presentation, relying on an embedded proof, including a self-asserted verifiable credential about the verifiable presentation">

Copy link
Contributor

Choose a reason for hiding this comment

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

for consistency with https://github.com/w3c/vc-data-model/pull/1186/files#r1264100438

Copy link
Contributor

Choose a reason for hiding this comment

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

@msporny as an aside, I suspect your PR to fix credential vs verifiable credential will be some version of these changes... over and over again... until you go mad.... : )

Basically replacing credential with unsecured verifiable credential or secured verifiable credential depending on the context... looking forward to that one : )

Maybe we can be clever and do that once in terms, and not have to be so verbose everywhere....

Copy link
Member

@msporny msporny Jul 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

until you go mad....

That happened long ago... during v1.0... :)

I'll see if we can be clever wrt. terminology, as you mention above... but at this point, I'm leaning toward NOT making the change (as it sounds like it's going to be a lot of trouble, and result in a confusing outcome for readers).

Copy link
Contributor

@OR13 OR13 Jul 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I don't see how we can avoid fixing it, given what it currently normatively states:

It makes the interpretation of <a>verifiable credential<a> and VerifiableCredential inconsistent.

The reader learns that embedded or external proof is required for <a>verifiable credential<a>.... and optional for VerifiableCredential... Seems like we will have to fix that... The same applies to presentations... This PR is digging the hole deeper.... but sometimes a tunnel is necessary to escape.

Copy link
Member Author

Choose a reason for hiding this comment

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

changed in 8590bc1

{
"@context": [
"https://www.w3.org/ns/credentials/v2",
"https://www.w3.org/ns/credentials/examples/v2"
],
"type": ["VerifiablePresentation", "ExamplePresentation"],
<span class="highlight">"id": "http://example.com/presentation/5223"</span>,
brentzundel marked this conversation as resolved.
Show resolved Hide resolved
"holder": "did:example:12345678",
"verifiableCredential": [{
"@context": "https://www.w3.org/ns/credentials/v2",
"type": ["VerifiableCredential", "ExampleTimeLimitCredential"],
"issuer": "did:example:12345678",
"credentialSubject": {
<span class="highlight">"id": "http://example.com/presentation/5223"</span>,
brentzundel marked this conversation as resolved.
Show resolved Hide resolved
"validUntil": "2024-01-01T19:23:24Z"
Copy link
Member

@msporny msporny Jul 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
"validUntil": "2024-01-01T19:23:24Z"
"validUntil": "2024-01-01T19:23:24Z"

Huh... this is valid, and proposes the question: Why not just put validUntil on the presentation itself? We might want to have an example of someone putting a self-asserted claim on the VP itself... possibly in another PR, we don't want to complicate this one further.

IOW, do we want to enable validFrom and validUntil onto the Presentation too? I think we do want to enable that. At present, it's only allowed on a VerifiableCredential. Historically, this was because presentations were expected to always be short lived... but the short-lived-ness being determined by the verifier and the created/iat date on the proof, not explicitly stated by the holder.

This comment is non-blocking.

Copy link
Member Author

Choose a reason for hiding this comment

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

@msporny I think you're probably right, but I don't want that conversation to derail this PR. I think a better example is probably be what's needed here.

This example came out of my attempt to address @jandrieu 's desire for a normative way for a holder to make claims about the VP. As I constructed the self-asserted VC, I had a hard time coming up with a claim the holder would want to make about the VP.
This is just to say that there is probably a better example of the sort of claims a holder would want to make about the VP, but I couldn't think one up. Looking to @jandrieu for suggestions.

Copy link
Member

@msporny msporny Jul 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Agreed, we don't want to derail this PR with trying to find the perfect example. I defer to @jandrieu to suggest a use case. I'll try to think of a use case (with markup) as well (for a future PR).

Copy link
Member Author

Choose a reason for hiding this comment

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

I've updated the example based on Joe's comment here: https://github.com/w3c/vc-data-model/pull/1186/files#r1268305835

},
{ <span class="comment">...</span> }
}],
"proof": [{ <span class="comment">...</span> }]</span>
}
</pre>
</section>
Expand Down