Add failing unit test for multiple "attr:<attr_name>::value" restrictions

[nrempel]

Illustrates the issue define here: https://jira.hyperledger.org/browse/IS-1381

[Artemkaaas]

I did changes to fit this test and sent PR to your repo.
nrempel#2

BUT
The generated proof doesn't contain any evidence that Prover really used a credential that matches to this restriction "attr::sex::value": "male". We can't check it somehow.

[nrempel]

I did changes to fit this test and sent PR to your repo.
nrempel#2

BUT
The generated proof doesn't contain any evidence that Prover really used a credential that matches to this restriction "attr::sex::value": "male". We can't check it somehow.

Thanks!

If I understand correctly, this means that the query will correctly return values for get_credentials_for_proof_request but the verification may fail when it reaches the verifier?

[Artemkaaas]

For your case:

...
"requested_attributes":{
    "attr1_referent":{
        "name":"name",
        "restrictions": json!([{
            "attr::name::value": "Alex",
            "attr::sex::value": "male"
        }])
    }
},

get_credentials_for_proof_request will correctly return credentials.
A proof will be generated successfully.
Verification will pass.
But neither math proof or requested_proof will no contain evidence that credential was used by Prover really met to this restrictions "attr::sex::value": "male". There is no way to check it from Verifier side.

[nrempel]

@Artemkaaas Ok thank you.

It's good that the results from the wallet are correct. I do think that the verification should fail if the verifier cannot prove all of the attribute restrictions are met – but this is another discussion.

Is anything left to merge this fix? We rely on the functionality.

[jovfer]

@Artemkaaas @nrempel

Is anything left to merge this fix?

I'm not sure if the "fix" better rather then original behavior.

As I understand from the ticket description, if multiply restriction are specified, the verification always fails. This PR allow to pass verification by ignoring non-related attributes restriction.

From the libindy consumer point of view returning the success is not so fair action by libindy: application put the restrictions in proof request, but libindy ignore part of them

[jovfer]

my and @nrempel DM:

Potential format suggested by Nicolas:

proof req {
    requestedAttrs: {
        attrXref: { name: X, restrictions: { attr::X::value == valX, attr::Y::value == valY }
        attrYref: { name: Y, restrictions: { attr::X::value == valX, attr::Y::value == valY }
    }
}

vs

possible flow now

proof req {
    requestedAttrs: {
        attrXref: { name: X, restrictions: { attr::X::value == valX }
        attrYref: { name: Y, restrictions: { attr::Y::value == valY }
    }
}

I don't have enough understanding why the first format is useful. My guess for the reason is "in the 2nd case we can't be sure what both attributes are from the same credential.

If it's a correct reason then we should solve the actual problem (no restriction to force the same credential to be used) instead of trying to make a not-always-possible-to-verify restrictions like attr::X::value == valX, attr::Y::value == valY (it's impossible to check if only X is disclosed)

@nrempel what do you think about the ability to force usage of attributes from the same credential and set restrictions similar to current flow (2nd)? like:

proof req {
    requestedAttrs: {
        attrXref: { name: X, restrictions: { attr::X::value == valX, groupedWith == groupRef1 }
        attrYref: { name: Y, restrictions: { attr::Y::value == valY, groupedWith == groupRef1 }
    }
}

[jovfer]

Another option suggested by Nicolas:

proof req {
  requestedAttrs: {
    referent_abc: { names: [X, Y, Z], restrictions: { ... }}
   } 
}

it has kind of breaking changes but they are manageable by an app

[nrempel]

Another option suggested by Nicolas:

proof req {
  requestedAttrs: {
    referent_abc: { names: [X, Y, Z], restrictions: { ... }}
   } 
}

it has kind of breaking changes but they are manageable by an app

If this approach is possible, It's also beneficial for us since it reduces the number of wallet queries we need to make in a large volume scenario.

fyi: @andrewwhitehead @swcurran

[jovfer]

This PR is superseded by #1958

[jovfer]

@nrempel we've merged the PR which allows to have names in proof request, please see updated API comments about new options in JSONs. I'm closing this PR as we have similar tests implemented. Fill free to raise a new PR with more tests, if they will be useful from your point of view

[nrempel]

@nrempel we've merged the PR which allows to have names in proof request, please see updated API comments about new options in JSONs. I'm closing this PR as we have similar tests implemented. Fill free to raise a new PR with more tests, if they will be useful from your point of view

Thanks @jovfer, I'll take a look.