search

decentralized-identity / presentation-exchange

Specification that codifies an inter-related pair of data formats for defining proof presentations (Presentation Definition) and subsequent proof submissions (Presentation Submission)
https://identity.foundation/presentation-exchange
Apache License 2.0
86 stars 37 forks source link

Clarify or remove formatMinimum and formatMaximum #312

Closed decentralgabe closed 2 years ago

decentralgabe commented 2 years ago

From https://github.com/decentralized-identity/presentation-exchange/pull/311

formatMaximum and formatMinimum are not defined in the Draft 7 specs:

There is a specific JS library that supports these additional fields...but it's not in the spec. So neither my change, nor the spec are correct and we probably need non-normative language to support non-integer min/max validation.

decentralgabe commented 2 years ago

I recommend we require implementations to use a JSON Schema extension to support these properties https://datatracker.ietf.org/doc/html/draft-handrews-json-schema-01#section-6.4

JaceHensley commented 2 years ago

so PE just says that the filter property "MUST be a JSON Schema descriptor", does the "JSON-schema" schema say that additional properties cannot be set?

decentralgabe commented 2 years ago

so PE just says that the filter property "MUST be a JSON Schema descriptor", does the "JSON-schema" schema say that additional properties cannot be set?

No: https://github.com/decentralized-identity/presentation-exchange/blob/master/test/presentation-definition/schema.json#L75

Even if it did, we would have to semantically define how to process non-normative filters.

JaceHensley commented 2 years ago

Oh sorry I was talking about this one https://github.com/decentralized-identity/presentation-exchange/blob/master/test/presentation-definition/schema.json#L191

If you look here https://www.jsonschemavalidator.net/s/bk8N3TEI it seems like you can add additional properties to a json schema. And then it would become a case of graceful upgrade/downgrade. If a verifier puts a formatMinimum in their filter then it doesn't matter if the holder's wallet can understand that because regardless the verifier will run the holder's Presentation Submission against the same IDOs and they would catch the invalid credentials. A holder may submit credentials that don't satisfy the requirements but that's something that could happen regardless

decentralgabe commented 2 years ago

in practice, yes. however, the examples in the spec reference JSON Schema Draft 7, and specific library implementations. Some of these libraries do support extensions. We also call out filter properties we support in the predicate feature section.

I see a few possible options (and possibly combinations of these):

David-Chadwick commented 2 years ago

The lesson that I draw from this is that PE cannot mandate any wallet to execute the procedure that is documented. The most PE should do is specify a procedure, say this is an example of a procedure that MAY be used, but an implementation is free to implement any procedure it wants to, providing that the output yields credentials that match the presentation definition.

JaceHensley commented 2 years ago

@decentralgabe gotcha okay, I like the first two options there, PE shouldn't define those but should remind people that json schema can have extensions and that's allowed for filter. I'll admit I forgot about predicate and that we restrict filter when that's used, so extra language around that makes sense to me too