Skip to content

Commit

Permalink
fix: handle empty object as input
Browse files Browse the repository at this point in the history
3 verifiers trigger an error when an empty object is passed down:
- OpenAttestationEthereumDocumentStoreStatus
- OpenAttestationDnsTxt
- OpenAttestationEthereumTokenRegistryStatus
For all those verifiers the error happens during the test function (which determines if a verifier should run or not).
The plan is to fix those test functions so that the verifiers doesn't run if issuers property (in that case) is not provided => the result for those verifiers will be skipped fragment.
On top of that OpenAttestationHash verifier currently return invalid while it shouldn't run because targethash/merkleroot/data are missing => the plan is to fix this behaviour so that the verifier is skipped in that case.
Also to make sure that verifiers test function handle correctly, we should provide a partial object (deeply)

closes #111
  • Loading branch information
Nebulis committed Sep 3, 2020
1 parent 5d301f6 commit d34a56f
Show file tree
Hide file tree
Showing 12 changed files with 323 additions and 12 deletions.
4 changes: 3 additions & 1 deletion commitlint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,7 @@ module.exports = {
extends: ["@commitlint/config-conventional"],

// Add your own rules. See http://marionebl.github.io/commitlint
rules: {}
rules: {
"body-max-line-length": [0], // disable
},
};
6 changes: 5 additions & 1 deletion src/types/core.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { SignedWrappedDocument, v2, v3, WrappedDocument } from "@govtechsg/open-attestation";
import { Reason } from "./error";

type DeepPartial<T> = {
[P in keyof T]?: DeepPartial<T[P]>;
};

