openwallet-foundation / owl-agent-test-harness

Aries agent test framework, with agent backchannel support
https://aries-interop.info
Apache License 2.0
60 stars 66 forks source link

Extend test harness with support for Issue credential and present proof v2 protocols (Approach) #171

Closed TimoGlastra closed 3 years ago

TimoGlastra commented 3 years ago

This issue documents the requirements to extend AATH to be able to test agents for interoperability using the issue credential and present proof v2 protocols. It is still a WIP and will be extended over time. Feedback on approach is welcome

V1 and V2 of the protocols share a lot of similarities, and therefore we can probably take some shortcuts to prevent starting from scratch.

The main difference between the protocols is the removal of indy-isms and the addition of an attachment registry to support different issue-credential flows.

V1 API

/agent/command/issue-credential/send-offer

{
  "data": {
    "connection_id": "ba0cb955-c265-4588-887b-5097aab3e9a5",
    "cred_def_id": "WgWxqztrNooG92RXvxSTWv:3:CL:20:tag",
    "credential_preview": {
      "@type": "did:sov:BzCbsNYhMrjHiqZDTUASHg;spec/issue-credential/1.0/credential-preview",
      "attributes": [
        {
          "name": "attr_1",
          "value": "value_1"
        },
        {
          "name": "attr_2",
          "value": "value_2"
        },
        {
          "name": "attr_3",
          "value": "value_3"
        }
      ]
    }
  }
}

V2 API

/agent/command/issue-credential/v2/send-offer

I think we should support to add format specific properties to the endpoints. See an example for credential offer below. I used the official format identifiers, but this could also be a more human friendly identifier. If the format key is present it means the message should be sent with that message attached. This makes it possible to start a credential issuance flow with multiple attachments.

{
  "data": {
    "connection_id": "ba0cb955-c265-4588-887b-5097aab3e9a5",
    "credential_preview": {
      "@type": "https://didcomm.org/issue-credential/2.0/credential-preview",
      "attributes": [
        {
          "name": "attr_1",
          "value": "value_1"
        },
        {
          "name": "attr_2",
          "value": "value_2"
        },
        {
          "name": "attr_3",
          "value": "value_3"
        }
      ]
    },
    "formats": {
      // indy specific attributes
      "hlindy-zkp-v1.0": {
        "cred_def_id": "WgWxqztrNooG92RXvxSTWv:3:CL:20:tag"
      },
      // cred manifest specific attributes
      "dif/credential-manifest@v1.0": {},
      // vc with ld-proof specific attributes
      "aries/vc-ld-proof@v1.0": {
        "@context": [
          "https://www.w3.org/2018/credentials/v1",
          "https://www.w3.org/2018/credentials/examples/v1"
        ],
        "type": ["VerifiableCredential", "UniversityDegreeCredential"],
        "issuer": "https://example.edu/issuers/14",
        "credentialSubject": {
          "id": "did:example:ebfeb1f712ebc6f1c276e12ec21",
          "degree": {
            "type": "BachelorDegree",
            "name": "Bachelor of Science and Arts"
          }
        },
        "proofType": ["Ed25519Signature2018", "BbsBlsSignature2020"]
      }
    }
  }
}

Another approach would be to create separate flow per format. .e.g.

/agent/command/issue-credential/v2/send-offer/hlindy-zkp-v1.0 /agent/command/issue-credential/v2/send-offer/aries/vc-ld-proof@v1.0

nodlesh commented 3 years ago

