feat: implement verifier manager #74

Nebulis · 2019-12-12T07:56:35Z

Quick summary

So the grand idea is that we will have a verificationManager that can take in verificationMethods

so each verificationMethod will have:
- a test to check if that method will run
- the verification mechanism itself

so, for the current one, we will have likely:
- OpenAttestationTamperCheck
- OpenAttestationEthereumDocumentStoreIssued
- OpenAttestationEthereumDocumentStoreRevoked
- OpenAttestationDnsTxtIdentity

it will also be loaded with:
- OpenCertsRegistryIdentity

each of these will be an object 
{
  test: (Document) => boolean,
  verify: (Document) => VerificationFragment
}

A VerificationFragement will be like: 

{
  type: "OpenAttestationDnsTxtIdentity",
  data: "mpa.gov.sg",
  message: "Error or warning message",
  status: "VALID" | "WARNING" | "ERROR"
}

The verificationManager will summarise all the Fragments and provide a summary as well (for those dev who are too lazy to go further)

The verification manager can also take in conditions to determine the summary status: ie Tamper && Issued && Revoke && (DNS || Registry), etc

Implementation details

verificationManager is a function that takes a list of verifiers and return a function. The function expect a signed document and some options as parameter. Options parameter expect the following:
- a mandatory network string (for instance ropsten)
- an optional infuraApiKey string
- an optional promisesCallback callback function that will provide back the promises resolving to the verifications.
There are 4 verifiers:
- openAttestationTamperCheck
- openAttestationEthereumDocumentStoreIssued
- openAttestationEthereumDocumentStoreRevoked
- openAttestationDnsTxtIdentity
There is a defaultVerificationManager pre-built having in parameter all the verifiers available in this repository
There are 2 validations functions: isValid and isInvalid
A verifier is an object that expect 3 functions:
- test: who must return true or false. The function must indicate whether condition are fulfilled for the verifier to run on a specific signed document
- verify: who must return the result of the verification as a VerificationFragment
- skip: who must return the result of a verification when it's skipped by providing additional data on why the validation didn't run.

TODO

add tests

src/common/smartContract/contractInstance.ts

src/verifiers/openAttestationDnsTxtIdentity.ts

Nebulis · 2019-12-13T11:34:42Z

I've added tests for verifiers (testing the verifier only not internals)
Didn't add test for the managers => there are already integration tests
Removed apikey as discussed

src/verifiers/verificationManager.ts

yehjxraymond · 2019-12-18T07:09:20Z

src/verify.integration.test.ts

-          revokedOnAny: false,
+        message: "Certificate has not been issued",
+        status: "INVALID",
+        type: "OpenAttestationEthereumDocumentStoreIssued"


Am thinking that the type should be succinct and general, serving more as a class of verification. We can put these as name of the verification method instead.

Valid class of checks:
INTEGRITY class verification methods checks if the document has been tampered with.
ISSUANCE_STATUES class verification methods checks if the document has been issued and is in the right status (ie. not revoked)
ISSUER_IDENTITY class verification methods checks for the issuer's identity.

In this case the default methods will be:
OpenAttestationTamperCheck -> INTEGRITY
OpenAttestationEthereumDocumentStoreIssued => ISSUANCE_STATUS
OpenAttestationEthereumDocumentStoreRevoked => ISSUANCE_STATUS
OpenAttestationDnsTxtIdentity => ISSUER_IDENTITY

Possible extension will look like:
OpenAttestationEthereumTokenRegistryIssued => ISSUANCE_STATUS
OpenCertsIdentityRegistry => ISSUER_IDENTITY
SomeCountryIdentityRegistry => ISSUER_IDENTITY
CustomExpiryCheck => ISSUANCE_STATUS

Using a known class, it can allow client application to check what type of checks has been performed it they like.

yehjxraymond · 2019-12-18T07:13:12Z

src/verify.integration.test.ts

-    });
+      {
+        message: 'Document issuers doesn\'t have "documentStore" or "token" property',
+        status: "SKIPPED",


Just a question...

If I have the OpenCertsRegistry extension:

This method will show skipped

OpenCertsRegistry method will show valid

How should I go about handling it in the method isValid?

src/index.ts

yehjxraymond · 2019-12-18T08:06:07Z

src/index.ts

-  ReturnType<typeof verifyIssued>,
-  ReturnType<typeof verifyRevoked>,
-  Promise<boolean>
+import { verificationManager } from "./verifiers/verificationManager";


Suggested change

import { verificationManager } from "./verifiers/verificationManager";

import { verificationBuilder } from "./verifiers/verificationBuilder";

yehjxraymond · 2019-12-18T08:06:35Z

src/index.ts

-    .catch(() => false);
-
-  return [hash, issued, revoked, valid];
+const defaultVerificationManager = verificationManager(defaultVerifiers);


Suggested change

const defaultVerificationManager = verificationManager(defaultVerifiers);

const verify = verificationBuilder(defaultVerifiers);

yehjxraymond · 2019-12-18T08:07:49Z

src/index.ts

+export {
+  verificationManager,
+  isValid,
+  isInvalid,
+  openAttestationEthereumDocumentStoreIssued,
+  openAttestationDnsTxtIdentity,
+  openAttestationTamperCheck,
+  openAttestationEthereumDocumentStoreRevoked,
+  defaultVerifiers,
+  defaultVerificationManager
 };


export { verify, verificationBuilder, isValid, isInvalid, defaultVerifiers, verifiers }

yehjxraymond · 2019-12-18T08:09:09Z

src/index.ts

-
-  return [hash, issued, revoked, valid];
+const defaultVerificationManager = verificationManager(defaultVerifiers);
+const isValid = (verificationFragments: VerificationFragment[]) =>


isValid(document, requiredChecksClasses)

usage example
isValid(document, ["ISSUER_IDENTITY", "ISSUANCE_STATUS", "INTEGRITY"])

Basically isValid checks the fragments that EVERY required checks classes to have at EVERY test either "VALID" or "SKIPPED" and has at least one "VALID".

For example for a documentStore
issued: valid
revoked: invalid
will result in "invalid"

In another case:
issued: valid
revoked: skipped
will result in "valid"

src/types/core.ts

src/hash/hash.ts

src/issued/contractInterface.ts

src/revoked/contractInterface.integration.test.ts

yehjxraymond · 2019-12-26T04:55:38Z

src/verifiers/openAttestationEthereumTokenRegistryMinted.ts

+      return documentData.proof.method === v3.Method.TokenRegistry;
+    }
+    const documentData = getData(document);
+    return documentData.issuers.every(issuer => "tokenRegistry" in issuer);


