Fixes for credential details in issue-credential webhook responses #1668

andrewwhitehead · 2022-03-14T22:35:05Z

Attempt to fix an issue with blank credential values seen in the test harness.

…ntial 2.0 Signed-off-by: Andrew Whitehead <cywolf@gmail.com>

codecov-commenter · 2022-03-14T22:49:45Z

Codecov Report

Merging #1668 (86f92dd) into main (e77d087) will increase coverage by 0.00%.
The diff coverage is 96.15%.

@@           Coverage Diff           @@
##             main    #1668   +/-   ##
=======================================
  Coverage   95.50%   95.50%           
=======================================
  Files         528      528           
  Lines       32736    32752   +16     
=======================================
+ Hits        31264    31280   +16     
  Misses       1472     1472

swcurran · 2022-03-15T00:41:12Z

AFAIK - this does not fix the problem. The AATH error was introduced with the refactor done on March 2 (12 days ago). It some of the tests were fixed with the change 4 days ago, but many tests are still failing. See this reporting from Allure on the ACA-Py tests: https://allure.vonx.io/api/allure-docker-service/projects/acapy-aip20/reports/latest/index.html

I used current ACA-Py main and Andrew's fork (after a rebuild) with AATH and this command ./manage run -d acapy-main -t @T001.1-HIPE0011 and it errored off as follow (below). It looks like all of the revocation tests are failing, but they seem to be failing on the issuance step, not on revocation. However, I think the "regular" issuance tests are working.

@revocation @AIP20
Feature: RFC 0183 Aries agent credential revocation and revocation notification # features/0183-revocation.feature:2

  Background: create a schema and credential definition in order to issue a credential  # features/0183-revocation.feature:4

  @T001.1-HIPE0011 @normal @AcceptanceTest @Schema_DriversLicense_Revoc @Indy @DIDExchangeConnection @RFC0441
  Scenario Outline: Credential revoked by Issuer and Holder attempts to prove with a prover that doesn't care if it was revoked with a DID Exchange connection -- @1.1   # features/0183-revocation.feature:42
    Given "Acme" has a public did                                                                                                                                        # features/steps/0036-issue-credential.py:38 0.012s
    And "Acme" is ready to issue a credential                                                                                                                            # features/steps/0036-issue-credential.py:58 5.769s
    Given "2" agents                                                                                                                                                     # features/steps/0160-connection.py:22 0.001s
      | name  | role     |
      | Bob   | prover   |
      | Faber | verifier |
    And "Faber" and "Bob" have an existing connection                                                                                                                    # features/steps/0160-connection.py:393 1.954s
    And "Bob" has an issued credential from Acme with Data_DL_MaxValues                                                                                                  # features/steps/0037-present-proof.py:8 38.385s
      Assertion Failed: FAILED SUB-STEP: Given "Bob" has an issued credential from Acme
      Substep info: Assertion Failed: FAILED SUB-STEP: And "Bob" acknowledges the credential issue
      Substep info: Traceback (most recent call last):
        File "/usr/local/lib/python3.7/site-packages/behave/model.py", line 1329, in run
          match.run(runner.context)
        File "/usr/local/lib/python3.7/site-packages/behave/matchers.py", line 98, in run
          self.func(context, *args, **kwargs)
        File "features/steps/0036-issue-credential.py", line 406, in step_impl
          context.cred_rev_id = resp_json["revocation_id"]
      KeyError: 'revocation_id'
      
      Traceback (of failed substep):
        File "/usr/local/lib/python3.7/site-packages/behave/model.py", line 1329, in run
          match.run(runner.context)
        File "/usr/local/lib/python3.7/site-packages/behave/matchers.py", line 98, in run
          self.func(context, *args, **kwargs)
        File "features/steps/0036-issue-credential.py", line 406, in step_impl
          context.cred_rev_id = resp_json["revocation_id"]
      
      Traceback (of failed substep):
        File "/usr/local/lib/python3.7/site-packages/behave/model.py", line 1329, in run
          match.run(runner.context)
        File "/usr/local/lib/python3.7/site-packages/behave/matchers.py", line 98, in run
          self.func(context, *args, **kwargs)
        File "features/steps/0037-present-proof.py", line 123, in step_impl
          context.execute_steps(context_steps)
        File "/usr/local/lib/python3.7/site-packages/behave/runner.py", line 404, in execute_steps
          assert False, message

    When Acme revokes the credential                                                                                                                                     # None
    And "Faber" sends a proof_request_DL_revoc_address presentation to "Bob"                                                                                             # None
    And "Bob" makes the presentation_DL_revoc_address of the proof                                                                                                       # None
    And "Faber" acknowledges the proof                                                                                                                                   # None
    Then "Bob" has the proof verified                                                                                                                                    # None

