Skip to content

Conversation

@tplooker
Copy link
Contributor

@tplooker tplooker commented Oct 15, 2024

As per #266 (comment) I also believe that optionality at the credential level as well as the claims level leads to difficult UX challenges and doesn't provide much value.

What I propose we do is elevate things so there is in effect only one level of optionality that a wallet would need to communicate to a user about, which is at the "credential set query" level (not sold on that name I believe "usecase" might be a better descriptor here).

The main things I have done in this PR is

  1. Remove optionality at the claim and credential level
  2. Expanded the structure at the credential_sets level to handle optional queries instead.

The easiest way to grok the difference from a syntax perspective IMO is through modification of the credentials_alternatives.json example

Note - I've included the purpose property mainly to help in understanding the examples, it hasn't been defined in spec text and we will likely want to remove it (if/before) this PR is to be merged or shift away from a free text field as way of conveying purpose.

@tplooker tplooker force-pushed the tl/vp-query-language-3+ branch 2 times, most recently from 755035f to 8caad42 Compare October 15, 2024 05:15
@tplooker tplooker force-pushed the tl/vp-query-language-3+ branch from 8caad42 to d23be71 Compare October 15, 2024 09:33
@jogu
Copy link
Collaborator

jogu commented Oct 15, 2024

I'm not clear how the use cases I mentioned in #266 (comment) would work here.

What I propose we do is elevate things so there is in effect only one level of optionality that a wallet would need to communicate to a user about, which is at the "credential set query" level (not sold on that name I believe "usecase" might be a better descriptor here).

I think this actually makes some cases harder, by forcing them to happen at credential selection time. In particular in the browser API because the credential selection has already happened by the time the wallet gets invoked, it seems like it leaves the wallet with nothing it needs to communicate with the user about as the only choice at that stage is "release selected credentials or don't". Trying to process the approach in this PR so that responsibility is divided between the wallet and the system credential selector actually feels harder for the wallet to me than the current approach.

I'm personally currently finding the "select credentlals then deal with what claims will be shared" model in the existing PR mentally easier as well as being more expressive / covering more use cases.

@tplooker
Copy link
Contributor Author

I've included an example here #266 (comment)

@Sakurann
Copy link
Collaborator

discussed on the wg call. i think the asks were:

  • please keep discussions on this topic (UX issue, optionality, etc.) in this PR so that there is one place to look for
  • please update the rationale/context for this proposed change
  • please change the syntax so that no optionality is allowed at all at the claims level - currently it still does
  • please update the examples to show how existing examples can still be represented with this new proposal

@tplooker
Copy link
Contributor Author

Post the WG call this morning I've done the following

  • Added text around how the claim_sets array is to be processed, clarifying that its not an array of options that the user is expected to decide between, instead the wallet is to pick the highest preference option it can satisfy. Simply removing the claim_sets functionality would mean alternative claims become impossible to convey.
  • Removed the "purpose" attribute for now
  • Examples have been refined to show how it works

@tplooker
Copy link
Contributor Author

tplooker commented Oct 15, 2024

Additionally here is some more detailed rationale for this change

With the syntax in the original proposal optionality is expressed at both the claim and credential level, which leads to several UX challenges.

