vc-data-model icon indicating copy to clipboard operation
vc-data-model copied to clipboard

feat: add name and description

Open tplooker opened this issue 3 years ago • 26 comments

Many use-cases for verifiable credentials require rendering them in some form of user experience, in these instances having a standard way to represent a name and description for a credential helps.

This PR does the following

  1. Adds two new properties to the core data model to support these expressions
  2. Defines a v2 of the context defining the new properties
  3. Shifts the v2 context to not use compact iri's

Outstanding questions

  1. Should the name and description fields have a maximum length?
  2. Should we define other properties that help with rendering a credential such as an image for a credential? If so what constraints would we apply to this property?

Preview | Diff

tplooker avatar Nov 02 '20 01:11 tplooker

Should the name and description fields have a maximum length?

Probably not, we've avoided specifying definitive lengths for human readable strings. The assumption here is that we're not dealing w/ space-constrained devices... and if folks do want to put limits, they'll enforce that via JSON Schema or API constraints.

Should we define other properties that help with rendering a credential such as an image for a credential? If so what constraints would we apply to this property?

I've been meaning to write back on that specific topic. There is a non-trivial tracking/privacy danger with images and any sort of URL that's requested to render a VC. That's one of the driver's of hashlinks -- the ability to cache based on a content-addressed identifier.

Digital Bazaar has also explored using iframes, sandboxed content, SVGs, and other approaches to provide richer experiences. We are fairly convinced that a name, description, and image will not work at scale given the combination of issuer requirements, privacy requirements, and credential size requirements. This is a fairly rich area and we should be careful about what we do in the space.

msporny avatar Nov 02 '20 03:11 msporny

This is good, but it's unclear what changes need to be made to accept it.

OR13 avatar Nov 02 '20 13:11 OR13

I'd prefer to see us separate out the HTML changes where we're defining the terms from the update to the v2 context. I've got a few other changes as well that I'd like to see go into v2 context as well, but would rather see us agree on them all together so that we can make a WG decision to address issue #755 and add a stable version of that context.

kdenhartog avatar Nov 19 '20 21:11 kdenhartog

Since the purpose of this PR is just to add two new properties to the core data model, should we either

  1. Just simplify this PR to address the spec changes and not update/create a new context
  2. Create a new context as a copy of v1 with the new properties added (what this PR is currently doing)
  3. Create a new context that tackles other changes we want to see in v2 (e.g refactoring how we handle security related terms)

@kdenhartog I believe your request is suggesting we do 1?

tplooker avatar Nov 19 '20 23:11 tplooker

Since the purpose of this PR is just to add two new properties to the core data model, should we either

1. Just simplify this PR to address the spec changes and not update (create a new context)

2. Create a new context as a copy of v1 with the new properties added (what this PR is currently doing)

3. Create a new context that tackles other changes we want to see in v2 (e.g refactoring how we handle security related terms)

@kdenhartog I believe your request is suggesting we do 1?

Yup for this PR just do option 1 in my opinion, and we'll start getting a second context defined so that we can present it to the WG and get approval to address #755 (which I'm not certain if we can do that right now without changes in charter or anything like that because it's a normative change)

kdenhartog avatar Nov 19 '20 23:11 kdenhartog

@msporny @peacekeeper @OR13 thoughts on the above ^?

tplooker avatar Nov 20 '20 00:11 tplooker

I suggest making this PR spec text changes only.

OR13 avatar Nov 23 '20 22:11 OR13

@OR13 changes to the context have been reverted out of this PR

tplooker avatar Nov 25 '20 09:11 tplooker

@OR13 changes to the context have been reverted out of this PR

Awesome thanks! I'll take care of the V2 VC Context. I've already got a partially working one that I've been playing around with to make everything work together better.

kdenhartog avatar Nov 25 '20 23:11 kdenhartog

ping @msporny @burnburn @iherman what are the next action items w.r.t this PR?

tplooker avatar Dec 02 '20 00:12 tplooker

