OpenID4VCI icon indicating copy to clipboard operation
OpenID4VCI copied to clipboard

Define claims display description and claims path query

Open danielfett opened this issue 1 year ago • 8 comments

Mainly fixes #266 based on https://vcstuff.github.io/sd-jwt-vc-types/danielfett/fix-claims/draft-fett-oauth-sd-jwt-vc-types.html#section-8

Also Fixes #272 Also Fixes #271

danielfett avatar Feb 20 '24 18:02 danielfett

So our preference is actually defining the new property nested which may contain descriptions about nested objects (original approach 2).

Then we need a new PR for that.

As a compromise for the existing implementations, my suggestion would be fixing the text that becomes ID-1 with nested approach. and than after ID-1, incorporating the array approach defined in this PR.

That would mean two breaking changes.

danielfett avatar Feb 23 '24 10:02 danielfett

As a compromise for the existing implementations, my suggestion would be fixing the text that becomes ID-1 with nested approach. and than after ID-1, incorporating the array approach defined in this PR.

That would mean two breaking changes.

Is it suggested to incorporate a temporary solution (the nested approach) into ID1, even though it may be replaced in the future with another solution (the "array" approach)? Is this a proposal to improve the specification, or a suggestion to ease the workload of developers working on ID1 in a specific company? If the answer is the latter, it is a significant concern for the community.

TakahikoKawasaki avatar Feb 23 '24 17:02 TakahikoKawasaki

It's a suggestion to improve interoperability among the companies that have already implemented the current specification text, of which there are quite a few, and which are part of the community, while improving the specification.

Sakurann avatar Feb 23 '24 18:02 Sakurann

I think we should have the discussion about what we want to prioritize: having a cleanest solution possible, or having a solution that builds up on what has already been implemented.

Sakurann avatar Feb 27 '24 14:02 Sakurann

@awoie Mattr has implementation of VCI in production. You are preferring a larger breaking change from what you have?

Sakurann avatar Feb 27 '24 15:02 Sakurann

I think the PR would bring a significant improvement to the VCI spec. However, I would like to point that it is a significant breaking change. Merging this PR now will make existing implementations incompatible and require us to restart the ID1 review process.

I would like to understand what existing VCI implementers think and whether they support the change being merged now.

The alternative would be to fix the most important issues brought up in https://github.com/openid/OpenID4VCI/issues/266 by allowing the key words mandatory, value_type, and display to be differentiated from ordinary claim names by adding a prefix _, for example. The text could also be changed to allow for the key words, at least display, in objects.

That might not be as nice and clean as the change proposed in this PR, but it could be specified in a backward compatible manner and would allow us to continue the ID process.

tlodderstedt avatar Feb 27 '24 16:02 tlodderstedt

How about not replacing the claims object but instead having a different parameter name for the new object. That would allow for better transition as Issuers could support both old and new format. We could leave a statement that old format will be deprecate with ID-2? This would give Wallet Providers some months to support the new structure and roll out updates.

paulbastian avatar Feb 27 '24 18:02 paulbastian

agreed to come back to this after agreeing on the new query language design first, since there is an overlap with the new query language (#266) how to clarify how to specify which claim (uses path rn)

Sakurann avatar Sep 19 '24 15:09 Sakurann

Todos for me:

  • [x] Ensure that the mso_mdoc format uses the namespace/claim_name syntax as DCQL does
  • [x] Check if the text for the claims path queries is the same as in DCQL
  • [x] Make the change described by Paul here: https://github.com/openid/OpenID4VCI/pull/276#discussion_r1499627153

danielfett avatar Nov 22 '24 13:11 danielfett

Updated, please re-review.

danielfett avatar Nov 26 '24 16:11 danielfett

I would prefer to exclude the value_type, as it seems under-specified and is not present in any example. Otherwise, I'd be ready to approve.

Done

danielfett avatar Dec 12 '24 16:12 danielfett