w3c / vc-json-schema

A mechanism to use JSON Schemas with Verifiable Credentials
https://w3c.github.io/vc-json-schema
Other
26 stars 8 forks source link

Pre-CR normative dependencies will trigger a 2nd CR #210

Closed msporny closed 11 months ago

msporny commented 1 year ago

The latest version of vc-json-schema seems to be taking a bunch of normative dependencies on specifications where it doesn't need to take normative dependencies:

https://www.w3.org/TR/2023/WD-vc-json-schema-20230903/

Namely, the following:

I know the spec tries to support 3 different versions of JSON Schema... so there might not be a way to get around this... but it exposes the spec to a bunch of "Exactly which feature in which spec do you need?" -- and if the answer is "all of them" -- then the next question becomes: "Ok, are /all/ of those features stable" -- and I don't know how we can answer "Yes".

Do we really need a normative dependency on OAS?

Why don't we inherit this dependency via the JSON Schema spec... do we really have to list it again?

It's not clear how SD-JWT would be utilized with this specification... and there are no normative statements in the specification related to SD-JWT. It's also not clear how one would use this spec w/ a selective disclosure scheme of any kind -- I expect guidance would have to be given -- do you allow SD?, do you not allow it?, what happens if you can mix/match SD?, what are the security and privacy implications of doing so? and so on... it might be best to stay silent on the topic, otherwise, I'd expect an almost automatic requirement to go through another CR as I don't know how we'd say "you can use this w/ SD" and then not say how you should use it w/ SD.

Don't we get this dependency via VCDM v2.0? If you're trying to support VCDM v1.x, you'll have to define relatedResource and digestSRI (and digestMultibase).

Same comments as SD-JWT above ... suggest you keep securing mechanisms out as normative dependencies as the VCDM v2.0 spec takes care of that (to a degree).

Is supporting v1.1 a goal? If so, you're going to have to include all the v2.0 things you want to take advantage of in the JSON-LD Context for vc-json-schema, or you won't be able to use it w/ v1.1.

Same commentary as the above... you might just want to depend on VCDM v2.0 to provide guidance on securing mechanisms for VCs and leave it at at that to reduce normative dependencies. If you don't do that, you might need to perpetually refer to new securing mechanisms in this spec.

Hrm, if this is supported, then there is no way to express it in the JsonSchemaCredential... seems like an issue; you cannot secure a YAML schema?

iherman commented 1 year ago

The issue was discussed in a meeting on 2023-09-05

View the transcript #### 2.1. Pre-CR normative dependencies will trigger a 2nd CR (issue vc-json-schema#210) _See github issue [vc-json-schema#210](https://github.com/w3c/vc-json-schema/issues/210)._ **Kristina Yasuda:** we will bring this up at tomorrows WG call.
OR13 commented 1 year ago

We write a lot of JSON Schema... I prefer to write them in YAML for the following reasons:

  1. They are more readable.
  2. YAML supports comments
  3. YAML can embed JSON directly, or as strings.
  4. YAML has tags which can unlock advanced capabilities for example !sd tag in sd-jwt tests.

IMO, we should be commenting on the supported media types, and allow for media types that are well used.

At a minimum:

decentralgabe commented 1 year ago

Thanks @msporny for the detailed review...

Numbered points for ease of responding

(1) I know the spec tries to support 3 different versions of JSON Schema... so there might not be a way to get around this... but it exposes the spec to a bunch of "Exactly which feature in which spec do you need?" -- and if the answer is "all of them" -- then the next question becomes: "Ok, are /all/ of those features stable" -- and I don't know how we can answer "Yes".

All of them, and yes they're stable, we've linked to the most stable references there are. We've resolved this concern here (please see all linked issues and discussion).

When there's a more stable JSON Schema we will move to it.

(2) Do we really need a normative dependency on OAS?

We do, because they've standardized on how to use JSON Schema with YAML. @OR13 to correct me if I'm wrong.

(3) Why don't we inherit this dependency via the JSON Schema spec... do we really have to list it again?

How do we inherit? Do you mean just remove the reference entirely? Or point to json schema and say "as defined in..."?

(4) It's not clear how SD-JWT would be utilized with this specification... and there are no normative statements in the specification related to SD-JWT. It's also not clear how one would use this spec w/ a selective disclosure scheme of any kind -- I expect guidance would have to be given -- do you allow SD?, do you not allow it?, what happens if you can mix/match SD?, what are the security and privacy implications of doing so? and so on... it might be best to stay silent on the topic, otherwise, I'd expect an almost automatic requirement to go through another CR as I don't know how we'd say "you can use this w/ SD" and then not say how you should use it w/ SD.

