diff --git a/app/stores/SharesStore.js b/app/stores/SharesStore.js index dcabd883..64f35cae 100644 --- a/app/stores/SharesStore.js +++ b/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); }); diff --git a/server/api/__snapshots__/shares.test.js.snap b/server/api/__snapshots__/shares.test.js.snap index 77cc5d4d..439b96bb 100644 --- a/server/api/__snapshots__/shares.test.js.snap +++ b/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, +} +`; diff --git a/server/api/documents.js b/server/api/documents.js index 0813df7e..f06e655f 100644 --- a/server/api/documents.js +++ b/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, diff --git a/server/api/documents.test.js b/server/api/documents.test.js index 770fc455..7abd0547 100644 --- a/server/api/documents.test.js +++ b/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({ diff --git a/server/api/shares.js b/server/api/shares.js index 677b0222..15caf93b 100644 --- a/server/api/shares.js +++ b/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, diff --git a/server/api/shares.test.js b/server/api/shares.test.js index 73000777..c768f42a 100644 --- a/server/api/shares.test.js +++ b/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); + }); +}); diff --git a/server/migrations/20180604191743-revoke-share-links.js b/server/migrations/20180604191743-revoke-share-links.js new file mode 100644 index 00000000..c6f759b7 --- /dev/null +++ b/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'); + } +} \ No newline at end of file diff --git a/server/models/Share.js b/server/models/Share.js index 3965b864..6d77b631 100644 --- a/server/models/Share.js +++ b/server/models/Share.js @@ -1,13 +1,25 @@ // @flow import { DataTypes, sequelize } from '../sequelize'; -const Share = sequelize.define('share', { - id: { - type: DataTypes.UUID, - defaultValue: DataTypes.UUIDV4, - primaryKey: true, +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; diff --git a/server/pages/Api.js b/server/pages/Api.js index dee26dd0..0d168b00 100644 --- a/server/pages/Api.js +++ b/server/pages/Api.js @@ -327,7 +327,7 @@ export default function Pricing() {

This method returns information for a document with a specific - ID. Following identifiers are allowed: + ID. The following identifiers are allowed:

- + + @@ -597,6 +594,34 @@ export default function Pricing() { + + + + List all your currently shared document links. + + + + + + + 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. + + + + + + + + + Makes the share link inactive so that it can no longer be used to + access the document. + + + + + diff --git a/server/policies/share.js b/server/policies/share.js index 3e2d1121..62151ed0 100644 --- a/server/policies/share.js +++ b/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;