swcurran · 2022-03-15T00:42:16Z

You can drill into Allure to see all the tests that are failing and where. I think we need to focus on getting back to all the tests running as per before the March 2 PR.

ianco · 2022-03-16T14:09:06Z

The test is failing here because there is no revocation info returned when the credential is created:

https://github.com/hyperledger/aries-agent-test-harness/blob/main/aries-test-harness/features/steps/0036-issue-credential.py#L398

context.cred_rev_id = resp_json["revocation_id"] but resp_json = "{}"

I'll drill down but just in case this rings a bell ...

I tried rewinding aca-py to just before March 2 and this code works (resp_json is the contents of the webhook that is fired after the credential is issued, from the looks of it).

Maybe something in the webhook has changed?

ianco · 2022-03-16T14:39:20Z

For the "old" code the issue credential webhook includes this:

    "initiator": "external",
    "revoc_reg_id": "JDRJc9wcm5i1GrX3dz38RM:4:JDRJc9wcm5i1GrX3dz38RM:3:CL:66:1739:CL_ACCUM:af86a694-ca1d-460e-993b-a05741b6ec76",
    "revocation_id": "1",
    "role": "issuer",
    "schema_id": "JDRJc9wcm5i1GrX3dz38RM:2:Schema_DriversLicense_Revoc:1.1.0",
    "state": "credential_issued",
    "thread_id": "547a23fd-3a04-4c60-8f92-d8b2d053fe12",
    "trace": false,
    "updated_at": "2022-03-16T14:22:03.867251Z"

For the latest code:

    "initiator": "external",
    "role": "issuer",
    "schema_id": "M3yWERSKqTQWomBWNeDWUH:2:Schema_DriversLicense_Revoc:1.1.0",
    "state": "credential_acked",
    "thread_id": "55ff5e3b-549f-4e0e-92b6-b8d568edb996",
    "trace": false,
    "updated_at": "2022-03-16T14:35:23.131590Z"

The revocation info is missing

ianco · 2022-03-16T14:43:31Z

(Confirmed that both cred defs had revocation enabled, and the credentials were issued with a revocation id)

andrewwhitehead · 2022-03-16T15:59:04Z

@ianco I added a potential fix for the missing revocation_id, although rev_reg_id was still present.

swcurran · 2022-03-16T16:03:57Z

Still failing from my AATH test.

ianco · 2022-03-16T16:09:50Z

you can run the AATH test against your aca-py:

check out the AATH repo
edit this file to point to your repo: https://github.com/hyperledger/aries-agent-test-harness/blob/main/aries-backchannels/acapy/Dockerfile.acapy-main
./manage build -a acapy-main
./manage run -d acapy-main -t @T001.1-HIPE0011

ianco · 2022-03-16T16:11:01Z

If you point the requirements-main to this then everything runs fine:

aries-cloudagent[indy, bbs, askar]@git+https://github.com/hyperledger/aries-cloudagent-python@19409c2400b4bee751910ad7eb63bbd569231786

swcurran · 2022-03-16T16:15:48Z

