Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: enabled handling VPs (request, creation, verification) with different VCs #1956

Merged
merged 17 commits into from
Dec 1, 2022

Conversation

teanas
Copy link
Contributor

@teanas teanas commented Sep 22, 2022

  • Enabled handing presentation requests with indy and dif

  • Enabled handling presentation requests with requests for claims from different JSON-LD VCs (verifier has to create separate submission requirement groups for each claim from different VC) In case, multiple claims should come from the same VC, the descriptors can be included in the same group

  • Updated demo/agent_controller to correctly construct presentations.

Signed-off-by: Anastasiia Sivirina [email protected]

@hkny
Copy link

hkny commented Sep 22, 2022

@swcurran Could we take this PR in the agenda of the upcoming Aca-Pug Call? Thanks and best

@swcurran
Copy link
Contributor

Yes -- super interested. @teanas are you working with @hkny ? Definitely would like to get a more detailed background about what is in here. From reading the description -- does this mean Indy proofs via a DIF Presentation Exchange?

We can have this on the Agenda for the first Tuesday in October -- the next ACA-Pug meeting.

Thanks!!!!

@swcurran
Copy link
Contributor

It would be great to get the branch updated and the SonarCloud issues looked at, please.

@teanas teanas changed the title Feature: enabled handling VPs /request, creation, verification) with different VCs Feature: enabled handling VPs (request, creation, verification) with different VCs Sep 23, 2022
@hkny
Copy link

hkny commented Sep 23, 2022

@swcurran this is correct. It's all @teanas' work but yes, we are working together :) The goal was to combine AnonCreds with W3C VCs in Presentation Proofs. Some combinations were not possible and those should be fixed with this PR without creating breaking changes to the protocol .

@codecov-commenter
Copy link

codecov-commenter commented Sep 25, 2022

Codecov Report

Merging #1956 (de84288) into main (6629d7d) will increase coverage by 0.00%.
The diff coverage is 98.55%.

@@           Coverage Diff           @@
##             main    #1956   +/-   ##
=======================================
  Coverage   93.49%   93.50%           
=======================================
  Files         539      539           
  Lines       34637    34666   +29     
=======================================
+ Hits        32383    32413   +30     
+ Misses       2254     2253    -1     

Signed-off-by: Anastasiia <[email protected]>
@teanas
Copy link
Contributor Author

teanas commented Sep 28, 2022

@swcurran for some reason SonarQube did not start a new analysis after my latest commit. Can I somehow manually trigger the analysis?

@swcurran
Copy link
Contributor

I had to manually trigger the other tests, but I'm still not seeing the SonarQube analysis -- not sure why. Any idea @WadeBarnes ?

@swcurran
Copy link
Contributor

Looks like there is some code formatting to clean up, which will result in another commit, so you can try again.

Signed-off-by: Anastasiia <[email protected]>
@teanas
Copy link
Contributor Author

teanas commented Sep 28, 2022

The 1 security hotspot refers to the usage of a path for a mock object, which has been previously used in other test cases as well. Is this hotspot safe if there are old tests containing the same structure? E. g., test_create_presentation.

@teanas
Copy link
Contributor Author

teanas commented Oct 9, 2022

@swcurran Are there any checks missing?

Signed-off-by: Anastasiia <[email protected]>
@swcurran
Copy link
Contributor

Sorry @teanas that the checks aren't running automagically due to a first commit -- I'll make sure I monitor them and run them with each of your commits.

Signed-off-by: Anastasiia <[email protected]>
@teanas
Copy link
Contributor Author

teanas commented Oct 11, 2022

The failing scenario in acapy-integration-tests is about issuing a credential and I haven't added any changes to the issuance process (failed scenario: features/0453-issue-credential.feature:57). The exception is ledger related and appears during create_schema_and_cred_def/create_and_send_credential_definition neither of which I altered. @swcurran do you have an idea what might be the problem?

