From f0c0834b0fdb45b4d729c40032f30177bd4a389a Mon Sep 17 00:00:00 2001 From: Ersun Warncke Date: Tue, 25 Sep 2018 05:42:04 -0400 Subject: [PATCH] only do v1 access check when api config present --- .../TokenAccess/TokenAccessController.coffee | 5 +- .../TokenAccess/TokenAccessHandler.coffee | 8 +++ .../TokenAccessControllerTests.coffee | 3 +- .../TokenAccessHandlerTests.coffee | 49 ++++++++++++++++++- 4 files changed, 59 insertions(+), 6 deletions(-) diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee index 49a3892855..f75b72d29e 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee +++ b/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee @@ -4,7 +4,6 @@ TokenAccessHandler = require './TokenAccessHandler' Errors = require '../Errors/Errors' logger = require 'logger-sharelatex' settings = require 'settings-sharelatex' -V1Api = require "../V1/V1Api" module.exports = TokenAccessController = @@ -92,9 +91,9 @@ module.exports = TokenAccessController = return next(new Errors.NotFoundError()) TokenAccessController._tryHigherAccess(token, userId, req, res, next) else - V1Api.request { url: "/api/v1/sharelatex/docs/#{token}/read" }, (err, respose, body) -> + TokenAccessHandler.checkV1Access token, (err, allow_access, redirect_path) -> return next err if err? - return res.redirect body.published_path if body.allow == false + return res.redirect redirect_path unless allow_access if !userId? logger.log {userId, projectId: project._id}, "[TokenAccess] adding anonymous user to project with readOnly token" diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee index ed7f51f0d7..b1778d3f12 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee +++ b/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee @@ -4,6 +4,7 @@ PublicAccessLevels = require '../Authorization/PublicAccessLevels' PrivilegeLevels = require '../Authorization/PrivilegeLevels' ObjectId = require("mongojs").ObjectId Settings = require('settings-sharelatex') +V1Api = require "../V1/V1Api" module.exports = TokenAccessHandler = @@ -97,3 +98,10 @@ module.exports = TokenAccessHandler = project.tokens.readAndWrite = '' if privilegeLevel != PrivilegeLevels.READ_ONLY project.tokens.readOnly = '' + + checkV1Access: (token, callback=(err, allow, redirect)->) -> + return callback(null, true) unless Settings.apis?.v1? + V1Api.request { url: "/api/v1/sharelatex/docs/#{token}/read" }, (err, response, body) -> + return callback err if err? + callback null, false, body.published_path if body.allow == false + callback null, true diff --git a/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee b/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee index 0d65948225..18967534ac 100644 --- a/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee +++ b/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee @@ -396,6 +396,7 @@ describe "TokenAccessController", -> describe 'readOnlyToken', -> beforeEach -> + @TokenAccessHandler.checkV1Access = sinon.stub().callsArgWith(1, null, true) describe 'when access not allowed by v1 api', -> beforeEach -> @@ -405,7 +406,7 @@ describe "TokenAccessController", -> @next = sinon.stub() @TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub() .callsArgWith(1, null, @project) - @V1Api.request = sinon.stub().callsArgWith(1, null, {}, { allow: false, published_path: 'doc-url'} ) + @TokenAccessHandler.checkV1Access = sinon.stub().callsArgWith(1, null, false, 'doc-url') @TokenAccessController.readOnlyToken @req, @res, @next it 'should redirect to doc-url', -> diff --git a/services/web/test/unit/coffee/TokenAccess/TokenAccessHandlerTests.coffee b/services/web/test/unit/coffee/TokenAccess/TokenAccessHandlerTests.coffee index 722a8496a2..d934f31957 100644 --- a/services/web/test/unit/coffee/TokenAccess/TokenAccessHandlerTests.coffee +++ b/services/web/test/unit/coffee/TokenAccess/TokenAccessHandlerTests.coffee @@ -19,9 +19,11 @@ describe "TokenAccessHandler", -> @req = {} @TokenAccessHandler = SandboxedModule.require modulePath, requires: '../../models/Project': {Project: @Project = {}} - 'settings-sharelatex': {} + 'settings-sharelatex': @settings = {} '../Collaborators/CollaboratorsHandler': @CollaboratorsHandler = {} - + '../V1/V1Api': @V1Api = { + request: sinon.stub() + } describe 'findProjectWithReadOnlyToken', -> beforeEach -> @@ -434,3 +436,46 @@ describe "TokenAccessHandler", -> @TokenAccessHandler.protectTokens(@project, 'owner') expect(@project.tokens.readAndWrite).to.equal 'rw' expect(@project.tokens.readOnly).to.equal 'ro' + + describe 'checkV1Access', -> + beforeEach -> + @callback = sinon.stub() + + describe 'when v1 api not set', -> + beforeEach -> + @TokenAccessHandler.checkV1Access @token, @callback + + it 'should not check access and return true', -> + expect(@V1Api.request.called).to.equal false + expect(@callback.calledWith null, true).to.equal true + + describe 'when v1 api is set', -> + beforeEach -> + @settings.apis = { v1: 'v1' } + + describe 'when access allowed', -> + beforeEach -> + @V1Api.request = sinon.stub().callsArgWith(1, null, {}, { allow: true} ) + @TokenAccessHandler.checkV1Access @token, @callback + + it 'should check api', -> + expect(@V1Api.request.calledWith { url: "/api/v1/sharelatex/docs/#{@token}/read" }).to.equal true + + it 'should callback with true', -> + expect(@callback.calledWith null, true).to.equal true + + describe 'when access denied', -> + beforeEach -> + @V1Api.request = sinon.stub().callsArgWith(1, null, {}, { allow: false, published_path: 'doc-url'} ) + @TokenAccessHandler.checkV1Access @token, @callback + + it 'should callback with false and redirect', -> + expect(@callback.calledWith null, false, 'doc-url').to.equal true + + describe 'on error', -> + beforeEach -> + @V1Api.request = sinon.stub().callsArgWith(1, 'error') + @TokenAccessHandler.checkV1Access @token, @callback + + it 'should callback with error', -> + expect(@callback.calledWith 'error').to.equal true