w3c / vc-jose-cose

Verifiable Credentials Working Group — VC JSON Web Tokens specification
https://w3c.github.io/vc-jose-cose/
Other
31 stars 9 forks source link

Add OWF attribution and explain !sd #270

Closed selfissued closed 1 month ago

selfissued commented 2 months ago

Fixes #269

Cc @danielfett


Preview | Diff

iherman commented 2 months ago

I guess the changes are, from a content point of view, fine.

That being said, I find it a strange choice to create an example that way. The role of the spec includes serving as a document that makes it possible for newcomers to understand what the technical content is. Examples play an essential role in this. Nowhere else in the whole family of specification do we use YAML, and even nontrivial aspects thereof, which is odd. If it is necessary from the SD-JWT point of view, then be it. If it is just because we use a particular tool to generate the example, I question the wisdom of the choice. I would prefer, if necessary, to manually make the example valid in JSON for readability's sake.

This is not a formal objection to the PR, just food for thoughts.

bc-pi commented 2 months ago

If it is necessary from the SD-JWT point of view

It is absolutely not necessary and as mentioned in https://github.com/w3c/vc-jose-cose/issues/269#issuecomment-2094979662 I think it's inappropriate for use directly in a specification (for reasons you mentioned among others). I'd note that the SD-JWT draft itself uses the tooling to produce (hopefully useful) examples like this one but does not include any YAML in the specification itself. But if the WG and editors of vc-jose-cose think it's okay, I'm not going to spend any more energy on the topic.

decentralgabe commented 2 months ago

thanks for the reference @bc-pi - I think us following what the SD-JWT spec itself does makes sense these are just examples, and should be broadly useful/readable. if we're failing to do that, or if there are other concerns (like IPR raised earlier) then it seems like we should change tacks.

bc-pi commented 2 months ago

[...] following what the SD-JWT spec itself does makes sense [...]

Pulling on that thread a little bit, I'll note that the SD-JWT draft has an example that tried to envision how SD-JWT might be used to express/secure a VCDM payload and a presentation of such with a subset of that payload disclosed. That's at:

https://www.ietf.org/archive/id/draft-ietf-oauth-selective-disclosure-jwt-08.html#appendix-A.4

There may be some opportunity to borrow content/concepts from that and/or unveil issues in either document.

The presentation part in particular is rather different than what's in vc-xose's Securing JSON-LD Verifiable Presentations with SD-JWT. And, to be honest, securing a VP with SD-JWT doesn't really make sense to me. But maybe I'm not seeing the grand vision or something.

decentralgabe commented 2 months ago

This should be closed in favor of #272

bc-pi commented 2 months ago

This should be closed in favor of #272

That PR does take the!sd YAML out but it introduces more issues than I'm able to identify or articulate. Maybe it'd be better to just go with the current content of this PR and defer more significant changes to https://github.com/w3c/vc-jose-cose/issues/264. And also defer addressing other things like the the entire concept of securing a VP with SD-JWT being nonsensical and/or other issues that maybe/hopefully will be surfaced by doing that work.

bc-pi commented 1 month ago

This should be closed in favor of #272

That PR does take the!sd YAML out but it introduces more issues than I'm able to identify or articulate. Maybe it'd be better to just go with the current content of this PR and defer more significant changes to #264. And also defer addressing other things like the the entire concept of securing a VP with SD-JWT being nonsensical and/or other issues that maybe/hopefully will be surfaced by doing that work.

273 is different but does not fundamentally change the position stated.

decentralgabe commented 1 month ago

superseded by #273