aries-cloudagent-python icon indicating copy to clipboard operation
aries-cloudagent-python copied to clipboard

Update the resolver for did:sov to use the latest DID Method Rules

Open swcurran opened this issue 2 years ago • 12 comments

Please update the DID Resolver within ACA-Py for "did:sov" to follow the rules as recently updated in the DID Method spec. This means adjusting the returned DIDDoc to exactly what is documented in the DID Method spec for the cases of their being no endpoint ATTRIB, a "current convention" endpoint ATTRIB and support for the defined new convention endpoint ATTRIB.

The PR for the DID Method that highlights the changes is here:

https://github.com/sovrin-foundation/sovrin/pull/296/files

@dbluhm -- great if you could point out where in the ACA-Py code this needs to go.

Thanks!

swcurran avatar Jul 13 '21 09:07 swcurran

https://github.com/hyperledger/aries-cloudagent-python/blob/main/aries_cloudagent/resolver/default/indy.py#L64-L91

dbluhm avatar Jul 13 '21 13:07 dbluhm

Please don't remove support for additional service types beside did-communication/DIDcomm. In the Business Partner Agent project we rely on service type "profile"

domwoe avatar Jul 15 '21 05:07 domwoe

That one is new to me. Could you give an example and where it comes from?

swcurran avatar Jul 15 '21 06:07 swcurran

https://hackmd.io/4oZOgwFOQDSFUuu3ruN-_g

domwoe avatar Jul 15 '21 07:07 domwoe

Dom -- the "profile" is not a convention that has been supported in Indy AFAIK -- e.g. are there any ATTRIBs on Sovrin MainNet using it? On the other hand, the "endpoint" ATTRIB is commonly used on Sovrin and likely other Indy networks.

For that and many other use cases, we really need to have "did:indy", so that the controller of the DID can define what they want in their DIDDoc. That is supported in what we have defined "did:indy" to be.

Let's work on that to achieve this capability.

swcurran avatar Jul 20 '21 12:07 swcurran

I agree that we should move forward with did:indy, but it would be great to preserve this functionality in did:sov as well to not break our BPA deployments.

