Sphereon-Opensource / PEX

A Typescript implementation of the DIF Presentation Exchange specification.
Apache License 2.0
38 stars 15 forks source link

`statuses` `constraints` not evaluated #163

Open DJHunn39 opened 1 week ago

DJHunn39 commented 1 week ago

I'm submitting a ...

It appears the library doesn't evaluate submissions against statuses constraints in proof definitions.

For example, for this proof definition:

{
    id: 'someId',
    name: 'someName',
    purpose: 'Active credential check',
    input_descriptors: [
      {
        id: 'someDescriptorId',
        constraints: {
          fields: [
            {
              path: ['$.vct'],
              filter: {
                type: 'string',
                const: 'someExpectedVct'
              }
            }
          ],
          statuses: {
            active: { directive: 'required' }
          }
        }
      }
    ]
  }

I would expect an SD-JWT VC with an exp in the past to be unusable when generating a submission for this proof definition. Instead, a submission is created, and is accepted by a verifier.

The input descriptor in the proof definition above aligns with the DIF PEX standard (both v1 and latest).

I've had a look at the handlers listed in the evaluationClient, and I think it's missing one for this type of constraint. statuses constraints in proof definitions are validated, though, so I might have missed some other piece of code that performs status evaluation.

TimoGlastra commented 1 week ago

Hey @DJHunn39, it is correct that statuses are not validated by the PEX library. I think this may be better suited also for a higher level framework to handle (such as Credo). As statuses also concerns revocation, which is quite expensive to check in a presentation, and thus might need some special handling.

Do you think it makes sense if we do check for expiration with statuses active, but ignore the revocation check? On the Credo verification side currently verification will always fail if the credential is expired or revoked, independent of what is in the Presentation Definition

nklomp commented 1 week ago

Agreed. We also check outside of the definition. Although there are use cases where for instance an expired, or maybe even a revoked credential might make sense. I think we could create a callback for these use cases, so people using Credo, Veramo or any other implementation can hookup there status implementation of choice. This library would never be able to do that, because we do not want to incorporate all kinds of libs/deps and having to support multiple implementations

DJHunn39 commented 1 week ago

Fair enough, we can perform the check outside of PEX (or Credo, if necessary) too.

I'll have another look at what happens when we use an expired credential in a submission, so far I haven't seen it fail but it sounds like there's either a problem on my end or in Credo based on what @TimoGlastra mentioned.

As @nklomp mentioned, there are cases where an expired credential could be acceptable, so ideally Credo/others would make provisions for that. It would get confusing if Credo partially supports status checks, imo.