From e9a939c02b897db22f98e8a80578ae6feb3e4904 Mon Sep 17 00:00:00 2001 From: SanttuA Date: Mon, 6 May 2024 14:56:36 +0300 Subject: [PATCH] Better overnight calendar permission checks (#321) Changed min period rule to only be skippable by unit admins and superusers. Changed other staff checks to check user to have at least unit manager instead of any unit permission. --- .../overnight-calendar/OvernightCalendar.js | 27 +++++---- .../OvernightCalendarSelector.js | 7 ++- .../tests/OvernightCalendar.spec.js | 3 +- .../tests/OvernightCalendarSelector.spec.js | 8 ++- app/state/selectors/authSelectors.js | 18 +++++- app/state/selectors/authSelectors.spec.js | 59 +++++++++++++++++++ app/utils/__tests__/resourceUtils.spec.js | 26 ++++++++ app/utils/resourceUtils.js | 32 ++++++++++ 8 files changed, 164 insertions(+), 16 deletions(-) diff --git a/app/shared/overnight-calendar/OvernightCalendar.js b/app/shared/overnight-calendar/OvernightCalendar.js index 5a20466a9..52a311ec2 100644 --- a/app/shared/overnight-calendar/OvernightCalendar.js +++ b/app/shared/overnight-calendar/OvernightCalendar.js @@ -38,10 +38,10 @@ import { hasMaxReservations } from '../../utils/resourceUtils'; import OvernightHiddenHeading from './OvernightHiddenHeading'; function OvernightCalendar({ - currentLanguage, resource, t, selected, actions, isStaff, + currentLanguage, resource, t, selected, actions, history, isLoggedIn, isStrongAuthSatisfied, isMaintenanceModeOn, reservationId, onEditCancel, onEditConfirm, handleDateChange, selectedDate, - isSuperuser, + isSuperuser, isResourceAdmin, isResourceManager, }) { if (!resource || !resource.reservations) { return null; @@ -70,10 +70,11 @@ function OvernightCalendar({ overnightStartTime, overnightEndTime, maxPeriod, minPeriod } = resource; - const hasAdminBypass = isSuperuser || isStaff; + const isUnitAdminOrHigher = isSuperuser || isResourceAdmin; + const isUnitManagerOrHigher = isSuperuser || isResourceAdmin || isResourceManager; useEffect(() => { - if (!hasAdminBypass && startDate && endDate && !datesSameAsInitial) { + if (!isUnitAdminOrHigher && startDate && endDate && !datesSameAsInitial) { const selectedDuration = getSelectedDuration( startDate, endDate, overnightStartTime, overnightEndTime); const isDurBelowMin = isDurationBelowMin(selectedDuration, minPeriod); @@ -98,7 +99,11 @@ function OvernightCalendar({ const now = moment(); const reservingIsAllowed = isReservingAllowed({ - isLoggedIn, isStrongAuthSatisfied, isMaintenanceModeOn, resource, hasAdminBypass + isLoggedIn, + isStrongAuthSatisfied, + isMaintenanceModeOn, + resource, + hasAdminBypass: isUnitManagerOrHigher }); const validateAndSelect = (day, { booked, nextBooked, nextClosed }) => { @@ -116,7 +121,7 @@ function OvernightCalendar({ minPeriod, overnightEndTime, overnightStartTime, - hasAdminBypass, + hasAdminBypass: isUnitManagerOrHigher, }); if (!reservingIsAllowed) { @@ -155,7 +160,7 @@ function OvernightCalendar({ const isEditing = !!initialStart; const handleSelectDatetimes = () => { - if (!hasAdminBypass && hasMaxReservations(resource) && !isEditing) { + if (!isUnitManagerOrHigher && hasMaxReservations(resource) && !isEditing) { actions.addNotification({ message: t('TimeSlots.maxReservationsPerUser'), type: 'error', @@ -176,7 +181,8 @@ function OvernightCalendar({ const selectedDuration = getSelectedDuration( startDate, endDate, overnightStartTime, overnightEndTime); - const isDurBelowMin = hasAdminBypass ? false : isDurationBelowMin(selectedDuration, minPeriod); + const isDurBelowMin = isUnitAdminOrHigher ? false + : isDurationBelowMin(selectedDuration, minPeriod); return (
@@ -199,7 +205,7 @@ function OvernightCalendar({ minPeriod, overnightEndTime, overnightStartTime, - hasAdminBypass, + hasAdminBypass: isUnitManagerOrHigher, })} enableOutsideDays firstDayOfWeek={1} @@ -271,7 +277,8 @@ OvernightCalendar.propTypes = { selected: PropTypes.array.isRequired, actions: PropTypes.object.isRequired, history: PropTypes.object.isRequired, - isStaff: PropTypes.bool.isRequired, + isResourceAdmin: PropTypes.bool.isRequired, + isResourceManager: PropTypes.bool.isRequired, isSuperuser: PropTypes.bool.isRequired, isLoggedIn: PropTypes.bool.isRequired, isStrongAuthSatisfied: PropTypes.bool.isRequired, diff --git a/app/shared/overnight-calendar/OvernightCalendarSelector.js b/app/shared/overnight-calendar/OvernightCalendarSelector.js index b03c46442..fb8542dc1 100644 --- a/app/shared/overnight-calendar/OvernightCalendarSelector.js +++ b/app/shared/overnight-calendar/OvernightCalendarSelector.js @@ -2,7 +2,9 @@ import { createStructuredSelector } from 'reselect'; import { currentLanguageSelector } from 'state/selectors/translationSelectors'; import { - isLoggedInSelector, createIsStaffSelector, isSuperUserSelector + isLoggedInSelector, isSuperUserSelector, + createIsAdminForResourceSelector, + createIsManagerForResourceSelector } from 'state/selectors/authSelectors'; import { createResourceSelector, createStrongAuthSatisfiedSelector } from 'state/selectors/dataSelectors'; @@ -17,7 +19,8 @@ const OvernightCalendarSelector = createStructuredSelector({ isStrongAuthSatisfied: createStrongAuthSatisfiedSelector(resourceSelector), currentLanguage: currentLanguageSelector, isLoggedIn: isLoggedInSelector, - isStaff: createIsStaffSelector(resourceSelector), + isResourceAdmin: createIsAdminForResourceSelector(resourceSelector), + isResourceManager: createIsManagerForResourceSelector(resourceSelector), isSuperuser: isSuperUserSelector, }); diff --git a/app/shared/overnight-calendar/tests/OvernightCalendar.spec.js b/app/shared/overnight-calendar/tests/OvernightCalendar.spec.js index 8df02987e..0b44d4f19 100644 --- a/app/shared/overnight-calendar/tests/OvernightCalendar.spec.js +++ b/app/shared/overnight-calendar/tests/OvernightCalendar.spec.js @@ -28,7 +28,8 @@ describe('app/shared/overnight-calendar/OvernightCalendar', () => { addNotification: () => {}, }, history: {}, - isStaff: false, + isResourceAdmin: false, + isResourceManager: false, isSuperuser: false, isLoggedIn: false, isStrongAuthSatisfied: false, diff --git a/app/shared/overnight-calendar/tests/OvernightCalendarSelector.spec.js b/app/shared/overnight-calendar/tests/OvernightCalendarSelector.spec.js index 86ac2be62..6a7571530 100644 --- a/app/shared/overnight-calendar/tests/OvernightCalendarSelector.spec.js +++ b/app/shared/overnight-calendar/tests/OvernightCalendarSelector.spec.js @@ -35,8 +35,12 @@ describe('app/shared/overnight-calendar/OvernightCalendarSelector', () => { expect(getSelected().currentLanguage).toBeDefined(); }); - test('returns isStaff', () => { - expect(getSelected().isStaff).toBeDefined(); + test('returns isResourceAdmin', () => { + expect(getSelected().isResourceAdmin).toBeDefined(); + }); + + test('returns isResourceManager', () => { + expect(getSelected().isResourceManager).toBeDefined(); }); test('returns isSuperUserSelector', () => { diff --git a/app/state/selectors/authSelectors.js b/app/state/selectors/authSelectors.js index 819b18abb..95ea0c354 100644 --- a/app/state/selectors/authSelectors.js +++ b/app/state/selectors/authSelectors.js @@ -2,7 +2,7 @@ import forIn from 'lodash/forIn'; import includes from 'lodash/includes'; import { createSelector } from 'reselect'; -import { isStaffForResource } from 'utils/resourceUtils'; +import { isStaffForResource, isAdminForResource, isManagerForResource } from 'utils/resourceUtils'; const authUserSelector = state => state.auth.user; const usersSelector = state => state.data.users; @@ -83,6 +83,20 @@ function createIsStaffSelector(resourceSelector) { ); } +function createIsAdminForResourceSelector(resourceSelector) { + return createSelector( + resourceSelector, + (resource) => isAdminForResource(resource) + ); +} + +function createIsManagerForResourceSelector(resourceSelector) { + return createSelector( + resourceSelector, + (resource) => isManagerForResource(resource) + ); +} + const authUserAmrSelector = createSelector( authUserSelector, (user) => { @@ -106,4 +120,6 @@ export { staffUnitsSelector, isManagerForSelector, hasStrongAuthSelector, + createIsAdminForResourceSelector, + createIsManagerForResourceSelector, }; diff --git a/app/state/selectors/authSelectors.spec.js b/app/state/selectors/authSelectors.spec.js index 10c630c56..1c596f4b0 100644 --- a/app/state/selectors/authSelectors.spec.js +++ b/app/state/selectors/authSelectors.spec.js @@ -12,7 +12,10 @@ import { loginExpiresAtSelector, hasStrongAuthSelector, authUserAmrSelector, + createIsAdminForResourceSelector, + createIsManagerForResourceSelector, } from './authSelectors'; +import Resource from '../../utils/fixtures/Resource'; describe('state/selectors/authSelectors', () => { describe('authUserSelector', () => { @@ -285,6 +288,62 @@ describe('state/selectors/authSelectors', () => { ); }); + describe('createIsAdminForResourceSelector', () => { + test('returns true if user has unit admin perm for resource', () => { + const resource = Resource.build({ + userPermissions: { + isAdmin: true, + }, + }); + const resourceSelector = () => resource; + const state = getState({ + 'data.resources': { [resource.id]: resource } + }); + expect(createIsAdminForResourceSelector(resourceSelector)(state)).toEqual(true); + }); + + test('returns false if user doesnt not have unit admin perm for resource', () => { + const resource = Resource.build({ + userPermissions: { + isAdmin: false, + }, + }); + const resourceSelector = () => resource; + const state = getState({ + 'data.resources': { [resource.id]: resource } + }); + expect(createIsAdminForResourceSelector(resourceSelector)(state)).toEqual(false); + }); + }); + + describe('createIsManagerForResourceSelector', () => { + test('returns true if user has unit manager perm for resource', () => { + const resource = Resource.build({ + userPermissions: { + isManager: true, + }, + }); + const resourceSelector = () => resource; + const state = getState({ + 'data.resources': { [resource.id]: resource } + }); + expect(createIsManagerForResourceSelector(resourceSelector)(state)).toEqual(true); + }); + + test('returns false if user doesnt not have unit manager perm for resource', () => { + const resource = Resource.build({ + userPermissions: { + isManager: false, + }, + }); + const resourceSelector = () => resource; + const state = getState({ + 'data.resources': { [resource.id]: resource } + }); + expect(createIsManagerForResourceSelector(resourceSelector)(state)).toEqual(false); + }); + }); + describe('authUserAmrSelector', () => { test('returns user profile amr value if it exists', () => { const amr = 'test-amr-1'; diff --git a/app/utils/__tests__/resourceUtils.spec.js b/app/utils/__tests__/resourceUtils.spec.js index cb782f555..a0edada3f 100644 --- a/app/utils/__tests__/resourceUtils.spec.js +++ b/app/utils/__tests__/resourceUtils.spec.js @@ -26,6 +26,8 @@ import { getEquipment, isStaffForResource, isStrongAuthSatisfied, + isAdminForResource, + isManagerForResource } from 'utils/resourceUtils'; import { getPrettifiedPeriodUnits } from '../timeUtils'; import Product from '../fixtures/Product'; @@ -1149,6 +1151,30 @@ describe('Utils: resourceUtils', () => { }); }); + describe('isAdminForResource', () => { + test('returns true when userPermissions is admin is true', () => { + const resource = Resource.build({ userPermissions: { isAdmin: true } }); + expect(isAdminForResource(resource)).toBe(true); + }); + + test('returns false when userPermissions is admin is false', () => { + const resource = Resource.build({ userPermissions: { isAdmin: false } }); + expect(isAdminForResource(resource)).toBe(false); + }); + }); + + describe('isManagerForResource', () => { + test('returns true when userPermissions is admin is true', () => { + const resource = Resource.build({ userPermissions: { isManager: true } }); + expect(isManagerForResource(resource)).toBe(true); + }); + + test('returns false when userPermissions is admin is false', () => { + const resource = Resource.build({ userPermissions: { isManager: false } }); + expect(isManagerForResource(resource)).toBe(false); + }); + }); + describe('isStrongAuthSatisfied', () => { describe('when resource requires strong auth', () => { const resource = Resource.build({ authentication: 'strong' }); diff --git a/app/utils/resourceUtils.js b/app/utils/resourceUtils.js index 5f57a30b9..7179bef49 100644 --- a/app/utils/resourceUtils.js +++ b/app/utils/resourceUtils.js @@ -308,6 +308,36 @@ function isStaffForResource(resource) { return false; } +/** + * Check whether current user is admin for given resource. + * @param {Object} resource + * @returns {boolean} true when user is admin for given resource, false if not. + */ +function isAdminForResource(resource) { + if (resource.userPermissions) { + const perms = resource.userPermissions; + if (perms.isAdmin) { + return true; + } + } + return false; +} + +/** + * Check whether current user is manager for given resource. + * @param {Object} resource + * @returns {boolean} true when user is manager for given resource, false if not. + */ +function isManagerForResource(resource) { + if (resource.userPermissions) { + const perms = resource.userPermissions; + if (perms.isManager) { + return true; + } + } + return false; +} + /** * Checks whether strong auth requirement is satisfied with given resource and * strong auth status. @@ -341,4 +371,6 @@ export { getMinPeriodText, isStaffForResource, isStrongAuthSatisfied, + isAdminForResource, + isManagerForResource, };