The did-sov driver of the uni resolver has always supported this feature and still is (see https://dev.uniresolver.io/1.0/identifiers/did%3Asov%3Aidu%3A4PDz9iiQqgVXhMA7nWqRzS and the implementation). Although an ID should be added to the service.

Furthermore, we added the feature to write/read services of different types to ACA-Py more than a year ago. https://github.com/hyperledger/aries-cloudagent-python/issues/589

If the did:sov resolver in ACA-Py gets implemented exactly like the did:sov driver of the ACA-Py our use case is still supported, but i'd suggest to generate an id field to the service object.

domwoe avatar Jul 21 '21 07:07 domwoe

Ah -- I've never heard of that before, and when you mentioned it, I thought it was in a custom resolver. When did that get added to the universal resolver and for what purpose?

So you are saying that in the did:sov, I need to add an additional service back that is identical to what is there for "endpoint", but:

  • uses "profile" instead of "endpoint",
  • adds "/profile.jsonld" to the end of the ATTRIB "endpoint" value.

Correct?

swcurran avatar Jul 21 '21 23:07 swcurran

The endpoint ATTRIB can be a (stringified) object. In the example above the ATTRIB data is the following

{\"endpoint\":{\"endpoint\":\"https://bank-bpa-acapy.stage.economyofthings.io\",\"profile\":\"https://bank-bpa.stage.economyofthings.io/profile.jsonld\"}}"

The did-sov driver will map all entries in the endpoint object to services. This allows to add additional service endpoints, if no custom properties are required.

{\"endpoint\":{\"profile\":\"https://bank-bpa.stage.economyofthings.io/profile.jsonld\"}}"

=>

"service": [{
  "type": "profile",
  "serviceEndpoint": "https://bank-bpa.stage.economyofthings.io/profile.jsonld"
}]

In earlier versions of the did-sov driver, an additional id property was generated from the type, e.g."id": "did:sov:example#profile"

domwoe avatar Jul 22 '21 06:07 domwoe

Just ran into some issues with this. As outlined in this issue, ACA-Py doesn't support the updated rules in the DID sov Method spec.

I'm currently implementing this in AFJ and dids written using AFJ will throw errors in ACA-Py.

We're currently setting the following attrib json:

{'endpoint': 'https://example.com/endpoint', 'routingKeys': ['a-routing-key'], 'types': ['DIDComm', 'did-communication', 'endpoint']}

This throws an error as it tries to the set the values of types and routingKeys as serviceEndpoint ,which will (correctly) fail validation.

We should add custom logic for the types / routingKeys properties.

See here for the logic we use in AFJ: https://github.com/hyperledger/aries-framework-javascript/blob/main/packages/core/src/modules/dids/methods/sov/SovDidResolver.ts#L76-L159

This will make sure the new types are supported, but cases as outlined by @domwoe will also keep working.

Here's an example did that is written to the bcvorin ledger: did:sov:XUeUZauFLeBNofY3NhaZCB. In ACA-Py it throws a 500 error. In AFJ it resolves to the following document:

{
  "@context": [
    "https://w3id.org/did/v1",
    "https://w3id.org/security/suites/ed25519-2018/v1",
    "https://w3id.org/security/suites/x25519-2019/v1",
    "https://didcomm.org/messaging/contexts/v2"
  ],
  "id": "did:sov:XUeUZauFLeBNofY3NhaZCB",
  "verificationMethod": [
    {
      "id": "did:sov:XUeUZauFLeBNofY3NhaZCB#key-1",
      "type": "Ed25519VerificationKey2018",
      "controller": "did:sov:XUeUZauFLeBNofY3NhaZCB",
      "publicKeyBase58": "HcRnAYfbhmd5oVSGdGwKh4icfjUGyBgqhTF7xoS1TyrC"
    },
    {
      "id": "did:sov:XUeUZauFLeBNofY3NhaZCB#key-agreement-1",
      "type": "X25519KeyAgreementKey2019",
      "controller": "did:sov:XUeUZauFLeBNofY3NhaZCB",
      "publicKeyBase58": "5ryv9jhxHErGRTxXYYWQck9u58gC69dfYFvV421qWhvr"
    }
  ],
  "service": [
    {
      "id": "did:sov:XUeUZauFLeBNofY3NhaZCB#endpoint",
      "serviceEndpoint": "https://example.com/endpoint",
      "type": "endpoint"
    },
    {
      "id": "did:sov:XUeUZauFLeBNofY3NhaZCB#did-communication",
      "serviceEndpoint": "https://example.com/endpoint",
      "type": "did-communication",
      "priority": 0,
      "recipientKeys": ["did:sov:XUeUZauFLeBNofY3NhaZCB#key-agreement-1"],
      "routingKeys": ["a-routing-key"],
      "accept": ["didcomm/aip2;env=rfc19"]
    },
    {
      "id": "did:sov:XUeUZauFLeBNofY3NhaZCB#didcomm-1",
      "serviceEndpoint": "https://example.com/endpoint",
      "type": "DIDComm",
      "routingKeys": ["a-routing-key"],
      "accept": ["didcomm/v2"]
    }
  ],
  "authentication": ["did:sov:XUeUZauFLeBNofY3NhaZCB#key-1"],
  "assertionMethod": ["did:sov:XUeUZauFLeBNofY3NhaZCB#key-1"],
  "keyAgreement": ["did:sov:XUeUZauFLeBNofY3NhaZCB#key-agreement-1"]
}

TimoGlastra avatar Jul 16 '22 12:07 TimoGlastra

I had been hoping we would get to a generally deployed did:indy before we needed this.

We'll talk on our team about how soon we can do this. If anyone else can take it, please let us know. Looks like a relatively small piece of code -- but important!!!

Ping @andrewwhitehead @shaangill025

swcurran avatar Jul 18 '22 17:07 swcurran

@TimoGlastra @swcurran I did a quick and dirty fix in #1863. I'm not up to date on the implications of the accept strings so there might be some tweaking that needs to occur there.

dbluhm avatar Jul 18 '22 18:07 dbluhm

I'm not up to date on the implications of the accept strings so there might be some tweaking that needs to occur there.

I think you implemented this correctly. At least that's how I also understand the spec. As there's no way to store the accept property, it just assumes it's always didcomm v2

TimoGlastra avatar Jul 19 '22 08:07 TimoGlastra

I believe this is resolved by #1863

dbluhm avatar Aug 22 '23 14:08 dbluhm