Added conditional logic following the ConditionalProof specification

[theblockstalk]

Adds the ability to verify JWTs with multi-signatures and delegated signatures and combinations of the two, against DIDs that follow the ConditionalProof specification.

https://github.com/w3c-ccg/verifiable-conditions

See slide deck for overview of changes: https://docs.google.com/presentation/d/13jaeireCRFoI1jFPFkbwBhTMQgUG7zEHRjmq0PWz-CI

[mirceanis]

Apologies for coming back to this review so late.

It seems that verifyJWS functionality is being changed with the expectation that the data is a JWT payload.
This is not a valid assumption and since it is an exported function the changes related to that should be reverted.

[theblockstalk]

I have made changes per your comments.

I have also added test suite ConditionalAlgorithm.test.ts which covers many scenarios. Test suite is now passing all tests.

[mirceanis]

This looks good.

I think there's a potential error introduced from trying to parse the issuer DID.
Can you clarify the intent there?

[mirceanis]

This comparison assumes that a didUrl that does not match the parsed DID refers to one of the verification methods.
The assumption will break if the didUrl contains, for example, query parameters.

Example: did:ethr:0xabcd...?versionId=1234.
This is a valid issuer with a DID document potentially different from did:ethr:0xabcd...

Is the intent here to avoid looping through the authenticators array?

[theblockstalk]

if the issuer of the DID is a DID URL, here we assume that it is pointing to a specific verification method in the DID Document. This is used in the recursive calling to verifyJWT() for delegation

did:ethr is not a good example, as it cannot express multi-sig conditions or delegations.

Take for example two DIDs which one verification method on each:

1. did:antelope:jack#delegated which delegates to did:antelope:thomas#base
2. did:antelope:thomas#base with a ES256K keypair

A VC is issued by did:antelope:jack#delegated. The VC is signed by the did:antelope:thomas#base keypair.

• verifyJWT() is called which finds the issuer to be did:antelope:jack#delegated
• it goes and finds the specific verification method, which tells that it is delegating to did:antelope:thomas#base
• verifyJWT() is then called recursively to check whether the signature matches the did:antelope:thomas#base which it does.

It is important to note that at each of the above steps, a specific verification method is found using the DID URL, required to make the above flow work.

[mirceanis]

Thank you for clarifying the intent.
I realize did:ethr is not relevant to the conditional verification, but the changes I'm asking about are relevant for all verification, not just conditional.

I'll add a test for the case I'm referring to.

[theblockstalk]

Yes I see your concern. If someone issues using a DIDUrl that does not point directly to a verification method, it will now fail. Correct?

[mirceanis]

exactly

[mirceanis]

That's one possibility.
Another option would be to just try to find the verification method, which would work for the conditional logic you need, but then to not fail if it's not found, which would continue to do the verification as before.

[theblockstalk]

JWT.ts verifyJWT()

  const { fragment } = parse(didUrl) as ParsedDID

  let signer: VerificationMethod | null = null

  // If a fragment is supplied, assume it is pointing to a specific Verification Method
  // This is a required feature for Conditional Proof delegated signatures
  if (fragment) {
    const authenticator = authenticators.find((auth) => (parse(auth.id) as ParsedDID).fragment === fragment)

this would avoid this situation (not 100%, e.g. if someone used fragments to refer to a non verification method it would still break)

[mirceanis]

Your solution with the fragment seems like the right approach, but somehow comparing just the fragment parts seems odd, as it seems close to being able to impersonate some other DID.

How about this pseudocode?

if (didURL contains fragment) {
  find authenticator that matches didUrl
} else {
  try to check the proof against all authenticators
}

[theblockstalk]

the original intent, was to treat the DID URL as a relative URL, which are assumed to dereference to a resource in the DID Document

[mirceanis]

OK, that's even better, as it would also cover the case where the issuer is a URL but there's also a kid specified in the header, relative to the issuer URL.

I'll merge this PR as is and make the relative kid changes as a separate item ( relates to #267 ).

[uport-automation-bot]

🎉 This PR is included in version 7.1.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