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 all 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.