diff --git a/services/web/app/src/Features/Exports/ExportsController.mjs b/services/web/app/src/Features/Exports/ExportsController.mjs index f38b161da6..d5ba10dbbe 100644 --- a/services/web/app/src/Features/Exports/ExportsController.mjs +++ b/services/web/app/src/Features/Exports/ExportsController.mjs @@ -3,6 +3,7 @@ import { expressify } from '@overleaf/promise-utils' import SessionManager from '../Authentication/SessionManager.mjs' import logger from '@overleaf/logger' import OError from '@overleaf/o-error' +import settings from '@overleaf/settings' async function exportProject(req, res, next) { const { project_id: projectId, brand_variation_id: brandVariationId } = @@ -52,6 +53,7 @@ async function exportProject(req, res, next) { ) return res.json({ export_v1_id: exportData.v1_id, + token: exportData.token, message: exportData.message, }) } catch (err) { @@ -70,9 +72,18 @@ async function exportProject(req, res, next) { async function exportStatus(req, res) { const { export_id: exportId } = req.params + const { token } = req.query + if (!token && settings.exports?.requireToken) { + return res.status(403).json({ + export_json: { + status_summary: 'failed', + status_detail: 'token is required', + }, + }) + } let exportJson try { - exportJson = await ExportsHandler.fetchExport(exportId) + exportJson = await ExportsHandler.fetchExport(exportId, token) } catch (err) { const json = { status_summary: 'failed', @@ -96,10 +107,27 @@ async function exportStatus(req, res) { async function exportDownload(req, res, next) { const { type, export_id: exportId } = req.params + const { token } = req.query + if (!token && settings.exports?.requireToken) { + return res.sendStatus(403) + } - SessionManager.getLoggedInUserId(req.session) - const exportFileUrl = await ExportsHandler.fetchDownload(exportId, type) - return res.redirect(exportFileUrl) + try { + SessionManager.getLoggedInUserId(req.session) + const exportFileUrl = await ExportsHandler.fetchDownload( + exportId, + type, + token + ) + return res.redirect(exportFileUrl) + } catch (err) { + const info = OError.getFullInfo(err) + // A bad/spoofed token is rejected by v1 as 404; expose as 400 to clients. + if (info?.statusCode === 404) { + return res.sendStatus(400) + } + throw err + } } export default { diff --git a/services/web/app/src/Features/Exports/ExportsHandler.mjs b/services/web/app/src/Features/Exports/ExportsHandler.mjs index b398701429..80b3875df6 100644 --- a/services/web/app/src/Features/Exports/ExportsHandler.mjs +++ b/services/web/app/src/Features/Exports/ExportsHandler.mjs @@ -19,6 +19,7 @@ export default ExportsHandler = { const body = await ExportsHandler._requestExport(exportData) exportData.v1_id = body.exportId + exportData.token = body.token exportData.message = body.message // TODO: possibly store the export data in Mongo return exportData @@ -161,9 +162,12 @@ export default ExportsHandler = { } }, - async fetchExport(exportId) { + async fetchExport(exportId, token) { const url = new URL(settings.apis.v1.url) url.pathname = `/api/v1/overleaf/exports/${exportId}` + if (token) { + url.searchParams.append('token', token) + } try { return await fetchString(url, { @@ -186,9 +190,12 @@ export default ExportsHandler = { } }, - async fetchDownload(exportId, type) { + async fetchDownload(exportId, type, token) { const url = new URL(settings.apis.v1.url) url.pathname = `/api/v1/overleaf/exports/${exportId}/${type}_url` + if (token) { + url.searchParams.append('token', token) + } try { return await fetchString(url, { diff --git a/services/web/test/acceptance/src/mocks/MockV1Api.mjs b/services/web/test/acceptance/src/mocks/MockV1Api.mjs index 0e803151f4..a0c7e35e9e 100644 --- a/services/web/test/acceptance/src/mocks/MockV1Api.mjs +++ b/services/web/test/acceptance/src/mocks/MockV1Api.mjs @@ -14,6 +14,7 @@ class MockV1Api extends AbstractMockApi { this.existingEmails = [] this.exportId = null this.exportParams = null + this.exportToken = null this.institutionDomains = {} this.institutionId = 1000 this.institutions = {} @@ -49,6 +50,10 @@ class MockV1Api extends AbstractMockApi { this.exportId = id } + setExportToken(token) { + this.exportToken = token + } + getLastExportParams() { return this.exportParams } @@ -209,7 +214,28 @@ class MockV1Api extends AbstractMockApi { this.app.post('/api/v1/overleaf/exports', (req, res) => { this.exportParams = Object.assign({}, req.body) - res.json({ exportId: this.exportId }) + res.json({ exportId: this.exportId, token: this.exportToken }) + }) + + this.app.get('/api/v1/overleaf/exports/:id', (req, res) => { + const { token } = req.query + if (token && token !== this.exportToken) { + return res.sendStatus(404) + } + res.json({ + id: parseInt(req.params.id, 10), + status_summary: 'succeeded', + token: this.exportToken, + }) + }) + + this.app.get('/api/v1/overleaf/exports/:id/zip_url', (req, res) => { + const { token } = req.query + if (token && token !== this.exportToken) { + return res.sendStatus(404) + } + res.set('Content-Type', 'text/plain') + return res.end('https://example.com/export.zip') }) this.app.get('/api/v2/users/:userId/affiliations', (req, res) => { diff --git a/services/web/test/unit/src/Exports/ExportsController.test.mjs b/services/web/test/unit/src/Exports/ExportsController.test.mjs index 99e744d0f4..77c25df599 100644 --- a/services/web/test/unit/src/Exports/ExportsController.test.mjs +++ b/services/web/test/unit/src/Exports/ExportsController.test.mjs @@ -20,11 +20,13 @@ describe('ExportsController', function () { beforeEach(async function (ctx) { ctx.handler = { getUserNotifications: sinon.stub().callsArgWith(1) } + ctx.settings = {} ctx.req = { params: { project_id: projectId, brand_variation_id: brandVariationId, }, + query: {}, body: { firstName, lastName, @@ -41,6 +43,8 @@ describe('ExportsController', function () { ctx.res = { json: sinon.stub(), status: sinon.stub(), + sendStatus: sinon.stub(), + redirect: sinon.stub(), } ctx.res.status.returns(ctx.res) ctx.next = sinon.stub() @@ -62,6 +66,10 @@ describe('ExportsController', function () { }) ) + vi.doMock('@overleaf/settings', () => ({ + default: ctx.settings, + })) + ctx.controller = (await import(modulePath)).default }) @@ -69,7 +77,7 @@ describe('ExportsController', function () { it('should ask the handler to perform the export', async function (ctx) { ctx.handler.exportProject = sinon .stub() - .resolves({ iAmAnExport: true, v1_id: 897 }) + .resolves({ iAmAnExport: true, v1_id: 897, token: 'mock-token' }) const expected = { project_id: projectId, user_id: userId, @@ -85,6 +93,7 @@ describe('ExportsController', function () { expect(ctx.handler.exportProject.args[0][0]).to.deep.equal(expected) expect(res.json.args[0][0]).to.deep.equal({ export_v1_id: 897, + token: 'mock-token', message: undefined, }) }) @@ -95,6 +104,7 @@ describe('ExportsController', function () { ctx.handler.exportProject = sinon.stub().resolves({ iAmAnExport: true, v1_id: 897, + token: 'mock-token', message: 'RESUBMISSION', }) const expected = { @@ -114,6 +124,7 @@ describe('ExportsController', function () { expect(ctx.handler.exportProject.args[0][0]).to.deep.equal(expected) expect(res.json.args[0][0]).to.deep.equal({ export_v1_id: 897, + token: 'mock-token', message: 'RESUBMISSION', }) }) @@ -131,7 +142,7 @@ describe('ExportsController', function () { it('should ask the handler to perform the export', async function (ctx) { ctx.handler.exportProject = sinon .stub() - .resolves({ iAmAnExport: true, v1_id: 897 }) + .resolves({ iAmAnExport: true, v1_id: 897, token: 'mock-token' }) const expected = { project_id: projectId, user_id: userId, @@ -153,6 +164,7 @@ describe('ExportsController', function () { expect(ctx.handler.exportProject.args[0][0]).to.deep.equal(expected) expect(res.json.args[0][0]).to.deep.equal({ export_v1_id: 897, + token: 'mock-token', message: undefined, }) }) @@ -192,7 +204,9 @@ describe('ExportsController', function () { } ctx.req.params = { project_id: projectId, export_id: 897 } + ctx.req.query = { token: 'mock-token' } await ctx.controller.exportStatus(ctx.req, res) + expect(ctx.handler.fetchExport).to.have.been.calledWith(897, 'mock-token') expect(res.json.args[0][0]).to.deep.equal({ export_json: { status_summary: 'completed', @@ -206,4 +220,152 @@ describe('ExportsController', function () { }, }) }) + + describe('exportStatus token validation', function () { + beforeEach(function (ctx) { + ctx.req.params = { project_id: projectId, export_id: 897 } + ctx.handler.fetchExport = sinon.stub().resolves( + `{ + "id":897, + "status_summary":"completed", + "status_detail":"all done", + "partner_submission_id":"abc123", + "v2_user_email":"la@tex.com", + "v2_user_first_name":"Arthur", + "v2_user_last_name":"Author", + "title":"my project", + "token":"token" + }` + ) + }) + + describe('when requireToken is enabled', function () { + beforeEach(function (ctx) { + ctx.settings.exports = { requireToken: true } + }) + + it('should return 403 when no token is provided', async function (ctx) { + ctx.req.query = {} + await ctx.controller.exportStatus(ctx.req, ctx.res) + expect(ctx.res.status.args[0][0]).to.equal(403) + expect(ctx.res.json.args[0][0]).to.deep.equal({ + export_json: { + status_summary: 'failed', + status_detail: 'token is required', + }, + }) + expect(ctx.handler.fetchExport).not.to.have.been.called + }) + + it('should proceed when a token is provided', async function (ctx) { + ctx.req.query = { token: 'mock-token' } + const res = { + json: sinon.stub(), + } + await ctx.controller.exportStatus(ctx.req, res) + expect(ctx.handler.fetchExport).to.have.been.calledWith( + 897, + 'mock-token' + ) + }) + }) + + describe('when requireToken is not enabled', function () { + it('should proceed without token', async function (ctx) { + ctx.req.query = {} + const res = { + json: sinon.stub(), + } + await ctx.controller.exportStatus(ctx.req, res) + expect(ctx.handler.fetchExport).to.have.been.calledWith(897, undefined) + }) + }) + + describe('when a spoofed token is provided', function () { + it('should return a failed status when v1 rejects the token', async function (ctx) { + ctx.req.query = { token: 'wrong-token' } + ctx.handler.fetchExport = sinon + .stub() + .rejects(new Error('Request failed: 404')) + await ctx.controller.exportStatus(ctx.req, ctx.res) + expect(ctx.handler.fetchExport).to.have.been.calledWith( + 897, + 'wrong-token' + ) + expect(ctx.res.json.args[0][0]).to.deep.equal({ + export_json: { + status_summary: 'failed', + status_detail: 'Error: Request failed: 404', + }, + }) + }) + }) + }) + + describe('exportDownload token validation', function () { + beforeEach(function (ctx) { + ctx.req.params = { + project_id: projectId, + export_id: 897, + type: 'zip', + } + ctx.handler.fetchDownload = sinon.stub().resolves('https://example.com') + }) + + describe('when requireToken is enabled', function () { + beforeEach(function (ctx) { + ctx.settings.exports = { requireToken: true } + }) + + it('should return 403 when no token is provided', async function (ctx) { + ctx.req.query = {} + await ctx.controller.exportDownload(ctx.req, ctx.res) + expect(ctx.res.sendStatus.args[0][0]).to.equal(403) + expect(ctx.handler.fetchDownload).not.to.have.been.called + }) + + it('should proceed when a token is provided', async function (ctx) { + ctx.req.query = { token: 'mock-token' } + await ctx.controller.exportDownload(ctx.req, ctx.res) + expect(ctx.handler.fetchDownload).to.have.been.calledWith( + 897, + 'zip', + 'mock-token' + ) + expect(ctx.res.redirect).to.have.been.calledWith('https://example.com') + }) + }) + + describe('when requireToken is not enabled', function () { + it('should proceed without token', async function (ctx) { + ctx.req.query = {} + await ctx.controller.exportDownload(ctx.req, ctx.res) + expect(ctx.handler.fetchDownload).to.have.been.calledWith( + 897, + 'zip', + undefined + ) + expect(ctx.res.redirect).to.have.been.calledWith('https://example.com') + }) + }) + + describe('when a spoofed token is provided', function () { + it('should return 400 when v1 rejects the token', async function (ctx) { + ctx.req.query = { token: 'wrong-token' } + ctx.handler.fetchDownload = sinon.stub().rejects( + OError.tag(new Error('Request failed: 404'), 'v1 error', { + statusCode: 404, + }) + ) + await ctx.controller.exportDownload(ctx.req, ctx.res, ctx.next) + expect(ctx.handler.fetchDownload).to.have.been.calledWith( + 897, + 'zip', + 'wrong-token' + ) + expect(ctx.res.sendStatus).to.have.been.calledWith(400) + expect(ctx.next).not.to.have.been.called + }) + }) + }) }) diff --git a/services/web/test/unit/src/Exports/ExportsHandler.test.mjs b/services/web/test/unit/src/Exports/ExportsHandler.test.mjs index a059b9718d..255d3805a4 100644 --- a/services/web/test/unit/src/Exports/ExportsHandler.test.mjs +++ b/services/web/test/unit/src/Exports/ExportsHandler.test.mjs @@ -466,7 +466,11 @@ describe('ExportsHandler', function () { } ctx.export_data = { iAmAnExport: true } ctx.export_id = 4096 - ctx.fetchUtils.fetchJson.resolves({ exportId: ctx.export_id }) + ctx.export_token = 'mock-export-token' + ctx.fetchUtils.fetchJson.resolves({ + exportId: ctx.export_id, + token: ctx.export_token, + }) }) describe('when all goes well', function () { @@ -490,8 +494,11 @@ describe('ExportsHandler', function () { ) }) - it('should return the body with v1 export id', function (ctx) { - expect(response).to.eql({ exportId: ctx.export_id }) + it('should return the body with v1 export id and token', function (ctx) { + expect(response).to.eql({ + exportId: ctx.export_id, + token: ctx.export_token, + }) }) }) @@ -554,19 +561,50 @@ describe('ExportsHandler', function () { }, } ctx.export_id = 897 + ctx.export_token = 'mock-export-token' ctx.body = '{"id":897, "status_summary":"completed"}' - ctx.fetchUtils.fetchString = sinon - .stub() - .resolves(JSON.stringify({ body: ctx.body })) + ctx.fetchUtils.fetchString = sinon.stub().resolves(ctx.body) }) - describe('when all goes well', function () { + describe('when all goes well with token', function () { + let exportResponse + beforeEach(async function (ctx) { + exportResponse = await ctx.ExportsHandler.fetchExport( + ctx.export_id, + ctx.export_token + ) + }) + + it('should issue the request with token', function (ctx) { + const expectedUrl = new URL( + '/api/v1/overleaf/exports/' + ctx.export_id, + ctx.settings.apis.v1.url + ) + expectedUrl.searchParams.append('token', ctx.export_token) + expect(ctx.fetchUtils.fetchString).to.have.been.calledWith( + expectedUrl, + { + basicAuth: { + user: ctx.settings.apis.v1.user, + password: ctx.settings.apis.v1.pass, + }, + signal: sinon.match.instanceOf(AbortSignal), + } + ) + }) + + it('should return the v1 export id', function (ctx) { + expect(exportResponse).to.eql(ctx.body) + }) + }) + + describe('when called without token', function () { let exportResponse beforeEach(async function (ctx) { exportResponse = await ctx.ExportsHandler.fetchExport(ctx.export_id) }) - it('should issue the request', function (ctx) { + it('should issue the request without token', function (ctx) { expect(ctx.fetchUtils.fetchString).to.have.been.calledWith( new URL( '/api/v1/overleaf/exports/' + ctx.export_id, @@ -583,7 +621,7 @@ describe('ExportsHandler', function () { }) it('should return the v1 export id', function (ctx) { - expect(exportResponse).to.eql(JSON.stringify({ body: ctx.body })) + expect(exportResponse).to.eql(ctx.body) }) }) }) @@ -599,12 +637,48 @@ describe('ExportsHandler', function () { }, } ctx.export_id = 897 + ctx.export_token = 'mock-export-token' ctx.body = 'https://writelatex-conversions-dev.s3.amazonaws.com/exports/ieee_latexqc/tnb/2912/xggmprcrpfwbsnqzqqmvktddnrbqkqkr.zip?X-Amz-Expires=14400&X-Amz-Date=20180730T181003Z&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAJDGDIJFGLNVGZH6A/20180730/us-east-1/s3/aws4_request&X-Amz-SignedHeaders=host&X-Amz-Signature=dec990336913cef9933f0e269afe99722d7ab2830ebf2c618a75673ee7159fee' ctx.fetchUtils.fetchString = sinon.stub().resolves(ctx.body) }) - describe('when all goes well', function () { + describe('when all goes well with token', function () { + let downloadResponse + beforeEach(async function (ctx) { + downloadResponse = await ctx.ExportsHandler.fetchDownload( + ctx.export_id, + 'zip', + ctx.export_token + ) + }) + + it('should issue the request with token', function (ctx) { + const expectedUrl = new URL( + ctx.settings.apis.v1.url + + '/api/v1/overleaf/exports/' + + ctx.export_id + + '/zip_url' + ) + expectedUrl.searchParams.append('token', ctx.export_token) + expect(ctx.fetchUtils.fetchString).to.have.been.calledWith( + expectedUrl, + { + basicAuth: { + user: ctx.settings.apis.v1.user, + password: ctx.settings.apis.v1.pass, + }, + signal: sinon.match.instanceOf(AbortSignal), + } + ) + }) + + it('should return the download URL', function (ctx) { + expect(downloadResponse).to.eql(ctx.body) + }) + }) + + describe('when called without token', function () { let downloadResponse beforeEach(async function (ctx) { downloadResponse = await ctx.ExportsHandler.fetchDownload( @@ -613,7 +687,7 @@ describe('ExportsHandler', function () { ) }) - it('should issue the request', function (ctx) { + it('should issue the request without token', function (ctx) { expect(ctx.fetchUtils.fetchString).to.have.been.calledWith( new URL( ctx.settings.apis.v1.url + @@ -631,7 +705,7 @@ describe('ExportsHandler', function () { ) }) - it('should return the v1 export id', function (ctx) { + it('should return the download URL', function (ctx) { expect(downloadResponse).to.eql(ctx.body) }) })