ping @msporny @burnburn @iherman what are the next action items w.r.t this PR?

There are several steps here

  1. I do not know what the "worfklow" was in the previous VC WG for the acceptance of PR-s (I was not part of that WG). There should be some sort of WG consensus to merge this, because it is an addition to the spec. Some sort of an email vote, for example.
  2. My comment on another PR partially stands. It is not an erratum, i.e., that part of my comment is irrelevant. But my note whereby there must be a change section in the final document, where this is duly recorded, stands.
  3. At some point in the future, when there are enough changes, we may want to re-issue a Rec. But that is down the line for now.

iherman avatar Dec 02 '20 09:12 iherman

This is the first proposed change to the VC spec under the maintenance charter, so I'm uncertain about how to move this forward.

I recommend giving this some air time at a future (hopefully next) CCG meeting to discuss. Ideally we'd have a quorum of @msporny, @burnburn, and @iherman to help with this.

fyi: @wyc and @vsnt

Created https://github.com/w3c-ccg/community/issues/170 with "review next" label to get air time

kimdhamilton avatar Dec 04 '20 03:12 kimdhamilton

@msporny addressed your comments, however the outstanding question is we will have to modify the normative statement around @context processing see here

The value of the @context property MUST be an ordered set where the first item is a URI with the value https://www.w3.org/2018/credentials/v1. For reference, a copy of the base context is provided in Appendix § B. Base Context. Subsequent items in the array MUST express context information and be composed of any combination of URIs or objects. It is RECOMMENDED that each URI in the @context be one which, if dereferenced, results in a document containing machine-readable information about the @context.

tplooker avatar Feb 19 '21 23:02 tplooker

@msporny addressed your comments, however the outstanding question is we will have to modify the normative statement around @context processing see here

Well, yes, we'll have to change that. That's what makes this whole PR a substantive change and potentially not a "maintenance work item". This is a v1.1 (or v2) specification change we're discussing... implementations will have to change, the planets will have to align and all that. Since we're going to that trouble anyway, we might as well do things like add the 2020 cryptosuites, deprecate the 2018 cryptosuites, etc. :)

Food for thought... but this PR needs to change that normative statement as well.

The other decision we may want to make is on versioning... do we want the context to be ../credentials/v1.1 or ../credentials/v2? This is definitely a breaking change from previous versions, so major revision in semver... but do we really want to issue a Verifiable Credentials v2.0 specification"? I'm leaning toward the latter... major breaking change, multiple cryptosuites affected, v2, let's just commit to it now.

msporny avatar Feb 20 '21 15:02 msporny

I have cleaned up all old stale branches and setup the current branch structure for the repo:

  • main - this is where changes to the next non-breaking release goes (bug fixes, errata, etc) -- the next target release is v1.1
  • v2 - this is where major breaking changes/additions go (new terms, contexts, features, etc.) -- the next target release is v2.0

This PR has been marked as a v2 feature and the target branch has been changed to v2. This enables the current repo to be used for both v1.1 and v2 work.

NOTE: None of this has any immediate effect on anything -- it's merely a bookkeeping measure for the Editors until the VC Maintenance WG can meet and decide if this structure is appropriate.

msporny avatar Feb 20 '21 16:02 msporny

@brentzundel just mentioned this PR in #248. This feature may have the potential to close #248. The idea there is to put data in a VC for the purpose of showing it to the user, e.g., as she browses through the VCs. It's a 'pet name' as @ChristopherA calls it. This implies that, while it seems a good idea to have an issuer supply the initial name and description, but enable the holder to change it so that it can fit in the holders world (i.e. the 'namespace' that every person manages for itself). I haven't thought over any implications, but it might seem reasonable to either have the issuer not sign these attributes (so the holder can change them without affecting the real contents), or to provide ways in which properties can be added to a VC by the holder regardless of the VC.

RieksJ avatar Jul 27 '22 05:07 RieksJ

either have the issuer not sign these attributes (so the holder can change them without affecting the real contents), or to provide ways in which properties can be added to a VC by the holder regardless of the VC.

