Merge pull request #680 from sharelatex/ta-smart-proxy-take2

Replace OldAssetProxy take 2

services/web/app/coffee/infrastructure/OldAssetProxy.coffee

@@ -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()

services/web/app/coffee/infrastructure/ProxyManager.coffee

@@ -0,0 +1,36 @@
+settings = require 'settings-sharelatex'
+logger = require 'logger-sharelatex'
+request = require 'request'
+URL = require 'url'
+
+module.exports = ProxyManager =
+	call: (req, res, next) ->
+		requestUrl = URL.parse(req.url)
+		requestPath = requestUrl.pathname # ignore the query part
+
+		target = settings.proxyUrls[requestPath]
+		return next() unless target? # nothing to proxy
+
+		targetUrl = makeTargetUrl(target, req.query)
+		logger.log targetUrl: targetUrl, reqUrl: req.url, "proxying url"
+
+		upstream = request(targetUrl)
+		upstream.on "error", (error) ->
+			logger.error err: error, "error in ProxyManager"
+
+		# TODO: better handling of status code
+		# see https://github.com/overleaf/write_latex/wiki/Streams-and-pipes-in-Node.js
+		upstream.pipe(res)
+
+# make a URL from a proxy target.
+# if the query is specified, set/replace the target's query with the given query
+makeTargetUrl = (target, query) ->
+	targetUrl = URL.parse(parseSettingUrl(target))
+	if query? and Object.keys(query).length > 0
+		targetUrl.query = query
+		targetUrl.search = null # clear `search` as it takes precedence over `query`
+	targetUrl.format()
+
+parseSettingUrl = (target) ->
+	return target if typeof target is 'string'
+	"#{target.baseUrl}#{target.path or ''}"

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
+app.use ProxyManager.call
 
 
 webRouter.use cookieParser(Settings.security.sessionSecret)

services/web/test/unit/coffee/infrastructure/ProxyManagerTests.coffee

@@ -0,0 +1,66 @@
+sinon = require('sinon')
+assertCalledWith = sinon.assert.calledWith
+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: {}
+		@request = sinon.stub().returns(
+			on: ()->
+			pipe: ()->
+		)
+		@proxyManager = SandboxedModule.require modulePath, requires:
+			'settings-sharelatex': @settings
+			'logger-sharelatex': log: ->
+			'request': @request
+		@proxyPath = '/foo/bar'
+		@req = new MockRequest()
+		@res = new MockResponse()
+		@next = sinon.stub()
+
+	describe 'proxyUrls', ->
+		beforeEach ->
+			@req.url = @proxyPath
+			@req.query = {}
+			@settings.proxyUrls = {}
+
+		afterEach ->
+			@next.reset()
+			@request.reset()
+
+		it 'calls next when no match', ->
+			@proxyManager.call(@req, @res, @next)
+			sinon.assert.called(@next)
+			sinon.assert.notCalled(@request)
+
+		it 'does not calls next when match', ->
+			@settings.proxyUrls[@proxyPath] = '/'
+			@proxyManager.call(@req, @res, @next)
+			sinon.assert.notCalled(@next)
+			sinon.assert.called(@request)
+
+		it 'proxy full URL', ->
+			targetUrl = 'https://user:pass@foo.bar:123/pa/th.ext?query#hash'
+			@settings.proxyUrls[@proxyPath] = targetUrl
+			@proxyManager.call(@req)
+			assertCalledWith(@request, targetUrl)
+
+		it 'overwrite query', ->
+			targetUrl = 'foo.bar/baz?query'
+			@req.query = { requestQuery: 'important' }
+			@settings.proxyUrls[@proxyPath] = targetUrl
+			@proxyManager.call(@req)
+			newTargetUrl = 'foo.bar/baz?requestQuery=important'
+			assertCalledWith(@request, newTargetUrl)
+
+		it 'handles target objects', ->
+			targetUrl = { baseUrl: 'api.v1', path: '/pa/th'}
+			@settings.proxyUrls[@proxyPath] = targetUrl
+			@proxyManager.call(@req, @res, @next)
+			assertCalledWith(@request, 'api.v1/pa/th')