Skip to content

Conversation

@Sakurann
Copy link
Collaborator

@Sakurann Sakurann commented Sep 1, 2023

addresses Issue #30 and #11.
Migrated from Bitbucket: https://bitbucket.org/openid/connect/pull-requests/612

The problem addressed is: lack of ability to communicate issuance of credentials that are same format/type, but different content.

there were few iterations, but the latest ver of a PR:

  • Token response: Issuer can communicate a specific credential for which the Access token is valid for using a new identifiers parameter (array)
  • Credential request: wallet must use identifiers received in token response using a new identifiers parameter (array)

@Sakurann
Copy link
Collaborator Author

Sakurann commented Sep 1, 2023

@tlodderstedt made a comment in Bitbucket regarding token response:

This list might be ok for scope-based requests. However, if authorization was requested using authorization details, the identifiers should be added to the authorization details objects.

For scope-based requests: How is the wallet supposed to relate the identifiers to the (potentially) different credential types it had requested?

I didn't fully understand why.. needs discussion.

@sloops77
Copy link
Contributor

sloops77 commented Sep 5, 2023

To clarify the PR - what is being proposed is that a human-friendly identifier will be required for each different combination of credential data that could be considered unique to a person (or their wallet).

For example, an Issuer issuing employee of the month badges would be required to continually use (& manage) new identifiers to differentiate to the wallet that EmployeeOfTheMonth-May2023 is different from EmployeeOfTheMonth-Dec2023, where the type: OpenBadgeCredential. In other cases the year may be sufficient for differentiating, in others it may not be time but another variable.

Is anyone else nervous about having Issuers figure out what is unique, in a human-readable format, about a credential?

Going with a straight generated id and describing in the /token response which generated id corresponds to which type would have the benefit of not requiring this effort from issuers (or their software)

@Sakurann
Copy link
Collaborator Author

Sakurann commented Sep 6, 2023

@sloops77 this PR is not about the human-readable id per type, it is about multiple ids per type, for the use-cases when there are multiple credentials being issued are of the same type but different content. and this identifier does not have to be human-readable, can be a UUID IMO... wallet just needs it as a signal that despite requesting only one type, multiple credentials are being issued.

@Sakurann
Copy link
Collaborator Author

Sakurann commented Sep 6, 2023

@tlodderstedt added authorization_details.identifiers parameter for the token response.

@Sakurann Sakurann requested a review from peppelinux September 14, 2023 14:38
@Sakurann
Copy link
Collaborator Author

@pmhsfelix

@tlodderstedt
Copy link
Collaborator

Should we require the AS to always return identifiers for all credentials? That might improve and simplify implementations.

@Sakurann
Copy link
Collaborator Author

Sakurann commented Sep 14, 2023

Kristina to make changes making identifier mandatory in the token response and credential request to solve another issue too - WG to review next week and make a decision.

cc @mdobrinic.