/**
* - network on which to run the verification (if needed to connect to ethereum), For instance "ropste" or "homestead"
* - promisesCallback callback function that will provide back the promises resolving to the verification fragment. It will be called before the promises are all resolved and thus give the possibility to consumers to perform their own extra checks.
Expand Down Expand Up @@ -54,7 +58,7 @@ export interface Verifier<
Data = any
> {
skip: (document: Document, options: Options) => Promise<SkippedVerificationFragment>;
test: (document: Document, options: Options) => boolean;
test: (document: DeepPartial<Document>, options: Options) => boolean;
verify: (document: Document, options: Options) => Promise<VerificationFragment<Data>>;
}
export type Hash = string;
Expand Down
1 change: 1 addition & 0 deletions src/types/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export enum OpenAttestationDnsTxtCode {
}
export enum OpenAttestationHashCode {
DOCUMENT_TAMPERED = 0,
SKIPPED = 2,
}

export interface EthersError extends Error {
Expand Down
11 changes: 8 additions & 3 deletions src/verifiers/dnsText/openAttestationDnsTxt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ const resolveIssuerIdentity = async (

const name = "OpenAttestationDnsTxt";
const type: VerificationFragmentType = "ISSUER_IDENTITY";
const isWrappedV2Document = (document: any): document is WrappedDocument<v2.OpenAttestationDocument> => {
return document.data && document.data.issuers;
};
export const openAttestationDnsTxt: Verifier<
WrappedDocument<v2.OpenAttestationDocument> | WrappedDocument<v3.OpenAttestationDocument>,
VerificationManagerOptions,
Expand All @@ -62,7 +65,10 @@ export const openAttestationDnsTxt: Verifier<
});
},
test: (document) => {
if (oaUtils.isWrappedV2Document(document)) {
if (oaUtils.isWrappedV3Document(document)) {
const documentData = getData(document);
return documentData.issuer.identityProof.type === v3.IdentityProofType.DNSTxt;
} else if (isWrappedV2Document(document)) {
const documentData = getData(document);
// at least one issuer uses DNS-TXT
return documentData.issuers.some((issuer) => {
Expand All @@ -72,8 +78,7 @@ export const openAttestationDnsTxt: Verifier<
);
});
}
const documentData = getData(document);
return documentData.issuer.identityProof.type === v3.IdentityProofType.DNSTxt;
return false;
},
verify: async (document, options) => {
try {
Expand Down
46 changes: 46 additions & 0 deletions src/verifiers/dnsText/openAttestationDnsTxt.v2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,52 @@ import { verificationBuilder } from "../verificationBuilder";
const verify = verificationBuilder([openAttestationDnsTxt]);
describe("OpenAttestationDnsTxt v2 document", () => {
describe("with one issuer", () => {
it("should return a skipped fragment when document does not have data", async () => {
const fragment = await verify(
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-expect-error
{ ...documentRopstenValidWithToken, data: null },
{
network: "ropsten",
}
);
expect(fragment).toStrictEqual([
{
type: "ISSUER_IDENTITY",
name: "OpenAttestationDnsTxt",
reason: {
code: 2,
codeString: "SKIPPED",
message:
'Document issuers doesn\'t have "documentStore" / "tokenRegistry" property or doesn\'t use DNS-TXT type',
},
status: "SKIPPED",
},
]);
});
it("should return a skipped fragment when document does not have issuers", async () => {
const fragment = await verify(
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-expect-error
{ ...documentRopstenValidWithToken, data: { ...documentRopstenValidWithToken.data, issuers: null } },
{
network: "ropsten",
}
);
expect(fragment).toStrictEqual([
{
type: "ISSUER_IDENTITY",
name: "OpenAttestationDnsTxt",
reason: {
code: 2,
codeString: "SKIPPED",
message:
'Document issuers doesn\'t have "documentStore" / "tokenRegistry" property or doesn\'t use DNS-TXT type',
},
status: "SKIPPED",
},
]);
});
it("should return a valid fragment when document has valid identity", async () => {
const fragment = await verify(documentRopstenValidWithToken, {
network: "ropsten",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,70 @@ const verify = verificationBuilder([openAttestationEthereumDocumentStoreStatus])

describe("OpenAttestationEthereumDocumentStoreStatus", () => {
describe("v2", () => {
it("should return a skipped fragment when document does not have data", async () => {
const fragment = await verify(
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-expect-error
{ ...v2documentRopstenValidWithDocumentStore, data: null },
{
network: "ropsten",
}
);
expect(fragment).toStrictEqual([
{
name: "OpenAttestationEthereumDocumentStoreStatus",
type: "DOCUMENT_STATUS",
reason: {
code: 4,
codeString: "SKIPPED",
message:
'Document issuers doesn\'t have "documentStore" or "certificateStore" property or DOCUMENT_STORE method',
},
status: "SKIPPED",
},
]);
});
it("should return a skipped fragment when document does not have issuers", async () => {
const fragment = await verify(
{
...v2documentRopstenValidWithDocumentStore,
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-expect-error
data: { ...v2documentRopstenValidWithDocumentStore.data, issuers: null },
},
{
network: "ropsten",
}
);
expect(fragment).toStrictEqual([
{
name: "OpenAttestationEthereumDocumentStoreStatus",
type: "DOCUMENT_STATUS",
reason: {
code: 4,
codeString: "SKIPPED",
message:
'Document issuers doesn\'t have "documentStore" or "certificateStore" property or DOCUMENT_STORE method',
},
status: "SKIPPED",
},
]);
});
it("should return a skipped fragment when document uses token registry", async () => {
const fragment = await verify(documentRopstenNotIssuedWithTokenRegistry, {
network: "ropsten",
});
expect(fragment).toStrictEqual([
{
name: "OpenAttestationEthereumDocumentStoreStatus",
type: "DOCUMENT_STATUS",
reason: {
code: 4,
codeString: "SKIPPED",
message:
'Document issuers doesn\'t have "documentStore" or "certificateStore" property or DOCUMENT_STORE method',
},
status: "SKIPPED",
type: "DOCUMENT_STATUS",
},
]);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ export const isAnyHashRevoked = async (smartContract: DocumentStore, intermediat
return revokedStatuses.find((hash) => hash);
};

const isWrappedV2Document = (document: any): document is WrappedDocument<v2.OpenAttestationDocument> => {
return document.data && document.data.issuers;
};
export const openAttestationEthereumDocumentStoreStatus: Verifier<
WrappedDocument<v2.OpenAttestationDocument> | WrappedDocument<v3.OpenAttestationDocument>
> = {
Expand All @@ -69,9 +72,11 @@ export const openAttestationEthereumDocumentStoreStatus: Verifier<
if (utils.isWrappedV3Document(document)) {
const documentData = getData(document);
return documentData.proof.method === v3.Method.DocumentStore;
} else if (isWrappedV2Document(document)) {
const documentData = getData(document);
return documentData.issuers.some((issuer) => "documentStore" in issuer || "certificateStore" in issuer);
}
const documentData = getData(document);
return documentData.issuers.some((issuer) => "documentStore" in issuer || "certificateStore" in issuer);
return false;
},
verify: async (document, options): Promise<VerificationFragment<DocumentStoreStatusFragment>> => {
try {
Expand Down
67 changes: 67 additions & 0 deletions src/verifiers/hash/openAttestationHash.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,73 @@ import { verificationBuilder } from "../verificationBuilder";
const verify = verificationBuilder([openAttestationHash]);

describe("OpenAttestationHash", () => {
it("should return a skipped fragment when document is missing target hash", async () => {
const newDocument = {
...tamperedDocumentWithCertificateStore,
signature: { ...tamperedDocumentWithCertificateStore.signature },
};
delete newDocument.signature.targetHash;
const fragment = await verify(newDocument, {
network: "ropsten",
});
expect(fragment).toStrictEqual([
{
name: "OpenAttestationHash",
type: "DOCUMENT_INTEGRITY",
reason: {
code: 2,
codeString: "SKIPPED",
message: "Document does not have merkle root, target hash or data.",
},
status: "SKIPPED",
},
]);
});
it("should return a skipped fragment when document is missing merkle root", async () => {
const newDocument = {
...tamperedDocumentWithCertificateStore,
signature: { ...tamperedDocumentWithCertificateStore.signature },
};
delete newDocument.signature.merkleRoot;
const fragment = await verify(newDocument, {
network: "ropsten",
});
expect(fragment).toStrictEqual([
{
name: "OpenAttestationHash",
type: "DOCUMENT_INTEGRITY",
reason: {
code: 2,
codeString: "SKIPPED",
message: "Document does not have merkle root, target hash or data.",
},
status: "SKIPPED",
},
]);
});
it("should return a skipped fragment when document is missing data", async () => {
const newDocument = {
...tamperedDocumentWithCertificateStore,
data: { ...tamperedDocumentWithCertificateStore.data },
};
delete newDocument.data;
const fragment = await verify(newDocument, {
network: "ropsten",
});
expect(fragment).toStrictEqual([
{
name: "OpenAttestationHash",
type: "DOCUMENT_INTEGRITY",
reason: {
code: 2,
codeString: "SKIPPED",
message: "Document does not have merkle root, target hash or data.",
},
status: "SKIPPED",
},
]);
});

it("should return an invalid fragment when document has been tampered", async () => {
const fragment = await verify(tamperedDocumentWithCertificateStore, { network: "" });
expect(fragment).toStrictEqual([
Expand Down
15 changes: 13 additions & 2 deletions src/verifiers/hash/openAttestationHash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,20 @@ const name = "OpenAttestationHash";
const type: VerificationFragmentType = "DOCUMENT_INTEGRITY";
export const openAttestationHash: Verifier = {
skip: () => {
throw new Error("This verifier is never skipped");
return Promise.resolve({
status: "SKIPPED",
type,
name,
reason: {
code: OpenAttestationHashCode.SKIPPED,
codeString: OpenAttestationHashCode[OpenAttestationHashCode.SKIPPED],
message: `Document does not have merkle root, target hash or data.`,
},
});
},
test: (document) => {
return !!document?.signature?.merkleRoot && !!document?.signature?.targetHash && !!document.data;
},
test: () => true,
verify: async (document) => {
const hash = await verifySignature(document);
if (!hash) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,53 @@ const verify = verificationBuilder([openAttestationEthereumTokenRegistryStatus])

describe("openAttestationEthereumTokenRegistryStatus", () => {
describe("v2", () => {
it("should return a skipped fragment when document does not have data", async () => {
const fragment = await verify(
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-expect-error
{ ...documentRopstenValidWithToken, data: null },
{
network: "ropsten",
}
);
expect(fragment).toStrictEqual([
{
name: "OpenAttestationEthereumTokenRegistryStatus",
reason: {
code: 4,
codeString: "SKIPPED",
message: 'Document issuers doesn\'t have "tokenRegistry" property or TOKEN_REGISTRY method',
},
status: "SKIPPED",
type: "DOCUMENT_STATUS",
},
]);
});
it("should return a skipped fragment when document does not have issuers", async () => {
const fragment = await verify(
{
...documentRopstenValidWithToken,
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-expect-error
data: { ...documentRopstenValidWithToken.data, issuers: null },
},
{
network: "ropsten",
}
);
expect(fragment).toStrictEqual([
{
name: "OpenAttestationEthereumTokenRegistryStatus",
reason: {
code: 4,
codeString: "SKIPPED",
message: 'Document issuers doesn\'t have "tokenRegistry" property or TOKEN_REGISTRY method',
},
status: "SKIPPED",
type: "DOCUMENT_STATUS",
},
]);
});
it("should return a skipped fragment when document uses certificate store", async () => {
const fragment = await verify(documentRopstenNotIssuedWithCertificateStore, {
network: "ropsten",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ interface Status {
address: string;
reason?: any;
}

const isWrappedV2Document = (document: any): document is WrappedDocument<v2.OpenAttestationDocument> => {
return document.data && document.data.issuers;
};
const name = "OpenAttestationEthereumTokenRegistryStatus";
const type: VerificationFragmentType = "DOCUMENT_STATUS";
export const openAttestationEthereumTokenRegistryStatus: Verifier<
Expand All @@ -33,9 +37,11 @@ export const openAttestationEthereumTokenRegistryStatus: Verifier<
if (utils.isWrappedV3Document(document)) {
const documentData = getData(document);
return documentData.proof.method === v3.Method.TokenRegistry;
} else if (isWrappedV2Document(document)) {
const documentData = getData(document);
return documentData.issuers.some((issuer) => "tokenRegistry" in issuer);
}
const documentData = getData(document);
return documentData.issuers.some((issuer) => "tokenRegistry" in issuer);
return false;
},
verify: async (document, options) => {
try {
Expand Down
Loading

0 comments on commit d34a56f

Please sign in to comment.