Add credential dataset version#472
Conversation
awoie
requested review from
Sakurann,
c2bo,
danielfett,
jogu,
paulbastian,
tlodderstedt and
tplooker
tplooker
left a comment
tplooker
left a comment
There was a problem hiding this comment.
Minor editorial review, generally very supportive of this proposal I think its a critical feature. Few other thoughts
- We should consider making this feature required as leaving it optional will make communicating credential updates/refreshes difficult.
- I believe the specification would benefit from a seperate additional endpoint that enables a wallet to ask if there are any updates for a specific credential. Otherwise without this a wallet is forced to ask for a new credential in order to determine whether anything has changed.
|
Only other thing that came to mind on this topic that perhaps we need to discuss is how we support different datasets versus different versions of the same dataset as I suspect in the event an issuer is issuing two different datasets for the same credential (e.g two credentials about different people), to the same wallet this identifier would become ambiguous. |
|
temporarily close to prevent confusion - will reopen once 1.0 goes out |
|
reopening now that 1.0 has been published. Please push the changes to 1.1.md, and not 1.0.md |
awoie
force-pushed
the
awoie/add-credential-versioning
branch
from
1af74c9 to
5846457
Compare
@tplooker If the same credential configuration is used for two different initial data sets, then you would need some additional mechanism. Wouldn't this be rather two distinct credential configurations, e.g., child, parent configuration? We could also introduce another layer between credential configuration and credential dataset identifier (version)? Is there a third option and do you have a proposal, e.g., through some new endpoint? |
Sakurann
left a comment
Sakurann
left a comment
There was a problem hiding this comment.
I think it would be good to add a bit more description of the feature this parameter enables outside the definition of a term?
GarethCOliver
left a comment
GarethCOliver
left a comment
There was a problem hiding this comment.
Add either normative, or implementation considerations that if a credential data does not change then the version MUST remain the same.
This prevents thrashing on the wallet side, for wallets that are making use of the versioning to discard old data sets etc.
Co-authored-by: Frederik Krogsdal Jacobsen <fkj@users.noreply.github.com>
Co-authored-by: Frederik Krogsdal Jacobsen <fkj@users.noreply.github.com>
paulbastian
left a comment
paulbastian
left a comment
There was a problem hiding this comment.
Please discuss:
- is parameter mandatory/optional
- is this a number or a string
- add clarity how a wallet compares versions (likely simple string comparison)
- I believe an implementation consideration would make sense, see Gareths comment and also this could shorten some of the texts in the normative part, that has become a little bloated
| * `transaction_id`: OPTIONAL. String identifying a Deferred Issuance transaction. This parameter is contained in the response if the Credential Issuer cannot immediately issue the Credential. The value is subsequently used to obtain the respective Credential with the Deferred Credential Endpoint (see (#deferred-credential-issuance)). It MUST not be used if the `credentials` parameter is present. It MUST be invalidated after the Credential for which it was meant has been obtained by the Wallet. | ||
| * `interval`: REQUIRED if `transaction_id` is present. Contains a positive number that represents the minimum amount of time in seconds that the Wallet SHOULD wait after receiving the response before sending a new request to the Deferred Credential Endpoint. It MUST NOT be used if the `credentials` parameter is present. | ||
| * `notification_id`: OPTIONAL. String identifying one or more Credentials issued in one Credential Response. It MUST be included in the Notification Request as defined in (#notification). It MUST not be used if the `credentials` parameter is not present. | ||
| * `credential_dataset_id`: REQUIRED for the Issuer to return. An opaque string containing the Credential Dataset Identifier associated with the returned Credential(s). This allows Wallets to detect changes to the underlying Credential Dataset across different Credential Responses. This is useful in situations where claim values change over time, such as an updated address, correction of previously issued personal data, or a change in legal or entitlement status (e.g., reaching the age of majority), enabling the Wallet to distinguish between a cryptographic re-issuance of unchanged data and the issuance of a credential containing modified claim values. Note that this information is only valid for the scope of a concrete credential format: if a Credential is offered in different formats, they would have different values for `credential_dataset_id`. The Wallet MUST NOT expect the `credential_dataset_id` to be always present in the Credential Response. |
There was a problem hiding this comment.
| * `credential_dataset_id`: REQUIRED for the Issuer to return. An opaque string containing the Credential Dataset Identifier associated with the returned Credential(s). This allows Wallets to detect changes to the underlying Credential Dataset across different Credential Responses. This is useful in situations where claim values change over time, such as an updated address, correction of previously issued personal data, or a change in legal or entitlement status (e.g., reaching the age of majority), enabling the Wallet to distinguish between a cryptographic re-issuance of unchanged data and the issuance of a credential containing modified claim values. Note that this information is only valid for the scope of a concrete credential format: if a Credential is offered in different formats, they would have different values for `credential_dataset_id`. The Wallet MUST NOT expect the `credential_dataset_id` to be always present in the Credential Response. | |
| * `credential_dataset_ver`: REQUIRED for the Issuer to return. An opaque string containing the Credential Dataset Version associated with the returned Credential(s). This allows Wallets to detect changes to the underlying Credential Dataset across different Credential Responses. This is useful in situations where claim values change over time, such as an updated address, correction of previously issued personal data, or a change in legal or entitlement status (e.g., reaching the age of majority), enabling the Wallet to distinguish between a cryptographic re-issuance of unchanged data and the issuance of a credential containing modified claim values. Note that this information is only valid for the scope of a concrete credential format: if a Credential is offered in different formats, they would have different values for `credential_dataset_ver`. The Wallet MUST NOT expect the `credential_dataset_ver` to be always present in the Credential Response. |
There was a problem hiding this comment.
The last and first sentence are contradicting, feels like you didn't finish the first sentence and there is a condition missing. I would not make this feature mandatory.
There was a problem hiding this comment.
No, it is not contradicting. For 1.1 issuers, the feature is mandatory. Because a 1.1 wallet may talk to a 1.0 issuer, wallets cannot expect this to be present in all cases.
Co-authored-by: Paul Bastian <paul.bastian@posteo.de>
Co-authored-by: Paul Bastian <paul.bastian@posteo.de>
|
Discussed in meeting today. @awoie has a clear view of what needs to be adjusted and plans to do the work before the next meeting. |
|
@fkj @paulbastian @GarethCOliver I incorporated all of your feedback. Please review again. (cc @brentzundel ) |
| * `interval`: REQUIRED if `transaction_id` is present. Contains a positive number that represents the minimum amount of time in seconds that the Wallet SHOULD wait after receiving the response before sending a new request to the Deferred Credential Endpoint. It MUST NOT be used if the `credentials` parameter is present. | ||
| * `notification_id`: OPTIONAL. String identifying one or more Credentials issued in one Credential Response. It MUST be included in the Notification Request as defined in (#notification). It MUST not be used if the `credentials` parameter is not present. | ||
| * `credential_metadata`: OPTIONAL. Object that contains additional metadata specific to the issued Credential(s). The definitions and contained parameters for this Object are identical to the `credential_metadata` parameter as defined in Credential Issuer Metadata (see (#credential-issuer-metadata)) See (#display-metadata-considerations) for implementation considerations on credential metadata. | ||
| * `credential_dataset_version`: REQUIRED for the Issuer to return. A string containing the Credential Dataset Version associated with the returned Credential(s). This allows Wallets to detect changes to the underlying Credential Dataset across different Credential Responses. The Wallet MUST NOT expect the `credential_dataset_version` parameter to always be present in the Credential Response for backward compatibility with Issuers conforming to earlier versions of this specification. See (#credential-dataset-version-implementation) for implementation considerations. |
There was a problem hiding this comment.
IMO we shouldn't say 'REQUIRED' if wallets can't expect it.
Say it's OPTIONAL but RECOMMENDED, with an intent to make it REQUIRED when we go to a 2.0.
There was a problem hiding this comment.
wallets cannot expect it if they are talking to an issuer conforming to a previous version of this specification. however, for 1.1 issuers we can require it with the language that is in this pr. does this make sense? otherwise, we might need to discuss.
There was a problem hiding this comment.
I agree that the spec is technically correct, but I also agree that this will confuse people. In principle I'm OK with making it REQUIRED for the issuer to return, but we should try to write the compatibility purpose in a clearer way.
There was a problem hiding this comment.
I agree with Oliver here:
- 1.1 issuer can interact with 1.0 wallet, because the wallet is supposed to ignore unknown fields.
- 1.1 wallet can interact with 1.0 issuer if they have a reasonable default behavior.
Imo, this default behavior could be to always assume a different credential dataset version, hence throw away all previous credentials
There was a problem hiding this comment.
Yes this technically works, but in practice consider that 1.1 is meant to be a minor version bump, so if I'm an issuer adopting a new feature of 1.1 (e.g. IAE, redirect_uri, credman create) I'd expect to only need to implement support for that new feature. But, I'll actually have to start supplying a completely unrelated field from a completely unrelated endpoint. This makes it 'feel' like a breaking change, and I don't think the benefits (where this is basically a quality-of-life feature) warrant that.
I also don't like that if I was to write a JSON Schema for this, I'd need different ones for the Issuer and the Wallet, which just seems like it'll lead to problems in the real world.
(unrelated, we should start adding schema versions and negotiation to our API, and maybe consider feature/extension support declaration, so these sorts of transitions become easier in the future).
There was a problem hiding this comment.
I would also be fine to make this a RECOMMENDED
| : A set of one or more claims about a subject, provided by a Credential Issuer. | ||
|
|
||
| Credential Dataset Version | ||
| : A String that refers to a specific version of a Credential Dataset. This version is identical for multiple instances of a Credential that share the same Credential Dataset, even when the Credential instances differ in cryptographic data, e.g., an Issuer signature. When any of the claim values in the Credential Dataset change, a new Credential Dataset Version is assigned. Note that a Credential Dataset Version is bound to a specific Credential Format. |
There was a problem hiding this comment.
It's not just cryptographic data, also other security claims. e.g. exp, iat, cnf etc.
There was a problem hiding this comment.
yep, i thought about this as well but we are using cryptographic data for these things. imo, we should have a term definition of cryptographic data, or credential metadata that includes these things. would it be ok to have a separate PR for this? we use the term cryptographic data in the batch issuance endpoint already. and there we define only three categories: format, dataset and cryptographic data which should be reworked a bit for clarity. i suggest to do that in the same PR.
There was a problem hiding this comment.
I'm pretty sure I've seen some language similar to "security-related claims" in other specs, but I can't seem to find it right now. Would be nice to have a standard term.
There was a problem hiding this comment.
Here it gets tricky, because for ISO mdocs this would essentially be all data contained in the MSO, right? We should think about the right term definitions and keep it very broad in this PR.
There was a problem hiding this comment.
I updated this PR to mention timestamps but created this issue to discuss term definitions: #752
| * `interval`: REQUIRED if `transaction_id` is present. Contains a positive number that represents the minimum amount of time in seconds that the Wallet SHOULD wait after receiving the response before sending a new request to the Deferred Credential Endpoint. It MUST NOT be used if the `credentials` parameter is present. | ||
| * `notification_id`: OPTIONAL. String identifying one or more Credentials issued in one Credential Response. It MUST be included in the Notification Request as defined in (#notification). It MUST not be used if the `credentials` parameter is not present. | ||
| * `credential_metadata`: OPTIONAL. Object that contains additional metadata specific to the issued Credential(s). The definitions and contained parameters for this Object are identical to the `credential_metadata` parameter as defined in Credential Issuer Metadata (see (#credential-issuer-metadata)) See (#display-metadata-considerations) for implementation considerations on credential metadata. | ||
| * `credential_dataset_version`: REQUIRED for the Issuer to return. A string containing the Credential Dataset Version associated with the returned Credential(s). This allows Wallets to detect changes to the underlying Credential Dataset across different Credential Responses. The Wallet MUST NOT expect the `credential_dataset_version` parameter to always be present in the Credential Response for backward compatibility with Issuers conforming to earlier versions of this specification. See (#credential-dataset-version-implementation) for implementation considerations. |
There was a problem hiding this comment.
I agree that the spec is technically correct, but I also agree that this will confuse people. In principle I'm OK with making it REQUIRED for the issuer to return, but we should try to write the compatibility purpose in a clearer way.
| * `credential_metadata`: OPTIONAL. Object that contains additional metadata specific to the issued Credential(s). The definitions and contained parameters for this Object are identical to the `credential_metadata` parameter as defined in Credential Issuer Metadata (see (#credential-issuer-metadata)) See (#display-metadata-considerations) for implementation considerations on credential metadata. | ||
| * `credential_dataset_version`: REQUIRED for the Issuer to return. A string containing the Credential Dataset Version associated with the returned Credential(s). This allows Wallets to detect changes to the underlying Credential Dataset across different Credential Responses. The Wallet MUST NOT expect the `credential_dataset_version` parameter to always be present in the Credential Response for backward compatibility with Issuers conforming to earlier versions of this specification. See (#credential-dataset-version-implementation) for implementation considerations. | ||
|
|
||
| If the Credential Issuer includes the `credential_dataset_version` parameter, the following requirements apply: |
There was a problem hiding this comment.
This sentence contradicts the REQUIRED from above?
There was a problem hiding this comment.
Can you clarify? Is this the same comment as here #472, or is this a new point?
There was a problem hiding this comment.
I think Paul means that line 1396 says "If the Credential Issuer ..." but now it is REQUIRED for the Issuer to do so. So the sentence should be rewritten to something like:
| If the Credential Issuer includes the `credential_dataset_version` parameter, the following requirements apply: | |
| The following requirements apply to the Credential Dataset Version: |
There was a problem hiding this comment.
I think the current text is still correct because the requirements only apply if the version was provided. The larger question is whether it is optional or required for 1.1 issuers. Perhaps we can discuss in the DCP call.
|
|
||
| If the Credential Issuer includes the `credential_dataset_version` parameter, the following requirements apply: | ||
|
|
||
| * For a given Credential Dataset within the scope of a concrete Credential Format, if the Credential Dataset has not changed, the Credential Issuer MUST return the same Credential Dataset Version, even when issuing a new Credential instance with different cryptographic data, e.g., an Issuer signature. |
There was a problem hiding this comment.
This text is missing the connection to credential_instance returned from Token Response.
There was a problem hiding this comment.
You mean credential_identifier from the token response (aka credential instance). This makes sense.
There was a problem hiding this comment.
I think "credential instance" may not be the best term, as multiple credential instances can correspond to the same credential identifier, credential configuration, and credential dataset, differing only in their cryptographic proof. Btw, we need a term definition for credential instance as well.
| * For a given Credential Dataset within the scope of a concrete Credential Format, if the Credential Dataset has not changed, the Credential Issuer MUST return the same Credential Dataset Version, even when issuing a new Credential instance with different cryptographic data, e.g., an Issuer signature. | |
| * For a given Credential Dataset as identified by the `credential_identifier`, if provided in the Token Response, within the scope of a concrete Credential Format, if the Credential Dataset has not changed, the Credential Issuer MUST return the same Credential Dataset Version, even when issuing a new Credential instance with different cryptographic data, e.g., an Issuer signature. |
paulbastian
changed the title
| * `interval`: REQUIRED if `transaction_id` is present. Contains a positive number that represents the minimum amount of time in seconds that the Wallet SHOULD wait after receiving the response before sending a new request to the Deferred Credential Endpoint. It MUST NOT be used if the `credentials` parameter is present. | ||
| * `notification_id`: OPTIONAL. String identifying one or more Credentials issued in one Credential Response. It MUST be included in the Notification Request as defined in (#notification). It MUST not be used if the `credentials` parameter is not present. | ||
| * `credential_metadata`: OPTIONAL. Object that contains additional metadata specific to the issued Credential(s). The definitions and contained parameters for this Object are identical to the `credential_metadata` parameter as defined in Credential Issuer Metadata (see (#credential-issuer-metadata)) See (#display-metadata-considerations) for implementation considerations on credential metadata. | ||
| * `credential_dataset_version`: REQUIRED for the Issuer to return. A string containing the Credential Dataset Version associated with the returned Credential(s). This allows Wallets to detect changes to the underlying Credential Dataset across different Credential Responses. Since Wallets might interact with Credential Issuers conforming to earlier versions of this specification that omit the `credential_dataset_version` parameter, Wallets MUST NOT rely on the parameter being present in every Credential Response. See (#credential-dataset-version-implementation) for implementation considerations. |
There was a problem hiding this comment.
Rethinking this, I would favor this to be RECOMMENDED. If the Issuer does not have a good way to determine this, he would omit the parameter and the Wallet should throw away old batches. This makes it more backwards compatible imo.
9 participants
Fixes #278