* `c_nonce`: OPTIONAL. JSON string containing a nonce to be used to create a proof of possession of key material when requesting a Credential (see (#credential_request)). When received, the Wallet MUST use this nonce value for its subsequent Credential Requests until the Credential Issuer provides a fresh nonce.
* `c_nonce_expires_in`: OPTIONAL. JSON integer denoting the lifetime in seconds of the `c_nonce`.
* `identifiers`: OPTIONAL. JSON array of JSON strings that each identify a credential that can be issued using Access Token returned in the same response. MUST NOT be used when `authorization_details` parameter was used to request issuance of a certain Credential type as defined in (#authorization-details).
Copy link
Contributor

Choose a reason for hiding this comment

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

The whole identifiers thing is not communicated well right now.

What does an identifier identify? What is it used for? Why are there two types of identifiers (for creds requested with scopes and authorization details)? Are they conflict-free? Why do identifiers show up in the credential response?

I can guess answers to all of these questions, but these things must be explicit in the text.

I suppose this should be integrated into an explanation of the various ways of communicating credential contents (offered or requested). (I.e., explaining the mental model behind the credential offer, the metadata, the authorization request, the token response, the credential request and credential response in a very explicit way.) Some of this is done in (#credential-authz-request), but it dives into details really fast and lacks the high-level 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.

great questions!
I started by clarifying the definition of the c_identifiers, which is in scope for this PR.
I think higher level explanation should be done in the PR to issue #77.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I think it is now much clearer!

One clarification question, and sorry if this has been answered elsewhere:

So the relationship currently described is the in any issuance process, multiple "types" of credentials can be issued (*), and for each "type" there can be multiple instances. An instance is described as "contains different claim values or different subset of claims within the claimset identified by the Credential type"

(issuance process) --- 1:n --- (credential definition*) --- 1:m --- (credential instance)

How is the relationship to batch issuance for unlinkability as suggested in SD-JWT? Is this already covered or should there be a separate mechanism for that? I.e., to have something like this for k copies of the same credential:

(issuance process) --- 1:n --- (credential definition*) --- 1:m --- (credential instance) --- 1:k --- (credential instance copy)

(*) entry in the credentials_supported Credential Issuer metadata - I added a comment suggesting to find a good term for this to use instead of credential type.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

great question. i think a separate issue is needed to discuss this. my current thinking is, credential_identifier is used only to issue multiple credentials of the same type, but different content, and there needs to be another switch for issuing copies of the same credential, as using this identifier for both seems really overloaded... cc issue #91

@David-Chadwick
Copy link
Contributor

in our initial implementation, prior to OIDC4VCI, we had an identifier for each credential that was issued so that the wallet could uniquely refer to each one. We found this to be of value, esp when the credential was to be refreshed.

Co-authored-by: Daniel Fett <fett@danielfett.de>
Copy link
Contributor

@danielfett danielfett left a comment

Choose a reason for hiding this comment

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

I left a bunch of mostly editorial comments and a question, but generally this looks good to me.

Comment on lines 626 to 639
"c_nonce_expires_in": 86400,
"authorization_details": [
{
"type": "openid_credential",
"format": "jwt_vc_json",
"credential_definition": {
"type": [
"VerifiableCredential",
"UniversityDegreeCredential"
]
},
"c_instance_identifiers": [ "CivilEngineeringDegree-2023", "ElectricalEngineeringDegree-2023" ]
}
]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"c_nonce_expires_in": 86400,
"authorization_details": [
{
"type": "openid_credential",
"format": "jwt_vc_json",
"credential_definition": {
"type": [
"VerifiableCredential",
"UniversityDegreeCredential"
]
},
"c_instance_identifiers": [ "CivilEngineeringDegree-2023", "ElectricalEngineeringDegree-2023" ]
}
]
"c_nonce_expires_in": 86400,
"authorization_details": [
{
"c_instance_identifiers": [ "CivilEngineeringDegree-2023", "ElectricalEngineeringDegree-2023" ]
}
]

Copy link
Contributor

Choose a reason for hiding this comment

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

Are the details really relevant? I would see benefit in keeping the Token Response format agnostic

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 get multiple credentials offered in a Credential Offer, we haven't actually picked one, so therefore we need those details? We could even use credentials_supported_identifier from #86 to solve this easier then.

Copy link
Collaborator Author

@Sakurann Sakurann Oct 26, 2023

Choose a reason for hiding this comment

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

On your second comment, nothing prevents AS/RS from using the same value for credentials_upported_identifier and c_instance_identifier, but conceptually, I really think these parameters needs to be separate.

Are the details really relevant?

I honestly don't know.. RAR spec says "The AS MAY omit values in the authorization_details to the client." in https://datatracker.ietf.org/doc/html/rfc9396#name-token-response. so I think that means that structure below is technically valid, because "type" is the only required parameter if I am correct. but I am not sure if format and type should be omitted here, maybe when there is only one credential being issued, but not when there are multiple, because having type and format here provides a clear binding between an identifier and type/format. cc @tlodderstedt @bc-pi as RAR experts

Suggested change
"c_nonce_expires_in": 86400,
"authorization_details": [
{
"type": "openid_credential",
"format": "jwt_vc_json",
"credential_definition": {
"type": [
"VerifiableCredential",
"UniversityDegreeCredential"
]
},
"c_instance_identifiers": [ "CivilEngineeringDegree-2023", "ElectricalEngineeringDegree-2023" ]
}
]
"c_nonce_expires_in": 86400,
"authorization_details": [
{
"type": "openid_credential",
"c_instance_identifiers": [ "CivilEngineeringDegree-2023", "ElectricalEngineeringDegree-2023" ]
}
]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You seem to want to make most of the requests in VCI format agnostic. why is that..?

* `c_nonce`: OPTIONAL. JSON string containing a nonce to be used when creating a proof of possession of the key proof (see (#credential_request)). When received, the Wallet MUST use this nonce value for its subsequent requests until the Credential Issuer provides a fresh nonce.
* `c_nonce_expires_in`: OPTIONAL. JSON integer denoting the lifetime in seconds of the `c_nonce`.
* `authorization_details`: REQUIRED when `authorization_details` parameter is used to request issuance of a certain Credential type as defined in (#authorization-details). MUST NOT be used otherwise. A JSON array of objects as defined in Section 7 of [@!RFC9396]. This specification defines the following parameter to be used with authorization details type `openid_credential` in the Token Response:
* `c_instance_identifiers`: OPTIONAL. JSON array of JSON strings that each uniquely identify a Credential instance that can be issued using Access Token returned in this response. Each Credential instance is a unique Credential described using the same entry in the `credentials_supported` Credential Issuer metadata, but can contain different claim values or different subset of claims within the claimset identified by the Credential type. This parameter can also be used to simplify the Credential Request, since as defined in (#credential_request) `c_instance_identifier` parameter replaces `format` and any other Credential format specific parameters in the Credential Request. When received, the Wallet MUST use these values together with an Access Token in the subsequent Credential Request(s).
Copy link
Contributor

Choose a reason for hiding this comment

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

Should c_instance_identifiers only be used when there are multiple credentials offered?

Copy link
Collaborator Author

@Sakurann Sakurann Oct 26, 2023

Choose a reason for hiding this comment

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

The current intention is that c_instance_identifiers can be used regardless of how many credentials are being issued. this means that identifier can be used to simplify the credential request even if only one credential is being issued.


* `format`: REQUIRED. Format of the Credential to be issued. This Credential format identifier determines further parameters required to determine the type and (optionally) the content of the credential to be issued. Credential Format Profiles consisting of the Credential format specific set of parameters are defined in (#format_profiles).
* `proof`: OPTIONAL. JSON object containing proof of possession of the key material the issued Credential shall be bound to. The `proof` object MUST contain a following claim:
* `format`: REQUIRED when the `c_instance_identifier` was not returned from the Token Response. MUST NOT be used otherwise. This parameter determines the format of the Credential to be issued, which may determine the type and any other information related to the Credential to be issued. Credential Format Profiles consisting of the Credential format specific set of parameters are defined in (#format_profiles). When this parameter is used, `c_instance_identifier` parameter MUST NOT be present.
Copy link
Contributor

Choose a reason for hiding this comment

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

When is format actually relevant?
In Authorization Code Flow we already made a choice in the Auth Request.
In Credential Offer we got all the options with c_instance_identifier in the Token Response and we can pick one here with c_instance_identifier. What am I missing?

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 is relevant when the multiple credentials of different formats are being issued using scope parameter mechanism in the authorization request.

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 think this is a great improvement to the spec.
A few questions remain for me:

  • Could we use this to solve the questions whether the Wallet needs to call Credential Endpoint or Batch Credential Endpoint?
  • Can we keep Token Response credential format agnostic, i.e. only keep c_instance_identifier instead of repeating the whole authorization_details object?
  • Could we even remove format from Credential Request and make c_instance_identifier mandatory?

I also have the feeling that the terminology is not right here yet. I see three levels to distinguish:

  1. different credential types, each corresponds to one credentials_supported section in the Issuer metadata, e.g. birth certificate and diploma
  2. different credentials (here called instances in this PR), each the same credentials_supported but with different claims data, e.g. my bachelor degree and my master degree
  3. different instances of the same credential (this I would call instances instead), these are only used for unlinkability purposes

We don't need to solve everything in this PR however, so I' fine if we know how to proceed with these questions in the future

Co-authored-by: Daniel Fett <fett@danielfett.de>
@Sakurann
Copy link
Collaborator Author

SIOP/DCP WG call - agreed to change the identifier parameter name to credential_identifier and merge. and continue bikeshedding the name and keep discussing the issues Paul is raising in new issues.

@Sakurann Sakurann merged commit 662f685 into main Oct 27, 2023
@Sakurann
Copy link
Collaborator Author

Sakurann commented Oct 27, 2023

PR ended up addressing Issue #29 too by simplifying the credential request even when only one credential is being issued.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.