For token registry, we should check of any/some instead of every.

The reason is that if it's using tokenRegistry it MUST be only one issuer, cannot mix it around with documentstore or other tokenregistry.

In the verify function we can throw INVALID if it has been mixed

done, returning error if more than one token registry or mix of methods

yehjxraymond · 2019-12-26T04:57:22Z

src/verifiers/openAttestationEthereumDocumentStoreIssued.ts

+      return documentData.proof.method === v3.Method.DocumentStore;
+    }
+    const documentData = getData(document);
+    return documentData.issuers.every(issuer => "documentStore" in issuer || "certificateStore" in issuer);


Should we test using some instead?
This will make the verify run for documents that mixes around documentStore and other methods, and that's where this function should throw?

done, returning error if one of the issuer is invalid (i.e. doesnt have document store or certificate store)

BREAKING CHANGE implements https://github.com/Open-Attestation/adr/blob/master/verifier.md

john-dot-oa · 2019-12-27T05:09:32Z

🎉 This PR is included in version 2.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Nebulis force-pushed the feat/verifier-manager branch from 661d150 to e95508b Compare December 12, 2019 10:59

rjchow reviewed Dec 13, 2019

View reviewed changes

src/common/smartContract/contractInstance.ts Outdated Show resolved Hide resolved

src/verifiers/openAttestationDnsTxtIdentity.ts Outdated Show resolved Hide resolved

rjchow approved these changes Dec 13, 2019

View reviewed changes

Nebulis commented Dec 16, 2019

View reviewed changes

src/verifiers/verificationManager.ts Outdated Show resolved Hide resolved

yehjxraymond reviewed Dec 18, 2019

View reviewed changes

Nebulis commented Dec 18, 2019

View reviewed changes

src/index.ts Outdated Show resolved Hide resolved

yehjxraymond reviewed Dec 18, 2019

View reviewed changes

Nebulis mentioned this pull request Dec 18, 2019

feat: upgrade to open attestation v3 #73

Closed

Nebulis force-pushed the feat/verifier-manager branch from 0d43910 to fc71611 Compare December 26, 2019 04:14

yehjxraymond reviewed Dec 26, 2019

View reviewed changes

Nebulis mentioned this pull request Dec 26, 2019

Export utility functions for providers #75

Closed

feat: implement verifier manager

5b569b1

BREAKING CHANGE implements https://github.com/Open-Attestation/adr/blob/master/verifier.md

Nebulis force-pushed the feat/verifier-manager branch from 7012bd0 to 5b569b1 Compare December 27, 2019 05:05

Nebulis merged commit 02c6305 into master Dec 27, 2019

Nebulis deleted the feat/verifier-manager branch December 27, 2019 05:08

john-dot-oa added the released label Dec 27, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement verifier manager #74

feat: implement verifier manager #74

Nebulis commented Dec 12, 2019 •

edited

Loading

Nebulis commented Dec 13, 2019

yehjxraymond Dec 18, 2019 •

edited

Loading

Nebulis Dec 19, 2019

yehjxraymond Dec 18, 2019

yehjxraymond Dec 18, 2019

Nebulis Dec 19, 2019

yehjxraymond Dec 18, 2019

Nebulis Dec 19, 2019

yehjxraymond Dec 18, 2019

yehjxraymond Dec 18, 2019

yehjxraymond Dec 18, 2019

Nebulis Dec 19, 2019

yehjxraymond Dec 26, 2019

Nebulis Dec 26, 2019

yehjxraymond Dec 26, 2019

Nebulis Dec 26, 2019

john-dot-oa commented Dec 27, 2019

	import { verificationManager } from "./verifiers/verificationManager";
	import { verificationBuilder } from "./verifiers/verificationBuilder";

	const defaultVerificationManager = verificationManager(defaultVerifiers);
	const verify = verificationBuilder(defaultVerifiers);

feat: implement verifier manager #74

feat: implement verifier manager #74

Conversation

Nebulis commented Dec 12, 2019 • edited Loading

Implementation details

TODO

Nebulis commented Dec 13, 2019

yehjxraymond Dec 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

john-dot-oa commented Dec 27, 2019

Nebulis commented Dec 12, 2019 •

edited

Loading

yehjxraymond Dec 18, 2019 •

edited

Loading