From 974b2765b71358dcf9efa5df9583e49e666087b0 Mon Sep 17 00:00:00 2001 From: Gianfranco Paoloni Date: Tue, 12 Dec 2023 13:09:30 -0300 Subject: [PATCH 1/9] refactor: rename initialize can function, export it's type --- .../hrm-service/src/case/caseService.ts | 12 +++++------ .../hrm-service/src/contact/contactService.ts | 20 +++++++++---------- .../permissions/canPerformActionOnObject.ts | 4 ++-- .../hrm-service/src/permissions/index.ts | 15 ++++++-------- .../src/permissions/setupCanForRules.ts | 4 +++- 5 files changed, 27 insertions(+), 28 deletions(-) diff --git a/hrm-domain/hrm-service/src/case/caseService.ts b/hrm-domain/hrm-service/src/case/caseService.ts index 934b70c6a..a9eaae329 100644 --- a/hrm-domain/hrm-service/src/case/caseService.ts +++ b/hrm-domain/hrm-service/src/case/caseService.ts @@ -36,7 +36,7 @@ import { } from './case-data-access'; import { randomUUID } from 'crypto'; import type { Contact } from '../contact/contact-data-access'; -import { setupCanForRules } from '../permissions/setupCanForRules'; +import { InitializedCan } from '../permissions/setupCanForRules'; import type { TwilioUser } from '@tech-matters/twilio-worker-auth'; import { bindApplyTransformations as bindApplyContactTransformations, @@ -211,7 +211,7 @@ const caseRecordToCase = (record: CaseRecordWithLegacyCategoryContacts): CaseSer }; const mapContactTransformations = - ({ can, user }: { can: ReturnType; user: TwilioUser }) => + ({ can, user }: { can: InitializedCan; user: TwilioUser }) => (caseRecord: CaseRecord) => { const applyTransformations = bindApplyContactTransformations(can, user); const withTransformedContacts = { @@ -254,7 +254,7 @@ export const updateCase = async ( body: Partial, accountSid: CaseService['accountSid'], workerSid: CaseService['twilioWorkerId'], - { can, user }: { can: ReturnType; user: TwilioUser }, + { can, user }: { can: InitializedCan; user: TwilioUser }, ): Promise => { const caseFromDB: CaseRecord = await getById(id, accountSid); if (!caseFromDB) { @@ -279,7 +279,7 @@ export const updateCase = async ( export const getCase = async ( id: number, accountSid: string, - { can, user }: { can: ReturnType; user: TwilioUser }, + { can, user }: { can: InitializedCan; user: TwilioUser }, ): Promise => { const caseFromDb = await getById(id, accountSid); @@ -348,7 +348,7 @@ const generalizedSearchCases = user, searchPermissions, }: { - can: ReturnType; + can: InitializedCan; user: TwilioUser; searchPermissions: SearchPermissions; }, @@ -412,7 +412,7 @@ export const getCasesByProfileId = async ( profileId: Profile['id'], query: Pick, ctx: { - can: ReturnType; + can: InitializedCan; user: TwilioUser; searchPermissions: SearchPermissions; }, diff --git a/hrm-domain/hrm-service/src/contact/contactService.ts b/hrm-domain/hrm-service/src/contact/contactService.ts index 53069427c..c70907211 100644 --- a/hrm-domain/hrm-service/src/contact/contactService.ts +++ b/hrm-domain/hrm-service/src/contact/contactService.ts @@ -32,7 +32,7 @@ import { retrieveCategories } from './categories'; import { PaginationQuery, getPaginationElements } from '../search'; import type { NewContactRecord } from './sql/contact-insert-sql'; import { ContactRawJson, ReferralWithoutContactId } from './contact-json'; -import { setupCanForRules } from '../permissions/setupCanForRules'; +import { InitializedCan } from '../permissions/setupCanForRules'; import { actionsMaps } from '../permissions'; import type { TwilioUser } from '@tech-matters/twilio-worker-auth'; import { connectContactToCsamReports, CSAMReport } from '../csam-report/csam-report'; @@ -203,7 +203,7 @@ const permissionsBasedTransformations: PermissionsBasedTransformation[] = [ ]; export const bindApplyTransformations = - (can: ReturnType, user: TwilioUser) => + (can: InitializedCan, user: TwilioUser) => (contact: Contact): WithLegacyCategories => { const permissionsBasedTransformed = permissionsBasedTransformations.reduce( (transformed, { action, transformation }) => @@ -220,7 +220,7 @@ export const bindApplyTransformations = export const getContactById = async ( accountSid: string, contactId: number, - { can, user }: { can: ReturnType; user: TwilioUser }, + { can, user }: { can: InitializedCan; user: TwilioUser }, ) => { const contact = await getById(accountSid, contactId); @@ -230,7 +230,7 @@ export const getContactById = async ( export const getContactByTaskId = async ( accountSid: string, taskId: string, - { can, user }: { can: ReturnType; user: TwilioUser }, + { can, user }: { can: InitializedCan; user: TwilioUser }, ) => { const contact = await getByTaskSid(accountSid, taskId); @@ -309,7 +309,7 @@ export const createContact = async ( createdBy: string, finalize: boolean, newContact: CreateContactPayload, - { can, user }: { can: ReturnType; user: TwilioUser }, + { can, user }: { can: InitializedCan; user: TwilioUser }, ): Promise> => { for (let retries = 1; retries < 4; retries++) { try { @@ -457,7 +457,7 @@ export const patchContact = async ( finalize: boolean, contactId: string, contactPatch: PatchPayload, - { can, user }: { can: ReturnType; user: TwilioUser }, + { can, user }: { can: InitializedCan; user: TwilioUser }, ): Promise> => { const { referrals, rawJson, ...restOfPatch } = adaptLegacyCategories(contactPatch); @@ -499,7 +499,7 @@ export const connectContactToCase = async ( updatedBy: string, contactId: string, caseId: string, - { can, user }: { can: ReturnType; user: TwilioUser }, + { can, user }: { can: InitializedCan; user: TwilioUser }, ): Promise> => { const updated: Contact | undefined = await connectToCase(accountSid, contactId, caseId); if (!updated) { @@ -514,7 +514,7 @@ export const addConversationMediaToContact = async ( accountSid: string, contactId: string, conversationMediaPayload: ConversationMedia[], - { can, user }: { can: ReturnType; user: TwilioUser }, + { can, user }: { can: InitializedCan; user: TwilioUser }, ): Promise> => { const contact = await getById(accountSid, parseInt(contactId)); if (!contact) { @@ -654,7 +654,7 @@ const generalizedSearchContacts = user, searchPermissions, }: { - can: ReturnType; + can: InitializedCan; user: TwilioUser; searchPermissions: SearchPermissions; }, @@ -717,7 +717,7 @@ export const getContactsByProfileId = async ( profileId: Profile['id'], query: Pick, ctx: { - can: ReturnType; + can: InitializedCan; user: TwilioUser; searchPermissions: SearchPermissions; }, diff --git a/hrm-domain/hrm-service/src/permissions/canPerformActionOnObject.ts b/hrm-domain/hrm-service/src/permissions/canPerformActionOnObject.ts index f43814081..a591d0bdd 100644 --- a/hrm-domain/hrm-service/src/permissions/canPerformActionOnObject.ts +++ b/hrm-domain/hrm-service/src/permissions/canPerformActionOnObject.ts @@ -16,7 +16,7 @@ import { TwilioUser } from '@tech-matters/twilio-worker-auth'; import { Actions, TargetKind, isValidSetOfActionsForTarget } from './actions'; -import { setupCanForRules } from './setupCanForRules'; +import { InitializedCan } from './setupCanForRules'; import { getContactById } from '../contact/contactService'; import { getCase as getCaseById } from '../case/caseService'; import { assertExhaustive } from '../contact-job/assertExhaustive'; @@ -38,7 +38,7 @@ export const canPerformActionsOnObject = async ({ objectId: number; targetKind: T; actions: string[]; - can: ReturnType; + can: InitializedCan; user: TwilioUser; }): Promise> => { try { diff --git a/hrm-domain/hrm-service/src/permissions/index.ts b/hrm-domain/hrm-service/src/permissions/index.ts index dbd4ad671..414b6759a 100644 --- a/hrm-domain/hrm-service/src/permissions/index.ts +++ b/hrm-domain/hrm-service/src/permissions/index.ts @@ -20,12 +20,12 @@ export { SafeRouter, publicEndpoint } from './safe-router'; export { rulesMap } from './rulesMap'; export { Actions, actionsMaps, getActions } from './actions'; -import { setupCanForRules } from './setupCanForRules'; +import { InitializedCan, initializeCanForRules } from './setupCanForRules'; import { RulesFile } from './rulesMap'; import type { Request, Response, NextFunction } from 'express'; import { getSearchPermissions, SearchPermissions } from './search-permissions'; -const canCache: Record> = {}; +const canCache: Record = {}; export type Permissions = { rules: (accountSid: string) => RulesFile; @@ -36,10 +36,7 @@ export type Permissions = { * Applies the permissions if valid. * @throws Will throw if initializedCan is not a function */ -export const applyPermissions = ( - req: Request, - initializedCan: ReturnType, -) => { +export const applyPermissions = (req: Request, initializedCan: InitializedCan) => { if (typeof initializedCan !== 'function') throw new Error(`Error in looked up permission rules: can is not a function.`); @@ -52,11 +49,11 @@ export const setupPermissions = const { accountSid } = req; if (lookup.cachePermissions) { canCache[accountSid] = - canCache[accountSid] ?? setupCanForRules(lookup.rules(accountSid)); + canCache[accountSid] ?? initializeCanForRules(lookup.rules(accountSid)); const initializedCan = canCache[accountSid]; applyPermissions(req, initializedCan); } else { - applyPermissions(req, setupCanForRules(lookup.rules(accountSid))); + applyPermissions(req, initializeCanForRules(lookup.rules(accountSid))); } //@ts-ignore TODO: Improve our custom Request type to override Express.Request req.searchPermissions = getSearchPermissions(req, lookup.rules(accountSid)); @@ -65,5 +62,5 @@ export const setupPermissions = export type RequestWithPermissions = SafeRouterRequest & SearchPermissions & { - can: ReturnType; + can: InitializedCan; }; diff --git a/hrm-domain/hrm-service/src/permissions/setupCanForRules.ts b/hrm-domain/hrm-service/src/permissions/setupCanForRules.ts index b66232eb5..bc49b6b12 100644 --- a/hrm-domain/hrm-service/src/permissions/setupCanForRules.ts +++ b/hrm-domain/hrm-service/src/permissions/setupCanForRules.ts @@ -76,7 +76,7 @@ const setupAllow = (targetKind: string, conditionsSets: ConditionsSets) => { }; }; -export const setupCanForRules = (rules: RulesFile) => { +export const initializeCanForRules = (rules: RulesFile) => { const actionCheckers = {} as { [action in Actions]: ReturnType }; const targetKinds = Object.keys(actionsMaps); @@ -93,3 +93,5 @@ export const setupCanForRules = (rules: RulesFile) => { return (performer: TwilioUser, action: Actions, target: any) => actionCheckers[action](performer, target); }; + +export type InitializedCan = ReturnType; From 82f45e9b3e4463ae654492f132496e166941ebd1 Mon Sep 17 00:00:00 2001 From: Gianfranco Paoloni Date: Tue, 12 Dec 2023 18:37:39 -0300 Subject: [PATCH 2/9] refactor: permission framework types and maps tighten accordingly to each TargetKind --- .../service-tests/search-permissions.test.ts | 12 +- .../hrm-service/src/permissions/rulesMap.ts | 112 ++++++++++++------ .../src/permissions/search-permissions.ts | 25 ++-- .../src/permissions/setupCanForRules.ts | 74 ++++++++---- 4 files changed, 153 insertions(+), 70 deletions(-) diff --git a/hrm-domain/hrm-service/service-tests/search-permissions.test.ts b/hrm-domain/hrm-service/service-tests/search-permissions.test.ts index b112c14f5..54abfe580 100644 --- a/hrm-domain/hrm-service/service-tests/search-permissions.test.ts +++ b/hrm-domain/hrm-service/service-tests/search-permissions.test.ts @@ -20,7 +20,7 @@ import { randomBytes } from 'crypto'; import { mockingProxy, mockSuccessfulTwilioAuthentication } from '@tech-matters/testing'; import { db } from '../src/connection-pool'; -import { ConditionsSets, RulesFile } from '../src/permissions/rulesMap'; +import { TKConditionsSets, RulesFile } from '../src/permissions/rulesMap'; import { headers, getRequest, @@ -31,6 +31,7 @@ import { } from './server'; import { SearchParameters as ContactSearchParameters } from '../src/contact/contact-data-access'; import { SearchParameters as CaseSearchParameters } from '../src/case/caseService'; +import { TargetKind } from '../src/permissions/actions'; useOpenRules(); const server = getServer(); @@ -98,7 +99,10 @@ afterEach(async () => { await cleanUpDB(); }); -const overridePermissions = (key: string, permissions: ConditionsSets) => { +const overridePermissions = ( + key: string, + permissions: TKConditionsSets, +) => { useOpenRules(); const rules: RulesFile = { ...(defaultConfig.permissions?.rules() as RulesFile), @@ -107,10 +111,10 @@ const overridePermissions = (key: string, permissions: ConditionsSets) => { setRules(rules); }; -const overrideViewContactPermissions = (permissions: ConditionsSets) => +const overrideViewContactPermissions = (permissions: TKConditionsSets<'contact'>) => overridePermissions('viewContact', permissions); -const overrideViewCasePermissions = (permissions: ConditionsSets) => +const overrideViewCasePermissions = (permissions: TKConditionsSets<'case'>) => overridePermissions('viewCase', permissions); describe('search contacts permissions', () => { diff --git a/hrm-domain/hrm-service/src/permissions/rulesMap.ts b/hrm-domain/hrm-service/src/permissions/rulesMap.ts index 395232016..f84099164 100644 --- a/hrm-domain/hrm-service/src/permissions/rulesMap.ts +++ b/hrm-domain/hrm-service/src/permissions/rulesMap.ts @@ -40,49 +40,89 @@ const zaRules = require('../../permission-rules/za.json'); const zmRules = require('../../permission-rules/zm.json'); const zwRules = require('../../permission-rules/zw.json'); -import { actionsMaps, Actions, TargetKind } from './actions'; - -const conditionTypes = [ - 'isSupervisor', - 'isCreator', - 'isCaseOpen', - 'isOwner', - 'everyone', -] as const; -export type Condition = (typeof conditionTypes)[number]; -export type ConditionsSet = Condition[]; -export type ConditionsSets = ConditionsSet[]; - -const isCondition = (c: any): c is Condition => c && conditionTypes.includes(c); -const isConditionsSet = (cs: any): cs is ConditionsSet => - cs && Array.isArray(cs) && cs.every(isCondition); -const isConditionsSets = (css: any): css is ConditionsSets => - css && Array.isArray(css) && css.every(isConditionsSet); - -export type RulesFile = { [k in Actions]: ConditionsSets }; +import { actionsMaps, Actions, TargetKind, isTargetKind } from './actions'; -export const isRulesFile = (rules: any): rules is RulesFile => - Object.values(actionsMaps).every(map => - Object.values(map).every(action => isConditionsSets(rules[action])), - ); +const timeBasedConditions = { + CREATED_HOURS_AGO: 'createdHoursAgo', + CREATED_DAYS_AGO: 'createdDaysAgo', +} as const; +const userBasedConditions = { + IS_SUPERVISOR: 'isSupervisor', + EVERYONE: 'everyone', +} as const; +const contactSpecificConditions = { + IS_OWNER: 'isOwner', +} as const; +const caseSpecificConditions = { + IS_CREATOR: 'isCreator', + IS_CASE_OPEN: 'isCaseOpen', +} as const; -// Defines which actions are supported on each TargetKind -const supportedTargetKindActions: { [k in TargetKind]: ConditionsSet } = { - case: ['isSupervisor', 'isCreator', 'isCaseOpen', 'everyone'], - contact: ['isSupervisor', 'isOwner', 'everyone'], - postSurvey: ['isSupervisor', 'everyone'], +export const supportedContactConditionsMap = { + ...timeBasedConditions, + ...userBasedConditions, + ...contactSpecificConditions, +} as const; +const supportedContactConditions = Object.values(supportedContactConditionsMap); + +export const supportedCaseConditionsMap = { + ...timeBasedConditions, + ...userBasedConditions, + ...caseSpecificConditions, }; +const supportedCaseConditions = Object.values(supportedCaseConditionsMap); + +const supportedPostSurveyConditions = [...Object.values(userBasedConditions)] as const; + +// Defines which actions are supported on each TargetKind +const supportedTKConditions = { + contact: supportedContactConditions, + case: supportedCaseConditions, + postSurvey: supportedPostSurveyConditions, +} as const; -const isValidTargetKind = (kind: string, css: ConditionsSets) => - css.every(cs => cs.every(c => supportedTargetKindActions[kind].includes(c))); +export type TKCondition = (typeof supportedTKConditions)[T][number]; +export type TKConditionsSet = TKCondition[]; +export type TKConditionsSets = TKConditionsSet[]; -const validateTargetKindActions = (rules: RulesFile) => +const isTKCondition = + (kind: T) => + (c: any): c is TKCondition => + c && supportedTKConditions[kind].includes(c); + +const isTKConditionsSet = + (kind: TargetKind) => + (cs: any): cs is TKConditionsSet => + cs && Array.isArray(cs) && cs.every(isTKCondition(kind)); + +const isTKConditionsSets = + (kind: TargetKind) => + (css: any): css is TKConditionsSets => + css && Array.isArray(css) && css.every(isTKConditionsSet(kind)); + +export type RulesFile = { [k in Actions]: TKConditionsSets }; + +export const isValidTKConditionsSets = + (kind: T) => + (css: TKConditionsSets): css is TKConditionsSets => + css.every(cs => cs.every(isTKCondition(kind))); + +export const isRulesFile = (rules: any): rules is RulesFile => + Object.values(actionsMaps).every(map => + Object.values(map).every(action => isTKConditionsSets(rules[action])), + ); + +/** + * Validates that for every TK, the ConditionsSets provided are valid + * (i.e. present in supportedTKConditions) + */ +const validateTKActions = (rules: RulesFile) => Object.entries(actionsMaps) .map(([kind, map]) => Object.values(map).reduce((accum, action) => { return { ...accum, - [action]: isValidTargetKind(kind, rules[action]), + [action]: isTargetKind(kind) && isValidTKConditionsSets(kind)(rules[action]), }; }, {}), ) @@ -122,6 +162,10 @@ const rulesMapDef = { e2e: e2eRules, } as const; +/** + * For every entry of rulesMapDef, validates that every are valid RulesFile definitions, + * and that the actions on each TK are provided with valid TKConditionsSets + */ export const validRulesMap = () => // This type assertion is legit as long as we check that every entry in rulesMapDef is indeed a RulesFile Object.entries(rulesMapDef).reduce<{ [k in keyof typeof rulesMapDef]: RulesFile }>( @@ -130,7 +174,7 @@ export const validRulesMap = () => throw new Error(`Error: rules file for ${k} is not a valid RulesFile`); } - const validated = validateTargetKindActions(rules); + const validated = validateTKActions(rules); if (!isValidTargetKindActions(validated)) { const invalidActions = Object.entries(validated) .filter(([, val]) => !val) diff --git a/hrm-domain/hrm-service/src/permissions/search-permissions.ts b/hrm-domain/hrm-service/src/permissions/search-permissions.ts index b8e361444..8d158f4bf 100644 --- a/hrm-domain/hrm-service/src/permissions/search-permissions.ts +++ b/hrm-domain/hrm-service/src/permissions/search-permissions.ts @@ -32,7 +32,8 @@ import { TwilioUser } from '@tech-matters/twilio-worker-auth'; import isEqual from 'lodash/isEqual'; import sortBy from 'lodash/sortBy'; -import { RulesFile, ConditionsSet } from './rulesMap'; +import type { RulesFile, TKConditionsSet } from './rulesMap'; +import type { TargetKind } from './actions'; type Request = { can?: any; @@ -44,12 +45,14 @@ export type SearchPermissions = { canOnlyViewOwnContacts?: boolean; }; -type TargetRule = Partial>; +type TargetRule = Partial< + Record> +>; const applySearchCasesPermissions = ( req: Request, searchPermissions: SearchPermissions, - checkRule: ReturnType, + checkRule: ReturnType>, ) => { const { isSupervisor } = req.user; const canViewAsSupervisor = isSupervisor && checkRule({ viewCase: ['isSupervisor'] }); @@ -65,7 +68,7 @@ const applySearchCasesPermissions = ( const applySearchContactsPermissions = ( req: Request, searchPermissions: SearchPermissions, - checkRule: ReturnType, + checkRule: ReturnType>, ) => { const { isSupervisor } = req.user; const canViewAsSupervisor = @@ -87,12 +90,14 @@ const applySearchContactsPermissions = ( * checkRule({ viewContact: ['isOwner'] }); // returns true or false * checkRule({ viewCase: ['isCreator'] }); // returns true or false */ -const buildCheckRule = (rulesFile: RulesFile) => (targetRule: TargetRule) => { - const rule = Object.keys(targetRule)[0]; - const conditionSetIsEqual = conditionSet => - isEqual(sortBy(conditionSet), sortBy(targetRule[rule])); - return rulesFile[rule].some(conditionSetIsEqual); -}; +const buildCheckRule = + (rulesFile: RulesFile) => + (targetRule: TargetRule) => { + const rule = Object.keys(targetRule)[0]; + const conditionSetIsEqual = conditionSet => + isEqual(sortBy(conditionSet), sortBy(targetRule[rule])); + return rulesFile[rule].some(conditionSetIsEqual); + }; export const getSearchPermissions = (req: Request, rulesFile: RulesFile) => { const checkRule = buildCheckRule(rulesFile); diff --git a/hrm-domain/hrm-service/src/permissions/setupCanForRules.ts b/hrm-domain/hrm-service/src/permissions/setupCanForRules.ts index bc49b6b12..c1b0a7008 100644 --- a/hrm-domain/hrm-service/src/permissions/setupCanForRules.ts +++ b/hrm-domain/hrm-service/src/permissions/setupCanForRules.ts @@ -15,64 +15,94 @@ */ import { isCounselorWhoCreated, isCaseOpen, isContactOwner } from './helpers'; -import { actionsMaps, Actions, isTargetKind } from './actions'; -import type { Condition, ConditionsSet, ConditionsSets, RulesFile } from './rulesMap'; +import { actionsMaps, Actions, isTargetKind, TargetKind } from './actions'; +import { + isValidTKConditionsSets, + type TKCondition, + type TKConditionsSet, + type TKConditionsSets, + type RulesFile, +} from './rulesMap'; import { TwilioUser } from '@tech-matters/twilio-worker-auth'; +type ConditionsState = { + [condition in TKCondition]: boolean; +}; + /** * Given a conditionsState and a condition, returns true if the condition is true in the conditionsState */ const checkCondition = - (conditionsState: { [condition in Condition]: boolean }) => - (condition: Condition): boolean => + (conditionsState: ConditionsState) => + (condition: TKCondition): boolean => conditionsState[condition]; /** * Given a conditionsState and a set of conditions, returns true if all the conditions are true in the conditionsState */ const checkConditionsSet = - (conditionsState: { [condition in Condition]: boolean }) => - (conditionsSet: ConditionsSet): boolean => + (conditionsState: ConditionsState) => + (conditionsSet: TKConditionsSet): boolean => conditionsSet.length > 0 && conditionsSet.every(checkCondition(conditionsState)); /** * Given a conditionsState and a set of conditions sets, returns true if one of the conditions sets contains conditions that are all true in the conditionsState */ -const checkConditionsSets = ( - conditionsState: { [condition in Condition]: boolean }, - conditionsSets: ConditionsSets, +const checkConditionsSets = ( + conditionsState: ConditionsState, + conditionsSets: TKConditionsSets, ): boolean => conditionsSets.some(checkConditionsSet(conditionsState)); -const setupAllow = (targetKind: string, conditionsSets: ConditionsSets) => { - if (!isTargetKind(targetKind)) - throw new Error(`Invalid target kind ${targetKind} provided to setupAllow`); - +// const setupAllow = (kind: T, conditionsSets: ConditionsSets) => { +const setupAllow = ( + kind: T, + conditionsSets: TKConditionsSets, +) => { // We could do type validation on target depending on targetKind if we ever want to make sure the "allow" is called on a proper target (same as cancan used to do) return (performer: TwilioUser, target: any) => { // Build the proper conditionsState depending on the targetKind - let conditionsState = null; - if (targetKind === 'case') { - conditionsState = { + if (kind === 'case') { + if (!isValidTKConditionsSets<'case'>(kind)(conditionsSets)) { + throw new Error(`setupAllow: Invalid conditionsSets provided for kind ${kind}`); + } + + const conditionsState: ConditionsState<'case'> = { isSupervisor: performer.isSupervisor, isCreator: isCounselorWhoCreated(performer, target), isCaseOpen: isCaseOpen(target), everyone: true, + createdDaysAgo: false, + createdHoursAgo: false, }; - } else if (targetKind === 'contact') { - conditionsState = { + + return checkConditionsSets(conditionsState, conditionsSets); + } else if (kind === 'contact') { + if (!isValidTKConditionsSets<'contact'>(kind)(conditionsSets)) { + throw new Error(`setupAllow: Invalid conditionsSets provided for kind ${kind}`); + } + + const conditionsState: ConditionsState<'contact'> = { isSupervisor: performer.isSupervisor, isOwner: isContactOwner(performer, target), everyone: true, + createdDaysAgo: false, + createdHoursAgo: false, }; - } else if (targetKind === 'postSurvey') { - conditionsState = { + + return checkConditionsSets(conditionsState, conditionsSets); + } else if (kind === 'postSurvey') { + if (!isValidTKConditionsSets<'postSurvey'>(kind)(conditionsSets)) { + throw new Error(`setupAllow: Invalid conditionsSets provided for kind ${kind}`); + } + + const conditionsState: ConditionsState<'postSurvey'> = { isSupervisor: performer.isSupervisor, everyone: true, }; - } - return checkConditionsSets(conditionsState, conditionsSets); + return checkConditionsSets(conditionsState, conditionsSets); + } }; }; From 9c5050e5536fd3b4e503758381e1c2ba2d5242a8 Mon Sep 17 00:00:00 2001 From: Gianfranco Paoloni Date: Fri, 15 Dec 2023 01:55:48 -0300 Subject: [PATCH 3/9] chore: add eslint unused rule --- .eslintrc.js | 1 + 1 file changed, 1 insertion(+) diff --git a/.eslintrc.js b/.eslintrc.js index 886e837f2..bf27b14f1 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -23,6 +23,7 @@ module.exports = { '@typescript-eslint/indent': 'off', '@typescript-eslint/quotes': 'off', 'import/prefer-default-export': 'off', + '@typescript-eslint/no-unused-vars': ['error', { argsIgnorePattern: '^_' }], }, settings: { 'import/resolver': { From 05b0b7b31c81362f39ea66f15b6687ebb6c3ea4b Mon Sep 17 00:00:00 2001 From: Gianfranco Paoloni Date: Fri, 15 Dec 2023 01:56:40 -0300 Subject: [PATCH 4/9] Added ts-parsec dependency --- hrm-domain/hrm-service/package.json | 3 ++- package-lock.json | 14 +++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/hrm-domain/hrm-service/package.json b/hrm-domain/hrm-service/package.json index 41cb6f356..006f398af 100644 --- a/hrm-domain/hrm-service/package.json +++ b/hrm-domain/hrm-service/package.json @@ -66,7 +66,8 @@ "pg-hstore": "^2.3.4", "pg-promise": "^10.11.1", "twilio": "^3.58.0", - "twilio-flex-token-validator": "^1.5.5" + "twilio-flex-token-validator": "^1.5.5", + "typescript-parsec": "^0.3.4" }, "devDependencies": { "@tech-matters/testing": "^1.0.0", diff --git a/package-lock.json b/package-lock.json index e852d2feb..599b9d136 100644 --- a/package-lock.json +++ b/package-lock.json @@ -89,7 +89,8 @@ "pg-hstore": "^2.3.4", "pg-promise": "^10.11.1", "twilio": "^3.58.0", - "twilio-flex-token-validator": "^1.5.5" + "twilio-flex-token-validator": "^1.5.5", + "typescript-parsec": "^0.3.4" }, "devDependencies": { "@tech-matters/testing": "^1.0.0", @@ -13631,6 +13632,11 @@ "node": ">=4.2.0" } }, + "node_modules/typescript-parsec": { + "version": "0.3.4", + "resolved": "https://registry.npmjs.org/typescript-parsec/-/typescript-parsec-0.3.4.tgz", + "integrity": "sha512-6RD4xOxp26BTZLopNbqT2iErqNhQZZWb5m5F07/UwGhldGvOAKOl41pZ3fxsFp04bNL+PbgMjNfb6IvJAC/uYQ==" + }, "node_modules/umzug": { "version": "3.3.1", "resolved": "https://registry.npmjs.org/umzug/-/umzug-3.3.1.tgz", @@ -17341,6 +17347,7 @@ "twilio": "^3.58.0", "twilio-flex-token-validator": "^1.5.5", "typescript": "^4.6.3", + "typescript-parsec": "*", "umzug": "^3.0.0", "util": "^0.12.2" }, @@ -24933,6 +24940,11 @@ "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.9.5.tgz", "integrity": "sha512-1FXk9E2Hm+QzZQ7z+McJiHL4NW1F2EzMu9Nq9i3zAaGqibafqYwCVU6WyWAuyQRRzOlxou8xZSyXLEN8oKj24g==" }, + "typescript-parsec": { + "version": "0.3.4", + "resolved": "https://registry.npmjs.org/typescript-parsec/-/typescript-parsec-0.3.4.tgz", + "integrity": "sha512-6RD4xOxp26BTZLopNbqT2iErqNhQZZWb5m5F07/UwGhldGvOAKOl41pZ3fxsFp04bNL+PbgMjNfb6IvJAC/uYQ==" + }, "umzug": { "version": "3.3.1", "resolved": "https://registry.npmjs.org/umzug/-/umzug-3.3.1.tgz", From 96c36812d2d6e5684df0793e82a91facf75503d7 Mon Sep 17 00:00:00 2001 From: Gianfranco Paoloni Date: Fri, 15 Dec 2023 02:02:30 -0300 Subject: [PATCH 5/9] refactor: Added permissions predicates parser module, parse predicates on initialization --- .../src/permissions/parser/parser.ts | 178 ++++++++++++++++++ .../src/permissions/parser/tokenizer.ts | 52 +++++ .../hrm-service/src/permissions/rulesMap.ts | 28 +-- .../src/permissions/setupCanForRules.ts | 96 ++-------- 4 files changed, 259 insertions(+), 95 deletions(-) create mode 100644 hrm-domain/hrm-service/src/permissions/parser/parser.ts create mode 100644 hrm-domain/hrm-service/src/permissions/parser/tokenizer.ts diff --git a/hrm-domain/hrm-service/src/permissions/parser/parser.ts b/hrm-domain/hrm-service/src/permissions/parser/parser.ts new file mode 100644 index 000000000..24fa6d551 --- /dev/null +++ b/hrm-domain/hrm-service/src/permissions/parser/parser.ts @@ -0,0 +1,178 @@ +/** + * 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/. + */ + +import * as parsec from 'typescript-parsec'; +import { TokenKind, lexer } from './tokenizer'; +import differenceInHours from 'date-fns/differenceInHours'; +import differenceInDays from 'date-fns/differenceInDays'; +import parseISO from 'date-fns/parseISO'; +import { isCaseOpen, isContactOwner, isCounselorWhoCreated } from '../helpers'; +import type { TargetKind } from '../actions'; +import type { TwilioUser } from '@tech-matters/twilio-worker-auth'; +import { TKConditionsSet, TKConditionsSets } from '../rulesMap'; + +/***************************************************************** + * Predicates (apply) + * + * Since we only need to convert each predicate function of type (performer: TwilioUser, target: any) => boolean + * We don't need the apply to be turned into complicated data structures. This could change if we ever need a real AST + ****************************************************************/ +type PredicateContext = { + curentTimestamp: Date; +}; +type PredicateEvaluator = ( + performer: TwilioUser, + target: any, + ctx: PredicateContext, +) => boolean; + +const applyIsSupervisor = + (_value: parsec.Token): PredicateEvaluator => + user => + user.isSupervisor; + +const applyEveryone = + (_value: parsec.Token): PredicateEvaluator => + () => + true; + +const applyCreatedHoursAgo = + (value: [parsec.Token, number]): PredicateEvaluator => + (_user, target, ctx) => + differenceInHours(ctx.curentTimestamp, parseISO(target.createdAt)) < value[1]; + +const applyCreatedDaysAgo = + (value: [parsec.Token, number]): PredicateEvaluator => + (_user, target, ctx) => + differenceInDays(ctx.curentTimestamp, parseISO(target.createdAt)) < value[1]; + +const applyIsOwner = + (_value: parsec.Token): PredicateEvaluator => + (user, target) => + isContactOwner(user, target); + +const applyIsCreator = + (_value: parsec.Token): PredicateEvaluator => + (user, target) => + isCounselorWhoCreated(user, target); + +const applyIsCaseOpen = + (_value: parsec.Token): PredicateEvaluator => + (user, target) => + isCaseOpen(target); + +/***************************************************************** + * Parsers + ****************************************************************/ + +const parseNumber = parsec.apply(parsec.tok(TokenKind.Number), v => +v.text); +const parseLParen = parsec.tok(TokenKind.LParen); +const parseRParen = parsec.tok(TokenKind.RParen); + +const parseIsSupervisorPredicate = parsec.apply( + parsec.tok(TokenKind.IsSupervisor), + applyIsSupervisor, +); +const parseEveryonePredicate = parsec.apply( + parsec.tok(TokenKind.Everyone), + applyEveryone, +); +const parseCreatedHoursAgoPredicate = parsec.apply( + parsec.seq( + parsec.tok(TokenKind.CreatedHoursAgo), + parsec.kmid(parseLParen, parseNumber, parseRParen), + ), + applyCreatedHoursAgo, +); +const parseCreatedDaysAgoPredicate = parsec.apply( + parsec.seq( + parsec.tok(TokenKind.CreatedDaysAgo), + parsec.kmid(parseLParen, parseNumber, parseRParen), + ), + applyCreatedDaysAgo, +); +const parseIsOwnerPredicate = parsec.apply(parsec.tok(TokenKind.IsOwner), applyIsOwner); +const parseIsCreatorPredicate = parsec.apply( + parsec.tok(TokenKind.IsCreator), + applyIsCreator, +); +const parseIsCaseOpenPredicate = parsec.apply( + parsec.tok(TokenKind.IsCaseOpen), + applyIsCaseOpen, +); + +/***************************************************************** + * Syntax + ****************************************************************/ + +const COMMON_PREDICATE = parsec.rule(); +COMMON_PREDICATE.setPattern( + parsec.alt( + parseIsSupervisorPredicate, + parseEveryonePredicate, + parseCreatedHoursAgoPredicate, + parseCreatedDaysAgoPredicate, + ), +); + +const CONTACT_PREDICATE = parsec.rule(); +CONTACT_PREDICATE.setPattern(parsec.alt(COMMON_PREDICATE, parseIsOwnerPredicate)); + +const CASE_PREDICATE = parsec.rule(); +CASE_PREDICATE.setPattern( + parsec.alt(COMMON_PREDICATE, parseIsCreatorPredicate, parseIsCaseOpenPredicate), +); + +const POSTSURVEY_PREDICATE = COMMON_PREDICATE; + +export const parseTKPredicate = (kind: TargetKind) => (input: string) => { + try { + const tokenized = lexer.parse(input); + switch (kind) { + case 'contact': { + return parsec.expectSingleResult( + parsec.expectEOF(CONTACT_PREDICATE.parse(tokenized)), + ); + } + case 'case': { + return parsec.expectSingleResult( + parsec.expectEOF(CASE_PREDICATE.parse(tokenized)), + ); + } + case 'postSurvey': { + return parsec.expectSingleResult( + parsec.expectEOF(POSTSURVEY_PREDICATE.parse(tokenized)), + ); + } + default: { + throw new Error(`Invalid TargetKind provided: ${kind}`); + } + } + } catch (err) { + console.error(`Error at parseTKPredicate with kind: ${kind} and input: ${input}`); + throw err; + } +}; + +const parseTKPredicateSet = + (kind: T) => + (conditionsSet: TKConditionsSet) => + conditionsSet.map(parseTKPredicate(kind)); + +export const parseConditionsSets = + (kind: T) => + (conditionsSets: TKConditionsSets) => + conditionsSets.map(parseTKPredicateSet(kind)); diff --git a/hrm-domain/hrm-service/src/permissions/parser/tokenizer.ts b/hrm-domain/hrm-service/src/permissions/parser/tokenizer.ts new file mode 100644 index 000000000..ca604982b --- /dev/null +++ b/hrm-domain/hrm-service/src/permissions/parser/tokenizer.ts @@ -0,0 +1,52 @@ +/** + * 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/. + */ + +import { buildLexer } from 'typescript-parsec'; + +export enum TokenKind { + Number, + LParen, + RParen, + + // user based predicate tokens + IsSupervisor, + Everyone, + + // time based predicate tokens + CreatedHoursAgo, + CreatedDaysAgo, + + // contact specific predicate tokens + IsOwner, + + // case specific predicate tokens + IsCreator, + IsCaseOpen, +} + +export const lexer = buildLexer([ + [true, /^\d+(\.\d+)?/g, TokenKind.Number], + [true, /^\(/g, TokenKind.LParen], + [true, /^\)/g, TokenKind.RParen], + + [true, /^isSupervisor/g, TokenKind.IsSupervisor], + [true, /^everyone/g, TokenKind.Everyone], + [true, /^createdHoursAgo/g, TokenKind.CreatedHoursAgo], + [true, /^createdDaysAgo/g, TokenKind.CreatedDaysAgo], + [true, /^isOwner/g, TokenKind.IsOwner], + [true, /^isCreator/g, TokenKind.IsCreator], + [true, /^isCaseOpen/g, TokenKind.IsCaseOpen], +]); diff --git a/hrm-domain/hrm-service/src/permissions/rulesMap.ts b/hrm-domain/hrm-service/src/permissions/rulesMap.ts index f84099164..d29de0e5b 100644 --- a/hrm-domain/hrm-service/src/permissions/rulesMap.ts +++ b/hrm-domain/hrm-service/src/permissions/rulesMap.ts @@ -42,37 +42,39 @@ const zwRules = require('../../permission-rules/zw.json'); import { actionsMaps, Actions, TargetKind, isTargetKind } from './actions'; -const timeBasedConditions = { - CREATED_HOURS_AGO: 'createdHoursAgo', - CREATED_DAYS_AGO: 'createdDaysAgo', -} as const; const userBasedConditions = { - IS_SUPERVISOR: 'isSupervisor', - EVERYONE: 'everyone', + IsSupervisor: 'isSupervisor', + Everyone: 'everyone', +} as const; +const timeBasedConditions = { + CreatedHoursAgo: 'createdHoursAgo', + CreatedDaysAgo: 'createdDaysAgo', } as const; const contactSpecificConditions = { - IS_OWNER: 'isOwner', + IsOwner: 'isOwner', } as const; const caseSpecificConditions = { - IS_CREATOR: 'isCreator', - IS_CASE_OPEN: 'isCaseOpen', + IsCreator: 'isCreator', + IsCaseOpen: 'isCaseOpen', } as const; -export const supportedContactConditionsMap = { +const supportedContactConditionsMap = { ...timeBasedConditions, ...userBasedConditions, ...contactSpecificConditions, } as const; const supportedContactConditions = Object.values(supportedContactConditionsMap); -export const supportedCaseConditionsMap = { +const supportedCaseConditionsMap = { ...timeBasedConditions, ...userBasedConditions, ...caseSpecificConditions, }; -const supportedCaseConditions = Object.values(supportedCaseConditionsMap); +export const supportedCaseConditions = Object.values(supportedCaseConditionsMap); -const supportedPostSurveyConditions = [...Object.values(userBasedConditions)] as const; +export const supportedPostSurveyConditions = [ + ...Object.values(userBasedConditions), +] as const; // Defines which actions are supported on each TargetKind const supportedTKConditions = { diff --git a/hrm-domain/hrm-service/src/permissions/setupCanForRules.ts b/hrm-domain/hrm-service/src/permissions/setupCanForRules.ts index c1b0a7008..95add8f23 100644 --- a/hrm-domain/hrm-service/src/permissions/setupCanForRules.ts +++ b/hrm-domain/hrm-service/src/permissions/setupCanForRules.ts @@ -14,44 +14,10 @@ * along with this program. If not, see https://www.gnu.org/licenses/. */ -import { isCounselorWhoCreated, isCaseOpen, isContactOwner } from './helpers'; -import { actionsMaps, Actions, isTargetKind, TargetKind } from './actions'; -import { - isValidTKConditionsSets, - type TKCondition, - type TKConditionsSet, - type TKConditionsSets, - type RulesFile, -} from './rulesMap'; -import { TwilioUser } from '@tech-matters/twilio-worker-auth'; - -type ConditionsState = { - [condition in TKCondition]: boolean; -}; - -/** - * Given a conditionsState and a condition, returns true if the condition is true in the conditionsState - */ -const checkCondition = - (conditionsState: ConditionsState) => - (condition: TKCondition): boolean => - conditionsState[condition]; - -/** - * Given a conditionsState and a set of conditions, returns true if all the conditions are true in the conditionsState - */ -const checkConditionsSet = - (conditionsState: ConditionsState) => - (conditionsSet: TKConditionsSet): boolean => - conditionsSet.length > 0 && conditionsSet.every(checkCondition(conditionsState)); - -/** - * Given a conditionsState and a set of conditions sets, returns true if one of the conditions sets contains conditions that are all true in the conditionsState - */ -const checkConditionsSets = ( - conditionsState: ConditionsState, - conditionsSets: TKConditionsSets, -): boolean => conditionsSets.some(checkConditionsSet(conditionsState)); +import { actionsMaps, Actions, isTargetKind, type TargetKind } from './actions'; +import { parseConditionsSets } from './parser/parser'; +import type { TKConditionsSets, RulesFile } from './rulesMap'; +import type { TwilioUser } from '@tech-matters/twilio-worker-auth'; // const setupAllow = (kind: T, conditionsSets: ConditionsSets) => { const setupAllow = ( @@ -59,50 +25,15 @@ const setupAllow = ( conditionsSets: TKConditionsSets, ) => { // We could do type validation on target depending on targetKind if we ever want to make sure the "allow" is called on a proper target (same as cancan used to do) + const parsedConditionsSets = parseConditionsSets(kind)(conditionsSets); return (performer: TwilioUser, target: any) => { - // Build the proper conditionsState depending on the targetKind - if (kind === 'case') { - if (!isValidTKConditionsSets<'case'>(kind)(conditionsSets)) { - throw new Error(`setupAllow: Invalid conditionsSets provided for kind ${kind}`); - } - - const conditionsState: ConditionsState<'case'> = { - isSupervisor: performer.isSupervisor, - isCreator: isCounselorWhoCreated(performer, target), - isCaseOpen: isCaseOpen(target), - everyone: true, - createdDaysAgo: false, - createdHoursAgo: false, - }; - - return checkConditionsSets(conditionsState, conditionsSets); - } else if (kind === 'contact') { - if (!isValidTKConditionsSets<'contact'>(kind)(conditionsSets)) { - throw new Error(`setupAllow: Invalid conditionsSets provided for kind ${kind}`); - } - - const conditionsState: ConditionsState<'contact'> = { - isSupervisor: performer.isSupervisor, - isOwner: isContactOwner(performer, target), - everyone: true, - createdDaysAgo: false, - createdHoursAgo: false, - }; + const ctx = { curentTimestamp: new Date() }; - return checkConditionsSets(conditionsState, conditionsSets); - } else if (kind === 'postSurvey') { - if (!isValidTKConditionsSets<'postSurvey'>(kind)(conditionsSets)) { - throw new Error(`setupAllow: Invalid conditionsSets provided for kind ${kind}`); - } - - const conditionsState: ConditionsState<'postSurvey'> = { - isSupervisor: performer.isSupervisor, - everyone: true, - }; - - return checkConditionsSets(conditionsState, conditionsSets); - } + // If every condition is true for at least one set, the action is allowed + return parsedConditionsSets.some( + cs => cs.length && cs.every(c => c(performer, target, ctx)), + ); }; }; @@ -115,9 +46,10 @@ export const initializeCanForRules = (rules: RulesFile) => { throw new Error(`Invalid target kind ${targetKind} found in setupCanForRules`); const actionsForTK = Object.values(actionsMaps[targetKind]); - actionsForTK.forEach( - action => (actionCheckers[action] = setupAllow(targetKind, rules[action])), - ); + actionsForTK.forEach(action => { + // console.log('action', action, 'targetKind', targetKind, 'rules[action]', rules[action]) + actionCheckers[action] = setupAllow(targetKind, rules[action]); + }); }); return (performer: TwilioUser, action: Actions, target: any) => From c3bc6b46f51f4f765707d82080ac043d2924fd10 Mon Sep 17 00:00:00 2001 From: Gianfranco Paoloni Date: Fri, 15 Dec 2023 02:02:47 -0300 Subject: [PATCH 6/9] chore: tests --- .../permissions/setupCanForRules.test.ts | 42 +++++++++++-------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/hrm-domain/hrm-service/unit-tests/permissions/setupCanForRules.test.ts b/hrm-domain/hrm-service/unit-tests/permissions/setupCanForRules.test.ts index 9a4d783df..6fe948775 100644 --- a/hrm-domain/hrm-service/unit-tests/permissions/setupCanForRules.test.ts +++ b/hrm-domain/hrm-service/unit-tests/permissions/setupCanForRules.test.ts @@ -16,24 +16,32 @@ /* eslint-disable jest/no-standalone-expect */ import each from 'jest-each'; -import { setupCanForRules } from '../../src/permissions/setupCanForRules'; +import { initializeCanForRules } from '../../src/permissions/setupCanForRules'; import { actionsMaps } from '../../src/permissions'; import { RulesFile } from '../../src/permissions/rulesMap'; import { workerSid, accountSid } from '../../service-tests/mocks'; import { twilioUser } from '@tech-matters/twilio-worker-auth'; +import { TargetKind } from '../../src/permissions/actions'; const helpline = 'helpline'; -const buildRules = (conditionsSets): RulesFile => { - const entries = Object.values(actionsMaps) - .flatMap(e => Object.values(e)) - .map(action => [action, conditionsSets]); +const buildRules = ( + partialRules: { + [K in TargetKind]?: string[][]; + } & { default?: string[][] }, +): RulesFile => { + const entries = Object.entries(actionsMaps) + .flatMap(([tk, obj]) => Object.values(obj).map(action => [tk, action])) + .map(([tk, action]) => { + return [action, partialRules[tk] || partialRules.default || []]; + }); + return Object.fromEntries(entries); }; describe('Test that all actions work fine (everyone)', () => { - const rules = buildRules([['everyone']]); - const can = setupCanForRules(rules); + const rules = buildRules({ default: [['everyone']] }); + const can = initializeCanForRules(rules); const notCreator = twilioUser('not creator', []); @@ -87,8 +95,8 @@ describe('Test that all actions work fine (everyone)', () => { }); describe('Test that all actions work fine (no one)', () => { - const rules = buildRules([]); - const can = setupCanForRules(rules); + const rules = buildRules({ default: [] }); + const can = initializeCanForRules(rules); const supervisor = twilioUser('creator', ['supervisor']); @@ -150,8 +158,8 @@ describe('Test that all actions work fine (no one)', () => { * The reason is how checkConditionsSet is implemented: [].every(predicate) evaluates true for all predicates */ describe('Test that an empty set of conditions does not grants permissions', () => { - const rules = buildRules([[]]); - const can = setupCanForRules(rules); + const rules = buildRules({ default: [[]] }); + const can = initializeCanForRules(rules); const supervisor = twilioUser('creator', ['supervisor']); @@ -347,8 +355,8 @@ describe('Test different scenarios (Case)', () => { ).describe( 'Expect $expectedResult when $expectedDescription with $prettyConditionsSets', ({ conditionsSets, caseObj, user, expectedResult }) => { - const rules = buildRules(conditionsSets); - const can = setupCanForRules(rules); + const rules = buildRules({ case: conditionsSets }); + const can = initializeCanForRules(rules); Object.values(actionsMaps.case).forEach(action => test(`${action}`, async () => { @@ -427,8 +435,8 @@ describe('Test different scenarios (Contact)', () => { ).describe( 'Expect $expectedResult when $expectedDescription with $prettyConditionsSets', ({ conditionsSets, contactObj, user, expectedResult }) => { - const rules = buildRules(conditionsSets); - const can = setupCanForRules(rules); + const rules = buildRules({ contact: conditionsSets }); + const can = initializeCanForRules(rules); Object.values(actionsMaps.contact).forEach(action => test(`${action}`, async () => { @@ -495,8 +503,8 @@ describe('Test different scenarios (PostSurvey)', () => { ).describe( 'Expect $expectedResult when $expectedDescription with $prettyConditionsSets', ({ conditionsSets, postSurveyObj, user, expectedResult }) => { - const rules = buildRules(conditionsSets); - const can = setupCanForRules(rules); + const rules = buildRules({ postSurvey: conditionsSets }); + const can = initializeCanForRules(rules); Object.values(actionsMaps.postSurvey).forEach(action => test(`${action}`, async () => { From 44185f6974a4213d2d79d391fe5d771b94f8edbc Mon Sep 17 00:00:00 2001 From: Gianfranco Paoloni Date: Fri, 15 Dec 2023 02:11:12 -0300 Subject: [PATCH 7/9] refactor: renamed file --- .eslintrc.js | 2 +- hrm-domain/hrm-service/src/case/caseService.ts | 2 +- hrm-domain/hrm-service/src/contact/contactService.ts | 2 +- .../hrm-service/src/permissions/canPerformActionOnObject.ts | 2 +- hrm-domain/hrm-service/src/permissions/index.ts | 2 +- .../{setupCanForRules.ts => initializeCanForRules.ts} | 2 +- hrm-domain/hrm-service/src/permissions/rulesMap.ts | 6 ++---- ...tupCanForRules.test.ts => initializeCanForRules.test.ts} | 2 +- 8 files changed, 9 insertions(+), 11 deletions(-) rename hrm-domain/hrm-service/src/permissions/{setupCanForRules.ts => initializeCanForRules.ts} (98%) rename hrm-domain/hrm-service/unit-tests/permissions/{setupCanForRules.test.ts => initializeCanForRules.test.ts} (99%) diff --git a/.eslintrc.js b/.eslintrc.js index bf27b14f1..a1b38ea2e 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -23,7 +23,7 @@ module.exports = { '@typescript-eslint/indent': 'off', '@typescript-eslint/quotes': 'off', 'import/prefer-default-export': 'off', - '@typescript-eslint/no-unused-vars': ['error', { argsIgnorePattern: '^_' }], + '@typescript-eslint/no-unused-vars': ['warn', { argsIgnorePattern: '^_' }], }, settings: { 'import/resolver': { diff --git a/hrm-domain/hrm-service/src/case/caseService.ts b/hrm-domain/hrm-service/src/case/caseService.ts index a9eaae329..8ff7f70ff 100644 --- a/hrm-domain/hrm-service/src/case/caseService.ts +++ b/hrm-domain/hrm-service/src/case/caseService.ts @@ -36,7 +36,7 @@ import { } from './case-data-access'; import { randomUUID } from 'crypto'; import type { Contact } from '../contact/contact-data-access'; -import { InitializedCan } from '../permissions/setupCanForRules'; +import type { InitializedCan } from '../permissions/initializeCanForRules'; import type { TwilioUser } from '@tech-matters/twilio-worker-auth'; import { bindApplyTransformations as bindApplyContactTransformations, diff --git a/hrm-domain/hrm-service/src/contact/contactService.ts b/hrm-domain/hrm-service/src/contact/contactService.ts index c70907211..8123482d8 100644 --- a/hrm-domain/hrm-service/src/contact/contactService.ts +++ b/hrm-domain/hrm-service/src/contact/contactService.ts @@ -32,7 +32,7 @@ import { retrieveCategories } from './categories'; import { PaginationQuery, getPaginationElements } from '../search'; import type { NewContactRecord } from './sql/contact-insert-sql'; import { ContactRawJson, ReferralWithoutContactId } from './contact-json'; -import { InitializedCan } from '../permissions/setupCanForRules'; +import type { InitializedCan } from '../permissions/initializeCanForRules'; import { actionsMaps } from '../permissions'; import type { TwilioUser } from '@tech-matters/twilio-worker-auth'; import { connectContactToCsamReports, CSAMReport } from '../csam-report/csam-report'; diff --git a/hrm-domain/hrm-service/src/permissions/canPerformActionOnObject.ts b/hrm-domain/hrm-service/src/permissions/canPerformActionOnObject.ts index a591d0bdd..eade50463 100644 --- a/hrm-domain/hrm-service/src/permissions/canPerformActionOnObject.ts +++ b/hrm-domain/hrm-service/src/permissions/canPerformActionOnObject.ts @@ -16,7 +16,6 @@ import { TwilioUser } from '@tech-matters/twilio-worker-auth'; import { Actions, TargetKind, isValidSetOfActionsForTarget } from './actions'; -import { InitializedCan } from './setupCanForRules'; import { getContactById } from '../contact/contactService'; import { getCase as getCaseById } from '../case/caseService'; import { assertExhaustive } from '../contact-job/assertExhaustive'; @@ -25,6 +24,7 @@ import { isS3StoredConversationMedia, } from '../conversation-media/conversation-media'; import { TResult, newErr, newOk } from '@tech-matters/types'; +import type { InitializedCan } from '../permissions/initializeCanForRules'; export const canPerformActionsOnObject = async ({ accountSid, diff --git a/hrm-domain/hrm-service/src/permissions/index.ts b/hrm-domain/hrm-service/src/permissions/index.ts index 414b6759a..7bc06fe6c 100644 --- a/hrm-domain/hrm-service/src/permissions/index.ts +++ b/hrm-domain/hrm-service/src/permissions/index.ts @@ -20,7 +20,7 @@ export { SafeRouter, publicEndpoint } from './safe-router'; export { rulesMap } from './rulesMap'; export { Actions, actionsMaps, getActions } from './actions'; -import { InitializedCan, initializeCanForRules } from './setupCanForRules'; +import { InitializedCan, initializeCanForRules } from './initializeCanForRules'; import { RulesFile } from './rulesMap'; import type { Request, Response, NextFunction } from 'express'; import { getSearchPermissions, SearchPermissions } from './search-permissions'; diff --git a/hrm-domain/hrm-service/src/permissions/setupCanForRules.ts b/hrm-domain/hrm-service/src/permissions/initializeCanForRules.ts similarity index 98% rename from hrm-domain/hrm-service/src/permissions/setupCanForRules.ts rename to hrm-domain/hrm-service/src/permissions/initializeCanForRules.ts index 95add8f23..8d38520e1 100644 --- a/hrm-domain/hrm-service/src/permissions/setupCanForRules.ts +++ b/hrm-domain/hrm-service/src/permissions/initializeCanForRules.ts @@ -43,7 +43,7 @@ export const initializeCanForRules = (rules: RulesFile) => { const targetKinds = Object.keys(actionsMaps); targetKinds.forEach((targetKind: string) => { if (!isTargetKind(targetKind)) - throw new Error(`Invalid target kind ${targetKind} found in setupCanForRules`); + throw new Error(`Invalid target kind ${targetKind} found in initializeCanForRules`); const actionsForTK = Object.values(actionsMaps[targetKind]); actionsForTK.forEach(action => { diff --git a/hrm-domain/hrm-service/src/permissions/rulesMap.ts b/hrm-domain/hrm-service/src/permissions/rulesMap.ts index d29de0e5b..05f890416 100644 --- a/hrm-domain/hrm-service/src/permissions/rulesMap.ts +++ b/hrm-domain/hrm-service/src/permissions/rulesMap.ts @@ -70,11 +70,9 @@ const supportedCaseConditionsMap = { ...userBasedConditions, ...caseSpecificConditions, }; -export const supportedCaseConditions = Object.values(supportedCaseConditionsMap); +const supportedCaseConditions = Object.values(supportedCaseConditionsMap); -export const supportedPostSurveyConditions = [ - ...Object.values(userBasedConditions), -] as const; +const supportedPostSurveyConditions = [...Object.values(userBasedConditions)] as const; // Defines which actions are supported on each TargetKind const supportedTKConditions = { diff --git a/hrm-domain/hrm-service/unit-tests/permissions/setupCanForRules.test.ts b/hrm-domain/hrm-service/unit-tests/permissions/initializeCanForRules.test.ts similarity index 99% rename from hrm-domain/hrm-service/unit-tests/permissions/setupCanForRules.test.ts rename to hrm-domain/hrm-service/unit-tests/permissions/initializeCanForRules.test.ts index 6fe948775..085ac87d5 100644 --- a/hrm-domain/hrm-service/unit-tests/permissions/setupCanForRules.test.ts +++ b/hrm-domain/hrm-service/unit-tests/permissions/initializeCanForRules.test.ts @@ -16,7 +16,7 @@ /* eslint-disable jest/no-standalone-expect */ import each from 'jest-each'; -import { initializeCanForRules } from '../../src/permissions/setupCanForRules'; +import { initializeCanForRules } from '../../src/permissions/initializeCanForRules'; import { actionsMaps } from '../../src/permissions'; import { RulesFile } from '../../src/permissions/rulesMap'; import { workerSid, accountSid } from '../../service-tests/mocks'; From ecd4cec664e05d914a66b6fbeb34bad62dce58b4 Mon Sep 17 00:00:00 2001 From: Gianfranco Paoloni Date: Fri, 15 Dec 2023 18:53:59 -0300 Subject: [PATCH 8/9] refactor: test if rules file is valid by parsing it --- .../src/permissions/parser/parser.ts | 2 +- .../hrm-service/src/permissions/rulesMap.ts | 50 ++++++------------- 2 files changed, 15 insertions(+), 37 deletions(-) diff --git a/hrm-domain/hrm-service/src/permissions/parser/parser.ts b/hrm-domain/hrm-service/src/permissions/parser/parser.ts index 24fa6d551..10e2af85a 100644 --- a/hrm-domain/hrm-service/src/permissions/parser/parser.ts +++ b/hrm-domain/hrm-service/src/permissions/parser/parser.ts @@ -138,7 +138,7 @@ CASE_PREDICATE.setPattern( const POSTSURVEY_PREDICATE = COMMON_PREDICATE; -export const parseTKPredicate = (kind: TargetKind) => (input: string) => { +const parseTKPredicate = (kind: TargetKind) => (input: string) => { try { const tokenized = lexer.parse(input); switch (kind) { diff --git a/hrm-domain/hrm-service/src/permissions/rulesMap.ts b/hrm-domain/hrm-service/src/permissions/rulesMap.ts index 05f890416..9e8fd662f 100644 --- a/hrm-domain/hrm-service/src/permissions/rulesMap.ts +++ b/hrm-domain/hrm-service/src/permissions/rulesMap.ts @@ -40,7 +40,8 @@ const zaRules = require('../../permission-rules/za.json'); const zmRules = require('../../permission-rules/zm.json'); const zwRules = require('../../permission-rules/zw.json'); -import { actionsMaps, Actions, TargetKind, isTargetKind } from './actions'; +import { actionsMaps, Actions, TargetKind } from './actions'; +import { parseConditionsSets } from './parser/parser'; const userBasedConditions = { IsSupervisor: 'isSupervisor', @@ -85,44 +86,27 @@ export type TKCondition = (typeof supportedTKConditions)[T export type TKConditionsSet = TKCondition[]; export type TKConditionsSets = TKConditionsSet[]; -const isTKCondition = - (kind: T) => - (c: any): c is TKCondition => - c && supportedTKConditions[kind].includes(c); - -const isTKConditionsSet = - (kind: TargetKind) => - (cs: any): cs is TKConditionsSet => - cs && Array.isArray(cs) && cs.every(isTKCondition(kind)); - -const isTKConditionsSets = - (kind: TargetKind) => - (css: any): css is TKConditionsSets => - css && Array.isArray(css) && css.every(isTKConditionsSet(kind)); - export type RulesFile = { [k in Actions]: TKConditionsSets }; -export const isValidTKConditionsSets = - (kind: T) => - (css: TKConditionsSets): css is TKConditionsSets => - css.every(cs => cs.every(isTKCondition(kind))); - -export const isRulesFile = (rules: any): rules is RulesFile => - Object.values(actionsMaps).every(map => - Object.values(map).every(action => isTKConditionsSets(rules[action])), - ); - /** * Validates that for every TK, the ConditionsSets provided are valid - * (i.e. present in supportedTKConditions) + * (i.e. that the all the predicates are properly parsed) */ const validateTKActions = (rules: RulesFile) => Object.entries(actionsMaps) .map(([kind, map]) => Object.values(map).reduce((accum, action) => { + let result: boolean; + try { + parseConditionsSets(kind as TargetKind)(rules[action]); + result = true; + } catch (err) { + result = false; + } + return { ...accum, - [action]: isTargetKind(kind) && isValidTKConditionsSets(kind)(rules[action]), + [action]: result, }; }, {}), ) @@ -131,9 +115,6 @@ const validateTKActions = (rules: RulesFile) => {} as any, ); -const isValidTargetKindActions = (validated: { [k in Actions]: boolean }) => - Object.values(validated).every(Boolean); - const rulesMapDef = { br: brRules, ca: caRules, @@ -170,12 +151,9 @@ export const validRulesMap = () => // This type assertion is legit as long as we check that every entry in rulesMapDef is indeed a RulesFile Object.entries(rulesMapDef).reduce<{ [k in keyof typeof rulesMapDef]: RulesFile }>( (accum, [k, rules]) => { - if (!isRulesFile(rules)) { - throw new Error(`Error: rules file for ${k} is not a valid RulesFile`); - } - const validated = validateTKActions(rules); - if (!isValidTargetKindActions(validated)) { + + if (!Object.values(validated).every(Boolean)) { const invalidActions = Object.entries(validated) .filter(([, val]) => !val) .map(([key]) => key); From 2954f323cdd03cba9080f3cbe1dec0dbc7fb078c Mon Sep 17 00:00:00 2001 From: Gianfranco Paoloni Date: Fri, 15 Dec 2023 18:54:08 -0300 Subject: [PATCH 9/9] chore: Added unit tests --- .../permissions/initializeCanForRules.test.ts | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/hrm-domain/hrm-service/unit-tests/permissions/initializeCanForRules.test.ts b/hrm-domain/hrm-service/unit-tests/permissions/initializeCanForRules.test.ts index 085ac87d5..b8e287f00 100644 --- a/hrm-domain/hrm-service/unit-tests/permissions/initializeCanForRules.test.ts +++ b/hrm-domain/hrm-service/unit-tests/permissions/initializeCanForRules.test.ts @@ -22,6 +22,7 @@ import { RulesFile } from '../../src/permissions/rulesMap'; import { workerSid, accountSid } from '../../service-tests/mocks'; import { twilioUser } from '@tech-matters/twilio-worker-auth'; import { TargetKind } from '../../src/permissions/actions'; +import { subDays, subHours } from 'date-fns'; const helpline = 'helpline'; @@ -350,6 +351,66 @@ describe('Test different scenarios (Case)', () => { }, user: twilioUser('not creator', []), }, + { + conditionsSets: [['createdHoursAgo(2)']], + expectedResult: true, + expectedDescription: 'createdHoursAgo within the provided range', + caseObj: { + status: 'open', + info: {}, + twilioWorkerId: 'creator', + helpline, + createdBy: workerSid, + accountSid, + createdAt: subHours(Date.now(), 1).toISOString(), + }, + user: twilioUser('not creator', []), + }, + { + conditionsSets: [['createdHoursAgo(1)']], + expectedResult: false, + expectedDescription: 'createdHoursAgo outside the provided range', + caseObj: { + status: 'open', + info: {}, + twilioWorkerId: 'creator', + helpline, + createdBy: workerSid, + accountSid, + createdAt: subHours(Date.now(), 1).toISOString(), + }, + user: twilioUser('not creator', []), + }, + { + conditionsSets: [['createdDaysAgo(2)']], + expectedResult: true, + expectedDescription: 'createdDaysAgo within the provided range', + caseObj: { + status: 'open', + info: {}, + twilioWorkerId: 'creator', + helpline, + createdBy: workerSid, + accountSid, + createdAt: subDays(Date.now(), 1).toISOString(), + }, + user: twilioUser('not creator', []), + }, + { + conditionsSets: [['createdDaysAgo(1)']], + expectedResult: false, + expectedDescription: 'createdDaysAgo outside the provided range', + caseObj: { + status: 'open', + info: {}, + twilioWorkerId: 'creator', + helpline, + createdBy: workerSid, + accountSid, + createdAt: subDays(Date.now(), 1).toISOString(), + }, + user: twilioUser('not creator', []), + }, ].map(addPrettyConditionsSets), // .flatMap(mapTestToActions(actionsMaps.case)), ).describe( @@ -431,6 +492,50 @@ describe('Test different scenarios (Contact)', () => { }, user: twilioUser('not creator', []), }, + { + conditionsSets: [['createdHoursAgo(2)']], + expectedResult: true, + expectedDescription: 'createdHoursAgo within the provided range', + contactObj: { + accountSid, + twilioWorkerId: 'creator', + createdAt: subHours(Date.now(), 1).toISOString(), + }, + user: twilioUser('not creator', []), + }, + { + conditionsSets: [['createdHoursAgo(1)']], + expectedResult: false, + expectedDescription: 'createdHoursAgo outside the provided range', + contactObj: { + accountSid, + twilioWorkerId: 'creator', + createdAt: subHours(Date.now(), 1).toISOString(), + }, + user: twilioUser('not creator', []), + }, + { + conditionsSets: [['createdDaysAgo(2)']], + expectedResult: true, + expectedDescription: 'createdDaysAgo within the provided range', + contactObj: { + accountSid, + twilioWorkerId: 'creator', + createdAt: subDays(Date.now(), 1).toISOString(), + }, + user: twilioUser('not creator', []), + }, + { + conditionsSets: [['createdDaysAgo(1)']], + expectedResult: false, + expectedDescription: 'createdDaysAgo outside the provided range', + contactObj: { + accountSid, + twilioWorkerId: 'creator', + createdAt: subDays(Date.now(), 1).toISOString(), + }, + user: twilioUser('not creator', []), + }, ].map(addPrettyConditionsSets), ).describe( 'Expect $expectedResult when $expectedDescription with $prettyConditionsSets',