Change processing rules to allow same credential fulfilling different queries #492
Conversation
Sakurann
requested review from
GarethCOliver,
TimoGlastra,
awoie,
babisRoutis,
bc-pi,
danielfett,
hlozi,
jogu,
martijnharing,
paulbastian and
tplooker
.DS_Store
Outdated
There was a problem hiding this comment.
shouldn't commit this file
TimoGlastra
left a comment
TimoGlastra
left a comment
There was a problem hiding this comment.
Besides the comment from @charsleysa, this looks good to me, and feels like a simple approach.
|
|
||
| #### Selecting Claims {#selecting_claims} | ||
| When the same Credential fulfills more than one Credential Query in a request, that Credential MUST contain | ||
| the combination of claims requested by those Credential Queries. However, that Credential MUST NOT contain |
There was a problem hiding this comment.
Doesn't this disallow the simple option to make two distinct presentations for the same credential?
There was a problem hiding this comment.
I guess the question would be 'what defines a credential'? Would it be more accurate to talk about this in terms of presentations, rather than credentials:
I.e.
'When the same Credential Presentation fulfills more than one Credential Query in a request, that Credential Presentation MUST contain the combination of claims requested by those Credential Queries. However, that Credential Presentation MUST NOT contain claims not requested by any of the Credential Queries that it fulfills."
That would allow the wallet to make two distinct or a single one.
There was a problem hiding this comment.
good catch, Paul. I was not sure if we wanted to allow "the simple option to make two distinct presentations for the same credential". if we do, Verifier would need to support both cases, which might be harder?
There was a problem hiding this comment.
Yes, but I don't see why we should make wallet implementations harder either. Most verifier implementations may not even check if more data than necessary was delivered
There was a problem hiding this comment.
I don't think it's actually more work for the verifier:
AIUI it is always valid in these cases to answer the different credential queries with different credentials (I don't believe we even have a way to express that two different credential queries must be the same credential, there are arguably use-cases but they are pretty thin).
As a consequence, verifiers must be able to support different credentials presented for the different credential queries (at least to be correct). If they support that, then they will naturally support @paulbastian's desired mechanism (as it is likely indistinguishable from a verifiers perspective). So being permissive for wallets lowers their burden at no interop cost, so seems like win.
(Verifier processing logic is really just: for each queryId verify credential, extract relevant claims from credential to internal format. All this text changes is to prevent Verifiers being overly strict (ones who want to be now have to do some extra work to de-dupe credentials before running their check for unexpected claims, but for those who don't it is no more work.))
There was a problem hiding this comment.
As a consequence, verifiers must be able to support different credentials presented for the different credential queries (at least to be correct). If they support that, then they will naturally support @paulbastian's desired mechanism
that is a fair point.
I'll commit my suggestion below that I believe fulfills @paulbastian's original point?
editorial. accepting as an editor of the spec and since it's a PR on a PR i did
.DS_Store
Outdated
There was a problem hiding this comment.
This file should be removed from the PR
| When the same Credential fulfills more than one Credential Query in a request, that Credential MAY contain | ||
| the combination of claims requested by those Credential Queries. However, in this case, the Credential MUST NOT contain | ||
| claims not requested by any of the Credential Queries. |
There was a problem hiding this comment.
This should go below "selecting claims".
|
This changes a lot of unrelated things, so should probably be rebased and checked again. |
|
WG discussion. |
Co-authored-by: Daniel Fett <mail@danielfett.de>
charsleysa
left a comment
charsleysa
left a comment
There was a problem hiding this comment.
as long as the .DS_Store file is removed
resolves #397
When the same Credential fulfills more than one Credential Query in a request, that Credential MUST contain the combination of claims requested by those Credential Queries. However, that Credential MUST NOT contain claims not requested by either of the Credential Queries.editorial changes