Skip to content

Commit

Permalink
Merge pull request #797 from techmatters/CHI-3069-contact_create_stat…
Browse files Browse the repository at this point in the history
…ic_key

CHI-3069: Allow contacts to be created with a static key
  • Loading branch information
stephenhand authored Dec 19, 2024
2 parents e6f75e9 + 2525487 commit 8b3721a
Show file tree
Hide file tree
Showing 15 changed files with 91 additions and 54 deletions.
18 changes: 12 additions & 6 deletions hrm-domain/hrm-core/contact/contactRoutesV0.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,18 @@ const contactsRouter = SafeRouter();
*
* @returns {Contact} - Created contact
*/
contactsRouter.post('/', publicEndpoint, async (req, res) => {
const { hrmAccountId, user } = req;
const contact = await createContact(hrmAccountId, user.workerSid, req.body, {
can: req.can,
user,
});
contactsRouter.post('/', publicEndpoint, async (req: Request, res) => {
const { hrmAccountId, user, body } = req;
const contact = await createContact(
hrmAccountId,
// Take the createdBy specified in the body if this is being created from a backend system, otherwise force use of the authenticated user's workerSid
user.isSystemUser ? body.createdBy : user.workerSid,
body,
{
can: req.can,
user,
},
);
res.json(contact);
});

Expand Down
4 changes: 2 additions & 2 deletions hrm-domain/hrm-core/contact/contactService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ import {
DatabaseErrorResult,
isDatabaseUniqueConstraintViolationErrorResult,
} from '../sql';
import { systemUser } from '@tech-matters/twilio-worker-auth';
import { newGlobalSystemUser } from '@tech-matters/twilio-worker-auth';
import { publishContactChangeNotification } from '../notifications/entityChangeNotify';
import type { RulesFile, TKConditionsSets } from '../permissions/rulesMap';
import {
Expand Down Expand Up @@ -165,7 +165,7 @@ const initProfile = async (
const profileResult = await getOrCreateProfileWithIdentifier(conn)(
hrmAccountId,
{ identifier: { identifier: contact.number }, profile: { name: null } },
{ user: { accountSid, isSupervisor: false, roles: [], workerSid: systemUser } }, // fake the worker since makes more sense to keep the new "profile created by system"
{ user: newGlobalSystemUser(accountSid) }, // system user since makes more sense to keep the new "profile created by system"
);

if (isErr(profileResult)) {
Expand Down
1 change: 1 addition & 0 deletions hrm-domain/hrm-core/permissions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ export const maxPermissions: {
workerSid: 'WKxxx',
roles: ['supervisor'],
isSupervisor: true,
isSystemUser: false,
},
permissions: rulesMap.open,
};
2 changes: 2 additions & 0 deletions hrm-domain/hrm-core/permissions/initializeCanForRules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ const setupAllow = <T extends TargetKind>(
);

return (performer: TwilioUser, target: any) => {
// Let system users do anything for now, might need to tighten this up in future
if (performer.isSystemUser) return true;
const ctx = { curentTimestamp: new Date() };

const appliedTimeBasedConditions = applyTimeBasedConditions(timeBasedConditions)(
Expand Down
4 changes: 2 additions & 2 deletions hrm-domain/hrm-core/unit-tests/case/caseDataAccess.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import each from 'jest-each';
import { db } from '../../connection-pool';
import { OrderByColumn, OrderByColumnType } from '../../case/sql/caseSearchSql';
import { expectValuesInSql, getSqlStatement } from '@tech-matters/testing';
import { TwilioUser } from '@tech-matters/twilio-worker-auth';
import { newTwilioUser, TwilioUser } from '@tech-matters/twilio-worker-auth';
import { AccountSID } from '@tech-matters/types';
import { rulesMap } from '../../permissions';
import { TKConditionsSets } from '../../permissions/rulesMap';
Expand All @@ -31,7 +31,7 @@ import { pick } from 'lodash';

const accountSid: AccountSID = 'ACCOUNT_SID';
const workerSid = 'WK-twilio-worker-id';
const user: TwilioUser = { accountSid, workerSid, isSupervisor: true, roles: [] };
const user: TwilioUser = newTwilioUser('ACxx', 'WKxx', ['supervisor']);
let conn: pgPromise.ITask<unknown>;
const caseId = 42;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@

import { parseISO, subHours } from 'date-fns';
import { ContactBuilder } from './contact-builder';
import { getClient } from '@tech-matters/twilio-client';
import { isTwilioTaskTransferTarget } from '@tech-matters/twilio-client';
import { getClient, isTwilioTaskTransferTarget } from '@tech-matters/twilio-client';
import { getContactById, PatchPayload } from '../../contact/contactService';
import createError from 'http-errors';
import {
Expand All @@ -27,6 +26,7 @@ import {
import { CaseService, getCase } from '../../case/caseService';
import { actionsMaps } from '../../permissions';
import { SafeRouterRequest } from '../../permissions/safe-router';
import { newTwilioUser } from '@tech-matters/twilio-worker-auth';

jest.mock('@tech-matters/twilio-client', () => ({
getClient: jest.fn().mockResolvedValue({}),
Expand Down Expand Up @@ -71,12 +71,7 @@ beforeEach(() => {
req = {
...mockRequestMethods,
params: { contactId: 'contact1' },
user: {
workerSid: 'WK-worker1',
accountSid: 'ACtwilio',
isSupervisor: false,
roles: [],
},
user: newTwilioUser('ACtwilio', 'WK-worker1', []),
hrmAccountId: accountSid1,
body: {},
} as Partial<SafeRouterRequest>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,13 @@ import { mockConnection, mockTask, mockTransaction } from '../mock-db';
import { search, create } from '../../contact/contactDataAccess';
import { ContactBuilder } from './contact-builder';
import { NewContactRecord } from '../../contact/sql/contactInsertSql';
import { TwilioUser } from '@tech-matters/twilio-worker-auth';
import { newTwilioUser, TwilioUser } from '@tech-matters/twilio-worker-auth';
import { OPEN_CONTACT_ACTION_CONDITIONS } from '../mocks';
import { newOkFromData } from '@tech-matters/types';

let conn: pgPromise.ITask<unknown>;

const twilioUser: TwilioUser = {
accountSid: 'ACxx',
workerSid: 'WKxx',
isSupervisor: true,
roles: [],
};
const twilioUser: TwilioUser = newTwilioUser('ACxx', 'WKxx', ['supervisor']);

beforeEach(() => {
conn = mockConnection();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { canPerformActionsOnObject } from '../../permissions/canPerformActionOnO
import { actionsMaps } from '../../permissions';
import * as contactApi from '../../contact/contactService';
import * as caseApi from '../../case/caseService';
import { newTwilioUser } from '@tech-matters/twilio-worker-auth';

const accountSid: HrmAccountId = 'ACxxxxxx';

Expand Down Expand Up @@ -83,12 +84,7 @@ describe('canPerformActionsOnObject', () => {
actions: [action],
objectId: 123,
can: mockedCan ? mockedCan : () => shouldCan,
user: {
accountSid,
workerSid: 'WK-workerSid',
isSupervisor: false,
roles: ['supervisor'],
},
user: newTwilioUser(accountSid, 'WK-workerSid', []),
});

if (shouldAccessDB) {
Expand Down
2 changes: 1 addition & 1 deletion hrm-domain/hrm-service/service-tests/case.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ describe('/cases route', () => {
const fromDb = await caseDb.getById(
cases.blank.id,
accountSid,
{ accountSid, workerSid, isSupervisor: true, roles: [] },
newTwilioUser(accountSid, workerSid, ['supervisor']),
[['everyone']],
);
expect(fromDb).toBeFalsy();
Expand Down
2 changes: 1 addition & 1 deletion hrm-domain/hrm-service/service-tests/contacts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ describe('/contacts route', () => {
identifier: { identifier: contact.number },
profile: { name: null },
},
{ user: { accountSid, isSupervisor: false, roles: [], workerSid } },
{ user: newTwilioUser(accountSid, workerSid, []) },
);

if (isErr(profileResult)) {
Expand Down
10 changes: 5 additions & 5 deletions hrm-domain/hrm-service/service-tests/profiles.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,14 @@ describe('/profiles', () => {
murray!.id,
defaultFlags[0].id,
null,
{ user: { accountSid, workerSid, isSupervisor: false, roles: [] } },
{ user: newTwilioUser(accountSid, workerSid, []) },
),
profilesDB.associateProfileToProfileFlag()(
accountSid,
antonella!.id,
defaultFlags[1].id,
null,
{ user: { accountSid, workerSid, isSupervisor: false, roles: [] } },
{ user: newTwilioUser(accountSid, workerSid, []) },
),
profilesDB.createProfileSection()(antonella!.accountSid, {
content: 'some example content',
Expand Down Expand Up @@ -231,7 +231,7 @@ describe('/profiles', () => {
getOrCreateProfileWithIdentifier(t)(
acc,
{ identifier: { identifier }, profile: { name: null } },
{ user: { accountSid: acc, isSupervisor: false, roles: [], workerSid } },
{ user: newTwilioUser(accountSid, workerSid, []) },
),
),
),
Expand Down Expand Up @@ -269,7 +269,7 @@ describe('/profiles', () => {
const result = await getOrCreateProfileWithIdentifier(t)(
accountSid,
{ identifier: { identifier }, profile: { name: null } },
{ user: { accountSid, isSupervisor: false, roles: [], workerSid } },
{ user: newTwilioUser(accountSid, workerSid, []) },
);
return result.unwrap().identifier;
});
Expand Down Expand Up @@ -546,7 +546,7 @@ describe('/profiles', () => {
createdProfile.profiles[0].id,
defaultFlags[0].id,
null,
{ user: { accountSid, workerSid, isSupervisor: false, roles: [] } },
{ user: newTwilioUser(accountSid, workerSid, []) },
);

const pfs = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { addDays, subDays } from 'date-fns';
import { db } from '@tech-matters/hrm-core/connection-pool';
import { accountSid, workerSid } from '../mocks';
import * as profileDB from '@tech-matters/hrm-core/profile/profileDataAccess';
import { systemUser } from '@tech-matters/twilio-worker-auth';
import { newTwilioUser, systemUser } from '@tech-matters/twilio-worker-auth';

let createdProfile: profileDB.Profile;
let createdProfileFlag: profileDB.ProfileFlag;
Expand Down Expand Up @@ -61,7 +61,7 @@ describe('cleanupProfileFlags', () => {
await Promise.all(
p.profileFlags.map(pf =>
profileDB.disassociateProfileFromProfileFlag()(accountSid, p.id, pf.id, {
user: { accountSid, isSupervisor: false, roles: [], workerSid },
user: newTwilioUser(accountSid, workerSid, []),
}),
),
);
Expand All @@ -75,7 +75,7 @@ describe('cleanupProfileFlags', () => {
createdProfile.id,
pf.id,
null,
{ user: { accountSid, isSupervisor: false, roles: [], workerSid } },
{ user: newTwilioUser(accountSid, workerSid, []) },
),
),
);
Expand All @@ -98,7 +98,7 @@ describe('cleanupProfileFlags', () => {
createdProfile.id,
pf.id,
futureDate,
{ user: { accountSid, isSupervisor: false, roles: [], workerSid } },
{ user: newTwilioUser(accountSid, workerSid, []) },
),
),
);
Expand All @@ -121,7 +121,7 @@ describe('cleanupProfileFlags', () => {
createdProfile.id,
pf.id,
pastDate,
{ user: { accountSid, isSupervisor: false, roles: [], workerSid } },
{ user: newTwilioUser(accountSid, workerSid, []) },
),
),
);
Expand Down
7 changes: 6 additions & 1 deletion packages/twilio-worker-auth/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,9 @@ export {
systemUser,
} from './twilioWorkerAuthMiddleware';
export { addAccountSidMiddleware } from './addAccountSidMiddleware';
export { newTwilioUser, TwilioUser } from './twilioUser';
export {
newTwilioUser,
newGlobalSystemUser,
newAccountSystemUser,
TwilioUser,
} from './twilioUser';
22 changes: 20 additions & 2 deletions packages/twilio-worker-auth/src/twilioUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,12 @@
*/
import { AccountSID, TwilioUserIdentifier } from '@tech-matters/types';

export const SYSTEM_USER = {};

export type TwilioUser = {
accountSid: AccountSID;
workerSid: TwilioUserIdentifier | undefined;
roles: string[];
isSupervisor: boolean;
isSystemUser: boolean;
};

export const newTwilioUser = (
Expand All @@ -34,4 +33,23 @@ export const newTwilioUser = (
workerSid,
roles,
isSupervisor: roles.includes('supervisor'),
isSystemUser: false,
});

export const newAccountSystemUser = (accountSid: AccountSID): Readonly<TwilioUser> =>
Object.freeze({
accountSid,
workerSid: `account-${accountSid}`,
roles: [],
isSupervisor: false,
isSystemUser: true,
});

export const newGlobalSystemUser = (accountSid: AccountSID): Readonly<TwilioUser> =>
Object.freeze({
accountSid,
workerSid: `system`,
roles: [],
isSupervisor: false,
isSystemUser: true,
});
35 changes: 27 additions & 8 deletions packages/twilio-worker-auth/src/twilioWorkerAuthMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,15 @@
import { validator as TokenValidator } from 'twilio-flex-token-validator';
import crypto from 'crypto';

import { newTwilioUser, TwilioUser } from './twilioUser';
import {
newAccountSystemUser,
newGlobalSystemUser,
newTwilioUser,
TwilioUser,
} from './twilioUser';
import { unauthorized } from '@tech-matters/http';
import type { Request, Response, NextFunction } from 'express';
import { AccountSID, TwilioUserIdentifier, WorkerSID } from '@tech-matters/types';
import { AccountSID, WorkerSID } from '@tech-matters/types';

declare global {
namespace Express {
Expand All @@ -36,11 +41,17 @@ declare global {
* @param {string} path
* @param {string} method
*
* IMPORTANT: This kind of static key acces should never be used to retrieve sensitive information.
* IMPORTANT: This kind of static key access should never be used to retrieve sensitive information.
*/
const canAccessResourceWithStaticKey = (path: string, method: string): boolean => {
// If the requests is to create a new post survey record, grant access
if (path.endsWith('/postSurveys') && method === 'POST') return true;
if (
(process.env.TASK_ROUTER_CONTACT_CREATION || '').toLowerCase() === 'true' &&
path.endsWith('/contacts') &&
method === 'POST'
)
return true;

// If the requests is retrieve the list of flags associated to a given identifier, grant access
if (/\/profiles\/identifier\/[^/]+\/flags$/.test(path) && method === 'GET') return true;
Expand All @@ -66,7 +77,7 @@ const extractAccountSid = (request: Request): AccountSID => {
const authenticateWithStaticKey = (
req: Request,
keySuffix: string,
userId?: TwilioUserIdentifier,
user: TwilioUser,
): boolean => {
if (!req.headers) return false;
const {
Expand All @@ -85,7 +96,7 @@ const authenticateWithStaticKey = (
crypto.timingSafeEqual(Buffer.from(requestSecret), Buffer.from(staticSecret));

if (isStaticSecretValid) {
req.user = newTwilioUser(extractAccountSid(req), userId, []);
req.user = user;
return true;
}
} catch (err) {
Expand Down Expand Up @@ -134,7 +145,7 @@ export const getAuthorizationMiddleware =

if (
canAccessResourceWithStaticKey(req.originalUrl, req.method) &&
authenticateWithStaticKey(req, accountSid, `account-${accountSid}`)
authenticateWithStaticKey(req, accountSid, newAccountSystemUser(accountSid))
)
return next();

Expand All @@ -153,14 +164,22 @@ export const staticKeyAuthorizationMiddleware = async (
);
}

if (authenticateWithStaticKey(req, accountSid, `account-${accountSid}`)) return next();
if (authenticateWithStaticKey(req, accountSid, newAccountSystemUser(accountSid)))
return next();
return unauthorized(res);
};

// TODO: do we want to differentiate what is actually system vs admin?
export const systemUser = 'system';
export const adminAuthorizationMiddleware =
(keySuffix: string) => async (req: Request, res: Response, next: NextFunction) => {
if (authenticateWithStaticKey(req, keySuffix, systemUser)) return next();
if (
authenticateWithStaticKey(
req,
keySuffix,
newGlobalSystemUser(extractAccountSid(req)),
)
)
return next();
return unauthorized(res);
};

0 comments on commit 8b3721a

Please sign in to comment.