For Acapy, the changes to the protocol is in the data and a minor change to the acapy admin api endpoints with -2.0 added. (Will this be the common changes in other aries frameworks as well?) Given that the flow and protocol states are the same as 1.0, we should make the impact minimal to non-existent to the 1.0 tests. Reuse of tests as they exist is possible. The @AIP20 tag added to the test scenarios in the feature files will designate that scenario as v2.0. A new test for v2.0 would look like this along side a AIP10 scenario.

   @T001-RFC0037 @AIP10 @critical @AcceptanceTest @Indy
   Scenario Outline: Present Proof where the prover does not propose a presentation of the proof and is acknowledged
      Given "2" agents
         | name  | role     |
         | Faber | verifier |
         | Bob   | prover   |
      And "Faber" and "Bob" have an existing connection
      And "Bob" has an issued credential from <issuer>
      When "Faber" sends a request for proof presentation to "Bob"
      And "Bob" makes the presentation of the proof
      And "Faber" acknowledges the proof
      Then "Bob" has the proof verified

      Examples:
         | issuer |
         | Acme   |
         | Faber  |

   @T001.1.1-RFC0037 @AIP20 @critical @AcceptanceTest
   Scenario Outline: Present Proof where the prover does not propose a presentation of the proof and is acknowledged
      Given "2" agents
         | name  | role     |
         | Faber | verifier |
         | Bob   | prover   |
      And "Faber" and "Bob" have an existing connection
      And "Bob" has an issued credential from <issuer>
      When "Faber" sends a request for proof presentation to "Bob"
      And "Bob" makes the presentation of the proof
      And "Faber" acknowledges the proof
      Then "Bob" has the proof verified

      Examples:
         | issuer |
         | Acme   |
         | Faber  |

The code in the steps above could be changed in 2 ways.

  1. Check for the AIP20 tag in the step and call the appropriate endpoint.
    If AIP20 in tags:
    POST /issue-credential-2.0/records/{cred_ex_id}/send-offer
    else:
    POST /issue-credential/records/{cred_ex_id}/send-offer
  2. Check for the AIP20 tag and add V2.0 to the message data sent and let the backchannel handle what call is to be made to the agent for that version.

My preference would be 2.

In either case, in each step, there would have to be a check for AIP20 when creating the message data to send, in order to decorate the message with formats key and the runtime data that needs to be added inside. The expectation is that new externalized data will be created for the 2.0 calls. This new data can be used in the test definitions as data tables as it is for the 1.0 version.

TimoGlastra commented 3 years ago

I'm fine with the second approach. However, there are changes needed to the API as we use for 1.0 as it uses indy specific properties. The ACA-Py 2.0 API adds a filter property that contains the format specific properties, we could use that?

V1:

{
  "data": {
    "connection_id": "ba0cb955-c265-4588-887b-5097aab3e9a5",
    "cred_def_id": "WgWxqztrNooG92RXvxSTWv:3:CL:20:tag",
    "credential_preview": {
      "@type": "did:sov:BzCbsNYhMrjHiqZDTUASHg;spec/issue-credential/1.0/credential-preview",
      "attributes": []
    }
  }
}

V2:

{
  "data": {
    "connection_id": "ba0cb955-c265-4588-887b-5097aab3e9a5",
    "credential_preview": {
      "@type": "https://didcomm.org/issue-credential/2.0/credential-preview",
      "attributes": [] 
    },
    "filters": {
      "indy": {
        "cred_def_id": "WgWxqztrNooG92RXvxSTWv:3:CL:20:tag"
      },
      "ld-proofs": {
        "@context": [
          "https://www.w3.org/2018/credentials/v1",
          "https://www.w3.org/2018/credentials/examples/v1"
        ],
        "type": ["VerifiableCredential", "UniversityDegreeCredential"],
        "issuer": "https://example.edu/issuers/14",
        "credentialSubject": {
          "id": "did:example:ebfeb1f712ebc6f1c276e12ec21",
          "degree": {
            "type": "BachelorDegree",
            "name": "Bachelor of Science and Arts"
          }
        },
        "proofType": ["Ed25519Signature2018", "BbsBlsSignature2020"]
      }
    }
  }
}

Or to put it differently, where would you put the data inside the filters.ld-proofs?

nodlesh commented 3 years ago

All V2.0 tests would have to use the externalized message data. So a V1 test that looks like this...

   @T001.2-RFC0037 @AIP10 @critical @AcceptanceTest @Schema_DriversLicense @Indy
   Scenario Outline: Present Proof of specific types and proof is acknowledged with a Drivers License credential type
      Given "2" agents
         | name  | role     |
         | Faber | verifier |
         | Bob   | prover   |
      And "Faber" and "Bob" have an existing connection
      And "Bob" has an issued credential from <issuer> with <credential_data>
      When "Faber" sends a <request_for_proof> presentation to "Bob"
      And "Bob" makes the <presentation> of the proof
      And "Faber" acknowledges the proof
      Then "Bob" has the proof verified

      Examples:
         | issuer | credential_data   | request_for_proof            | presentation                |
         | Acme   | Data_DL_MaxValues | proof_request_DL_address     | presentation_DL_address     |
         | Faber  | Data_DL_MinValues | proof_request_DL_age_over_19 | presentation_DL_age_over_19 |

