From add98c889c95784e3e99e154bd040bde4bad56c9 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 7 Nov 2019 10:28:34 +0000 Subject: [PATCH] Merge pull request #2312 from overleaf/sk-fix-join-project-null When we can't join project, produce a 403 response GitOrigin-RevId: 7a52dd019ed33474e18cdb378fd3d4622f378e56 --- .../Features/Editor/EditorHttpController.js | 3 +++ .../test/acceptance/src/AuthorizationTests.js | 7 ++++-- .../test/acceptance/src/TokenAccessTests.js | 24 ++++++++++++------- .../src/Editor/EditorHttpControllerTests.js | 20 ++++++++++++++++ 4 files changed, 44 insertions(+), 10 deletions(-) diff --git a/services/web/app/src/Features/Editor/EditorHttpController.js b/services/web/app/src/Features/Editor/EditorHttpController.js index 47bb405420..b06067160b 100644 --- a/services/web/app/src/Features/Editor/EditorHttpController.js +++ b/services/web/app/src/Features/Editor/EditorHttpController.js @@ -32,6 +32,9 @@ module.exports = EditorHttpController = { if (error) { return next(error) } + if (!project) { + return res.sendStatus(403) + } // Hide access tokens if this is not the project owner TokenAccessHandler.protectTokens(project, privilegeLevel) if (isRestrictedUser) { diff --git a/services/web/test/acceptance/src/AuthorizationTests.js b/services/web/test/acceptance/src/AuthorizationTests.js index cd79169b73..20ed444bfa 100644 --- a/services/web/test/acceptance/src/AuthorizationTests.js +++ b/services/web/test/acceptance/src/AuthorizationTests.js @@ -204,7 +204,10 @@ function expectNoReadAccess(user, projectId, options, callback) { tryContentAccess( user, projectId, - (response, body) => expect(body.privilegeLevel).to.be.equal(false), + (response, body) => { + expect(response.statusCode).to.equal(403) + expect(body).to.equal('Forbidden') + }, cb ) ], @@ -217,7 +220,7 @@ function expectNoContentWriteAccess(user, projectId, callback) { user, projectId, (response, body) => - expect(body.privilegeLevel).to.be.oneOf([false, 'readOnly']), + expect(body.privilegeLevel).to.be.oneOf([undefined, null, 'readOnly']), callback ) } diff --git a/services/web/test/acceptance/src/TokenAccessTests.js b/services/web/test/acceptance/src/TokenAccessTests.js index 9691721214..c83cd69a6b 100644 --- a/services/web/test/acceptance/src/TokenAccessTests.js +++ b/services/web/test/acceptance/src/TokenAccessTests.js @@ -168,7 +168,8 @@ describe('TokenAccess', function() { this.other1, this.projectId, (response, body) => { - expect(body.privilegeLevel).to.equal(false) + expect(response.statusCode).to.equal(403) + expect(body).to.equal('Forbidden') }, cb ) @@ -286,7 +287,8 @@ describe('TokenAccess', function() { this.other1, this.projectId, (response, body) => { - expect(body.privilegeLevel).to.equal(false) + expect(response.statusCode).to.equal(403) + expect(body).to.equal('Forbidden') }, cb ) @@ -401,7 +403,8 @@ describe('TokenAccess', function() { this.projectId, this.tokens.readOnly, (response, body) => { - expect(body.privilegeLevel).to.equal(false) + expect(response.statusCode).to.equal(403) + expect(body).to.equal('Forbidden') }, cb ) @@ -518,7 +521,8 @@ describe('TokenAccess', function() { this.other1, this.projectId, (response, body) => { - expect(body.privilegeLevel).to.equal(false) + expect(response.statusCode).to.equal(403) + expect(body).to.equal('Forbidden') }, cb ) @@ -585,7 +589,8 @@ describe('TokenAccess', function() { this.projectId, this.tokens.readAndWrite, (response, body) => { - expect(body.privilegeLevel).to.equal(false) + expect(response.statusCode).to.equal(403) + expect(body).to.equal('Forbidden') }, cb ) @@ -690,7 +695,8 @@ describe('TokenAccess', function() { this.projectId, this.tokens.readAndWrite, (response, body) => { - expect(body.privilegeLevel).to.equal(false) + expect(response.statusCode).to.equal(403) + expect(body).to.equal('Forbidden') }, cb ) @@ -771,7 +777,8 @@ describe('TokenAccess', function() { this.other2, this.projectId, (response, body) => { - expect(body.privilegeLevel).to.equal(false) + expect(response.statusCode).to.equal(403) + expect(body).to.equal('Forbidden') }, cb ) @@ -865,7 +872,8 @@ describe('TokenAccess', function() { this.other2, this.projectId, (response, body) => { - expect(body.privilegeLevel).to.equal(false) + expect(response.statusCode).to.equal(403) + expect(body).to.equal('Forbidden') }, cb ) diff --git a/services/web/test/unit/src/Editor/EditorHttpControllerTests.js b/services/web/test/unit/src/Editor/EditorHttpControllerTests.js index 5915f5252c..fd65d28dd7 100644 --- a/services/web/test/unit/src/Editor/EditorHttpControllerTests.js +++ b/services/web/test/unit/src/Editor/EditorHttpControllerTests.js @@ -141,6 +141,26 @@ describe('EditorHttpController', function() { }) }) + describe('when no project', function() { + beforeEach(function() { + this.EditorHttpController._buildJoinProjectView = sinon + .stub() + .callsArgWith(3, null, null, null, false) + this.EditorHttpController.joinProject(this.req, this.res) + }) + + it('should send a 403 response', function() { + this.res.json + .calledWith({ + project: null, + privilegeLevel: null, + isRestrictedUser: null + }) + .should.equal(false) + this.res.sendStatus.calledWith(403).should.equal(true) + }) + }) + describe('with an anonymous user', function() { beforeEach(function() { this.req.query = { user_id: 'anonymous-user' }