diff --git a/packages/strapi-admin/config/routes.json b/packages/strapi-admin/config/routes.json index 7f99133960..3516bb7569 100644 --- a/packages/strapi-admin/config/routes.json +++ b/packages/strapi-admin/config/routes.json @@ -150,6 +150,11 @@ "path": "/users/:id", "handler": "user.update" }, + { + "method": "DELETE", + "path": "/users/:id", + "handler": "user.delete" + }, { "method": "GET", "path": "/roles/:id/permissions", diff --git a/packages/strapi-admin/controllers/__tests__/user.test.js b/packages/strapi-admin/controllers/__tests__/user.test.js index 1b8555109e..87efddbf4d 100644 --- a/packages/strapi-admin/controllers/__tests__/user.test.js +++ b/packages/strapi-admin/controllers/__tests__/user.test.js @@ -202,10 +202,10 @@ describe('User Controller', () => { test('User not found', async () => { const fakeId = 42; const exists = jest.fn(() => false); - const badRequest = jest.fn(); + const notFound = jest.fn(); const body = { username: 'Foo' }; - const ctx = createContext({ params: { id: fakeId }, body }, { badRequest }); + const ctx = createContext({ params: { id: fakeId }, body }, { notFound }); global.strapi = { admin: { @@ -219,7 +219,7 @@ describe('User Controller', () => { expect(exists).toHaveBeenCalledWith({ id: fakeId }); expect(exists).toHaveReturnedWith(false); - expect(badRequest).toHaveBeenCalledWith('User does not exist'); + expect(notFound).toHaveBeenCalledWith('User does not exist'); }); test('Validation error', async () => { diff --git a/packages/strapi-admin/controllers/user.js b/packages/strapi-admin/controllers/user.js index 1ced884f55..a3e4e81d2b 100644 --- a/packages/strapi-admin/controllers/user.js +++ b/packages/strapi-admin/controllers/user.js @@ -50,7 +50,7 @@ module.exports = { const user = await strapi.admin.services.user.findOne({ id }); if (!user) { - return ctx.badRequest('User does not exist'); + return ctx.notFound('User does not exist'); } ctx.body = { @@ -68,16 +68,32 @@ module.exports = { return ctx.badRequest('ValidationError', err); } - const userExists = await strapi.admin.services.user.exists({ id }); + const updatedUser = await strapi.admin.services.user.update({ id }, input); - if (!userExists) { + if (!updatedUser) { return ctx.badRequest('User does not exist'); } - const updatedUser = await strapi.admin.services.user.update({ id }, input); - ctx.body = { data: strapi.admin.services.user.sanitizeUser(updatedUser), }; }, + + async delete(ctx) { + const { id } = ctx.params; + + if (!id) { + return ctx.badRequest('Invalid ID'); + } + + const deletedUser = await strapi.admin.services.user.deleteOne({ id }); + + if (!deletedUser) { + return ctx.notFound('User not found'); + } + + return ctx.deleted({ + data: strapi.admin.services.user.sanitizeUser(deletedUser), + }); + }, }; diff --git a/packages/strapi-admin/domain/user.js b/packages/strapi-admin/domain/user.js index 82218f26c5..ce247b1092 100644 --- a/packages/strapi-admin/domain/user.js +++ b/packages/strapi-admin/domain/user.js @@ -8,6 +8,7 @@ function createUser(attributes) { return { roles: [], isActive: false, + username: null, ...attributes, }; } diff --git a/packages/strapi-admin/ee/validation/role.js b/packages/strapi-admin/ee/validation/role.js index 30f4364d0b..042b98195d 100644 --- a/packages/strapi-admin/ee/validation/role.js +++ b/packages/strapi-admin/ee/validation/role.js @@ -1,7 +1,6 @@ 'use strict'; const { yup, formatYupErrors } = require('strapi-utils'); -const { strapiId } = require('../../validation/common-validators'); const handleReject = error => Promise.reject(formatYupErrors(error)); @@ -21,7 +20,7 @@ const roleDeleteSchema = yup .shape({ ids: yup .array() - .of(strapiId) + .of(yup.strapiID()) .min(1) .required(), }) diff --git a/packages/strapi-admin/models/User.settings.json b/packages/strapi-admin/models/User.settings.json index fcc3302740..212632208d 100644 --- a/packages/strapi-admin/models/User.settings.json +++ b/packages/strapi-admin/models/User.settings.json @@ -29,7 +29,8 @@ "type": "email", "minLength": 6, "configurable": false, - "required": true + "required": true, + "unique": true }, "password": { "type": "password", diff --git a/packages/strapi-admin/services/__tests__/user.test.js b/packages/strapi-admin/services/__tests__/user.test.js index acb06a38e1..d8de4bc3f6 100644 --- a/packages/strapi-admin/services/__tests__/user.test.js +++ b/packages/strapi-admin/services/__tests__/user.test.js @@ -284,7 +284,7 @@ describe('User', () => { }; }); - test('Find and returns a user by its ID', async () => { + test('Finds and returns a user by its ID', async () => { const input = { id: 1 }; const res = await userService.findOne(input); @@ -292,7 +292,7 @@ describe('User', () => { expect(res).toStrictEqual(user); }); - test('Fail to find a user with provided params', async () => { + test('Fails to find a user with provided params', async () => { const input = { id: 27 }; const res = await userService.findOne(input); diff --git a/packages/strapi-admin/services/user.js b/packages/strapi-admin/services/user.js index b2e97b681b..4397af4c77 100644 --- a/packages/strapi-admin/services/user.js +++ b/packages/strapi-admin/services/user.js @@ -132,6 +132,14 @@ const searchPage = async query => { return strapi.query('user', 'admin').searchPage(query); }; +/** Delete a user + * @param query + * @returns {Promise} + */ +const deleteOne = async query => { + return strapi.query('user', 'admin').delete(query); +}; + module.exports = { create, update, @@ -142,4 +150,5 @@ module.exports = { findOne, findPage, searchPage, + deleteOne, }; diff --git a/packages/strapi-admin/test/admin-auth.test.e2e.js b/packages/strapi-admin/test/admin-auth.test.e2e.js index 6ae0e5c6b7..0f5aed545e 100644 --- a/packages/strapi-admin/test/admin-auth.test.e2e.js +++ b/packages/strapi-admin/test/admin-auth.test.e2e.js @@ -4,25 +4,50 @@ const { createAuthRequest } = require('../../../test/helpers/request'); let rq; -const createUser = data => { - return rq({ - url: '/admin/users', +const createAuthRole = async () => { + const res = await rq({ + url: '/admin/roles', method: 'POST', body: { - roles: [1], - ...data, + name: 'auth_test_role', + description: 'Only used for auth crud test (e2e)', }, }); + + return res && res.body && res.body.data; }; describe('Admin Auth End to End', () => { + const testData = { + role: null, + }; + + const createUser = data => { + return rq({ + url: '/admin/users', + method: 'POST', + body: { + roles: [testData.role.id], + ...data, + }, + }); + }; + beforeAll(async () => { const token = await registerAndLogin(); rq = createAuthRequest(token); + testData.role = await createAuthRole(); + }, 60000); + + afterAll(async () => { + await rq({ + url: `/admin/roles/${testData.role.id}`, + method: 'DELETE', + }); }, 60000); describe('Login', () => { - test('Can connect successfuklly', async () => { + test('Can connect successfully', async () => { const res = await rq({ url: '/admin/login', method: 'POST', diff --git a/packages/strapi-admin/test/admin-user-crud.test.e2e.js b/packages/strapi-admin/test/admin-user-crud.test.e2e.js deleted file mode 100644 index 2e788caf8f..0000000000 --- a/packages/strapi-admin/test/admin-user-crud.test.e2e.js +++ /dev/null @@ -1,253 +0,0 @@ -// Helpers. -const { registerAndLogin } = require('../../../test/helpers/auth'); -const { createAuthRequest } = require('../../../test/helpers/request'); - -let rq; - -describe('Admin User CRUD End to End', () => { - beforeAll(async () => { - const token = await registerAndLogin(); - rq = createAuthRequest(token); - }, 60000); - - describe('Create a new user', () => { - // FIXME: Waiting for strapi-admin::roles API - test.skip('Can create a user successfully', async () => { - const user = { - email: 'new-user@strapi.io', - firstname: 'New', - lastname: 'User', - roles: ['41224d776a326fb40f000001'], - }; - - const res = await rq({ - url: '/admin/users', - method: 'POST', - body: user, - }); - - expect(res.statusCode).toBe(201); - expect(res.body).toMatchObject({ - firstname: user.firstname, - lastname: user.lastname, - email: user.email, - registrationToken: expect.any(String), - isActive: false, - roles: [], - }); - }); - - test('Fails on missing field (email)', async () => { - const user = { - firstname: 'New', - lastname: 'User', - roles: [1, 2], - }; - - const res = await rq({ - url: '/admin/users', - method: 'POST', - body: user, - }); - - expect(res.statusCode).toBe(400); - expect(res.body).toMatchObject({ - statusCode: 400, - error: 'Bad Request', - message: 'ValidationError', - data: { - email: ['email is a required field'], - }, - }); - }); - - test('Fails on invalid field type (firstname)', async () => { - const user = { - email: 'new-user@strapi.io', - firstname: 1, - lastname: 'User', - roles: [1, 2], - }; - - const res = await rq({ - url: '/admin/users', - method: 'POST', - body: user, - }); - - expect(res.statusCode).toBe(400); - expect(res.body).toMatchObject({ - statusCode: 400, - error: 'Bad Request', - message: 'ValidationError', - data: { - firstname: ['firstname must be a `string` type, but the final value was: `1`.'], - }, - }); - }); - }); - - describe('Update a user', () => { - test('Fails on invalid payload', async () => { - const body = { firstname: 42 }; - - const res = await rq({ - url: '/admin/users/1', - method: 'PUT', - body, - }); - - expect(res.statusCode).toBe(400); - expect(res.body).toStrictEqual({ - data: { - firstname: ['firstname must be a `string` type, but the final value was: `42`.'], - }, - error: 'Bad Request', - message: 'ValidationError', - statusCode: 400, - }); - }); - - test('Fails on user not found', async () => { - const body = { firstname: 'foo' }; - - const res = await rq({ - url: '/admin/users/999999', - method: 'PUT', - body, - }); - - expect(res.statusCode).toBe(400); - expect(res.body).toStrictEqual({ - error: 'Bad Request', - message: 'User does not exist', - statusCode: 400, - }); - }); - - test('Can update a user successfully', async () => { - const body = { firstname: 'foo' }; - - const res = await rq({ - url: '/admin/users/1', - method: 'PUT', - body, - }); - - expect(res.statusCode).toBe(200); - expect(res.body).toStrictEqual({ - data: { - email: 'admin@strapi.io', - firstname: 'foo', - lastname: 'admin', - username: null, - id: 1, - isActive: true, - registrationToken: null, - roles: [], - }, - }); - }); - }); - - describe('Fetch a user', () => { - test('User does not exist', async () => { - const res = await rq({ - url: '/admin/users/999', - method: 'GET', - }); - - expect(res.statusCode).toBe(400); - expect(res.body).toStrictEqual({ - error: 'Bad Request', - message: 'User does not exist', - statusCode: 400, - }); - }); - - test('Find one user successfully', async () => { - const res = await rq({ - url: '/admin/users/1', - method: 'GET', - }); - - expect(res.statusCode).toBe(200); - expect(res.body).toStrictEqual({ - data: { - email: 'admin@strapi.io', - firstname: 'foo', - lastname: 'admin', - username: null, - id: 1, - isActive: true, - registrationToken: null, - roles: [], - }, - }); - }); - }); - - describe('Fetch a list of user', () => { - test('Using findPage', async () => { - const res = await rq({ - url: '/admin/users', - method: 'GET', - }); - - expect(res.statusCode).toBe(200); - expect(res.body).toStrictEqual({ - data: { - pagination: { - page: 1, - pageSize: 100, - total: 1, - pageCount: 1, - }, - results: [ - { - email: 'admin@strapi.io', - firstname: 'foo', - lastname: 'admin', - username: null, - registrationToken: null, - roles: [], - id: 1, - isActive: true, - }, - ], - }, - }); - }); - - test('Using searchPage', async () => { - const res = await rq({ - url: '/admin/users?_q=foo', - method: 'GET', - }); - - expect(res.statusCode).toBe(200); - expect(res.body).toStrictEqual({ - data: { - pagination: { - page: 1, - pageSize: 100, - total: 1, - pageCount: 1, - }, - results: [ - { - email: 'admin@strapi.io', - firstname: 'foo', - lastname: 'admin', - username: null, - registrationToken: null, - roles: [], - id: 1, - isActive: true, - }, - ], - }, - }); - }); - }); -}); diff --git a/packages/strapi-admin/test/admin-user.test.e2e.js b/packages/strapi-admin/test/admin-user.test.e2e.js new file mode 100644 index 0000000000..7dbca0858a --- /dev/null +++ b/packages/strapi-admin/test/admin-user.test.e2e.js @@ -0,0 +1,276 @@ +'use strict'; + +const { login, registerAndLogin } = require('../../../test/helpers/auth'); +const { createAuthRequest } = require('../../../test/helpers/request'); + +const getAuthToken = async () => { + let token = await login(); + + if (!token) { + token = await registerAndLogin(); + } + + return token; +}; + +const createUserRole = async () => { + const res = await rq({ + url: '/admin/roles', + method: 'POST', + body: { + name: 'user_test_role', + description: 'Only used for user crud test (e2e)', + }, + }); + + return res && res.body && res.body.data; +}; + +const deleteUserRole = async id => { + await rq({ + url: `/admin/roles/${id}`, + method: 'DELETE', + }); +}; + +let rq; + +/** + * == Test Suite Overview == + * + * N° Description + * ------------------------------------------- + * 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) + */ +describe('Admin User CRUD (e2e)', () => { + // Local test data used across the test suite + let testData = { + user: undefined, + role: undefined, + }; + + // Initialization Actions + beforeAll(async () => { + const token = await getAuthToken(); + rq = createAuthRequest(token); + testData.role = await createUserRole(); + }); + + // Cleanup actions + afterAll(async () => { + await deleteUserRole(testData.role.id); + }); + + test('1. Creates a user (wrong body)', async () => { + const body = { + firstname: 'user_tests-firstname', + lastname: 'user_tests-lastname', + roles: [testData.role.id], + }; + + const res = await rq({ + url: '/admin/users', + method: 'POST', + body, + }); + + expect(res.statusCode).toBe(400); + expect(res.body).toMatchObject({ + statusCode: 400, + error: 'Bad Request', + message: 'ValidationError', + data: { + email: ['email is a required field'], + }, + }); + }); + + test('2. Creates a user (successfully)', async () => { + const body = { + email: 'user-tests@strapi-e2e.com', + firstname: 'user_tests-firstname', + lastname: 'user_tests-lastname', + roles: [testData.role.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.user = res.body.data; + }); + + test('3. Updates a user (wrong body)', async () => { + const body = { + email: 42, + }; + + const res = await rq({ + url: `/admin/users/${testData.user.id}`, + method: 'PUT', + body, + }); + + expect(res.statusCode).toBe(400); + expect(res.body).toMatchObject({ + statusCode: 400, + error: 'Bad Request', + message: 'ValidationError', + data: { + email: ['email must be a `string` type, but the final value was: `42`.'], + }, + }); + }); + + test('4. Updates a user (successfully)', async () => { + const body = { + firstname: 'foobar', + }; + + const res = await rq({ + url: `/admin/users/${testData.user.id}`, + method: 'PUT', + body, + }); + + expect(res.statusCode).toBe(200); + expect(res.body).not.toBeNull(); + expect({ updatedAt: null, ...res.body.data }).toMatchObject({ + ...testData.user, + ...body, + updatedAt: expect.stringOrNull(), + }); + + // Update the local copy of the user + testData.user = res.body.data; + }); + + test('5. Finds a user (successfully)', async () => { + const res = await rq({ + url: `/admin/users/${testData.user.id}`, + method: 'GET', + }); + + expect(res.statusCode).toBe(200); + expect(res.body.data).toMatchObject(testData.user); + }); + + describe('6. Finds a list of users (contains user)', () => { + const expectedResults = () => ({ + data: { + pagination: { + page: 1, + pageSize: expect.any(Number), + pageCount: expect.any(Number), + total: expect.any(Number), + }, + results: expect.any(Array), + }, + }); + + test('6.1. Using findPage', async () => { + const res = await rq({ + url: `/admin/users?email=${testData.user.email}`, + method: 'GET', + }); + + expect(res.statusCode).toBe(200); + expect(res.body).toMatchObject(expectedResults()); + expect(res.body.data.results).toContainEqual(testData.user); + }); + + test('6.2. Using searchPage', async () => { + const res = await rq({ + url: `/admin/users?_q=${testData.user.email}`, + method: 'GET', + }); + + expect(res.statusCode).toBe(200); + expect(res.body).toMatchObject(expectedResults()); + expect(res.body.data.results).toContainEqual(testData.user); + }); + }); + + test('7. Deletes a user (successfully)', async () => { + const res = await rq({ + url: `/admin/users/${testData.user.id}`, + method: 'DELETE', + }); + + expect(res.statusCode).toBe(200); + expect(res.body.data).toMatchObject(testData.user); + }); + + test('8. Deletes a user (not found)', async () => { + const res = await rq({ + url: `/admin/users/${testData.user.id}`, + method: 'DELETE', + }); + + expect(res.statusCode).toBe(404); + }); + + test('9. Updates a user (not found)', async () => { + const body = { + lastname: 'doe', + }; + + const res = await rq({ + url: `/admin/users/${testData.user.id}`, + method: 'PUT', + body, + }); + + expect(res.statusCode).toBe(404); + expect(res.body).toMatchObject({ + error: 'Not Found', + message: 'entry.notFound', + statusCode: 404, + }); + }); + + test('10. Finds a user (not found)', async () => { + const res = await rq({ + url: `/admin/users/${testData.user.id}`, + method: 'GET', + }); + + expect(res.statusCode).toBe(404); + expect(res.body.data).toBeUndefined(); + }); + + test('12. Finds a list of users (missing user)', async () => { + const res = await rq({ + url: `/admin/users?email=${testData.user.email}`, + method: 'GET', + }); + + expect(res.statusCode).toBe(200); + expect(res.body.data).toMatchObject({ + pagination: { + page: 1, + pageSize: expect.any(Number), + pageCount: expect.any(Number), + total: expect.any(Number), + }, + results: expect.any(Array), + }); + expect(res.body.data.results).not.toContainEqual(testData.user); + }); +}); diff --git a/packages/strapi-admin/validation/role.js b/packages/strapi-admin/validation/role.js index 0968f8670a..e3aef20d05 100644 --- a/packages/strapi-admin/validation/role.js +++ b/packages/strapi-admin/validation/role.js @@ -7,7 +7,7 @@ const handleReject = error => Promise.reject(formatYupErrors(error)); const roleUpdateSchema = yup .object() .shape({ - description: yup.string(), + description: yup.string().nullable(), }) .noUnknown(); diff --git a/packages/strapi-admin/validation/user.js b/packages/strapi-admin/validation/user.js index c96c9097b3..490ebb6c39 100644 --- a/packages/strapi-admin/validation/user.js +++ b/packages/strapi-admin/validation/user.js @@ -22,11 +22,11 @@ const validateUserCreationInput = data => { const profileUpdateSchema = yup .object() .shape({ - email: validators.email, - firstname: validators.firstname, - lastname: validators.lastname, + email: validators.email.notNull(), + firstname: validators.firstname.notNull(), + lastname: validators.lastname.notNull(), username: validators.username.nullable(), - password: validators.password, + password: validators.password.notNull(), }) .noUnknown(); @@ -39,13 +39,13 @@ const validateProfileUpdateInput = data => { const userUpdateSchema = yup .object() .shape({ - email: validators.email, - firstname: validators.firstname, - lastname: validators.lastname, + email: validators.email.notNull(), + firstname: validators.firstname.notNull(), + lastname: validators.lastname.notNull(), username: validators.username.nullable(), - password: validators.password, - isActive: yup.bool(), - roles: validators.roles.min(1), + password: validators.password.notNull(), + isActive: yup.bool().notNull(), + roles: validators.roles.min(1).notNull(), }) .noUnknown(); diff --git a/packages/strapi/lib/middlewares/boom/index.js b/packages/strapi/lib/middlewares/boom/index.js index 60b0019671..5b8c866413 100644 --- a/packages/strapi/lib/middlewares/boom/index.js +++ b/packages/strapi/lib/middlewares/boom/index.js @@ -129,7 +129,19 @@ module.exports = strapi => { this.body = data; }; - this.delegator.method('send').method('created'); + strapi.app.response.deleted = function(data) { + if (_.isNil(data)) { + this.status = 204; + } else { + this.status = 200; + this.body = data; + } + }; + + this.delegator + .method('send') + .method('created') + .method('deleted'); }, }; }; diff --git a/test/helpers/auth.js b/test/helpers/auth.js index 0767214c24..79f9030ebe 100644 --- a/test/helpers/auth.js +++ b/test/helpers/auth.js @@ -40,7 +40,13 @@ module.exports = { await register(); // login - const { token } = await login(); - return token; + const user = await login(); + + return user && user.token; + }, + async login() { + const user = await login(); + + return user && user.token; }, };