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 17 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.
I noticed that we could have a situation where the VC uses an
issuer
property that expresses an object with the issuer's identifier in theid
property (typically to express other properties about the issuer) -- and a VP uses aholder
property that directly expresses the identifier of the holder. To account for this, I think we should indicate that it's the identifiers that must match and just reference those sections for how they could be expressed: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.
@jandrieu ^ this was the intention of citing the normative definitions related to graph objects, in the "holder" validation section PR, can you review here as well?
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.
Seems like we can perhaps remove that text from validation, if this section repeats it.
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 think that the language already present is simpler and is sufficient.
The holder is the entity constructing both the VP and the self-asserted VC, so they can make sure those values are identical, whether they choose to use an object with an
id
or just a URL.Plus, we already have this line above, which was introduced specifically to avoid pointing back to each normative section of the spec that also applies: "All of the normative requirements defined for verifiable credentials apply to self-asserted verifiable credentials."
I would prefer to not make this change.
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'm not going to block this PR if the change isn't made either.
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.
Interesting.
This doesn't say anything about how the holder makes a VC about the current presentation.
I was expecting something like
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.
as an aside, presentations and credentials don't require
id
, but when one is present, the claims are bound to theid
.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.
IMO this suggestion should be applied, we can clean up when the dust settles.
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