7hAcme.agent | 2022-10-10 19:25:10,919 aries_cloudagent.ledger.base WARNING Credential definition KSKnzboJ3zTeZVHobSkYQh:3:CL:339254:Acme.agent.Schema_DriversLicense already exists on ledger default
7hAcme.agent | 2022-10-10 19:25:10,921 aries_cloudagent.admin.server ERROR Handler error with exception: Credential definition KSKnzboJ3zTeZVHobSkYQh:3:CL:339254:Acme.agent.Schema_DriversLicense is on ledger default but not in wallet acme.agent716296
7hAcme.agent | Traceback (most recent call last):
7hAcme.agent | File "/home/indy/aries_cloudagent/messaging/credential_definitions/routes.py", line 262, in credential_definitions_send_credential_definition
7hAcme.agent | endorser_did=endorser_did,
7hAcme.agent | File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/asyncio/futures.py", line 327, in iter
7hAcme.agent | yield self # This tells Task to wait for completion.
7hAcme.agent | File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/asyncio/tasks.py", line 250, in _wakeup
7hAcme.agent | future.result()
7hAcme.agent | File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/asyncio/futures.py", line 243, in result
7hAcme.agent | raise self._exception
7hAcme.agent | File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/asyncio/tasks.py", line 180, in _step
7hAcme.agent | result = coro.send(None)
7hAcme.agent | File "/home/indy/aries_cloudagent/ledger/base.py", line 462, in create_and_send_credential_definition
7hAcme.agent | f"Credential definition {credential_definition_id} is on "
7hAcme.agent | aries_cloudagent.ledger.error.LedgerError: Credential definition KSKnzboJ3zTeZVHobSkYQh:3:CL:339254:Acme.agent.Schema_DriversLicense is on ledger default but not in wallet acme.agent716296
7hAcme.agent |
Assertion Failed: FAILED SUB-STEP: Given "Acme" is ready to issue a credential for driverslicense
Substep info: Traceback (most recent call last):
File "/home/indy/demo/runners/support/agent.py", line 850, in admin_request
resp.raise_for_status()
File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/aiohttp/client_reqrep.py", line 1010, in raise_for_status
headers=self.headers,
aiohttp.client_exceptions.ClientResponseError: 400, message='Credential definition KSKnzboJ3zTeZVHobSkYQh:3:CL:339254:Acme.agent.Schema_DriversLicense is on ledger default but not in wallet acme.agent716296', url=URL('http://10.1.1.75:8021/credential-definitions')

  The above exception was the direct cause of the following exception:
  
  Traceback (most recent call last):
    File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/behave/model.py", line 1329, in run
      match.run(runner.context)
    File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/behave/matchers.py", line 98, in run
      self.func(context, *args, **kwargs)
    File "features/steps/0453-issue-credential.py", line 42, in step_impl
      version=schema_info["schema"]["schema_version"],
    File "/home/indy/demo/bdd_support/agent_backchannel_client.py", line 102, in aries_container_create_schema_cred_def
      version=version,
    File "/home/indy/demo/bdd_support/agent_backchannel_client.py", line 19, in run_coroutine
      return loop.run_until_complete(coroutine(*args, **kwargs))
    File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/asyncio/base_events.py", line 488, in run_until_complete
      return future.result()
    File "/home/indy/demo/runners/agent_container.py", line 826, in create_schema_and_cred_def
      schema_name, schema_attrs, self.revocation, version=version
    File "/home/indy/demo/runners/agent_container.py", line 629, in create_schema_and_cred_def
      revocation_registry_size=TAILS_FILE_COUNT if revocation else None,
    File "/home/indy/demo/runners/support/agent.py", line 292, in register_schema_and_creddef
      "/credential-definitions", credential_definition_body
    File "/home/indy/demo/runners/support/agent.py", line 951, in admin_POST
      "POST", path, data, text, params, headers=headers
    File "/home/indy/demo/runners/support/agent.py", line 853, in admin_request
      raise Exception(f"Error: {resp_text}") from e
  Exception: Error: 400: Credential definition KSKnzboJ3zTeZVHobSkYQh:3:CL:339254:Acme.agent.Schema_DriversLicense is on ledger default but not in wallet acme.agent716296
  
  Traceback (of failed substep):
    File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/behave/model.py", line 1329, in run
      match.run(runner.context)
    File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/behave/matchers.py", line 98, in run
      self.func(context, *args, **kwargs)
    File "features/steps/0453-issue-credential.py", line 42, in step_impl
      version=schema_info["schema"]["schema_version"],
    File "/home/indy/demo/bdd_support/agent_backchannel_client.py", line 102, in aries_container_create_schema_cred_def
      version=version,
    File "/home/indy/demo/bdd_support/agent_backchannel_client.py", line 19, in run_coroutine
      return loop.run_until_complete(coroutine(*args, **kwargs))
    File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/asyncio/base_events.py", line 488, in run_until_complete
      return future.result()
    File "/home/indy/demo/runners/agent_container.py", line 826, in create_schema_and_cred_def
      schema_name, schema_attrs, self.revocation, version=version
    File "/home/indy/demo/runners/agent_container.py", line 629, in create_schema_and_cred_def
      revocation_registry_size=TAILS_FILE_COUNT if revocation else None,
    File "/home/indy/demo/runners/support/agent.py", line 292, in register_schema_and_creddef
      "/credential-definitions", credential_definition_body
    File "/home/indy/demo/runners/support/agent.py", line 951, in admin_POST
      "POST", path, data, text, params, headers=headers
    File "/home/indy/demo/runners/support/agent.py", line 853, in admin_request
      raise Exception(f"Error: {resp_text}") from e