As Orie mentioned, it's just for enabling a media type to use this spec with SD JWT. If we need a new media type for using data integrity, then we'll need to be able to normatively reference that too.

For selective disclosure more broadly, we have this issue to address which I aim to take this week https://github.com/w3c/vc-json-schema/issues/202

(5) Don't we get this dependency via VCDM v2.0? If you're trying to support VCDM v1.x, you'll have to define relatedResource and digestSRI (and digestMultibase).

(For SRI). Yes we do. And no desire to support VCDM v1.x, just 2.x. I can answer this the same as the asnwer to (2) above.

(6) Same comments as SD-JWT above ... suggest you keep securing mechanisms out as normative dependencies as the VCDM v2.0 spec takes care of that (to a degree).

Same as (5) and (2), agree.

(7) Is supporting v1.1 a goal? If so, you're going to have to include all the v2.0 things you want to take advantage of in the JSON-LD Context for vc-json-schema, or you won't be able to use it w/ v1.1.

No this is a mistake, and I'll remove.

(Skipped 8)

(9) Hrm, if this is supported, then there is no way to express it in the JsonSchemaCredential... seems like an issue; you cannot secure a YAML schema?

This is the choice we've gone with, since no one has expressed interest in securing YAML based JSON Schemas. Is it an issue to not support securing a YAML schema?

decentralgabe commented 1 year ago

@msporny please take a look at https://github.com/w3c/vc-json-schema/pull/212

It should address at least (2), (5), (6), and (7).

decentralgabe commented 12 months ago

pending close...will wait 7 days and/or until I hear back from Manu. #212 has been merged

msporny commented 12 months ago

Looking at the PR, the sections that refer to a number of the media types don't use normative RFC language to say "you MUST" do X or you "MUST" do Y. For all of these normative dependencies, we would expect language that says something like this:

"When transmitting a JSON Schema file expressed as JSON over HTTP, the application/schema+json media type, as defined in [[JSON-SCHEMA]], SHOULD be used. Clients receiving JSON Schema files over HTTP MAY accept content using either the application/schema+json media type or the application/json media type as defined in [[JSON]]."

^^^ You will want to make sure that language exists otherwise implementers might say: "You didn't say I had to do that -- you just said the spec 'acknowledges' these media types... you didn't say I had to use them" <--- that's also the sort of stuff I was concerned about w/ the normative references.

Hope the above makes sense -- you need to make sure you've linked how you're using these normative references with normative spec language that compels implementers to use the technologies you're citing (otherwise, there is no reason to cite those specifications normatively).

msporny commented 12 months ago

Regarding this normative reference: https://pr-preview.s3.amazonaws.com/w3c/vc-json-schema/pull/212.html#id

... I would expect the JSON Schema specs to say that for $id -- do they not? They don't say "You have to use the canonical link URI in the $id field"? That is, are you adding a requirement on top of what JSON Schema requires for $id? If not, you don't need the normative reference.

msporny commented 12 months ago

Your normative dependencies on [JSON-SCHEMA-2019-09] and [JSON-SCHEMA-DRAFT-7] don't make any sense, because implementers don't have to implement them. You can have a non-normative dependency on them (since implementers don't have to implement them).

In short, I still don't think you're doing what you mean to do in the specification. The normative language just doesn't line up with the normative references.

Hope all the above makes sense... I'm trying to help you get down to the set of normative dependencies I think you have rather than a few that you don't need. I'm not going to block going to CR because of this, but you are eventually going to have to defend why you have these normative dependencies during the transition call and I don't think some of them are defensible right now (because you don't have normative language requiring them to be normative dependencies).

decentralgabe commented 12 months ago

thanks @msporny all makes sense, will address in a follow-up PR

decentralgabe commented 12 months ago

Regarding this normative reference: https://pr-preview.s3.amazonaws.com/w3c/vc-json-schema/pull/212.html#id

... I would expect the JSON Schema specs to say that for $id -- do they not? They don't say "You have to use the canonical link URI in the $id field"? That is, are you adding a requirement on top of what JSON Schema requires for $id? If not, you don't need the normative reference.

$id is not required, we're adding that as a normative requirement. I will remove the URI reference and just point to how it's defined by the json schema spec itself.

decentralgabe commented 12 months ago

https://github.com/w3c/vc-json-schema/pull/216