From dd2b72359a23943204f6e23f7dc20dc620ba94a8 Mon Sep 17 00:00:00 2001 From: Robert Bo Davis Date: Tue, 28 Nov 2023 14:29:22 -0500 Subject: [PATCH 1/2] refactor: eliminate poor abstraction in hrm-authentictopackage --- .../lambdas/files-urls/authenticate.ts | 39 ++++++++----------- .../lambdas/files-urls/getSignedS3Url.ts | 22 +++-------- .../lambdas/files-urls/parseParameters.ts | 9 ++++- .../packages/hrm-authentication/callHrmApi.ts | 8 ++-- lambdas/packages/hrm-authentication/index.ts | 38 +----------------- 5 files changed, 35 insertions(+), 81 deletions(-) rename lambdas/packages/hrm-authentication/filesUrlsAuthenticator.ts => hrm-domain/lambdas/files-urls/authenticate.ts (84%) diff --git a/lambdas/packages/hrm-authentication/filesUrlsAuthenticator.ts b/hrm-domain/lambdas/files-urls/authenticate.ts similarity index 84% rename from lambdas/packages/hrm-authentication/filesUrlsAuthenticator.ts rename to hrm-domain/lambdas/files-urls/authenticate.ts index d517053fa..9641e8d27 100644 --- a/lambdas/packages/hrm-authentication/filesUrlsAuthenticator.ts +++ b/hrm-domain/lambdas/files-urls/authenticate.ts @@ -16,24 +16,21 @@ import { newOk, isErr } from '@tech-matters/types'; import { - HrmAuthenticateParameters, - HrmAuthenticateResult, + callHrmApi, HRMAuthenticationObjectTypes, -} from './index'; -import callHrmApi from './callHrmApi'; - -export const mockBuckets = ['mock-bucket']; + HrmAuthenticateResult, +} from '@tech-matters/hrm-authentication'; -export const fileTypes = { - recording: 'Recording', - transcript: 'ExternalTranscript', - document: 'Case', -} as const; +import { fileTypes, FileTypes, Parameters } from './parseParameters'; -export type FileTypes = keyof typeof fileTypes; +export const mockBuckets = ['mock-bucket']; export type FileMethods = 'getObject' | 'putObject' | 'deleteObject'; +export type AuthenticateParams = Parameters & { + authHeader: string; +}; + export const fileMethods: Record< HRMAuthenticationObjectTypes, Partial> @@ -80,22 +77,18 @@ export type HrmAuthenticateFilesUrlsRequestData = { export const authUrlPathGenerator = ({ accountSid, objectType, - requestData: { fileType, method }, -}: HrmAuthenticateParameters) => { + fileType, + method, +}: AuthenticateParams) => { const permission = getPermission({ objectType, fileType, method }); return `v0/accounts/${accountSid}/permissions/${permission}`; }; -const filesUrlsAuthenticator = async ( - params: HrmAuthenticateParameters, +const authenticate = async ( + params: AuthenticateParams, ): Promise => { - const { - objectId, - objectType, - authHeader, - requestData: { bucket, key }, - } = params; + const { objectId, objectType, authHeader, bucket, key } = params; // This is a quick and dirty way to lock this down so we can test // with fake data without exposing real data in the test environment. @@ -120,4 +113,4 @@ const filesUrlsAuthenticator = async ( return newOk({ data: true }); }; -export default filesUrlsAuthenticator; +export default authenticate; diff --git a/hrm-domain/lambdas/files-urls/getSignedS3Url.ts b/hrm-domain/lambdas/files-urls/getSignedS3Url.ts index b7025e0dc..d480ba787 100644 --- a/hrm-domain/lambdas/files-urls/getSignedS3Url.ts +++ b/hrm-domain/lambdas/files-urls/getSignedS3Url.ts @@ -16,8 +16,8 @@ import { AlbHandlerEvent } from '@tech-matters/alb-handler'; import { TResult, newErr, isErr, newOk } from '@tech-matters/types'; -import { authenticate } from '@tech-matters/hrm-authentication'; import { getSignedUrl } from '@tech-matters/s3-client'; +import authenticate from './authenticate'; import { parseParameters } from './parseParameters'; export type GetSignedS3UrlSuccessResultData = { @@ -51,28 +51,16 @@ const getSignedS3Url = async (event: AlbHandlerEvent): Promise> = { contact: { requiredParameters: ['objectId'], diff --git a/lambdas/packages/hrm-authentication/callHrmApi.ts b/lambdas/packages/hrm-authentication/callHrmApi.ts index 1c5c98d97..eeefe4a4c 100644 --- a/lambdas/packages/hrm-authentication/callHrmApi.ts +++ b/lambdas/packages/hrm-authentication/callHrmApi.ts @@ -23,7 +23,11 @@ export type CallHrmApiParameters = { requestData?: any; }; -const callHrmApi = async ({ urlPath, requestData, authHeader }: CallHrmApiParameters) => { +export const callHrmApi = async ({ + urlPath, + requestData, + authHeader, +}: CallHrmApiParameters) => { const params = new URLSearchParams(requestData).toString(); const fullUrl = params ? `${process.env.HRM_BASE_URL}/${urlPath}?${params}` @@ -49,5 +53,3 @@ const callHrmApi = async ({ urlPath, requestData, authHeader }: CallHrmApiParame const data = await response.json(); return newOk({ data }); }; - -export default callHrmApi; diff --git a/lambdas/packages/hrm-authentication/index.ts b/lambdas/packages/hrm-authentication/index.ts index e75353356..c0b46f573 100644 --- a/lambdas/packages/hrm-authentication/index.ts +++ b/lambdas/packages/hrm-authentication/index.ts @@ -14,25 +14,6 @@ * along with this program. If not, see https://www.gnu.org/licenses/. */ import { TResult } from '@tech-matters/types'; -import filesUrlsAuthenticator, { - HrmAuthenticateFilesUrlsRequestData, -} from './filesUrlsAuthenticator'; - -/** - * The authenticator will call the authenticator based on the type. - * In a perfect world the hrm side of authentication would be a single endpoint - * that would accept a common payload and return a common response. - * And this very leaky abstraction would not be needed. - * - * For now we have to support multiple endpoints and multiple payloads with - * different responses, so the function is basically an adapter. - * - * The goal was to keep all hrm authentication transformations centralized - * in a single place to aid in the future refactoring. - */ -const types = { - filesUrls: (params: HrmAuthenticateParameters) => filesUrlsAuthenticator(params), -}; /** * The object types that can be authenticated. @@ -48,23 +29,6 @@ export const isAuthenticationObjectType = ( type: string, ): type is HRMAuthenticationObjectTypes => Object.keys(objectTypes).includes(type); -export type HrmAuthenticateTypes = keyof typeof types; - export type HrmAuthenticateResult = TResult; -export type HrmAuthenticateParameters = { - accountSid: string; - objectType: HRMAuthenticationObjectTypes; - objectId?: string; - type: HrmAuthenticateTypes; - authHeader: string; - requestData: HrmAuthenticateFilesUrlsRequestData; -}; - -export const authenticate = async ( - params: HrmAuthenticateParameters, -): Promise => { - return types[params.type](params); -}; - -export { FileTypes } from './filesUrlsAuthenticator'; +export * from './callHrmApi'; From bf68353d3fadd0323ebc50d759a0161534fb6c6c Mon Sep 17 00:00:00 2001 From: Robert Bo Davis Date: Wed, 29 Nov 2023 11:24:05 -0500 Subject: [PATCH 2/2] refactor: continue hrm-auth separation of concerns refactor --- hrm-domain/lambdas/files-urls/authenticate.ts | 42 ++++++++----------- lambdas/packages/alb-handler/package.json | 1 - .../integration/hrm-autentication.test.ts | 21 ---------- .../packages/hrm-authentication/callHrmApi.ts | 17 ++++---- lambdas/packages/hrm-authentication/index.ts | 15 ++++++- 5 files changed, 41 insertions(+), 55 deletions(-) delete mode 100644 lambdas/packages/alb-handler/tests/integration/hrm-autentication.test.ts diff --git a/hrm-domain/lambdas/files-urls/authenticate.ts b/hrm-domain/lambdas/files-urls/authenticate.ts index 9641e8d27..7e4fef983 100644 --- a/hrm-domain/lambdas/files-urls/authenticate.ts +++ b/hrm-domain/lambdas/files-urls/authenticate.ts @@ -14,9 +14,9 @@ * along with this program. If not, see https://www.gnu.org/licenses/. */ -import { newOk, isErr } from '@tech-matters/types'; +import { newOk } from '@tech-matters/types'; import { - callHrmApi, + authenticate, HRMAuthenticationObjectTypes, HrmAuthenticateResult, } from '@tech-matters/hrm-authentication'; @@ -74,30 +74,29 @@ export type HrmAuthenticateFilesUrlsRequestData = { fileType: FileTypes; }; -export const authUrlPathGenerator = ({ +const authenticateFilesUrls = async ({ accountSid, - objectType, - fileType, method, -}: AuthenticateParams) => { - const permission = getPermission({ objectType, fileType, method }); - - return `v0/accounts/${accountSid}/permissions/${permission}`; -}; - -const authenticate = async ( - params: AuthenticateParams, -): Promise => { - const { objectId, objectType, authHeader, bucket, key } = params; - + fileType, + objectId, + objectType, + authHeader, + bucket, + key, +}: AuthenticateParams): Promise => { // This is a quick and dirty way to lock this down so we can test // with fake data without exposing real data in the test environment. if (mockBuckets.includes(bucket)) { return newOk({ data: true }); } - const result = await callHrmApi({ - urlPath: authUrlPathGenerator(params), + return authenticate({ + accountSid, + permission: getPermission({ + objectType, + fileType, + method, + }), authHeader, requestData: { objectType, @@ -106,11 +105,6 @@ const authenticate = async ( key, }, }); - if (isErr(result)) { - return result; - } - - return newOk({ data: true }); }; -export default authenticate; +export default authenticateFilesUrls; diff --git a/lambdas/packages/alb-handler/package.json b/lambdas/packages/alb-handler/package.json index 7fa4ab2e8..51620e4c6 100644 --- a/lambdas/packages/alb-handler/package.json +++ b/lambdas/packages/alb-handler/package.json @@ -12,7 +12,6 @@ "@types/aws-lambda": "^8.10.108" }, "scripts": { - "test:integration": "cross-env S3_FORCE_PATH_STYLE=true S3_ENDPOINT=http://localhost:4566 SQS_ENDPOINT=http://localhost:4566 jest tests/integration", "test:unit": "jest tests/unit" } } diff --git a/lambdas/packages/alb-handler/tests/integration/hrm-autentication.test.ts b/lambdas/packages/alb-handler/tests/integration/hrm-autentication.test.ts deleted file mode 100644 index 051a44563..000000000 --- a/lambdas/packages/alb-handler/tests/integration/hrm-autentication.test.ts +++ /dev/null @@ -1,21 +0,0 @@ -/** - * Copyright (C) 2021-2023 Technology Matters - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published - * by the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see https://www.gnu.org/licenses/. - */ - -describe('alb-handler', () => { - it('fake test', () => { - expect(true).toBe(true); - }); -}); diff --git a/lambdas/packages/hrm-authentication/callHrmApi.ts b/lambdas/packages/hrm-authentication/callHrmApi.ts index eeefe4a4c..73d0769e6 100644 --- a/lambdas/packages/hrm-authentication/callHrmApi.ts +++ b/lambdas/packages/hrm-authentication/callHrmApi.ts @@ -17,18 +17,21 @@ import { newErr, newOk } from '@tech-matters/types'; import { URLSearchParams } from 'url'; -export type CallHrmApiParameters = { - urlPath: string; +export type CallHrmApiParameters = { + accountSid: string; + permission: string; authHeader: string; - requestData?: any; + requestData?: TData; }; -export const callHrmApi = async ({ - urlPath, +export const callHrmApi = async ({ + accountSid, + permission, requestData, authHeader, -}: CallHrmApiParameters) => { - const params = new URLSearchParams(requestData).toString(); +}: CallHrmApiParameters) => { + const urlPath = `v0/accounts/${accountSid}/permissions/${permission}`; + const params = requestData ? new URLSearchParams(requestData).toString() : ''; const fullUrl = params ? `${process.env.HRM_BASE_URL}/${urlPath}?${params}` : `${process.env.HRM_BASE_URL}/${urlPath}`; diff --git a/lambdas/packages/hrm-authentication/index.ts b/lambdas/packages/hrm-authentication/index.ts index c0b46f573..cbb2a4139 100644 --- a/lambdas/packages/hrm-authentication/index.ts +++ b/lambdas/packages/hrm-authentication/index.ts @@ -13,7 +13,9 @@ * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see https://www.gnu.org/licenses/. */ -import { TResult } from '@tech-matters/types'; +import { TResult, newOk, isErr } from '@tech-matters/types'; + +import { callHrmApi, CallHrmApiParameters } from './callHrmApi'; /** * The object types that can be authenticated. @@ -31,4 +33,13 @@ export const isAuthenticationObjectType = ( export type HrmAuthenticateResult = TResult; -export * from './callHrmApi'; +export const authenticate = async ( + params: CallHrmApiParameters, +): Promise => { + const result = await callHrmApi(params); + if (isErr(result)) { + return result; + } + + return newOk({ data: true }); +};