Merge pull request #12819 from overleaf/jpa-institutions-lookup

[web] migrate /institutions/ proxies to explicit V1 requests

GitOrigin-RevId: 535da280a6350dacbe2c957d2f2cedaeee02a48a

services/web/app/src/infrastructure/ProxyManager.js

@@ -1,89 +0,0 @@
-/* eslint-disable
-    max-len,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS205: Consider reworking code to avoid use of IIFEs
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-let ProxyManager
-const settings = require('@overleaf/settings')
-const logger = require('@overleaf/logger')
-const request = require('request')
-const { URL, URLSearchParams } = require('url')
-
-module.exports = ProxyManager = {
-  apply(publicApiRouter) {
-    return (() => {
-      const result = []
-      for (const proxyUrl in settings.proxyUrls) {
-        const target = settings.proxyUrls[proxyUrl]
-        result.push(
-          (function (target) {
-            const method =
-              (target.options != null ? target.options.method : undefined) ||
-              'get'
-            return publicApiRouter[method](
-              proxyUrl,
-              ProxyManager.createProxy(target)
-            )
-          })(target)
-        )
-      }
-      return result
-    })()
-  },
-
-  createProxy(target) {
-    return function (req, res, next) {
-      const targetUrl = makeTargetUrl(target, req)
-      logger.debug({ targetUrl, reqUrl: req.url }, 'proxying url')
-
-      const options = { url: targetUrl }
-      if (req.headers != null ? req.headers.cookie : undefined) {
-        options.headers = { Cookie: req.headers.cookie }
-      }
-      if ((target != null ? target.options : undefined) != null) {
-        Object.assign(options, target.options)
-      }
-      if (['post', 'put'].includes(options.method)) {
-        options.form = req.body
-      }
-      const upstream = request(options)
-      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
-      return 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
-function makeTargetUrl(target, req) {
-  const targetUrl = new URL(parseSettingUrl(target, req))
-  if (req.query != null && Object.keys(req.query).length > 0) {
-    targetUrl.search = new URLSearchParams(req.query).toString()
-  }
-  return targetUrl.href
-}
-
-function parseSettingUrl(target, { params }) {
-  let path
-  if (typeof target === 'string') {
-    return target
-  }
-  if (typeof target.path === 'function') {
-    path = target.path(params)
-  } else {
-    ;({ path } = target)
-  }
-  return `${target.baseUrl}${path || ''}`
-}

services/web/app/src/infrastructure/Server.js

@@ -28,7 +28,6 @@ const LocalStrategy = require('passport-local').Strategy
 const oneDayInMilliseconds = 86400000
 const ReferalConnect = require('../Features/Referal/ReferalConnect')
 const RedirectManager = require('./RedirectManager')
-const ProxyManager = require('./ProxyManager')
 const translations = require('./Translations')
 const Views = require('./Views')
 const Features = require('./Features')
@@ -145,7 +144,6 @@ if (Settings.blockCrossOriginRequests) {
 }
 
 RedirectManager.apply(webRouter)
-ProxyManager.apply(publicApiRouter)
 
 webRouter.use(cookieParser(Settings.security.sessionSecret))
 SessionAutostartMiddleware.applyInitialMiddleware(webRouter)

services/web/test/acceptance/src/mocks/MockV1Api.js

@@ -312,7 +312,20 @@ class MockV1Api extends AbstractMockApi {
       }
     })
 
-    this.app.get('/universities/list', (req, res) => res.json([]))
+    this.app.get('/universities/list', (req, res) => {
+      if (req.query.country_code === 'en') {
+        res.json([
+          {
+            id: 1337,
+            name: 'Institution 1337',
+            country_code: 'en',
+            departments: [],
+          },
+        ])
+      } else {
+        res.json([])
+      }
+    })
 
     this.app.get('/universities/list/:id', (req, res) =>
       res.json({
@@ -321,7 +334,27 @@ class MockV1Api extends AbstractMockApi {
       })
     )
 
-    this.app.get('/university/domains', (req, res) => res.json([]))
+    this.app.get('/university/domains', (req, res) => {
+      if (req.query.hostname === 'overleaf.com') {
+        res.json([
+          {
+            id: 42,
+            hostname: 'overleaf.com',
+            department: 'Overleaf',
+            confirmed: true,
+            university: {
+              id: 1337,
+              name: 'Institution 1337',
+              departments: [],
+              ssoBeta: false,
+              ssoEnabled: false,
+            },
+          },
+        ])
+      } else {
+        res.json([])
+      }
+    })
 
     this.app.put('/api/v1/sharelatex/users/:id/email', (req, res) => {
       const { email } = req.body && req.body.user

services/web/test/unit/src/infrastructure/ProxyManagerTests.js

@@ -1,200 +0,0 @@
-/* eslint-disable
-    max-len,
-    no-return-assign,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-const sinon = require('sinon')
-const assertCalledWith = sinon.assert.calledWith
-const { expect } = require('chai')
-const modulePath = '../../../../app/src/infrastructure/ProxyManager'
-const SandboxedModule = require('sandboxed-module')
-const MockRequest = require('../helpers/MockRequest')
-const MockResponse = require('../helpers/MockResponse')
-
-describe('ProxyManager', function () {
-  beforeEach(function () {
-    this.settings = { proxyUrls: {} }
-    this.request = sinon.stub().returns({
-      on() {},
-      pipe() {},
-    })
-    this.proxyManager = SandboxedModule.require(modulePath, {
-      requires: {
-        '@overleaf/settings': this.settings,
-        request: this.request,
-      },
-    })
-    this.proxyPath = '/foo/bar'
-    this.req = new MockRequest()
-    this.res = new MockResponse()
-    return (this.next = sinon.stub())
-  })
-
-  describe('apply', function () {
-    it('applies all paths', function () {
-      this.router = { get: sinon.stub() }
-      this.settings.proxyUrls = {
-        '/foo/bar': '',
-        '/foo/:id': '',
-      }
-      this.proxyManager.apply(this.router)
-      sinon.assert.calledTwice(this.router.get)
-      assertCalledWith(this.router.get, '/foo/bar')
-      return assertCalledWith(this.router.get, '/foo/:id')
-    })
-
-    it('applies methods other than get', function () {
-      this.router = {
-        post: sinon.stub(),
-        put: sinon.stub(),
-      }
-      this.settings.proxyUrls = {
-        '/foo/bar': { options: { method: 'post' } },
-        '/foo/:id': { options: { method: 'put' } },
-      }
-      this.proxyManager.apply(this.router)
-      sinon.assert.calledOnce(this.router.post)
-      sinon.assert.calledOnce(this.router.put)
-      assertCalledWith(this.router.post, '/foo/bar')
-      return assertCalledWith(this.router.put, '/foo/:id')
-    })
-  })
-
-  describe('createProxy', function () {
-    beforeEach(function () {
-      this.req.url = this.proxyPath
-      this.req.route.path = this.proxyPath
-      this.req.query = {}
-      this.req.params = {}
-      this.req.headers = {}
-      return (this.settings.proxyUrls = {})
-    })
-
-    afterEach(function () {
-      this.next.reset()
-      return this.request.reset()
-    })
-
-    it('proxy full URL', function () {
-      const targetUrl = 'https://user:pass@foo.bar:123/pa/th.ext?query#hash'
-      this.settings.proxyUrls[this.proxyPath] = targetUrl
-      this.proxyManager.createProxy(targetUrl)(this.req)
-      return assertCalledWith(this.request, { url: targetUrl })
-    })
-
-    it('overwrite query', function () {
-      const targetUrl = 'https://foo.bar/baz?query'
-      this.req.query = { requestQuery: 'important' }
-      this.settings.proxyUrls[this.proxyPath] = targetUrl
-      this.proxyManager.createProxy(targetUrl)(this.req)
-      const newTargetUrl = 'https://foo.bar/baz?requestQuery=important'
-      return assertCalledWith(this.request, { url: newTargetUrl })
-    })
-
-    it('handles target objects', function () {
-      const target = { baseUrl: 'https://api.v1', path: '/pa/th' }
-      this.settings.proxyUrls[this.proxyPath] = target
-      this.proxyManager.createProxy(target)(this.req, this.res, this.next)
-      return assertCalledWith(this.request, { url: 'https://api.v1/pa/th' })
-    })
-
-    it('handles missing baseUrl', function () {
-      const target = { path: '/pa/th' }
-      this.settings.proxyUrls[this.proxyPath] = target
-      expect(() =>
-        this.proxyManager.createProxy(target)(this.req, this.res, this.next)
-      ).to.throw
-    })
-
-    it('handles dynamic path', function () {
-      const target = {
-        baseUrl: 'https://api.v1',
-        path(params) {
-          return `/resource/${params.id}`
-        },
-      }
-      this.settings.proxyUrls['/res/:id'] = target
-      this.req.url = '/res/123'
-      this.req.route.path = '/res/:id'
-      this.req.params = { id: 123 }
-      this.proxyManager.createProxy(target)(this.req, this.res, this.next)
-      return assertCalledWith(this.request, {
-        url: 'https://api.v1/resource/123',
-      })
-    })
-
-    it('set arbitrary options on request', function () {
-      const target = {
-        baseUrl: 'https://api.v1',
-        path: '/foo',
-        options: { foo: 'bar' },
-      }
-      this.req.url = '/foo'
-      this.req.route.path = '/foo'
-      this.proxyManager.createProxy(target)(this.req, this.res, this.next)
-      return assertCalledWith(this.request, {
-        foo: 'bar',
-        url: 'https://api.v1/foo',
-      })
-    })
-
-    it('passes cookies', function () {
-      const target = { baseUrl: 'https://api.v1', path: '/foo' }
-      this.req.url = '/foo'
-      this.req.route.path = '/foo'
-      this.req.headers = { cookie: 'cookie' }
-      this.proxyManager.createProxy(target)(this.req, this.res, this.next)
-      return assertCalledWith(this.request, {
-        headers: {
-          Cookie: 'cookie',
-        },
-        url: 'https://api.v1/foo',
-      })
-    })
-
-    it('passes body for post', function () {
-      const target = {
-        baseUrl: 'https://api.v1',
-        path: '/foo',
-        options: { method: 'post' },
-      }
-      this.req.url = '/foo'
-      this.req.route.path = '/foo'
-      this.req.body = { foo: 'bar' }
-      this.proxyManager.createProxy(target)(this.req, this.res, this.next)
-      return assertCalledWith(this.request, {
-        form: {
-          foo: 'bar',
-        },
-        method: 'post',
-        url: 'https://api.v1/foo',
-      })
-    })
-
-    it('passes body for put', function () {
-      const target = {
-        baseUrl: 'https://api.v1',
-        path: '/foo',
-        options: { method: 'put' },
-      }
-      this.req.url = '/foo'
-      this.req.route.path = '/foo'
-      this.req.body = { foo: 'bar' }
-      this.proxyManager.createProxy(target)(this.req, this.res, this.next)
-      return assertCalledWith(this.request, {
-        form: {
-          foo: 'bar',
-        },
-        method: 'put',
-        url: 'https://api.v1/foo',
-      })
-    })
-  })
-})