From 237810509adff8dd4b67bd164b031fca87a5d731 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 24 Sep 2018 17:06:11 +0100 Subject: [PATCH 1/6] If no project found for read token, redirect to v1 --- .../TokenAccess/TokenAccessController.coffee | 6 ++- .../TokenAccess/TokenAccessHandler.coffee | 14 +++++-- .../TokenAccessControllerTests.coffee | 18 +++++---- .../TokenAccessHandlerTests.coffee | 39 ++++++++++++++++++- 4 files changed, 63 insertions(+), 14 deletions(-) diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee index 08aa4663f1..ada8c6cc10 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee +++ b/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee @@ -77,11 +77,15 @@ module.exports = TokenAccessController = userId = AuthenticationController.getLoggedInUserId(req) token = req.params['read_only_token'] logger.log {userId, token}, "[TokenAccess] requesting read-only token access" - TokenAccessHandler.findProjectWithReadOnlyToken token, (err, project) -> + TokenAccessHandler.findProjectWithReadOnlyToken token, (err, project, projectExists) -> if err? logger.err {err, token, userId}, "[TokenAccess] error getting project by readOnly token" return next(err) + if !projectExists and settings.overleaf + logger.log {token, userId}, + "[TokenAccess] no project found for this token" + return res.redirect(302, settings.overleaf.host + '/read/' + token) if !project? logger.log {token, userId}, "[TokenAccess] no project found for readOnly token" diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee index ed7f51f0d7..0f785abbc8 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee +++ b/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee @@ -10,11 +10,17 @@ module.exports = TokenAccessHandler = ANONYMOUS_READ_AND_WRITE_ENABLED: Settings.allowAnonymousReadAndWriteSharing == true - findProjectWithReadOnlyToken: (token, callback=(err, project)->) -> + findProjectWithReadOnlyToken: (token, callback=(err, project, projectExists)->) -> Project.findOne { - 'tokens.readOnly': token, - 'publicAccesLevel': PublicAccessLevels.TOKEN_BASED - }, {_id: 1, publicAccesLevel: 1, owner_ref: 1}, callback + 'tokens.readOnly': token + }, {_id: 1, publicAccesLevel: 1, owner_ref: 1}, (err, project) -> + if err? + return callback(err) + if !project? + return callback(null, null, false) + if project.publicAccesLevel != PublicAccessLevels.TOKEN_BASED + return callback(null, null, true) + return callback(null, project, true) findProjectWithReadAndWriteToken: (token, callback=(err, project)->) -> Project.findOne { diff --git a/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee b/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee index 0bdb6d59e6..cb9e6afa39 100644 --- a/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee +++ b/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee @@ -405,7 +405,7 @@ describe "TokenAccessController", -> @next = sinon.stub() @req.params['read_only_token'] = @readOnlyToken @TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub() - .callsArgWith(1, null, @project) + .callsArgWith(1, null, @project, true) @TokenAccessHandler.addReadOnlyUserToProject = sinon.stub() .callsArgWith(2, null) @ProjectController.loadEditor = sinon.stub() @@ -441,7 +441,7 @@ describe "TokenAccessController", -> @req.params['read_only_token'] = @readOnlyToken @project.owner_ref = @userId @TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub() - .callsArgWith(1, null, @project) + .callsArgWith(1, null, @project, true) @TokenAccessHandler.addReadOnlyUserToProject = sinon.stub() .callsArgWith(2, null) @ProjectController.loadEditor = sinon.stub() @@ -513,7 +513,7 @@ describe "TokenAccessController", -> @next = sinon.stub() @req.params['read_and_write_token'] = '123abc' @TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub() - .callsArgWith(1, null, null) + .callsArgWith(1, null, null, true) @TokenAccessHandler.findProjectWithHigherAccess = sinon.stub() .callsArgWith(2, null, @project, false) @@ -626,7 +626,7 @@ describe "TokenAccessController", -> @next = sinon.stub() @req.params['read_only_token'] = @readOnlyToken @TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub() - .callsArgWith(1, null, @project) + .callsArgWith(1, null, @project, true) @TokenAccessHandler.addReadOnlyUserToProject = sinon.stub() .callsArgWith(2, new Error('woops')) @ProjectController.loadEditor = sinon.stub() @@ -670,7 +670,7 @@ describe "TokenAccessController", -> @next = sinon.stub() @req.params['read_only_token'] = @readOnlyToken @TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub() - .callsArgWith(1, null, @project) + .callsArgWith(1, null, @project, true) @TokenAccessHandler.addReadOnlyUserToProject = sinon.stub() .callsArgWith(2, null) @ProjectController.loadEditor = sinon.stub() @@ -748,6 +748,7 @@ describe "TokenAccessController", -> beforeEach -> @req = new MockRequest() @res = new MockResponse() + @res.redirect = sinon.stub() @next = sinon.stub() @req.params['read_only_token'] = @readOnlyToken @TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub() @@ -780,7 +781,10 @@ describe "TokenAccessController", -> done() it 'should call next with a not-found error', (done) -> - expect(@next.callCount).to.equal 1 - expect(@next.lastCall.args[0]).to.be.instanceof Error + expect(@res.redirect.callCount).to.equal 1 + expect(@res.redirect.calledWith( + 302, + "http://overleaf.test:5000/read/#{@readOnlyToken}" + )).to.equal true done() diff --git a/services/web/test/unit/coffee/TokenAccess/TokenAccessHandlerTests.coffee b/services/web/test/unit/coffee/TokenAccess/TokenAccessHandlerTests.coffee index 722a8496a2..8a2e66bb64 100644 --- a/services/web/test/unit/coffee/TokenAccess/TokenAccessHandlerTests.coffee +++ b/services/web/test/unit/coffee/TokenAccess/TokenAccessHandlerTests.coffee @@ -31,8 +31,7 @@ describe "TokenAccessHandler", -> @TokenAccessHandler.findProjectWithReadOnlyToken @token, (err, project) => expect(@Project.findOne.callCount).to.equal 1 expect(@Project.findOne.calledWith({ - 'tokens.readOnly': @token, - 'publicAccesLevel': 'tokenBased' + 'tokens.readOnly': @token })).to.equal true done() @@ -43,6 +42,11 @@ describe "TokenAccessHandler", -> expect(project).to.deep.equal @project done() + it 'should return projectExists flag as true', (done) -> + @TokenAccessHandler.findProjectWithReadOnlyToken @token, (err, project, projectExists) -> + expect(projectExists).to.equal true + done() + describe 'when Project.findOne produces an error', -> beforeEach -> @Project.findOne = sinon.stub().callsArgWith(2, new Error('woops')) @@ -54,6 +58,37 @@ describe "TokenAccessHandler", -> expect(err).to.be.instanceof Error done() + describe 'when project is not tokenBased', -> + beforeEach -> + @project.publicAccesLevel = 'private' + @Project.findOne = sinon.stub().callsArgWith(2, null, @project, true) + + it 'should not return a project', (done) -> + @TokenAccessHandler.findProjectWithReadOnlyToken @token, (err, project) -> + expect(err).to.not.exist + expect(project).to.not.exist + done() + + it 'should return projectExists flag as true', (done) -> + @TokenAccessHandler.findProjectWithReadOnlyToken @token, (err, project, projectExists) -> + expect(projectExists).to.equal true + done() + + describe 'when project does not exist', -> + beforeEach -> + @Project.findOne = sinon.stub().callsArgWith(2, null, null) + + it 'should not return a project', (done) -> + @TokenAccessHandler.findProjectWithReadOnlyToken @token, (err, project) -> + expect(err).to.not.exist + expect(project).to.not.exist + done() + + it 'should return projectExists flag as false', (done) -> + @TokenAccessHandler.findProjectWithReadOnlyToken @token, (err, project, projectExists) -> + expect(projectExists).to.equal false + done() + describe 'findProjectWithReadAndWriteToken', -> beforeEach -> @Project.findOne = sinon.stub().callsArgWith(2, null, @project) From 99dec02266ee69d56ea155ced9d748bd38129ecd Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 24 Sep 2018 18:16:30 +0100 Subject: [PATCH 2/6] If no project found for read/write token, redirect to v1 --- .../TokenAccess/TokenAccessController.coffee | 6 ++- .../TokenAccess/TokenAccessHandler.coffee | 14 +++-- .../TokenAccessControllerTests.coffee | 52 ++++++------------- .../TokenAccessHandlerTests.coffee | 26 ++++++++-- 4 files changed, 53 insertions(+), 45 deletions(-) diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee index ada8c6cc10..6c169b0bac 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee +++ b/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee @@ -34,11 +34,15 @@ module.exports = TokenAccessController = userId = AuthenticationController.getLoggedInUserId(req) token = req.params['read_and_write_token'] logger.log {userId, token}, "[TokenAccess] requesting read-and-write token access" - TokenAccessHandler.findProjectWithReadAndWriteToken token, (err, project) -> + TokenAccessHandler.findProjectWithReadAndWriteToken token, (err, project, projectExists) -> if err? logger.err {err, token, userId}, "[TokenAccess] error getting project by readAndWrite token" return next(err) + if !projectExists and settings.overleaf + logger.log {token, userId}, + "[TokenAccess] no project found for this token" + return res.redirect(302, settings.overleaf.host + '/' + token) if !project? logger.log {token, userId}, "[TokenAccess] no token-based project found for readAndWrite token" diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee index 0f785abbc8..41b7ed0ead 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee +++ b/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee @@ -22,11 +22,17 @@ module.exports = TokenAccessHandler = return callback(null, null, true) return callback(null, project, true) - findProjectWithReadAndWriteToken: (token, callback=(err, project)->) -> + findProjectWithReadAndWriteToken: (token, callback=(err, project, projectExists)->) -> Project.findOne { - 'tokens.readAndWrite': token, - 'publicAccesLevel': PublicAccessLevels.TOKEN_BASED - }, {_id: 1, publicAccesLevel: 1, owner_ref: 1}, callback + 'tokens.readAndWrite': token + }, {_id: 1, publicAccesLevel: 1, owner_ref: 1}, (err, project) -> + if err? + return callback(err) + if !project? + return callback(null, null, false) + if project.publicAccesLevel != PublicAccessLevels.TOKEN_BASED + return callback(null, null, true) + return callback(null, project, true) findProjectWithHigherAccess: (token, userId, callback=(err, project, projectExists)->) -> Project.findOne { diff --git a/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee b/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee index cb9e6afa39..6a5f79151f 100644 --- a/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee +++ b/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee @@ -48,7 +48,7 @@ describe "TokenAccessController", -> @next = sinon.stub() @req.params['read_and_write_token'] = @readAndWriteToken @TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub() - .callsArgWith(1, null, @project) + .callsArgWith(1, null, @project, true) @TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub() .callsArgWith(2, null) @ProjectController.loadEditor = sinon.stub() @@ -85,7 +85,7 @@ describe "TokenAccessController", -> @req.params['read_and_write_token'] = @readAndWriteToken @project.owner_ref = @userId @TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub() - .callsArgWith(1, null, @project) + .callsArgWith(1, null, @project, true) @TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub() .callsArgWith(2, null) @ProjectController.loadEditor = sinon.stub() @@ -123,7 +123,7 @@ describe "TokenAccessController", -> @next = sinon.stub() @req.params['read_and_write_token'] = @readAndWriteToken @TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub() - .callsArgWith(1, null, @project) + .callsArgWith(1, null, @project, true) @TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub() .callsArgWith(2, null) @ProjectController.loadEditor = sinon.stub() @@ -159,7 +159,7 @@ describe "TokenAccessController", -> @next = sinon.stub() @req.params['read_and_write_token'] = @readAndWriteToken @TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub() - .callsArgWith(1, null, @project) + .callsArgWith(1, null, @project, true) @TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub() .callsArgWith(2, null) @ProjectController.loadEditor = sinon.stub() @@ -244,7 +244,7 @@ describe "TokenAccessController", -> @next = sinon.stub() @req.params['read_and_write_token'] = '123abc' @TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub() - .callsArgWith(1, null, null) + .callsArgWith(1, null, null, false) @TokenAccessHandler.findProjectWithHigherAccess = sinon.stub() .callsArgWith(2, null, @project, false) @@ -252,8 +252,10 @@ describe "TokenAccessController", -> it 'should redirect to v1', (done) -> expect(@res.redirect.callCount).to.equal 1 - expect(@res.redirect.firstCall.args[0]) - .to.equal 'http://overleaf.test:5000/123abc' + expect(@res.redirect.calledWith( + 302, + 'http://overleaf.test:5000/123abc' + )).to.equal true done() describe 'when token access is off, but user has higher access anyway', -> @@ -264,7 +266,7 @@ describe "TokenAccessController", -> @next = sinon.stub() @req.params['read_and_write_token'] = @readAndWriteToken @TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub() - .callsArgWith(1, null, null) + .callsArgWith(1, null, null, true) @TokenAccessHandler.findProjectWithHigherAccess = sinon.stub() .callsArgWith(2, null, @project, true) @@ -313,7 +315,7 @@ describe "TokenAccessController", -> @next = sinon.stub() @req.params['read_and_write_token'] = @readAndWriteToken @TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub() - .callsArgWith(1, null, null) + .callsArgWith(1, null, null, true) @TokenAccessHandler.findProjectWithHigherAccess = sinon.stub() .callsArgWith(2, null, null, true) @@ -358,7 +360,7 @@ describe "TokenAccessController", -> @next = sinon.stub() @req.params['read_and_write_token'] = @readAndWriteToken @TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub() - .callsArgWith(1, null, @project) + .callsArgWith(1, null, @project, true) @TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub() .callsArgWith(2, new Error('woops')) @ProjectController.loadEditor = sinon.stub() @@ -500,31 +502,7 @@ describe "TokenAccessController", -> expect(@next.lastCall.args[0]).to.be.instanceof Error done() - ## describe 'when findProject does not find a project', -> - beforeEach -> - - describe 'when project does not exist', -> - beforeEach -> - @req = new MockRequest() - @req.url = '/123abc' - @res = new MockResponse() - @res.redirect = sinon.stub() - @next = sinon.stub() - @req.params['read_and_write_token'] = '123abc' - @TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub() - .callsArgWith(1, null, null, true) - @TokenAccessHandler.findProjectWithHigherAccess = - sinon.stub() - .callsArgWith(2, null, @project, false) - @TokenAccessController.readOnlyToken @req, @res, @next - - it 'should return a ProjectNotTokenAccessError', (done) -> - expect(@res.redirect.callCount).to.equal 1 - expect(@res.redirect.firstCall.args[0]) - .to.equal 'http://overleaf.test:5000/123abc' - done() - describe 'when token access is off, but user has higher access anyway', -> beforeEach -> @req = new MockRequest() @@ -533,7 +511,7 @@ describe "TokenAccessController", -> @next = sinon.stub() @req.params['read_and_write_token'] = @readAndWriteToken @TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub() - .callsArgWith(1, null, null) + .callsArgWith(1, null, null, true) @TokenAccessHandler.findProjectWithHigherAccess = sinon.stub() .callsArgWith(2, null, @project, true) @@ -581,7 +559,7 @@ describe "TokenAccessController", -> @next = sinon.stub() @req.params['read_and_write_token'] = @readAndWriteToken @TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub() - .callsArgWith(1, null, null) + .callsArgWith(1, null, null, true) @TokenAccessHandler.findProjectWithHigherAccess = sinon.stub() .callsArgWith(2, null, null, true) @@ -780,7 +758,7 @@ describe "TokenAccessController", -> .to.equal 0 done() - it 'should call next with a not-found error', (done) -> + it 'should redirect to v1', (done) -> expect(@res.redirect.callCount).to.equal 1 expect(@res.redirect.calledWith( 302, diff --git a/services/web/test/unit/coffee/TokenAccess/TokenAccessHandlerTests.coffee b/services/web/test/unit/coffee/TokenAccess/TokenAccessHandlerTests.coffee index 8a2e66bb64..ed6ba79877 100644 --- a/services/web/test/unit/coffee/TokenAccess/TokenAccessHandlerTests.coffee +++ b/services/web/test/unit/coffee/TokenAccess/TokenAccessHandlerTests.coffee @@ -58,7 +58,7 @@ describe "TokenAccessHandler", -> expect(err).to.be.instanceof Error done() - describe 'when project is not tokenBased', -> + describe 'when project does not have tokenBased access level', -> beforeEach -> @project.publicAccesLevel = 'private' @Project.findOne = sinon.stub().callsArgWith(2, null, @project, true) @@ -97,8 +97,7 @@ describe "TokenAccessHandler", -> @TokenAccessHandler.findProjectWithReadAndWriteToken @token, (err, project) => expect(@Project.findOne.callCount).to.equal 1 expect(@Project.findOne.calledWith({ - 'tokens.readAndWrite': @token, - 'publicAccesLevel': 'tokenBased' + 'tokens.readAndWrite': @token })).to.equal true done() @@ -109,6 +108,11 @@ describe "TokenAccessHandler", -> expect(project).to.deep.equal @project done() + it 'should return projectExists flag as true', (done) -> + @TokenAccessHandler.findProjectWithReadAndWriteToken @token, (err, project, projectExists) -> + expect(projectExists).to.equal true + done() + describe 'when Project.findOne produces an error', -> beforeEach -> @Project.findOne = sinon.stub().callsArgWith(2, new Error('woops')) @@ -120,6 +124,22 @@ describe "TokenAccessHandler", -> expect(err).to.be.instanceof Error done() + describe 'when project does not have tokenBased access level', -> + beforeEach -> + @project.publicAccesLevel = 'private' + @Project.findOne = sinon.stub().callsArgWith(2, null, @project, true) + + it 'should not return a project', (done) -> + @TokenAccessHandler.findProjectWithReadAndWriteToken @token, (err, project) -> + expect(err).to.not.exist + expect(project).to.not.exist + done() + + it 'should return projectExists flag as true', (done) -> + @TokenAccessHandler.findProjectWithReadAndWriteToken @token, (err, project, projectExists) -> + expect(projectExists).to.equal true + done() + describe 'findProjectWithHigherAccess', -> describe 'when user does have higher access', -> From d6350c963e35ffbc1c3d3735ce566edc3ef9102d Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 24 Sep 2018 18:31:07 +0100 Subject: [PATCH 3/6] Remove projectExists flag from higher access check Now that find project by read and read/write token methods check whether the project exists, it is not neccessary to check whether the project exists in the higher access check. Therefore it has been removed --- .../TokenAccess/TokenAccessController.coffee | 7 +------ .../TokenAccess/TokenAccessHandler.coffee | 15 +++++++-------- .../TokenAccess/TokenAccessControllerTests.coffee | 10 +++++----- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee index 6c169b0bac..d5d704703d 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee +++ b/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee @@ -12,16 +12,11 @@ module.exports = TokenAccessController = return ProjectController.loadEditor(req, res, next) _tryHigherAccess: (token, userId, req, res, next) -> - TokenAccessHandler.findProjectWithHigherAccess token, userId, (err, project, projectExists) -> + TokenAccessHandler.findProjectWithHigherAccess token, userId, (err, project) -> if err? logger.err {err, token, userId}, "[TokenAccess] error finding project with higher access" return next(err) - if !projectExists and settings.overleaf - logger.log {token, userId}, - "[TokenAccess] no project found for this token" - # Project does not exist, but may be unimported - try it on v1 - return res.redirect(settings.overleaf.host + req.url) if !project? logger.log {token, userId}, "[TokenAccess] no project with higher access found for this user and token" diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee index 41b7ed0ead..d2dedca76e 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee +++ b/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee @@ -17,9 +17,9 @@ module.exports = TokenAccessHandler = if err? return callback(err) if !project? - return callback(null, null, false) + return callback(null, null, false) # Project doesn't exist, so we handle differently if project.publicAccesLevel != PublicAccessLevels.TOKEN_BASED - return callback(null, null, true) + return callback(null, null, true) # Project does exist, but it isn't token based return callback(null, project, true) findProjectWithReadAndWriteToken: (token, callback=(err, project, projectExists)->) -> @@ -29,12 +29,12 @@ module.exports = TokenAccessHandler = if err? return callback(err) if !project? - return callback(null, null, false) + return callback(null, null, false) # Project doesn't exist, so we handle differently if project.publicAccesLevel != PublicAccessLevels.TOKEN_BASED - return callback(null, null, true) + return callback(null, null, true) # Project does exist, but it isn't token based return callback(null, project, true) - findProjectWithHigherAccess: (token, userId, callback=(err, project, projectExists)->) -> + findProjectWithHigherAccess: (token, userId, callback=(err, project)->) -> Project.findOne { $or: [ {'tokens.readAndWrite': token}, @@ -44,15 +44,14 @@ module.exports = TokenAccessHandler = if err? return callback(err) if !project? - return callback(null, null, false) # Project doesn't exist, so we handle differently + return callback(null, null) projectId = project._id CollaboratorsHandler.isUserInvitedMemberOfProject userId, projectId, (err, isMember) -> if err? return callback(err) callback( null, - if isMember == true then project else null, - true # Project does exist, but user doesn't have access + if isMember == true then project else null ) addReadOnlyUserToProject: (userId, projectId, callback=(err)->) -> diff --git a/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee b/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee index 6a5f79151f..97312ab415 100644 --- a/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee +++ b/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee @@ -247,7 +247,7 @@ describe "TokenAccessController", -> .callsArgWith(1, null, null, false) @TokenAccessHandler.findProjectWithHigherAccess = sinon.stub() - .callsArgWith(2, null, @project, false) + .callsArgWith(2, null, @project) @TokenAccessController.readAndWriteToken @req, @res, @next it 'should redirect to v1', (done) -> @@ -269,7 +269,7 @@ describe "TokenAccessController", -> .callsArgWith(1, null, null, true) @TokenAccessHandler.findProjectWithHigherAccess = sinon.stub() - .callsArgWith(2, null, @project, true) + .callsArgWith(2, null, @project) @TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub() .callsArgWith(2, null) @ProjectController.loadEditor = sinon.stub() @@ -318,7 +318,7 @@ describe "TokenAccessController", -> .callsArgWith(1, null, null, true) @TokenAccessHandler.findProjectWithHigherAccess = sinon.stub() - .callsArgWith(2, null, null, true) + .callsArgWith(2, null, null) @TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub() .callsArgWith(2, null) @ProjectController.loadEditor = sinon.stub() @@ -514,7 +514,7 @@ describe "TokenAccessController", -> .callsArgWith(1, null, null, true) @TokenAccessHandler.findProjectWithHigherAccess = sinon.stub() - .callsArgWith(2, null, @project, true) + .callsArgWith(2, null, @project) @TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub() .callsArgWith(2, null) @ProjectController.loadEditor = sinon.stub() @@ -562,7 +562,7 @@ describe "TokenAccessController", -> .callsArgWith(1, null, null, true) @TokenAccessHandler.findProjectWithHigherAccess = sinon.stub() - .callsArgWith(2, null, null, true) + .callsArgWith(2, null, null) @TokenAccessHandler.addReadOnlyUserToProject = sinon.stub() .callsArgWith(2, null) @ProjectController.loadEditor = sinon.stub() From ca895ae1b1484259a199b0e8fc55b2e99b07cf0c Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Tue, 25 Sep 2018 09:37:22 +0100 Subject: [PATCH 4/6] Redirect to v1 via sign in link --- .../coffee/Features/TokenAccess/TokenAccessController.coffee | 2 +- services/web/test/acceptance/coffee/TokenAccessTests.coffee | 2 +- .../unit/coffee/TokenAccess/TokenAccessControllerTests.coffee | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee index d5d704703d..504a7fc24f 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee +++ b/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee @@ -37,7 +37,7 @@ module.exports = TokenAccessController = if !projectExists and settings.overleaf logger.log {token, userId}, "[TokenAccess] no project found for this token" - return res.redirect(302, settings.overleaf.host + '/' + token) + return res.redirect(302, "/sign_in_to_v1?return_to=#{settings.overleaf.host}/#{token}") if !project? logger.log {token, userId}, "[TokenAccess] no token-based project found for readAndWrite token" diff --git a/services/web/test/acceptance/coffee/TokenAccessTests.coffee b/services/web/test/acceptance/coffee/TokenAccessTests.coffee index b0fbd7a0a1..a2e6a1ee7c 100644 --- a/services/web/test/acceptance/coffee/TokenAccessTests.coffee +++ b/services/web/test/acceptance/coffee/TokenAccessTests.coffee @@ -422,6 +422,6 @@ describe 'TokenAccess', -> try_read_and_write_token_access(@owner, unimportedV1Token, (response, body) => expect(response.statusCode).to.equal 302 expect(response.headers.location).to.equal( - 'http://overleaf.test:5000/123abc' + '/sign_in_to_v1?return_to=http://overleaf.test:5000/123abc' ) , done) diff --git a/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee b/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee index 97312ab415..a94a502396 100644 --- a/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee +++ b/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee @@ -254,7 +254,7 @@ describe "TokenAccessController", -> expect(@res.redirect.callCount).to.equal 1 expect(@res.redirect.calledWith( 302, - 'http://overleaf.test:5000/123abc' + '/sign_in_to_v1?return_to=http://overleaf.test:5000/123abc' )).to.equal true done() From da16e8d01f49752bb143333341f460bbbffd705a Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Tue, 25 Sep 2018 09:43:39 +0100 Subject: [PATCH 5/6] Add acceptance test for unimported read only token --- .../test/acceptance/coffee/TokenAccessTests.coffee | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/services/web/test/acceptance/coffee/TokenAccessTests.coffee b/services/web/test/acceptance/coffee/TokenAccessTests.coffee index a2e6a1ee7c..8636012f23 100644 --- a/services/web/test/acceptance/coffee/TokenAccessTests.coffee +++ b/services/web/test/acceptance/coffee/TokenAccessTests.coffee @@ -417,7 +417,7 @@ describe 'TokenAccess', -> , done) describe 'unimported v1 project', -> - it 'should redirect to v1', (done) -> + it 'should redirect read and write token to v1', (done) -> unimportedV1Token = '123abc' try_read_and_write_token_access(@owner, unimportedV1Token, (response, body) => expect(response.statusCode).to.equal 302 @@ -425,3 +425,12 @@ describe 'TokenAccess', -> '/sign_in_to_v1?return_to=http://overleaf.test:5000/123abc' ) , done) + + it 'should redirect read only token to v1', (done) -> + unimportedV1Token = 'abcd' + try_read_only_token_access(@owner, unimportedV1Token, (response, body) => + expect(response.statusCode).to.equal 302 + expect(response.headers.location).to.equal( + 'http://overleaf.test:5000/read/abcd' + ) + , done) From 298ee2dbb494c733583a6b5831fc67c9f6c79c8d Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Tue, 25 Sep 2018 10:06:24 +0100 Subject: [PATCH 6/6] Fix v1 return to path --- .../coffee/Features/TokenAccess/TokenAccessController.coffee | 2 +- services/web/test/acceptance/coffee/TokenAccessTests.coffee | 2 +- .../unit/coffee/TokenAccess/TokenAccessControllerTests.coffee | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee index 504a7fc24f..a0bd471630 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee +++ b/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee @@ -37,7 +37,7 @@ module.exports = TokenAccessController = if !projectExists and settings.overleaf logger.log {token, userId}, "[TokenAccess] no project found for this token" - return res.redirect(302, "/sign_in_to_v1?return_to=#{settings.overleaf.host}/#{token}") + return res.redirect(302, "/sign_in_to_v1?return_to=/#{token}") if !project? logger.log {token, userId}, "[TokenAccess] no token-based project found for readAndWrite token" diff --git a/services/web/test/acceptance/coffee/TokenAccessTests.coffee b/services/web/test/acceptance/coffee/TokenAccessTests.coffee index 8636012f23..31be9523ba 100644 --- a/services/web/test/acceptance/coffee/TokenAccessTests.coffee +++ b/services/web/test/acceptance/coffee/TokenAccessTests.coffee @@ -422,7 +422,7 @@ describe 'TokenAccess', -> try_read_and_write_token_access(@owner, unimportedV1Token, (response, body) => expect(response.statusCode).to.equal 302 expect(response.headers.location).to.equal( - '/sign_in_to_v1?return_to=http://overleaf.test:5000/123abc' + '/sign_in_to_v1?return_to=/123abc' ) , done) diff --git a/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee b/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee index a94a502396..a0177a79a7 100644 --- a/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee +++ b/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee @@ -254,7 +254,7 @@ describe "TokenAccessController", -> expect(@res.redirect.callCount).to.equal 1 expect(@res.redirect.calledWith( 302, - '/sign_in_to_v1?return_to=http://overleaf.test:5000/123abc' + '/sign_in_to_v1?return_to=/123abc' )).to.equal true done()