From 5bb63d252e4364468f01968a890eed131aa78280 Mon Sep 17 00:00:00 2001 From: marysieek Date: Thu, 4 Feb 2021 12:20:56 +0100 Subject: [PATCH 01/25] Fix track docs --- README.md | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/README.md b/README.md index 603aae0..8090bb5 100644 --- a/README.md +++ b/README.md @@ -62,18 +62,13 @@ This is the asynchronous version of the castle integration. This is for events w ```js import { EVENTS } from '@castleio/sdk'; -track({ +castle.track({ event: EVENTS.EMAIL_CHANGE_SUCCEEDED, user_id: user.id, user_traits: { email: user.email, registered_at: user.registered_at, }, - context: { - ip: request.ip, - client_id: request.cookies['__cid'], - headers: request.cookies, - }, }); ``` From 0a5cbf6b3a889ddb5ea021cc9fe6716c56af529d Mon Sep 17 00:00:00 2001 From: marysieek Date: Thu, 4 Feb 2021 12:21:23 +0100 Subject: [PATCH 02/25] Fix authenticate docs --- README.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/README.md b/README.md index 8090bb5..26077a7 100644 --- a/README.md +++ b/README.md @@ -86,11 +86,6 @@ try { email: user.email, registered_at: user.registered_at, }, - context: { - ip: request.ip, - client_id: request.cookies['__cid'], - headers: request.headers, - }, }); } catch (e) { console.error(e); From 2a8788273d2fe5f114d7881f2f73cd3b7766e5da Mon Sep 17 00:00:00 2001 From: marysieek Date: Thu, 4 Feb 2021 16:35:06 +0100 Subject: [PATCH 03/25] Update readme --- README.md | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 26077a7..b6e09b7 100644 --- a/README.md +++ b/README.md @@ -109,18 +109,12 @@ Response format ### All config options for `track` and `authenticate` -| Config option | Explanation | -| ------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | -| event | `string` - The event generated by the user. It can be either an event from `EVENTS` or a custom one. | -| user_id | `string` - The `user_id` of the end user. | -| user_traits | `object` - An optional, recommended, object containing user information, such as `email` and `registered_at`. | -| properties | `object` - An optional object containing custom information. | +| Config option | Explanation | +| ------------- | ----------- | +| event | `string` - The event generated by the user. It can be either an event from `EVENTS` or a custom one. | +| user_id | `string` - The `user_id` of the end user. | +| user_traits | `object` - An optional, recommended, object containing user information, such as `email` and `registered_at`. | +| properties | `object` - An optional object containing custom information. | | created_at | `string` - An optional ISO date string indicating when the event occurred, in cases where this might be different from the time when the request is made. | -| device_token | `string` - The optional device token, used for mitigating or escalating. | -| context | `object` - The request context information. See information below. | - -| Context option | Explanation | -| -------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| ip | `string` - The IP address of the request. Note that this needs to be the original request IP, not the IP of an internal proxy, such as nginx. | -| client_id | `string` - The client ID, generated by the `c.js` integration on the front end. Commonly found in the `__cid` cookie in `request.cookies` or `request.cookie`, or in some cases the `X-CASTLE-CLIENT-ID` header. | -| headers | `object` - The headers object on the request, commonly `request.headers`. | +| device_token | `string` - The optional device token, used for mitigating or escalating. | +| context | `object` - The request context information. | From 31520b442089af0fe1d9eb9d7739fa58ece2614c Mon Sep 17 00:00:00 2001 From: marysieek Date: Thu, 4 Feb 2021 17:50:58 +0100 Subject: [PATCH 04/25] Add authenticate sketch --- src/Castle.ts | 65 +---------- src/api/services/api-authenticate.service.ts | 112 ++++++++++++++++++- 2 files changed, 112 insertions(+), 65 deletions(-) diff --git a/src/Castle.ts b/src/Castle.ts index dcddc81..ab8e0cc 100644 --- a/src/Castle.ts +++ b/src/Castle.ts @@ -3,8 +3,8 @@ import AbortController from 'abort-controller'; import pino from 'pino'; import { AuthenticateResult, Payload } from './models'; +import { APIAuthenticateService } from './api/api.module'; import { - CommandAuthenticateService, CommandTrackService, } from './command/command.module'; import { @@ -61,49 +61,7 @@ export class Castle { ); } - let response: Response; - const controller = new AbortController(); - const timeout = setTimeout(() => { - controller.abort(); - }, this.configuration.timeout); - const { requestUrl, requestOptions } = CommandAuthenticateService.call( - controller, - params, - this.configuration - ); - - try { - response = await this.getFetch()(requestUrl, requestOptions); - } catch (err) { - LoggerService.call({ requestUrl, requestOptions, err }, this.logger); - - if (isTimeout(err)) { - return this.handleFailover(params.user_id, 'timeout', err); - } else { - throw err; - } - } finally { - clearTimeout(timeout); - } - - // Wait to get body here to prevent race conditions - // on `.json()` because we attempt to read it in - // multiple places. - const body = await getBody(response); - - LoggerService.call( - { requestUrl, requestOptions, response, body }, - this.logger - ); - - if (response.status >= 500) { - return this.handleFailover(params.user_id, 'server error'); - } - - this.handleUnauthorized(response); - this.handleBadResponse(response); - - return body; + return APIAuthenticateService.call(params, this.configuration, this.logger); } public async track(params: Payload): Promise { @@ -148,25 +106,6 @@ export class Castle { return this.configuration.overrideFetch || fetch; } - private handleFailover( - userId: string, - reason: string, - err?: Error - ): AuthenticateResult { - // Have to check it this way to make sure TS understands - // that this.failoverStrategy is of type Verdict, - // not FailoverStrategyType. - if (this.configuration.failoverStrategy === FailoverStrategy.throw) { - throw err; - } - - return FailoverResponsePrepareService.call( - userId, - reason, - this.configuration.failoverStrategy - ); - } - private handleUnauthorized(response: Response) { if (response.status === 401) { throw new Error( diff --git a/src/api/services/api-authenticate.service.ts b/src/api/services/api-authenticate.service.ts index e7240bf..c644d39 100644 --- a/src/api/services/api-authenticate.service.ts +++ b/src/api/services/api-authenticate.service.ts @@ -1,5 +1,113 @@ +import { Configuration } from '../../configuraton'; +import { AuthenticateResult, Payload } from '../../models'; +import { + CommandAuthenticateService, +} from '../../command/command.module'; +import { + FailoverResponsePrepareService, + FailoverStrategy, +} from '../../failover/failover.module'; +import { LoggerService } from '../../logger/logger.module'; +import fetch from 'node-fetch'; +import pino from 'pino'; + +// The body on the request is a stream and can only be +// read once, by default. This is a workaround so that the +// logging functions can read the body independently +// of the handlers. +const getBody = async (response: any) => { + if (response.cachedBody) { + return response.cachedBody; + } + + try { + response.cachedBody = await response.json(); + } catch (e) { + response.cachedBody = {}; + } + + return response.cachedBody; +}; + +const isTimeout = (e: Error) => e.name === 'AbortError'; + + // private handleFailover( + // userId: string, + // reason: string, + // err?: Error + // ): AuthenticateResult { + // // Have to check it this way to make sure TS understands + // // that this.failoverStrategy is of type Verdict, + // // not FailoverStrategyType. + // if (this.configuration.failoverStrategy === FailoverStrategy.throw) { + // throw err; + // } + + // return FailoverResponsePrepareService.call( + // userId, + // reason, + // this.configuration.failoverStrategy + // ); + // } + export const APIAuthenticateService = { - call: () => { - return; + call: async (params: Payload, configuration: Configuration, logger: pino.Logger): Promise => { + let response: Response; + const controller = new AbortController(); + const timeout = setTimeout(() => { + controller.abort(); + }, configuration.timeout); + const { requestUrl, requestOptions } = CommandAuthenticateService.call( + controller, + params, + configuration + ); + + const fetcher = configuration.overrideFetch || fetch + + try { + response = await fetcher()(requestUrl, requestOptions); + } catch (err) { + LoggerService.call({ requestUrl, requestOptions, err }, logger); + + if (isTimeout(err)) { + // return FailoverResponsePrepareService.call( + // params.user_id, + // 'server error', + // configuration.failoverStrategy + // ); + } else { + throw err; + } + } finally { + clearTimeout(timeout); + } + + // Wait to get body here to prevent race conditions + // on `.json()` because we attempt to read it in + // multiple places. + const body = await getBody(response); + + LoggerService.call( + { requestUrl, requestOptions, response, body }, + logger + ); + + if (response.status >= 500) { + // if (configuration.failoverStrategy === FailoverStrategy.throw) { + // throw 'server error'; + // } + + // return FailoverResponsePrepareService.call( + // params.user_id, + // 'server error', + // configuration.failoverStrategy + // ); + } + + this.handleUnauthorized(response); + this.handleBadResponse(response); + + return body; }, }; From 5b3770e9d63e7f74d54b2ecf0a7868f9650b2c1b Mon Sep 17 00:00:00 2001 From: marysieek Date: Fri, 5 Feb 2021 19:32:07 +0100 Subject: [PATCH 05/25] Revert changes in Castle.ts --- src/Castle.ts | 66 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/src/Castle.ts b/src/Castle.ts index ab8e0cc..75b30a3 100644 --- a/src/Castle.ts +++ b/src/Castle.ts @@ -3,10 +3,11 @@ import AbortController from 'abort-controller'; import pino from 'pino'; import { AuthenticateResult, Payload } from './models'; -import { APIAuthenticateService } from './api/api.module'; import { + CommandAuthenticateService, CommandTrackService, } from './command/command.module'; +import { CoreProcessResponseService } from './core/core.module'; import { FailoverResponsePrepareService, FailoverStrategy, @@ -61,7 +62,49 @@ export class Castle { ); } - return APIAuthenticateService.call(params, this.configuration, this.logger); + let response: Response; + const controller = new AbortController(); + const timeout = setTimeout(() => { + controller.abort(); + }, this.configuration.timeout); + const { requestUrl, requestOptions } = CommandAuthenticateService.call( + controller, + params, + this.configuration + ); + + try { + response = await this.getFetch()(requestUrl, requestOptions); + } catch (err) { + LoggerService.call({ requestUrl, requestOptions, err }, this.logger); + + if (isTimeout(err)) { + return this.handleFailover(params.user_id, 'timeout', err); + } else { + throw err; + } + } finally { + clearTimeout(timeout); + } + + // Wait to get body here to prevent race conditions + // on `.json()` because we attempt to read it in + // multiple places. + const body = await getBody(response); + + LoggerService.call( + { requestUrl, requestOptions, response, body }, + this.logger + ); + + if (response.status >= 500) { + return this.handleFailover(params.user_id, 'server error'); + } + + this.handleUnauthorized(response); + this.handleBadResponse(response); + + return body; } public async track(params: Payload): Promise { @@ -106,6 +149,25 @@ export class Castle { return this.configuration.overrideFetch || fetch; } + private handleFailover( + userId: string, + reason: string, + err?: Error + ): AuthenticateResult { + // Have to check it this way to make sure TS understands + // that this.failoverStrategy is of type Verdict, + // not FailoverStrategyType. + if (this.configuration.failoverStrategy === FailoverStrategy.throw) { + throw err; + } + + return FailoverResponsePrepareService.call( + userId, + reason, + this.configuration.failoverStrategy + ); + } + private handleUnauthorized(response: Response) { if (response.status === 401) { throw new Error( From 0de023fca31f9ceff02e7d6989f6e5b19b7f2dbe Mon Sep 17 00:00:00 2001 From: marysieek Date: Fri, 5 Feb 2021 19:39:05 +0100 Subject: [PATCH 06/25] Add response processor --- src/Castle.ts | 20 ++--------- .../services/core-process-response.service.ts | 36 +++++++++++++++++++ src/core/services/index.ts | 1 + 3 files changed, 39 insertions(+), 18 deletions(-) create mode 100644 src/core/services/core-process-response.service.ts diff --git a/src/Castle.ts b/src/Castle.ts index 75b30a3..3fedc8d 100644 --- a/src/Castle.ts +++ b/src/Castle.ts @@ -101,8 +101,7 @@ export class Castle { return this.handleFailover(params.user_id, 'server error'); } - this.handleUnauthorized(response); - this.handleBadResponse(response); + CoreProcessResponseService.call(response); return body; } @@ -141,8 +140,7 @@ export class Castle { } LoggerService.call({ requestUrl, requestOptions, response }, this.logger); - this.handleUnauthorized(response); - this.handleBadResponse(response); + CoreProcessResponseService.call(response); } private getFetch() { @@ -167,18 +165,4 @@ export class Castle { this.configuration.failoverStrategy ); } - - private handleUnauthorized(response: Response) { - if (response.status === 401) { - throw new Error( - 'Castle: Failed to authenticate with API, please verify the secret.' - ); - } - } - - private handleBadResponse(response: Response) { - if (response.status >= 400 && response.status < 500) { - throw new Error(`Castle: API response not ok, got ${response.status}.`); - } - } } diff --git a/src/core/services/core-process-response.service.ts b/src/core/services/core-process-response.service.ts new file mode 100644 index 0000000..6968ed2 --- /dev/null +++ b/src/core/services/core-process-response.service.ts @@ -0,0 +1,36 @@ +import { + BadRequestError, + UnauthorizedError, + ForbiddenError, + NotFoundError, + UserUnauthorizedError, + InvalidParametersError, + InternalServerError, +} from '../../errors'; + +const RESPONSE_ERRORS = { + '400': BadRequestError, + '401': UnauthorizedError, + '403': ForbiddenError, + '404': NotFoundError, + '419': UserUnauthorizedError, + '422': InvalidParametersError, +}; + +export const CoreProcessResponseService = { + call: (response: Response) => { + if (response.status >= 200 && response.status <= 299) { + return; + } + + if (response.status >= 500 && response.status <= 599) { + throw new InternalServerError( + `Castle: Responsed with ${response.status} code` + ); + } + + const err = RESPONSE_ERRORS[response.status.toString()]; + + throw new err(`Castle: Responsed with ${response.status} code`); + }, +}; diff --git a/src/core/services/index.ts b/src/core/services/index.ts index b120c16..32a1b12 100644 --- a/src/core/services/index.ts +++ b/src/core/services/index.ts @@ -1,2 +1,3 @@ export * from './core-generate-default-headers.service'; export * from './core-generate-request-body.service'; +export * from './core-process-response.service'; From 5e8f6ef678271dca7a67857a60a27139598681a9 Mon Sep 17 00:00:00 2001 From: marysieek Date: Fri, 5 Feb 2021 19:39:21 +0100 Subject: [PATCH 07/25] Fix castle test --- test/Castle.test.ts | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/test/Castle.test.ts b/test/Castle.test.ts index 449bb30..5241702 100644 --- a/test/Castle.test.ts +++ b/test/Castle.test.ts @@ -191,12 +191,9 @@ describe('Castle', () => { // Promise based expectations have to be awaited to properly fail // tests, instead of just logging unhandled rejections. - await expect( - castle.authenticate(sampleRequestData) - ).rejects.toMatchObject({ - message: - 'Castle: Failed to authenticate with API, please verify the secret.', - }); + await expect(castle.authenticate(sampleRequestData)).rejects.toThrowError( + 'Castle: Responsed with 401 code' + ); }); }); @@ -353,12 +350,9 @@ describe('Castle', () => { // Promise based expectations have to be awaited to properly fail // tests, instead of just logging unhandled rejections. - await expect( - castle.authenticate(sampleRequestData) - ).rejects.toMatchObject({ - message: - 'Castle: Failed to authenticate with API, please verify the secret.', - }); + await expect(castle.authenticate(sampleRequestData)).rejects.toThrowError( + 'Castle: Responsed with 401 code' + ); }); }); }); From b566bb80374778df811a253586100c59dcfaa1e7 Mon Sep 17 00:00:00 2001 From: marysieek Date: Fri, 5 Feb 2021 19:40:02 +0100 Subject: [PATCH 08/25] Comment out unused import --- src/api/services/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/services/index.ts b/src/api/services/index.ts index 6b98e11..0958913 100644 --- a/src/api/services/index.ts +++ b/src/api/services/index.ts @@ -1 +1 @@ -export * from './api-authenticate.service'; +// export * from './api-authenticate.service'; From e4a9453a1b82281449b8edc1d59b240801ae1dce Mon Sep 17 00:00:00 2001 From: marysieek Date: Fri, 5 Feb 2021 19:40:48 +0100 Subject: [PATCH 09/25] Comment out api module --- src/api/api.module.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/api.module.ts b/src/api/api.module.ts index e371345..98e10db 100644 --- a/src/api/api.module.ts +++ b/src/api/api.module.ts @@ -1 +1 @@ -export * from './services'; +// export * from './services'; From 726f6e601b4318179c8042db1158d56dce3a0edd Mon Sep 17 00:00:00 2001 From: marysieek Date: Fri, 5 Feb 2021 20:24:17 +0100 Subject: [PATCH 10/25] Add new module --- src/Castle.ts | 65 ++--------------- src/api/api.module.ts | 2 +- src/api/services/api-authenticate.service.ts | 76 +++++++++----------- src/api/services/index.ts | 2 +- 4 files changed, 39 insertions(+), 106 deletions(-) diff --git a/src/Castle.ts b/src/Castle.ts index 3fedc8d..77b4f46 100644 --- a/src/Castle.ts +++ b/src/Castle.ts @@ -3,6 +3,7 @@ import AbortController from 'abort-controller'; import pino from 'pino'; import { AuthenticateResult, Payload } from './models'; +import { APIAuthenticateService } from './api/api.module'; import { CommandAuthenticateService, CommandTrackService, @@ -55,55 +56,10 @@ export class Castle { } if (this.configuration.doNotTrack) { - return FailoverResponsePrepareService.call( - params.user_id, - 'do not track', - this.configuration.failoverStrategy - ); + return this.generateDoNotTrackResponse(params.user_id); } - let response: Response; - const controller = new AbortController(); - const timeout = setTimeout(() => { - controller.abort(); - }, this.configuration.timeout); - const { requestUrl, requestOptions } = CommandAuthenticateService.call( - controller, - params, - this.configuration - ); - - try { - response = await this.getFetch()(requestUrl, requestOptions); - } catch (err) { - LoggerService.call({ requestUrl, requestOptions, err }, this.logger); - - if (isTimeout(err)) { - return this.handleFailover(params.user_id, 'timeout', err); - } else { - throw err; - } - } finally { - clearTimeout(timeout); - } - - // Wait to get body here to prevent race conditions - // on `.json()` because we attempt to read it in - // multiple places. - const body = await getBody(response); - - LoggerService.call( - { requestUrl, requestOptions, response, body }, - this.logger - ); - - if (response.status >= 500) { - return this.handleFailover(params.user_id, 'server error'); - } - - CoreProcessResponseService.call(response); - - return body; + return APIAuthenticateService.call(params, this.configuration, this.logger); } public async track(params: Payload): Promise { @@ -147,21 +103,10 @@ export class Castle { return this.configuration.overrideFetch || fetch; } - private handleFailover( - userId: string, - reason: string, - err?: Error - ): AuthenticateResult { - // Have to check it this way to make sure TS understands - // that this.failoverStrategy is of type Verdict, - // not FailoverStrategyType. - if (this.configuration.failoverStrategy === FailoverStrategy.throw) { - throw err; - } - + private generateDoNotTrackResponse(userId) { return FailoverResponsePrepareService.call( userId, - reason, + 'do not track', this.configuration.failoverStrategy ); } diff --git a/src/api/api.module.ts b/src/api/api.module.ts index 98e10db..e371345 100644 --- a/src/api/api.module.ts +++ b/src/api/api.module.ts @@ -1 +1 @@ -// export * from './services'; +export * from './services'; diff --git a/src/api/services/api-authenticate.service.ts b/src/api/services/api-authenticate.service.ts index c644d39..3f06dcd 100644 --- a/src/api/services/api-authenticate.service.ts +++ b/src/api/services/api-authenticate.service.ts @@ -1,8 +1,7 @@ import { Configuration } from '../../configuraton'; import { AuthenticateResult, Payload } from '../../models'; -import { - CommandAuthenticateService, -} from '../../command/command.module'; +import { CommandAuthenticateService } from '../../command/command.module'; +import { CoreProcessResponseService } from '../../core/core.module'; import { FailoverResponsePrepareService, FailoverStrategy, @@ -31,27 +30,34 @@ const getBody = async (response: any) => { const isTimeout = (e: Error) => e.name === 'AbortError'; - // private handleFailover( - // userId: string, - // reason: string, - // err?: Error - // ): AuthenticateResult { - // // Have to check it this way to make sure TS understands - // // that this.failoverStrategy is of type Verdict, - // // not FailoverStrategyType. - // if (this.configuration.failoverStrategy === FailoverStrategy.throw) { - // throw err; - // } +const handleFailover = ( + userId: string, + reason: string, + configuration: Configuration, + err?: Error +): AuthenticateResult => { + // Have to check it this way to make sure TS understands + // that this.failoverStrategy is of type Verdict, + // not FailoverStrategyType. + if (configuration.failoverStrategy === FailoverStrategy.throw) { + throw err; + } - // return FailoverResponsePrepareService.call( - // userId, - // reason, - // this.configuration.failoverStrategy - // ); - // } + return FailoverResponsePrepareService.call( + userId, + reason, + configuration.failoverStrategy + ); +}; export const APIAuthenticateService = { - call: async (params: Payload, configuration: Configuration, logger: pino.Logger): Promise => { + call: async ( + params: Payload, + configuration: Configuration, + logger: pino.Logger + ): Promise => { + const fetcher = configuration.overrideFetch || fetch; + let response: Response; const controller = new AbortController(); const timeout = setTimeout(() => { @@ -63,19 +69,13 @@ export const APIAuthenticateService = { configuration ); - const fetcher = configuration.overrideFetch || fetch - try { - response = await fetcher()(requestUrl, requestOptions); + response = await fetcher(requestUrl, requestOptions); } catch (err) { LoggerService.call({ requestUrl, requestOptions, err }, logger); if (isTimeout(err)) { - // return FailoverResponsePrepareService.call( - // params.user_id, - // 'server error', - // configuration.failoverStrategy - // ); + return handleFailover(params.user_id, 'timeout', configuration, err); } else { throw err; } @@ -88,25 +88,13 @@ export const APIAuthenticateService = { // multiple places. const body = await getBody(response); - LoggerService.call( - { requestUrl, requestOptions, response, body }, - logger - ); + LoggerService.call({ requestUrl, requestOptions, response, body }, logger); if (response.status >= 500) { - // if (configuration.failoverStrategy === FailoverStrategy.throw) { - // throw 'server error'; - // } - - // return FailoverResponsePrepareService.call( - // params.user_id, - // 'server error', - // configuration.failoverStrategy - // ); + return handleFailover(params.user_id, 'server error', configuration); } - this.handleUnauthorized(response); - this.handleBadResponse(response); + CoreProcessResponseService.call(response); return body; }, diff --git a/src/api/services/index.ts b/src/api/services/index.ts index 0958913..6b98e11 100644 --- a/src/api/services/index.ts +++ b/src/api/services/index.ts @@ -1 +1 @@ -// export * from './api-authenticate.service'; +export * from './api-authenticate.service'; From 155fea3f6d9853a5bbf9092f5908f4cf0baf094f Mon Sep 17 00:00:00 2001 From: marysieek Date: Fri, 5 Feb 2021 20:31:39 +0100 Subject: [PATCH 11/25] Clean up response processing --- src/Castle.ts | 32 ++++------------- src/api/services/api-authenticate.service.ts | 34 ++++--------------- .../services/core-process-response.service.ts | 33 ++++++++++++++++-- 3 files changed, 45 insertions(+), 54 deletions(-) diff --git a/src/Castle.ts b/src/Castle.ts index 77b4f46..5fe3cf1 100644 --- a/src/Castle.ts +++ b/src/Castle.ts @@ -9,33 +9,11 @@ import { CommandTrackService, } from './command/command.module'; import { CoreProcessResponseService } from './core/core.module'; -import { - FailoverResponsePrepareService, - FailoverStrategy, -} from './failover/failover.module'; +import { FailoverResponsePrepareService } from './failover/failover.module'; import { LoggerService } from './logger/logger.module'; import { Configuration } from './configuraton'; -// The body on the request is a stream and can only be -// read once, by default. This is a workaround so that the -// logging functions can read the body independently -// of the handlers. -const getBody = async (response: any) => { - if (response.cachedBody) { - return response.cachedBody; - } - - try { - response.cachedBody = await response.json(); - } catch (e) { - response.cachedBody = {}; - } - - return response.cachedBody; -}; - const isTimeout = (e: Error) => e.name === 'AbortError'; - export class Castle { private logger: pino.Logger; private configuration: Configuration; @@ -95,8 +73,12 @@ export class Castle { clearTimeout(timeout); } - LoggerService.call({ requestUrl, requestOptions, response }, this.logger); - CoreProcessResponseService.call(response); + CoreProcessResponseService.call( + requestUrl, + requestOptions, + response, + this.logger + ); } private getFetch() { diff --git a/src/api/services/api-authenticate.service.ts b/src/api/services/api-authenticate.service.ts index 3f06dcd..cc45aa5 100644 --- a/src/api/services/api-authenticate.service.ts +++ b/src/api/services/api-authenticate.service.ts @@ -10,24 +10,6 @@ import { LoggerService } from '../../logger/logger.module'; import fetch from 'node-fetch'; import pino from 'pino'; -// The body on the request is a stream and can only be -// read once, by default. This is a workaround so that the -// logging functions can read the body independently -// of the handlers. -const getBody = async (response: any) => { - if (response.cachedBody) { - return response.cachedBody; - } - - try { - response.cachedBody = await response.json(); - } catch (e) { - response.cachedBody = {}; - } - - return response.cachedBody; -}; - const isTimeout = (e: Error) => e.name === 'AbortError'; const handleFailover = ( @@ -83,19 +65,17 @@ export const APIAuthenticateService = { clearTimeout(timeout); } - // Wait to get body here to prevent race conditions - // on `.json()` because we attempt to read it in - // multiple places. - const body = await getBody(response); - - LoggerService.call({ requestUrl, requestOptions, response, body }, logger); - if (response.status >= 500) { return handleFailover(params.user_id, 'server error', configuration); } - CoreProcessResponseService.call(response); + const processedResponse = await CoreProcessResponseService.call( + requestUrl, + requestOptions, + response, + logger + ); - return body; + return processedResponse; }, }; diff --git a/src/core/services/core-process-response.service.ts b/src/core/services/core-process-response.service.ts index 6968ed2..86eaabe 100644 --- a/src/core/services/core-process-response.service.ts +++ b/src/core/services/core-process-response.service.ts @@ -7,6 +7,8 @@ import { InvalidParametersError, InternalServerError, } from '../../errors'; +import { LoggerService } from '../../logger/logger.module'; +import pino from 'pino'; const RESPONSE_ERRORS = { '400': BadRequestError, @@ -17,10 +19,37 @@ const RESPONSE_ERRORS = { '422': InvalidParametersError, }; +// The body on the request is a stream and can only be +// read once, by default. This is a workaround so that the +// logging functions can read the body independently +// of the handlers. +const getBody = async (response: any) => { + if (response.cachedBody) { + return response.cachedBody; + } + + try { + response.cachedBody = await response.json(); + } catch (e) { + response.cachedBody = {}; + } + + return response.cachedBody; +}; + export const CoreProcessResponseService = { - call: (response: Response) => { + call: async ( + requestUrl, + requestOptions, + response: Response, + logger: pino.Logger + ) => { + const body = await getBody(response); + + LoggerService.call({ requestUrl, requestOptions, response, body }, logger); + if (response.status >= 200 && response.status <= 299) { - return; + return body; } if (response.status >= 500 && response.status <= 599) { From 0d883dbc23bbf725aa94ed9cfe60929025afe087 Mon Sep 17 00:00:00 2001 From: marysieek Date: Fri, 5 Feb 2021 20:39:51 +0100 Subject: [PATCH 12/25] Add missing abort controller --- src/api/services/api-authenticate.service.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/api/services/api-authenticate.service.ts b/src/api/services/api-authenticate.service.ts index cc45aa5..50af9d7 100644 --- a/src/api/services/api-authenticate.service.ts +++ b/src/api/services/api-authenticate.service.ts @@ -7,6 +7,7 @@ import { FailoverStrategy, } from '../../failover/failover.module'; import { LoggerService } from '../../logger/logger.module'; +import AbortController from 'abort-controller'; import fetch from 'node-fetch'; import pino from 'pino'; From 61d7771775cf3b973419f72182797f9380ee53b5 Mon Sep 17 00:00:00 2001 From: marysieek Date: Fri, 5 Feb 2021 20:43:46 +0100 Subject: [PATCH 13/25] default headers --- src/client-id/services/client-id-extract.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client-id/services/client-id-extract.service.ts b/src/client-id/services/client-id-extract.service.ts index a050981..2ccad07 100644 --- a/src/client-id/services/client-id-extract.service.ts +++ b/src/client-id/services/client-id-extract.service.ts @@ -2,7 +2,7 @@ import { IncomingHttpHeaders } from 'http'; import { HeadersGetCookieService } from '../../headers/headers.module'; export const ClientIdExtractService = { - call: (headers: IncomingHttpHeaders, cookies = '') => { + call: (headers: IncomingHttpHeaders = {}, cookies = '') => { return ( headers['x-castle-client-id'] || HeadersGetCookieService.call(cookies, '__cid') || From 56b74b91cfa7b860664d4c12f126a7dd8854f6bc Mon Sep 17 00:00:00 2001 From: marysieek Date: Fri, 5 Feb 2021 21:00:46 +0100 Subject: [PATCH 14/25] Revert context --- README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/README.md b/README.md index b6e09b7..cb954ef 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,11 @@ castle.track({ email: user.email, registered_at: user.registered_at, }, + context: { + ip: request.ip, + client_id: request.cookies['__cid'], + headers: request.headers, + }, }); ``` @@ -86,6 +91,11 @@ try { email: user.email, registered_at: user.registered_at, }, + context: { + ip: request.ip, + client_id: request.cookies['__cid'], + headers: request.headers, + }, }); } catch (e) { console.error(e); From 4177ec4c68bcf885d31ab68ec069374b762e9af0 Mon Sep 17 00:00:00 2001 From: marysieek Date: Fri, 5 Feb 2021 21:11:32 +0100 Subject: [PATCH 15/25] Export track action --- src/Castle.ts | 46 ++---------------------- src/api/services/api-track.service.ts | 52 +++++++++++++++++++++++++++ src/api/services/index.ts | 1 + src/models/authenticate-result.ts | 1 + 4 files changed, 56 insertions(+), 44 deletions(-) create mode 100644 src/api/services/api-track.service.ts diff --git a/src/Castle.ts b/src/Castle.ts index 5fe3cf1..6c5ceeb 100644 --- a/src/Castle.ts +++ b/src/Castle.ts @@ -1,19 +1,10 @@ -import fetch from 'node-fetch'; -import AbortController from 'abort-controller'; import pino from 'pino'; import { AuthenticateResult, Payload } from './models'; -import { APIAuthenticateService } from './api/api.module'; -import { - CommandAuthenticateService, - CommandTrackService, -} from './command/command.module'; -import { CoreProcessResponseService } from './core/core.module'; +import { APIAuthenticateService, APITrackService } from './api/api.module'; import { FailoverResponsePrepareService } from './failover/failover.module'; -import { LoggerService } from './logger/logger.module'; import { Configuration } from './configuraton'; -const isTimeout = (e: Error) => e.name === 'AbortError'; export class Castle { private logger: pino.Logger; private configuration: Configuration; @@ -49,40 +40,7 @@ export class Castle { return; } - let response: Response; - const controller = new AbortController(); - const timeout = setTimeout(() => { - controller.abort(); - }, this.configuration.timeout); - const { requestUrl, requestOptions } = CommandTrackService.call( - controller, - params, - this.configuration - ); - - try { - response = await this.getFetch()(requestUrl, requestOptions); - } catch (err) { - if (isTimeout(err)) { - return LoggerService.call( - { requestUrl, requestOptions, err }, - this.logger - ); - } - } finally { - clearTimeout(timeout); - } - - CoreProcessResponseService.call( - requestUrl, - requestOptions, - response, - this.logger - ); - } - - private getFetch() { - return this.configuration.overrideFetch || fetch; + return APITrackService.call(params, this.configuration, this.logger); } private generateDoNotTrackResponse(userId) { diff --git a/src/api/services/api-track.service.ts b/src/api/services/api-track.service.ts new file mode 100644 index 0000000..8c71fd8 --- /dev/null +++ b/src/api/services/api-track.service.ts @@ -0,0 +1,52 @@ +import { Configuration } from '../../configuraton'; +import { AuthenticateResult, Payload } from '../../models'; +import { CommandTrackService } from '../../command/command.module'; +import { CoreProcessResponseService } from '../../core/core.module'; +import { + FailoverResponsePrepareService, + FailoverStrategy, +} from '../../failover/failover.module'; +import { LoggerService } from '../../logger/logger.module'; +import AbortController from 'abort-controller'; +import fetch from 'node-fetch'; +import pino from 'pino'; + +const isTimeout = (e: Error) => e.name === 'AbortError'; + +export const APITrackService = { + call: async ( + params: Payload, + configuration: Configuration, + logger: pino.Logger + ): Promise => { + const fetcher = configuration.overrideFetch || fetch; + + let response: Response; + const controller = new AbortController(); + const timeout = setTimeout(() => { + controller.abort(); + }, configuration.timeout); + const { requestUrl, requestOptions } = CommandTrackService.call( + controller, + params, + configuration + ); + + try { + response = await fetcher(requestUrl, requestOptions); + } catch (err) { + if (isTimeout(err)) { + return LoggerService.call({ requestUrl, requestOptions, err }, logger); + } + } finally { + clearTimeout(timeout); + } + + CoreProcessResponseService.call( + requestUrl, + requestOptions, + response, + logger + ); + }, +}; diff --git a/src/api/services/index.ts b/src/api/services/index.ts index 6b98e11..778bda8 100644 --- a/src/api/services/index.ts +++ b/src/api/services/index.ts @@ -1 +1,2 @@ export * from './api-authenticate.service'; +export * from './api-track.service'; diff --git a/src/models/authenticate-result.ts b/src/models/authenticate-result.ts index 3331c3d..24085f6 100644 --- a/src/models/authenticate-result.ts +++ b/src/models/authenticate-result.ts @@ -9,4 +9,5 @@ export type AuthenticateResult = { failover?: boolean; failover_reason?: string; risk_policy?: RiskPolicy; + internal?: string; }; From 015174358441a694bf10035389973f6524260861 Mon Sep 17 00:00:00 2001 From: marysieek Date: Fri, 5 Feb 2021 21:35:41 +0100 Subject: [PATCH 16/25] Add drafted test --- .../services/core-process-response.service.ts | 7 +---- .../core-process-response.service.test.ts | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 6 deletions(-) create mode 100644 test/core/services/core-process-response.service.test.ts diff --git a/src/core/services/core-process-response.service.ts b/src/core/services/core-process-response.service.ts index 86eaabe..1bda64f 100644 --- a/src/core/services/core-process-response.service.ts +++ b/src/core/services/core-process-response.service.ts @@ -38,12 +38,7 @@ const getBody = async (response: any) => { }; export const CoreProcessResponseService = { - call: async ( - requestUrl, - requestOptions, - response: Response, - logger: pino.Logger - ) => { + call: async (requestUrl, requestOptions, response, logger: pino.Logger) => { const body = await getBody(response); LoggerService.call({ requestUrl, requestOptions, response, body }, logger); diff --git a/test/core/services/core-process-response.service.test.ts b/test/core/services/core-process-response.service.test.ts new file mode 100644 index 0000000..bc69cb9 --- /dev/null +++ b/test/core/services/core-process-response.service.test.ts @@ -0,0 +1,30 @@ +import { CoreProcessResponseService } from '../../../src/core/core.module'; +import { version } from '../../../package.json'; +import { Configuration } from '../../../src/configuraton'; + +import pino from 'pino'; +import { Response } from 'node-fetch'; + +describe('CoreProcessResponseService', () => { + describe('call', () => { + describe('when success', () => { + const response = new Response(JSON.stringify({ user: 1 }), { + headers: { + 'Content-Type': 'application/json', + }, + status: 200, + }); + + it('generates request body', async () => { + expect( + await CoreProcessResponseService.call( + 'authenticate', + {}, + response, + pino({ enabled: false }) + ) + ).toEqual({ user: 1 }); + }); + }); + }); +}); From 06c4981cf332d73f575b300b5ba0ad37dbe49300 Mon Sep 17 00:00:00 2001 From: marysieek Date: Fri, 5 Feb 2021 21:43:37 +0100 Subject: [PATCH 17/25] Add response processing test --- .../core-process-response.service.test.ts | 130 +++++++++++++++--- 1 file changed, 112 insertions(+), 18 deletions(-) diff --git a/test/core/services/core-process-response.service.test.ts b/test/core/services/core-process-response.service.test.ts index bc69cb9..375f067 100644 --- a/test/core/services/core-process-response.service.test.ts +++ b/test/core/services/core-process-response.service.test.ts @@ -1,29 +1,123 @@ import { CoreProcessResponseService } from '../../../src/core/core.module'; -import { version } from '../../../package.json'; -import { Configuration } from '../../../src/configuraton'; - import pino from 'pino'; import { Response } from 'node-fetch'; describe('CoreProcessResponseService', () => { describe('call', () => { - describe('when success', () => { - const response = new Response(JSON.stringify({ user: 1 }), { - headers: { - 'Content-Type': 'application/json', - }, - status: 200, + describe('authenticate', () => { + describe('when success', () => { + const result = { user: 1 }; + const response = new Response(JSON.stringify({ user: 1 }), { + headers: { + 'Content-Type': 'application/json', + }, + status: 200, + }); + + it('generates request body', async () => { + expect( + await CoreProcessResponseService.call( + 'authenticate', + {}, + response, + pino({ enabled: false }) + ) + ).toEqual(result); + }); + }); + + describe('when allow without additional props', () => { + const result = { action: 'allow', user_id: '12345' }; + const response = new Response(JSON.stringify(result), { + headers: { + 'Content-Type': 'application/json', + }, + status: 200, + }); + + it('generates request body', async () => { + expect( + await CoreProcessResponseService.call( + 'authenticate', + {}, + response, + pino({ enabled: false }) + ) + ).toEqual(result); + }); + }); + + describe('when allow with additional props', () => { + const result = { action: 'allow', user_id: '12345', internal: {} }; + const response = new Response(JSON.stringify(result), { + headers: { + 'Content-Type': 'application/json', + }, + status: 200, + }); + + it('generates request body', async () => { + expect( + await CoreProcessResponseService.call( + 'authenticate', + {}, + response, + pino({ enabled: false }) + ) + ).toEqual(result); + }); + }); + + describe('when deny without risk policy', () => { + const result = { action: 'deny', user_id: '1', device_token: 'abc' }; + const response = new Response(JSON.stringify(result), { + headers: { + 'Content-Type': 'application/json', + }, + status: 200, + }); + + it('generates request body', async () => { + expect( + await CoreProcessResponseService.call( + 'authenticate', + {}, + response, + pino({ enabled: false }) + ) + ).toEqual(result); + }); }); - it('generates request body', async () => { - expect( - await CoreProcessResponseService.call( - 'authenticate', - {}, - response, - pino({ enabled: false }) - ) - ).toEqual({ user: 1 }); + describe('when deny with risk policy', () => { + const result = { + action: 'deny', + user_id: '1', + device_token: 'abc', + risk_policy: { + id: '123', + revision_id: 'abc', + name: 'def', + type: 'bot', + }, + }; + const response = new Response(JSON.stringify(result), { + headers: { + 'Content-Type': 'application/json', + }, + status: 200, + }); + + it('generates request body', async () => { + expect( + await CoreProcessResponseService.call( + 'authenticate', + {}, + response, + pino({ enabled: false }) + ) + ).toEqual(result); + }); }); }); }); From 58971d2ffa85b4ba733886335a9562755bc36613 Mon Sep 17 00:00:00 2001 From: marysieek Date: Sat, 6 Feb 2021 19:13:44 +0100 Subject: [PATCH 18/25] Add new test cases --- .../services/core-process-response.service.ts | 7 ++- .../core-process-response.service.test.ts | 63 +++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/src/core/services/core-process-response.service.ts b/src/core/services/core-process-response.service.ts index 1bda64f..6446c46 100644 --- a/src/core/services/core-process-response.service.ts +++ b/src/core/services/core-process-response.service.ts @@ -29,7 +29,12 @@ const getBody = async (response: any) => { } try { - response.cachedBody = await response.json(); + const parsedResponse = await response.json(); + if (parsedResponse) { + response.cachedBody = parsedResponse; + } else { + response.cachedBody = {}; + } } catch (e) { response.cachedBody = {}; } diff --git a/test/core/services/core-process-response.service.test.ts b/test/core/services/core-process-response.service.test.ts index 375f067..885fae0 100644 --- a/test/core/services/core-process-response.service.test.ts +++ b/test/core/services/core-process-response.service.test.ts @@ -119,6 +119,69 @@ describe('CoreProcessResponseService', () => { ).toEqual(result); }); }); + + describe('when response empty', () => { + const result = { }; + const response = new Response(JSON.stringify(''), { + headers: { + 'Content-Type': 'application/json', + }, + status: 200, + }); + + it('generates empty request body', async () => { + expect( + await CoreProcessResponseService.call( + 'authenticate', + {}, + response, + pino({ enabled: false }) + ) + ).toEqual(result); + }); + }); + + describe('when response undefined', () => { + const result = { }; + const response = new Response(undefined, { + headers: { + 'Content-Type': 'application/json', + }, + status: 200, + }); + + it('generates empty request body', async () => { + expect( + await CoreProcessResponseService.call( + 'authenticate', + {}, + response, + pino({ enabled: false }) + ) + ).toEqual(result); + }); + }); + + describe('when JSON is malformed', () => { + const result = { }; + const response = new Response('{a', { + headers: { + 'Content-Type': 'application/json', + }, + status: 200, + }); + + it('generates request body', async () => { + expect( + await CoreProcessResponseService.call( + 'authenticate', + {}, + response, + pino({ enabled: false }) + ) + ).toEqual(result); + }); + }); }); }); }); From 667c1d0358a4aa44d8f9647ea831df52da05ffd9 Mon Sep 17 00:00:00 2001 From: marysieek Date: Sat, 6 Feb 2021 19:34:28 +0100 Subject: [PATCH 19/25] Add more test cases --- .../services/core-process-response.service.ts | 9 ++--- .../core-process-response.service.test.ts | 33 ++++--------------- 2 files changed, 11 insertions(+), 31 deletions(-) diff --git a/src/core/services/core-process-response.service.ts b/src/core/services/core-process-response.service.ts index 6446c46..01cfd26 100644 --- a/src/core/services/core-process-response.service.ts +++ b/src/core/services/core-process-response.service.ts @@ -6,6 +6,7 @@ import { UserUnauthorizedError, InvalidParametersError, InternalServerError, + APIError, } from '../../errors'; import { LoggerService } from '../../logger/logger.module'; import pino from 'pino'; @@ -29,14 +30,14 @@ const getBody = async (response: any) => { } try { - const parsedResponse = await response.json(); - if (parsedResponse) { - response.cachedBody = parsedResponse; + const preparsedResponse = await response.text(); + if (preparsedResponse) { + response.cachedBody = await JSON.parse(preparsedResponse); } else { response.cachedBody = {}; } } catch (e) { - response.cachedBody = {}; + throw new APIError('Castle: Malformed JSON response'); } return response.cachedBody; diff --git a/test/core/services/core-process-response.service.test.ts b/test/core/services/core-process-response.service.test.ts index 885fae0..bbe3537 100644 --- a/test/core/services/core-process-response.service.test.ts +++ b/test/core/services/core-process-response.service.test.ts @@ -1,4 +1,5 @@ import { CoreProcessResponseService } from '../../../src/core/core.module'; +import { APIError } from '../../../src/errors'; import pino from 'pino'; import { Response } from 'node-fetch'; @@ -121,29 +122,8 @@ describe('CoreProcessResponseService', () => { }); describe('when response empty', () => { - const result = { }; - const response = new Response(JSON.stringify(''), { - headers: { - 'Content-Type': 'application/json', - }, - status: 200, - }); - - it('generates empty request body', async () => { - expect( - await CoreProcessResponseService.call( - 'authenticate', - {}, - response, - pino({ enabled: false }) - ) - ).toEqual(result); - }); - }); - - describe('when response undefined', () => { - const result = { }; - const response = new Response(undefined, { + const result = {}; + const response = new Response('', { headers: { 'Content-Type': 'application/json', }, @@ -163,7 +143,6 @@ describe('CoreProcessResponseService', () => { }); describe('when JSON is malformed', () => { - const result = { }; const response = new Response('{a', { headers: { 'Content-Type': 'application/json', @@ -172,14 +151,14 @@ describe('CoreProcessResponseService', () => { }); it('generates request body', async () => { - expect( - await CoreProcessResponseService.call( + await expect( + CoreProcessResponseService.call( 'authenticate', {}, response, pino({ enabled: false }) ) - ).toEqual(result); + ).rejects.toThrowError('Castle: Malformed JSON response'); }); }); }); From bacd499939b78e65134d08d219dae3dd835b63aa Mon Sep 17 00:00:00 2001 From: marysieek Date: Sat, 6 Feb 2021 19:45:27 +0100 Subject: [PATCH 20/25] Add missing test context --- .../services/core-process-response.service.ts | 4 +-- .../core-process-response.service.test.ts | 29 ++++++++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/core/services/core-process-response.service.ts b/src/core/services/core-process-response.service.ts index 01cfd26..a986229 100644 --- a/src/core/services/core-process-response.service.ts +++ b/src/core/services/core-process-response.service.ts @@ -55,12 +55,12 @@ export const CoreProcessResponseService = { if (response.status >= 500 && response.status <= 599) { throw new InternalServerError( - `Castle: Responsed with ${response.status} code` + `Castle: Responded with ${response.status} code` ); } const err = RESPONSE_ERRORS[response.status.toString()]; - throw new err(`Castle: Responsed with ${response.status} code`); + throw new err(`Castle: Responded with ${response.status} code`); }, }; diff --git a/test/core/services/core-process-response.service.test.ts b/test/core/services/core-process-response.service.test.ts index bbe3537..6a1ef1a 100644 --- a/test/core/services/core-process-response.service.test.ts +++ b/test/core/services/core-process-response.service.test.ts @@ -1,5 +1,4 @@ import { CoreProcessResponseService } from '../../../src/core/core.module'; -import { APIError } from '../../../src/errors'; import pino from 'pino'; import { Response } from 'node-fetch'; @@ -162,5 +161,33 @@ describe('CoreProcessResponseService', () => { }); }); }); + + describe('erroneous response statuses', () => { + const erroneousStatuses = [400, 401, 403, 404, 419, 422]; + + erroneousStatuses.forEach((errorStatus) => { + describe(`when ${errorStatus}`, () => { + const response = new Response(JSON.stringify({}), { + headers: { + 'Content-Type': 'application/json', + }, + status: errorStatus, + }); + + it('throws BadRequestError', async () => { + await expect( + CoreProcessResponseService.call( + 'authenticate', + {}, + response, + pino({ enabled: false }) + ) + ).rejects.toThrowError( + `Castle: Responded with ${errorStatus} code` + ); + }); + }); + }); + }); }); }); From 2df9429f7271b143368ad82b21eb692e3845ff44 Mon Sep 17 00:00:00 2001 From: marysieek Date: Sat, 6 Feb 2021 19:46:10 +0100 Subject: [PATCH 21/25] Fix typo --- test/Castle.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Castle.test.ts b/test/Castle.test.ts index 5241702..8d9bc10 100644 --- a/test/Castle.test.ts +++ b/test/Castle.test.ts @@ -192,7 +192,7 @@ describe('Castle', () => { // Promise based expectations have to be awaited to properly fail // tests, instead of just logging unhandled rejections. await expect(castle.authenticate(sampleRequestData)).rejects.toThrowError( - 'Castle: Responsed with 401 code' + 'Castle: Responded with 401 code' ); }); }); @@ -351,7 +351,7 @@ describe('Castle', () => { // Promise based expectations have to be awaited to properly fail // tests, instead of just logging unhandled rejections. await expect(castle.authenticate(sampleRequestData)).rejects.toThrowError( - 'Castle: Responsed with 401 code' + 'Castle: Responded with 401 code' ); }); }); From df7cadc6176d82ba2a71663b5a87b36aa7902773 Mon Sep 17 00:00:00 2001 From: marysieek Date: Sat, 6 Feb 2021 20:00:09 +0100 Subject: [PATCH 22/25] Add api test --- .../services/api-authenticate.service.test.ts | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 test/api/services/api-authenticate.service.test.ts diff --git a/test/api/services/api-authenticate.service.test.ts b/test/api/services/api-authenticate.service.test.ts new file mode 100644 index 0000000..a9a168d --- /dev/null +++ b/test/api/services/api-authenticate.service.test.ts @@ -0,0 +1,70 @@ +import { APIAuthenticateService } from '../../../src/api/api.module'; +import { Configuration } from '../../../src/configuraton'; +import { EVENTS } from '../../../src/events'; +import MockDate from 'mockdate'; +import fetchMock from 'fetch-mock'; +import pino from 'pino'; + +describe('APIAuthenticateService', () => { + beforeEach(() => { + MockDate.set(new Date('2021-01-25T00:00:00.000Z')); + }); + + afterEach(() => { + MockDate.reset(); + }); + + const sampleRequestData = { + event: EVENTS.LOGIN_SUCCEEDED, + created_at: 'now', + user_id: 'userid', + user_traits: { + email: 'myemail', + updated_at: 'today', + }, + context: { + ip: '8.8.8.8', + headers: {}, + }, + }; + + describe('call', () => { + it('handles allow response', async () => { + const fetch = fetchMock.sandbox().mock('*', { + action: 'allow', + device_token: 'device_token', + user_id: 'user_id', + risk_policy: { + id: 'q-rbeMzBTdW2Fd09sbz55A', + revision_id: 'pke4zqO2TnqVr-NHJOAHEg', + name: 'Block Users from X', + type: 'bot', + }, + }); + + const config = new Configuration({ + apiSecret: 'test', + overrideFetch: fetch, + }); + + const response = await APIAuthenticateService.call( + sampleRequestData, + config, + pino({ enabled: false }) + ); + expect(response).toHaveProperty('action', 'allow'); + expect(response).toHaveProperty('device_token', 'device_token'); + expect(response).toHaveProperty('user_id', 'user_id'); + expect(response.risk_policy).toHaveProperty( + 'id', + 'q-rbeMzBTdW2Fd09sbz55A' + ); + expect(response.risk_policy).toHaveProperty( + 'revision_id', + 'pke4zqO2TnqVr-NHJOAHEg' + ); + expect(response.risk_policy).toHaveProperty('type', 'bot'); + expect(response.risk_policy).toHaveProperty('name', 'Block Users from X'); + }); + }); +}); From 280740d5753e26245881f97f52dab854d3598a44 Mon Sep 17 00:00:00 2001 From: marysieek Date: Sat, 6 Feb 2021 20:02:37 +0100 Subject: [PATCH 23/25] Update authenticate service test --- .../services/api-authenticate.service.test.ts | 46 ++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/test/api/services/api-authenticate.service.test.ts b/test/api/services/api-authenticate.service.test.ts index a9a168d..b35a2d0 100644 --- a/test/api/services/api-authenticate.service.test.ts +++ b/test/api/services/api-authenticate.service.test.ts @@ -34,6 +34,50 @@ describe('APIAuthenticateService', () => { action: 'allow', device_token: 'device_token', user_id: 'user_id', + }); + + const config = new Configuration({ + apiSecret: 'test', + overrideFetch: fetch, + }); + + const response = await APIAuthenticateService.call( + sampleRequestData, + config, + pino({ enabled: false }) + ); + expect(response).toHaveProperty('action', 'allow'); + expect(response).toHaveProperty('device_token', 'device_token'); + expect(response).toHaveProperty('user_id', 'user_id'); + }); + + it('handles deny response without risk policy', async () => { + const fetch = fetchMock.sandbox().mock('*', { + action: 'deny', + device_token: 'device_token', + user_id: 'user_id', + }); + + const config = new Configuration({ + apiSecret: 'test', + overrideFetch: fetch, + }); + + const response = await APIAuthenticateService.call( + sampleRequestData, + config, + pino({ enabled: false }) + ); + expect(response).toHaveProperty('action', 'deny'); + expect(response).toHaveProperty('device_token', 'device_token'); + expect(response).toHaveProperty('user_id', 'user_id'); + }); + + it('handles deny response with risk policy', async () => { + const fetch = fetchMock.sandbox().mock('*', { + action: 'deny', + device_token: 'device_token', + user_id: 'user_id', risk_policy: { id: 'q-rbeMzBTdW2Fd09sbz55A', revision_id: 'pke4zqO2TnqVr-NHJOAHEg', @@ -52,7 +96,7 @@ describe('APIAuthenticateService', () => { config, pino({ enabled: false }) ); - expect(response).toHaveProperty('action', 'allow'); + expect(response).toHaveProperty('action', 'deny'); expect(response).toHaveProperty('device_token', 'device_token'); expect(response).toHaveProperty('user_id', 'user_id'); expect(response.risk_policy).toHaveProperty( From 548f2f41b672264a39d677ec4f3c83d651bf92eb Mon Sep 17 00:00:00 2001 From: marysieek Date: Sat, 6 Feb 2021 20:09:47 +0100 Subject: [PATCH 24/25] Add track test --- test/api/services/api-track.service.test.ts | 48 +++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 test/api/services/api-track.service.test.ts diff --git a/test/api/services/api-track.service.test.ts b/test/api/services/api-track.service.test.ts new file mode 100644 index 0000000..d99eb7e --- /dev/null +++ b/test/api/services/api-track.service.test.ts @@ -0,0 +1,48 @@ +import { APITrackService } from '../../../src/api/api.module'; +import { Configuration } from '../../../src/configuraton'; +import { EVENTS } from '../../../src/events'; +import MockDate from 'mockdate'; +import fetchMock from 'fetch-mock'; +import pino from 'pino'; + +describe('APITrackService', () => { + beforeEach(() => { + MockDate.set(new Date('2021-01-25T00:00:00.000Z')); + }); + + afterEach(() => { + MockDate.reset(); + }); + + const sampleRequestData = { + event: EVENTS.LOGIN_SUCCEEDED, + created_at: 'now', + user_id: 'userid', + user_traits: { + email: 'myemail', + updated_at: 'today', + }, + context: { + ip: '8.8.8.8', + headers: {}, + }, + }; + + describe('call', () => { + it('handles track response', async () => { + const fetch = fetchMock.sandbox().post('*', 204); + + const config = new Configuration({ + apiSecret: 'test', + overrideFetch: fetch, + }); + + const response = await APITrackService.call( + sampleRequestData, + config, + pino({ enabled: false }) + ); + expect(response).toBeUndefined(); + }); + }); +}); From e300d6ca0b1b791301d52712260b5f5a3fdd3a14 Mon Sep 17 00:00:00 2001 From: marysieek Date: Mon, 8 Feb 2021 12:05:21 +0100 Subject: [PATCH 25/25] Handle failover in case of internal server error --- src/api/services/api-authenticate.service.ts | 24 ++++++++++++-------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/api/services/api-authenticate.service.ts b/src/api/services/api-authenticate.service.ts index 50af9d7..83e510e 100644 --- a/src/api/services/api-authenticate.service.ts +++ b/src/api/services/api-authenticate.service.ts @@ -1,4 +1,5 @@ import { Configuration } from '../../configuraton'; +import { InternalServerError } from '../../errors'; import { AuthenticateResult, Payload } from '../../models'; import { CommandAuthenticateService } from '../../command/command.module'; import { CoreProcessResponseService } from '../../core/core.module'; @@ -66,17 +67,22 @@ export const APIAuthenticateService = { clearTimeout(timeout); } - if (response.status >= 500) { - return handleFailover(params.user_id, 'server error', configuration); + let processedResponse; + try { + processedResponse = await CoreProcessResponseService.call( + requestUrl, + requestOptions, + response, + logger + ); + } catch (e) { + if (e instanceof InternalServerError) { + return handleFailover(params.user_id, 'server error', configuration); + } else { + throw e; + } } - const processedResponse = await CoreProcessResponseService.call( - requestUrl, - requestOptions, - response, - logger - ); - return processedResponse; }, };