723 duplicate claims entry

[c2bo]

Closes #723

Open question from the issue. SHOULD -> MUST?

[martijnharing]

Alternatively to the solution proposed in this PR, what if we remove the restrictions on the RP and requirements on the wallet to check anything related to duplicates?
It feels like there isn't a direct security or privacy problem without those requirements / recommendations and it would mean we don't need to define what we mean exactly with duplicate.

[fkj]

I agree with @martijnharing that it might be better to just remove this text. Is there a specific argument for this text to exist?

[paulbastian]

Alternatively to the solution proposed in this PR, what if we remove the restrictions on the RP and requirements on the wallet to check anything related to duplicates? It feels like there isn't a direct security or privacy problem without those requirements / recommendations and it would mean we don't need to define what we mean exactly with duplicate.

Despite any personal opinion, removing the entire text about duplicate entries is likely a breaking change.

[paulbastian]

I think it adds clarity while avoiding a breaking change.

[fkj]

I'm OK with doing this to clarify without a breaking change, though it's still not clear to me why this text exists in the first place.

[jogu]

I'm still not sure what this means for this case, does it mean:

claims = [
  { id = 1, path = [ "nationalities", 0 ] }, 
  { id = 2, path = [ "nationalities", null ] }
}

is treated as a duplicate, i.e. '2' is discarded?

The latest change causes a change in the behaviour for this query:

claims = [
  {  path = [ name ], values = [ oliver ] }, 
  {  path = [ name ], values = [ colette ] }
]

I think it's gone from 'ignore the 'collete' line' (in 1.0 final) to 'process that line but the result is a query that can never match a credential'.

I kind of feel like we're going the wrong direction down a rabbit hole, but I'm not sure what the right direction is.

My suspicion is the original text was really only trying to say that the second line here has no meaning:

claims = [
  {  path = [ name ] }, 
  {  path = [ name ] }
]

[martijnharing]

Alternatively to the solution proposed in this PR, what if we remove the restrictions on the RP and requirements on the wallet to check anything related to duplicates? It feels like there isn't a direct security or privacy problem without those requirements / recommendations and it would mean we don't need to define what we mean exactly with duplicate.

Despite any personal opinion, removing the entire text about duplicate entries is likely a breaking change.

I'm not sure if it's actually a breaking change.

It would allow an RP to include a duplicate in the request, but I don't think that actually breaks anything.
If we consider that duplicate entry referred to exact duplicates (e.g. two path strings that are equal and no value elements that are different) then whether you ignore one of them or not doesn't make a difference.

[brentzundel]

WG discussion: let's try removing the lines as recommended by @martijnharing

[martijnharing]

Proposed changing the changelog text, but not gating approval on that.

[brentzundel]

@jogu the PR has been adjusted to match the direction the group felt was best during our last call.
Please review again.

[Sakurann]

@jogu please re-review text you commented on got removed with the new direction