search

openid / OpenID4VCI

68 stars 20 forks source link

Define claims display description and claims path query #276

Open danielfett opened 9 months ago

danielfett commented 9 months ago

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 commented 9 months ago

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.

TakahikoKawasaki commented 9 months ago

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.

Sakurann commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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

tlodderstedt commented 9 months ago

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.

paulbastian commented 9 months ago

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.

Sakurann commented 2 months ago

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)

danielfett commented 3 days ago

Todos for me: