Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better overnight calendar permission checks #321

Merged
merged 2 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions app/shared/overnight-calendar/OvernightCalendar.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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 }) => {
Expand All @@ -116,7 +121,7 @@ function OvernightCalendar({
minPeriod,
overnightEndTime,
overnightStartTime,
hasAdminBypass,
hasAdminBypass: isUnitManagerOrHigher,
});

if (!reservingIsAllowed) {
Expand Down Expand Up @@ -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',
Expand All @@ -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 (
<div className="overnight-calendar">
Expand All @@ -199,7 +205,7 @@ function OvernightCalendar({
minPeriod,
overnightEndTime,
overnightStartTime,
hasAdminBypass,
hasAdminBypass: isUnitManagerOrHigher,
})}
enableOutsideDays
firstDayOfWeek={1}
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 5 additions & 2 deletions app/shared/overnight-calendar/OvernightCalendarSelector.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ describe('app/shared/overnight-calendar/OvernightCalendar', () => {
addNotification: () => {},
},
history: {},
isStaff: false,
isResourceAdmin: false,
isResourceManager: false,
isSuperuser: false,
isLoggedIn: false,
isStrongAuthSatisfied: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
18 changes: 17 additions & 1 deletion app/state/selectors/authSelectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) => {
Expand All @@ -106,4 +120,6 @@ export {
staffUnitsSelector,
isManagerForSelector,
hasStrongAuthSelector,
createIsAdminForResourceSelector,
createIsManagerForResourceSelector,
};
59 changes: 59 additions & 0 deletions app/state/selectors/authSelectors.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ import {
loginExpiresAtSelector,
hasStrongAuthSelector,
authUserAmrSelector,
createIsAdminForResourceSelector,
createIsManagerForResourceSelector,
} from './authSelectors';
import Resource from '../../utils/fixtures/Resource';

describe('state/selectors/authSelectors', () => {
describe('authUserSelector', () => {
Expand Down Expand Up @@ -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';
Expand Down
26 changes: 26 additions & 0 deletions app/utils/__tests__/resourceUtils.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import {
getEquipment,
isStaffForResource,
isStrongAuthSatisfied,
isAdminForResource,
isManagerForResource
} from 'utils/resourceUtils';
import { getPrettifiedPeriodUnits } from '../timeUtils';
import Product from '../fixtures/Product';
Expand Down Expand Up @@ -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' });
Expand Down
32 changes: 32 additions & 0 deletions app/utils/resourceUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -341,4 +371,6 @@ export {
getMinPeriodText,
isStaffForResource,
isStrongAuthSatisfied,
isAdminForResource,
isManagerForResource,
};
Loading