Test output:

Tags:  --tags=@T001.1-HIPE0011

@revocation @AIP20
Feature: RFC 0183 Aries agent credential revocation and revocation notification # features/0183-revocation.feature:2

  Background: create a schema and credential definition in order to issue a credential  # features/0183-revocation.feature:4

  @T001.1-HIPE0011 @normal @AcceptanceTest @Schema_DriversLicense_Revoc @Indy @DIDExchangeConnection @RFC0441
  Scenario Outline: Credential revoked by Issuer and Holder attempts to prove with a prover that doesn't care if it was revoked with a DID Exchange connection -- @1.1   # features/0183-revocation.feature:42
    Given "Acme" has a public did                                                                                                                                        # features/steps/0036-issue-credential.py:38 0.012s
    And "Acme" is ready to issue a credential                                                                                                                            # features/steps/0036-issue-credential.py:58 16.186s
    Given "2" agents                                                                                                                                                     # features/steps/0160-connection.py:22 0.001s
      | name  | role     |
      | Bob   | prover   |
      | Faber | verifier |
    And "Faber" and "Bob" have an existing connection                                                                                                                    # features/steps/0160-connection.py:393 1.955s
    And "Bob" has an issued credential from Acme with Data_DL_MaxValues                                                                                                  # features/steps/0037-present-proof.py:8 32.115s
      Assertion Failed: FAILED SUB-STEP: Given "Bob" has an issued credential from Acme
      Substep info: Assertion Failed: FAILED SUB-STEP: And "Bob" acknowledges the credential issue
      Substep info: Traceback (most recent call last):
        File "/usr/local/lib/python3.7/site-packages/behave/model.py", line 1329, in run
          match.run(runner.context)
        File "/usr/local/lib/python3.7/site-packages/behave/matchers.py", line 98, in run
          self.func(context, *args, **kwargs)
        File "features/steps/0036-issue-credential.py", line 407, in step_impl
          context.rev_reg_id = resp_json["revoc_reg_id"]
      KeyError: 'revoc_reg_id'
      
      Traceback (of failed substep):
        File "/usr/local/lib/python3.7/site-packages/behave/model.py", line 1329, in run
          match.run(runner.context)
        File "/usr/local/lib/python3.7/site-packages/behave/matchers.py", line 98, in run
          self.func(context, *args, **kwargs)
        File "features/steps/0036-issue-credential.py", line 407, in step_impl
          context.rev_reg_id = resp_json["revoc_reg_id"]
      
      Traceback (of failed substep):
        File "/usr/local/lib/python3.7/site-packages/behave/model.py", line 1329, in run
          match.run(runner.context)
        File "/usr/local/lib/python3.7/site-packages/behave/matchers.py", line 98, in run
          self.func(context, *args, **kwargs)
        File "features/steps/0037-present-proof.py", line 123, in step_impl
          context.execute_steps(context_steps)
        File "/usr/local/lib/python3.7/site-packages/behave/runner.py", line 404, in execute_steps
          assert False, message

    When Acme revokes the credential                                                                                                                                     # None
    And "Faber" sends a proof_request_DL_revoc_address presentation to "Bob"                                                                                             # None
    And "Bob" makes the presentation_DL_revoc_address of the proof                                                                                                       # None
    And "Faber" acknowledges the proof                                                                                                                                   # None
    Then "Bob" has the proof verified                                                                                                                                    # None


Failing scenarios:
  features/0183-revocation.feature:42  Credential revoked by Issuer and Holder attempts to prove with a prover that doesn't care if it was revoked with a DID Exchange connection -- @1.1 

0 features passed, 1 failed, 10 skipped
0 scenarios passed, 1 failed, 135 skipped
4 steps passed, 1 failed, 1169 skipped, 0 undefined
Took 0m50.268s

swcurran · 2022-03-16T16:19:38Z

