Add support for multiple signatures.

[mattcollier]

Nothing changes for documents with single signatures. There is a boolean return or an error. The existing test suite passes without any modifications.

In the case when verify is called on a document with multiple signatures, the return value will look like this, where the signature.creator is the key for the map.

{
  "did:v1:53ebca61-5687-4558-b90a-03167e4c2838/keys/1": {
    "verified": true
  },
  "did:v1:5627622e-0ab3-479a-bfe7-0f4983a1f7ce/keys/1": {
    "verified": true
  },
  "did:v1:777ea7ad-ab68-4039-b85b-a45a795b2d93/keys/1": {
    "verified": false,
    "error": "jsonld.LoadDocumentError: URL could not be dereferenced, an error occurred."
  }
}

Currently, there are some tests that exercise this new functionality in bedrock-ledger-authz-multisignature. If the general concept presented here is approved, I will add additional tests to the test suite in this module.

[dlongley]

Thinking through how this will be used...

It may be odd if you had to inspect the number of signatures in the data you were verifying before you could know what the output would look like. Similarly, it may be odd to have to check for two different types of output after calling verify.

I wonder if it would be better to add a output: 'boolean' | 'hash' or result: 'simple' | 'detailed' (can bikeshed this) option to control whether the output will be a boolean or a hash. When setting output to a hash, you'll always get a hash as the return value, even for a single signature. When setting a boolean, you will only get true if all of the signatures verify. This seems like a more intentional, clear API.

[mattcollier]

@dlongley I like the idea of telling verify to always return a hash. However the unusual/possibly unexpected detail about the boolean option (which would presumably be the default for backward compatibility) is that in the case of a single signature, an error is returned for a wide variety of failures. These errors cannot be reported in any fashion in the boolean instance.

What occurred to me in reference to this conundrum is that we could release a breaking/major release that always returns a hash (at least by default) and does not return errors in any case as now, but adds the error to the hash as implemented here in the multisignatures algorithm.

[dlongley]

@mattcollier,

If we're going to do a major breaking change to always return something other than a boolean, then I think we should make it more friendly for the single signature case, namely, the output should have some meta data at the top level and then have a hash for individual signatures. For example:

{
  verified: true, // true if all signatures passed, false otherwise
  // the detailed hash
  keyResults: {
  }
}

[davidlehn]

I also think the verify output, regardless of type or types of signatures, should have a single flag to indicate success or failure.

[davidlehn]

This processing should happen after the normalize() call:

• The input might be using some alias other than "signature" for the signature property (or even no alias).
• It would be better to leave the input unmodified and do manipulations on the new object that normalize() creates.

[davidlehn]

The README will need updates. It might be easier to discuss and review the API and proposed error handling method if that was updated earlier rather than later.

[mattcollier]

@dlongley @davidlehn ready for another pass.

[davidlehn]

Looking at the verify() API I'm wondering if it might make sense to use a different style for async data options such as publicKey:

• Just name the option publicKey.
• if it's a function it would have a signature like the getPublicKey that was added.
• If it's a promise it resolves to the data.
• If it's an object it's just the data (internally can wrap it in a promise or something as needed).

Advantages are:

• There is just one obvious name.
• Simple case of just data is easy for callers.
• Easy to support the three above styles to pass data.

The code for this is simple but could probably be reused for each option or in other APIs. Does it make sense to use that sort of style here?