Account Deletion (#716)

Adds ability to remove user account, wipes personal information and soft-deletes record.

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 {
@@ -61,29 +70,6 @@ Object {
 }
 `;
 
-exports[`#user.info should require authentication 1`] = `
-Object {
-  "error": "authentication_required",
-  "message": "Authentication required",
-  "ok": false,
-  "status": 401,
-}
-`;
-
-exports[`#user.info should return known user 1`] = `
-Object {
-  "data": Object {
-    "avatarUrl": "http://example.com/avatar.png",
-    "createdAt": "2018-01-01T00:00:00.000Z",
-    "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
-    "name": "User 1",
-    "username": "user1",
-  },
-  "ok": true,
-  "status": 200,
-}
-`;
-
 exports[`#user.promote should promote a new admin 1`] = `
 Object {
   "data": Object {

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;

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,19 +12,60 @@ 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() },
     });
     const body = await res.json();
 
     expect(res.status).toEqual(200);
-    expect(body).toMatchSnapshot();
+    expect(body.data.id).toEqual(user.id);
+    expect(body.data.name).toEqual(user.name);
   });
 
   it('should require authentication', async () => {
-    await seed();
     const res = await server.post('/api/user.info');
+    expect(res.status).toEqual(401);
+  });
+});
+
+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);
@@ -44,7 +86,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 +108,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 +137,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 +152,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 +180,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 +194,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 +227,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 },
     });

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');
+  }
+}

server/migrations/20180708231200-serviceid-null.js

@@ -0,0 +1,14 @@
+module.exports = {
+  up: async (queryInterface, Sequelize) => {
+    await queryInterface.changeColumn('users', 'serviceId', {
+      type: Sequelize.STRING,
+      allowNull: true,
+    });
+  },
+  down: async (queryInterface, Sequelize) => {
+    await queryInterface.changeColumn('users', 'serviceId', {
+      type: Sequelize.STRING,
+      allowNull: false,
+    });
+  }
+}

server/models/ApiKey.js

@@ -31,4 +31,11 @@ const ApiKey = sequelize.define(
   }
 );
 
+ApiKey.associate = models => {
+  ApiKey.belongsTo(models.User, {
+    as: 'user',
+    foreignKey: 'userId',
+  });
+};
+
 export default ApiKey;

server/models/Document.js

@@ -141,8 +141,8 @@ Document.associate = models => {
     {
       include: [
         { model: models.Collection, as: 'collection' },
-        { model: models.User, as: 'createdBy' },
-        { model: models.User, as: 'updatedBy' },
+        { model: models.User, as: 'createdBy', paranoid: false },
+        { model: models.User, as: 'updatedBy', paranoid: false },
       ],
       where: {
         publishedAt: {
@@ -156,8 +156,8 @@ Document.associate = models => {
   Document.addScope('withUnpublished', {
     include: [
       { model: models.Collection, as: 'collection' },
-      { model: models.User, as: 'createdBy' },
-      { model: models.User, as: 'updatedBy' },
+      { model: models.User, as: 'createdBy', paranoid: false },
+      { model: models.User, as: 'updatedBy', paranoid: false },
     ],
   });
   Document.addScope('withViews', userId => ({

server/models/User.js

@@ -6,6 +6,7 @@ import subMinutes from 'date-fns/sub_minutes';
 import { DataTypes, sequelize, encryptedFields } from '../sequelize';
 import { publicS3Endpoint, uploadToS3FromUrl } from '../utils/s3';
 import { sendEmail } from '../mailer';
+import { Star, ApiKey } from '.';
 
 const User = sequelize.define(
   'user',
@@ -25,13 +26,14 @@ const User = sequelize.define(
     slackData: DataTypes.JSONB,
     jwtSecret: encryptedFields.vault('jwtSecret'),
     lastActiveAt: DataTypes.DATE,
-    lastActiveIp: DataTypes.STRING,
+    lastActiveIp: { type: DataTypes.STRING, allowNull: true },
     lastSignedInAt: DataTypes.DATE,
-    lastSignedInIp: DataTypes.STRING,
+    lastSignedInIp: { type: DataTypes.STRING, allowNull: true },
     suspendedAt: DataTypes.DATE,
     suspendedById: DataTypes.UUID,
   },
   {
+    paranoid: true,
     getterMethods: {
       isSuspended() {
         return !!this.suspendedAt;
@@ -91,6 +93,41 @@ const setRandomJwtSecret = model => {
   model.jwtSecret = crypto.randomBytes(64).toString('hex');
 };
 
+const removeIdentifyingInfo = async model => {
+  await ApiKey.destroy({ where: { userId: model.id } });
+  await Star.destroy({ where: { userId: model.id } });
+
+  model.email = '';
+  model.name = 'Unknown';
+  model.avatarUrl = '';
+  model.serviceId = null;
+  model.username = null;
+  model.slackData = null;
+  model.lastActiveIp = null;
+  model.lastSignedInIp = null;
+
+  // this shouldn't be needed once this issue is resolved:
+  // https://github.com/sequelize/sequelize/issues/9318
+  await model.save({ hooks: 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.beforeDestroy(checkLastAdmin);
+User.beforeDestroy(removeIdentifyingInfo);
 User.beforeSave(uploadAvatar);
 User.beforeCreate(setRandomJwtSecret);
 User.afterCreate(user => sendEmail('welcome', user.email));