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.

Sign up for GitHub

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

Enforce Stricter Validation on Access Type Edits #392

Merged
merged 3 commits into from
Feb 8, 2024
Merged

Enforce Stricter Validation on Access Type Edits #392

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c3d985f
conditionals to disallow self demotion/admin edits
nik-dange Jan 28, 2024
7410ef9
unit tests and linting
nik-dange Jan 30, 2024
5d72c43
added ForbiddenError
nik-dange Jan 30, 2024
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
18 changes: 14 additions & 4 deletions services/UserAccountService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,13 +225,23 @@ export default class UserAccountService {
return map;
}, {});

const updatedUsers = await Promise.all(accessUpdates.map(async (accessUpdate, index) => {
const updatedUsers = await Promise.all(accessUpdates.map(async (accessUpdate) => {
const { user: userEmail, accessType } = accessUpdate;

const currUser = emailToUserMap[userEmail];
const oldAccess = currUser.accessType;
// Prevent a user from demoting themselves
if (currentUser.email === userEmail) {
throw new BadRequestError('Cannot alter own access level');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be a ForbiddenError but I'll get around to changing it in a bit

}

const updatedUser = await userRepository.upsertUser(currUser, { accessType });
const userToUpdate = emailToUserMap[userEmail];
const oldAccess = userToUpdate.accessType;

// Prevent users from promoting to admin or demoting from admin
if (oldAccess === 'ADMIN' || accessType === 'ADMIN') {
throw new BadRequestError('Cannot alter access level of admin users');
}

const updatedUser = await userRepository.upsertUser(userToUpdate, { accessType });

const activity = {
user: currentUser,
Expand Down
78 changes: 66 additions & 12 deletions tests/admin.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,33 +302,50 @@ describe('updating user access level', () => {
expect(updatedUsers.accessType).toEqual(UserAccessType.STAFF);
});

test('admin ability to demote another admin', async () => {
test('ensure that admins cannot demote/promote other admins', async () => {
const conn = await DatabaseConnection.get();
const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN });

const secondAdmin = UserFactory.fake({ accessType: UserAccessType.ADMIN });
const regularUser = UserFactory.fake({ accessType: UserAccessType.STANDARD });

await new PortalState()
.createUsers(admin, secondAdmin)
.createUsers(admin, secondAdmin, regularUser)
.write();

const adminController = ControllerFactory.admin(conn);

const accessLevelResponse = await adminController.updateUserAccessLevel({
accessUpdates: [
{ user: secondAdmin.email, accessType: UserAccessType.MERCH_STORE_MANAGER },
],
}, admin);
// attempt to demote an admin to merch store manager
await expect(async () => {
await adminController.updateUserAccessLevel({
accessUpdates: [
{ user: secondAdmin.email, accessType: UserAccessType.MERCH_STORE_MANAGER },
],
}, admin);
}).rejects.toThrow(BadRequestError);

const repository = conn.getRepository(UserModel);
const updatedUser = await repository.findOne({ email: secondAdmin.email });
const secondAdminFromDatabase = await repository.findOne({ email: secondAdmin.email });

expect(updatedUser.email).toEqual(secondAdmin.email);
expect(updatedUser.accessType).toEqual(UserAccessType.MERCH_STORE_MANAGER);
expect(accessLevelResponse.updatedUsers[0].accessType).toEqual(UserAccessType.MERCH_STORE_MANAGER);
expect(secondAdminFromDatabase.email).toEqual(secondAdmin.email);
expect(secondAdminFromDatabase.accessType).toEqual(UserAccessType.ADMIN);

// attempt to promote a regular user to admin
await expect(async () => {
await adminController.updateUserAccessLevel({
accessUpdates: [
{ user: regularUser.email, accessType: UserAccessType.ADMIN },
],
}, admin);
}).rejects.toThrow(BadRequestError);

const regularUserFromDatabase = await repository.findOne({ email: regularUser.email });

expect(regularUserFromDatabase.email).toEqual(regularUser.email);
expect(regularUserFromDatabase.accessType).toEqual(UserAccessType.STANDARD);
});

test("ensure that the updating user's access level is not changed", async () => {
test("ensure that the updating user's access level is not changed & cannot demote themselves", async () => {
const conn = await DatabaseConnection.get();
const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN });

Expand Down Expand Up @@ -360,4 +377,41 @@ describe('updating user access level', () => {
expect(existingAdmin[0].email).toEqual(admin.email);
expect(existingAdmin[0].accessType).toEqual(UserAccessType.ADMIN);
});

test('ensure that a user cannot demote themselves', async () => {
const conn = await DatabaseConnection.get();
const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN });

const staffUser = UserFactory.fake({ accessType: UserAccessType.STAFF });
const standardUser = UserFactory.fake({ accessType: UserAccessType.STANDARD });
const marketingUser = UserFactory.fake({ accessType: UserAccessType.MARKETING });
const merchStoreDistributorUser = UserFactory.fake({ accessType: UserAccessType.MERCH_STORE_DISTRIBUTOR });

await new PortalState()
.createUsers(staffUser, standardUser, marketingUser, merchStoreDistributorUser, admin)
.write();

const adminController = ControllerFactory.admin(conn);

// attempt to demote oneself
await expect(async () => {
await adminController.updateUserAccessLevel({
accessUpdates: [
{ user: staffUser.email, accessType: UserAccessType.MERCH_STORE_MANAGER },
{ user: standardUser.email, accessType: UserAccessType.MARKETING },
{ user: marketingUser.email, accessType: UserAccessType.MERCH_STORE_DISTRIBUTOR },
{ user: merchStoreDistributorUser.email, accessType: UserAccessType.STAFF },
{ user: admin.email, accessType: UserAccessType.STANDARD },
],
}, admin);
}).rejects.toThrow(BadRequestError);

const repository = conn.getRepository(UserModel);
const existingAdmin = await repository.find({
email: admin.email,
});

expect(existingAdmin[0].email).toEqual(admin.email);
expect(existingAdmin[0].accessType).toEqual(UserAccessType.ADMIN);
});
});
Loading