From 3e54616d79eff0a43e0f8f682834e4dd028166ad Mon Sep 17 00:00:00 2001 From: euanmillar Date: Fri, 15 Nov 2024 08:57:59 +0000 Subject: [PATCH 1/4] Stop sys admins from de-activating themselves and stop local sys admins from creating national level users --- .../src/views/SysAdmin/Team/user/UserList.tsx | 13 +++++-- .../client/src/views/SysAdmin/Team/utils.ts | 5 +++ .../client/src/views/UserAudit/UserAudit.tsx | 11 +++++- .../src/features/createUser/handler.ts | 11 +++++- .../src/features/getRoles/handler.ts | 11 ++---- packages/user-mgnt/src/model/systemRole.ts | 25 +++++++++++++ packages/user-mgnt/src/utils/userUtils.ts | 37 +++++++++++++++++++ 7 files changed, 98 insertions(+), 15 deletions(-) diff --git a/packages/client/src/views/SysAdmin/Team/user/UserList.tsx b/packages/client/src/views/SysAdmin/Team/user/UserList.tsx index 34a7659c9ab..e48985f71b1 100644 --- a/packages/client/src/views/SysAdmin/Team/user/UserList.tsx +++ b/packages/client/src/views/SysAdmin/Team/user/UserList.tsx @@ -39,7 +39,8 @@ import { SysAdminContentWrapper } from '@client/views/SysAdmin/SysAdminContentWr import { getAddressName, getUserRoleIntlKey, - UserStatus + UserStatus, + canDeactivateUser } from '@client/views/SysAdmin/Team/utils' import { LinkButton } from '@opencrvs/components/lib/buttons' import { Button } from '@opencrvs/components/lib/Button' @@ -396,7 +397,7 @@ function UserListComponent(props: IProps) { ) const getMenuItems = useCallback( - function getMenuItems(user: User) { + function getMenuItems(user: User, userDetails: UserDetails | null) { const menuItems = [ { label: intl.formatMessage(messages.editUserDetailsTitle), @@ -432,7 +433,11 @@ function UserListComponent(props: IProps) { }) } - if (user.status === 'active') { + if ( + userDetails && + user.status === 'active' && + canDeactivateUser(user.id, userDetails) + ) { menuItems.push({ label: intl.formatMessage(messages.deactivate), handler: () => toggleUserActivationModal(user) @@ -530,7 +535,7 @@ function UserListComponent(props: IProps) { toggleButton={ } - menuItems={getMenuItems(user)} + menuItems={getMenuItems(user, userDetails)} /> )} diff --git a/packages/client/src/views/SysAdmin/Team/utils.ts b/packages/client/src/views/SysAdmin/Team/utils.ts index 19f647bb920..c02e5ac0556 100644 --- a/packages/client/src/views/SysAdmin/Team/utils.ts +++ b/packages/client/src/views/SysAdmin/Team/utils.ts @@ -13,6 +13,7 @@ import { IntlShape, MessageDescriptor } from 'react-intl' import { messages } from '@client/i18n/messages/views/userSetup' import { SystemRoleType } from '@client/utils/gateway' import { ILocation, IOfflineData } from '@client/offline/reducer' +import { UserDetails } from '@client/utils/userUtils' export enum UserStatus { ACTIVE, @@ -112,3 +113,7 @@ export function getUserSystemRole( export const getUserRoleIntlKey = (_roleId: string) => { return `role.${_roleId}` } + +export const canDeactivateUser = (id: string, userDetails: UserDetails) => { + return id !== userDetails.id ? true : false +} diff --git a/packages/client/src/views/UserAudit/UserAudit.tsx b/packages/client/src/views/UserAudit/UserAudit.tsx index 680ed4cec5d..fcf6fb32d8c 100644 --- a/packages/client/src/views/UserAudit/UserAudit.tsx +++ b/packages/client/src/views/UserAudit/UserAudit.tsx @@ -23,7 +23,10 @@ import { AvatarSmall } from '@client/components/Avatar' import styled from 'styled-components' import { ToggleMenu } from '@opencrvs/components/lib/ToggleMenu' import { Button } from '@opencrvs/components/lib/Button' -import { getUserRoleIntlKey } from '@client/views/SysAdmin//Team/utils' +import { + getUserRoleIntlKey, + canDeactivateUser +} from '@client/views/SysAdmin/Team/utils' import { EMPTY_STRING, LANG_EN } from '@client/utils/constants' import { Loader } from '@opencrvs/components/lib/Loader' import { messages as userSetupMessages } from '@client/i18n/messages/views/userSetup' @@ -246,7 +249,11 @@ export const UserAudit = () => { ) } - if (status === 'active') { + if ( + status === 'active' && + userDetails && + canDeactivateUser(userId, userDetails) + ) { menuItems.push({ label: intl.formatMessage(sysMessages.deactivate), handler: () => toggleUserActivationModal() diff --git a/packages/user-mgnt/src/features/createUser/handler.ts b/packages/user-mgnt/src/features/createUser/handler.ts index d5e06328c7c..b6f0ac68dce 100644 --- a/packages/user-mgnt/src/features/createUser/handler.ts +++ b/packages/user-mgnt/src/features/createUser/handler.ts @@ -23,7 +23,12 @@ import { generateSaltedHash, generateRandomPassword } from '@user-mgnt/utils/hash' -import { statuses, hasDemoScope, getUserId } from '@user-mgnt/utils/userUtils' +import { + statuses, + hasDemoScope, + getUserId, + getValidRoles +} from '@user-mgnt/utils/userUtils' import { userRoleScopes } from '@opencrvs/commons/authentication' import { QA_ENV } from '@user-mgnt/constants' import * as Hapi from '@hapi/hapi' @@ -36,7 +41,9 @@ export default async function createUser( ) { const user = request.payload as IUser & { password?: string } const token = request.headers.authorization - + if (!getValidRoles({}, 'asc', 'creationDate', token, user.systemRole)) { + return h.response().code(400) + } // construct Practitioner resource and save them let practitionerId = null let roleId = null diff --git a/packages/user-mgnt/src/features/getRoles/handler.ts b/packages/user-mgnt/src/features/getRoles/handler.ts index 4068c364de5..c7a450d584a 100644 --- a/packages/user-mgnt/src/features/getRoles/handler.ts +++ b/packages/user-mgnt/src/features/getRoles/handler.ts @@ -10,9 +10,8 @@ */ import * as Hapi from '@hapi/hapi' import * as Joi from 'joi' - -import SystemRole from '@user-mgnt/model/systemRole' import { SortOrder } from 'mongoose' +import { getValidRoles } from '@user-mgnt/utils/userUtils' interface IVerifyPayload { value?: string @@ -34,6 +33,7 @@ export default async function getSystemRoles( sortOrder = 'asc' } = request.payload as IVerifyPayload let criteria = {} + const token = request.headers.authorization if (value) { criteria = { ...criteria, value } @@ -45,11 +45,8 @@ export default async function getSystemRoles( criteria = { ...criteria, active } } - return await SystemRole.find(criteria) - .populate('roles') - .sort({ - [sortBy]: sortOrder - }) + const validRoles = await getValidRoles(criteria, sortOrder, sortBy, token) + return validRoles } export const searchRoleSchema = Joi.object({ diff --git a/packages/user-mgnt/src/model/systemRole.ts b/packages/user-mgnt/src/model/systemRole.ts index ee7ea98a052..f1a0ade06b7 100644 --- a/packages/user-mgnt/src/model/systemRole.ts +++ b/packages/user-mgnt/src/model/systemRole.ts @@ -20,6 +20,31 @@ export const SYSTEM_ROLE_TYPES = [ 'REGISTRATION_AGENT' ] as const +export const sysAdminAccessMap: Map = new Map([ + [ + 'LOCAL_SYSTEM_ADMIN', + [ + 'FIELD_AGENT', + 'LOCAL_REGISTRAR', + 'LOCAL_SYSTEM_ADMIN', + 'PERFORMANCE_MANAGEMENT', + 'REGISTRATION_AGENT' + ] + ], + [ + 'NATIONAL_SYSTEM_ADMIN', + [ + 'FIELD_AGENT', + 'LOCAL_REGISTRAR', + 'LOCAL_SYSTEM_ADMIN', + 'NATIONAL_REGISTRAR', + 'NATIONAL_SYSTEM_ADMIN', + 'PERFORMANCE_MANAGEMENT', + 'REGISTRATION_AGENT' + ] + ] +]) + interface ISystemRole { value: (typeof SYSTEM_ROLE_TYPES)[number] roles: Types.ObjectId[] diff --git a/packages/user-mgnt/src/utils/userUtils.ts b/packages/user-mgnt/src/utils/userUtils.ts index 91e50a34c42..9de1399ad5f 100644 --- a/packages/user-mgnt/src/utils/userUtils.ts +++ b/packages/user-mgnt/src/utils/userUtils.ts @@ -11,6 +11,11 @@ import * as Hapi from '@hapi/hapi' import { ITokenPayload } from '@user-mgnt/utils/token' import * as decode from 'jwt-decode' +import SystemRole, { + ISystemRoleModel, + sysAdminAccessMap +} from '@user-mgnt/model/systemRole' +import { SortOrder, ObjectId } from 'mongoose' export const statuses = { PENDING: 'pending', @@ -57,3 +62,35 @@ export const getUserId = (authHeader: IAuthHeader): string => { const tokenPayload = getTokenPayload(authHeader.Authorization.split(' ')[1]) return tokenPayload.sub } + +export async function getValidRoles( + criteria = {}, + sortOrder: SortOrder = 'asc', + sortBy: string, + token: string, + systemRole?: string +): Promise[] | null> { + const scope = getTokenPayload(token).scope + const allRoles = await SystemRole.find(criteria) + .populate('roles') + .sort({ + [sortBy]: sortOrder + }) + let roleFilter = '' + if (scope.includes('natlsysadmin')) { + roleFilter = 'NATIONAL_SYSTEM_ADMIN' + } else if (scope.includes('sysadmin')) { + roleFilter = 'LOCAL_SYSTEM_ADMIN' + } else { + return null + } + const accessibleRoleValues = sysAdminAccessMap.get(roleFilter) || [] + if ( + (systemRole && accessibleRoleValues.includes(systemRole)) || + !systemRole + ) { + return allRoles.filter((role) => accessibleRoleValues.includes(role.value)) + } else { + return null + } +} From 36d715ad899e5479ba5cef7d117518b54895162e Mon Sep 17 00:00:00 2001 From: euanmillar Date: Fri, 15 Nov 2024 12:30:01 +0000 Subject: [PATCH 2/4] Test for reduced role list --- .../src/features/getRoles/handler.test.ts | 195 +++++++++++++++++- 1 file changed, 185 insertions(+), 10 deletions(-) diff --git a/packages/user-mgnt/src/features/getRoles/handler.test.ts b/packages/user-mgnt/src/features/getRoles/handler.test.ts index b3cd849a8b7..8f8e5799fff 100644 --- a/packages/user-mgnt/src/features/getRoles/handler.test.ts +++ b/packages/user-mgnt/src/features/getRoles/handler.test.ts @@ -19,7 +19,7 @@ beforeEach(async () => { server = await createServer() }) -const token = jwt.sign( +const localSysAdmintoken = jwt.sign( { scope: ['sysadmin'] }, readFileSync('./test/cert.key'), { @@ -28,7 +28,16 @@ const token = jwt.sign( audience: 'opencrvs:user-mgnt-user' } ) -const dummyRoleList = [ +const nationalSysAdmintoken = jwt.sign( + { scope: ['natlsysadmin', 'sysadmin'] }, + readFileSync('./test/cert.key'), + { + algorithm: 'RS256', + issuer: 'opencrvs:auth-service', + audience: 'opencrvs:user-mgnt-user' + } +) +const dummyRoleListNatlSysAdmin = [ { _id: '63a06b979538ca7ab52f9759', active: true, @@ -207,24 +216,169 @@ const dummyRoleList = [ } ] +const dummyRoleListLocalSysAdmin = [ + { + _id: '63a06b979538ca7ab52f9759', + active: true, + value: 'FIELD_AGENT', + creationDate: 1671457687106, + roles: [ + { + labels: [ + { + lang: 'en', + label: 'Healthcare Worker' + }, + { + lang: 'fr', + label: 'Professionnel de Santé' + } + ] + }, + { + labels: [ + { + lang: 'en', + label: 'Police Officer' + }, + { + lang: 'fr', + label: 'Agent de Police' + } + ] + }, + { + labels: [ + { + lang: 'en', + label: 'Social Worker' + }, + { + lang: 'fr', + label: 'Travailleur Social' + } + ] + }, + { + labels: [ + { + lang: 'en', + label: 'Local Leader' + }, + { + lang: 'fr', + label: 'Leader Local' + } + ] + } + ] + }, + { + _id: '63a06b979538ca7ab52f975a', + active: true, + value: 'REGISTRATION_AGENT', + creationDate: 1671457687107, + roles: [ + { + labels: [ + { + lang: 'en', + label: 'Registration Agent' + }, + { + lang: 'fr', + label: "Agent d'enregistrement" + } + ] + } + ] + }, + { + _id: '63a06b979538ca7ab52f975b', + active: true, + value: 'LOCAL_REGISTRAR', + creationDate: 1671457687107, + roles: [ + { + labels: [ + { + lang: 'en', + label: 'Local Registrar' + }, + { + lang: 'fr', + label: 'Registraire local' + } + ] + } + ] + }, + { + _id: '63a06b979538ca7ab52f975c', + active: true, + value: 'LOCAL_SYSTEM_ADMIN', + creationDate: 1671457687107, + roles: [ + { + labels: [ + { + lang: 'en', + label: 'Local System_admin' + }, + { + lang: 'fr', + label: 'Administrateur système local' + } + ] + } + ] + }, + { + _id: '63a06b979538ca7ab52f975e', + active: true, + value: 'PERFORMANCE_MANAGEMENT', + creationDate: 1671457687107, + roles: [ + { + labels: [ + { + lang: 'en', + label: 'Performance Management' + }, + { + lang: 'fr', + label: 'Gestion des performances' + } + ] + } + ] + } +] + describe('getSystemRoles tests', () => { it('Successfully returns full role list', async () => { - SystemRole.find = jest.fn().mockReturnValue(dummyRoleList) - SystemRole.find().sort = jest.fn().mockReturnValue(dummyRoleList) - SystemRole.find().populate = jest.fn().mockReturnValue(dummyRoleList) + SystemRole.find = jest.fn().mockReturnValue(dummyRoleListNatlSysAdmin) + SystemRole.find().sort = jest + .fn() + .mockReturnValue(dummyRoleListNatlSysAdmin) + SystemRole.find().populate = jest + .fn() + .mockReturnValue(dummyRoleListNatlSysAdmin) const res = await server.server.inject({ method: 'POST', url: '/getSystemRoles', payload: {}, headers: { - Authorization: `Bearer ${token}` + Authorization: `Bearer ${nationalSysAdmintoken}` } }) - expect(res.result).toEqual(dummyRoleList) + expect(JSON.stringify(res.result)).toEqual( + JSON.stringify(dummyRoleListNatlSysAdmin) + ) }) it('Successfully returns filtered user list', async () => { - const filteredList = [dummyRoleList[2]] + const filteredList = [dummyRoleListNatlSysAdmin[2]] SystemRole.find = jest.fn().mockReturnValue(filteredList) SystemRole.find().sort = jest.fn().mockReturnValue(filteredList) SystemRole.find().populate = jest.fn().mockReturnValue(filteredList) @@ -240,10 +394,31 @@ describe('getSystemRoles tests', () => { active: true }, headers: { - Authorization: `Bearer ${token}` + Authorization: `Bearer ${nationalSysAdmintoken}` } }) - expect(res.result).toEqual(filteredList) + expect(JSON.stringify(res.result)).toEqual(JSON.stringify(filteredList)) + }) + it('Successfully returns local sys admin role list minus national roles', async () => { + SystemRole.find = jest.fn().mockReturnValue(dummyRoleListNatlSysAdmin) + SystemRole.find().sort = jest + .fn() + .mockReturnValue(dummyRoleListNatlSysAdmin) + SystemRole.find().populate = jest + .fn() + .mockReturnValue(dummyRoleListNatlSysAdmin) + + const res = await server.server.inject({ + method: 'POST', + url: '/getSystemRoles', + payload: {}, + headers: { + Authorization: `Bearer ${localSysAdmintoken}` + } + }) + expect(JSON.stringify(res.result)).toEqual( + JSON.stringify(dummyRoleListLocalSysAdmin) + ) }) }) From bbab98a1850c1fb38b523f19659f81046faf2307 Mon Sep 17 00:00:00 2001 From: euanmillar Date: Fri, 15 Nov 2024 12:47:27 +0000 Subject: [PATCH 3/4] Fix createUser test and update CHANGELOG --- CHANGELOG.md | 9 ++++++++- .../user-mgnt/src/features/createUser/handler.test.ts | 10 ++++++++++ .../user-mgnt/src/features/getRoles/handler.test.ts | 2 +- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a1a3a1628c9..ff1699cff61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Breaking changes -- **Dashboard:** Changes made to the dashboard configuration will reset after upgrading OpenCRVS. +- **Dashboard:** Changes made to the dashboard configuration will reset after upgrading OpenCRVS. ## Improvements @@ -37,6 +37,13 @@ - Only show items with values in review [#5192](https://github.com/opencrvs/opencrvs-core/pull/5192) - Fix prefix text overlap issue in form text inputs +## 1.6.1 + +## Bug fixes + +- Stops local sys admins creating national level users. [#7698](https://github.com/opencrvs/opencrvs-core/issues/7698) +- Stops sys admins deactivating themselves by mistake. [#7691](https://github.com/opencrvs/opencrvs-core/issues/7691) + ## 1.6.0 Release candidate ## Improvements diff --git a/packages/user-mgnt/src/features/createUser/handler.test.ts b/packages/user-mgnt/src/features/createUser/handler.test.ts index 980f00ea2c0..269eb4a06fd 100644 --- a/packages/user-mgnt/src/features/createUser/handler.test.ts +++ b/packages/user-mgnt/src/features/createUser/handler.test.ts @@ -15,6 +15,8 @@ import * as fetchMock from 'jest-fetch-mock' import * as jwt from 'jsonwebtoken' import * as mockingoose from 'mockingoose' import { Types } from 'mongoose' +import { dummyRoleListNatlSysAdmin } from '../getRoles/handler.test' +import SystemRole from '@user-mgnt/model/systemRole' const fetch = fetchMock as fetchMock.FetchMock @@ -71,10 +73,18 @@ describe('createUser handler', () => { mockingoose.resetAll() server = await createServer() fetch.resetMocks() + SystemRole.find = jest.fn().mockReturnValue(dummyRoleListNatlSysAdmin) + SystemRole.find().sort = jest + .fn() + .mockReturnValue(dummyRoleListNatlSysAdmin) + SystemRole.find().populate = jest + .fn() + .mockReturnValue(dummyRoleListNatlSysAdmin) }) it('creates and saves fhir resources and adds user using mongoose', async () => { fetch.mockResponses( + ['', { status: 201, headers: { Location: 'Practitioner/123' } }], ['', { status: 201, headers: { Location: 'Practitioner/123' } }], ['', { status: 201, headers: { Location: 'PractitionerRole/123' } }], ['', { status: 200 }] diff --git a/packages/user-mgnt/src/features/getRoles/handler.test.ts b/packages/user-mgnt/src/features/getRoles/handler.test.ts index 8f8e5799fff..c071a2b68a7 100644 --- a/packages/user-mgnt/src/features/getRoles/handler.test.ts +++ b/packages/user-mgnt/src/features/getRoles/handler.test.ts @@ -37,7 +37,7 @@ const nationalSysAdmintoken = jwt.sign( audience: 'opencrvs:user-mgnt-user' } ) -const dummyRoleListNatlSysAdmin = [ +export const dummyRoleListNatlSysAdmin = [ { _id: '63a06b979538ca7ab52f9759', active: true, From e653b8fc0b032aaa9b76d1a6e88918e37f0e2952 Mon Sep 17 00:00:00 2001 From: euanmillar Date: Fri, 15 Nov 2024 12:50:46 +0000 Subject: [PATCH 4/4] Fix import --- packages/user-mgnt/src/features/createUser/handler.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/user-mgnt/src/features/createUser/handler.test.ts b/packages/user-mgnt/src/features/createUser/handler.test.ts index 269eb4a06fd..c568f8bf275 100644 --- a/packages/user-mgnt/src/features/createUser/handler.test.ts +++ b/packages/user-mgnt/src/features/createUser/handler.test.ts @@ -15,7 +15,7 @@ import * as fetchMock from 'jest-fetch-mock' import * as jwt from 'jsonwebtoken' import * as mockingoose from 'mockingoose' import { Types } from 'mongoose' -import { dummyRoleListNatlSysAdmin } from '../getRoles/handler.test' +import { dummyRoleListNatlSysAdmin } from '@user-mgnt/features/getRoles/handler.test' import SystemRole from '@user-mgnt/model/systemRole' const fetch = fetchMock as fetchMock.FetchMock