diff --git a/server/api/__snapshots__/user.test.js.snap b/server/api/__snapshots__/user.test.js.snap index 451c115c..913cd0c3 100644 --- a/server/api/__snapshots__/user.test.js.snap +++ b/server/api/__snapshots__/user.test.js.snap @@ -26,6 +26,15 @@ Object { } `; +exports[`#user.delete should require authentication 1`] = ` +Object { + "error": "authentication_required", + "message": "Authentication required", + "ok": false, + "status": 401, +} +`; + exports[`#user.demote should demote an admin 1`] = ` Object { "data": Object { diff --git a/server/api/user.js b/server/api/user.js index 0ab1d9a7..ad1fcb1a 100644 --- a/server/api/user.js +++ b/server/api/user.js @@ -164,4 +164,22 @@ router.post('user.activate', auth(), async ctx => { }; }); +router.post('user.delete', auth(), async ctx => { + const { confirmation } = ctx.body; + ctx.assertPresent(confirmation, 'confirmation is required'); + + const user = ctx.state.user; + authorize(user, 'delete', user); + + try { + await user.destroy(); + } catch (err) { + throw new ValidationError(err.message); + } + + ctx.body = { + success: true, + }; +}); + export default router; diff --git a/server/api/user.test.js b/server/api/user.test.js index f714eac6..b82e37f7 100644 --- a/server/api/user.test.js +++ b/server/api/user.test.js @@ -3,6 +3,7 @@ import TestServer from 'fetch-test-server'; import app from '..'; import { flushdb, seed } from '../test/support'; +import { buildUser } from '../test/factories'; const server = new TestServer(app.callback()); @@ -11,7 +12,7 @@ afterAll(server.close); describe('#user.info', async () => { it('should return known user', async () => { - const { user } = await seed(); + const user = await buildUser(); const res = await server.post('/api/user.info', { body: { token: user.getJwtToken() }, }); @@ -22,7 +23,6 @@ describe('#user.info', async () => { }); it('should require authentication', async () => { - await seed(); const res = await server.post('/api/user.info'); const body = await res.json(); @@ -31,6 +31,50 @@ describe('#user.info', async () => { }); }); +describe('#user.delete', async () => { + it('should not allow deleting without confirmation', async () => { + const user = await buildUser(); + const res = await server.post('/api/user.delete', { + body: { token: user.getJwtToken() }, + }); + expect(res.status).toEqual(400); + }); + + it('should allow deleting last admin if only user', async () => { + const user = await buildUser({ isAdmin: true }); + const res = await server.post('/api/user.delete', { + body: { token: user.getJwtToken(), confirmation: true }, + }); + expect(res.status).toEqual(200); + }); + + it('should not allow deleting last admin if many users', async () => { + const user = await buildUser({ isAdmin: true }); + await buildUser({ teamId: user.teamId, isAdmin: false }); + + const res = await server.post('/api/user.delete', { + body: { token: user.getJwtToken(), confirmation: true }, + }); + expect(res.status).toEqual(400); + }); + + it('should allow deleting user account with confirmation', async () => { + const user = await buildUser(); + const res = await server.post('/api/user.delete', { + body: { token: user.getJwtToken(), confirmation: true }, + }); + expect(res.status).toEqual(200); + }); + + it('should require authentication', async () => { + const res = await server.post('/api/user.delete'); + const body = await res.json(); + + expect(res.status).toEqual(401); + expect(body).toMatchSnapshot(); + }); +}); + describe('#user.update', async () => { it('should update user profile information', async () => { const { user } = await seed(); @@ -44,7 +88,6 @@ describe('#user.update', async () => { }); it('should require authentication', async () => { - await seed(); const res = await server.post('/api/user.update'); const body = await res.json(); @@ -67,7 +110,7 @@ describe('#user.promote', async () => { }); it('should require admin', async () => { - const { user } = await seed(); + const user = await buildUser(); const res = await server.post('/api/user.promote', { body: { token: user.getJwtToken(), id: user.id }, }); @@ -96,7 +139,7 @@ describe('#user.demote', async () => { }); it("shouldn't demote admins if only one available ", async () => { - const { admin } = await seed(); + const admin = await buildUser({ isAdmin: true }); const res = await server.post('/api/user.demote', { body: { @@ -111,7 +154,7 @@ describe('#user.demote', async () => { }); it('should require admin', async () => { - const { user } = await seed(); + const user = await buildUser(); const res = await server.post('/api/user.promote', { body: { token: user.getJwtToken(), id: user.id }, }); @@ -139,8 +182,7 @@ describe('#user.suspend', async () => { }); it("shouldn't allow suspending the user themselves", async () => { - const { admin } = await seed(); - + const admin = await buildUser({ isAdmin: true }); const res = await server.post('/api/user.suspend', { body: { token: admin.getJwtToken(), @@ -154,7 +196,7 @@ describe('#user.suspend', async () => { }); it('should require admin', async () => { - const { user } = await seed(); + const user = await buildUser(); const res = await server.post('/api/user.suspend', { body: { token: user.getJwtToken(), id: user.id }, }); @@ -187,7 +229,7 @@ describe('#user.activate', async () => { }); it('should require admin', async () => { - const { user } = await seed(); + const user = await buildUser(); const res = await server.post('/api/user.activate', { body: { token: user.getJwtToken(), id: user.id }, }); diff --git a/server/migrations/20180707220121-more-soft-delete.js b/server/migrations/20180707220121-more-soft-delete.js new file mode 100644 index 00000000..fb179eeb --- /dev/null +++ b/server/migrations/20180707220121-more-soft-delete.js @@ -0,0 +1,16 @@ +module.exports = { + up: async (queryInterface, Sequelize) => { + await queryInterface.addColumn('users', 'deletedAt', { + type: Sequelize.DATE, + allowNull: true + }); + await queryInterface.addColumn('teams', 'deletedAt', { + type: Sequelize.DATE, + allowNull: true + }); + }, + down: async (queryInterface, Sequelize) => { + await queryInterface.removeColumn('users', 'deletedAt'); + await queryInterface.removeColumn('teams', 'deletedAt'); + } +} \ No newline at end of file diff --git a/server/models/User.js b/server/models/User.js index ff3b1a6e..f1e92226 100644 --- a/server/models/User.js +++ b/server/models/User.js @@ -37,6 +37,7 @@ const User = sequelize.define( suspendedById: DataTypes.UUID, }, { + paranoid: true, getterMethods: { isSuspended() { return !!this.suspendedAt; @@ -132,8 +133,33 @@ const hashPassword = model => { }); }; +const removeIdentifyingInfo = model => { + model.email = ''; + model.username = ''; + model.slackData = null; + model.serviceId = null; + model.isAdmin = false; +}; + +const checkLastAdmin = async model => { + const teamId = model.teamId; + + if (model.isAdmin) { + const userCount = await User.count({ where: { teamId } }); + const adminCount = await User.count({ where: { isAdmin: true, teamId } }); + + if (userCount > 1 && adminCount <= 1) { + throw new Error( + 'Cannot delete account as only admin. Please transfer admin permissions to another user and try again.' + ); + } + } +}; + User.beforeCreate(hashPassword); User.beforeUpdate(hashPassword); +User.beforeDestroy(checkLastAdmin); +User.beforeDestroy(removeIdentifyingInfo); User.beforeSave(uploadAvatar); User.beforeCreate(setRandomJwtSecret); User.afterCreate(user => sendEmail('welcome', user.email));