The main issue boils down to how complex the consenting process would have to be in order to support optionality at both levels. For example optional claims would likely mean three possible different options

  1. The wallet makes a blanket rule to not support optional claims and doesn't even render an option to the user, rejecting all user claims - which is a critical loss of functionality.
  2. The wallet makes a blanket rule to support all optional claims and return them always without rendering an option to the user, which is potential privacy issue (hopefully wallets wouldn't do this in practise).
  3. The wallet invests in a complex consent UX that allows a user to toggle optional claims in the consent process, arguably this is a poor user experience and critically it would lack conveying the context to the user as to for why these claims are being optionally requested AND perhaps how some of the optional claims relate to one and other.

Instead if we step back and consider when a verifier is making a request for some claims that are optional to return, the group of optional claims being requested likely has a purpose (in the case there is more then one) and being able to group these optional claims and in future attach a purpose to that request is a useful UX simplification.

Requests can shift from the framing of

I want your "first_name", "last_name" and "birth_date" and optionally your "resident_address", "resident_state" and "resident_country" attributes from your mDL. Which could lead to any combo of returned attributes such as

Possible Response 1 "first_name", "last_name", "birth_date", "resident_address", "resident_state" and "resident_country"
Possible Response 2 "first_name", "last_name", "birth_date", "resident_address", and "resident_state"
Possible Response 3 "first_name", "last_name", "birth_date", "resident_address"
Possible Response 4 "first_name", "last_name", "birth_date", "resident_state"
Possible Response 5 "first_name", "last_name", "birth_date", "resident_country"
etc....

Which if the verifier needed all three of "resident_address", "resident_state" and "resident_country" attributes to prove address, all but the first example response would be useless and actually result in a privacy issue because additional information (a partial address) could have been revealed to the verifier for no reason.

To the framing of

I want "proof of your ID" which involves sharing the "first_name", "last_name" and "birth_date" from your mDL AND optionally I want "proof of your address" which involves sharing the "resident_address", "resident_state" and "resident_country" attributes from your mDL.

Where the only two possible successful responses are

Possible Response 1 "first_name", "last_name", "birth_date", "resident_address", "resident_state" and "resident_country"

OR

Possible Response 2 "first_name", "last_name", "birth_date"

@tplooker tplooker changed the title Proposed alterations to how optionality is handled in Dr Fetts query language proposal Proposed alterations to how optionality is handled in query language (Approach 3) Oct 16, 2024
@David-Chadwick
Copy link
Contributor

I will argue that the revised reframing I want "proof of your ID" which involves sharing the "first_name", "last_name" and "birth_date" from your mDL AND optionally I want "proof of your address" which involves sharing the "resident_address", "resident_state" and "resident_country" attributes from your mDL. is insufficient on its own as the user has no idea why the optional attributes are being requested. Thus the reframing must include the purpose (i.e. extended service) that is being offered.

I want "proof of your ID" which involves sharing the "first_name", "last_name" and "birth_date" from your mDL AND optionally I want "proof of your address" so that we can post you a receipt which involves sharing the "resident_address", "resident_state" and "resident_country" attributes from your mDL.

In addition the original service could be included as the purpose for requesting the mandatory attributes, but this may not be necessary if we can assume that the user already knows the service that s/he is requesting.

{"id": "e", "path": ["date_of_birth"]},
{"id": "f", "path": ["email"]}
],
"claim_sets": [ // defines the rules
Copy link
Contributor

Choose a reason for hiding this comment

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

@tplooker I think you wanted to remove claim_sets? For a fair comparison, please modify all examples so that it is not being used any longer.

],
"claim_sets": [
["current", "legacy?"]
["current"]
Copy link
Contributor

Choose a reason for hiding this comment

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

As long as claim_sets is preserved, the ? semantic is still here, but hidden. Either we remove claim_sets or we should keep ? as an option.

Comment on lines +23 to +25
["a", "b", "e", "f?!"],
["a", "c", "d", "e", "f?!"]
// Note: the ?! to mark a claim as `required if present` is syntactical sugar; the rules could be rewritten without it, but that would require a lot of repetition
Copy link
Collaborator

Choose a reason for hiding this comment

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

so this basically allows optionality around claim "c", even without ? syntax. so if we are removing optionality at the claim level, entireclaim_sets needs to be removed

},
"claims": [
{
"id": "given_name",
Copy link
Contributor

@paulbastian paulbastian Oct 17, 2024

Choose a reason for hiding this comment

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

id is unnecessary?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree.

Copy link
Collaborator

@tlodderstedt tlodderstedt 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 the claim_set can and should be dropped. The combinations can be expressed on the ' credentials' level. That is sufficient and would simplify the syntax.

},
"claims": [
{
"id": "given_name",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree.

"claims": [
{
"id": "given_name",
"namespace": "org.iso.18013.5.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

add flag "provideIfAvailable" : true to imitate Daniel's ?! feature?

@Sakurann Sakurann dismissed tlodderstedt’s stale review October 17, 2024 16:10

Torsten verbally agreed to merge this PR and review another PR once daniel applies torsten's suggested changes

@Sakurann Sakurann merged commit 9cd1bbb into danielfett/vp-query-language-3 Oct 17, 2024
@Sakurann
Copy link
Collaborator

Agreed on the WG call that in a new query language, we will:

  1. remove claim_sets
  2. introduce equivalent of ?! in credentials object (provide_if_available or something)
  3. change credential_sets to array of objects
  4. add purpose to the object in credential_sets array - it can be a string or an integer, but it has to be shown only if RP is authenticated. (from processing, probably easier to defined purpose_code that is an integer separately? shouldn't matter, we can discuss in the PR)

for the sake of making progress, merging this PR and @danielfett will apply full set of these changes in his PR.

no consensus on value matching.

Claims postfixed by `?!` have to be provided by the Wallet if they exist on
the matched Credential. The order of the options conveyed in the `claim_sets`
array expresses the Verifiers preference for what is returned. A wallet MUST
prioritise returning the most preferred option.
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
prioritise returning the most preferred option.
prioritize returning the most preferred option.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants