getVerificationMethod should be framing the verificationMethod URI

[JaceHensley]

Ed25519Signaute2020#getVerificatoinMethod should frame proof.verificationMethod like the base LinkedDataSignature#getVerificationMethod.

If the framing doesn't happen within the signature suite the framing/deferencing then it is up to the documentLoader to handle that (which is happening here in the tests). But that puts more requirements on the user to know how to handle framing/deferencing from within their documentLoader (and handling it for all the different sig suites).

[OR13]

The results in type ambiguity:

developers expect:

getVerificationMethod: (iri:string) => Promise<PublicKey>

developers observe:

getVerificationMethod: (iri:string) => Promise<PublicKey | DID Document>

[OR13]

I think this is the line that allows your tests to pass, while hiding this bug from you:

https://github.com/digitalbazaar/ed25519-signature-2020/blob/main/test/Ed25519VerificationKey2020.spec.js#L39

[JaceHensley]

If I extend Ed25519Signature2020 and override getVerificationMethod like:

class Ed25519Signature2020Patched extends Ed25519Signature2020 {
  constructor({
    key,
    signer,
    verifier,
    proof,
    date,
    useNativeCanonize,
  }: any = {}) {
    super({
      key,
      signer,
      verifier,
      proof,
      date,
      useNativeCanonize,
    });
  }


  async getVerificationMethod({proof, documentLoader}: any) {
    let {verificationMethod} = proof;

    if(typeof verificationMethod === 'object') {
      verificationMethod = verificationMethod.id;
    }

    if(!verificationMethod) {
      throw new Error('No "verificationMethod" found in proof.');
    }

    // Note: `expansionMap` is intentionally not passed; we can safely drop
    // properties here and must allow for it
    const framed = await jsonld.frame(verificationMethod, {
      '@context': this.contextUrl,
      '@embed': '@always',
      id: verificationMethod
    }, {documentLoader, compactToRelative: false});
    if(!framed) {
      throw new Error(`Verification method ${verificationMethod} not found.`);
    }

    // ensure verification method has not been revoked
    if(framed.revoked !== undefined) {
      throw new Error('The verification method has been revoked.');
    }

    await this.assertVerificationMethod({verificationMethod: framed});

    return framed;
  }
}

Then it works as expected and my documentLoader always returns the DID Doc for DID URLs even when they include the fragment

[dlongley]

Framing is intentionally not used in this library because it is not always a requirement, it causes a performance hit when used, and it is not the right separation of concerns -- so it was determined that it is therefore best left to the document loader layer.

If your document loader is resolving a verification method that is referenced via a DID URL, for example, then it should properly construct the verification method. It should not rely on whomever you're handing the resolution result off to to do that for you -- nor should it return something other than the verification method (e.g., it should not return the DID Doc, that is not what was referenced by the DID URL, the verification method was).

So, yes, it is the case that the new (current) version of this library requires the party to use a document loader that understands how to provide well-formed verification methods. Most DID drivers/resolvers can be written to handle this properly (and some potentially without the need for framing themselves, but a more direct/templated restructuring as needed). So users shouldn't have to think about it; they just use a reasonable DID driver/resolver that provides them a doc loader.

[dlongley]

A possible compromise position would be for the signature suite to first check if the ID of the returned document matches the verification method ID -- and, if not, use framing. This would be an automatic process that would avoid framing in cases where the user provided a document loader that fully dereferences DID URLs vs. one that just returns a document in which the referenced entity exists somewhere -- leaving the suite to do the rest of the dereferencing via the framing operation. Of course, this causes responsibility spillage into suites when, IMO, doc loaders should be dereferencing.

[dmitrizagidulin]

@dlongley +1, I think we should do that. (Allow (& document) documentLoaders to optimize key resolving, but also fall back to framing if a key id is not present in the document cache etc).

[OR13]

+1 to not assuming a documentLoader will do DID Document Dereferencing...

You don't need to do framing to find a key in a did document, you can iterate all verification relationships, handling both absolute and not absolute IRIs until you find a match.... if you only support a document loader interface you are exposing callers to that assumption.

I do think that you are treading pretty confidently into a killing field by conflating resolution and dereferencing at the document loader interface... particularly because of the wording in did core today.

your proposed type signature for documentLoader:

iri: string => Promise<context>
iri: string => Promise<did document>
iri: string => Promise<key>

for me the issue is consistent treatment of the resource / document identified by the iri.

https://www.w3.org/TR/json-ld11-api/#dom-jsonldoptions-documentloader

The callback of the loader to be used to retrieve remote documents and contexts, 
implementing the LoadDocumentCallback. 
If specified, it is used to retrieve remote documents and contexts; 
otherwise, if not specified, the processor's built-in loader is used.

the document resource is identified with did:example:123/path/456?query=foo

per the did core spec:

https://w3c.github.io/did-core/#fragment

Additional semantics for fragment identifiers, which are 
compatible with and layered upon the semantics in this section, 
are described for JSON-LD representations in § B.2 application/did+ld+json. 
For information about how to dereference a DID fragment, see § 7.2 DID URL Dereferencing.

^ this right here is the beginning of danger land… just above this it says:

In order to maximize interoperability, implementers are urged 
to ensure that DID fragments are interpreted in the same way 
across representations (see § 6. Representations).

… when you add them together, you get a foot gun.

for the record I think its very valid to abstract framing / key lookup into a function like getVerificationMethod

that handles fragments either with or without JSON-LD frame…

what I am objecting too is hiding that logic in the documentLoader itself.

certainly we (Transmute) won't be having a documentLoader ever return something other than the resource (fragments identify sub resources on a per content-type basis)... we think its a type error to conflate the return type.

We could be convinced by a pointer to the the JSON-LD spec that says documentLoaders perform dereferencing and are aware of the requested content type.

We could also be convinced by additional language in did core regarding expectations on resolution vs dereferencing that pointed to documentLoaders.

[OR13]

This issue is related: w3c/did-imp-guide#3

If we could assume that documentLoaders do dereferencing, we could solve some interop issues with JSON-LD / JSON.... but at the cost of telling everyone they MUST dereference did+json and did+ld+json they exact same way....

Obviously the spec says that assumption cannot be made today.

[dmitrizagidulin]

@dlongley @OR13 - if you don't mind, I'd like to move this issue to jsonld-signatures repo. (since it applies to all suites)

[dlongley]

I'm fine with it being moved.

[OR13]

I am considering updating all our suites to be compatible with this breaking change : )

The current work around is to pass a document loader that does did resolution to our suites, and a document loader that does did dereferencing to digital bazaar's...

I worry that I will make this update, and then something related to the structure of dereferencing will change again... this is currently working with digital bazaar code:

  {
        "@context": [
          "https://www.w3.org/ns/did/v1",
          "https://w3id.org/security/suites/ed25519-2018/v1"
        ],
        "id": "did:key:z6MkhRZ7XMcydYWTYGQrVxKXaG9Bpd3LrbA345QGWNMaSxeE#z6MkhRZ7XMcydYWTYGQrVxKXaG9Bpd3LrbA345QGWNMaSxeE",
        "type": "Ed25519VerificationKey2018",
        "controller": {
          "id": "did:key:z6MkhRZ7XMcydYWTYGQrVxKXaG9Bpd3LrbA345QGWNMaSxeE",
          "assertionMethod": [
            "did:key:z6MkhRZ7XMcydYWTYGQrVxKXaG9Bpd3LrbA345QGWNMaSxeE#z6MkhRZ7XMcydYWTYGQrVxKXaG9Bpd3LrbA345QGWNMaSxeE"
          ]
        },
        "publicKeyBase58": "3yJ4w7NYJ11zRma9pPMgjAbC13mVShugN4VLg6PZXjrr"
      }

This is just jsonld.frame(didDocument, verificationMethod.id)

This could be implemented without JSON-LD, but @context would be required.... in digital bazaar libraries, at least.

my main question is, will we have problems with DID Core, given the language regarding dereferencing there is pretty weak....

I think most folks in did core would expect this:

  {
        "id": "did:key:z6MkhRZ7XMcydYWTYGQrVxKXaG9Bpd3LrbA345QGWNMaSxeE#z6MkhRZ7XMcydYWTYGQrVxKXaG9Bpd3LrbA345QGWNMaSxeE",
        "type": "Ed25519VerificationKey2018",
        "controller": "did:key:z6MkhRZ7XMcydYWTYGQrVxKXaG9Bpd3LrbA345QGWNMaSxeE",
        "publicKeyBase58": "3yJ4w7NYJ11zRma9pPMgjAbC13mVShugN4VLg6PZXjrr"
      }

[OR13]

ping @peacekeeper @msporny

should documentLoader's be understood to do "did dereferencing only".... dereferencing IS a content type specific action, we WILL start to see greater differences between JSON and JSON-LD.

for example, JSON dereferencing is very likely to never include an @context.

There are dangerous differences between the 2 JSON above... 1 of them includes a purpose, and the other does not.... At least when we were using documentLoaders that only did resolution, the entire JSON document was returned and we could see the purpose off the verification method.... this change takes down a path that will likely require checking that somewhere else....

[JaceHensley]

I still prefer where the documentLoader only resolves the DID, not dereferencing. I'm curious as to what's changed your mind @OR13

[OR13]

@JaceHensley more consistent explanations of vc verification for jwt and ld proofs.... dereferencing a key identifier to key material is required for both... returning the key instead of the document, means hitting the same code paths for both, and not handling the same behavior the same in 2 places...

[kdenhartog]

I'm realizing the issue I've encountered in #154 is very similar to this here. Probably worth giving a read to it as well. FWIW, in my experience dealing with our BBS library the frame needs to be highly specific to the document it's framing and presumes the document is JSON-LD. Isn't that going to be an issue?

[dlongley]

So I proposed this above:

A possible compromise position would be for the signature suite to first check if the ID of the returned document matches the verification method ID -- and, if not, use framing. This would be an automatic process that would avoid framing in cases where the user provided a document loader that fully dereferences DID URLs vs. one that just returns a document in which the referenced entity exists somewhere -- leaving the suite to do the rest of the dereferencing via the framing operation.

#141 (comment)

If that were implemented, would it solve everyone's problems here? Or would something more be required?

[kdenhartog]

So I proposed this above:

A possible compromise position would be for the signature suite to first check if the ID of the returned document matches the verification method ID -- and, if not, use framing. This would be an automatic process that would avoid framing in cases where the user provided a document loader that fully dereferences DID URLs vs. one that just returns a document in which the referenced entity exists somewhere -- leaving the suite to do the rest of the dereferencing via the framing operation.

#141 (comment)

If that were implemented, would it solve everyone's problems here? Or would something more be required?

This seems like it would tentatively solve the issue for us. The tradeoff here is that people are going to have to be extra careful around the effects of the documentLoader throughout the stack which is edging us into some weird territory around expectations from the contract of the function. It seems like we've already found ourselves here though and this is better solved by making sure people understand the impacts of the passed in documentLoader on the reliability of the function. As an example, a documentLoader that doesn't support resolving dids won't be usable with any documents that rely on DIDs. This is obvious to most of us, but isn't quite clear based on the contract of the function.

[OR13]

To be clear, documentLoaders are "DID Dereferencers", if they process content type and fragments... thats a step after resolution.