prevent delete user route to delete last superAdmin

Signed-off-by: Pierre Noël <petersg83@gmail.com>
This commit is contained in:
Pierre Noël 2020-06-24 16:48:53 +02:00 committed by Alexandre Bodin
parent 1d3a054a5d
commit 25bea051e0
5 changed files with 155 additions and 28 deletions

View File

@ -93,7 +93,7 @@ module.exports = {
async delete(ctx) {
const { id } = ctx.params;
const deletedUser = await strapi.admin.services.user.delete({ id });
const deletedUser = await strapi.admin.services.user.deleteById(id);
if (!deletedUser) {
return ctx.notFound('User not found');

View File

@ -2,6 +2,7 @@
const _ = require('lodash');
const userService = require('../user');
const { SUPER_ADMIN_CODE } = require('../constants');
describe('User', () => {
describe('sanitizeUser', () => {
@ -175,6 +176,45 @@ describe('User', () => {
});
});
describe('updateById', () => {
test('Cannot delete last super admin', async () => {
const findOne = jest.fn(() =>
Promise.resolve({ id: 11, roles: [{ code: SUPER_ADMIN_CODE }] })
);
const getSuperAdminWithUsersCount = jest.fn(() => Promise.resolve({ id: 1, usersCount: 1 }));
const badRequest = jest.fn();
global.strapi = {
query: () => ({ findOne }),
admin: { services: { role: { getSuperAdminWithUsersCount } } },
errors: { badRequest },
};
try {
await userService.deleteById(2);
} catch (e) {
//nothing
}
expect(badRequest).toHaveBeenCalledWith(
'ValidationError',
'You must have at least one user with super admin role.'
);
});
test('Can delete a super admin if he/she is not the last one', async () => {
const user = { id: 11, roles: [{ code: SUPER_ADMIN_CODE }] };
const findOne = jest.fn(() => Promise.resolve(user));
const getSuperAdminWithUsersCount = jest.fn(() => Promise.resolve({ id: 1, usersCount: 2 }));
const deleteFn = jest.fn(() => user);
global.strapi = {
query: () => ({ findOne, delete: deleteFn }),
admin: { services: { role: { getSuperAdminWithUsersCount } } },
};
const res = await userService.deleteById(user.id);
expect(deleteFn).toHaveBeenCalledWith({ id: user.id });
expect(res).toEqual(user);
});
});
describe('exists', () => {
test('Return true if the user already exists', async () => {
const count = jest.fn(() => Promise.resolve(1));

View File

@ -3,6 +3,7 @@
const _ = require('lodash');
const { stringIncludes, stringEquals } = require('strapi-utils');
const { createUser } = require('../domain/user');
const { SUPER_ADMIN_CODE } = require('./constants');
const sanitizeUserRoles = role => _.pick(role, ['id', 'name', 'description']);
@ -156,8 +157,24 @@ const searchPage = async query => {
* @param query
* @returns {Promise<user>}
*/
const deleteFn = async query => {
return strapi.query('user', 'admin').delete(query);
const deleteById = async id => {
// Check at least one super admin remains
const userToDelete = await strapi.query('user', 'admin').findOne({ id });
if (userToDelete) {
if (userToDelete.roles.some(r => r.code === SUPER_ADMIN_CODE)) {
const superAdminRole = await strapi.admin.services.role.getSuperAdminWithUsersCount();
if (superAdminRole.usersCount === 1) {
throw strapi.errors.badRequest(
'ValidationError',
'You must have at least one user with super admin role.'
);
}
}
} else {
return null;
}
return strapi.query('user', 'admin').delete({ id });
};
/** Count the users that don't have any associated roles
@ -230,7 +247,7 @@ module.exports = {
findOne,
findPage,
searchPage,
delete: deleteFn,
deleteById,
countUsersWithoutRole,
assignARoleToAll,
displayWarningIfUsersDontHaveRole,

View File

@ -1,8 +1,9 @@
'use strict';
const _ = require('lodash');
const { login, registerAndLogin } = require('../../../test/helpers/auth');
const { login, registerAndLogin, getUser } = require('../../../test/helpers/auth');
const { createAuthRequest } = require('../../../test/helpers/request');
const { SUPER_ADMIN_CODE } = require('../services/constants');
const omitTimestamps = obj => _.omit(obj, ['updatedAt', 'createdAt', 'updated_at', 'created_at']);
@ -36,6 +37,15 @@ const deleteUserRole = async id => {
});
};
const getSuperAdminRole = async () => {
const res = await rq({
url: '/admin/roles',
method: 'GET',
});
return res.body.data.find(r => r.code === SUPER_ADMIN_CODE);
};
let rq;
/**
@ -46,20 +56,27 @@ let rq;
* 1. Create a user (fail/body)
* 2. Create a user (success)
* 3. Update a user (success)
* 4. Update a user (fail/body)
* 5. Get a user (success)
* 6. Get a list of users (success/full)
* 7. Delete a user (success)
* 8. Delete a user (fail/notFound)
* 9. Update a user (fail/notFound)
* 10. Get a user (fail/notFound)
* 11. Get a list of users (success/empty)
* 4. Create a user with superAdmin role (success)
* 5. Update a user (fail/body)
* 6. Get a user (success)
* 7. Get a list of users (success/full)
* 8. Delete a user (success)
* 9. Delete a user (fail/notFound)
* 10. Deletes a super admin user (successfully)
* 11. Deletes last super admin user (bad request)
* 12. Update a user (fail/notFound)
* 13. Get a user (fail/notFound)
* 14. Get a list of users (success/empty)
*/
describe('Admin User CRUD (e2e)', () => {
// Local test data used across the test suite
let testData = {
firstSuperAdminUser: undefined,
user: undefined,
secondSuperAdminUser: undefined,
role: undefined,
superAdminRole: undefined,
};
// Initialization Actions
@ -67,6 +84,8 @@ describe('Admin User CRUD (e2e)', () => {
const token = await getAuthToken();
rq = createAuthRequest(token);
testData.role = await createUserRole();
testData.firstSuperAdminUser = await getUser();
testData.superAdminRole = await getSuperAdminRole();
});
// Cleanup actions
@ -119,7 +138,28 @@ describe('Admin User CRUD (e2e)', () => {
testData.user = res.body.data;
});
test('3. Updates a user (wrong body)', async () => {
test('3. Creates a user with superAdmin role (success)', async () => {
const body = {
email: 'user-tests2@strapi-e2e.com',
firstname: 'user_tests-firstname',
lastname: 'user_tests-lastname',
roles: [testData.superAdminRole.id],
};
const res = await rq({
url: '/admin/users',
method: 'POST',
body,
});
expect(res.statusCode).toBe(201);
expect(res.body.data).not.toBeNull();
// Using the created user as an example for the rest of the tests
testData.secondSuperAdminUser = res.body.data;
});
test('4. Updates a user (wrong body)', async () => {
const body = {
email: 42,
};
@ -141,7 +181,7 @@ describe('Admin User CRUD (e2e)', () => {
});
});
test('4. Updates a user (successfully)', async () => {
test('5. Updates a user (successfully)', async () => {
const body = {
firstname: 'foobar',
};
@ -163,7 +203,7 @@ describe('Admin User CRUD (e2e)', () => {
testData.user = res.body.data;
});
test('5. Finds a user (successfully)', async () => {
test('6. Finds a user (successfully)', async () => {
const res = await rq({
url: `/admin/users/${testData.user.id}`,
method: 'GET',
@ -173,7 +213,7 @@ describe('Admin User CRUD (e2e)', () => {
expect(res.body.data).toMatchObject(testData.user);
});
describe('6. Finds a list of users (contains user)', () => {
describe('7. Finds a list of users (contains user)', () => {
const expectedBodyFormat = () => ({
data: {
pagination: {
@ -186,7 +226,7 @@ describe('Admin User CRUD (e2e)', () => {
},
});
test('6.1. Using findPage', async () => {
test('7.1. Using findPage', async () => {
const res = await rq({
url: `/admin/users?email=${testData.user.email}`,
method: 'GET',
@ -197,7 +237,7 @@ describe('Admin User CRUD (e2e)', () => {
expect(res.body.data.results).toContainEqual(testData.user);
});
test('6.2. Using searchPage', async () => {
test('7.2. Using searchPage', async () => {
const res = await rq({
url: `/admin/users?_q=${testData.user.email}`,
method: 'GET',
@ -209,7 +249,7 @@ describe('Admin User CRUD (e2e)', () => {
});
});
test('7. Deletes a user (successfully)', async () => {
test('8. Deletes a user (successfully)', async () => {
const res = await rq({
url: `/admin/users/${testData.user.id}`,
method: 'DELETE',
@ -219,7 +259,7 @@ describe('Admin User CRUD (e2e)', () => {
expect(res.body.data).toMatchObject(testData.user);
});
test('8. Deletes a user (not found)', async () => {
test('9. Deletes a user (not found)', async () => {
const res = await rq({
url: `/admin/users/${testData.user.id}`,
method: 'DELETE',
@ -228,7 +268,32 @@ describe('Admin User CRUD (e2e)', () => {
expect(res.statusCode).toBe(404);
});
test('9. Updates a user (not found)', async () => {
test('10. Deletes a super admin user (successfully)', async () => {
const res = await rq({
url: `/admin/users/${testData.secondSuperAdminUser.id}`,
method: 'DELETE',
});
expect(res.statusCode).toBe(200);
expect(res.body.data).toMatchObject(testData.secondSuperAdminUser);
});
test('11. Deletes last super admin user (bad request)', async () => {
const res = await rq({
url: `/admin/users/${testData.firstSuperAdminUser.id}`,
method: 'DELETE',
});
expect(res.statusCode).toBe(400);
expect(res.body).toMatchObject({
statusCode: 400,
error: 'Bad Request',
message: 'ValidationError',
data: 'You must have at least one user with super admin role.',
});
});
test('12. Updates a user (not found)', async () => {
const body = {
lastname: 'doe',
};
@ -247,7 +312,7 @@ describe('Admin User CRUD (e2e)', () => {
});
});
test('10. Finds a user (not found)', async () => {
test('13. Finds a user (not found)', async () => {
const res = await rq({
url: `/admin/users/${testData.user.id}`,
method: 'GET',
@ -261,7 +326,7 @@ describe('Admin User CRUD (e2e)', () => {
});
});
test('11. Finds a list of users (missing user)', async () => {
test('14. Finds a list of users (missing user)', async () => {
const res = await rq({
url: `/admin/users?email=${testData.user.email}`,
method: 'GET',

View File

@ -40,13 +40,18 @@ module.exports = {
await register();
// login
const user = await login();
const res = await login();
return user && user.token;
return res && res.token;
},
async login() {
const user = await login();
const res = await login();
return user && user.token;
return res && res.token;
},
async getUser() {
const res = await login();
return res.user;
},
};