From cb6a1ebe9340c27ec6f91d08d75a2fb55af28bd7 Mon Sep 17 00:00:00 2001 From: Rohan Port <59544282+rohan-bes@users.noreply.github.com> Date: Mon, 12 Feb 2024 10:38:10 +1100 Subject: [PATCH] RN-1001: Limit which users and permissions can be seen in the Admin Panel (#5350) --- packages/access-policy/src/AccessPolicy.js | 3 - .../admin-panel-server/src/app/createApp.ts | 2 +- .../resources/PermissionGroupsViewerPage.jsx | 14 +- .../src/pages/resources/UsersPage.jsx | 22 +++ .../src/apiV2/GETPermissionGroups.js | 4 +- .../accessRequests/EditAccessRequests.js | 6 +- .../assertAccessRequestPermissions.js | 89 +++++++++--- .../src/apiV2/import/importUserPermissions.js | 53 ++++--- .../src/apiV2/import/importUsers.js | 35 ++++- .../apiV2/userAccounts/CreateUserAccounts.js | 130 +++++++++++------- .../userAccounts/RegisterUserAccounts.js | 35 +++-- .../assertUserAccountPermissions.js | 121 ++++++++++++---- .../DeleteUserEntityPermissions.js | 4 +- .../assertUserEntityPermissionPermissions.js | 97 ++++++++++--- .../constructNewRecordValidationRules.js | 2 + .../getAdminPanelAllowedCountries.js | 19 +++ .../src/apiV2/utilities/index.js | 1 + .../src/permissions/assertions.js | 3 + .../accessRequests/EditAccessRequests.test.js | 27 +++- .../accessRequests/GETAccessRequests.test.js | 11 +- .../src/tests/apiV2/user.test.js | 5 +- .../userAccounts/EditUserAccounts.test.js | 23 +++- .../userAccounts/GETUserAccounts.test.js | 12 +- .../CreateUserEntityPermissions.test.js | 2 +- .../DeleteUserEntityPermissions.test.js | 30 +++- .../EditUserEntityPermissions.test.js | 12 +- .../GETUserEntityPermissions.test.js | 13 +- ...erEntityPermissionFKEYs-modifies-schema.js | 42 ++++++ .../src/api/mutations/useRegisterUser.js | 2 - .../src/microService/api/ApiBuilder.ts | 2 +- .../src/orchestrator/api/ApiBuilder.ts | 2 +- .../src/utils/initialiseApiClient.ts | 15 +- packages/types/src/schemas/schemas.ts | 12 +- packages/types/src/types/models.ts | 18 +-- 34 files changed, 645 insertions(+), 223 deletions(-) create mode 100644 packages/database/src/migrations/20240122235629-EnforceNotNullOnUserEntityPermissionFKEYs-modifies-schema.js diff --git a/packages/access-policy/src/AccessPolicy.js b/packages/access-policy/src/AccessPolicy.js index 83726723d1..de9998c3f4 100644 --- a/packages/access-policy/src/AccessPolicy.js +++ b/packages/access-policy/src/AccessPolicy.js @@ -15,9 +15,6 @@ export class AccessPolicy { throw new Error('Cannot instantiate an AccessPolicy without providing the policy details'); } const permissionGroupLists = Object.values(this.policy); - if (permissionGroupLists.length === 0) { - throw new Error('At least one entity should be specified in an access policy'); - } if (permissionGroupLists.some(permissionGroups => !Array.isArray(permissionGroups))) { throw new Error( 'Each entity should contain an array of permissionGroups for which the user has access', diff --git a/packages/admin-panel-server/src/app/createApp.ts b/packages/admin-panel-server/src/app/createApp.ts index dfe6c745d5..15c93dad9f 100644 --- a/packages/admin-panel-server/src/app/createApp.ts +++ b/packages/admin-panel-server/src/app/createApp.ts @@ -145,7 +145,7 @@ export async function createApp() { .use('hierarchies', forwardToEntityApi) .use('*', forwardRequest(CENTRAL_API_URL)); - await builder.initialiseApiClient([{ entityCode: 'DL', permissionGroupName: 'Public' }]); + await builder.initialiseApiClient(); const app = builder.build(); diff --git a/packages/admin-panel/src/pages/resources/PermissionGroupsViewerPage.jsx b/packages/admin-panel/src/pages/resources/PermissionGroupsViewerPage.jsx index bc0fb29a96..3287fecd21 100644 --- a/packages/admin-panel/src/pages/resources/PermissionGroupsViewerPage.jsx +++ b/packages/admin-panel/src/pages/resources/PermissionGroupsViewerPage.jsx @@ -29,15 +29,21 @@ const StyledHorizontalTree = styled(HorizontalTree)` } `; -function getDescendantData(parentNodeId, data) { - return data - .filter(node => node.parent_id === parentNodeId) +const getRootLevelNodes = data => + data.filter(({ parent_id: parentId }) => !data.some(({ id }) => id === parentId)); // Cannot find parent + +const getChildrenOfNode = (data, nodeId) => + data.filter(({ parent_id: parentId }) => parentId === nodeId); + +const getDescendantData = (parentNodeId, data) => { + const nodes = !parentNodeId ? getRootLevelNodes(data) : getChildrenOfNode(data, parentNodeId); + return nodes .sort((a, b) => a.name.localeCompare(b.name)) .map(node => { const children = data.filter(childNode => childNode.parent_id === node.id); return { id: node.id, name: node.name, children }; }); -} +}; const usePermissionGroups = () => { const { isLoading, data } = useQuery( diff --git a/packages/admin-panel/src/pages/resources/UsersPage.jsx b/packages/admin-panel/src/pages/resources/UsersPage.jsx index bebb685ab5..21f2e386d1 100644 --- a/packages/admin-panel/src/pages/resources/UsersPage.jsx +++ b/packages/admin-panel/src/pages/resources/UsersPage.jsx @@ -140,6 +140,28 @@ const CREATE_CONFIG = { editEndpoint: 'users', fields: [ ...EDIT_FIELDS, + { + Header: 'Country', + source: 'countryName', + editConfig: { + sourceKey: 'countryName', + optionsEndpoint: 'countries', + optionLabelKey: 'name', + optionValueKey: 'name', + secondaryLabel: 'Select the country to grant this user access to', + }, + }, + { + Header: 'Permission Group', + source: 'permissionGroupName', + editConfig: { + sourceKey: 'permissionGroupName', + optionsEndpoint: 'permissionGroups', + optionLabelKey: 'name', + optionValueKey: 'name', + secondaryLabel: 'Select the permission group to grant this user access to', + }, + }, { Header: 'Api Client (Not required for most users, see Readme of admin-panel for usage)', source: 'is_api_client', diff --git a/packages/central-server/src/apiV2/GETPermissionGroups.js b/packages/central-server/src/apiV2/GETPermissionGroups.js index 803ffc09ef..268d46a832 100644 --- a/packages/central-server/src/apiV2/GETPermissionGroups.js +++ b/packages/central-server/src/apiV2/GETPermissionGroups.js @@ -4,7 +4,7 @@ */ import { GETHandler } from './GETHandler'; -import { assertAdminPanelAccess, hasTupaiaAdminPanelAccess } from '../permissions'; +import { assertAdminPanelAccess, hasBESAdminAccess } from '../permissions'; export class GETPermissionGroups extends GETHandler { permissionsFilteredInternally = true; @@ -22,7 +22,7 @@ export class GETPermissionGroups extends GETHandler { } async getPermissionsFilter(dbConditions, dbOptions) { - if (hasTupaiaAdminPanelAccess(this.accessPolicy)) { + if (hasBESAdminAccess(this.accessPolicy)) { return { dbConditions, dbOptions }; } diff --git a/packages/central-server/src/apiV2/accessRequests/EditAccessRequests.js b/packages/central-server/src/apiV2/accessRequests/EditAccessRequests.js index 5c89d85449..ac699818cf 100644 --- a/packages/central-server/src/apiV2/accessRequests/EditAccessRequests.js +++ b/packages/central-server/src/apiV2/accessRequests/EditAccessRequests.js @@ -46,9 +46,13 @@ export class EditAccessRequests extends BulkEditHandler { async checkPermissionForRecord(models, recordId, updatedRecord) { const accessRequest = await models.accessRequest.findById(recordId); + const accessRequestData = await accessRequest.getData(); // Check Permissions const accessRequestChecker = accessPolicy => - assertAccessRequestEditPermissions(accessPolicy, models, recordId, updatedRecord); + assertAccessRequestEditPermissions(accessPolicy, models, recordId, { + ...accessRequestData, + ...updatedRecord, + }); await this.assertPermissions( assertAnyPermissions([assertBESAdminAccess, accessRequestChecker]), ); diff --git a/packages/central-server/src/apiV2/accessRequests/assertAccessRequestPermissions.js b/packages/central-server/src/apiV2/accessRequests/assertAccessRequestPermissions.js index 13948c9a10..99d8940e32 100644 --- a/packages/central-server/src/apiV2/accessRequests/assertAccessRequestPermissions.js +++ b/packages/central-server/src/apiV2/accessRequests/assertAccessRequestPermissions.js @@ -3,11 +3,11 @@ * Copyright (c) 2017 - 2020 Beyond Essential Systems Pty Ltd */ +import { QUERY_CONJUNCTIONS, SqlQuery } from '@tupaia/database'; import { hasBESAdminAccess, BES_ADMIN_PERMISSION_GROUP } from '../../permissions'; import { - getAdminPanelAllowedCountryIds, getAdminPanelAllowedCountryCodes, - mergeFilter, + getAdminPanelAllowedPermissionGroupIdsByCountryIds, } from '../utilities'; export const assertAccessRequestPermissions = async (accessPolicy, models, accessRequestId) => { @@ -18,10 +18,21 @@ export const assertAccessRequestPermissions = async (accessPolicy, models, acces const entity = await models.entity.findById(accessRequest.entity_id); const accessibleCountryCodes = getAdminPanelAllowedCountryCodes(accessPolicy); - if (accessibleCountryCodes.includes(entity.country_code)) { - return true; + if (!accessibleCountryCodes.includes(entity.country_code)) { + throw new Error('Need Admin Panel access to the country this access request is for'); } - throw new Error('Need Admin Panel access to the country this access request is for'); + + if (accessRequest.permission_group_id) { + const permissionGroup = await models.permissionGroup.findById( + accessRequest.permission_group_id, + ); + + if (!accessPolicy.allows(entity.country_code, permissionGroup.name)) { + throw new Error(`Need ${permissionGroup.name} access to ${entity.country_code}`); + } + } + + return true; }; export const assertAccessRequestEditPermissions = async ( @@ -40,7 +51,7 @@ export const assertAccessRequestEditPermissions = async ( const accessRequest = await models.accessRequest.findById(accessRequestId); const permissionGroup = await models.permissionGroup.findById(accessRequest.permission_group_id); if (permissionGroup.name === BES_ADMIN_PERMISSION_GROUP) { - throw new Error('Need BES Admin access to make this change'); + throw new Error('Need Admin Panel access to the country this access request is for'); } return true; @@ -51,34 +62,70 @@ export const assertAccessRequestUpsertPermissions = async ( models, { permission_group_id: permissionGroupId, entity_id: entityId }, ) => { - // Check we're not trying to change this access request to give someone: - // BES admin access - // Access to an entity we don't have admin panel access + const entity = await models.entity.findById(entityId); + const accessibleCountryCodes = getAdminPanelAllowedCountryCodes(accessPolicy); + if (!accessibleCountryCodes.includes(entity.country_code)) { + throw new Error('Need access to the newly edited entity'); + } + if (permissionGroupId) { + // Check we're not trying to change this access request to give someone: + // BES admin access + // Access to an entity we don't have admin panel access const permissionGroup = await models.permissionGroup.findById(permissionGroupId); if (permissionGroup.name === BES_ADMIN_PERMISSION_GROUP) { throw new Error('Need BES Admin access to make this change'); } - } - if (entityId) { - const entity = await models.entity.findById(entityId); - const accessibleCountryCodes = getAdminPanelAllowedCountryCodes(accessPolicy); - if (!accessibleCountryCodes.includes(entity.country_code)) { - throw new Error('Need access to the newly edited entity'); + + if (!accessPolicy.allows(entity.country_code, permissionGroup.name)) { + throw new Error(`Need ${permissionGroup.name} access to ${entity.country_code}`); } } }; +/** + * Filter to check if the entity permission is within our access policy. + * + * eg. { DL: [Admin, Public], TO: ['Donor'] } + * => + * (entity = 'DL' AND (permission_group IS NULL OR permission_group IN ('Admin', 'Public')) + * OR (entity = 'TO' AND (permission_group IS NULL OR permission_group IN ('Donor')) + */ +const buildRawSqlAccessRequestFilter = async (accessPolicy, models) => { + const allowedPermissionIdsByCountryIds = await getAdminPanelAllowedPermissionGroupIdsByCountryIds( + accessPolicy, + models, + ); + const sql = Object.values(allowedPermissionIdsByCountryIds) + .map( + permissionGroupIds => + `(access_request.entity_id = ? AND (access_request.permission_group_id IS NULL OR access_request.permission_group_id IN ${SqlQuery.record( + permissionGroupIds, + )}))`, + ) + .join(' OR '); + + const parameters = Object.entries(allowedPermissionIdsByCountryIds).flat(Infinity); + return { sql, parameters }; +}; + export const createAccessRequestDBFilter = async (accessPolicy, models, criteria) => { if (hasBESAdminAccess(accessPolicy)) { return criteria; } // If we don't have BES Admin access, add a filter to the SQL query - const dbConditions = { ...criteria }; - dbConditions['access_request.entity_id'] = mergeFilter( - await getAdminPanelAllowedCountryIds(accessPolicy, models), - dbConditions['access_request.entity_id'], - ); + const rawSqlFilter = await buildRawSqlAccessRequestFilter(accessPolicy, models); + if (!criteria || Object.keys(criteria).length === 0) { + // No given criteria, just return raw SQL + return { + [QUERY_CONJUNCTIONS.RAW]: rawSqlFilter, + }; + } - return dbConditions; + return { + ...criteria, + [QUERY_CONJUNCTIONS.AND]: { + [QUERY_CONJUNCTIONS.RAW]: rawSqlFilter, + }, + }; }; diff --git a/packages/central-server/src/apiV2/import/importUserPermissions.js b/packages/central-server/src/apiV2/import/importUserPermissions.js index 8c978d946d..0e0dbaf2a8 100644 --- a/packages/central-server/src/apiV2/import/importUserPermissions.js +++ b/packages/central-server/src/apiV2/import/importUserPermissions.js @@ -11,16 +11,16 @@ import { ObjectValidator, constructRecordExistsWithCode, constructRecordExistsWithField, - DatabaseError, } from '@tupaia/utils'; -import { assertBESAdminAccess } from '../../permissions'; +import { assertAnyPermissions, assertBESAdminAccess } from '../../permissions'; +import { assertUserEntityPermissionUpsertPermissions } from '../userEntityPermissions/assertUserEntityPermissionPermissions'; const extractItems = filePath => { const { Sheets: sheets } = xlsx.readFile(filePath); return xlsx.utils.sheet_to_json(Object.values(sheets)[0]); }; -async function create(transactingModels, items) { +async function create(req, transactingModels, items) { const validator = new ObjectValidator({ user_email: [constructRecordExistsWithField(transactingModels.user, 'email')], entity_code: [ @@ -48,19 +48,26 @@ async function create(transactingModels, items) { excelRowNumber++; await validator.validate(item, constructImportValidationError); - const { user_email, entity_code, permission_group_name } = item; + const { + user_email: email, + entity_code: entityCode, + permission_group_name: permissionGroupName, + } = item; - const user = await transactingModels.user.findOne({ email: user_email }); - const entity = await transactingModels.entity.findOne({ code: entity_code }); + const user = await transactingModels.user.findOne({ email }); + const entity = await transactingModels.entity.findOne({ code: entityCode }); const permissionGroup = await transactingModels.permissionGroup.findOne({ - name: permission_group_name, + name: permissionGroupName, }); - - const existingRecord = await transactingModels.userEntityPermission.findOne({ + const userEntityPermissionData = { user_id: user.id, entity_id: entity.id, permission_group_id: permissionGroup.id, - }); + }; + + const existingRecord = await transactingModels.userEntityPermission.findOne( + userEntityPermissionData, + ); if (existingRecord) { // Already added console.info( @@ -68,11 +75,23 @@ async function create(transactingModels, items) { ); continue; } else { - await transactingModels.userEntityPermission.create({ - user_id: user.id, - entity_id: entity.id, - permission_group_id: permissionGroup.id, - }); + const createUserEntityPermissionChecker = async accessPolicy => { + await assertUserEntityPermissionUpsertPermissions( + accessPolicy, + transactingModels, + userEntityPermissionData, + ); + }; + + try { + await req.assertPermissions( + assertAnyPermissions([assertBESAdminAccess, createUserEntityPermissionChecker]), + ); + } catch (error) { + throw constructImportValidationError(error.message); + } + + await transactingModels.userEntityPermission.create(userEntityPermissionData); } } } @@ -83,8 +102,6 @@ async function create(transactingModels, items) { export async function importUserPermissions(req, res) { const { models } = req; - await req.assertPermissions(assertBESAdminAccess); - let items; try { items = extractItems(req.file.path); @@ -93,7 +110,7 @@ export async function importUserPermissions(req, res) { } await models.wrapInTransaction(async transactingModels => { - await create(transactingModels, items); + await create(req, transactingModels, items); respond(res, { message: `Imported User Permissions` }); }); } diff --git a/packages/central-server/src/apiV2/import/importUsers.js b/packages/central-server/src/apiV2/import/importUsers.js index 9001b77e94..4e19c84f8d 100644 --- a/packages/central-server/src/apiV2/import/importUsers.js +++ b/packages/central-server/src/apiV2/import/importUsers.js @@ -16,11 +16,14 @@ import { } from '@tupaia/utils'; import { hashAndSaltPassword } from '@tupaia/auth'; import { VerifiedEmail } from '@tupaia/types'; -import { assertBESAdminAccess } from '../../permissions'; +import { + TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, + assertAnyPermissions, + assertBESAdminAccess, + hasTupaiaAdminPanelAccessToCountry, +} from '../../permissions'; export async function importUsers(req, res) { - await req.assertPermissions(assertBESAdminAccess); - try { const { models } = req; if (!req.file) { @@ -70,6 +73,32 @@ export async function importUsers(req, res) { countryName, ); } + + const createUserPermissionChecker = accessPolicy => { + if (!hasTupaiaAdminPanelAccessToCountry(accessPolicy, countryEntity.code)) { + throw new Error( + `Need ${TUPAIA_ADMIN_PANEL_PERMISSION_GROUP} access to ${countryEntity.name}`, + ); + } + + if (!accessPolicy.allows(countryEntity.code, permissionGroup.name)) { + throw new Error(`Need ${permissionGroup.name} access to ${countryEntity.name}`); + } + }; + + try { + await req.assertPermissions( + assertAnyPermissions([assertBESAdminAccess, createUserPermissionChecker]), + ); + } catch (error) { + throw new ImportValidationError( + error.message, + excelRowNumber, + 'permission_group', + countryName, + ); + } + await transactingModels.userEntityPermission.findOrCreate({ user_id: user.id, entity_id: countryEntity.id, diff --git a/packages/central-server/src/apiV2/userAccounts/CreateUserAccounts.js b/packages/central-server/src/apiV2/userAccounts/CreateUserAccounts.js index a0beecf1c3..0834082066 100644 --- a/packages/central-server/src/apiV2/userAccounts/CreateUserAccounts.js +++ b/packages/central-server/src/apiV2/userAccounts/CreateUserAccounts.js @@ -5,7 +5,13 @@ import { hashAndSaltPassword, encryptPassword, generateSecretKey } from '@tupaia/auth'; import { CreateHandler } from '../CreateHandler'; -import { assertBESAdminAccess } from '../../permissions'; +import { + TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, + assertAdminPanelAccess, + assertAnyPermissions, + assertBESAdminAccess, + hasTupaiaAdminPanelAccessToCountry, +} from '../../permissions'; /** * Handles POST endpoints: @@ -14,58 +20,23 @@ import { assertBESAdminAccess } from '../../permissions'; export class CreateUserAccounts extends CreateHandler { async assertUserHasAccess() { - await this.assertPermissions(assertBESAdminAccess); + await this.assertPermissions(assertAdminPanelAccess); } async createRecord() { - return this.createUserRecord(this.newRecordData); - } - - async createUserRecord({ - firstName, - lastName, - emailAddress, - contactNumber, - password, - countryName = 'Demo Land', - permissionGroupName, - primaryPlatform, - is_api_client: isApiClient, - verifiedEmail, - ...restOfUser - }) { return this.models.wrapInTransaction(async transactingModels => { - const permissionGroup = await transactingModels.permissionGroup.findOne({ - name: permissionGroupName, - }); - const country = await transactingModels.entity.findOne({ - name: countryName, - type: 'country', - }); + const { + countryName, + permissionGroupName, + is_api_client: isApiClient, + ...restOfUser + } = this.newRecordData; - if (!permissionGroup) { - throw new Error(`No such permission group: ${permissionGroupName}`); - } - - if (!country) { - throw new Error(`No such country: ${countryName}`); - } - - const user = await transactingModels.user.create({ - first_name: firstName, - last_name: lastName, - email: emailAddress, - mobile_number: contactNumber, - primary_platform: primaryPlatform, - ...hashAndSaltPassword(password), - verified_email: verifiedEmail, - ...restOfUser, - }); - - await transactingModels.userEntityPermission.findOrCreate({ - user_id: user.id, - entity_id: country.id, - permission_group_id: permissionGroup.id, + const user = await this.createUserRecord(transactingModels, restOfUser); + await this.createUserPermission(transactingModels, { + userId: user.id, + countryName, + permissionGroupName, }); let secretKey; @@ -81,4 +52,67 @@ export class CreateUserAccounts extends CreateHandler { return { userId: user.id, secretKey }; }); } + + async createUserPermission(transactingModels, { userId, countryName, permissionGroupName }) { + const permissionGroup = await transactingModels.permissionGroup.findOne({ + name: permissionGroupName, + }); + const country = await transactingModels.entity.findOne({ + name: countryName, + type: 'country', + }); + + if (!permissionGroup) { + throw new Error(`No such permission group: ${permissionGroupName}`); + } + + if (!country) { + throw new Error(`No such country: ${countryName}`); + } + + const countryPermissionChecker = accessPolicy => { + if (!hasTupaiaAdminPanelAccessToCountry(accessPolicy, country.code)) { + throw new Error(`Need ${TUPAIA_ADMIN_PANEL_PERMISSION_GROUP} access to ${country.name}`); + } + + if (!accessPolicy.allows(country.code, permissionGroup.name)) { + throw new Error(`Need ${permissionGroup.name} access to ${country.name}`); + } + }; + + await this.assertPermissions( + assertAnyPermissions([assertBESAdminAccess, countryPermissionChecker]), + ); + + return transactingModels.userEntityPermission.findOrCreate({ + user_id: userId, + entity_id: country.id, + permission_group_id: permissionGroup.id, + }); + } + + async createUserRecord( + transactingModels, + { + firstName, + lastName, + emailAddress, + contactNumber, + password, + primaryPlatform, + verifiedEmail, + ...restOfUser + }, + ) { + return transactingModels.user.create({ + first_name: firstName, + last_name: lastName, + email: emailAddress, + mobile_number: contactNumber, + primary_platform: primaryPlatform, + ...hashAndSaltPassword(password), + verified_email: verifiedEmail, + ...restOfUser, + }); + } } diff --git a/packages/central-server/src/apiV2/userAccounts/RegisterUserAccounts.js b/packages/central-server/src/apiV2/userAccounts/RegisterUserAccounts.js index 7c8af55118..4409abffec 100644 --- a/packages/central-server/src/apiV2/userAccounts/RegisterUserAccounts.js +++ b/packages/central-server/src/apiV2/userAccounts/RegisterUserAccounts.js @@ -67,18 +67,29 @@ export class RegisterUserAccounts extends CreateUserAccounts { async createRecord() { const { - accessPolicy, - body: { countryName = 'Demo Land', permissionGroupName = 'Public' }, - } = this.req; - const country = await this.models.entity.findOne({ name: countryName, type: 'country' }); - if (!country) { - throw new Error(`No such country: ${countryName}`); - } - // check the api client has access to the country they are trying to register a user for - if (!accessPolicy.allows(country.code, permissionGroupName)) { - throw new Error(`User does not have ${permissionGroupName} access to ${countryName}`); - } - const { userId } = await this.createUserRecord(this.newRecordData); + firstName, + lastName, + emailAddress, + employer, + position, + contactNumber, + password, + primaryPlatform, + } = this.newRecordData; + + const userData = { + firstName, + lastName, + emailAddress, + employer, + position, + contactNumber, + password, + primaryPlatform, + }; + + const { id: userId } = await this.createUserRecord(this.models, userData); + const user = await this.models.user.findById(userId); await sendEmailVerification(user); diff --git a/packages/central-server/src/apiV2/userAccounts/assertUserAccountPermissions.js b/packages/central-server/src/apiV2/userAccounts/assertUserAccountPermissions.js index abb77b4bf1..85382a3cfb 100644 --- a/packages/central-server/src/apiV2/userAccounts/assertUserAccountPermissions.js +++ b/packages/central-server/src/apiV2/userAccounts/assertUserAccountPermissions.js @@ -3,15 +3,14 @@ * Copyright (c) 2017 - 2020 Beyond Essential Systems Pty Ltd */ -import { QUERY_CONJUNCTIONS } from '@tupaia/database'; +import keyBy from 'lodash.keyby'; +import { QUERY_CONJUNCTIONS, SqlQuery } from '@tupaia/database'; import { hasBESAdminAccess, TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, LESMIS_ADMIN_PERMISSION_GROUP, } from '../../permissions'; -const { RAW } = QUERY_CONJUNCTIONS; - export const assertUserAccountPermissions = async (accessPolicy, models, userAccountId) => { const userAccount = await models.user.findById(userAccountId); if (!userAccount) { @@ -19,40 +18,116 @@ export const assertUserAccountPermissions = async (accessPolicy, models, userAcc } const entityPermissions = await models.userEntityPermission.find({ user_id: userAccount.id }); + const permissions = await models.permissionGroup.findManyById( + entityPermissions.map(ep => ep.permission_group_id), + ); + const permissionsById = keyBy(permissions, 'id'); const entities = await models.entity.findManyById(entityPermissions.map(ep => ep.entity_id)); - const countryCodes = entities.map(e => e.country_code).filter(c => c !== 'DL'); + const entitiesById = keyBy(entities, 'id'); + const countryCodes = entities.map(e => e.country_code); if (countryCodes.length === 0) { - // User only has access to Demo Land, and it got filtered out + // User has no permissions, so anyone can view/edit their account return true; } + + // Must have Admin Panel access to all countries the user has access to if (!accessPolicy.allowsAll(countryCodes, TUPAIA_ADMIN_PANEL_PERMISSION_GROUP)) { throw new Error('Need Admin Panel access to all countries this user has access to'); } + // Must have equal or higher permissions than the user has for each permission they have + entityPermissions.forEach(({ entity_id: entityId, permission_group_id: permissionGroupId }) => { + const countryCode = entitiesById[entityId].country_code; + const permissionGroup = permissionsById[permissionGroupId].name; + if (!accessPolicy.allows(countryCode, permissionGroup)) { + throw new Error(`Need ${permissionGroup} access to ${countryCode}`); + } + }); + return true; }; +const buildUserAccountRawSqlFilter = accessPolicy => { + const accessibleCountryCodes = [ + ...accessPolicy.getEntitiesAllowed(TUPAIA_ADMIN_PANEL_PERMISSION_GROUP), + ...accessPolicy.getEntitiesAllowed(LESMIS_ADMIN_PERMISSION_GROUP), + ]; + const permissionsByCountryCode = accessibleCountryCodes.reduce( + (obj, countryCode) => ({ + ...obj, + [countryCode]: accessPolicy.getPermissionGroups([countryCode]), + }), + {}, + ); + + /** + * Here we're building up an inner query to work out which user permissions exist that we don't + * have access to. Once we know that, we just filter to find the users whose ids are not in that list + * + * eg. If my access policy is: { DL: ['Admin', 'Public'], TO: ['Donor'] } + * The query is: + user_account.id NOT IN ( + SELECT uep.user_id FROM user_entity_permission uep + JOIN entity e ON uep.entity_id = e.id + JOIN permission_group pg ON uep.permission_group_id = pg.id + + WHERE + -- Either we don't have access to a country they have access to + e.code NOT IN ('DL', 'TO') + + -- Or we have access to it, but not with the level of permissions they do + OR (e.code = 'DL' AND pg.name NOT IN ('Admin', 'Public')) + OR (e.code = 'TO' AND pg.name NOT IN ('Donor')) + ) + */ + const sql = ` + user_account.id NOT IN ( + -- Inner query to detect which users have permissions that we do not + SELECT uep.user_id FROM user_entity_permission uep + JOIN entity e ON uep.entity_id = e.id + JOIN permission_group pg ON uep.permission_group_id = pg.id + + WHERE + -- Either we don't have access to a country they have access to + e.code NOT IN ${SqlQuery.record(accessibleCountryCodes)} + + -- Or we have access to it, but not with the level of permissions they do +${accessibleCountryCodes + .map( + countryCode => + ` OR (e.code = ? AND pg.name NOT IN ${SqlQuery.record( + permissionsByCountryCode[countryCode], + )})`, + ) + .join('\n')} + )`; + + const parameters = [ + ...accessibleCountryCodes, + ...accessibleCountryCodes + .map(countryCode => [countryCode, ...permissionsByCountryCode[countryCode]]) + .flat(), + ]; + return { sql, parameters }; +}; + export const createUserAccountDBFilter = async (accessPolicy, models, criteria) => { if (hasBESAdminAccess(accessPolicy)) { return criteria; } + // If we don't have BES Admin access, add a filter to the SQL query - const dbConditions = { ...criteria }; - const accessibleCountryCodes = [ - ...accessPolicy.getEntitiesAllowed(TUPAIA_ADMIN_PANEL_PERMISSION_GROUP), - ...accessPolicy.getEntitiesAllowed(LESMIS_ADMIN_PERMISSION_GROUP), - ]; - accessibleCountryCodes.push('DL'); // If we have admin panel anywhere, we can also view Demo Land - const entities = await models.entity.find({ - code: accessibleCountryCodes, - }); - const entityIds = entities.map(e => e.id); - // Checks list of entity ids the user has access to is contained within the list of entity ids - // the accessPolicy permits (plus Demo Land) - dbConditions[RAW] = { - sql: `array(select entity_id from user_entity_permission uep where uep.user_id = user_account.id) <@ array[${entityIds - .map(() => '?') - .join(',')}]`, - parameters: entityIds, + const rawSqlFilter = buildUserAccountRawSqlFilter(accessPolicy, models); + if (!criteria || Object.keys(criteria).length === 0) { + // No given criteria, just return raw SQL + return { + [QUERY_CONJUNCTIONS.RAW]: rawSqlFilter, + }; + } + + return { + ...criteria, + [QUERY_CONJUNCTIONS.AND]: { + [QUERY_CONJUNCTIONS.RAW]: rawSqlFilter, + }, }; - return dbConditions; }; diff --git a/packages/central-server/src/apiV2/userEntityPermissions/DeleteUserEntityPermissions.js b/packages/central-server/src/apiV2/userEntityPermissions/DeleteUserEntityPermissions.js index 7e913940c9..813d7c68cf 100644 --- a/packages/central-server/src/apiV2/userEntityPermissions/DeleteUserEntityPermissions.js +++ b/packages/central-server/src/apiV2/userEntityPermissions/DeleteUserEntityPermissions.js @@ -5,7 +5,7 @@ import { DeleteHandler } from '../DeleteHandler'; import { assertAnyPermissions, assertBESAdminAccess } from '../../permissions'; -import { assertUserEntityPermissionEditPermissions } from './assertUserEntityPermissionPermissions'; +import { assertUserEntityPermissionPermissions } from './assertUserEntityPermissionPermissions'; /** * Handles DELETE endpoints: @@ -16,7 +16,7 @@ export class DeleteUserEntityPermissions extends DeleteHandler { async assertUserHasAccess() { // Check Permissions const userEntityPermissionChecker = accessPolicy => - assertUserEntityPermissionEditPermissions(accessPolicy, this.models, this.recordId); + assertUserEntityPermissionPermissions(accessPolicy, this.models, this.recordId); await this.assertPermissions( assertAnyPermissions([assertBESAdminAccess, userEntityPermissionChecker]), ); diff --git a/packages/central-server/src/apiV2/userEntityPermissions/assertUserEntityPermissionPermissions.js b/packages/central-server/src/apiV2/userEntityPermissions/assertUserEntityPermissionPermissions.js index ea6716298f..95d8f15613 100644 --- a/packages/central-server/src/apiV2/userEntityPermissions/assertUserEntityPermissionPermissions.js +++ b/packages/central-server/src/apiV2/userEntityPermissions/assertUserEntityPermissionPermissions.js @@ -3,12 +3,17 @@ * Copyright (c) 2017 - 2020 Beyond Essential Systems Pty Ltd */ +import { QUERY_CONJUNCTIONS, SqlQuery } from '@tupaia/database'; import { hasBESAdminAccess, BES_ADMIN_PERMISSION_GROUP, TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, } from '../../permissions'; -import { getAdminPanelAllowedCountryIds, getAdminPanelAllowedCountryCodes } from '../utilities'; +import { + getAdminPanelAllowedCountryCodes, + getAdminPanelAllowedPermissionGroupIdsByCountryIds, +} from '../utilities'; +import { assertUserAccountPermissions } from '../userAccounts/assertUserAccountPermissions'; export const assertUserEntityPermissionPermissions = async ( accessPolicy, @@ -21,11 +26,19 @@ export const assertUserEntityPermissionPermissions = async ( } const entity = await models.entity.findById(userEntityPermission.entity_id); + const permissionGroup = await models.permissionGroup.findById( + userEntityPermission.permission_group_id, + ); const accessibleCountryCodes = getAdminPanelAllowedCountryCodes(accessPolicy); - if (accessibleCountryCodes.includes(entity.country_code)) { - return true; + if (!accessibleCountryCodes.includes(entity.country_code)) { + throw new Error(`Need Admin Panel access to ${entity.country_code}`); + } + + if (!accessPolicy.allows(entity.code, permissionGroup.name)) { + throw new Error(`Need ${permissionGroup.name} access to ${entity.code}`); } - throw new Error('Need Admin Panel access to the country this entity is in'); + + return true; }; export const assertUserEntityPermissionEditPermissions = async ( @@ -36,12 +49,16 @@ export const assertUserEntityPermissionEditPermissions = async ( ) => { // Check we have permission to access the record we're trying to edit await assertUserEntityPermissionPermissions(accessPolicy, models, userEntityPermissionId); + const userEntityPermission = await models.userEntityPermission.findById(userEntityPermissionId); + const userEntityPermissionData = await userEntityPermission.getData(); // Check we have permission for the changes - await assertUserEntityPermissionUpsertPermissions(accessPolicy, models, updatedFields); + await assertUserEntityPermissionUpsertPermissions(accessPolicy, models, { + ...userEntityPermissionData, + ...updatedFields, + }); // Final check to make sure we're not editing a BES admin access permission // Changing any of the pieces of data in a BES admin UEP is abusable, so completely forbid it - const userEntityPermission = await models.userEntityPermission.findById(userEntityPermissionId); const permissionGroup = await userEntityPermission.permissionGroup(); if (permissionGroup.name === BES_ADMIN_PERMISSION_GROUP) { throw new Error('Need BES Admin access to make this change'); @@ -53,34 +70,72 @@ export const assertUserEntityPermissionEditPermissions = async ( export const assertUserEntityPermissionUpsertPermissions = async ( accessPolicy, models, - { permission_group_id: permissionGroupId, entity_id: entityId }, + { user_id: userId, permission_group_id: permissionGroupId, entity_id: entityId }, ) => { // Check we're not trying to give someone: // BES admin access // Access to an entity we don't have admin panel access - if (permissionGroupId) { - const permissionGroup = await models.permissionGroup.findById(permissionGroupId); - if (permissionGroup.name === BES_ADMIN_PERMISSION_GROUP) { - throw new Error('Need BES Admin access to make this change'); - } + const permissionGroup = await models.permissionGroup.findById(permissionGroupId); + if (permissionGroup.name === BES_ADMIN_PERMISSION_GROUP) { + throw new Error('Need BES Admin access to make this change'); + } + + const entity = await models.entity.findById(entityId); + if (!accessPolicy.allows(entity.country_code, TUPAIA_ADMIN_PANEL_PERMISSION_GROUP)) { + throw new Error(`Need Admin Panel access to ${entity.country_code}`); } - if (entityId) { - const entity = await models.entity.findById(entityId); - if (!accessPolicy.allows(entity.country_code, TUPAIA_ADMIN_PANEL_PERMISSION_GROUP)) { - throw new Error('Need Admin Panel access to the updated entity'); - } + + if (!accessPolicy.allows(entity.code, permissionGroup.name)) { + throw new Error(`Need ${permissionGroup.name} access to ${entity.code}`); } + + await assertUserAccountPermissions(accessPolicy, models, userId); +}; + +/** + * Filter to check if the entity permission is within our access policy. + * + * eg. { DL: [Admin, Public], TO: ['Donor'] } + * => + * (entity = 'DL' AND permission_group IN ('Admin', 'Public') + * OR (entity = 'TO' AND permission_group IN ('Donor') + */ +const buildRawSqlUserEntityPermissionsFilter = async (accessPolicy, models) => { + const allowedPermissionIdsByCountryIds = await getAdminPanelAllowedPermissionGroupIdsByCountryIds( + accessPolicy, + models, + ); + const sql = Object.values(allowedPermissionIdsByCountryIds) + .map( + permissionGroupIds => + `(user_entity_permission.entity_id = ? AND user_entity_permission.permission_group_id IN ${SqlQuery.record( + permissionGroupIds, + )})`, + ) + .join(' OR '); + + const parameters = Object.entries(allowedPermissionIdsByCountryIds).flat(Infinity); + return { sql, parameters }; }; export const createUserEntityPermissionDBFilter = async (accessPolicy, models, criteria) => { if (hasBESAdminAccess(accessPolicy)) { return criteria; } + // If we don't have BES Admin access, add a filter to the SQL query - const dbConditions = { - 'user_entity_permission.entity_id': await getAdminPanelAllowedCountryIds(accessPolicy, models), + const rawSqlFilter = await buildRawSqlUserEntityPermissionsFilter(accessPolicy, models); + if (!criteria || Object.keys(criteria).length === 0) { + // No given criteria, just return raw SQL + return { + [QUERY_CONJUNCTIONS.RAW]: rawSqlFilter, + }; + } + + return { ...criteria, + [QUERY_CONJUNCTIONS.AND]: { + [QUERY_CONJUNCTIONS.RAW]: rawSqlFilter, + }, }; - - return dbConditions; }; diff --git a/packages/central-server/src/apiV2/utilities/constructNewRecordValidationRules.js b/packages/central-server/src/apiV2/utilities/constructNewRecordValidationRules.js index ec731cef2e..325ced3925 100644 --- a/packages/central-server/src/apiV2/utilities/constructNewRecordValidationRules.js +++ b/packages/central-server/src/apiV2/utilities/constructNewRecordValidationRules.js @@ -73,6 +73,8 @@ export const constructForSingle = (models, recordType) => { last_name: [hasContent], email: [hasContent, isEmail], password: [isValidPassword], + countryName: [hasContent], + permissionGroupName: [hasContent], }; case TYPES.FEED_ITEM: return { diff --git a/packages/central-server/src/apiV2/utilities/getAdminPanelAllowedCountries.js b/packages/central-server/src/apiV2/utilities/getAdminPanelAllowedCountries.js index 91bd15de99..77bbe2d97f 100644 --- a/packages/central-server/src/apiV2/utilities/getAdminPanelAllowedCountries.js +++ b/packages/central-server/src/apiV2/utilities/getAdminPanelAllowedCountries.js @@ -54,3 +54,22 @@ export const getAdminPanelAllowedCountryIds = async (accessPolicy, models) => { return entities.map(e => e.id); }; + +/** + * Get a mapping of countryIds to permissionGroupIds for countries that the user has Tupaia Admin Panel access to + */ +export const getAdminPanelAllowedPermissionGroupIdsByCountryIds = async (accessPolicy, models) => { + const allowedCountryCodes = getAdminPanelAllowedCountryCodes(accessPolicy); + return Object.fromEntries( + await Promise.all( + allowedCountryCodes.map(async countryCode => [ + (await models.entity.findOne({ code: countryCode })).id, + ( + await models.permissionGroup.find({ + name: accessPolicy.getPermissionGroups([countryCode]), + }) + ).map(({ id }) => id), + ]), + ), + ); +}; diff --git a/packages/central-server/src/apiV2/utilities/index.js b/packages/central-server/src/apiV2/utilities/index.js index d52ba91f84..ce172d1cdf 100644 --- a/packages/central-server/src/apiV2/utilities/index.js +++ b/packages/central-server/src/apiV2/utilities/index.js @@ -14,6 +14,7 @@ export { fetchRequestingMeditrakDevice } from '../meditrakApp/utilities/fetchReq export { getAdminPanelAllowedCountryIds, getAdminPanelAllowedCountryCodes, + getAdminPanelAllowedPermissionGroupIdsByCountryIds, } from './getAdminPanelAllowedCountries'; export { getArrayQueryParameter } from './getArrayQueryParameter'; export { hasAccessToEntityForVisualisation } from './hasAccessToEntityForVisualisation'; diff --git a/packages/central-server/src/permissions/assertions.js b/packages/central-server/src/permissions/assertions.js index a5ddc176f8..583b03b56c 100644 --- a/packages/central-server/src/permissions/assertions.js +++ b/packages/central-server/src/permissions/assertions.js @@ -99,6 +99,9 @@ export const assertBESAdminAccess = accessPolicy => { export const hasTupaiaAdminPanelAccess = accessPolicy => accessPolicy.allowsSome(undefined, TUPAIA_ADMIN_PANEL_PERMISSION_GROUP); +export const hasTupaiaAdminPanelAccessToCountry = (accessPolicy, countryCode) => + accessPolicy.allows(countryCode, TUPAIA_ADMIN_PANEL_PERMISSION_GROUP); + export const assertAdminPanelAccess = accessPolicy => { if (hasTupaiaAdminPanelAccess(accessPolicy) || hasLESMISAdminAccess(accessPolicy)) { return true; diff --git a/packages/central-server/src/tests/apiV2/accessRequests/EditAccessRequests.test.js b/packages/central-server/src/tests/apiV2/accessRequests/EditAccessRequests.test.js index 2b474861ce..df7659931d 100644 --- a/packages/central-server/src/tests/apiV2/accessRequests/EditAccessRequests.test.js +++ b/packages/central-server/src/tests/apiV2/accessRequests/EditAccessRequests.test.js @@ -14,11 +14,11 @@ import { TestableApp } from '../../testUtilities'; describe('Permissions checker for EditAccessRequests', async () => { const DEFAULT_POLICY = { DL: ['Public'], - KI: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin'], + KI: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin', 'Public'], SB: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Royal Australasian College of Surgeons'], - VU: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin'], - LA: ['Admin'], - TO: ['Admin'], + VU: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin', 'Public'], + LA: ['Admin', 'Public'], + TO: ['Admin', 'Public'], }; const BES_ADMIN_POLICY = { @@ -29,6 +29,7 @@ describe('Permissions checker for EditAccessRequests', async () => { const { models } = app; let vanuatuPublicRequest; let laosPublicRequest; + let kiribatiCatRequest; let kiribatiBESRequest; let laosEntityId; let besPermissionGroupId; @@ -48,6 +49,9 @@ describe('Permissions checker for EditAccessRequests', async () => { const publicPermissionGroup = await findOrCreateDummyRecord(models.permissionGroup, { name: 'Public', }); + const catPermissionGroup = await findOrCreateDummyRecord(models.permissionGroup, { + name: 'Cat', + }); const besPermissionGroup = await findOrCreateDummyRecord(models.permissionGroup, { name: BES_ADMIN_PERMISSION_GROUP, }); @@ -65,6 +69,12 @@ describe('Permissions checker for EditAccessRequests', async () => { approved: null, processed_by: null, }); + kiribatiCatRequest = await findOrCreateDummyRecord(models.accessRequest, { + entity_id: kiribatiEntity.id, + permission_group_id: catPermissionGroup.id, + approved: null, + processed_by: null, + }); kiribatiBESRequest = await findOrCreateDummyRecord(models.accessRequest, { entity_id: kiribatiEntity.id, permission_group_id: besPermissionGroup.id, @@ -100,6 +110,15 @@ describe('Permissions checker for EditAccessRequests', async () => { expect(result).to.have.keys('error'); }); + it('Throw an exception if we do not have equal or greater access to the entity of the access request are editing', async () => { + await app.grantAccess(DEFAULT_POLICY); + const { body: result } = await app.put(`accessRequests/${kiribatiCatRequest.id}`, { + body: { approved: true }, + }); + + expect(result).to.have.keys('error'); + }); + it('Throw an exception when trying to edit an access request to point to an entity we lack permission for', async () => { await app.grantAccess(DEFAULT_POLICY); const { body: result } = await app.put(`accessRequests/${vanuatuPublicRequest.id}`, { diff --git a/packages/central-server/src/tests/apiV2/accessRequests/GETAccessRequests.test.js b/packages/central-server/src/tests/apiV2/accessRequests/GETAccessRequests.test.js index ad6a0de4ec..a5ffd6ba09 100644 --- a/packages/central-server/src/tests/apiV2/accessRequests/GETAccessRequests.test.js +++ b/packages/central-server/src/tests/apiV2/accessRequests/GETAccessRequests.test.js @@ -14,15 +14,14 @@ import { resetTestData, TestableApp } from '../../testUtilities'; describe('Permissions checker for GETAccessRequests', async () => { const DEFAULT_POLICY = { DL: ['Public'], - KI: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin'], - SB: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Royal Australasian College of Surgeons'], - VU: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin'], - LA: ['Admin'], - TO: ['Admin'], + KI: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin', 'Public'], + VU: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin', 'Public'], + LA: ['Admin', 'Public'], + TO: ['Admin', 'Public'], }; const BES_ADMIN_POLICY = { - SB: [BES_ADMIN_PERMISSION_GROUP], + DL: [BES_ADMIN_PERMISSION_GROUP], }; const app = new TestableApp(); diff --git a/packages/central-server/src/tests/apiV2/user.test.js b/packages/central-server/src/tests/apiV2/user.test.js index 99f50fec27..1730f4282b 100644 --- a/packages/central-server/src/tests/apiV2/user.test.js +++ b/packages/central-server/src/tests/apiV2/user.test.js @@ -57,10 +57,9 @@ describe('/user', () => { }); }); - it('should have only Demo Land UserEntityPermission model in database', async () => { + it('should have no UserEntityPermission model in database', async () => { const userEntityPermissions = await models.userEntityPermission.find({ user_id: userId }); - expect(userEntityPermissions.length).to.equal(1); - expect(userEntityPermissions[0].entity_code).to.equal('DL'); + expect(userEntityPermissions.length).to.equal(0); }); }); }); diff --git a/packages/central-server/src/tests/apiV2/userAccounts/EditUserAccounts.test.js b/packages/central-server/src/tests/apiV2/userAccounts/EditUserAccounts.test.js index 6bd4aa99d5..2ad3136a69 100644 --- a/packages/central-server/src/tests/apiV2/userAccounts/EditUserAccounts.test.js +++ b/packages/central-server/src/tests/apiV2/userAccounts/EditUserAccounts.test.js @@ -13,12 +13,12 @@ import { TestableApp } from '../../testUtilities'; describe('Permissions checker for EditUserAccounts', async () => { const DEFAULT_POLICY = { - DL: ['Public'], - KI: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin'], + DL: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Public'], + KI: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin', 'Public'], SB: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Royal Australasian College of Surgeons'], - VU: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin'], - LA: ['Admin'], - TO: ['Admin'], + VU: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin', 'Public'], + LA: ['Admin', 'Public'], + TO: ['Admin', 'Public'], }; const BES_ADMIN_POLICY = { @@ -50,11 +50,11 @@ describe('Permissions checker for EditUserAccounts', async () => { // Create test users userAccount1 = await findOrCreateDummyRecord(models.user, { first_name: 'Barry', - last_name: 'Allen', + last_name: 'EditUserAccounts', }); userAccount2 = await findOrCreateDummyRecord(models.user, { first_name: 'Hal', - last_name: 'Jordan', + last_name: 'EditUserAccounts', }); // Give the test users some permissions @@ -112,6 +112,15 @@ describe('Permissions checker for EditUserAccounts', async () => { expect(result).to.have.keys('error'); }); + + it('Throw an exception if we do not have equal or greater access to all the countries the user we are editing has access to', async () => { + await app.grantAccess({ ...DEFAULT_POLICY, KI: ['Donor'] }); + const { body: result } = await app.put(`users/${userAccount2.id}`, { + body: { email: 'barry.allen@ccpd.gov' }, + }); + + expect(result).to.have.keys('error'); + }); }); describe('Sufficient permissions', async () => { diff --git a/packages/central-server/src/tests/apiV2/userAccounts/GETUserAccounts.test.js b/packages/central-server/src/tests/apiV2/userAccounts/GETUserAccounts.test.js index 387e2c294a..cb11d8b472 100644 --- a/packages/central-server/src/tests/apiV2/userAccounts/GETUserAccounts.test.js +++ b/packages/central-server/src/tests/apiV2/userAccounts/GETUserAccounts.test.js @@ -13,10 +13,10 @@ import { TestableApp } from '../../testUtilities'; describe('Permissions checker for GETUserAccounts', async () => { const DEFAULT_POLICY = { - DL: ['Public'], - KI: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin'], + DL: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Public'], + KI: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin', 'Public'], SB: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Royal Australasian College of Surgeons'], - VU: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin'], + VU: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin', 'Public'], LA: ['Admin'], TO: ['Admin'], }; @@ -57,15 +57,15 @@ describe('Permissions checker for GETUserAccounts', async () => { // Create test users userAccount1 = await findOrCreateDummyRecord(models.user, { first_name: 'Clark', - last_name: 'Kent', + last_name: 'GETUserAccounts', }); userAccount2 = await findOrCreateDummyRecord(models.user, { first_name: 'Bruce', - last_name: 'Wayne', + last_name: 'GETUserAccounts', }); userAccount3 = await findOrCreateDummyRecord(models.user, { first_name: 'Diana', - last_name: 'Prince', + last_name: 'GETUserAccounts', }); // Give the test users some permissions diff --git a/packages/central-server/src/tests/apiV2/userEntityPermissions/CreateUserEntityPermissions.test.js b/packages/central-server/src/tests/apiV2/userEntityPermissions/CreateUserEntityPermissions.test.js index 0ded09afc8..03d367f327 100644 --- a/packages/central-server/src/tests/apiV2/userEntityPermissions/CreateUserEntityPermissions.test.js +++ b/packages/central-server/src/tests/apiV2/userEntityPermissions/CreateUserEntityPermissions.test.js @@ -55,7 +55,7 @@ describe('Permissions checker for CreateUserEntityPermissions', async () => { // Create test users const userAccount = await findOrCreateDummyRecord(models.user, { first_name: 'Clark', - last_name: 'Kent', + last_name: 'CreateUserEntityPermissions', }); userAccountId = userAccount.id; }); diff --git a/packages/central-server/src/tests/apiV2/userEntityPermissions/DeleteUserEntityPermissions.test.js b/packages/central-server/src/tests/apiV2/userEntityPermissions/DeleteUserEntityPermissions.test.js index 5f63bab77a..abf3ebcb54 100644 --- a/packages/central-server/src/tests/apiV2/userEntityPermissions/DeleteUserEntityPermissions.test.js +++ b/packages/central-server/src/tests/apiV2/userEntityPermissions/DeleteUserEntityPermissions.test.js @@ -14,11 +14,11 @@ import { TestableApp } from '../../testUtilities'; describe('Permissions checker for DeleteUserEntityPermissions', async () => { const DEFAULT_POLICY = { DL: ['Public'], - KI: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin'], + KI: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin', 'Public'], SB: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Royal Australasian College of Surgeons'], - VU: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin'], - LA: ['Admin'], - TO: ['Admin'], + VU: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin', 'Public'], + LA: ['Admin', 'Public'], + TO: ['Admin', 'Public'], }; const BES_ADMIN_POLICY = { @@ -28,12 +28,16 @@ describe('Permissions checker for DeleteUserEntityPermissions', async () => { const app = new TestableApp(); const { models } = app; let vanuatuPublicPermission; + let vanuatuCatPermission; let laosPublicPermission; before(async () => { const publicPermissionGroup = await findOrCreateDummyRecord(models.permissionGroup, { name: 'Public', }); + const catPermissionGroup = await findOrCreateDummyRecord(models.permissionGroup, { + name: 'Cat', + }); const { entity: vanuatuEntity } = await findOrCreateDummyCountryEntity(models, { code: 'VU', @@ -45,7 +49,7 @@ describe('Permissions checker for DeleteUserEntityPermissions', async () => { // Create test users const userAccount = await findOrCreateDummyRecord(models.user, { first_name: 'Clark', - last_name: 'Kent', + last_name: 'DeleteUserEntityPermissions', }); // Give the test user some permissions @@ -54,6 +58,11 @@ describe('Permissions checker for DeleteUserEntityPermissions', async () => { entity_id: vanuatuEntity.id, permission_group_id: publicPermissionGroup.id, }); + vanuatuCatPermission = await findOrCreateDummyRecord(models.userEntityPermission, { + user_id: userAccount.id, + entity_id: vanuatuEntity.id, + permission_group_id: catPermissionGroup.id, + }); laosPublicPermission = await findOrCreateDummyRecord(models.userEntityPermission, { user_id: userAccount.id, entity_id: laosEntity.id, @@ -91,6 +100,17 @@ describe('Permissions checker for DeleteUserEntityPermissions', async () => { expect(result).to.have.keys('error'); expect(record).to.not.equal(null); }); + + it('Throw an exception if we do not have permissions for the permission of the user entity permission', async () => { + await app.grantAccess(DEFAULT_POLICY); + const { body: result } = await app.delete( + `userEntityPermissions/${vanuatuCatPermission.id}`, + ); + const record = await models.userEntityPermission.findById(vanuatuCatPermission.id); + + expect(result).to.have.keys('error'); + expect(record).to.not.equal(null); + }); }); describe('Sufficient permissions', async () => { diff --git a/packages/central-server/src/tests/apiV2/userEntityPermissions/EditUserEntityPermissions.test.js b/packages/central-server/src/tests/apiV2/userEntityPermissions/EditUserEntityPermissions.test.js index 9b211fff05..9172e14970 100644 --- a/packages/central-server/src/tests/apiV2/userEntityPermissions/EditUserEntityPermissions.test.js +++ b/packages/central-server/src/tests/apiV2/userEntityPermissions/EditUserEntityPermissions.test.js @@ -14,11 +14,11 @@ import { TestableApp } from '../../testUtilities'; describe('Permissions checker for EditUserEntityPermissions', async () => { const DEFAULT_POLICY = { DL: ['Public'], - KI: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin'], + KI: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin', 'Public'], SB: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Royal Australasian College of Surgeons'], - VU: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin'], - LA: ['Admin'], - TO: ['Admin'], + VU: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin', 'Public'], + LA: ['Admin', 'Public'], + TO: ['Admin', 'Public'], }; const BES_ADMIN_POLICY = { @@ -55,11 +55,11 @@ describe('Permissions checker for EditUserEntityPermissions', async () => { // Create test users const userAccount = await findOrCreateDummyRecord(models.user, { first_name: 'Clark', - last_name: 'Kent', + last_name: 'EditUserEntityPermissions', }); const userAccount2 = await findOrCreateDummyRecord(models.user, { first_name: 'Bruce', - last_name: 'Wayne', + last_name: 'EditUserEntityPermissions', }); userAccountId2 = userAccount2.id; diff --git a/packages/central-server/src/tests/apiV2/userEntityPermissions/GETUserEntityPermissions.test.js b/packages/central-server/src/tests/apiV2/userEntityPermissions/GETUserEntityPermissions.test.js index 1302ff2996..ad7e8e7d6f 100644 --- a/packages/central-server/src/tests/apiV2/userEntityPermissions/GETUserEntityPermissions.test.js +++ b/packages/central-server/src/tests/apiV2/userEntityPermissions/GETUserEntityPermissions.test.js @@ -14,15 +14,14 @@ import { TestableApp } from '../../testUtilities'; describe('Permissions checker for GETUserEntityPermissions', async () => { const DEFAULT_POLICY = { DL: ['Public'], - KI: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin'], - SB: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Royal Australasian College of Surgeons'], - VU: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin'], - LA: ['Admin'], - TO: ['Admin'], + KI: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin', 'Public'], + VU: [TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, 'Admin', 'Public'], + LA: ['Admin', 'Public'], + TO: ['Admin', 'Public'], }; const BES_ADMIN_POLICY = { - SB: [BES_ADMIN_PERMISSION_GROUP], + DL: [BES_ADMIN_PERMISSION_GROUP], }; const app = new TestableApp(); @@ -53,7 +52,7 @@ describe('Permissions checker for GETUserEntityPermissions', async () => { // Create test users const userAccount = await findOrCreateDummyRecord(models.user, { first_name: 'Clark', - last_name: 'Kent', + last_name: 'GETUserEntityPermissions', }); // Give the test user some permissions diff --git a/packages/database/src/migrations/20240122235629-EnforceNotNullOnUserEntityPermissionFKEYs-modifies-schema.js b/packages/database/src/migrations/20240122235629-EnforceNotNullOnUserEntityPermissionFKEYs-modifies-schema.js new file mode 100644 index 0000000000..df38a9c3ed --- /dev/null +++ b/packages/database/src/migrations/20240122235629-EnforceNotNullOnUserEntityPermissionFKEYs-modifies-schema.js @@ -0,0 +1,42 @@ +'use strict'; + +var dbm; +var type; +var seed; + +/** + * We receive the dbmigrate dependency from dbmigrate initially. + * This enables us to not have to rely on NODE_PATH. + */ +exports.setup = function (options, seedLink) { + dbm = options.dbmigrate; + type = dbm.dataType; + seed = seedLink; +}; + +exports.up = async function (db) { + await db.runSql(` + DELETE FROM user_entity_permission + WHERE + user_id IS NULL + OR entity_id IS NULL + OR permission_group_id IS NULL; + `); + return db.runSql(` + ALTER TABLE user_entity_permission ALTER COLUMN user_id SET NOT NULL; + ALTER TABLE user_entity_permission ALTER COLUMN entity_id SET NOT NULL; + ALTER TABLE user_entity_permission ALTER COLUMN permission_group_id SET NOT NULL; + `); +}; + +exports.down = function (db) { + return db.runSql(` + ALTER TABLE user_entity_permission ALTER COLUMN user_id DROP NOT NULL; + ALTER TABLE user_entity_permission ALTER COLUMN entity_id DROP NOT NULL; + ALTER TABLE user_entity_permission ALTER COLUMN permission_group_id DROP NOT NULL; +`); +}; + +exports._meta = { + version: 1, +}; diff --git a/packages/lesmis/src/api/mutations/useRegisterUser.js b/packages/lesmis/src/api/mutations/useRegisterUser.js index aebac59c83..8a6305b0ec 100644 --- a/packages/lesmis/src/api/mutations/useRegisterUser.js +++ b/packages/lesmis/src/api/mutations/useRegisterUser.js @@ -30,8 +30,6 @@ export const useRegisterUser = () => { passwordConfirm, deviceName: window.navigator.userAgent, primaryPlatform: 'lesmis', - permissionGroupName: 'LESMIS Public', - countryName: 'Laos', }, }), ); diff --git a/packages/server-boilerplate/src/microService/api/ApiBuilder.ts b/packages/server-boilerplate/src/microService/api/ApiBuilder.ts index da05987170..b541dd6102 100644 --- a/packages/server-boilerplate/src/microService/api/ApiBuilder.ts +++ b/packages/server-boilerplate/src/microService/api/ApiBuilder.ts @@ -103,7 +103,7 @@ export class ApiBuilder { } public async initialiseApiClient( - permissions: { entityCode: string; permissionGroupName: string }[], + permissions: { entityCode: string; permissionGroupName: string }[] = [], ) { await initialiseApiClient(this.models, permissions); return this; diff --git a/packages/server-boilerplate/src/orchestrator/api/ApiBuilder.ts b/packages/server-boilerplate/src/orchestrator/api/ApiBuilder.ts index 9990d3baec..be5220711b 100644 --- a/packages/server-boilerplate/src/orchestrator/api/ApiBuilder.ts +++ b/packages/server-boilerplate/src/orchestrator/api/ApiBuilder.ts @@ -199,7 +199,7 @@ export class ApiBuilder { } public async initialiseApiClient( - permissions: { entityCode: string; permissionGroupName: string }[], + permissions: { entityCode: string; permissionGroupName: string }[] = [], ) { await initialiseApiClient(this.models, permissions); return this; diff --git a/packages/server-boilerplate/src/utils/initialiseApiClient.ts b/packages/server-boilerplate/src/utils/initialiseApiClient.ts index bff099209f..0e5bf859c0 100644 --- a/packages/server-boilerplate/src/utils/initialiseApiClient.ts +++ b/packages/server-boilerplate/src/utils/initialiseApiClient.ts @@ -5,6 +5,7 @@ import winston from 'winston'; import { generateId } from '@tupaia/database'; +import { isNotNullish } from '@tupaia/tsutils'; import { requireEnv } from '@tupaia/utils'; import { encryptPassword } from '@tupaia/auth'; import { ServerBoilerplateModelRegistry } from '../types'; @@ -110,14 +111,20 @@ const upsertPermissions = async ({ permissionGroups.map(permissionGroup => [permissionGroup.name, permissionGroup.id]), ); - await models.userEntityPermission.createMany( - permissions.map(p => ({ + const userEntityPermissions = permissions + .map(p => ({ id: generateId(), user_id: userAccountId, entity_id: entityIdByCode[p.entityCode], permission_group_id: permissionGroupIdByName[p.permissionGroupName], - })), - ); + })) + // Filtering out invalid user entity permissions so that this will pass during the tests + .filter( + ({ entity_id, permission_group_id }) => + isNotNullish(entity_id) && isNotNullish(permission_group_id), + ); + + await models.userEntityPermission.createMany(userEntityPermissions); }; /** diff --git a/packages/types/src/schemas/schemas.ts b/packages/types/src/schemas/schemas.ts index d5922a2437..831417d3c1 100644 --- a/packages/types/src/schemas/schemas.ts +++ b/packages/types/src/schemas/schemas.ts @@ -58826,7 +58826,10 @@ export const UserEntityPermissionSchema = { }, "additionalProperties": false, "required": [ - "id" + "entity_id", + "id", + "permission_group_id", + "user_id" ] } @@ -58843,7 +58846,12 @@ export const UserEntityPermissionCreateSchema = { "type": "string" } }, - "additionalProperties": false + "additionalProperties": false, + "required": [ + "entity_id", + "permission_group_id", + "user_id" + ] } export const UserEntityPermissionUpdateSchema = { diff --git a/packages/types/src/types/models.ts b/packages/types/src/types/models.ts index d6394fb24c..a74456748b 100644 --- a/packages/types/src/types/models.ts +++ b/packages/types/src/types/models.ts @@ -1635,21 +1635,21 @@ export interface UserAccountUpdate { 'verified_email'?: VerifiedEmail | null; } export interface UserEntityPermission { - 'entity_id'?: string | null; + 'entity_id': string; 'id': string; - 'permission_group_id'?: string | null; - 'user_id'?: string | null; + 'permission_group_id': string; + 'user_id': string; } export interface UserEntityPermissionCreate { - 'entity_id'?: string | null; - 'permission_group_id'?: string | null; - 'user_id'?: string | null; + 'entity_id': string; + 'permission_group_id': string; + 'user_id': string; } export interface UserEntityPermissionUpdate { - 'entity_id'?: string | null; + 'entity_id'?: string; 'id'?: string; - 'permission_group_id'?: string | null; - 'user_id'?: string | null; + 'permission_group_id'?: string; + 'user_id'?: string; } export interface UserFavouriteDashboardItem { 'dashboard_item_id': string;