w3c / vc-data-model

W3C Verifiable Credentials v2.0 Specification
https://w3c.github.io/vc-data-model/
Other
293 stars 106 forks source link

Default context contains terms outside of the spec #1314

Closed mtaimela closed 10 months ago

mtaimela commented 1 year ago

Hi all.

Currently the https://www.w3.org/ns/credentials/v2 context contains terms regarding the "JWT standard claims" and "SD-JWT claims". These are actually not defined by the VCDM2.0 specification, so I think these should not belong into VCDM2.0 default context.

Could it be possible to remove these, and ask the other specifications to include VCDM2.0 standard claims into their own context definitions. It is not sustainable to include all possible terms from all possible containers into VCDM2.0 default context.

@OR13 I think you might have a strong opinions here, but from objects relation view point the current implementation is inverted. SD-JWT "has-a" VCDM2.0, current context implementation says VCDM2.0 "is-a" SD-JWT and JWT from context mapping.

TL;DR;

  1. VCDM2.0 default context should focus into own specification boundaries, remove excess.
  2. SD-JWT should define their own terms, and include VCDM2.0 context
  3. JWT should define their own terms, and include VCDM2.0 context

This way, the relations are correct, and lifecycles are with-in the specs.

brentzundel commented 1 year ago

The result of long conversations within the VCWG on this topic, and the closest we can get to a strong consensus opinion, is that the VCDM v2.0 context file will include terms that will ease the implementation burden for those using it. This means that the VCDM v2.0 context file includes many terms that are not defined in VCDM v2.0, even beyond those you've outlined above. This is a deliberate choice and I do not believe re-opening this conversation will lead to a different outcome.

It may be helpful to review the conversations on this topic recorded here

mtaimela commented 1 year ago

Hi @brentzundel,

thank you for the historical information. Embedding other specifications vocabs into VCDM2.0 context is clearly violating the architectural boundaries. These boundaries have different lifecycle, thus non-backward compatible changes or conflicts in naming, will propagate up to the dependency tree and cause a new version in the VCDM2.0 context that will destroy all existing VCs.

I fully understand the "let's be nice and accommodate everything and save space", but this approach is clearly wrong from the boundaries, dependencies, and their lifecycle point of view.

I still would like to keep this issue. One option is to remove the "required default context" and say that the VCDM2.0 @vocab "must be included in the chosen context (embedded), or in the VC as composite context (new entry in @context)". This way the DIP, JWT and SD-JWT can embed the version of their choice, while having full control of their own terms. Dependencies are correct (all depends on VCDM2.0 vocab), and specs lifecycle does not interfere with the main spec.

OR13 commented 1 year ago

I'd be ok moving all securing format specific details out of the core context, that was my original proposal.

The compromise was to include all the securing format specific terms in the core context, thats why it's the way it is now.

dlongley commented 1 year ago

The main driver here is the presence of @vocab in the core context, which results in erroneously defined terms when people forget to include other contexts.