This is from the ./manage rebuild... command:

 ---> Running in e64abb984e2e
Collecting web.py
  Cloning https://github.com/webpy/webpy.git to /tmp/pip-install-hfzkuiez/web-py_7ccf436ea1fb46c9ba7572055ea2a541
  Running command git clone -q https://github.com/webpy/webpy.git /tmp/pip-install-hfzkuiez/web-py_7ccf436ea1fb46c9ba7572055ea2a541
  Resolved https://github.com/webpy/webpy.git to commit b6f778f382c4e34d5808179fc249622c72a7ac19
  Installing build dependencies: started
  Installing build dependencies: finished with status 'done'
  Getting requirements to build wheel: started
  Getting requirements to build wheel: finished with status 'done'
    Preparing wheel metadata: started
    Preparing wheel metadata: finished with status 'done'
Collecting aries-cloudagent[askar,bbs,indy]@ git+https://github.com/andrewwhitehead/aries-cloudagent-python@fix/store-response
  Cloning https://github.com/andrewwhitehead/aries-cloudagent-python (to revision fix/store-response) to /tmp/pip-install-hfzkuiez/aries-cloudagent_ed0ab69104bd465da76a942e2250a25f
  Running command git clone -q https://github.com/andrewwhitehead/aries-cloudagent-python /tmp/pip-install-hfzkuiez/aries-cloudagent_ed0ab69104bd465da76a942e2250a25f
  Running command git checkout -b fix/store-response --track origin/fix/store-response
  Switched to a new branch 'fix/store-response'
  Branch 'fix/store-response' set up to track remote branch 'fix/store-response' from 'origin'.
  Resolved https://github.com/andrewwhitehead/aries-cloudagent-python to commit 043ad3cffd620b5fbd094611fc86278c8b845e08
Collecting acapy-resolver-universal@ git+https://github.com/sicpa-dlab/acapy-resolver-universal.git@main
  Cloning https://github.com/sicpa-dlab/acapy-resolver-universal.git (to revision main) to /tmp/pip-install-hfzkuiez/acapy-resolver-universal_ae341ebb20a04b1e997e41eaf3ac0a98
  Running command git clone -q https://github.com/sicpa-dlab/acapy-resolver-universal.git /tmp/pip-install-hfzkuiez/acapy-resolver-universal_ae341ebb20a04b1e997e41eaf3ac0a98
  Resolved https://github.com/sicpa-dlab/acapy-resolver-universal.git to commit 3292b6a59338643ac20d85999413b4825666981a
  Installing build dependencies: started
  Installing build dependencies: finished with status 'done'

swcurran · 2022-03-16T16:27:23Z

If you point the requirements-main to this then everything runs fine:

aries-cloudagent[indy, bbs, askar]@git+https://github.com/hyperledger/aries-cloudagent-python@19409c2400b4bee751910ad7eb63bbd569231786

@ianco -- I changed the file to this: aries-cloudagent[indy, bbs, askar]@git+https://github.com/andrewwhitehead/aries-cloudagent-python@fix/store-response and then did a rebuild and run the first test that failed, and that's where I get the error.

What is the commit that you are pointing to in your example -- the one before March 2, or Andrew's current PR?

ianco · 2022-03-16T16:32:59Z

The example that "works" is just before we merged the PR on March 2

Signed-off-by: Andrew Whitehead <cywolf@gmail.com>

ianco · 2022-03-16T17:16:22Z

I think it's related to this change here:

https://github.com/hyperledger/aries-cloudagent-python/pull/1619/files#diff-104ce36838002017a60d035e6dc377cd9ada5680d2a7f6233544b4b7e4deb743L811

