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

Can the status constraint feature property be simplified? #310

Closed decentralgabe closed 2 years ago

decentralgabe commented 2 years ago

https://identity.foundation/presentation-exchange/#credential-status-constraint-feature

The spec says:

The values of all status properties are objects, composed as follows:

status objects MUST include a directive property, and its value MUST be one of the following strings:
required - the credential MUST be of the specified status.
allowed - the credential MAY be of the specified status.
disallowed - the credential MUST NOT be of the specified status.
  "statuses": {
    "active": {
      "directive": "required"  // other values: "allowed", "disallowed"
    },
    "suspended": {...},
    "revoked": {...}
  }

Why objects if they contain a single key and a single value? What about...

  "statuses": {
    "active": "allowed",  // other values: "required", "disallowed"
    "suspended": "allowed",
    "revoked": "disallowed",
  }
OR13 commented 2 years ago

Short answer would seem to be that object can be extended.

When writing specs with JSON, prefer objects to arrays or strings, since you can add properties to fix issues that might arise in the future.

decentralgabe commented 2 years ago

eh, premature optimization. it's a similar amount of work either way.

brentzundel commented 2 years ago

I vaguely recall the conversations that resulted in the value being an object with a directive property than simply a value, but I can't recall the exact reasoning why that decision was made. I'm fine either way.

csuwildcat commented 2 years ago

The reasoning here was that it was likely that you would need to specify which mechanisms you would accept for status checks, not just the actual high-level status value they must be. The addition of which mechanisms are acceptable, with any annotation required to note them, would require at least one additional property. This is admittedly a forward looking structure, but I don't know how else you would do this without allowing for more props as methods for revocation checks become common enough to distinguish between.

decentralgabe commented 2 years ago

Discussed on the call today: we will extend the field to add a type property a-la StatusList2021

decentralgabe commented 2 years ago

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

decentralgabe commented 2 years ago

Fixed in #328