Would look like this for V2.0

   @T001.2-RFC0454 @AIP20 @critical @AcceptanceTest @Schema_DriversLicense_v2
   Scenario Outline: Present Proof of specific types and proof is acknowledged with a Drivers License credential type
      Given "2" agents
         | name  | role     |
         | Faber | verifier |
         | Bob   | prover   |
      And "Faber" and "Bob" have an existing connection
      And "Bob" has an issued credential from <issuer> with <credential_data>
      When "Faber" sends a <request_for_proof> presentation to "Bob"
      And "Bob" makes the <presentation> of the proof
      And "Faber" acknowledges the proof
      Then "Bob" has the proof verified

      Examples:
         | issuer | credential_data   | request_for_proof            | presentation                |
         | Acme   | Data_DL_MaxValues_v2 | proof_request_DL_address_v2     | presentation_DL_address_v2     |
         | Faber  | Data_DL_MinValues_v2 | proof_request_DL_age_over_19_v2 | presentation_DL_age_over_19_v2 |

Where the Examples above point to json files external to the tests containing everything needed for the request. Anything needed at runtime, like identifiers, can be amended to the message body in the test code. The tests do this now for V1, if an id is needed, the property in the json file has a "replace me" value in it. It gets replaced with the actual identifier needed at the time of the call. The tests themselves do this, not the backchannels.

The backchannels job is to format the message body particular for the agent framework being called, and call the correct version of the endpoint.

Since these are new RFCs, it may also be wise to start new feature files for the v2 protocols. We can still use the same tests from the v1 feature file, and it will call the code in the v1 .py file. If anything extra is needed for v2 it will be in its own steps .py file for the feature. Files:

0036-issue-credential.feature
0453-issue-credential-v2.feature 
0037-present-proof.feature
0454-present-proof-v2.feature
steps/0036-issue-credential.py (holds common steps between 0036 and 0453)
steps/0453-issue-credential-v2.py (holds customs steps for 0453, if any)
steps/0037-present-proof.py (holds common steps between 0037 and 0454)
steps/0454-present-proof-v2.py (holds customs steps for 0454, if any)
swcurran commented 3 years ago

That sounds right to me. Y'all know this better, but reading what Shel wrote LGTM.

nodlesh commented 3 years ago

With this approach I'm finding I'm still having to adjust the Test to Backchannel API. With issue credential v1, there are operations that do not take any data. I'm now modifying the API to make all API calls for Issue Credential to take data, because at the very least it will need,

{
   "aip_version": "AIP20"
}

I'm still thinking this is the best approach, with the least amount of impact to v1 tests and other backchannels, but thought I'd mention it. I thought about adding a parameter for version to the Test to Backchannel API endpoints, but that would cause complete change across all tests, and all backchannels. With the data approach, Only backchannels and tests that need to be aware of API20 need to adjust.

nodlesh commented 3 years ago