Interesting topic. I think it definitely makes sense to be able to associate unsigned (meta-)data with VCs such as petnames, but not sure which approach is best. This feels like a broader non-VC-specific question that maybe Linked Data experts can answer. Looking at the Data Integrity and RDF Dataset Canonicalization specifications, my best guess is that the unsigned (meta-)data would have to somehow go into a separate RDF graph (container) than the rest of the data.

In any case, these are probably two separate issues. I believe this issue here is originally about signed name+description rather than unsigned petnames.

peacekeeper avatar Jul 27 '22 08:07 peacekeeper

Putting on my former linked-data-expert-hat :-)

Interesting topic. I think it definitely makes sense to be able to associate unsigned (meta-)data with VCs such as petnames, but not sure which approach is best. This feels like a broader non-VC-specific question that maybe Linked Data experts can answer. […], my best guess is that the unsigned (meta-)data would have to somehow go into a separate RDF graph (container) than the rest of the data.

That would work, but it means that the metadata portion should be very clearly separated from the rest. Ie,

{
    "id" : "https://example.org/id",
    "pet_name" : "Bobby",
    "pet_type" : "Golden Retriever" 
}

would not really work; one should, rather, say something like:

{
    "id" : "https://example.org/id",
    "metadata" : {
        "pet_name" : "Bobby",
        "pet_type" : "Golden Retriever" 
    }
}

and fiddle around the @context file to make it clear that the object of metadata is a separate RDF graph. Actually, regardless of the RDF serialization aspect, I think it is a good idea and engineering practice to clearly separate the metadata from the rest of the data (this may affect the current PR which does not seem to do that!). This would be analogous to the <meta> tag of, say, HTML…

(How exactly the Data Integrity spec and the RDF Dataset Canonicalization spec will specify that, although we have a collection of RDF Datasets, some of the constituent datasets should not be signed is, I believe, doable but not done at this moment. Something for the RCH WG?)

In any case, these are probably two separate issues. I believe this issue here is originally about signed name+description rather than unsigned petnames.

Maybe so, I do not know whether this was so clearly stated. If that is not the case, ie, these name+descriptions are not to be signed, then we would have to clearly separate them from the rest, imho, even in the current PR.

iherman avatar Jul 27 '22 10:07 iherman

I haven't thought over any implications, but it might seem reasonable to either have the issuer not sign these attributes

There are dangerous implications here. One of the biggest flaws we could introduce at this point is "unsigned data in the VC" -- at present, I'm a very strong -1 to that because it introduces the possibility that a developer would process data from within a package that they thought was signed, that wasn't signed.

This "how to display things to the individual" is a "digital wallet" problem, not something we need to address at the VC layer. If the individual wants to give their credentials pet names, the wallet software can provide that mechanism, and even create an exportable version of that (as a VC, so you know who set the petnames).

That said, this is a separate topic. @RieksJ, you should raise an issue if you'd like to pursue the idea/concept in the VCWG.

msporny avatar Jul 27 '22 13:07 msporny

@msporny mentions "it introduces the possibility that a developer would process data from within a package that they thought was signed, that wasn't signed". If it is a matter of choice between requiring developers to do a proper job (for which they are hired) as opposed to set requirements for holders (with various (dis)abilities), I'd go for the first.

The issue of 'pet names' is indeed a "how to display things to the individual", but not exclusively a "digital wallet" problem. It is also for the user, discussions over which were started as issue #248. So no need to raise yet another issue.

I can live that. And for @brentzundel: that would imply that #248 is NOT addressed as part of this PR.

RieksJ avatar Jul 27 '22 13:07 RieksJ

The issue was discussed in a meeting on 2022-07-27

  • no resolutions were taken
View the transcript