An option would be to shift that burden onto only those that want to use @vocab instead of all VC v2 consumers. If @vocab were removed from the core context and instead the current default @vocab was provided in a utility context (that could be used by devs running experiments or issuers that don't want to define or reuse other contexts), that would enable the decoupling sought here. This has been proposed before, but there was not consensus to do it.

OR13 commented 1 year ago

These are controlled by W3C:

 "@vocab": "https://www.w3.org/ns/credentials/issuer-dependent#",

^ This one just says, if it's not defined in the spec (the v2 core context), don't throw an error, and give it a definition in the context of W3C Verifiable Credentials via the v2 core context.

"refreshService": {
  "@id": "https://www.w3.org/2018/credentials#refreshService",
  "@type": "@id"
},

^ this is defined in the spec.

There are not controlled by W3C:

 "mediaType": {
      "@id": "https://schema.org/encodingFormat"
    },

^ this is commented on normatively in the spec.

"proof": {
  "@id": "https://w3id.org/security#proof",
  "@type": "@id",
  "@container": "@graph"
},

^ this is commented on normatively in the spec.

dlongley commented 1 year ago
 "@vocab": "https://www.w3.org/ns/credentials/issuer-dependent#",

^ This one just says, if it's not defined in the spec (the v2 core context), don't throw an error, and give it a definition in the context of W3C Verifiable Credentials via the v2 core context.

Yes, this is precisely the problem. Something that should generate an error will not (for people who have made a mistake). It could also be the case that someone intended to use @vocab -- and no error should be thrown in that case. We could have that person signal that by including a context that uses @vocab. Instead, by forcing a default @vocab in the core context, these two cases are conflated... creating a problem. So to help mitigate this, at least for common security terms, we remove the possibility of making the (non-thrown) errors by defining those terms in the core context.

OR13 commented 1 year ago

@dlongley I think we can retain @vocab and still remove proof and DataIntegrityProof terms from the core context.

The difference becomes that the RDF terms won't match unless a data integrity context is added... but that can still be normatively required by data integrity proofs, for example:

 "@context": [
    "https://www.w3.org/ns/credentials/v2",
    "https://www.w3.org/ns/credentials/examples/v2",
    "https://w3id.org/security/data-integrity/v2" // not controlled by w3c
  ],

See also: https://w3c.github.io/vc-data-integrity/#example-a-simple-signed-json-ld-data-document

dlongley commented 1 year ago

I think we can retain @vocab and still remove proof and DataIntegrityProof terms from the core context.

I disagree that we should try to solve this with what I think would amount to a half measure. I think we have to choose to remove @vocab and have a clean separation -- or bundle things together to avoid the situation I've described above and that has been discussed in a number of other issues over the lifetime of the working group. The problem isn't limited to just one particular set of bundled terms; all bundled terms are affected and could be mismapped to the wrong things because of @vocab. Solving this in one way for one set of VCWG-mapped terms and another for a different set will just lead to more confusion, errors, and / or frustration, IMO.

It would be better to isolate the possible problems to those people who want to use @vocab, as they should be best prepared for them.

So, we could create separate contexts for every different domain-specific set of terms (such as the JWT ones that this issue requests removal of) and remove @vocab and put it into its own context to resolve this. This establishes a clear pattern for usage. Libraries could catch unmapped terms and domain-specific specs could require term mappings or specific contexts to be present and, if they were not present, errors could be thrown properly.

Instead of what you recommend above, we could do this:

 "@context": [
    "https://www.w3.org/ns/credentials/v2",
    "https://<status list>", // optional
    "https://www.w3.org/ns/credentials/issuer-dependent/v1", // optional
    "https://w3id.org/security/data-integrity/v2" and / or "<https://some JWT / VC-JOSE-COSE context>"
  ],

This would move @vocab into https://www.w3.org/ns/credentials/issuer-dependent/v1 for those people who wish to use it. Then, if any contexts were forgotten, VC libraries could generate errors that would signal to people to add the missing context(s). As it stands, there is no signal, just silent mistakes -- but at least we prevent all mistakes related to terms mapped by the VCWG (across various specs), meaning only issuer / shared-credential-vocab specific mistakes can be made.

AFlowOfCode commented 12 months ago

After switching to credentials/v2, using contexts to strictly limit claims to terms defined in the referenced vocabularies ceased being useful, as anything can now be considered (artificially) defined. I have not see any mention of "issuer dependent" or @vocab in VCDM2.0 so this was unexpected.

Removing only that line from a locally cached version of the context file easily sidesteps this issue of course. It makes me think that it would be useful to be able to have a standard way of excluding items by creating a strict subset of a context. Extending a context is simple, but I'm unaware of a method of reducing one aside from simply replacing it with a different file.

Anyway, for my purposes, a separately included https://www.w3.org/ns/credentials/issuer-dependent/v1 context would be an optimal solution. My experience of switching to v2 was that useful functionality was eliminated without the change being mentioned anywhere obvious and expected such as the specification itself. I think v2 also adds much improvement, and changes are to be expected of course. But the idea of changes like this not being transparently documented anywhere outside of working group minutes or Github issues does make the implementation of new versions seem slightly more unpredictable.

Just my 2 cents as someone working closely with VCs. Thanks for your time and effort.

OR13 commented 10 months ago

I suggest closing this issue, there is unlikely to be consensus to move securing mechanism specific claims out of the v2 context.

mtaimela commented 10 months ago

closing as consensus was not found

iherman commented 10 months ago

The issue was discussed in a meeting on 2023-12-06

View the transcript #### 2.2. Default context contains terms outside of the spec (issue vc-data-model#1314) _See github issue [vc-data-model#1314](https://github.com/w3c/vc-data-model/issues/1314)._ **Brent Zundel:** Do we have a sense for whether this issue has been addressed? **Orie Steele:** I don't think we were able to establish consensus to resolve this previously, so I suggest it be closed. **Sebastian Crane:** I'm not familiar with this issue, but even if we don't have consensus to resolve the core essence of what's being asked. We perhaps have a responsibility to document this within the spec anyway, these issues might be experienced by multiple people until they understand the reasoning for it. **Manu Sporny:** We could add some text into the specification explaining how and why the v2 data model includes more helpful properties. **Sebastian Crane:** Ok, I'll raise an issue to add some text.