openwallet-foundation / acapy

ACA-Py is a foundation for building decentralized identity applications and services running in non-mobile environments.
https://aca-py.org
Apache License 2.0
421 stars 514 forks source link

WQL is not working for 'marker' option #2652

Open konda-kalyan opened 11 months ago

konda-kalyan commented 11 months ago

The WQL {"attr::Name::marker": "1"} is not working when we use with GET /credentials API. From below line of code (commented line), it seems 'marker' info is not getting stored wallet while storing Credentials.

https://github.com/hyperledger/aries-cloudagent-python/blob/main/aries_cloudagent/anoncreds/holder.py#L235

andrewwhitehead commented 11 months ago

The marker tag is not needed on Askar because the $exist filter can be used to check for the presence of an attribute: https://github.com/hyperledger/aries-cloudagent-python/blob/cd4f1dc8fddc1194e0abc00ef4fb3d671745ad51/aries_cloudagent/anoncreds/holder.py#L339C23-L339C23

swcurran commented 11 months ago

@andrewwhitehead — can you explain that a bit more? Notably:

Thanks!

konda-kalyan commented 11 months ago

@andrewwhitehead : Could you give JSON WQL for to retrieve the all Credentials which have 'Name' as one of the attribute. I tried with $exist tag in different ways, but couldn't find the right way to use to get expected results.

I am planning to use this in GET /credentials and /present-proof/records/{pres_ex_id}/credentials. Hope, same WQL works in both the APIs.

stockschlaeger commented 10 months ago

I also get an error if I use the marker field on the /present-proof/send-request endpoint with acapy 0.7.5

"proof_request": { "requested_attributes": { "0_Participant": { "value": { "restrictions": { "0": { "attr::XXX::marker": { "key": [ "String does not match expected pattern."

swcurran commented 10 months ago

Per this comment: https://github.com/hyperledger/aries-cloudagent-python/issues/1257#issuecomment-1895873036

The answer is that in ACA-Py restrictions to use:

Thanks @stockschlaeger for finding/noting that. I'll get this added to the documentation at some point.

swcurran commented 10 months ago

"At some point" meaning in the best place I can find to put it. I'll make the PR in the next day or two. :-)

swcurran commented 10 months ago

And updating the credit -- it was @pstuermlinger that noted the solution. Sorry about that!

pstuermlinger commented 10 months ago

The credit for @stockschlaeger is ok since we're working on the same project and it was his finding :) However, the lower-case solution works only when using aca-py as verifier and the Lissi Wallet as holder. If you're using aca-py as holder as well then the attribute must be written exactly as in the schema definition.

The anoncred specification lacks a clear rule on that. For name and names it says it's case insensitive. For the restriction part it says nothing on that matter.

Besides that I think the marker issue is not related to the attribute value restriction. The former should filter for the existence of an attribute while the latter filters for a specific value.