5.3. Add a summary text as meta information (issue vc-data-model#248)

See github issue vc-data-model#248.

Brent Zundel: This has received some back and forth alongside another PR.
… The proposal here is that we add some summary text/meta info that would allow the issuer or the holder to indicate "this is what this credential is about".
… A 'summary property' could be useful.
… I suggested that PR752 addressed this, but it seems not.

Manu Sporny: +1 to PR that tplooker raised as addressing this..

Brent Zundel: There some folks on the call who raised comments....

Manu Sporny: My understanding of Tobias's proposal is to add name and description.

See github pull request vc-data-model#752.

Manu Sporny: I'd say the description is the summary.
… Do we need name, description and summary?.
… There are some preliminary proposals for rendering.
… if we can't describe the difference between summary and description then go with tplooker.

Ted Thibodeau Jr.: It seems the me the issuer will have no idea what pet name the holder may use.
… The pet name might go into an envelope that contains the credential but that's not within the VC.

Manu Sporny: -1 to petnames in VCs... associated metadata, fine, but that's a Holder preference..

Ivan Herman: That was a bit of a discussion on the pull request, not the issue.
… Are these additional metadata things to be signed/normalised?.

Manu Sporny: -1 for "bag of metadata" :) -- like, we have a mechanism for semantics... let's use those..

Ivan Herman: That's relevant because one way is to define a bunch of properties. The other way is to say there is a single metadata property that points to a bunch of stuff that is not part of the credential.
… Personally I'd prefer to separate the metadata from the core set of statements, but we have to decide on that.

Tobias Looker: yeah I agree with manu.

Ivan Herman: If you don't want to sign it then it's separate.

Brent Zundel: No time for all comments here. Please add your comments to the issue (not the PR).

Tobias Looker: The original proposal is to add name and description. I see description and summary as synonyms..
… The other clarification - it is proof format-specific, but it's issuer-signed.
… I don't think that stops a wallet also assigning a pet name.

Kristina Yasuda: Thanks everyone.

Manu Sporny: +1 to what tplooker just said..

Kristina Yasuda: We're running out of time, so DavidC, JoeAndrieu oliver please comment on the issue..
… Sorry we're over time.


iherman avatar Jul 27 '22 16:07 iherman

I haven't thought over any implications, but it might seem reasonable to either have the issuer not sign these attributes

There are dangerous implications here. One of the biggest flaws we could introduce at this point is "unsigned data in the VC" -- at present, I'm a very strong -1 to that because it introduces the possibility that a developer would process data from within a package that they thought was signed, that wasn't signed.

I agree that it is dangerous to mix signed and unsigned data for this use case. I don't see any reason why metadata can't sit outside the VC when it is just for the holder to manage their credentials.

logpo avatar Aug 03 '22 04:08 logpo

don't see any reason why metadata can't sit outside the VC when it is just for the holder to manage their credentials.

Yes, agreed. There will be metadata that systems that manage VCs will associate with those VCs... petnames being one of the potentials there. That metadata does not have to go in the VC, rather, it can live "beside" the VC in the system that manages the VCs.

We should probably put something to this effect in the implementation guide.

msporny avatar Aug 03 '22 14:08 msporny

I think there is still a case for a standard place to put a human readable name and description, but it should be signed with the rest of the VC. And if the holder want's to assign a different name after signing then that should be stored next to the VC and not in it

logpo avatar Aug 03 '22 22:08 logpo

I think there is still a case for a standard place to put a human readable name and description

Sure, similar to the standard VISA or MC or AMEX logo on their plastic, which the human carrier may tag in various ways to help them pull the employer-expense-card, or the triple-miles-bonus, etc. -- none of which human tags are actually part of the physical card nor stored in the data bearing mag-strip.

TallTed avatar Aug 05 '22 16:08 TallTed

Raised PR https://github.com/w3c/vc-data-model/pull/905 to implement the JSON-LD Context portion of what this PR was going for. Once we have agreement on the v2 context, we can adjust the main specification to update all of the examples to include names and descriptions for VCs.

msporny avatar Aug 14 '22 20:08 msporny

name and description have been added to the v2 context in PR #905. Closing.

msporny avatar Aug 24 '22 20:08 msporny