From d05d4e5da51f30d5c741a6d0528f1395052922ea Mon Sep 17 00:00:00 2001 From: Emma Casolin Date: Mon, 14 Feb 2022 10:59:28 +1100 Subject: [PATCH] Fixing the GRPC `toError()` bug with `ErrorValidation` --- src/validation/errors.ts | 37 +++++++++++++++---- src/validation/index.ts | 8 ++-- .../service/identitiesAuthenticate.test.ts | 14 ++++++- tests/client/service/identitiesClaim.test.ts | 28 ++++++++++++++ tests/client/service/nodesAdd.test.ts | 34 +++++++++++++++++ tests/client/service/nodesClaim.test.ts | 11 ++++++ tests/client/service/nodesFind.test.ts | 11 ++++++ tests/client/service/nodesPing.test.ts | 11 ++++++ 8 files changed, 141 insertions(+), 13 deletions(-) diff --git a/src/validation/errors.ts b/src/validation/errors.ts index 2322ef90c0..e918ce38b0 100644 --- a/src/validation/errors.ts +++ b/src/validation/errors.ts @@ -2,24 +2,45 @@ import { CustomError } from 'ts-custom-error'; import { ErrorPolykey, sysexits } from '../errors'; /** - * This packages the `ErrorParse` array into the `data` property - * This is to allow encoding to and decoding from GRPC errors + * Generic error containing all parsing errors that occurred during + * execution. */ class ErrorValidation extends ErrorPolykey { - public readonly errors: Array; - constructor(errors: Array) { + description = 'Input data failed validation'; + exitCode = sysexits.DATAERR; + public errors: Array; + constructor(message, data) { + super(message, data); + if (data.errors != null) { + const errors: Array = []; + for (const eData of data.errors) { + const errorParse = new ErrorParse(eData.message); + errorParse.keyPath = eData.keyPath; + errorParse.value = eData.value; + errorParse.context = eData.context; + errors.push(errorParse); + } + this.errors = errors; + } + } + + /** + * This packages an `ErrorParse` array into the `data` property + * This is to allow encoding to and decoding from GRPC errors + */ + static createFromErrors(errors: Array): ErrorValidation { const message = errors.map((e) => e.message).join('; '); const data = { errors: errors.map((e) => ({ message: e.message, keyPath: e.keyPath, value: e.value.valueOf(), + context: e.context, })), }; - super(message, data); - this.description = 'Input data failed validation'; - this.exitCode = sysexits.DATAERR; - this.errors = errors; + const e = new ErrorValidation(message, data); + e.errors = errors; + return e; } } diff --git a/src/validation/index.ts b/src/validation/index.ts index 295e63c89f..87164a2fa8 100644 --- a/src/validation/index.ts +++ b/src/validation/index.ts @@ -41,13 +41,13 @@ async function validate( data = await parse_([], data, { undefined: data }); } catch (e) { if (e instanceof validationErrors.ErrorParse) { - throw new validationErrors.ErrorValidation(errors); + throw validationErrors.ErrorValidation.createFromErrors(errors); } else { throw e; } } if (errors.length > 0) { - throw new validationErrors.ErrorValidation(errors); + throw validationErrors.ErrorValidation.createFromErrors(errors); } return data; } @@ -88,13 +88,13 @@ function validateSync( data = parse_([], data, { undefined: data }); } catch (e) { if (e instanceof validationErrors.ErrorParse) { - throw new validationErrors.ErrorValidation(errors); + throw validationErrors.ErrorValidation.createFromErrors(errors); } else { throw e; } } if (errors.length > 0) { - throw new validationErrors.ErrorValidation(errors); + throw validationErrors.ErrorValidation.createFromErrors(errors); } return data; } diff --git a/tests/client/service/identitiesAuthenticate.test.ts b/tests/client/service/identitiesAuthenticate.test.ts index cd273de329..0d45a68969 100644 --- a/tests/client/service/identitiesAuthenticate.test.ts +++ b/tests/client/service/identitiesAuthenticate.test.ts @@ -16,6 +16,7 @@ import { import identitiesAuthenticate from '@/client/service/identitiesAuthenticate'; import * as identitiesPB from '@/proto/js/polykey/v1/identities/identities_pb'; import { utils as nodesUtils } from '@/nodes'; +import * as validationErrors from '@/validation/errors'; import TestProvider from '../../identities/TestProvider'; describe('identitiesAuthenticate', () => { @@ -87,7 +88,6 @@ describe('identitiesAuthenticate', () => { test('authenticates identity', async () => { const request = new identitiesPB.Provider(); request.setProviderId(testToken.providerId); - request.setIdentityId(testToken.identityId); const response = grpcClient.identitiesAuthenticate( request, clientUtils.encodeAuthFromPassword(password), @@ -123,4 +123,16 @@ describe('identitiesAuthenticate', () => { testToken.identityId, ); }); + test('cannot authenticate invalid provider', async () => { + const request = new identitiesPB.Provider(); + request.setProviderId(''); + await expect( + grpcClient + .identitiesAuthenticate( + request, + clientUtils.encodeAuthFromPassword(password), + ) + .next(), + ).rejects.toThrow(validationErrors.ErrorValidation); + }); }); diff --git a/tests/client/service/identitiesClaim.test.ts b/tests/client/service/identitiesClaim.test.ts index 88b00e205c..f4b42c2775 100644 --- a/tests/client/service/identitiesClaim.test.ts +++ b/tests/client/service/identitiesClaim.test.ts @@ -22,6 +22,7 @@ import { import identitiesClaim from '@/client/service/identitiesClaim'; import * as identitiesPB from '@/proto/js/polykey/v1/identities/identities_pb'; import * as claimsUtils from '@/claims/utils'; +import * as validationErrors from '@/validation/errors'; import TestProvider from '../../identities/TestProvider'; import * as testUtils from '../../utils'; @@ -199,4 +200,31 @@ describe('identitiesClaim', () => { testToken.identityId, ); }); + test('cannot claim invalid identity', async () => { + const request = new identitiesPB.Provider(); + request.setIdentityId(''); + request.setProviderId(testToken.providerId); + await expect( + grpcClient.identitiesClaim( + request, + clientUtils.encodeAuthFromPassword(password), + ), + ).rejects.toThrow(validationErrors.ErrorValidation); + request.setIdentityId(testToken.identityId); + request.setProviderId(''); + await expect( + grpcClient.identitiesClaim( + request, + clientUtils.encodeAuthFromPassword(password), + ), + ).rejects.toThrow(validationErrors.ErrorValidation); + request.setIdentityId(''); + request.setProviderId(''); + await expect( + grpcClient.identitiesClaim( + request, + clientUtils.encodeAuthFromPassword(password), + ), + ).rejects.toThrow(validationErrors.ErrorValidation); + }); }); diff --git a/tests/client/service/nodesAdd.test.ts b/tests/client/service/nodesAdd.test.ts index 740cd5f4ff..2b77f6312d 100644 --- a/tests/client/service/nodesAdd.test.ts +++ b/tests/client/service/nodesAdd.test.ts @@ -19,6 +19,7 @@ import nodesAdd from '@/client/service/nodesAdd'; import * as nodesPB from '@/proto/js/polykey/v1/nodes/nodes_pb'; import * as utilsPB from '@/proto/js/polykey/v1/utils/utils_pb'; import { utils as nodesUtils } from '@/nodes'; +import * as validationErrors from '@/validation/errors'; import * as testUtils from '../../utils'; describe('nodesAdd', () => { @@ -155,4 +156,37 @@ describe('nodesAdd', () => { expect(result!.host).toBe('127.0.0.1'); expect(result!.port).toBe(11111); }); + test('cannot add invalid node', async () => { + // Invalid host + const addressMessage = new nodesPB.Address(); + addressMessage.setHost(''); + addressMessage.setPort(11111); + const request = new nodesPB.NodeAddress(); + request.setNodeId('vrsc24a1er424epq77dtoveo93meij0pc8ig4uvs9jbeld78n9nl0'); + request.setAddress(addressMessage); + await expect( + grpcClient.nodesAdd( + request, + clientUtils.encodeAuthFromPassword(password), + ), + ).rejects.toThrow(validationErrors.ErrorValidation); + // Invalid port + addressMessage.setHost('127.0.0.1'); + addressMessage.setPort(111111); + await expect( + grpcClient.nodesAdd( + request, + clientUtils.encodeAuthFromPassword(password), + ), + ).rejects.toThrow(validationErrors.ErrorValidation); + // Invalid nodeid + addressMessage.setPort(11111); + request.setNodeId('nodeId'); + await expect( + grpcClient.nodesAdd( + request, + clientUtils.encodeAuthFromPassword(password), + ), + ).rejects.toThrow(validationErrors.ErrorValidation); + }); }); diff --git a/tests/client/service/nodesClaim.test.ts b/tests/client/service/nodesClaim.test.ts index 8982e2df77..2c2da79287 100644 --- a/tests/client/service/nodesClaim.test.ts +++ b/tests/client/service/nodesClaim.test.ts @@ -22,6 +22,7 @@ import { import nodesClaim from '@/client/service/nodesClaim'; import * as nodesPB from '@/proto/js/polykey/v1/nodes/nodes_pb'; import * as utilsPB from '@/proto/js/polykey/v1/utils/utils_pb'; +import * as validationErrors from '@/validation/errors'; import * as testUtils from '../../utils'; describe('nodesClaim', () => { @@ -212,4 +213,14 @@ describe('nodesClaim', () => { expect(response).toBeInstanceOf(utilsPB.StatusMessage); expect(response.getSuccess()).toBeTruthy(); }); + test('cannot claim an invalid node', async () => { + const request = new nodesPB.Claim(); + request.setNodeId('nodeId'); + await expect( + grpcClient.nodesClaim( + request, + clientUtils.encodeAuthFromPassword(password), + ), + ).rejects.toThrow(validationErrors.ErrorValidation); + }); }); diff --git a/tests/client/service/nodesFind.test.ts b/tests/client/service/nodesFind.test.ts index 87d3def186..e09fe05119 100644 --- a/tests/client/service/nodesFind.test.ts +++ b/tests/client/service/nodesFind.test.ts @@ -17,6 +17,7 @@ import { } from '@/client'; import nodesFind from '@/client/service/nodesFind'; import * as nodesPB from '@/proto/js/polykey/v1/nodes/nodes_pb'; +import * as validationErrors from '@/validation/errors'; import * as testUtils from '../../utils'; describe('nodesFind', () => { @@ -153,4 +154,14 @@ describe('nodesFind', () => { expect(response.getAddress()!.getHost()).toBe('127.0.0.1'); expect(response.getAddress()!.getPort()).toBe(11111); }); + test('cannot find an invalid node', async () => { + const request = new nodesPB.Node(); + request.setNodeId('nodeId'); + await expect( + grpcClient.nodesFind( + request, + clientUtils.encodeAuthFromPassword(password), + ), + ).rejects.toThrow(validationErrors.ErrorValidation); + }); }); diff --git a/tests/client/service/nodesPing.test.ts b/tests/client/service/nodesPing.test.ts index 1d44b4d292..9633d1e068 100644 --- a/tests/client/service/nodesPing.test.ts +++ b/tests/client/service/nodesPing.test.ts @@ -18,6 +18,7 @@ import { import nodesPing from '@/client/service/nodesPing'; import * as nodesPB from '@/proto/js/polykey/v1/nodes/nodes_pb'; import * as utilsPB from '@/proto/js/polykey/v1/utils/utils_pb'; +import * as validationErrors from '@/validation/errors'; import * as testUtils from '../../utils'; describe('nodesPing', () => { @@ -158,4 +159,14 @@ describe('nodesPing', () => { expect(response).toBeInstanceOf(utilsPB.StatusMessage); expect(response.getSuccess()).toBeTruthy(); }); + test('cannot ping an invalid node', async () => { + const request = new nodesPB.Node(); + request.setNodeId('nodeId'); + await expect( + grpcClient.nodesPing( + request, + clientUtils.encodeAuthFromPassword(password), + ), + ).rejects.toThrow(validationErrors.ErrorValidation); + }); });