Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 interoperable way for holder-asserted claims in a VP #1186
Add interoperable way for holder-asserted claims in a VP #1186
Changes from 11 commits
7490cfb
e85b745
93696db
5bf51b2
37c32cb
47a67ef
14fb906
9446c62
bef11ba
56965c2
8eca073
b4e16a6
6d943aa
8590bc1
31dc79c
3c308a4
9dec2f3
9559c36
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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 withholder
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
vsverifiable credential
will be some version of these changes... over and over again... until you go mad.... : )Basically replacing
credential
withunsecured verifiable credential
orsecured 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....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).
There was a problem hiding this comment.
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>
andVerifiableCredential
inconsistent.The reader learns that embedded or external proof is required for
<a>verifiable credential<a>
.... and optional forVerifiableCredential
... 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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed in 8590bc1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
andvalidUntil
onto the Presentation too? I think we do want to enable that. At present, it's only allowed on aVerifiableCredential
. Historically, this was because presentations were expected to always be short lived... but the short-lived-ness being determined by the verifier and thecreated/iat
date on the proof, not explicitly stated by the holder.This comment is non-blocking.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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