Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add intent to retain to claims query in DCQL #338

Merged
merged 7 commits into from
Jan 23, 2025
Merged

Conversation

Sakurann
Copy link
Collaborator

resolves #321

@Sakurann Sakurann changed the title add intent to retain to claims query add intent to retain to claims query in DCQL Nov 20, 2024
@Sakurann Sakurann requested a review from danielfett November 21, 2024 15:39
@Sakurann Sakurann added the ISO? label Nov 28, 2024
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'm not in favor of adding this and I think there have been good reasons/argumentations in the Issue. In short, the feature as-is is under-specified and likely of not much use and it seems more reasonable to convey this via the purpose field.

@Sakurann
Copy link
Collaborator Author

WG discussion: agreement that this is not perfect (PR text should be improved), but probably good enough as a starting point to meet the requirements and have a transition path for those who us intent_to_retain in 18013-5/PE.

@nemqe
Copy link
Contributor

nemqe commented Dec 19, 2024

A potential issue I see is that if this option is added in this form it will probably need to be maintained in the version it is introduced as it could be a breaking change to remove it. (maybe a parallel mechanism can be introduced that could be marked as preferred in 1.1? something like PEX vs DCQL)

Other than that if the group is short on time and this option is required it may be a good compromise until a better mechanism is found.

@Sakurann Sakurann added ISO_VirtualMeeting relevant for ISO OID4VP mdoc profile over DC API and removed ISO? labels Jan 13, 2025
@Sakurann Sakurann requested a review from paulbastian January 14, 2025 13:13
@Sakurann
Copy link
Collaborator Author

if this option is added in this form it will probably need to be maintained in the version it is introduced as it could be a breaking change to remove it

the feature is optional. I don't think we plan to remove it, but if we come up with a better mechanism in v1.1 in the future, we can recommend that feature over intent_to_retain.

another option I considered when doing PR is to add intent_to_retain only to mdocs, but that did not feel in sprit of the specification, or the discussions we have been having..

@Sakurann Sakurann added this to the Final 1.0 milestone Jan 20, 2025
Co-authored-by: Stefan Charsley <[email protected]>
Copy link
Member

@peppelinux peppelinux left a comment

Choose a reason for hiding this comment

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

good one!

Copy link
Member

@c2bo c2bo left a comment

Choose a reason for hiding this comment

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

I am still not fully convinced that this feature on its own makes sense, but as it is an optional credential format specific feature that just matches the current capabilities in the ISO protocols, I guess it is fine to merge.

We can (and probably should imho) provide another solution in the direction of purpose in the long run.

@awoie awoie self-requested a review January 23, 2025 11:37
Copy link
Contributor

@awoie awoie left a comment

Choose a reason for hiding this comment

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

IMO, this would cause interop issues. I know that certain members are in favor of providing some guidance to the user but it would cause confusion if this was done only for mdocs.

@awoie awoie self-requested a review January 23, 2025 11:39
@Sakurann Sakurann dismissed paulbastian’s stale review January 23, 2025 16:06

verbal approval to dismiss during the wg call

@Sakurann Sakurann merged commit b693582 into main Jan 23, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ISO_VirtualMeeting relevant for ISO OID4VP mdoc profile over DC API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add intent_to_retain to DCQL
9 participants