From 2317fd6edbf56f84f05251c6139b08db38d40f28 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Wed, 26 Aug 2026 16:29:05 +0200 Subject: [PATCH] [web] remove generic analytics service proxy (#36934) GitOrigin-RevId: 19b3613b7cc84e73ad556067da1b3cd2f956e1c5 --- .../eslint-plugin/prefer-kebab-url-ignore.js | 1 + .../Analytics/AnalyticsController.mjs | 23 +++++ .../src/Features/Analytics/AnalyticsProxy.mjs | 28 ------- .../Features/Analytics/AnalyticsRouter.mjs | 11 ++- services/web/package.json | 1 - .../src/AnalyticsControllerTests.mjs | 84 +++++++++++++++++++ .../acceptance/src/mocks/MockAnalyticsApi.mjs | 13 +++ yarn.lock | 18 +--- 8 files changed, 129 insertions(+), 50 deletions(-) delete mode 100644 services/web/app/src/Features/Analytics/AnalyticsProxy.mjs diff --git a/libraries/eslint-plugin/prefer-kebab-url-ignore.js b/libraries/eslint-plugin/prefer-kebab-url-ignore.js index 92ee23de6b..d11ee1ac0e 100644 --- a/libraries/eslint-plugin/prefer-kebab-url-ignore.js +++ b/libraries/eslint-plugin/prefer-kebab-url-ignore.js @@ -71,6 +71,7 @@ const ignoreWords = { 'resendManagedUserInvite', 'salesContactForm', 'showSupport', + 'uniExternalCollaboration', ]), other: new Set([ 'Project', diff --git a/services/web/app/src/Features/Analytics/AnalyticsController.mjs b/services/web/app/src/Features/Analytics/AnalyticsController.mjs index 46c5b9a476..3871f274a7 100644 --- a/services/web/app/src/Features/Analytics/AnalyticsController.mjs +++ b/services/web/app/src/Features/Analytics/AnalyticsController.mjs @@ -1,10 +1,13 @@ import metrics from '@overleaf/metrics' +import Settings from '@overleaf/settings' +import { fetchJson } from '@overleaf/fetch-utils' import AnalyticsManager from './AnalyticsManager.mjs' import SessionManager from '../Authentication/SessionManager.mjs' import GeoIpLookup from '../../infrastructure/GeoIpLookup.mjs' import Features from '../../infrastructure/Features.mjs' import { expressify } from '@overleaf/promise-utils' import AccountMappingHelper from './AccountMappingHelper.mjs' +import Errors from '../Errors/Errors.js' import { z, zz, parseReq } from '../../infrastructure/Validation.mjs' // The set of segmentation/event keys is caller-supplied and dynamic (every @@ -125,8 +128,28 @@ function recordEvent(req, res, next) { res.sendStatus(202) } +const uniExternalCollaborationSchema = z.object({ + query: z.object({ + // sent by v1's Api::V2::InstitutionsController#external_collaboration_data + university_id: z.coerce.number().int().positive(), + }), +}) + +async function uniExternalCollaboration(req, res) { + if (!Settings.apis.analytics) { + throw new Errors.ServiceNotConfiguredError( + 'Analytics service not configured' + ) + } + const { query } = parseReq(req, uniExternalCollaborationSchema) + const url = new URL('/uniExternalCollaboration', Settings.apis.analytics.url) + url.searchParams.set('university_id', query.university_id) + res.json(await fetchJson(url, { signal: AbortSignal.timeout(20_000) })) +} + export default { registerSalesforceMapping: expressify(registerSalesforceMapping), + uniExternalCollaboration: expressify(uniExternalCollaboration), updateEditingSession: expressify(updateEditingSession), recordEvent, } diff --git a/services/web/app/src/Features/Analytics/AnalyticsProxy.mjs b/services/web/app/src/Features/Analytics/AnalyticsProxy.mjs deleted file mode 100644 index e7296ad740..0000000000 --- a/services/web/app/src/Features/Analytics/AnalyticsProxy.mjs +++ /dev/null @@ -1,28 +0,0 @@ -import settings from '@overleaf/settings' -import Errors from '../Errors/Errors.js' -import httpProxy from 'express-http-proxy' - -export default { - call(basePath) { - if (!settings.apis.analytics) { - return (req, res, next) => - next( - new Errors.ServiceNotConfiguredError( - 'Analytics service not configured' - ) - ) - } - - return httpProxy(settings.apis.analytics.url, { - proxyReqPathResolver(req) { - // req.url is the part of the path that comes after the mount point in - // app.use() - return `${basePath}${req.url}` - }, - proxyReqOptDecorator(proxyReqOpts, srcReq) { - proxyReqOpts.headers = {} // unset all headers - return proxyReqOpts - }, - }) - }, -} diff --git a/services/web/app/src/Features/Analytics/AnalyticsRouter.mjs b/services/web/app/src/Features/Analytics/AnalyticsRouter.mjs index f19df459cb..69a505037e 100644 --- a/services/web/app/src/Features/Analytics/AnalyticsRouter.mjs +++ b/services/web/app/src/Features/Analytics/AnalyticsRouter.mjs @@ -1,6 +1,5 @@ import AuthenticationController from './../Authentication/AuthenticationController.mjs' import AnalyticsController from './AnalyticsController.mjs' -import AnalyticsProxy from './AnalyticsProxy.mjs' import { RateLimiter } from '../../infrastructure/RateLimiter.mjs' import RateLimiterMiddleware from '../Security/RateLimiterMiddleware.mjs' @@ -13,8 +12,8 @@ const rateLimiters = { points: 20, duration: 60, }), - uniExternalCollabProxy: new RateLimiter( - 'analytics-uni-external-collab-proxy', + uniExternalCollaboration: new RateLimiter( + 'analytics-uni-external-collaboration', { points: 20, duration: 60 } ), } @@ -35,11 +34,11 @@ export default { AnalyticsController.updateEditingSession ) - publicApiRouter.use( + publicApiRouter.get( '/analytics/uniExternalCollaboration', AuthenticationController.requirePrivateApiAuth(), - RateLimiterMiddleware.rateLimit(rateLimiters.uniExternalCollabProxy), - AnalyticsProxy.call('/uniExternalCollaboration') + RateLimiterMiddleware.rateLimit(rateLimiters.uniExternalCollaboration), + AnalyticsController.uniExternalCollaboration ) publicApiRouter.post( diff --git a/services/web/package.json b/services/web/package.json index 4be06e333d..b0cc24b8f8 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -136,7 +136,6 @@ "eventsource-parser": "^1.1.2", "express": "4.22.1", "express-bearer-token": "patch:express-bearer-token@npm%3A2.4.0#~/.yarn/patches/express-bearer-token-npm-2.4.0-731c8c5c99.patch", - "express-http-proxy": "^1.6.0", "express-session": "^1.17.1", "file-type": "^21.3.4", "focus-trap-react": "^11.0.4", diff --git a/services/web/test/acceptance/src/AnalyticsControllerTests.mjs b/services/web/test/acceptance/src/AnalyticsControllerTests.mjs index 01a014e133..8ee753b855 100644 --- a/services/web/test/acceptance/src/AnalyticsControllerTests.mjs +++ b/services/web/test/acceptance/src/AnalyticsControllerTests.mjs @@ -2,6 +2,7 @@ import { expect } from 'chai' import UserHelper from './helpers/User.mjs' import request from './helpers/request.js' import Features from '../../../app/src/infrastructure/Features.mjs' +import MockAnalyticsApiClass from './mocks/MockAnalyticsApi.mjs' import { expectValidationErrorRaw } from '@overleaf/validation-tools/testUtils.js' const User = UserHelper.promises @@ -19,6 +20,14 @@ function privateApiRequest(options) { }) } +let MockAnalyticsApi + +before(function () { + if (Features.hasFeature('saas')) { + MockAnalyticsApi = MockAnalyticsApiClass.instance() + } +}) + describe('AnalyticsController', function () { let owner @@ -88,6 +97,81 @@ describe('AnalyticsController', function () { }) }) + describe('GET /analytics/uniExternalCollaboration', function () { + beforeEach(function () { + // both the analytics API URL and its mock are saas-only, see + // test/acceptance/config/settings.test.saas.js and Init.mjs + if (!Features.hasFeature('saas')) { + this.skip() + } + MockAnalyticsApi.reset() + }) + + it('should return the data from the analytics service', async function () { + const response = await privateApiRequest({ + method: 'get', + url: '/analytics/uniExternalCollaboration?university_id=42', + }) + expect(response.statusCode).to.equal(200) + expect(response.body).to.deep.equal([ + { university_id: 123, external_collaborations: 321 }, + ]) + expect( + MockAnalyticsApi.getLastUniExternalCollaborationRequest().query + ).to.deep.equal({ university_id: '42' }) + }) + + it('should reject a non-numeric university id', async function () { + const response = await privateApiRequest({ + method: 'get', + url: '/analytics/uniExternalCollaboration?university_id=not-a-number', + }) + expectValidationErrorRaw(response, 400, 'university_id') + }) + + it('should reject a non-positive university id', async function () { + const response = await privateApiRequest({ + method: 'get', + url: '/analytics/uniExternalCollaboration?university_id=0', + }) + expectValidationErrorRaw(response, 400, 'university_id') + }) + + it('should reject a missing university id', async function () { + const response = await privateApiRequest({ + method: 'get', + url: '/analytics/uniExternalCollaboration', + }) + expectValidationErrorRaw(response, 400, 'university_id') + }) + + it('should require private API auth', async function () { + const response = await authedRequest.request({ + method: 'get', + url: '/analytics/uniExternalCollaboration?university_id=42', + json: true, + }) + expect(response.statusCode).to.equal(401) + }) + + it('should not route a sub-path to the analytics service', async function () { + const response = await privateApiRequest({ + method: 'get', + url: '/analytics/uniExternalCollaboration/extra?university_id=42', + }) + expect(response.statusCode).to.equal(404) + }) + + it('should not route other methods to the analytics service', async function () { + await privateApiRequest({ + method: 'post', + url: '/analytics/uniExternalCollaboration?university_id=42', + }) + expect(MockAnalyticsApi.getLastUniExternalCollaborationRequest()).to.be + .null + }) + }) + describe('POST /analytics/register-v-1-salesforce-mapping', function () { it('should accept a well-formed mapping', async function () { if (!Features.hasFeature('analytics')) { diff --git a/services/web/test/acceptance/src/mocks/MockAnalyticsApi.mjs b/services/web/test/acceptance/src/mocks/MockAnalyticsApi.mjs index 4d8276b2b7..0d43d4dc7d 100644 --- a/services/web/test/acceptance/src/mocks/MockAnalyticsApi.mjs +++ b/services/web/test/acceptance/src/mocks/MockAnalyticsApi.mjs @@ -8,6 +8,7 @@ class MockAnalyticsApi extends AbstractMockApi { this.lastSplitTestAssignmentsRequest = null this.lastSplitTestCalcRequest = null this.lastSplitTestEventsRequest = null + this.lastUniExternalCollaborationRequest = null } getLastGraphRequest() { @@ -26,6 +27,10 @@ class MockAnalyticsApi extends AbstractMockApi { return this.lastSplitTestEventsRequest } + getLastUniExternalCollaborationRequest() { + return this.lastUniExternalCollaborationRequest + } + applyRoutes() { this.app.get('/graphs/:graph', (req, res) => { // case 3: recorded verbatim for later assertion by acceptance tests @@ -80,6 +85,14 @@ class MockAnalyticsApi extends AbstractMockApi { this.lastSplitTestEventsRequest = { query: getRawReqInput(req).query } res.json({ 'editor-open': { source: ['ide-open'] } }) }) + + this.app.get('/uniExternalCollaboration', (req, res) => { + // case 3: recorded verbatim for later assertion by acceptance tests + this.lastUniExternalCollaborationRequest = { + query: getRawReqInput(req).query, + } + res.json([{ university_id: 123, external_collaborations: 321 }]) + }) } } diff --git a/yarn.lock b/yarn.lock index 45c540dc46..12c2b41c92 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8128,7 +8128,6 @@ __metadata: eventsource-parser: "npm:^1.1.2" express: "npm:4.22.1" express-bearer-token: "patch:express-bearer-token@npm%3A2.4.0#~/.yarn/patches/express-bearer-token-npm-2.4.0-731c8c5c99.patch" - express-http-proxy: "npm:^1.6.0" express-session: "npm:^1.17.1" fake-indexeddb: "npm:^6.0.0" fast-fuzzy: "npm:^1.12.0" @@ -16502,7 +16501,7 @@ __metadata: languageName: node linkType: hard -"debug@npm:^3.0.1, debug@npm:^3.1.0, debug@npm:^3.2.7": +"debug@npm:^3.1.0, debug@npm:^3.2.7": version: 3.2.7 resolution: "debug@npm:3.2.7" dependencies: @@ -17660,7 +17659,7 @@ __metadata: languageName: node linkType: hard -"es6-promise@npm:^4.1.0, es6-promise@npm:^4.1.1, es6-promise@npm:^4.2.8": +"es6-promise@npm:^4.1.0, es6-promise@npm:^4.2.8": version: 4.2.8 resolution: "es6-promise@npm:4.2.8" checksum: 10c0/2373d9c5e9a93bdd9f9ed32ff5cb6dd3dd785368d1c21e9bbbfd07d16345b3774ae260f2bd24c8f836a6903f432b4151e7816a7fa8891ccb4e1a55a028ec42c3 @@ -18499,17 +18498,6 @@ __metadata: languageName: node linkType: hard -"express-http-proxy@npm:^1.6.0": - version: 1.6.3 - resolution: "express-http-proxy@npm:1.6.3" - dependencies: - debug: "npm:^3.0.1" - es6-promise: "npm:^4.1.1" - raw-body: "npm:^2.3.0" - checksum: 10c0/e27d85b923a812aa8d2ce90bc15cc8254bcd1691d43ea1c070760d4e3fe985d913e4e9f4523be649aff36a0c827b0d35222f5f5e62a5b03c8ffe58284aa91ef6 - languageName: node - linkType: hard - "express-rate-limit@npm:^2.11.0": version: 2.14.2 resolution: "express-rate-limit@npm:2.14.2" @@ -28706,7 +28694,7 @@ __metadata: languageName: node linkType: hard -"raw-body@npm:^2.3.0, raw-body@npm:~2.5.3": +"raw-body@npm:~2.5.3": version: 2.5.3 resolution: "raw-body@npm:2.5.3" dependencies: