diff --git a/services/web/app/coffee/infrastructure/ProxyManager.coffee b/services/web/app/coffee/infrastructure/ProxyManager.coffee index 3895ef6e2f..4d64b25bae 100644 --- a/services/web/app/coffee/infrastructure/ProxyManager.coffee +++ b/services/web/app/coffee/infrastructure/ProxyManager.coffee @@ -4,33 +4,37 @@ 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 + apply: (publicApiRouter) -> + for proxyUrl, target of settings.proxyUrls + do (target) -> + publicApiRouter.get proxyUrl, ProxyManager.createProxy(target) - target = settings.proxyUrls[requestPath] - return next() unless target? # nothing to proxy + createProxy: (target) -> + (req, res, next) -> + targetUrl = makeTargetUrl(target, req) + logger.log targetUrl: targetUrl, reqUrl: req.url, "proxying url" - 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" - 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) + # 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 +makeTargetUrl = (target, req) -> + targetUrl = URL.parse(parseSettingUrl(target, req)) + if req.query? and Object.keys(req.query).length > 0 + targetUrl.query = req.query targetUrl.search = null # clear `search` as it takes precedence over `query` targetUrl.format() -parseSettingUrl = (target) -> +parseSettingUrl = (target, { params }) -> return target if typeof target is 'string' - "#{target.baseUrl}#{target.path or ''}" + if typeof target.path is 'function' + path = target.path(params) + else + path = target.path + "#{target.baseUrl}#{path or ''}" diff --git a/services/web/app/coffee/infrastructure/Server.coffee b/services/web/app/coffee/infrastructure/Server.coffee index 37359ac797..cf2757c80e 100644 --- a/services/web/app/coffee/infrastructure/Server.coffee +++ b/services/web/app/coffee/infrastructure/Server.coffee @@ -74,7 +74,7 @@ app.use methodOverride() app.use metrics.http.monitor(logger) app.use RedirectManager -app.use ProxyManager.call +ProxyManager.apply(publicApiRouter) webRouter.use cookieParser(Settings.security.sessionSecret) diff --git a/services/web/config/settings.defaults.coffee b/services/web/config/settings.defaults.coffee index 9d471e6642..a9383714c9 100644 --- a/services/web/config/settings.defaults.coffee +++ b/services/web/config/settings.defaults.coffee @@ -425,10 +425,6 @@ module.exports = settings = redirects: "/templates/index": "/templates/" - proxyUrls: - '/institutions/list': { baseUrl: v1Api.url, path: '/universities/list' } - '/institutions/domains': { baseUrl: v1Api.url, path: '/university/domains' } - reloadModuleViewsOnEachRequest: true domainLicences: [ diff --git a/services/web/test/acceptance/coffee/ProxyUrls.coffee b/services/web/test/acceptance/coffee/ProxyUrls.coffee new file mode 100644 index 0000000000..5767d42bef --- /dev/null +++ b/services/web/test/acceptance/coffee/ProxyUrls.coffee @@ -0,0 +1,42 @@ +should = require('chai').should() +assert = require('chai').assert +async = require("async") +request = require "./helpers/request" +MockV1Api = require "./helpers/MockV1Api" + +assertResponse = (path, expectedStatusCode, expectedBody, cb) -> + request.get path, (error, response) -> + should.not.exist error + response.statusCode.should.equal expectedStatusCode + assert.deepEqual(JSON.parse(response.body), expectedBody) if expectedBody + cb() + +describe "ProxyUrls", -> + before -> + @timeout(1000) + + it 'proxy static URLs', (done) -> + async.series [ + (cb) -> assertResponse '/institutions/list', 200, [], cb + (cb) -> assertResponse '/institutions/domains', 200, [], cb + ], + done + + it 'proxy dynamic URLs', (done) -> + async.series [ + (cb) -> assertResponse '/institutions/list/123', 200, { id: 123 }, cb + (cb) -> assertResponse '/institutions/list/456', 200, { id: 456 }, cb + ], + done + + it 'return 404 if proxy is not set', (done) -> + async.series [ + (cb) -> assertResponse '/institutions/foobar', 404, null, cb + ], + done + + it 'handle missing baseUrl', (done) -> + async.series [ + (cb) -> assertResponse '/proxy/missing/baseUrl', 500, null, cb + ], + done diff --git a/services/web/test/acceptance/coffee/helpers/MockV1Api.coffee b/services/web/test/acceptance/coffee/helpers/MockV1Api.coffee index 91e3f0b25c..f537614f17 100644 --- a/services/web/test/acceptance/coffee/helpers/MockV1Api.coffee +++ b/services/web/test/acceptance/coffee/helpers/MockV1Api.coffee @@ -51,6 +51,15 @@ module.exports = MockV1Api = app.delete "/api/v2/users/:userId/affiliations/:email", (req, res, next) => res.sendStatus 204 + app.get '/universities/list', (req, res, next) -> + res.json [] + + app.get '/universities/list/:id', (req, res, next) -> + res.json id: parseInt(req.params.id) + + app.get '/university/domains', (req, res, next) -> + res.json [] + app.listen 5000, (error) -> throw error if error? .on "error", (error) -> diff --git a/services/web/test/acceptance/config/settings.test.coffee b/services/web/test/acceptance/config/settings.test.coffee index 3d665f8a54..b64090da4b 100644 --- a/services/web/test/acceptance/config/settings.test.coffee +++ b/services/web/test/acceptance/config/settings.test.coffee @@ -1,3 +1,6 @@ +v1Api = + url: "http://#{process.env['V1_HOST'] or 'localhost'}:5000" + module.exports = enableSubscriptions: true @@ -93,3 +96,11 @@ module.exports = collaborators: -1 dropbox: true versioning: true + + proxyUrls: + '/institutions/list': { baseUrl: v1Api.url, path: '/universities/list' } + '/institutions/list/:id': + baseUrl: v1Api.url + path: (params) -> "/universities/list/#{params.id}" + '/institutions/domains': { baseUrl: v1Api.url, path: '/university/domains' } + '/proxy/missing/baseUrl': path: '/foo/bar' diff --git a/services/web/test/unit/coffee/helpers/MockRequest.coffee b/services/web/test/unit/coffee/helpers/MockRequest.coffee index d993763d27..51208dba11 100644 --- a/services/web/test/unit/coffee/helpers/MockRequest.coffee +++ b/services/web/test/unit/coffee/helpers/MockRequest.coffee @@ -9,6 +9,8 @@ class MockRequest _parsedUrl:{} i18n: translate:-> + route: + path: '' module.exports = MockRequest diff --git a/services/web/test/unit/coffee/infrastructure/ProxyManagerTests.coffee b/services/web/test/unit/coffee/infrastructure/ProxyManagerTests.coffee index 89a9c0c6f2..3f1c1e9f78 100644 --- a/services/web/test/unit/coffee/infrastructure/ProxyManagerTests.coffee +++ b/services/web/test/unit/coffee/infrastructure/ProxyManagerTests.coffee @@ -24,9 +24,21 @@ describe "ProxyManager", -> @res = new MockResponse() @next = sinon.stub() - describe 'proxyUrls', -> + describe 'apply', -> + it 'applies all paths', -> + @router = get: sinon.stub() + @settings.proxyUrls = + '/foo/bar': '' + '/foo/:id': '' + @proxyManager.apply(@router) + sinon.assert.calledTwice(@router.get) + assertCalledWith(@router.get, '/foo/bar') + assertCalledWith(@router.get, '/foo/:id') + + describe 'createProxy', -> beforeEach -> @req.url = @proxyPath + @req.route.path = @proxyPath @req.query = {} @settings.proxyUrls = {} @@ -34,33 +46,44 @@ describe "ProxyManager", -> @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) + target = '/' + @settings.proxyUrls[@proxyPath] = target + @proxyManager.createProxy(target)(@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) + @proxyManager.createProxy(targetUrl)(@req) assertCalledWith(@request, targetUrl) it 'overwrite query', -> targetUrl = 'foo.bar/baz?query' @req.query = { requestQuery: 'important' } @settings.proxyUrls[@proxyPath] = targetUrl - @proxyManager.call(@req) + @proxyManager.createProxy(targetUrl)(@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) + target = { baseUrl: 'api.v1', path: '/pa/th'} + @settings.proxyUrls[@proxyPath] = target + @proxyManager.createProxy(target)(@req, @res, @next) assertCalledWith(@request, 'api.v1/pa/th') + + it 'handles missing baseUrl', -> + target = { path: '/pa/th'} + @settings.proxyUrls[@proxyPath] = target + @proxyManager.createProxy(target)(@req, @res, @next) + assertCalledWith(@request, 'undefined/pa/th') + + it 'handles dynamic path', -> + target = baseUrl: 'api.v1', path: (params) -> "/resource/#{params.id}" + @settings.proxyUrls['/res/:id'] = target + @req.url = '/res/123' + @req.route.path = '/res/:id' + @req.params = id: 123 + @proxyManager.createProxy(target)(@req, @res, @next) + assertCalledWith(@request, 'api.v1/resource/123')