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
84 stars 37 forks source link

Ambiguous Language in Input Evaluation #237

Closed kimdhamilton closed 3 years ago

kimdhamilton commented 3 years ago

In Input Evaluation 2, there is an ambiguity, possibly because "Field Query Result" is undefined.

Suppose we have an input constraint like this:

"constraints": {
  "fields": [
    {
      "path": ["$.issuer", "$.vc.issuer", "$.iss", "$.issuer.id"],
      "purpose": "Looking for credentials from did:web issuer",
      "filter": {
        "type": "string",
        "pattern": "did:web:example.com"
      }
    }
  ]
}

Then suppose the input contains this VC:

{
  "issuer": {
    "id": "did:web:example.com"
  }
}

Input Evaluation Step 2 excerpt is below:

  1. If the constraints property of the Input Descriptor is present, and it contains a fields property with one or more field objects, evaluate each against the candidate input as follows:
  2. Iterate the Input Descriptor path array of JSONPath string expressions from 0-index, executing each expression against the candidate input. Cease iteration at the first expression that returns a matching Field Query Result and use the result for the rest of the field’s evaluation. If no result is returned for any of the expressions, skip to the next candidate input.
  3. If the filter property of the field entry is present, validate the Field Query Result from the step above against the JSON Schema descriptor value.

Not knowing what "Field Query Result" means, based on context, I would expect this input VC not to match this filter. Here's why:

Step 2.1 seems to imply we stop at the first JSONPath match -- not the first JSONPath match that also matches the filter.

Asking because this behavior differs from here, which considers both JSONPath and filter, and therefore my VC matches.

Obviously, it's easy to clean up my example to avoid this problem, but this seems like a good opportunity to clarify the spec as well.

kimdhamilton commented 3 years ago

p.s. I can make a stab at a PR, just wanted to confirm which is intended before doing so. I'd lean towards removing the term "Field Query Result", which is undefined and otherwise unused in the rest of the spec.

JaceHensley commented 3 years ago

Oh interesting that's a really good edge case, I agree with you

csuwildcat commented 3 years ago

@kimdhamilton good catch! My code is written to do the right thing, but the language does need a tweak to tell the loop to keep going if the filter doesn't eval against a given JSON Path-located expression. Thank you for this, we all really appreciate your eagle eyes 🦅 👀

csuwildcat commented 3 years ago

@kimdhamilton merged #241, will close this when Brent has tweaked all the predicate language throughout the spec.

csuwildcat commented 3 years ago

This has been addressed in @kimdhamilton's merged PR