From 3d272ca297075cf2473697bf862a25e9573bd163 Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Mon, 18 Jun 2018 18:37:58 +0200 Subject: [PATCH 1/2] replace OldAssetProxy --- .../infrastructure/OldAssetProxy.coffee | 16 ----- .../coffee/infrastructure/ProxyManager.coffee | 43 +++++++++++++ .../app/coffee/infrastructure/Server.coffee | 4 +- .../infrastructure/ProxyManagerTests.coffee | 64 +++++++++++++++++++ 4 files changed, 109 insertions(+), 18 deletions(-) delete mode 100644 services/web/app/coffee/infrastructure/OldAssetProxy.coffee create mode 100644 services/web/app/coffee/infrastructure/ProxyManager.coffee create mode 100644 services/web/test/unit/coffee/infrastructure/ProxyManagerTests.coffee diff --git a/services/web/app/coffee/infrastructure/OldAssetProxy.coffee b/services/web/app/coffee/infrastructure/OldAssetProxy.coffee deleted file mode 100644 index 7912f0ed14..0000000000 --- a/services/web/app/coffee/infrastructure/OldAssetProxy.coffee +++ /dev/null @@ -1,16 +0,0 @@ -settings = require("settings-sharelatex") -logger = require("logger-sharelatex") -request = require("request") - -module.exports = (req, res, next)-> - requestedUrl = req.url - - redirectUrl = settings.proxyUrls[requestedUrl] - if redirectUrl? - logger.log redirectUrl:redirectUrl, reqUrl:req.url, "proxying url" - upstream = request(redirectUrl) - upstream.on "error", (error) -> - logger.error err: error, "error in OldAssetProxy" - upstream.pipe(res) - else - next() diff --git a/services/web/app/coffee/infrastructure/ProxyManager.coffee b/services/web/app/coffee/infrastructure/ProxyManager.coffee new file mode 100644 index 0000000000..45ca6b7a48 --- /dev/null +++ b/services/web/app/coffee/infrastructure/ProxyManager.coffee @@ -0,0 +1,43 @@ +settings = require("settings-sharelatex") +logger = require("logger-sharelatex") +httpProxy = require 'express-http-proxy' + +module.exports = + # add proxy for all paths listed in `settings.proxyUrls`and log errors + apply: (app) -> + for requestUrl, target of settings.proxyUrls + targetUrl = @makeTargetUrl(target) + if targetUrl? + app.use requestUrl, httpProxy(targetUrl) + else + logger.error "Cannot make proxy target from #{target}" + + # takes a 'target' and return an URL to proxy to. + # 'target' can be: + # - a String, representing the URL + # - an Object with: + # - a path attribute (String) + # - a baseURL attribute (String) + # - a baseURL attribute (Object) with: + # - a setting attribute pointing to a value in the settings + makeTargetUrl: (target) -> + return target if typeof target is 'string' + return target.path unless target.baseUrl? + + if typeof target.baseUrl is 'string' + baseUrl = target.baseUrl + else if target.baseUrl.setting? + baseUrl = digSettingValue target.baseUrl.setting + + return null unless baseUrl? + "#{baseUrl}#{target.path}" + +# given a setting path (e.g. 'apis.v1.url') recursively find the corresponding +# settings value +digSettingValue = (attributesPath, dig = null) -> + dig ||= settings + [nextAttribute, leftAttributes...] = attributesPath.split('.') + dig = dig[nextAttribute] + return null unless dig? + return dig if leftAttributes.length == 0 + digSettingValue(leftAttributes.join('.'), dig) diff --git a/services/web/app/coffee/infrastructure/Server.coffee b/services/web/app/coffee/infrastructure/Server.coffee index 39fc3c131b..522599c659 100644 --- a/services/web/app/coffee/infrastructure/Server.coffee +++ b/services/web/app/coffee/infrastructure/Server.coffee @@ -32,7 +32,7 @@ Mongoose = require("./Mongoose") oneDayInMilliseconds = 86400000 ReferalConnect = require('../Features/Referal/ReferalConnect') RedirectManager = require("./RedirectManager") -OldAssetProxy = require("./OldAssetProxy") +ProxyManager = require("./ProxyManager") translations = require("translations-sharelatex").setup(Settings.i18n) Modules = require "./Modules" @@ -74,7 +74,7 @@ app.use methodOverride() app.use metrics.http.monitor(logger) app.use RedirectManager -app.use OldAssetProxy +ProxyManager.apply(app) webRouter.use cookieParser(Settings.security.sessionSecret) diff --git a/services/web/test/unit/coffee/infrastructure/ProxyManagerTests.coffee b/services/web/test/unit/coffee/infrastructure/ProxyManagerTests.coffee new file mode 100644 index 0000000000..3c3c4c7ccd --- /dev/null +++ b/services/web/test/unit/coffee/infrastructure/ProxyManagerTests.coffee @@ -0,0 +1,64 @@ +sinon = require('sinon') +chai = require('chai') +should = chai.should() +expect = chai.expect +modulePath = '../../../../app/js/infrastructure/ProxyManager' +SandboxedModule = require('sandboxed-module') +MockRequest = require "../helpers/MockRequest" +MockResponse = require "../helpers/MockResponse" + +describe "ProxyManager", -> + before -> + @settings = proxyUrls: {} + @httpProxy = sinon.stub() + @proxyManager = SandboxedModule.require modulePath, requires: + 'settings-sharelatex': @settings + 'logger-sharelatex': + error: -> + log: -> console.log(arguments) + 'httpProxy': @httpProxy + + describe 'apply', -> + before -> + @sandbox = sinon.sandbox.create() + @app = use: sinon.stub() + @sandbox.stub(@proxyManager, 'makeTargetUrl') + + after -> + @sandbox.restore() + + beforeEach -> + @app.use.reset() + @requestUrl = '/foo/bar' + @settings.proxyUrls[@requestUrl] = 'http://whatever' + + it 'sets routes', -> + @proxyManager.makeTargetUrl.returns('http://whatever') + @proxyManager.apply(@app) + @app.use.calledWith(@requestUrl).should.equal true + + it 'logs errors', -> + @proxyManager.makeTargetUrl.returns(null) + @proxyManager.apply(@app) + @app.use.called.should.equal false + + describe 'makeTargetUrl', -> + it 'returns Strings', -> + target = 'http://whatever' + @proxyManager.makeTargetUrl(target).should.equal target + + it 'makes from object', -> + target = path: 'baz' + @proxyManager.makeTargetUrl(target).should.equal 'baz' + + it 'makes with baseUrl', -> + target = path: 'baz', baseUrl: 'foo.bar/' + @proxyManager.makeTargetUrl(target).should.equal 'foo.bar/baz' + + it 'makes with settingsUrl', -> + @settings.apis = v1: url: 'foo.bar/' + target = path: 'baz', baseUrl: { setting: 'apis.v1.url' } + @proxyManager.makeTargetUrl(target).should.equal 'foo.bar/baz' + + target = path: 'baz', baseUrl: { setting: 'incorrect.setting' } + expect(@proxyManager.makeTargetUrl(target)).to.equal null From bbed5fca9ab6f140b339562df56132426e226ad3 Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Wed, 20 Jun 2018 10:58:19 +0200 Subject: [PATCH 2/2] simplify proxy --- .../coffee/infrastructure/ProxyManager.coffee | 20 +------------------ .../infrastructure/ProxyManagerTests.coffee | 14 +++++-------- 2 files changed, 6 insertions(+), 28 deletions(-) diff --git a/services/web/app/coffee/infrastructure/ProxyManager.coffee b/services/web/app/coffee/infrastructure/ProxyManager.coffee index 45ca6b7a48..d93c696ab1 100644 --- a/services/web/app/coffee/infrastructure/ProxyManager.coffee +++ b/services/web/app/coffee/infrastructure/ProxyManager.coffee @@ -18,26 +18,8 @@ module.exports = # - an Object with: # - a path attribute (String) # - a baseURL attribute (String) - # - a baseURL attribute (Object) with: - # - a setting attribute pointing to a value in the settings makeTargetUrl: (target) -> return target if typeof target is 'string' return target.path unless target.baseUrl? + "#{target.baseUrl}#{target.path or ''}" - if typeof target.baseUrl is 'string' - baseUrl = target.baseUrl - else if target.baseUrl.setting? - baseUrl = digSettingValue target.baseUrl.setting - - return null unless baseUrl? - "#{baseUrl}#{target.path}" - -# given a setting path (e.g. 'apis.v1.url') recursively find the corresponding -# settings value -digSettingValue = (attributesPath, dig = null) -> - dig ||= settings - [nextAttribute, leftAttributes...] = attributesPath.split('.') - dig = dig[nextAttribute] - return null unless dig? - return dig if leftAttributes.length == 0 - digSettingValue(leftAttributes.join('.'), dig) diff --git a/services/web/test/unit/coffee/infrastructure/ProxyManagerTests.coffee b/services/web/test/unit/coffee/infrastructure/ProxyManagerTests.coffee index 3c3c4c7ccd..5782a4e2fc 100644 --- a/services/web/test/unit/coffee/infrastructure/ProxyManagerTests.coffee +++ b/services/web/test/unit/coffee/infrastructure/ProxyManagerTests.coffee @@ -47,18 +47,14 @@ describe "ProxyManager", -> target = 'http://whatever' @proxyManager.makeTargetUrl(target).should.equal target - it 'makes from object', -> + it 'makes with path', -> target = path: 'baz' @proxyManager.makeTargetUrl(target).should.equal 'baz' it 'makes with baseUrl', -> + target = baseUrl: 'foo.bar' + @proxyManager.makeTargetUrl(target).should.equal 'foo.bar' + + it 'makes with baseUrl and path', -> target = path: 'baz', baseUrl: 'foo.bar/' @proxyManager.makeTargetUrl(target).should.equal 'foo.bar/baz' - - it 'makes with settingsUrl', -> - @settings.apis = v1: url: 'foo.bar/' - target = path: 'baz', baseUrl: { setting: 'apis.v1.url' } - @proxyManager.makeTargetUrl(target).should.equal 'foo.bar/baz' - - target = path: 'baz', baseUrl: { setting: 'incorrect.setting' } - expect(@proxyManager.makeTargetUrl(target)).to.equal null