@swcurran
Copy link
Contributor

@ianco -- can you please take a look at this test? Thanks!

@ianco
Copy link
Contributor

ianco commented Oct 11, 2022

@ianco -- can you please take a look at this test? Thanks!

It runs fine on my local, sometimes tests fail randomly :-(

ianco
ianco previously approved these changes Oct 11, 2022
@teanas
Copy link
Contributor Author

teanas commented Oct 26, 2022

@ianco @swcurran Is there anything else that needs to be done before the merge? :)

@hkny
Copy link

hkny commented Oct 26, 2022

Hi @teanas,

please add the multicredential.md in the root folder of the project and add the following description (copy/paste)

Multi-Credentials

It is a known fact that multiple AnonCreds can be combined to present a presentation proof with an "and" logical operator: For instance, a verifier can ask for the "name" claim from an eID and the "address" claim from a bank statement to have a single proof that is either valid or invalid. With the Present Proof Protocol v2, it is possible to have "and" and "or" logical operators for AnonCreds and/or W3C Verifiable Credentials.

With the Present Proof Protocol v2, verifiers can ask for a combination of credentials as proof. For instance, a Verifier can ask a claim from an AnonCreds and a verifiable presentation from a W3C Verifiable Credential, which would open the possibilities of Aries Cloud Agent Python being used for rather complex presentation proof requests that wouldn't be possible without the support of AnonCreds or W3C Verifiable Credentials.

Moreover, it is possible to make similar presentation proof requests using the or logical operator. For instance, a verifier can ask for either an eID in AnonCreds format or an eID in W3C Verifiable Credential format. This has the potential to solve the interoperability problem of different credential formats and ecosystems from a user point of view by shifting the requirement of holding/accepting different credential formats from identity holders to verifiers. Here again, using Aries Cloud Agent Python as the underlying verifier agent can tackle such complex presentation proof requests since the agent is capable of verifying both type of credential formats and proof types.

In the future, it would be even possible to put mDoc as an attachment with an and or or logical operation, along with AnonCreds and/or W3C Verifiable Credentials. For this to happen, Aca-Py either needs the capabilities to validate mDocs internally or to connect third-party endpoints to validate and get a response.


After that I think this is ready to merge. Thanks again.

@hkny
Copy link

hkny commented Nov 3, 2022

Hello,
@ianco @swcurran Can a maintainer approve the workflows and review the PR? I think its ready to go.

@swcurran
Copy link
Contributor

swcurran commented Nov 3, 2022

Sorry for the delay -- this is on @andrewwhitehead 's list for review. We'd like to get it completed.

@hkny
Copy link

hkny commented Nov 15, 2022

Hi,
@andrewwhitehead Could you please take a look at the PR? I would like to get it merged soon.
Thanks

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@swcurran swcurran merged commit 043d8c1 into openwallet-foundation:main Dec 1, 2022
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.

6 participants