[FIX] DIDExchange should handle legacy DIDDoc format

[gmulhearn]

As found here, aries-vcx DIDExchange handlers are not resilient to the case where the request or response message has a did_doc attachment that is a legacy DIDDoc structure (i.e. an older "aries" diddoc: https://hyperledger.github.io/aries-rfcs/latest/features/0067-didcomm-diddoc-conventions/#implementations).

This is particularly problematic when the peer is using a legacy DID, as this is when the peer would attach a did_doc. E.g. ACApy has been found to do this, causing AATH failures.

Other agents, like acapy, are tolerant to both legacy and spec-compliant DIDDoc structures in their DIDExchange handling.

To fix this, i believe we should do the following:

1. add a method to convert from the legacy structure (AriesDidDoc in aries/misc/legacy/diddoc_legacy/src/aries/diddoc.rs) into the spec-compliant structure (did_core/did_doc/src/schema/did_doc.rs). This method could probably live in the diddoc_legacy crate as a signal to consumers to migrate to use the new format; AriesDidDoc::try_into_compliant
2. Have the attachment_to_diddoc DIDExchange helper function in aries-vcx have a fallback of; 1. deserialize into AriesDidDoc, 2. convert AriesDidDoc into compliant DidDocument.

Alternatively to 2. ^, if it's more efficient, we could have some internal helper serde struct defined to represent both variants like a union type, then deserialize into that union type, then run the 2.2. conversion step if needed. e.g.:

#[serde(untagged)]
enum TolerantDidDocument {
    Compliant(DidDocument),
    Legacy(AriesDidDoc)
}

impl TolerantDidDocument {
    pub fn try_into_compliant(self) -> Result<DidDocument> {
       match self {
           Self::Compliant(c) -> c,
           Self::Legacy(l) -> l.try_into_compliant()
      }
   }
}

ACAPY helpful reference

ACAPy also has a converter for legacy -> compliant, found here: https://github.com/hyperledger/aries-cloudagent-python/blob/5ad52c15d2f4f62db1678b22a7470776d78b36f5/aries_cloudagent/resolver/default/legacy_peer.py#L27. it could be used as a reference

[gmulhearn-anonyome]

@JamesKEbert there's arguably no point in doing this if we don't care for supporting for unqualified DIDs in DIDExchange. my understanding is that did_doc attachment is only used if unqualified DIDs are being used (i.e. bcus they are unresolvable). thoughts?

[JamesKEbert]

@gmulhearn-anonyome I'd tend to agree in that since we're attempting to migrate away from using unqualified DIDs as a community then it's not really worth the effort to add support for them, unless there's some strong need for that backwards compatibility that I'm not aware of.

[JamesKEbert]

@swcurran, we're inclined to not develop support for this legacy format given that we as a community are working to move to qualified DIDs entirely and it makes less sense to add support for something we'll be shortly thereafter removing support for. Is there something I'm missing though that should change this viewpoint?

[swcurran]

Trying to understand the context. I think you are saying that support for DID Exchange instances where the DIDDoc is inline is not going to be supported. Rather, only DID Exchange instances where a DID is specified will be used.

I think that is definitely what to do on the sending side. If it is “easy” to support receiving DIDs in both ways, it would be nice to handle that. I’m pushing hard these days in a few areas where we need to be very clear to say “You MUST only send this”, but also say “On receipt, assume that senders are idiots and might send this or this or this — accept them all”.

That said, in this case, because of how far we are in getting rid of unqualified DIDs (“You MUST only send qualified DIDs”), you would excused to not add the extra complication of not accepting any unqualified forms.

[JamesKEbert]

Thank you for the helpful comments Stephen.

I’m pushing hard these days in a few areas where we need to be very clear to say “You MUST only send this”, but also say “On receipt, assume that senders are idiots and might send this or this or this — accept them all”.

I think this is wise :)

Given the ongoing headaches that it would impose to add backwards support for any unqualified DIDs to VCX/DID Exchange and the discussion above, I am going to close this issue.