@TimoGlastra @swcurran Guys, I need your opinion on this. I'm thinking of changing the approach I mentioned in the last comment. I'm finding I'm adding a data payload to every call on the test to backchannel api, even if it never has had data sent, like the GETs for instance. All backchannels are going to have to support this, so they will have to change. What is making sense to me now is, why don't I just add one POST to the API, that is set-aip-version. Every backchannel has to support this one extra call and set the version internally in the backchannel for any preceding calls. In this solution the other calls that do not take any data, do not need to change and can continue to work as is. I don't see any negatives about this approach, as I do not see a case where a scenario will run both AIP10 and AIP20 in the same scenario. I am 95% ready to refactor everything I've done in the last day and a half and go with the `set-aip-version call. Anyone care to nudge me that 5%?

nodlesh commented 3 years ago

There is one problem with doing a new POST set-aip-version on the test api. If we ever decide to run tests in parallel against a running agent, and those tests may be using different AIP versions, the agent is going to have a hard time with which version to run for which caller. Though we don't have the parallel requirement right now, if we ever wanted to do Performance/Load testing with AATH, it will become a problem.

swcurran commented 3 years ago

Sorry for the delay in responding. My $0.02CDN on this:

In ACA-Py (and likely other frameworks), for every major (e.g. 1 to 2) revision to a protocol, we create an entire new protocol (usually by doing a copy, paste and rename), so that there is no impact on the existing implementation (it remains supported) and we don't have to worry about the changes working for both versions. Essentially, a major version means that we are now supported an entirely new protocol (even though it shares characteristics with a new one), and we accept that overhead.

I think we should do the same in AATH and just treat the two as different and don't try to be tricky in handling the two versions within a single chunk of code. Create different endpoints and create different tests. Ideally, do that by copy and paste to start, but don't worry about trying to make things work with flags and if/else statements..

Note that this is not the approach to take in minor version changes -- e.g. 1.0 to 1.1.

nodlesh commented 3 years ago

Yeah, I was trying to avoid writing a large amount of test step code. 1.0 and 2.0 are so close, it's just payload and the topic that are different.

What do you think about a different backchannel altogether? acapy_backchannel20 inherits from the original backchannel. You would have to know what tests you are going to run before you do an execution. You would then need to do ./manage build acapy-20-main and './manage run -a acapy-20-main ...`

TimoGlastra commented 3 years ago

I agree with Stephen on adding new test cases. Rather have some duplicate code than complicating the current code.

Instead of adding a new backchannel, couldn't we use a v2 prefix in the path? So if v2 is in the path (e.g. /agent/command/issue-credential/v2/send-offer) the backchannel will call the v2 operations.

Maintaining multiple backchannels for a single agent/framework maybe seems easy now, but is not something I'd like to maintain over time

swcurran commented 3 years ago

Good idea -- since that is analogous to the versioning used in the message types.

Now -- who do we reference the version of the protocol we are using in BDD for the tests? Worst case, we put "V2" into the test steps, but is there a more BDD-happy way?

nodlesh commented 3 years ago

I like the v2 in the path. very manageable.

Agreed, V2 would be the last choice in the BDD(gherkin). Maybe we can state it as what the buisiness goal of the V2 protocol is? This is the V1 propose step When "Bob" proposes a credential to "Acme" with <credential_data> Maybe V2 could look like this? When "Bob" proposes a credential with formalized messages to "Acme" with <credential_data> or When "Bob" proposes a credential with filter and format support to "Acme" with <credential_data>

swcurran commented 3 years ago

My $0.02 is that it is very subtle -- kind of like the subtlety of using Acme and Bob instead of Issuer and Verifier :-). Imagine having to explain the difference to people. I think something more specific.

One way could be When "Bob" proposes an "indy" credential to "Acme" with <data>

Then, indy could be replace with jsonld and jsonld_bbs for the different types. And, only V2 handles the variations.

nodlesh commented 3 years ago

That will work. The gherkin would possibly end up looking like this.

    Given "2" agents
      | name | role   |
      | Acme | issuer |
      | Bob  | holder |
    And "Acme" and "Bob" have an existing connection
    When "Bob" proposes a <cred_format> credential to "Acme" with <credential_data>
    And "Acme" offers the credential
    And "Bob" requests the credential
    And "Acme" issues the credential
    And "Bob" acknowledges the credential issue
    Then "Bob" has the credential issued

    Examples:
      | cred_format | credential_data | 
      | indy                | Data_DL_MaxValues_indy |
      | jsonld             | Data_DL_MaxValues_jsonld |
      | jsonld_bbs   | Data_DL_MaxValues_jsonld_bbs |
nodlesh commented 3 years ago

Actually the way you had it is better. When "Bob" proposes an "indy" credential to "Acme" with <data> A new test scenario will be created for jsonld, etc, instead of using the Examples. This is because the way I had it, if you run the indy test, you will run the rest of the examples as well.

TimoGlastra commented 3 years ago

Now that PE is merged into ACA-Py I'll start working on adding PE support to the test harness @swcurran @nodlesh. Hoping to have something ready before the end of next week