openid / OpenID4VCI

60 stars 16 forks source link

Multiple value types in definition of credentials #53

Closed OIDF-automation closed 8 months ago

OIDF-automation commented 11 months ago

Imported from AB/Connect bitbucket: https://bitbucket.org/openid/connect/issues/2001

Original Reporter: rolandh

In 4.1.1 credentials is defined to be: "A JSON array, where every entry is a JSON object or a JSON string."

To an implementer this is terrible. Getting a piece of information where you have to figure out what kind of values are included is not very efficient.

I propose that credentials be limited to "A JSON array, where every entry is a JSON object" and then add

credential_references which are defined as "A JSON array of strings"

OIDF-automation commented 11 months ago

Imported from AB/Connect bitbucket - Original Commenter: KristinaYasuda

introducing credentials_references might work.

OIDF-automation commented 11 months ago

Imported from AB/Connect bitbucket - Original Commenter: mbj

Splitting the set of credentials into two pieces has its own downsides. I don’t think there’s a compelling reason to not continue to use the current polymorphic JSON arrays.

OIDF-automation commented 11 months ago

Imported from AB/Connect bitbucket - Original Commenter: rolandh

If the split is done the code becomes so much simpler and less error prone. Which to me is reason enough.

OIDF-automation commented 11 months ago

Imported from AB/Connect bitbucket - Original Commenter: KristinaYasuda

we discussed it in the editor’s call. there was not much support in making this change. We already have places where there is polymorphism - for example, in the VP Token when the entry can be a string or an object depending on the credential format.

what you are suggesting would probably look like below and it didn’t look very pretty..

{
   "credential_issuer": "https://credential-issuer.example.com",
   "credentials": [
      {
         "format": "mso_mdoc",
         "doctype": "org.iso.18013.5.1.mDL"
      }
   ],
   "credentials_references": ["UniversityDegree_JWT"],
   "grants": {
      "authorization_code": {
         "issuer_state": "eyJhbGciOiJSU0Et...FYUaBy"
      },
      "urn:ietf:params:oauth:grant-type:pre-authorized_code": {
         "pre-authorized_code": "adhjhdjajkdkhjhdj",
         "user_pin_required": true
      }
   }
}

We think that one way to address this might be by removing the option of the entry in the “credentials” array to be an object, by introducing an id element in the credentials_supported (issue #1923)…

Sakurann commented 9 months ago

this might be addressed by the discussion in #77

Sakurann commented 8 months ago

PR merged. not the only option is a string, no more polymorphism