(It's in protocols/issue_credential/v1_0/manager.py you have to expand to see the diff and see the line #)

(OK never mind looks like Andrew has fixed it lol)

swcurran · 2022-03-16T17:17:16Z

That worked! However -- the Circle CI tests are now failing. Back to you @andrewwhitehead .

In the meantime, I'll run all the tests with this version vs. just the one so far.

swcurran · 2022-03-16T17:38:35Z

All the tests pass on AATH. Once the CircleCI and Integration Tests get fixed I think we're good to go.

Does the change you had to make look correct, Andrew, or do you think it is an AATH bug?

Signed-off-by: Andrew Whitehead <cywolf@gmail.com>

swcurran · 2022-03-16T19:40:13Z

Looking good. One AATH test run and passed, now running the rest of them. CircleCI passed, integration tests in progress.

Thanks!

swcurran

All AATH tests passing. w00t!

andrewwhitehead requested a review from ianco March 14, 2022 22:35

fetch stored credentials prior to sending ack response in issue-crede…

9c65346

…ntial 2.0 Signed-off-by: Andrew Whitehead <cywolf@gmail.com>

andrewwhitehead force-pushed the fix/store-response branch from 9416b42 to 9c65346 Compare March 14, 2022 22:44

set revoc_reg_id, revocation_id on issued credential exchange

ac3f21c

Signed-off-by: Andrew Whitehead <cywolf@gmail.com>

andrewwhitehead force-pushed the fix/store-response branch from 043ad3c to ac3f21c Compare March 16, 2022 16:38

swcurran added the 0.7.4 label Mar 16, 2022

tolerate multiple calls to create_request, issue_credential

86f92dd

Signed-off-by: Andrew Whitehead <cywolf@gmail.com>

andrewwhitehead changed the title ~~Fetch stored credentials prior to sending ack response in issue-credential 2.0~~ Fixes for credential details in issue-credential webhook responses Mar 16, 2022

swcurran approved these changes Mar 16, 2022

View reviewed changes

swcurran merged commit d4c60ff into openwallet-foundation:main Mar 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes for credential details in issue-credential webhook responses #1668

Fixes for credential details in issue-credential webhook responses #1668

andrewwhitehead commented Mar 14, 2022

codecov-commenter commented Mar 14, 2022 •

edited

Loading

swcurran commented Mar 15, 2022

swcurran commented Mar 15, 2022

ianco commented Mar 16, 2022

ianco commented Mar 16, 2022

ianco commented Mar 16, 2022

andrewwhitehead commented Mar 16, 2022

swcurran commented Mar 16, 2022

ianco commented Mar 16, 2022

ianco commented Mar 16, 2022

swcurran commented Mar 16, 2022

swcurran commented Mar 16, 2022

swcurran commented Mar 16, 2022

ianco commented Mar 16, 2022

ianco commented Mar 16, 2022 •

edited

Loading

swcurran commented Mar 16, 2022

swcurran commented Mar 16, 2022

swcurran commented Mar 16, 2022

swcurran left a comment

Fixes for credential details in issue-credential webhook responses #1668

Fixes for credential details in issue-credential webhook responses #1668

Conversation

andrewwhitehead commented Mar 14, 2022

codecov-commenter commented Mar 14, 2022 • edited Loading

Codecov Report

swcurran commented Mar 15, 2022

swcurran commented Mar 15, 2022

ianco commented Mar 16, 2022

ianco commented Mar 16, 2022

ianco commented Mar 16, 2022

andrewwhitehead commented Mar 16, 2022

swcurran commented Mar 16, 2022

ianco commented Mar 16, 2022

ianco commented Mar 16, 2022

swcurran commented Mar 16, 2022

swcurran commented Mar 16, 2022

swcurran commented Mar 16, 2022

ianco commented Mar 16, 2022

ianco commented Mar 16, 2022 • edited Loading

swcurran commented Mar 16, 2022

swcurran commented Mar 16, 2022

swcurran commented Mar 16, 2022

swcurran left a comment

Choose a reason for hiding this comment

codecov-commenter commented Mar 14, 2022 •

edited

Loading

ianco commented Mar 16, 2022 •

edited

Loading