Revoked share links (#664)

* Move to revokation API for share links

* Respect revoked share links
Add documentation for shares endpoints

* 💚

app/stores/SharesStore.js

@@ -38,7 +38,7 @@ class SharesStore {
   @action
   revoke = async (share: Share) => {
     try {
-      await client.post('/shares.delete', { id: share.id });
+      await client.post('/shares.revoke', { id: share.id });
       runInAction('revoke', () => {
         this.data.delete(share.id);
       });

server/api/__snapshots__/shares.test.js.snap

@@ -17,3 +17,12 @@ Object {
   "status": 401,
 }
 `;
+
+exports[`#shares.revoke should require authentication 1`] = `
+Object {
+  "error": "authentication_required",
+  "message": "Authentication required",
+  "ok": false,
+  "status": 401,
+}
+`;

server/api/documents.js

@@ -165,7 +165,12 @@ router.post('documents.info', auth({ required: false }), async ctx => {
   let document;
 
   if (shareId) {
-    const share = await Share.findById(shareId, {
+    const share = await Share.find({
+      where: {
+        // $FlowFixMe
+        revokedAt: { [Op.eq]: null },
+        id: shareId,
+      },
       include: [
         {
           model: Document,

server/api/documents.test.js

@@ -36,7 +36,7 @@ describe('#documents.info', async () => {
     expect(body.data.id).toEqual(document.id);
   });
 
-  it('should return redacted documents from shareId without token', async () => {
+  it('should return redacted document from shareId without token', async () => {
     const { document } = await seed();
     const share = await buildShare({
       documentId: document.id,
@@ -55,6 +55,20 @@ describe('#documents.info', async () => {
     expect(body.data.updatedBy).toEqual(undefined);
   });
 
+  it('should not return document from revoked shareId', async () => {
+    const { document, user } = await seed();
+    const share = await buildShare({
+      documentId: document.id,
+      teamId: document.teamId,
+    });
+    await share.revoke(user.id);
+
+    const res = await server.post('/api/documents.info', {
+      body: { shareId: share.id },
+    });
+    expect(res.status).toEqual(400);
+  });
+
   it('should return documents from shareId with token', async () => {
     const { user, document, collection } = await seed();
     const share = await buildShare({

server/api/shares.js

@@ -1,11 +1,13 @@
 // @flow
 import Router from 'koa-router';
+import Sequelize from 'sequelize';
 import auth from '../middlewares/authentication';
 import pagination from './middlewares/pagination';
 import { presentShare } from '../presenters';
 import { Document, User, Share } from '../models';
 import policy from '../policies';
 
+const Op = Sequelize.Op;
 const { authorize } = policy;
 const router = new Router();
 
@@ -14,7 +16,12 @@ router.post('shares.list', auth(), pagination(), async ctx => {
   if (direction !== 'ASC') direction = 'DESC';
 
   const user = ctx.state.user;
-  const where = { teamId: user.teamId, userId: user.id };
+  const where = {
+    teamId: user.teamId,
+    userId: user.id,
+    // $FlowFixMe
+    revokedAt: { [Op.eq]: null },
+  };
 
   if (user.isAdmin) delete where.userId;
 
@@ -68,15 +75,15 @@ router.post('shares.create', auth(), async ctx => {
   };
 });
 
-router.post('shares.delete', auth(), async ctx => {
+router.post('shares.revoke', auth(), async ctx => {
   const { id } = ctx.body;
   ctx.assertPresent(id, 'id is required');
 
   const user = ctx.state.user;
   const share = await Share.findById(id);
-  authorize(user, 'delete', share);
+  authorize(user, 'revoke', share);
 
-  await share.destroy();
+  await share.revoke(user.id);
 
   ctx.body = {
     success: true,

server/api/shares.test.js

@@ -32,6 +32,24 @@ describe('#shares.list', async () => {
     expect(body.data[0].documentTitle).toBe(document.title);
   });
 
+  it('should not return revoked shares', async () => {
+    const { user, document } = await seed();
+    const share = await buildShare({
+      documentId: document.id,
+      teamId: user.teamId,
+      userId: user.id,
+    });
+    await share.revoke(user.id);
+
+    const res = await server.post('/api/shares.list', {
+      body: { token: user.getJwtToken() },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.data.length).toEqual(0);
+  });
+
   it('admins should only return shares created by all users', async () => {
     const { admin, document } = await seed();
     const share = await buildShare({
@@ -106,3 +124,58 @@ describe('#shares.create', async () => {
     expect(res.status).toEqual(403);
   });
 });
+
+describe('#shares.revoke', async () => {
+  it('should allow author to revoke a share', async () => {
+    const { user, document } = await seed();
+    const share = await buildShare({
+      documentId: document.id,
+      teamId: user.teamId,
+      userId: user.id,
+    });
+
+    const res = await server.post('/api/shares.revoke', {
+      body: { token: user.getJwtToken(), id: share.id },
+    });
+    expect(res.status).toEqual(200);
+  });
+
+  it('should allow admin to revoke a share', async () => {
+    const { user, admin, document } = await seed();
+    const share = await buildShare({
+      documentId: document.id,
+      teamId: user.teamId,
+      userId: user.id,
+    });
+
+    const res = await server.post('/api/shares.revoke', {
+      body: { token: admin.getJwtToken(), id: share.id },
+    });
+    expect(res.status).toEqual(200);
+  });
+
+  it('should require authentication', async () => {
+    const { user, document } = await seed();
+    const share = await buildShare({
+      documentId: document.id,
+      teamId: user.teamId,
+      userId: user.id,
+    });
+    const res = await server.post('/api/shares.revoke', {
+      body: { id: share.id },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(401);
+    expect(body).toMatchSnapshot();
+  });
+
+  it('should require authorization', async () => {
+    const { document } = await seed();
+    const user = await buildUser();
+    const res = await server.post('/api/shares.create', {
+      body: { token: user.getJwtToken(), documentId: document.id },
+    });
+    expect(res.status).toEqual(403);
+  });
+});

server/migrations/20180604191743-revoke-share-links.js

@@ -0,0 +1,19 @@
+module.exports = {
+  up: async (queryInterface, Sequelize) => {
+    await queryInterface.addColumn('shares', 'revokedAt', {
+      type: Sequelize.DATE,
+      allowNull: true
+    });
+    await queryInterface.addColumn('shares', 'revokedById', {
+      type: Sequelize.UUID,
+      allowNull: true,
+      references: {
+        model: 'users',
+      },
+    });
+  },
+  down: async (queryInterface, Sequelize) => {
+    await queryInterface.removeColumn('shares', 'revokedAt');
+    await queryInterface.removeColumn('shares', 'revokedById');
+  }
+}

server/models/Share.js

@@ -1,13 +1,25 @@
 // @flow
 import { DataTypes, sequelize } from '../sequelize';
 
-const Share = sequelize.define('share', {
+const Share = sequelize.define(
+  'share',
+  {
     id: {
       type: DataTypes.UUID,
       defaultValue: DataTypes.UUIDV4,
       primaryKey: true,
     },
-});
+    revokedAt: DataTypes.DATE,
+    revokedById: DataTypes.UUID,
+  },
+  {
+    getterMethods: {
+      isRevoked() {
+        return !!this.revokedAt;
+      },
+    },
+  }
+);
 
 Share.associate = models => {
   Share.belongsTo(models.User, {
@@ -24,4 +36,10 @@ Share.associate = models => {
   });
 };
 
+Share.prototype.revoke = function(userId) {
+  this.revokedAt = new Date();
+  this.revokedById = userId;
+  return this.save();
+};
+
 export default Share;

server/pages/Api.js

@@ -327,7 +327,7 @@ export default function Pricing() {
             <Description>
               <p>
                 This method returns information for a document with a specific
-                ID. Following identifiers are allowed:
+                ID. The following identifiers are allowed:
               </p>
               <ul>
                 <li>
@@ -340,11 +340,8 @@ export default function Pricing() {
               </ul>
             </Description>
             <Arguments>
-              <Argument
+              <Argument id="id" description="Document ID or URI identifier" />
-                id="id"
+              <Argument id="shareId" description="An active shareId" />
-                description="Document ID or URI identifier"
-                required
-              />
             </Arguments>
           </Method>
 
@@ -597,6 +594,34 @@ export default function Pricing() {
             </Description>
             <Arguments pagination />
           </Method>
+
+          <Method method="shares.list" label="List shared document links">
+            <Description>
+              List all your currently shared document links.
+            </Description>
+            <Arguments pagination />
+          </Method>
+
+          <Method method="shares.create" label="Create a share link">
+            <Description>
+              Creates a new share link that can be used by anyone to access a
+              document. If you request multiple shares for the same document
+              with the same user the same share will be returned.
+            </Description>
+            <Arguments>
+              <Argument id="documentId" description="Document ID" required />
+            </Arguments>
+          </Method>
+
+          <Method method="shares.revoke" label="Revoke a share link">
+            <Description>
+              Makes the share link inactive so that it can no longer be used to
+              access the document.
+            </Description>
+            <Arguments>
+              <Argument id="id" description="Share ID" required />
+            </Arguments>
+          </Method>
         </Methods>
       </Container>
     </Grid>

server/policies/share.js

@@ -7,7 +7,7 @@ const { allow } = policy;
 
 allow(User, ['read'], Share, (user, share) => user.teamId === share.teamId);
 allow(User, ['update'], Share, (user, share) => false);
-allow(User, ['delete'], Share, (user, share) => {
+allow(User, ['revoke'], Share, (user, share) => {
   if (!share || user.teamId !== share.teamId) return false;
   if (user.id === share.userId) return true;
   if (user.isAdmin) return true;