Skip to content

Conversation

@Sakurann
Copy link
Collaborator

@Sakurann Sakurann commented Oct 16, 2023

As discussed during IIW week, changing the structure of credentials parameter in the credential offer back to the identifier that can be used to obtain specific credentials_supported object from the issuer metadata. Also removing an option to pass format/type in the credential offer. Also agreed that inline metadata in the credential offer will not be supported. Addresses issues #83, #82, #77, #53.

During pre-IIW DCP WG mtg, we discussed enabling credentials_supported object being hosted not necessarily at .well-known/openid-credential-issuer as currently defined in the specification but at the private location that is known privately between the wallet and the Issuer. Current text A JSON array of JSON strings that each uniquely identify one of the objects in the credentials_supported array included in the Credential Issuer metadata as defined in (#credential-issuer-parameters). should be open enough to allow this.

There are separate issues like #42 on how the wallet could potentially add this identifier as query to the issuer metadata location so that the issuer returns part of the maya data instead of the wallet needing to search potentially large metadata file, but that is probably a separate PR.

@Sakurann Sakurann changed the title add credentials_supported identifiers credentials Credentials Offer parameter values identifying credentials_supported object(s) in the issuer's metadata. Oct 16, 2023
This section defines the structure of the objects that appear in the `credentials_supported` metadata parameter.

* `format`: REQUIRED. A JSON string identifying the format of this credential, i.e., `jwt_vc_json` or `ldp_vc`. Depending on the format value, the object contains further elements defining the type and (optionally) particular claims the credential MAY contain and information about how to display the credential. (#format_profiles) defines Credential Format Profiles introduced by this specification.
* `credentials_supported_identifier`: REQUIRED when Credential Offer is used. A JSON string uniquely identifying this object within the `credentials_supported` array. It is used in the Credential Offer as defined in (#credential_offer_parameters) to enable the Wallet to discover the information about the Credential being offered.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a unique identifier could we consider renaming this to id and moving it above format in this definition to signify the importance?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Within VCI, there are two different identifiers: 1) identifier that points to a credentials_supported object in the issuer metadata, and 2) identifier that identifies a credential instance during issuance (PR #65), so I was trying to differentiate those.

Having said that, there is only one identifier within issuer metadata, so maybe just "id" is ok...

There are so many "id" claims in the Credential space: in the DIF presentation exchange, in W3C VCDM, that i would prefer differentiating from those, but maybe no need...

On the order, I usually like putting mandatory parameters at the top, but in this case, logically, identifier should probably be at the top.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @cobward , it is the primary identifier and belongs to the top. I was also wondering why its not named id but I'm fine with the explanation

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with credentials_subject structure changing to a map, there will be no more credentials_supported_identifier :)
@cobward I think this data structure change of a credentials_supported_identifier is a breaking change for your implementation.. please check.


* `credential_issuer`: REQUIRED. The URL of the Credential Issuer, as defined in (#credential-issuer-identifier), from which the Wallet is requested to obtain one or more Credentials. The Wallet uses it to obtain the Credential Issuer's Metadata following the steps defined in (#credential-issuer-wellknown).
* `credentials`: REQUIRED. A JSON array, where every entry is a JSON object or a JSON string. If the entry is an object, the object contains the data related to a certain credential type the Wallet MAY request. Each object MUST contain a `format` Claim determining the format of the credential to be requested and further parameters characterizing the type of the credential to be requested as defined in (#format_profiles). If the entry is a string, the string value MUST be one of the `scope` values in one of the objects in the `credentials_supported` Credential Issuer metadata parameter. When processing, the Wallet MUST resolve this string value to the respective object.
* `credentials`: REQUIRED. A JSON array of JSON strings that each uniquely identify one of the objects in the `credentials_supported` array included in the Credential Issuer metadata as defined in (#credential-issuer-parameters). The Wallet resolves this string value to the respective object to obtain information about the Credential being offered defined in (credential-metadata-object). For example, this string value can be used to obtain `scope` value to be used in the Authorization Request.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be reworded to make it clear which element in the credentials_supported object it is supposed to match against? I also think it is redundant to say uniquely identify here, as the definition of the identifier already includes that.

Maybe something like:

Suggested change
* `credentials`: REQUIRED. A JSON array of JSON strings that each uniquely identify one of the objects in the `credentials_supported` array included in the Credential Issuer metadata as defined in (#credential-issuer-parameters). The Wallet resolves this string value to the respective object to obtain information about the Credential being offered defined in (credential-metadata-object). For example, this string value can be used to obtain `scope` value to be used in the Authorization Request.
* `credentials`: REQUIRED. A JSON array of JSON strings that each can be matched against the `credentials_supported_identifier` parameter of the objects in the `credentials_supported` array included in the Credential Issuer metadata as defined in (#credential-issuer-parameters). The Wallet resolves this string value to the respective object to identify and obtain information about the Credential being offered defined in (credential-metadata-object). For example, this string value can be used to obtain `scope` value to be used in the Authorization Request.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* `credentials`: REQUIRED. A JSON array of JSON strings that each uniquely identify one of the objects in the `credentials_supported` array included in the Credential Issuer metadata as defined in (#credential-issuer-parameters). The Wallet resolves this string value to the respective object to obtain information about the Credential being offered defined in (credential-metadata-object). For example, this string value can be used to obtain `scope` value to be used in the Authorization Request.
* `credentials`: REQUIRED. An array of strings that each can be matched against the `credentials_supported_identifier` parameter of the objects in the `credentials_supported` array included in the Credential Issuer metadata as defined in (#credential-issuer-parameters). The Wallet resolves this string value to the respective object to identify and obtain information about the Credential being offered defined in (credential-metadata-object). For example, this string value can be used to obtain `scope` value to be used in the Authorization Request.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the structure has changed to map, please check an updated language..

@danielfett
Copy link
Contributor

IMO, this proposal can be greatly simplified by changing the type of credentials_supported from an array to an object. The object keys would be what is now credentials_supported_identifier. This would also avoid having to name this identifier.

Example for credentials_supported:

{
   "UniversityDegreeCredential":{
      "format":"jwt_vc_json",
      "cryptographic_binding_methods_supported":[
         
      ]
   },
   "Example2":{
      "format":"mso_mdoc",
      "doctype":"org.iso.18013.5.1.mDL"
   }
}

In the credential offer would then contain UniversityDegreeCredential or Example2 as required.

@paulbastian
Copy link
Contributor

@danielfett and I jsut realized the potential of this PR is even bigger. We could simplify Token Request and Credential Request as well and get rid off alomst all credential-format specific things

Copy link
Contributor

@paulbastian paulbastian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to understand the reasoning behind Authorization Details.

If I look at the Credential Request, it currently includes format and additional credential-format specific data, e.g.:

{
   "format": "jwt_vc_json",
   "credential_definition": {
      "type": [
         "VerifiableCredential",
         "UniversityDegreeCredential"
      ],
      "credentialSubject": {
         "given_name": {},
         "family_name": {},
         "degree": {}
      }
   },
   "proof": {
      "proof_type": "jwt",
      "jwt":"eyJraWQiOiJkaWQ6ZXhhbXBsZTplYmZlYjFmNzEyZWJjNmYxYzI3NmUxMmVjMjEva2V5cy8
      xIiwiYWxnIjoiRVMyNTYiLCJ0eXAiOiJKV1QifQ.eyJpc3MiOiJzNkJoZFJrcXQzIiwiYXVkIjoiaHR
      0cHM6Ly9zZXJ2ZXIuZXhhbXBsZS5jb20iLCJpYXQiOiIyMDE4LTA5LTE0VDIxOjE5OjEwWiIsIm5vbm
      NlIjoidFppZ25zbkZicCJ9.ewdkIkPV50iOeBUqMXCC_aZKPxgihac0aW9EkL1nOzM"
   }
}

or

{
   "format": "mso_mdoc",
   "doctype": "org.iso.18013.5.1.mDL",
   "claims": {
      "org.iso.18013.5.1": {
         "given_name": {},
         "family_name": {},
         "birth_date": {}
      },
      "org.iso.18013.5.1.aamva": {
         "organ_donor": {}
      }
   },
   "proof": {
      "proof_type": "jwt",
      "jwt": "eyJraWQiOiJkaWQ6ZXhhbXBsZ...KPxgihac0aW9EkL1nOzM"
   }
}

Both could be changed to include only the id from the Issuer Metadata, similar to what I proposed to the simplification of Credential Offer. In that way we get rid of almost all Credential-format specific data in the Appendix E:

{
   "id": "UniversityDegreeCredential",
   "proof": {
      "proof_type": "jwt",
      "jwt": "eyJraWQiOiJkaWQ6ZXhhbXBsZ...KPxgihac0aW9EkL1nOzM"
   }
}

@Sakurann
Copy link
Collaborator Author

@danielfett and I jsut realized the potential of this PR is even bigger. We could simplify Token Request and Credential Request as well and get rid off alomst all credential-format specific things

@paulbastian that is already addressed in PR #65. it is out of scope of this PR because two identifiers need to be clearly distinguished: this PR talks only about the identifier used to match against an object within issuer metadata, while PR #65 talks about an identifier of a credential instance that is being issued.

PR #65 allows to use c_instance_identifier instead of format specific set of parameters in the credential request, which could greatly simplify the request when authorization_details are used. there is no consensus whether instance identifiers should be enabled when scopes are used instead of RAR, because the mapping of an instance identifier to a certain scope value can be quite complex.

To prevent further confusion, summarizing current E2E flows we will have in the spec if both this and PR #65 get merged.

  1. Credential Offer with scopes
    a. wallet gets credential offer, uses credentials_supported_identifier to obtain a relevant part of a metadata and a scope value associated with it.
    b. wallet uses scope and format specific parameters in authz~credential requests, no credential_instance_identifiers

  2. Credential offer with authorization details
    a. wallet gets credential offer, uses credentials_supported_identifier to obtain a relevant part of a metadata
    b. wallet uses authorization details in authz~token requests and can simplify credential request by using only credential_instance_identifiers if AS/RS support it.

(steps 1a and 1b can obviously be used stand-alone without 1a and 2a.)

so in short, I think there is no further simplification needed in this PR.

@Sakurann
Copy link
Collaborator Author

IMO, this proposal can be greatly simplified by changing the type of credentials_supported from an array to an object. The object keys would be what is now credentials_supported_identifier. This would also avoid having to name this identifier.

I agree changing to a map looks cleaner, but I don't have a strong opinion - would like feedback from the WG.

@Sakurann Sakurann requested a review from paulbastian October 25, 2023 01:29
@Sakurann
Copy link
Collaborator Author

@TimoGlastra @nklomp can you please review if you get a chance..?

Copy link
Contributor

@paulbastian paulbastian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great improvement.

It seems to me that there is some redundancy between credentials_supported_identifier , scope , c_instance_identifier and authorization_details but that become clearer once both PRs are merged.

@Sakurann
Copy link
Collaborator Author

as part of changing the data structure to a map, I also merged two sub-sections within the issuer metadata section. so there is no more section dedicated to explaining parameters comprising credentials_supported object.

also, originally, credentials_supported_identifier was mandatory only when credential offer was used, but now it is always mandatory. so... what if the flow does not start with the credential offer? the wallet can obtain object values even without knowing the key value, but thought it is worth pointing out.

* `text_color`: OPTIONAL. String value of a text color of the Credential represented as numerical color values defined in CSS Color Module Level 37 [@!CSS-Color].

It is dependent on the Credential format where the available claims will appear and how they are represented (see (#format_profiles)).
* `credentials_supported`: REQUIRED. An object that describes specifics of the Credential that the Credential Issuer supports issuance of. This object contains a list of name/value pairs, where each name is an identifier of the credential being described. This identifier is used in the Credential Offer as defined in (#credential_offer_parameters) to communicate to the Wallet which Credential is being offered. The value is an object that contains metadata about specific credential and contains the following parameters defined by this specification:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have a breaking change here, it is a convenient opportunity for any renaming from #91 . This would also enable some backwards-compatiblity

Copy link
Member

@TimoGlastra TimoGlastra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look good to me 👍

Co-authored-by: Timo Glastra <timo@animo.id>
Copy link

@nklomp nklomp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall liking the change. Have some small comments to make it more clear the keys need to be unique and a remark about the scope, for which the lookup now has a bit of different behaviour.

But these are non-blocking

In addition to a mechanism defined in (#credential-authz-request), Credential Issuers MAY support requesting authorization to issue a credential using OAuth 2.0 scope parameter.

When the Wallet does not know which scope value to use to request issuance of a certain credential, it can discover it using the `scope` Credential Issuer metadata parameter defined in (#credential-metadata-object). When the flow starts with a Credential Offer, the Wallet can use the information in the `credentials` Credential Offer parameter. When the entry of the `credentials` parameter is a string, it can be used as a scope value. When the entry of the `credentials` parameter is an object, it can be used to discover the desired scope value from the Credential Issuer metadata that can be obtained using `credential_issuer` Credential Offer parameter as defined in (#credential-issuer-wellknown).
When the Wallet does not know which scope value to use to request issuance of a certain credential, it can discover it using the `scope` Credential Issuer metadata parameter defined in (#credential-issuer-parameters). When the flow starts with a Credential Offer, the Wallet can use the `credentials` parameter values to identify object(s) in the `credentials_supported` map in the Credential Issuer metadata parameter and use `scope` parameter value from that object.
Copy link

@nklomp nklomp Nov 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously it allowed for the param to either be a string, which then needed to be used as a scope, or it was the object, from which you then had to lookup the scope in the respective credentials_supported object.

scopes are however completely optional in credentials_supported. Since the string will now always map onto the respective key of the credentials_supported map, this change means we now always need to look at the scrope value. Is that desired, or should the text be reworked that you need to lookup the scope and if not present use the string value?

Copy link
Collaborator Author

@Sakurann Sakurann Nov 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is a desired/intended behavior that if the wallet needs to look up the scope value, it uses the identifier to look it up. if it does not need scope value, it just uses the parameters other than scope values to formulate authorization_details request. allowing to use identifier as a scope value will lead to ambiguities when that is allowed and when it is not. and will also prevent using the same scope values across multiple credentials_supported entries

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, just pointing out the difference with previous version

Copy link
Contributor

@pmhsfelix pmhsfelix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with minor comments.

In addition to a mechanism defined in (#credential-authz-request), Credential Issuers MAY support requesting authorization to issue a credential using OAuth 2.0 scope parameter.

When the Wallet does not know which scope value to use to request issuance of a certain credential, it can discover it using the `scope` Credential Issuer metadata parameter defined in (#credential-metadata-object). When the flow starts with a Credential Offer, the Wallet can use the information in the `credentials` Credential Offer parameter. When the entry of the `credentials` parameter is a string, it can be used as a scope value. When the entry of the `credentials` parameter is an object, it can be used to discover the desired scope value from the Credential Issuer metadata that can be obtained using `credential_issuer` Credential Offer parameter as defined in (#credential-issuer-wellknown).
When the Wallet does not know which scope value to use to request issuance of a certain credential, it can discover it using the `scope` Credential Issuer metadata parameter defined in (#credential-issuer-parameters). When the flow starts with a Credential Offer, the Wallet can use the `credentials` parameter values to identify object(s) in the `credentials_supported` map in the Credential Issuer metadata parameter and use `scope` parameter value from that object.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest "credentials_supported object in" instead of "credentials_supported map in" (i.e. use object instead of map).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it used to say object and Timo suggested to change to map and I agreed. happy to change back if this comes up again..

@Sakurann Sakurann merged commit 1b7cd74 into main Nov 5, 2023
jogu added a commit that referenced this pull request Jan 17, 2024
This is the only mention of credentials_supported_identifier in the whole
specificaiton, I believe this was just overlooked in
#86
jogu added a commit that referenced this pull request Jan 17, 2024
This is the only mention of credentials_supported_identifier in the whole
specification, I believe this was just overlooked in
#86
Sakurann pushed a commit that referenced this pull request Jan 18, 2024
)

editorial. approval of one of the editors.

This is the only mention of credentials_supported_identifier in the whole
specification, I believe this was just overlooked in
#86
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants