diff --git a/libraries/validation-tools/test/unit/src/zodHelpers.test.ts b/libraries/validation-tools/test/unit/src/zodHelpers.test.ts index 0a5befff0c..b57ebfd310 100644 --- a/libraries/validation-tools/test/unit/src/zodHelpers.test.ts +++ b/libraries/validation-tools/test/unit/src/zodHelpers.test.ts @@ -842,40 +842,6 @@ describe('zodHelpers', () => { }) }) - describe('streamedBody', () => { - it('parses undefined (no body parser mounted) to undefined', () => { - const parsed = zz.streamedBody().safeParse(undefined) - expect(parsed.success).toBe(true) - expect(parsed.data).toBe(undefined) - }) - - it('parses the empty object left by a skipping body parser to undefined', () => { - const parsed = zz.streamedBody().safeParse({}) - expect(parsed.success).toBe(true) - expect(parsed.data).toBe(undefined) - }) - - it('fails to parse a parsed JSON body', () => { - const parsed = zz.streamedBody().safeParse({ some: 'payload' }) - expect(parsed.success).toBe(false) - expect(parsed.error?.issues).toMatchObject([ - expect.objectContaining({ - message: 'unexpected parsed request body on streamed endpoint', - }), - ]) - }) - - it('fails to parse a raw buffer body', () => { - const parsed = zz.streamedBody().safeParse(Buffer.alloc(0)) - expect(parsed.success).toBe(false) - }) - - it('fails to parse a string body', () => { - const parsed = zz.streamedBody().safeParse('text body') - expect(parsed.success).toBe(false) - }) - }) - describe('uploadedFile', () => { const validFile = { fieldname: 'qqfile', diff --git a/libraries/validation-tools/zodHelpers.js b/libraries/validation-tools/zodHelpers.js index 5e14cc2376..f03b1aacee 100644 --- a/libraries/validation-tools/zodHelpers.js +++ b/libraries/validation-tools/zodHelpers.js @@ -206,29 +206,6 @@ const zz = { .min(1, { message: 'invalid event name' }) .regex(/^[a-zA-Z0-9-_]+$/, { message: 'invalid event name' }) .max(240, { message: 'event name is too long' }), - /** - * For endpoints that consume the request as a raw stream (e.g. blob - * uploads): asserts that no body parser consumed the stream before the - * handler. Accepts only undefined or the empty object that a skipping - * body parser leaves behind, and parses to undefined. If a body parser is - * later mounted on such a route, validation fails loudly instead of the - * handler silently reading an already-consumed stream. - */ - streamedBody: () => - z - .custom( - val => { - if (val === undefined) return true - return ( - typeof val === 'object' && - val !== null && - !Buffer.isBuffer(val) && - Object.keys(val).length === 0 - ) - }, - { message: 'unexpected parsed request body on streamed endpoint' } - ) - .transform(() => undefined), /** * A multer file object (disk storage — the only storage backend we use), * for validating req.file / req.files on multipart routes. The diff --git a/package.json b/package.json index 8c7c18a63b..34c07d3fd1 100644 --- a/package.json +++ b/package.json @@ -75,6 +75,7 @@ "forwarded@npm:0.2.0": "patch:forwarded@npm%3A0.2.0#~/.yarn/patches/forwarded-npm-0.2.0.patch", "multer@npm:2.2.0": "patch:multer@npm%3A2.2.0#~/.yarn/patches/multer-npm-2.2.0-4ed181c78f.patch", "node-fetch": "patch:node-fetch@npm%3A2.7.0#~/.yarn/patches/node-fetch-npm-2.7.0.patch", + "passport-local": "patch:passport-local@npm%3A1.0.0#~/.yarn/patches/passport-local-npm-1.0.0-ed89961a0c.patch", "passport-oauth2": "patch:passport-oauth2@npm%3A1.6.1#~/.yarn/patches/passport-oauth2-npm-1.6.1.patch", "send": "patch:send@npm%3A0.19.0#~/.yarn/patches/send-npm-0.19.0.patch", "serve-static": "1.16.2", diff --git a/services/clsi/test/acceptance/js/UrlCachingTests.js b/services/clsi/test/acceptance/js/UrlCachingTests.js index 91cab81453..12658f95a6 100644 --- a/services/clsi/test/acceptance/js/UrlCachingTests.js +++ b/services/clsi/test/acceptance/js/UrlCachingTests.js @@ -32,14 +32,14 @@ const projectFileParamsSchema = z.object({ const bucketKeyParamsSchema = z.object({ params: z.strictObject({ bucket: z.string(), - 0: zz.filepath(), + key: zz.filepath(), }), }) const staticFileParamsSchema = z.object({ params: z.strictObject({ random_id: z.string(), - 0: zz.filepath(), + path: zz.filepath(), }), }) @@ -81,18 +81,18 @@ const Server = { res.send(`${projectId}:${fileId}`) }) - app.get('/bucket/:bucket/key/*', (req, res, next) => { + app.get('/bucket/:bucket/key/:key(.+)', (req, res, next) => { this.getFile(req.url) const { - params: { bucket, 0: key }, + params: { bucket, key }, } = parseReq(req, bucketKeyParamsSchema) res.send(`${bucket}:${key}`) }) - app.get('/:random_id/*', (req, res, next) => { + app.get('/:random_id/:path(.+)', (req, res, next) => { this.getFile(req.url) const { - params: { 0: path }, + params: { path }, } = parseReq(req, staticFileParamsSchema) req.url = `/${path}` staticServer(req, res, next) diff --git a/services/filestore/app.js b/services/filestore/app.js index db5b24a53f..ec55b246fc 100644 --- a/services/filestore/app.js +++ b/services/filestore/app.js @@ -69,13 +69,8 @@ if (settings.filestore.stores.template_files) { ) } -// Not a named `:key(.*)` param: Express 4.22.1's bundled path-to-regexp has a -// bug where combining a named param with a custom (.*) regex both duplicates -// the match under a numeric key AND corrupts the captured value (off-by-one -// substring). The anonymous `*` wildcard doesn't have this bug, so keep it -// and validate the numeric params[0] key directly (see schemas.js). app.get( - '/bucket/:bucket/key/*', + '/bucket/:bucket/key/:key(.+)', keyBuilder.bucketFileKeyMiddleware, fileController.getFile ) diff --git a/services/filestore/app/js/FileController.js b/services/filestore/app/js/FileController.js index 5e00ca93dc..19336a83a3 100644 --- a/services/filestore/app/js/FileController.js +++ b/services/filestore/app/js/FileController.js @@ -4,11 +4,7 @@ import parseRange from 'range-parser' import Errors from './Errors.js' import { pipeline } from 'node:stream' import { parseReq } from '@overleaf/validation-tools' -import { - getFileQuerySchema, - getFileQueryFallbackSchema, - insertFileSchema, -} from './schemas.js' +import { getFileQuerySchema, getFileQueryFallbackSchema } from './schemas.js' const maxSizeInBytes = 1024 * 1024 * 1024 // 1GB @@ -125,7 +121,6 @@ function getFileHead(req, res, next) { } function insertFile(req, res, next) { - parseReq(req, insertFileSchema, { logOnly: true }) metrics.inc('insertFile') const { key, bucket } = req diff --git a/services/filestore/app/js/KeyBuilder.js b/services/filestore/app/js/KeyBuilder.js index bdc23ae3e6..cd884d6958 100644 --- a/services/filestore/app/js/KeyBuilder.js +++ b/services/filestore/app/js/KeyBuilder.js @@ -44,7 +44,7 @@ function bucketFileKeyMiddleware(req, res, next) { logOnly: true, }) req.bucket = params.bucket - req.key = params[0] + req.key = params.key next() } diff --git a/services/filestore/app/js/schemas.js b/services/filestore/app/js/schemas.js index 2a20978f96..1cefc19c3b 100644 --- a/services/filestore/app/js/schemas.js +++ b/services/filestore/app/js/schemas.js @@ -46,10 +46,7 @@ export const templateFileParamsFallbackSchema = z.object({ export const bucketFileParamsSchema = z.object({ params: z.strictObject({ bucket: z.string().regex(/^[a-z0-9-]+$/), - // the anonymous `*` wildcard in app.js (kept anonymous rather than a - // named `:key(.*)` -- see the comment there for why); Express exposes - // its capture under the numeric key '0'. May contain nested segments. - 0: zz.filepath(), + key: zz.filepath(), }), }) @@ -111,13 +108,3 @@ export const getFileQueryFallbackSchema = z.object({ range: z.string().optional(), }), }) - -// insertFile reads the raw request stream itself (no body parser is -// mounted), so the body is validated as a streamed body. No fallbackSchema: -// insertFile never reads parseReq()'s return value (FileHandler.insertFile -// reads the stream off `req` directly), so a raw passthrough on failure -// can't hand the handler a wrong type -- a fallback here would just -// duplicate the primary schema. -export const insertFileSchema = z.object({ - body: zz.streamedBody(), -}) diff --git a/services/filestore/test/unit/js/schemas.test.js b/services/filestore/test/unit/js/schemas.test.js index e915592540..bdb3161337 100644 --- a/services/filestore/test/unit/js/schemas.test.js +++ b/services/filestore/test/unit/js/schemas.test.js @@ -5,21 +5,21 @@ describe('schemas', function () { describe('bucketFileParamsSchema', function () { it('accepts a well-formed bucket name', function () { const result = bucketFileParamsSchema.safeParse({ - params: { bucket: 'my-bucket-123', 0: 'some/key' }, + params: { bucket: 'my-bucket-123', key: 'some/key' }, }) expect(result.success).to.equal(true) }) it('rejects a bucket name with disallowed characters', function () { const result = bucketFileParamsSchema.safeParse({ - params: { bucket: 'My_Bucket!', 0: 'some/key' }, + params: { bucket: 'My_Bucket!', key: 'some/key' }, }) expect(result.success).to.equal(false) }) it('rejects an empty bucket name', function () { const result = bucketFileParamsSchema.safeParse({ - params: { bucket: '', 0: 'some/key' }, + params: { bucket: '', key: 'some/key' }, }) expect(result.success).to.equal(false) }) diff --git a/services/web/app/src/Features/Analytics/AnalyticsController.mjs b/services/web/app/src/Features/Analytics/AnalyticsController.mjs index 97753be08c..46c5b9a476 100644 --- a/services/web/app/src/Features/Analytics/AnalyticsController.mjs +++ b/services/web/app/src/Features/Analytics/AnalyticsController.mjs @@ -5,25 +5,72 @@ import GeoIpLookup from '../../infrastructure/GeoIpLookup.mjs' import Features from '../../infrastructure/Features.mjs' import { expressify } from '@overleaf/promise-utils' import AccountMappingHelper from './AccountMappingHelper.mjs' +import { z, zz, parseReq } from '../../infrastructure/Validation.mjs' + +// The set of segmentation/event keys is caller-supplied and dynamic (every +// analytics event and editing session sends a different set of dimensions), +// so it can't be enumerated up front -- treat it as a generic string-keyed +// record (genuinely open map, mirroring SplitTestHandler's getAssignmentSchema). +// The key regex mirrors AnalyticsManager's own _isAttributeValid, the only +// validation applied to these keys today; values are never validated there +// either, hence z.unknown(). +const attributeKeySchema = z.string().regex(/^[a-zA-Z0-9-_.:;,/]+$/) +const segmentationSchema = z.record(attributeKeySchema, z.unknown()) + +const registerSalesforceMappingSchema = z.object({ + body: z.strictObject({ + // sent by v1's V2RequestWorker#register_salesforce_mapping + createdAt: zz.datetime({ offset: true }), + salesforceId: z.string(), + v1Id: z.coerce.number().int(), + }), +}) + +// Rollout-temporary fallback (loosened primary schema; no zod validation +// existed for this route on main); delete when this route's +// REQ_VALIDATION_MODE instrumentation is removed. +const registerSalesforceMappingFallbackSchema = z.object({ + body: z.object({ + createdAt: z.coerce.date(), + salesforceId: z.string(), + v1Id: z.coerce.number(), + }), +}) async function registerSalesforceMapping(req, res, next) { if (!Features.hasFeature('analytics')) { return res.sendStatus(202) } - const { createdAt, salesforceId, v1Id } = req.body + const { body } = parseReq(req, registerSalesforceMappingSchema, { + logOnly: true, + fallbackSchema: registerSalesforceMappingFallbackSchema, + }) + const { createdAt, salesforceId, v1Id } = body AnalyticsManager.registerAccountMapping( AccountMappingHelper.generateV1Mapping(v1Id, salesforceId, createdAt) ) res.sendStatus(202) } +const updateEditingSessionSchema = z.object({ + params: z.strictObject({ + projectId: zz.objectId(), + }), + body: z.strictObject({ + segmentation: segmentationSchema.optional(), + }), +}) + async function updateEditingSession(req, res, next) { if (!Features.hasFeature('analytics')) { return res.sendStatus(202) } const userId = SessionManager.getLoggedInUserId(req.session) - const { projectId } = req.params - const segmentation = req.body.segmentation || {} + const { params, body } = parseReq(req, updateEditingSessionSchema, { + logOnly: true, + }) + const { projectId } = params + const segmentation = body.segmentation || {} let countryCode = null if (userId) { @@ -45,6 +92,13 @@ async function updateEditingSession(req, res, next) { res.sendStatus(202) } +const recordEventSchema = z.object({ + params: z.strictObject({ + event: zz.eventName(), + }), + body: segmentationSchema, +}) + // TODO: remove after clients running pre-#36315 code have drained function isIdleConnectionRestoredEvent(event, body) { if (event !== 'connection-restored') { @@ -60,16 +114,14 @@ function recordEvent(req, res, next) { if (!Features.hasFeature('analytics')) { return res.sendStatus(202) } - if (isIdleConnectionRestoredEvent(req.params.event, req.body)) { + const { params, body } = parseReq(req, recordEventSchema, { + logOnly: true, + }) + if (isIdleConnectionRestoredEvent(params.event, body)) { metrics.inc('analytics_idle_connection_restored_blocked') return res.sendStatus(202) } - delete req.body._csrf - AnalyticsManager.recordEventForSession( - req.session, - req.params.event, - req.body - ) + AnalyticsManager.recordEventForSession(req.session, params.event, body) res.sendStatus(202) } diff --git a/services/web/app/src/Features/Analytics/AnalyticsRegistrationSourceMiddleware.mjs b/services/web/app/src/Features/Analytics/AnalyticsRegistrationSourceMiddleware.mjs index 36945911c0..585ac8d297 100644 --- a/services/web/app/src/Features/Analytics/AnalyticsRegistrationSourceMiddleware.mjs +++ b/services/web/app/src/Features/Analytics/AnalyticsRegistrationSourceMiddleware.mjs @@ -2,6 +2,8 @@ import logger from '@overleaf/logger' import OError from '@overleaf/o-error' import AnalyticsRegistrationSourceHelper from './AnalyticsRegistrationSourceHelper.mjs' import SessionManager from '../../Features/Authentication/SessionManager.mjs' +import RequestHelper from './RequestHelper.mjs' +import { parseReq } from '../../infrastructure/Validation.mjs' function setSource(medium, source) { return function (req, res, next) { @@ -34,10 +36,14 @@ function setInbound() { const referrer = req.header('referrer') try { + // NOTE: The utm query flags will be removed in the next middleware. + const { query } = parseReq(req, RequestHelper.utmQuerySchema, { + logOnly: true, + }) AnalyticsRegistrationSourceHelper.setInbound( req.session, req.url, - req.query, + query, referrer ) } catch (error) { diff --git a/services/web/app/src/Features/Analytics/AnalyticsUTMTrackingMiddleware.mjs b/services/web/app/src/Features/Analytics/AnalyticsUTMTrackingMiddleware.mjs index 7b5d6a2b63..79b38bb3b8 100644 --- a/services/web/app/src/Features/Analytics/AnalyticsUTMTrackingMiddleware.mjs +++ b/services/web/app/src/Features/Analytics/AnalyticsUTMTrackingMiddleware.mjs @@ -1,16 +1,20 @@ import RequestHelper from './RequestHelper.mjs' import AnalyticsManager from './AnalyticsManager.mjs' -import { URL } from 'node:url' import Settings from '@overleaf/settings' import logger from '@overleaf/logger' +import { parseReq, getRawReqInput } from '../../infrastructure/Validation.mjs' import UrlHelper from '../Helpers/UrlHelper.mjs' function recordUTMTags() { return function (req, res, next) { - const rawQuery = req.query + const rawQuery = getRawReqInput(req).query try { - const utmValues = RequestHelper.parseUtm(rawQuery) + const { query } = parseReq(req, RequestHelper.utmQuerySchema, { + logOnly: true, + }) + + const utmValues = RequestHelper.parseUtm(query) if (utmValues) { const path = new URL(req.url, Settings.siteUrl).pathname diff --git a/services/web/app/src/Features/Compile/ClsiCacheController.mjs b/services/web/app/src/Features/Compile/ClsiCacheController.mjs index 92ee7b2b20..d32dd58ba1 100644 --- a/services/web/app/src/Features/Compile/ClsiCacheController.mjs +++ b/services/web/app/src/Features/Compile/ClsiCacheController.mjs @@ -17,6 +17,18 @@ import { z, zz } from '@overleaf/validation-tools' import { parseReq } from '../../infrastructure/Validation.mjs' const downloadFromCacheSchema = z.object({ + params: z.strictObject({ + Project_id: zz.objectId(), + editorBuildId: zz.editorBuildId(), + filename: zz.filepath().refine(s => ClsiCacheHandler.isAllowedFilename(s), { + message: 'path is not allowed', + }), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const downloadFromCacheFallbackSchema = z.object({ params: z.object({ Project_id: zz.objectId(), editorBuildId: zz.editorBuildId(), @@ -26,6 +38,12 @@ const downloadFromCacheSchema = z.object({ }), }) +const getLatestBuildFromCacheSchema = z.object({ + params: z.strictObject({ + Project_id: zz.objectId(), + }), +}) + /** * Download a file from a specific build on the clsi-cache. * @@ -36,7 +54,9 @@ const downloadFromCacheSchema = z.object({ async function downloadFromCache(req, res) { const { params: { Project_id: projectId, editorBuildId, filename }, - } = parseReq(req, downloadFromCacheSchema) + } = parseReq(req, downloadFromCacheSchema, { + fallbackSchema: downloadFromCacheFallbackSchema, + }) return await _downloadFromCacheWithParams( req, res, @@ -173,7 +193,10 @@ async function _downloadFromCacheWithParams( * @return {Promise} */ async function getLatestBuildFromCache(req, res) { - const { Project_id: projectId } = req.params + const { params } = parseReq(req, getLatestBuildFromCacheSchema, { + logOnly: true, + }) + const projectId = params.Project_id const userId = CompileController._getUserIdForCompile(req) try { const { diff --git a/services/web/app/src/Features/Compile/ClsiCacheManager.mjs b/services/web/app/src/Features/Compile/ClsiCacheManager.mjs index b96d916d5d..70b4cdc83a 100644 --- a/services/web/app/src/Features/Compile/ClsiCacheManager.mjs +++ b/services/web/app/src/Features/Compile/ClsiCacheManager.mjs @@ -50,7 +50,9 @@ async function getLatestBuildFromCache(projectId, userId, filename, signal) { ? lastUpdatedInRedis : lastUpdatedInMongo const isUpToDate = lastCompiled >= lastUpdated - const imageName = Path.basename(fullImageName) + // fullImageName is unset for projects outside SaaS (no + // Settings.currentImageName to default from on creation) + const imageName = fullImageName ? Path.basename(fullImageName) : undefined return { internal: { diff --git a/services/web/app/src/Features/Compile/CompileController.mjs b/services/web/app/src/Features/Compile/CompileController.mjs index 2dc538d622..b94f794ecc 100644 --- a/services/web/app/src/Features/Compile/CompileController.mjs +++ b/services/web/app/src/Features/Compile/CompileController.mjs @@ -45,7 +45,25 @@ function getOutputFilesArchiveSpecification(projectId, userId, buildId) { } } +// Only reads the one query field this helper itself consumes -- the route +// handlers that call this (compile, ClsiCacheController.getLatestBuildFromCache) +// separately validate their own full query/params/body with their own schema. +const splitTestOptionsQuerySchema = z.object({ + body: z.object({ + png2pdf: z.boolean().optional(), + }), + query: z.object({ + // presence-based flag: the frontend only ever sets this to "true" or + // omits it (see compiler.ts buildCompileParams), so a bare optional + // string preserves the existing truthy check below + enable_pdf_caching: z.string().optional(), + }), +}) + async function _getSplitTestOptions(req, res) { + const { body, query } = parseReq(req, splitTestOptionsQuerySchema, { + logOnly: true, + }) const compileFromHistory = await SplitTestHandler.promises.featureFlagEnabled( req, res, @@ -58,7 +76,7 @@ async function _getSplitTestOptions(req, res) { const pdfCachingMinChunkSize = Settings.pdfCachingMinChunkSize const pdfCachingOptions = - !enablePdfCaching || !req.query.enable_pdf_caching + !enablePdfCaching || !query.enable_pdf_caching ? // The frontend does not want to do pdf caching. { enablePdfCaching: false } : { enablePdfCaching, pdfCachingMinChunkSize } @@ -78,7 +96,7 @@ async function _getSplitTestOptions(req, res) { ) const png2PdfOptions = - !enablePng2Pdf || !req.body.png2pdf + !enablePng2Pdf || !body.png2pdf ? { enablePng2Pdf: false } : { enablePng2Pdf } @@ -91,12 +109,21 @@ async function _getSplitTestOptions(req, res) { } } -async function _syncTeX(req, res, direction, validatedOptions) { - const projectId = req.params.Project_id - const { editorId, buildId, clsiserverid: clsiServerId } = req.query - if (!editorId?.match(/^[a-f0-9-]+$/)) throw new Error('invalid ?editorId') - if (!buildId?.match(/^[a-f0-9-]+$/)) throw new Error('invalid ?buildId') - +// buildId in syncTeXBaseQuery below is validated with zz.buildId(), which +// requires a hyphen-separated pair of hex runs (/^[0-9a-f]+-[0-9a-f]+$/) -- +// stricter than the bare hex-ish regex the manual guards restored below +// check editorId/buildId against. Both layers run: the schema during this +// rollout's logOnly phase, and the manual guards unconditionally. +async function _syncTeX( + req, + res, + projectId, + editorId, + buildId, + clsiServerId, + direction, + validatedOptions +) { const userId = CompileController._getUserIdForCompile(req) try { const body = await CompileManager.promises.syncTeX(projectId, userId, { @@ -116,7 +143,77 @@ async function _syncTeX(req, res, direction, validatedOptions) { } } +const compileSchema = z.object({ + params: z.strictObject({ + Project_id: zz.objectId(), + }), + query: z.object({ + // presence-based flags: the frontend only ever sets these to "true" or + // omits them (see compiler.ts buildCompileParams); bare optional + // strings preserve the existing truthy checks below rather than + // z.stringbool(), which would treat an (unused-in-practice) explicit + // "false" value differently than the current code does + auto_compile: z.string().optional(), + file_line_errors: z.string().optional(), + enable_pdf_caching: z.string().optional(), + }), + body: z.strictObject({ + stopOnFirstError: z.boolean().optional(), + editorId: z.uuid().optional(), + rootResourcePath: zz.filepath().optional(), + rootDoc_id: zz.objectId().nullish(), + // legacy nested duplicate of rootDoc_id, only sent by old cached + // frontend bundles during a deploy transition -- can be removed once + // that's no longer a concern. Deliberately non-strict: this object's + // shape before that cleanup isn't fully known here, and the only field + // ever read from it is rootDoc_id. + settingsOverride: z + .object({ rootDoc_id: zz.objectId().nullish() }) + .optional(), + compiler: z.string().optional(), + draft: z.boolean().optional(), + png2pdf: z.boolean().optional(), + // silently ignored (not rejected) when not one of these three, to + // match the existing .includes() check below + check: z.string().optional(), + incrementalCompilesEnabled: z.boolean().optional(), + }), +}) + +const stopCompileSchema = z.object({ + params: z.strictObject({ + Project_id: zz.objectId(), + }), +}) + +const compileSubmissionSchema = z.object({ + params: z.strictObject({ + submission_id: zz.submissionId(), + }), + // This is the public compile-submission API used by external callers (see + // modules/publish-modal/app/src/PublishModalRouter.mjs): the body is a + // whole CLSI compile job spec ("resources" etc.) forwarded verbatim to + // ClsiManager.sendExternalRequest -- CLSI's own compileSchema + // (services/clsi/app/js/CompileController.js) is what actually validates + // that job spec, so this is treated as a genuinely open map, letting the + // few option flags below still be read out named without stripping the + // rest of the payload before it's forwarded (a non-strict z.object() would + // strip unrecognized keys from the parsed result, breaking the forward). + body: z.record(z.string(), z.unknown()), +}) + const deleteAuxFilesSchema = z.object({ + params: z.strictObject({ + Project_id: zz.objectId(), + }), + query: z.object({ + clsiserverid: zz.clsiServerId().optional(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const deleteAuxFilesFallbackSchema = z.object({ params: z.object({ Project_id: zz.objectId(), }), @@ -126,6 +223,18 @@ const deleteAuxFilesSchema = z.object({ }) const wordCountSchema = z.object({ + params: z.strictObject({ + Project_id: zz.objectId(), + }), + query: z.object({ + clsiserverid: zz.clsiServerId().optional(), + file: z.string().optional(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const wordCountFallbackSchema = z.object({ params: z.object({ Project_id: zz.objectId(), }), @@ -136,6 +245,26 @@ const wordCountSchema = z.object({ }) const getFileForSubmissionFromClsiSchema = z.object({ + params: z.strictObject({ + submissionId: zz.submissionId(), + build_id: zz.buildId(), + file: zz.filepath(), + }), + query: z.object({ + clsiserverid: zz.clsiServerId().optional(), + // V1's CLSI::Response#v2_api_url always appends this (from the + // compile response's compileGroup field, which compileSubmission below + // never populates -- see its res.json()), so it arrives here blank + // rather than one of zz.compileGroup()'s enum values. Unread by the + // handler either way; accept the real enum plus that one specific + // blank case, rather than opening this up to an arbitrary string. + compileGroup: zz.compileGroup().or(z.literal('')).optional(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const getFileForSubmissionFromClsiFallbackSchema = z.object({ params: z.object({ submissionId: zz.submissionId(), build_id: zz.buildId(), @@ -146,7 +275,33 @@ const getFileForSubmissionFromClsiSchema = z.object({ }), }) +// getFileFromClsi/getOutputZipFromClsi below (not downloadPdf, which only +// has one route) are each mounted on two routes: one plain, and one with an +// extra (unused by the handler) :user_id segment for a specific user's +// build -- see router.mjs. user_id is declared but never read, purely so +// the strict params schema still accepts the second route's extra segment. const getFileFromClsiSchema = z.object({ + params: z.strictObject({ + Project_id: zz.objectId(), + user_id: zz.objectId().optional(), + build_id: zz.buildId(), + file: zz.filepath(), + }), + query: z.object({ + clsiserverid: zz.clsiServerId().optional(), + editorId: z.uuid().optional(), + // frontend's buildFileList always appends this to every per-file + // download link it builds for the "Other logs and files" menu + // (services/web/frontend/js/features/pdf-preview/util/file-list.ts) + compileGroup: zz.compileGroup().optional(), + // not consumed, traffic tag + enable_pdf_caching: z.stringbool().optional(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const getFileFromClsiFallbackSchema = z.object({ params: z.object({ Project_id: zz.objectId(), build_id: zz.buildId(), @@ -155,10 +310,32 @@ const getFileFromClsiSchema = z.object({ query: z.object({ clsiserverid: zz.clsiServerId().optional(), editorId: z.uuid().optional(), + // not consumed, traffic tag + enable_pdf_caching: z.string().optional(), }), }) const getOutputPDFFromClsiSchema = z.object({ + params: z.strictObject({ + Project_id: zz.objectId(), + build_id: zz.buildId(), + }), + query: z.object({ + clsiserverid: zz.clsiServerId().optional(), + editorId: z.uuid().optional(), + // presence-based flag, see compile()'s query fields above + popupDownload: z.string().optional(), + // the PDF download link is built from the same query params as the + // preview's pdfUrl (see output-files.ts), which includes compileGroup + compileGroup: zz.compileGroup().optional(), + // not consumed, traffic tag + enable_pdf_caching: z.stringbool().optional(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const getOutputPDFFromClsiFallbackSchema = z.object({ params: z.object({ Project_id: zz.objectId(), build_id: zz.buildId(), @@ -166,54 +343,123 @@ const getOutputPDFFromClsiSchema = z.object({ query: z.object({ clsiserverid: zz.clsiServerId().optional(), editorId: z.uuid().optional(), + // not consumed, traffic tag + enable_pdf_caching: z.string().optional(), }), }) const getOutputZipFromClsiSchema = z.object({ + params: z.strictObject({ + Project_id: zz.objectId(), + user_id: zz.objectId().optional(), + build_id: zz.buildId(), + }), + query: z.object({ + clsiserverid: zz.clsiServerId().optional(), + // frontend's buildFileList always appends editorId/compileGroup, and + // (for the "Download all" archive link specifically) one `files` entry + // per output file, to the query string it builds + // (services/web/frontend/js/features/pdf-preview/util/file-list.ts). + // None of these three are read by the handler below -- the backend + // already knows the build's full file list -- they're carried along + // only because the archive URL is built from the same query-params + // object as the per-file download links. + editorId: z.uuid().optional(), + compileGroup: zz.compileGroup().optional(), + // a single file in the archive arrives as a bare string, not a + // one-element array -- Express's query parser only produces an array + // once a query key repeats + files: z.union([z.string(), z.array(z.string())]).optional(), + // not consumed, traffic tag + enable_pdf_caching: z.stringbool().optional(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const getOutputZipFromClsiFallbackSchema = z.object({ params: z.object({ Project_id: zz.objectId(), build_id: zz.buildId(), }), query: z.object({ clsiserverid: zz.clsiServerId().optional(), + // not consumed, traffic tag + enable_pdf_caching: z.string().optional(), + }), +}) + +const compileAndDownloadPdfSchema = z.object({ + params: z.strictObject({ + project_id: zz.objectId(), + }), +}) + +const syncTeXBaseQuery = { + editorId: z.uuid().optional(), + buildId: zz.buildId(), + clsiserverid: zz.clsiServerId().optional(), +} + +const proxySyncPdfSchema = z.object({ + params: z.strictObject({ + Project_id: zz.objectId(), + }), + query: z.object({ + page: z.string().regex(/^\d+$/), + h: z.string().regex(/^-?\d+\.\d+$/), + v: z.string().regex(/^-?\d+\.\d+$/), + ...syncTeXBaseQuery, + }), +}) + +const proxySyncCodeSchema = z.object({ + params: z.strictObject({ + Project_id: zz.objectId(), + }), + query: z.object({ + file: zz.filepath(), + line: z.string().regex(/^\d+$/), + column: z.string().regex(/^\d+$/), + ...syncTeXBaseQuery, }), }) const _CompileController = { async compile(req, res) { res.setTimeout(COMPILE_TIMEOUT_MS) - const projectId = req.params.Project_id - const isAutoCompile = !!req.query.auto_compile - const fileLineErrors = !!req.query.file_line_errors - const stopOnFirstError = !!req.body.stopOnFirstError + const { params, query, body } = parseReq(req, compileSchema, { + logOnly: true, + }) + const projectId = params.Project_id + const isAutoCompile = !!query.auto_compile + const fileLineErrors = !!query.file_line_errors + const stopOnFirstError = !!body.stopOnFirstError const userId = SessionManager.getLoggedInUserId(req.session) const options = { isAutoCompile, fileLineErrors, stopOnFirstError, - editorId: req.body.editorId, - rootResourcePath: req.body.rootResourcePath, + editorId: body.editorId, + rootResourcePath: body.rootResourcePath, } - if (req.body.rootDoc_id) { - options.rootDoc_id = req.body.rootDoc_id - } else if ( - req.body.settingsOverride && - req.body.settingsOverride.rootDoc_id - ) { + if (body.rootDoc_id) { + options.rootDoc_id = body.rootDoc_id + } else if (body.settingsOverride && body.settingsOverride.rootDoc_id) { // Can be removed after deploy - options.rootDoc_id = req.body.settingsOverride.rootDoc_id + options.rootDoc_id = body.settingsOverride.rootDoc_id } - if (req.body.compiler) { - options.compiler = req.body.compiler + if (body.compiler) { + options.compiler = body.compiler } - if (req.body.draft) { - options.draft = req.body.draft + if (body.draft) { + options.draft = body.draft } - if (['validate', 'error', 'silent'].includes(req.body.check)) { - options.check = req.body.check + if (['validate', 'error', 'silent'].includes(body.check)) { + options.check = body.check } - if (req.body.incrementalCompilesEnabled) { + if (body.incrementalCompilesEnabled) { options.incrementalCompilesEnabled = true } @@ -363,7 +609,8 @@ const _CompileController = { }, async stopCompile(req, res) { - const projectId = req.params.Project_id + const { params } = parseReq(req, stopCompileSchema, { logOnly: true }) + const projectId = params.Project_id const userId = SessionManager.getLoggedInUserId(req.session) await CompileManager.promises.stopCompile(projectId, userId) res.sendStatus(200) @@ -372,30 +619,32 @@ const _CompileController = { // Used for submissions through the public API async compileSubmission(req, res) { res.setTimeout(COMPILE_TIMEOUT_MS) - const submissionId = req.params.submission_id + const { params, body } = parseReq(req, compileSubmissionSchema, { + logOnly: true, + }) + const submissionId = params.submission_id const options = {} - if (req.body?.rootResourcePath != null) { - options.rootResourcePath = req.body.rootResourcePath + if (body?.rootResourcePath != null) { + options.rootResourcePath = body.rootResourcePath } - if (req.body?.compiler) { - options.compiler = req.body.compiler + if (body?.compiler) { + options.compiler = body.compiler } - if (req.body?.draft) { - options.draft = req.body.draft + if (body?.draft) { + options.draft = body.draft } - if (['validate', 'error', 'silent'].includes(req.body?.check)) { - options.check = req.body.check + if (['validate', 'error', 'silent'].includes(body?.check)) { + options.check = body.check } options.compileGroup = - req.body?.compileGroup || Settings.defaultFeatures.compileGroup + body?.compileGroup || Settings.defaultFeatures.compileGroup options.compileBackendClass = Settings.apis.clsi.submissionCompileBackendClass - options.timeout = - req.body?.timeout || Settings.defaultFeatures.compileTimeout + options.timeout = body?.timeout || Settings.defaultFeatures.compileTimeout const { status, outputFiles, clsiServerId, validationProblems } = await ClsiManager.promises.sendExternalRequest( submissionId, - req.body, + body, options ) res.json({ @@ -416,8 +665,10 @@ const _CompileController = { async downloadPdf(req, res) { const { params: { Project_id: projectId, build_id: buildId }, - query: { clsiserverid: clsiServerId, editorId }, - } = parseReq(req, getOutputPDFFromClsiSchema) + query: { clsiserverid: clsiServerId, editorId, popupDownload }, + } = parseReq(req, getOutputPDFFromClsiSchema, { + fallbackSchema: getOutputPDFFromClsiFallbackSchema, + }) Metrics.inc('pdf-downloads') try { await pdfDownloadRateLimiter.consume(req.ip, 1, { method: 'ip' }) @@ -439,7 +690,7 @@ const _CompileController = { res.contentType('application/pdf') const filename = `${_CompileController._getSafeProjectName(project)}.pdf` - if (req.query.popupDownload) { + if (popupDownload) { res.setContentDisposition('attachment', { filename }) } else { res.setContentDisposition('inline', { filename }) @@ -468,7 +719,9 @@ const _CompileController = { const { params: { Project_id: projectId }, query: { clsiserverid }, - } = parseReq(req, deleteAuxFilesSchema) + } = parseReq(req, deleteAuxFilesSchema, { + fallbackSchema: deleteAuxFilesFallbackSchema, + }) const userId = CompileController._getUserIdForCompile(req) await CompileManager.promises.deleteAuxFiles( projectId, @@ -480,7 +733,10 @@ const _CompileController = { // this is only used by templates, so is not called with a userId async compileAndDownloadPdf(req, res) { - const projectId = req.params.project_id + const { params } = parseReq(req, compileAndDownloadPdfSchema, { + logOnly: true, + }) + const projectId = params.project_id let outputFiles, clsiServerId, buildId try { @@ -522,7 +778,9 @@ const _CompileController = { const { params: { Project_id: projectId, build_id: buildId }, query: { clsiserverid: clsiServerId }, - } = parseReq(req, getOutputZipFromClsiSchema) + } = parseReq(req, getOutputZipFromClsiSchema, { + fallbackSchema: getOutputZipFromClsiFallbackSchema, + }) const project = await ProjectGetter.promises.getProject(projectId, { name: 1, @@ -548,7 +806,9 @@ const _CompileController = { const { params: { Project_id: projectId, build_id: buildId, file }, query: { clsiserverid: clsiServerId, editorId }, - } = parseReq(req, getFileFromClsiSchema) + } = parseReq(req, getFileFromClsiSchema, { + fallbackSchema: getFileFromClsiFallbackSchema, + }) await _downloadFromClsiNginx( projectId, @@ -567,7 +827,9 @@ const _CompileController = { const { params: { submissionId, build_id: buildId, file }, query: { clsiserverid: clsiServerId }, - } = parseReq(req, getFileForSubmissionFromClsiSchema) + } = parseReq(req, getFileForSubmissionFromClsiSchema, { + fallbackSchema: getFileForSubmissionFromClsiFallbackSchema, + }) await _downloadFromClsiNginx( submissionId, null, @@ -582,7 +844,12 @@ const _CompileController = { }, async proxySyncPdf(req, res) { - const { page, h, v } = req.query + const { + params: { Project_id: projectId }, + query: { page, h, v, editorId, buildId, clsiserverid: clsiServerId }, + } = parseReq(req, proxySyncPdfSchema, { logOnly: true }) + if (!editorId?.match(/^[a-f0-9-]+$/)) throw new Error('invalid ?editorId') + if (!buildId?.match(/^[a-f0-9-]+$/)) throw new Error('invalid ?buildId') if (!page?.match(/^\d+$/)) { throw new Error('invalid page parameter') } @@ -592,11 +859,32 @@ const _CompileController = { if (!v?.match(/^-?\d+\.\d+$/)) { throw new Error('invalid v parameter') } - await _syncTeX(req, res, 'pdf', { page, h, v }) + await _syncTeX( + req, + res, + projectId, + editorId, + buildId, + clsiServerId, + 'pdf', + { page, h, v } + ) }, async proxySyncCode(req, res) { - const { file, line, column } = req.query + const { + params: { Project_id: projectId }, + query: { + file, + line, + column, + editorId, + buildId, + clsiserverid: clsiServerId, + }, + } = parseReq(req, proxySyncCodeSchema, { logOnly: true }) + if (!editorId?.match(/^[a-f0-9-]+$/)) throw new Error('invalid ?editorId') + if (!buildId?.match(/^[a-f0-9-]+$/)) throw new Error('invalid ?buildId') if (!file) { throw new Error('missing file parameter') } @@ -615,11 +903,22 @@ const _CompileController = { if (!column?.match(/^\d+$/)) { throw new Error('invalid column parameter') } - await _syncTeX(req, res, 'code', { file, line, column }) + await _syncTeX( + req, + res, + projectId, + editorId, + buildId, + clsiServerId, + 'code', + { file, line, column } + ) }, async wordCount(req, res) { - const { params, query } = parseReq(req, wordCountSchema) + const { params, query } = parseReq(req, wordCountSchema, { + fallbackSchema: wordCountFallbackSchema, + }) const projectId = params.Project_id const file = query.file || false const { clsiserverid } = query diff --git a/services/web/app/src/Features/Documents/DocumentController.mjs b/services/web/app/src/Features/Documents/DocumentController.mjs index 1c325e5ad2..d5b1bbbec8 100644 --- a/services/web/app/src/Features/Documents/DocumentController.mjs +++ b/services/web/app/src/Features/Documents/DocumentController.mjs @@ -27,12 +27,14 @@ const insertOp = z.strictObject({ i: z.string(), p: z.number().int().min(0), u: z.boolean().optional(), + fixedRemoveChange: z.boolean().optional(), }) const deleteOp = z.strictObject({ d: z.string(), p: z.number().int().min(0), u: z.boolean().optional(), + fixedRemoveChange: z.boolean().optional(), }) const commentOp = z.strictObject({ diff --git a/services/web/app/src/Features/FileStore/FileStoreController.mjs b/services/web/app/src/Features/FileStore/FileStoreController.mjs index ab5e4f014b..174b4f057b 100644 --- a/services/web/app/src/Features/FileStore/FileStoreController.mjs +++ b/services/web/app/src/Features/FileStore/FileStoreController.mjs @@ -8,15 +8,34 @@ import ProjectLocator from '../Project/ProjectLocator.mjs' import HistoryManager from '../History/HistoryManager.mjs' import Errors from '../Errors/Errors.js' import { preparePlainTextResponse } from '../../infrastructure/Response.mjs' +import { z, zz, parseReq } from '../../infrastructure/Validation.mjs' + +const getFileSchema = z.object({ + params: z.strictObject({ + Project_id: zz.objectId(), + File_id: zz.objectId(), + }), + // the query string isn't read for any decision-making -- only logged + // verbatim for debugging, so treat it as a generic open map + query: z.record(z.string(), z.unknown()), +}) + +const getFileHeadSchema = z.object({ + params: z.strictObject({ + Project_id: zz.objectId(), + File_id: zz.objectId(), + }), +}) /** * @param {any} req * @param {any} res */ async function getFile(req, res) { - const projectId = req.params.Project_id - const fileId = req.params.File_id - const queryString = req.query + const { params, query } = parseReq(req, getFileSchema, { logOnly: true }) + const projectId = params.Project_id + const fileId = params.File_id + const queryString = query const userAgent = req.get('User-Agent') req.logger.addFields({ projectId, fileId, queryString }) @@ -110,8 +129,9 @@ async function getFile(req, res) { * @param {any} res */ async function getFileHead(req, res) { - const projectId = req.params.Project_id - const fileId = req.params.File_id + const { params } = parseReq(req, getFileHeadSchema, { logOnly: true }) + const projectId = params.Project_id + const fileId = params.File_id let file try { diff --git a/services/web/app/src/Features/InactiveData/InactiveProjectController.mjs b/services/web/app/src/Features/InactiveData/InactiveProjectController.mjs index 825845acc8..6c32cf6c73 100644 --- a/services/web/app/src/Features/InactiveData/InactiveProjectController.mjs +++ b/services/web/app/src/Features/InactiveData/InactiveProjectController.mjs @@ -10,14 +10,38 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ import InactiveProjectManager from './InactiveProjectManager.mjs' +import { z, zz, parseReq } from '../../infrastructure/Validation.mjs' + +const deactivateOldProjectsSchema = z.object({ + body: z.strictObject({ + numberOfProjectsToArchive: z.coerce.number().int().optional(), + ageOfProjects: z.coerce.number().int().optional(), + }), +}) + +// Rollout-temporary fallback (loosened primary schema; no zod validation +// existed for this route on main); delete when this route's +// REQ_VALIDATION_MODE instrumentation is removed. +const deactivateOldProjectsFallbackSchema = z.object({ + body: z.object({ + numberOfProjectsToArchive: z.coerce.number().optional(), + ageOfProjects: z.coerce.number().optional(), + }), +}) + +const deactivateProjectSchema = z.object({ + params: z.strictObject({ + project_id: zz.objectId(), + }), +}) export default { deactivateOldProjects(req, res) { - const numberOfProjectsToArchive = parseInt( - req.body.numberOfProjectsToArchive, - 10 - ) - const { ageOfProjects } = req.body + const { body } = parseReq(req, deactivateOldProjectsSchema, { + logOnly: true, + fallbackSchema: deactivateOldProjectsFallbackSchema, + }) + const { numberOfProjectsToArchive, ageOfProjects } = body return InactiveProjectManager.deactivateOldProjects( numberOfProjectsToArchive, ageOfProjects, @@ -32,7 +56,10 @@ export default { }, deactivateProject(req, res) { - const { project_id: projectId } = req.params + const { params } = parseReq(req, deactivateProjectSchema, { + logOnly: true, + }) + const { project_id: projectId } = params return InactiveProjectManager.deactivateProject(projectId, function (err) { if (err != null) { return res.sendStatus(500) diff --git a/services/web/app/src/Features/LinkedFiles/LinkedFilesController.mjs b/services/web/app/src/Features/LinkedFiles/LinkedFilesController.mjs index ee9d5c0dbf..daf01bd782 100644 --- a/services/web/app/src/Features/LinkedFiles/LinkedFilesController.mjs +++ b/services/web/app/src/Features/LinkedFiles/LinkedFilesController.mjs @@ -48,7 +48,84 @@ const { let LinkedFilesController +// Per-provider shape of the client-submitted `data` object. This mirrors +// overleaf-editor-core's `rawLinkedFileData` discriminated union (the +// *stored* linkedFileData shape), but is not identical to it: `provider` +// lives as a sibling of `data` here rather than inside it, `importedAt` is +// never client-submitted (set server-side after validation), and a couple +// of fields are intentionally looser than the stored shape because the +// agents apply defaults/coercions between validation and storage -- +// `zotero`'s `format` defaults to 'bibtex' when absent (ZoteroAgent +// `_getFormat`), and `project_file`/`project_output_file`'s +// `v1_source_doc_id` may arrive as a number (existing callers do this; the +// agent's `_canCreate` check rejects v1 ids with its own 403, so this must +// not be intercepted by validation first). const createLinkedFileSchema = z.object({ + params: z.strictObject({ + project_id: zz.objectId(), + }), + body: z.discriminatedUnion('provider', [ + z.strictObject({ + name: z.string(), + parent_folder_id: zz.objectId(), + provider: z.literal('url'), + data: z.strictObject({ + url: z.string(), + }), + }), + z.strictObject({ + name: z.string(), + parent_folder_id: zz.objectId(), + provider: z.literal('project_file'), + data: z.strictObject({ + source_project_id: zz.objectId().optional(), + v1_source_doc_id: z.union([z.string(), z.number()]).optional(), + source_entity_path: z.string(), + }), + }), + z.strictObject({ + name: z.string(), + parent_folder_id: zz.objectId(), + provider: z.literal('project_output_file'), + data: z.strictObject({ + source_project_id: zz.objectId().optional(), + v1_source_doc_id: z.union([z.string(), z.number()]).optional(), + source_output_file_path: z.string(), + build_id: zz.buildId().optional(), + clsiServerId: z.string().optional(), + }), + }), + z.strictObject({ + name: z.string(), + parent_folder_id: zz.objectId(), + provider: z.literal('mendeley'), + data: z.strictObject({ + group_id: zz.routeSegment().optional(), + }), + }), + z.strictObject({ + name: z.string(), + parent_folder_id: zz.objectId(), + provider: z.literal('zotero'), + data: z.strictObject({ + format: z.enum(['bibtex', 'biblatex']).optional(), + group_id: zz.routeSegment().optional(), + }), + }), + z.strictObject({ + name: z.string(), + parent_folder_id: zz.objectId(), + provider: z.literal('papers'), + data: z.strictObject({ + group_id: zz.routeSegment().optional(), + }), + }), + ]), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const createLinkedFileFallbackSchema = z.object({ params: z.object({ project_id: zz.objectId(), }), @@ -61,6 +138,19 @@ const createLinkedFileSchema = z.object({ }) const refreshLinkedFileSchema = z.object({ + params: z.strictObject({ + project_id: zz.objectId(), + file_id: zz.objectId(), + }), + body: z.strictObject({ + clientId: z.string().optional(), + shouldReindexReferences: z.boolean().optional(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const refreshLinkedFileFallbackSchema = z.object({ params: z.object({ project_id: zz.objectId(), file_id: zz.objectId(), @@ -72,7 +162,9 @@ const refreshLinkedFileSchema = z.object({ }) async function createLinkedFile(req, res, next) { - const { params, body } = parseReq(req, createLinkedFileSchema) + const { params, body } = parseReq(req, createLinkedFileSchema, { + fallbackSchema: createLinkedFileFallbackSchema, + }) const { project_id: projectId } = params const { name, provider, data, parent_folder_id: parentFolderId } = body const userId = SessionManager.getLoggedInUserId(req.session) @@ -105,7 +197,9 @@ async function createLinkedFile(req, res, next) { } async function refreshLinkedFile(req, res, next) { - const { params, body } = parseReq(req, refreshLinkedFileSchema) + const { params, body } = parseReq(req, refreshLinkedFileSchema, { + fallbackSchema: refreshLinkedFileFallbackSchema, + }) const { project_id: projectId, file_id: fileId } = params const { clientId, shouldReindexReferences } = body const userId = SessionManager.getLoggedInUserId(req.session) diff --git a/services/web/app/src/Features/PasswordReset/PasswordResetController.mjs b/services/web/app/src/Features/PasswordReset/PasswordResetController.mjs index 8079e54187..4d52bcd775 100644 --- a/services/web/app/src/Features/PasswordReset/PasswordResetController.mjs +++ b/services/web/app/src/Features/PasswordReset/PasswordResetController.mjs @@ -12,6 +12,15 @@ import { z, parseReq } from '../../infrastructure/Validation.mjs' import Features from '../../infrastructure/Features.mjs' const setNewUserPasswordSchema = z.object({ + body: z.strictObject({ + email: z.string().optional(), + password: z.string(), + passwordResetToken: z.string(), + }), +}) +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const setNewUserPasswordFallbackSchema = z.object({ body: z.object({ email: z.string().optional(), password: z.string(), @@ -21,7 +30,9 @@ const setNewUserPasswordSchema = z.object({ async function setNewUserPassword(req, res, next) { let user - const { body } = parseReq(req, setNewUserPasswordSchema) + const { body } = parseReq(req, setNewUserPasswordSchema, { + fallbackSchema: setNewUserPasswordFallbackSchema, + }) let { passwordResetToken, password, email } = body if (!passwordResetToken || !password) { return res.status(400).json({ @@ -114,13 +125,22 @@ async function setNewUserPassword(req, res, next) { } const requestResetSchema = z.object({ + body: z.strictObject({ + email: z.string(), + }), +}) +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const requestResetFallbackSchema = z.object({ body: z.object({ email: z.string(), }), }) async function requestReset(req, res, next) { - const { body } = parseReq(req, requestResetSchema) + const { body } = parseReq(req, requestResetSchema, { + fallbackSchema: requestResetFallbackSchema, + }) const email = EmailsHelper.parseEmail(body.email) if (!email) { return res.status(400).json({ diff --git a/services/web/app/src/Features/ServerAdmin/AdminController.mjs b/services/web/app/src/Features/ServerAdmin/AdminController.mjs index 76bf71143e..fb16871654 100644 --- a/services/web/app/src/Features/ServerAdmin/AdminController.mjs +++ b/services/web/app/src/Features/ServerAdmin/AdminController.mjs @@ -10,6 +10,52 @@ import ProjectGetter from '../Project/ProjectGetter.mjs' import Modules from '../../infrastructure/Modules.mjs' import Features from '../../infrastructure/Features.mjs' import { expressify } from '@overleaf/promise-utils' +import { z, zz, parseReq } from '../../infrastructure/Validation.mjs' + +const closeEditorSchema = z.object({ + body: z.strictObject({ + // the real form (views/admin/index.pug) never actually sends this + // field; a falsy/missing value is treated as "closed" + isOpen: z.boolean().optional(), + }), +}) + +const disconnectAllUsersSchema = z.object({ + query: z.object({ + delay: z.coerce.number().optional(), + }), +}) + +// Rollout-temporary fallback (loosened primary schema; no zod validation +// existed for this route on main); delete when this route's +// REQ_VALIDATION_MODE instrumentation is removed. `delay` is used +// numerically below, so a raw passthrough still needs to coerce it. +const disconnectAllUsersFallbackSchema = z.object({ + query: z.object({ + delay: z + .unknown() + .optional() + .transform(v => (v === undefined ? undefined : Number(v))), + }), +}) + +const flushProjectToTpdsSchema = z.object({ + body: z.strictObject({ + project_id: zz.objectId(), + }), +}) + +const pollDropboxForUserSchema = z.object({ + body: z.strictObject({ + user_id: zz.objectId(), + }), +}) + +const createMessageSchema = z.object({ + body: z.strictObject({ + content: z.string(), + }), +}) const AdminController = { _sendDisconnectAllUsersMessage: delay => { @@ -61,7 +107,11 @@ const AdminController = { disconnectAllUsers: (req, res) => { logger.warn('disconecting everyone') - const delay = (req.query && req.query.delay) > 0 ? req.query.delay : 10 + const { query } = parseReq(req, disconnectAllUsersSchema, { + logOnly: true, + fallbackSchema: disconnectAllUsersFallbackSchema, + }) + const delay = query.delay > 0 ? query.delay : 10 AdminController._sendDisconnectAllUsersMessage(delay) res.redirect('/admin#open-close-editor') }, @@ -74,12 +124,16 @@ const AdminController = { closeEditor(req, res) { logger.warn('closing editor') - Settings.editorIsOpen = req.body.isOpen + const { body } = parseReq(req, closeEditorSchema, { logOnly: true }) + Settings.editorIsOpen = body.isOpen res.redirect('/admin#open-close-editor') }, flushProjectToTpds(req, res, next) { - TpdsProjectFlusher.flushProjectToTpds(req.body.project_id, error => { + const { body } = parseReq(req, flushProjectToTpdsSchema, { + logOnly: true, + }) + TpdsProjectFlusher.flushProjectToTpds(body.project_id, error => { if (error) { return next(error) } @@ -88,12 +142,15 @@ const AdminController = { }, pollDropboxForUser(req, res) { - const { user_id: userId } = req.body - TpdsUpdateSender.pollDropboxForUser(userId, () => res.sendStatus(200)) + const { body } = parseReq(req, pollDropboxForUserSchema, { + logOnly: true, + }) + TpdsUpdateSender.pollDropboxForUser(body.user_id, () => res.sendStatus(200)) }, createMessage(req, res, next) { - SystemMessageManager.createMessage(req.body.content, function (error) { + const { body } = parseReq(req, createMessageSchema, { logOnly: true }) + SystemMessageManager.createMessage(body.content, function (error) { if (error) { return next(error) } diff --git a/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs b/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs index e9ecb612ec..93a041bb13 100644 --- a/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs +++ b/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs @@ -15,6 +15,7 @@ import SessionManager from '../Authentication/SessionManager.mjs' import logger from '@overleaf/logger' import SplitTestSessionHandler from './SplitTestSessionHandler.mjs' import SplitTestUserGetter from './SplitTestUserGetter.mjs' +import { getRawReqInput } from '../../infrastructure/Validation.mjs' /** * @import { Assignment } from "./types" @@ -69,7 +70,10 @@ async function getAssignment( await _loadSplitTestInfoInLocals(res.locals, splitTestName, req.session) if (!ignoreOverrides) { - let query = req.query || {} + // query keys are caller-supplied and dynamic (one per split test + // name), so this is read raw rather than by name (case 1: verbatim + // forwarding) + let query = getRawReqInput(req).query || {} if (includeReferer && req.headers.referer) { // Pick up the query of the top-level page, i.e. what's in the browsers address bar, from ajax requests. // E.g. /project/:id?split-test=foo -> ajax /project/:id/compile should see split-test=foo. @@ -200,7 +204,10 @@ async function hasUserBeenAssignedToVariant( ignoreVersion = false ) { try { - const { session = {}, query = {} } = req + const { session = {} } = req + // same dynamic caller-supplied key as getAssignment() above (case 1: + // verbatim forwarding) + const { query } = getRawReqInput(req) const splitTest = await _getSplitTest(splitTestName) const currentVersion = SplitTestUtils.getCurrentVersion(splitTest) diff --git a/services/web/app/src/Features/TokenAccess/TokenAccessController.mjs b/services/web/app/src/Features/TokenAccess/TokenAccessController.mjs index ae7a670949..d832818a5a 100644 --- a/services/web/app/src/Features/TokenAccess/TokenAccessController.mjs +++ b/services/web/app/src/Features/TokenAccess/TokenAccessController.mjs @@ -23,6 +23,7 @@ import UserGetter from '../User/UserGetter.mjs' import Settings from '@overleaf/settings' import LimitationsManager from '../Subscription/LimitationsManager.mjs' import SplitTestHandler from '../SplitTests/SplitTestHandler.mjs' +import { z, zz, parseReq } from '../../infrastructure/Validation.mjs' const { getSafeAdminDomainRedirect } = UrlHelper const { canRedirectToAdminDomain } = AdminAuthorizationHelper @@ -91,8 +92,18 @@ async function _isOverleafStaff(userId) { ) } +const tokenAccessPageSchema = z.object({ + // the router constrains :token to READ_ONLY_TOKEN_PATTERN / + // READ_AND_WRITE_TOKEN_PATTERN already, so a bare string is sufficient + // here -- Express never invokes this handler for a non-matching token. + params: z.strictObject({ + token: z.string(), + }), +}) + async function tokenAccessPage(req, res, next) { - const { token } = req.params + const { params } = parseReq(req, tokenAccessPageSchema, { logOnly: true }) + const { token } = params if (!TokenAccessHandler.isValidToken(token)) { return next(new Errors.NotFoundError()) } @@ -285,9 +296,29 @@ async function checkAndGetProjectOrResponseAction( return [project, null, { projectId, action: 'continue' }] } +const grantTokenAccessSchema = z.object({ + // the router constrains :token to READ_ONLY_TOKEN_PATTERN / + // READ_AND_WRITE_TOKEN_PATTERN already, so a bare string is sufficient + // here -- Express never invokes this handler for a non-matching token. + params: z.strictObject({ + token: z.string(), + }), + body: z.strictObject({ + // sent as a genuine JSON boolean by the frontend (postJSON), not a + // query-string value -- z.stringbool() would reject it. + confirmedByUser: z.boolean().optional(), + // opaque: only ever compared for equality / logged, never used as a + // path or query value (see TokenAccessHandler.checkTokenHashPrefix). + tokenHashPrefix: z.string().optional(), + }), +}) + async function grantTokenAccessReadAndWrite(req, res, next) { - const { token } = req.params - const { confirmedByUser, tokenHashPrefix } = req.body + const { params, body } = parseReq(req, grantTokenAccessSchema, { + logOnly: true, + }) + const { token } = params + const { confirmedByUser, tokenHashPrefix } = body const userId = SessionManager.getLoggedInUserId(req.session) if (!TokenAccessHandler.isReadAndWriteToken(token)) { return res.sendStatus(400) @@ -391,8 +422,11 @@ async function grantTokenAccessReadAndWrite(req, res, next) { } async function grantTokenAccessReadOnly(req, res, next) { - const { token } = req.params - const { confirmedByUser, tokenHashPrefix } = req.body + const { params, body } = parseReq(req, grantTokenAccessSchema, { + logOnly: true, + }) + const { token } = params + const { confirmedByUser, tokenHashPrefix } = body const userId = SessionManager.getLoggedInUserId(req.session) if (!TokenAccessHandler.isReadOnlyToken(token)) { return res.sendStatus(400) @@ -469,8 +503,22 @@ async function grantTokenAccessReadOnly(req, res, next) { } } +// non-strict: this is middleware, chained before sharingUpdatesConsent / +// moveReadWriteToCollaborators / moveReadWriteToReadOnly, which apply their +// own strict schemas to the same params. +const ensureUserCanUseSharingUpdatesConsentPageSchema = z.object({ + params: z.object({ + Project_id: zz.objectId(), + }), +}) + async function ensureUserCanUseSharingUpdatesConsentPage(req, res, next) { - const { Project_id: projectId } = req.params + const { params } = parseReq( + req, + ensureUserCanUseSharingUpdatesConsentPageSchema, + { logOnly: true } + ) + const { Project_id: projectId } = params const userId = SessionManager.getLoggedInUserId(req.session) const project = await ProjectGetter.promises.getProject(projectId, { owner_ref: 1, @@ -499,8 +547,17 @@ async function ensureUserCanUseSharingUpdatesConsentPage(req, res, next) { next() } +const sharingUpdatesConsentSchema = z.object({ + params: z.strictObject({ + Project_id: zz.objectId(), + }), +}) + async function sharingUpdatesConsent(req, res, next) { - const { Project_id: projectId } = req.params + const { params } = parseReq(req, sharingUpdatesConsentSchema, { + logOnly: true, + }) + const { Project_id: projectId } = params AnalyticsManager.recordEventForSession(req.session, 'notification-prompt', { page: req.path, name: 'link-sharing-collaborator', @@ -510,8 +567,17 @@ async function sharingUpdatesConsent(req, res, next) { }) } +const moveReadWriteToCollaboratorsSchema = z.object({ + params: z.strictObject({ + Project_id: zz.objectId(), + }), +}) + async function moveReadWriteToCollaborators(req, res, next) { - const { Project_id: projectId } = req.params + const { params } = parseReq(req, moveReadWriteToCollaboratorsSchema, { + logOnly: true, + }) + const { Project_id: projectId } = params const userId = SessionManager.getLoggedInUserId(req.session) const project = await ProjectGetter.promises.getProject(projectId, { owner_ref: 1, @@ -579,8 +645,17 @@ async function moveReadWriteToCollaborators(req, res, next) { res.sendStatus(204) } +const moveReadWriteToReadOnlySchema = z.object({ + params: z.strictObject({ + Project_id: zz.objectId(), + }), +}) + async function moveReadWriteToReadOnly(req, res, next) { - const { Project_id: projectId } = req.params + const { params } = parseReq(req, moveReadWriteToReadOnlySchema, { + logOnly: true, + }) + const { Project_id: projectId } = params const userId = SessionManager.getLoggedInUserId(req.session) await ProjectAuditLogHandler.promises.addEntry( projectId, diff --git a/services/web/app/src/Features/TokenAccess/TokenAccessHandler.mjs b/services/web/app/src/Features/TokenAccess/TokenAccessHandler.mjs index c76d572e5d..2614ea374b 100644 --- a/services/web/app/src/Features/TokenAccess/TokenAccessHandler.mjs +++ b/services/web/app/src/Features/TokenAccess/TokenAccessHandler.mjs @@ -13,8 +13,8 @@ import Features from '../../infrastructure/Features.mjs' const { ObjectId } = mongodb -const READ_AND_WRITE_TOKEN_PATTERN = '([0-9]+[a-z]{6,12})' -const READ_ONLY_TOKEN_PATTERN = '([a-z]{12})' +const READ_AND_WRITE_TOKEN_PATTERN = '[0-9]+[a-z]{6,12}' +const READ_ONLY_TOKEN_PATTERN = '[a-z]{12}' const TokenAccessHandler = { TOKEN_TYPES: { diff --git a/services/web/app/src/router.mjs b/services/web/app/src/router.mjs index 263076b570..84cddfd079 100644 --- a/services/web/app/src/router.mjs +++ b/services/web/app/src/router.mjs @@ -643,7 +643,7 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) { ) webRouter.get( - '/download/project/:Project_id/build/:editorBuildId/output/cached/:filename(.*)', + '/download/project/:Project_id/build/:editorBuildId/output/cached/:filename', AuthorizationMiddleware.ensureUserCanReadProject, ClsiCacheController.downloadFromCache ) @@ -677,7 +677,7 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) { // direct url access to output files for a specific build webRouter.get( - '/project/:Project_id/build/:build_id/output/:file(.*)', + '/project/:Project_id/build/:build_id/output/:file(.+)', rateLimiterMiddlewareOutputFiles, AuthorizationMiddleware.ensureUserCanReadProject, CompileController.getFileFromClsi @@ -685,7 +685,7 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) { // direct url access to output files for a specific user and build webRouter.get( - '/project/:Project_id/user/:user_id/build/:build_id/output/:file(.*)', + '/project/:Project_id/user/:user_id/build/:build_id/output/:file(.+)', rateLimiterMiddlewareOutputFiles, AuthorizationMiddleware.ensureUserCanReadProject, CompileController.getFileFromClsi @@ -791,7 +791,7 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) { ProjectDownloadsController.exportProjectConversion ) webRouter.get( - '/project/:Project_id/download/conversion/:conversionId/:type/build/:buildId/output/:file(.*)', + '/project/:Project_id/download/conversion/:conversionId/:type/build/:buildId/output/:file(.+)', AuthenticationController.requireLogin(), RateLimiterMiddleware.rateLimit(rateLimiters.documentExportDownload, { params: ['Project_id'], @@ -1003,33 +1003,33 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) { TpdsController.updateFolder ) privateApiRouter.post( - '/user/:user_id/update/*', + '/user/:user_id/update/:path(.+)', AuthenticationController.requirePrivateApiAuth(), TpdsController.mergeUpdate ) privateApiRouter.delete( - '/user/:user_id/update/*', + '/user/:user_id/update/:path(.+)', AuthenticationController.requirePrivateApiAuth(), TpdsController.deleteUpdate ) privateApiRouter.post( - '/project/:project_id/user/:user_id/update/*', + '/project/:project_id/user/:user_id/update/:path(.+)', AuthenticationController.requirePrivateApiAuth(), TpdsController.mergeUpdate ) privateApiRouter.delete( - '/project/:project_id/user/:user_id/update/*', + '/project/:project_id/user/:user_id/update/:path(.+)', AuthenticationController.requirePrivateApiAuth(), TpdsController.deleteUpdate ) privateApiRouter.post( - '/project/:project_id/contents/*', + '/project/:project_id/contents/:path(.+)', AuthenticationController.requirePrivateApiAuth(), TpdsController.updateProjectContents ) privateApiRouter.delete( - '/project/:project_id/contents/*', + '/project/:project_id/contents/:path(.+)', AuthenticationController.requirePrivateApiAuth(), TpdsController.deleteProjectContents ) diff --git a/services/web/modules/user-activate/app/src/UserActivateController.mjs b/services/web/modules/user-activate/app/src/UserActivateController.mjs index 6bee167e03..616450d5ed 100644 --- a/services/web/modules/user-activate/app/src/UserActivateController.mjs +++ b/services/web/modules/user-activate/app/src/UserActivateController.mjs @@ -4,15 +4,23 @@ import UserGetter from '../../../../app/src/Features/User/UserGetter.mjs' import UserRegistrationHandler from '../../../../app/src/Features/User/UserRegistrationHandler.mjs' import ErrorController from '../../../../app/src/Features/Errors/ErrorController.mjs' import { expressify } from '@overleaf/promise-utils' +import { z, parseReq } from '../../../../app/src/infrastructure/Validation.mjs' const __dirname = Path.dirname(fileURLToPath(import.meta.url)) +const registerSchema = z.object({ + body: z.strictObject({ + email: z.string().optional(), + }), +}) + function registerNewUser(req, res, next) { res.render(Path.resolve(__dirname, '../views/user/register')) } async function register(req, res, next) { - const { email } = req.body + const { body } = parseReq(req, registerSchema, { logOnly: true }) + const { email } = body if (email == null || email === '') { return res.sendStatus(422) // Unprocessable Entity } @@ -26,18 +34,43 @@ async function register(req, res, next) { }) } +const activateAccountPageSchema = z.object({ + // Non-strict: this link is emailed out and may get sent back through an + // email gateway/link-rewriter that appends its own tracking query params + // (e.g. utm_* or a click-tracking id) -- rejecting the whole request over + // an unrecognized key would turn a legitimate activation click into a 400 + // for the user, so extra params are tolerated rather than named/rejected. + query: z.object({ + // A legitimate activation link only ever carries a bare user_id string, + // but a tampered URL can turn it into a nested object via qs bracket + // notation (`?user_id[x]=y`), which the handler below explicitly + // detects (the `typeof` check) and turns into its own rendered 403 + // page -- not the generic JSON 400 that handleValidationError would + // produce for a value rejected by the schema itself. So this stays a + // loose union rather than zz.objectId(), leaving the type check (and + // its response) to the handler. + user_id: z + .union([z.string(), z.record(z.string(), z.unknown())]) + .optional(), + token: z.string().optional(), + }), +}) + async function activateAccountPage(req, res, next) { + const { query } = parseReq(req, activateAccountPageSchema, { + logOnly: true, + }) // An 'activation' is actually just a password reset on an account that // was set with a random password originally. - if (req.query.user_id == null || req.query.token == null) { + if (query.user_id == null || query.token == null) { return ErrorController.notFound(req, res) } - if (typeof req.query.user_id !== 'string') { + if (typeof query.user_id !== 'string') { return ErrorController.forbidden(req, res) } - const user = await UserGetter.promises.getUser(req.query.user_id, { + const user = await UserGetter.promises.getUser(query.user_id, { email: 1, loginCount: 1, }) @@ -58,7 +91,7 @@ async function activateAccountPage(req, res, next) { res.render(Path.resolve(__dirname, '../views/user/activate'), { title: 'activate_account', email: user.email, - token: req.query.token, + token: query.token, }) } diff --git a/services/web/modules/user-activate/test/unit/src/UserActivateController.test.mjs b/services/web/modules/user-activate/test/unit/src/UserActivateController.test.mjs index 54dfca5b46..47468f4aba 100644 --- a/services/web/modules/user-activate/test/unit/src/UserActivateController.test.mjs +++ b/services/web/modules/user-activate/test/unit/src/UserActivateController.test.mjs @@ -162,4 +162,38 @@ describe('UserActivateController', function () { .should.equal(true) }) }) + + describe('register with a missing email', function () { + beforeEach(function (ctx) { + ctx.req.body = {} + ctx.res.sendStatus = sinon.stub() + ctx.UserRegistrationHandler.promises.registerNewUserAndSendActivationEmail = + sinon.stub().resolves({ user: ctx.user, setNewPasswordUrl: 'mock/url' }) + }) + + it('should return 422', async function (ctx) { + await ctx.UserActivateController.register(ctx.req, ctx.res) + ctx.res.sendStatus.calledWith(422).should.equal(true) + }) + + it('should not register a user', async function (ctx) { + await ctx.UserActivateController.register(ctx.req, ctx.res) + sinon.assert.notCalled( + ctx.UserRegistrationHandler.promises + .registerNewUserAndSendActivationEmail + ) + }) + }) + + describe('register with an empty email', function () { + beforeEach(function (ctx) { + ctx.req.body = { email: '' } + ctx.res.sendStatus = sinon.stub() + }) + + it('should return 422', async function (ctx) { + await ctx.UserActivateController.register(ctx.req, ctx.res) + ctx.res.sendStatus.calledWith(422).should.equal(true) + }) + }) }) diff --git a/services/web/test/acceptance/config/settings.test.defaults.js b/services/web/test/acceptance/config/settings.test.defaults.js index 8f32bb9b08..33ed4f88fc 100644 --- a/services/web/test/acceptance/config/settings.test.defaults.js +++ b/services/web/test/acceptance/config/settings.test.defaults.js @@ -71,6 +71,9 @@ module.exports = { url: 'http://127.0.0.1:23013', downloadHost: 'http://127.0.0.1:23080', }, + clsiCache: { + instances: [{ url: 'http://127.0.0.1:23081', shard: 'cache' }], + }, realTime: { url: 'http://127.0.0.1:23026', }, diff --git a/services/web/test/acceptance/src/AdminControllerTests.mjs b/services/web/test/acceptance/src/AdminControllerTests.mjs new file mode 100644 index 0000000000..dc6a901585 --- /dev/null +++ b/services/web/test/acceptance/src/AdminControllerTests.mjs @@ -0,0 +1,180 @@ +import { expect } from 'chai' +import Settings from '@overleaf/settings' +import UserHelper from './helpers/User.mjs' +import Features from '../../../app/src/infrastructure/Features.mjs' +import { expectValidationErrorRaw } from '@overleaf/validation-tools/testUtils.js' + +const User = UserHelper.promises + +describe('AdminController', function () { + let admin + + beforeEach(async function () { + admin = new User() + await admin.login() + await admin.ensureAdmin() + await admin.login() + }) + + describe('GET /admin', function () { + it('should render the admin page', async function () { + const { response, body } = await admin.doRequest('get', { + url: '/admin', + }) + expect(response.statusCode).to.equal(200) + expect(body).to.include('System Admin') + }) + }) + + describe('POST /admin/closeEditor', function () { + before(function () { + // only mounted in Server CE/Pro; see router.mjs + if (Features.hasFeature('saas')) { + this.skip() + } + }) + + // closeEditor flips the site-wide Settings.editorIsOpen flag, which + // gates every non-/admin route (see checkIfEditorClosed in Server.mjs); + // reset it directly (not via an HTTP round-trip, which could itself + // fail) so a closed site doesn't leak into whatever runs next. + afterEach(function () { + Settings.editorIsOpen = true + }) + + it('should reject an unexpected field with 400', async function () { + const { response, body } = await admin.doRequest('post', { + url: '/admin/closeEditor', + json: { notAField: true }, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'notAField' + ) + }) + + it('should close the editor and redirect', async function () { + const { response } = await admin.doRequest('post', { + url: '/admin/closeEditor', + json: {}, + }) + expect(response.statusCode).to.equal(302) + expect(response.headers.location).to.equal('/admin#open-close-editor') + }) + + it('should block a non-admin route while closed, then restore access via openEditor', async function () { + await admin.doRequest('post', { url: '/admin/closeEditor', json: {} }) + + const closed = await admin.doRequest('get', { + url: '/login', + json: true, + }) + expect(closed.response.statusCode).to.equal(503) + expect(closed.body.message).to.include('maintenance') + + const { response } = await admin.doRequest('post', { + url: '/admin/openEditor', + json: {}, + }) + expect(response.statusCode).to.equal(302) + expect(response.headers.location).to.equal('/admin#open-close-editor') + + const reopened = await admin.doRequest('get', { + url: '/login', + json: true, + }) + expect(reopened.response.statusCode).to.equal(200) + }) + }) + + describe('POST /admin/disconnectAllUsers', function () { + before(function () { + // only mounted in Server CE/Pro; see router.mjs + if (Features.hasFeature('saas')) { + this.skip() + } + }) + + it('should reject a non-numeric delay with 400', async function () { + const { response, body } = await admin.doRequest('post', { + url: '/admin/disconnectAllUsers?delay=not-a-number', + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'delay' + ) + }) + + it('should disconnect users and redirect', async function () { + const { response } = await admin.doRequest('post', { + url: '/admin/disconnectAllUsers?delay=1', + }) + expect(response.statusCode).to.equal(302) + expect(response.headers.location).to.equal('/admin#open-close-editor') + }) + }) + + describe('POST /admin/flushProjectToTpds', function () { + it('should reject a malformed project_id with 400', async function () { + const { response, body } = await admin.doRequest('post', { + url: '/admin/flushProjectToTpds', + json: { project_id: 'not-an-object-id' }, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'project_id' + ) + }) + }) + + describe('POST /admin/pollDropboxForUser', function () { + it('should reject a malformed user_id with 400', async function () { + const { response, body } = await admin.doRequest('post', { + url: '/admin/pollDropboxForUser', + json: { user_id: 'not-an-object-id' }, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'user_id' + ) + }) + }) + + describe('POST /admin/messages', function () { + it('should reject a missing content field with 400', async function () { + const { response, body } = await admin.doRequest('post', { + url: '/admin/messages', + json: {}, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'content' + ) + }) + + it('should create the message and redirect', async function () { + const { response } = await admin.doRequest('post', { + url: '/admin/messages', + json: { content: 'a system message' }, + }) + expect(response.statusCode).to.equal(302) + expect(response.headers.location).to.equal('/admin#system-messages') + }) + }) + + describe('POST /admin/messages/clear', function () { + it('should clear messages and redirect', async function () { + const { response } = await admin.doRequest('post', { + url: '/admin/messages/clear', + json: {}, + }) + expect(response.statusCode).to.equal(302) + expect(response.headers.location).to.equal('/admin#system-messages') + }) + }) +}) diff --git a/services/web/test/acceptance/src/AnalyticsControllerTests.mjs b/services/web/test/acceptance/src/AnalyticsControllerTests.mjs new file mode 100644 index 0000000000..01a014e133 --- /dev/null +++ b/services/web/test/acceptance/src/AnalyticsControllerTests.mjs @@ -0,0 +1,124 @@ +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 { expectValidationErrorRaw } from '@overleaf/validation-tools/testUtils.js' + +const User = UserHelper.promises +const authedRequest = request.promises + +const auth = Buffer.from('overleaf:password').toString('base64') +function privateApiRequest(options) { + return authedRequest.request({ + ...options, + json: options.json === undefined ? true : options.json, + headers: { + ...options.headers, + Authorization: `Basic ${auth}`, + }, + }) +} + +describe('AnalyticsController', function () { + let owner + + beforeEach(async function () { + owner = new User() + await owner.login() + }) + + describe('POST /event/:event', function () { + it('should accept a well-formed event', async function () { + const { response } = await owner.doRequest('post', { + url: '/event/i-did-something', + json: { editorType: 'abc' }, + }) + expect(response.statusCode).to.equal(202) + }) + + it('should reject a segmentation key with disallowed characters', async function () { + if (!Features.hasFeature('analytics')) { + this.skip() + } + const { response, body } = await owner.doRequest('post', { + url: '/event/i-did-something', + json: { 'bad key!': 'x' }, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'bad key!' + ) + }) + + it('should reject an event name with disallowed characters', async function () { + // AnalyticsRouter's `:event([a-z0-9-_]+)` route pattern rejects this + // before the request ever reaches the controller/schema, so this holds + // regardless of the analytics feature flag. + const { response } = await owner.doRequest('post', { + url: '/event/Bad!Event', + json: { editorType: 'abc' }, + }) + expect(response.statusCode).to.equal(404) + }) + + it('should not match the route for an event name containing a dot', async function () { + const { response } = await owner.doRequest('post', { + url: '/event/foo.bar', + json: { editorType: 'abc' }, + }) + expect(response.statusCode).to.equal(404) + }) + }) + + describe('PUT /editingSession/:projectId', function () { + it('should reject a malformed project id with 404', async function () { + if (!Features.hasFeature('analytics')) { + this.skip() + } + const { response, body } = await owner.doRequest('put', { + url: '/editingSession/not-an-object-id', + json: { segmentation: { editorType: 'abc' } }, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 404, + 'projectId' + ) + }) + }) + + describe('POST /analytics/register-v-1-salesforce-mapping', function () { + it('should accept a well-formed mapping', async function () { + if (!Features.hasFeature('analytics')) { + this.skip() + } + const response = await privateApiRequest({ + method: 'post', + url: '/analytics/register-v-1-salesforce-mapping', + json: { + createdAt: new Date().toISOString(), + salesforceId: '000000000000A0aaaA', + v1Id: 42, + }, + }) + expect(response.statusCode).to.equal(202) + }) + + it('should reject a non-numeric v1Id', async function () { + if (!Features.hasFeature('analytics')) { + this.skip() + } + const response = await privateApiRequest({ + method: 'post', + url: '/analytics/register-v-1-salesforce-mapping', + json: { + createdAt: new Date().toISOString(), + salesforceId: '000000000000A0aaaA', + v1Id: 'not-a-number', + }, + }) + expectValidationErrorRaw(response, 400, 'v1Id') + }) + }) +}) diff --git a/services/web/test/acceptance/src/AuthenticationTests.mjs b/services/web/test/acceptance/src/AuthenticationTests.mjs index 16f8c1a704..a8aa18f73f 100644 --- a/services/web/test/acceptance/src/AuthenticationTests.mjs +++ b/services/web/test/acceptance/src/AuthenticationTests.mjs @@ -50,6 +50,31 @@ describe('Authentication', function () { }) }) + describe('logout', function () { + beforeEach('login', async function () { + await user.login() + }) + + it('should accept a genuine native HTML form submission (form-encoded body, no CSRF header)', async function () { + // The real "Log Out" menu item is a plain HTML form (see + // account-menu-items.tsx): no JS involved, so the token only ever + // travels as a hidden _csrf form field, never as a header. With no + // Accept header favouring JSON, logoutSchema's handler redirects + // the browser directly rather than returning a JSON body. + const { response } = await user.submitNativeForm('/logout') + expect(response.statusCode).to.equal(302) + expect(response.headers.location).to.equal('/login') + }) + + it('should still reject an unrecognized field in the body', async function () { + const { response } = await user.doRequest('POST', { + url: '/logout', + json: { _csrf: user.csrfToken, notARealField: 'nope' }, + }) + expect(response.statusCode).to.equal(400) + }) + }) + describe('login', function () { beforeEach('doLogin', async function () { await user.login() diff --git a/services/web/test/acceptance/src/ClsiCacheControllerTests.mjs b/services/web/test/acceptance/src/ClsiCacheControllerTests.mjs new file mode 100644 index 0000000000..3185023a8a --- /dev/null +++ b/services/web/test/acceptance/src/ClsiCacheControllerTests.mjs @@ -0,0 +1,142 @@ +import { expect } from 'chai' +import UserHelper from './helpers/User.mjs' +import { expectValidationErrorRaw } from '@overleaf/validation-tools/testUtils.js' +import MockClsiCacheClass from './mocks/MockClsiCache.mjs' + +const User = UserHelper.promises + +let MockClsiCache + +before(function () { + MockClsiCache = MockClsiCacheClass.instance() +}) + +describe('ClsiCacheController', function () { + let owner, projectId + + beforeEach(async function () { + owner = new User() + await owner.login() + projectId = await owner.createProject('clsi-cache-controller-test', { + template: 'blank', + }) + }) + + describe('GET /project/:Project_id/output/cached/output.overleaf.json', function () { + it('should reject a malformed project id with 404', async function () { + const { response, body } = await owner.doRequest('get', { + url: '/project/not-an-object-id/output/cached/output.overleaf.json', + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 404, + 'Project_id' + ) + }) + + it('should return 404 when nothing is cached', async function () { + // getLatestBuildFromCache uses res.sendStatus(), unlike + // downloadFromCache's res.status().end() -- sendStatus fills the + // body with the status text, so this is genuinely different from + // (and not to be confused with) an empty-body response. + const { response, body } = await owner.doRequest('get', { + url: `/project/${projectId}/output/cached/output.overleaf.json`, + }) + expect(response.statusCode).to.equal(404) + expect(body).to.equal('Not Found') + }) + + it('should return 410 when the cached build is stale', async function () { + // isUpToDate compares this X-Last-Modified against the project's own + // lastUpdated timestamp (always "now", since the project was just + // created) -- an ancient date always loses that comparison. + MockClsiCache.addEntry( + `/project/${projectId}/user/${owner.id}/latest/output/output.overleaf.json`, + { + content: '{}', + contentType: 'application/json', + lastModified: new Date(0), + } + ) + const { response, body } = await owner.doRequest('get', { + url: `/project/${projectId}/output/cached/output.overleaf.json`, + }) + expect(response.statusCode).to.equal(410) + expect(body).to.equal('Gone') + }) + }) + + describe('GET /download/project/:Project_id/build/:editorBuildId/output/cached/:filename', function () { + it('should reject a malformed project id with 404', async function () { + const { response, body } = await owner.doRequest('get', { + url: '/download/project/not-an-object-id/build/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa-1111111-2222222/output/cached/output.pdf', + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 404, + 'Project_id' + ) + }) + + it('should reject a malformed editorBuildId with 404', async function () { + const { response, body } = await owner.doRequest('get', { + url: `/download/project/${projectId}/build/not-a-valid-build-id/output/cached/output.pdf`, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 404, + 'editorBuildId' + ) + }) + + it('should reject a path traversal filename with 404', async function () { + const { response, body } = await owner.doRequest('get', { + url: `/download/project/${projectId}/build/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa-1111111-2222222/output/cached/..%2F..%2F..%2F..%2Fetc%2Fpasswd`, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 404, + 'filename' + ) + }) + + it('should return 404 when nothing is cached for this build', async function () { + // Asserting the empty body (rather than just the status code) is what + // distinguishes "reached the handler and the real cache genuinely has + // nothing" from "rejected by the schema", which also 404s but with a + // JSON `{ error, statusCode }` body. + const { response, body } = await owner.doRequest('get', { + url: `/download/project/${projectId}/build/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa-1111111-2222222/output/cached/output.pdf`, + }) + expect(response.statusCode).to.equal(404) + expect(body).to.equal('') + }) + + it('should return the cached file when the build is genuinely cached', async function () { + MockClsiCache.addEntry( + `/project/${projectId}/user/${owner.id}/build/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa-1111111-2222222/search/output/output.pdf`, + { content: 'mock-pdf-bytes', contentType: 'application/pdf' } + ) + const { response, body } = await owner.doRequest('get', { + url: `/download/project/${projectId}/build/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa-1111111-2222222/output/cached/output.pdf`, + }) + expect(response.statusCode).to.equal(200) + expect(body).to.equal('mock-pdf-bytes') + }) + + it('should return a nested filename when the build is genuinely cached', async function () { + // some.blg is allowed by isAllowedFilename's `.blg` suffix branch + // rather than its list of literal names -- proves that branch reaches + // the cache lookup and back. + MockClsiCache.addEntry( + `/project/${projectId}/user/${owner.id}/build/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa-1111111-2222222/search/output/some.blg`, + { content: 'mock-blg-bytes', contentType: 'text/plain' } + ) + const { response, body } = await owner.doRequest('get', { + url: `/download/project/${projectId}/build/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa-1111111-2222222/output/cached/some.blg`, + }) + expect(response.statusCode).to.equal(200) + expect(body).to.equal('mock-blg-bytes') + }) + }) +}) diff --git a/services/web/test/acceptance/src/CompileControllerTests.mjs b/services/web/test/acceptance/src/CompileControllerTests.mjs new file mode 100644 index 0000000000..1295ebcb1f --- /dev/null +++ b/services/web/test/acceptance/src/CompileControllerTests.mjs @@ -0,0 +1,278 @@ +import { expect } from 'chai' +import UserHelper from './helpers/User.mjs' +import request from './helpers/request.js' +import settings from '@overleaf/settings' +import { expectValidationErrorRaw } from '@overleaf/validation-tools/testUtils.js' + +const User = UserHelper.promises +const authedRequest = request.promises + +const auth = Buffer.from('overleaf:password').toString('base64') +function privateApiRequest(options) { + return authedRequest.request({ + ...options, + json: options.json === undefined ? true : options.json, + headers: { + ...options.headers, + Authorization: `Basic ${auth}`, + }, + }) +} + +describe('CompileController', function () { + let owner, projectId + + beforeEach(async function () { + owner = new User() + await owner.login() + projectId = await owner.createProject('compile-controller-test', { + template: 'blank', + }) + }) + + describe('POST /project/:Project_id/compile', function () { + it('should reject a malformed project id with 404', async function () { + const { response, body } = await owner.doRequest('post', { + url: '/project/not-an-object-id/compile', + json: {}, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 404, + 'Project_id' + ) + }) + + it('should reject a non-boolean draft flag with 400', async function () { + const { response, body } = await owner.doRequest('post', { + url: `/project/${projectId}/compile`, + json: { draft: 'not-a-boolean' }, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'draft' + ) + }) + }) + + describe('POST /project/:Project_id/compile/stop', function () { + it('should reject a malformed project id with 404', async function () { + const { response, body } = await owner.doRequest('post', { + url: '/project/not-an-object-id/compile/stop', + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 404, + 'Project_id' + ) + }) + }) + + describe('DELETE /project/:Project_id/output', function () { + it('should reject a malformed project id with 404', async function () { + const { response, body } = await owner.doRequest('delete', { + url: '/project/not-an-object-id/output', + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 404, + 'Project_id' + ) + }) + }) + + describe('GET /project/:Project_id/wordcount', function () { + it('should reject a malformed project id with 404', async function () { + const { response, body } = await owner.doRequest('get', { + url: '/project/not-an-object-id/wordcount', + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 404, + 'Project_id' + ) + }) + }) + + describe('GET /download/project/:Project_id/build/:build_id/output/output.pdf', function () { + it('should reject a malformed build id with 404', async function () { + const { response, body } = await owner.doRequest('get', { + url: `/download/project/${projectId}/build/not-a-build-id/output/output.pdf`, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 404, + 'build_id' + ) + }) + + it('should accept an enable_pdf_caching query param and proxy to CLSI', async function () { + const { response, body } = await owner.doRequest('get', { + url: `/download/project/${projectId}/build/18fbe9e7564-30dcb2f71250c690/output/output.pdf?enable_pdf_caching=true`, + }) + expect(response.statusCode).to.equal(200) + expect(body).to.equal('hello') + }) + }) + + describe('GET /project/:Project_id/build/:build_id/output/output.zip', function () { + it('should reject a malformed build id with 404', async function () { + const { response, body } = await owner.doRequest('get', { + url: `/project/${projectId}/build/not-a-build-id/output/output.zip`, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 404, + 'build_id' + ) + }) + + it('should accept an enable_pdf_caching query param and proxy to CLSI', async function () { + const { response, body } = await owner.doRequest('get', { + url: `/project/${projectId}/build/18fbe9e7564-30dcb2f71250c690/output/output.zip?enable_pdf_caching=true`, + }) + expect(response.statusCode).to.equal(200) + expect(body).to.equal('mock-zip: 18fbe9e7564-30dcb2f71250c690') + }) + }) + + describe('GET /project/:Project_id/build/:build_id/output/:file', function () { + it('should reject a path traversal filename with 404', async function () { + const { response, body } = await owner.doRequest('get', { + url: `/project/${projectId}/build/18fbe9e7564-30dcb2f71250c690/output/../../../../etc/passwd`, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 404, + 'file' + ) + }) + + it('should accept a genuine filename and proxy to CLSI', async function () { + // This route resolves to a per-user build path (see + // CompileController._getUserIdForCompile), which MockClsiNginxApi + // (this test environment's stand-in CLSI) handles with an + // unconditional 200 ("hello") -- proving the request reached CLSI, + // rather than being rejected by the schema. + const { response, body } = await owner.doRequest('get', { + url: `/project/${projectId}/build/18fbe9e7564-30dcb2f71250c690/output/nested.txt`, + }) + expect(response.statusCode).to.equal(200) + expect(body).to.equal('hello') + }) + + it('should accept an enable_pdf_caching query param and proxy to CLSI', async function () { + const { response, body } = await owner.doRequest('get', { + url: `/project/${projectId}/build/18fbe9e7564-30dcb2f71250c690/output/nested.txt?enable_pdf_caching=true`, + }) + expect(response.statusCode).to.equal(200) + expect(body).to.equal('hello') + }) + }) + + describe('GET /project/:Project_id/sync/code', function () { + it('should reject a non-numeric line with 400', async function () { + const { response, body } = await owner.doRequest('get', { + url: `/project/${projectId}/sync/code?file=main.tex&line=not-a-number&column=1&editorId=abc123&buildId=abc-123`, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'line' + ) + }) + + it('should reject a buildId that does not match the hex-hyphen-hex shape with 400', async function () { + const { response, body } = await owner.doRequest('get', { + url: `/project/${projectId}/sync/code?file=main.tex&line=1&column=1&editorId=abc123&buildId=not-a-valid-build-id`, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'buildId' + ) + }) + }) + + describe('GET /project/:Project_id/sync/pdf', function () { + it('should reject a non-numeric page with 400', async function () { + const { response, body } = await owner.doRequest('get', { + url: `/project/${projectId}/sync/pdf?page=not-a-number&h=1.0&v=1.0&editorId=abc123&buildId=abc-123`, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'page' + ) + }) + + it('should reject a buildId that does not match the hex-hyphen-hex shape with 400', async function () { + const { response, body } = await owner.doRequest('get', { + url: `/project/${projectId}/sync/pdf?page=1&h=1.0&v=1.0&editorId=abc123&buildId=not-a-valid-build-id`, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'buildId' + ) + }) + }) + + describe('GET /internal/project/:project_id/compile/pdf', function () { + it('should reject a malformed project id with 404', async function () { + const response = await privateApiRequest({ + method: 'get', + url: '/internal/project/not-an-object-id/compile/pdf', + }) + expectValidationErrorRaw(response, 404, 'project_id') + }) + }) + + describe('POST /api/clsi/compile/:submission_id', function () { + beforeEach(function () { + // route is only registered by the publish-modal module (SaaS-only) + if (!settings.moduleImportSequence.includes('publish-modal')) { + this.skip() + } + }) + + it('should reject a malformed submission_id with 404', async function () { + const response = await privateApiRequest({ + method: 'post', + url: '/api/clsi/compile/invalid submission id!', + json: {}, + }) + expectValidationErrorRaw(response, 404, 'submission_id') + }) + }) + + describe('GET /api/clsi/compile/:submissionId/build/:build_id/output/:file', function () { + beforeEach(function () { + // route is only registered by the publish-modal module (SaaS-only) + if (!settings.moduleImportSequence.includes('publish-modal')) { + this.skip() + } + }) + + it('should reject a malformed build_id with 404', async function () { + const response = await privateApiRequest({ + method: 'get', + url: '/api/clsi/compile/sub-1234/build/not-a-build-id/output/output.pdf', + }) + expectValidationErrorRaw(response, 404, 'build_id') + }) + + it('should accept a blank compileGroup and proxy to CLSI', async function () { + // V1's CLSI::Response#v2_api_url always appends compileGroup (taken + // from the compile response); compileSubmission (above) never + // populates that field, so it arrives here blank rather than one of + // zz.compileGroup()'s enum values. + const response = await privateApiRequest({ + method: 'get', + url: '/api/clsi/compile/sub-1234/build/18fbe9e7564-30dcb2f71250c690/output/output.pdf?compileGroup=', + }) + expect(response.statusCode).to.equal(200) + }) + }) +}) diff --git a/services/web/test/acceptance/src/ConfirmEmailTests.mjs b/services/web/test/acceptance/src/ConfirmEmailTests.mjs new file mode 100644 index 0000000000..afd79e0231 --- /dev/null +++ b/services/web/test/acceptance/src/ConfirmEmailTests.mjs @@ -0,0 +1,89 @@ +import { expect } from 'chai' +import UserHelper from './helpers/User.mjs' +import OneTimeTokenHandler from '../../../app/src/Features/Security/OneTimeTokenHandler.mjs' +import { expectValidationErrorRaw } from '@overleaf/validation-tools/testUtils.js' + +const User = UserHelper.promises + +describe('Confirm email via link/token (UserEmailsController.confirm)', function () { + let user + + beforeEach('login', async function () { + user = new User() + await user.login() + }) + + async function getTokenForUser() { + return OneTimeTokenHandler.promises.getNewToken( + 'email_confirmation', + { user_id: user.id, email: user.email }, + { expiresIn: 24 * 60 * 60 } + ) + } + + it('should accept the redundant _csrf field the async-form helper packs into the body', async function () { + // views/user/confirm_email.pug's confirmEmailForm carries + // data-ol-async-form: hydrate-form.ts's async-form helper packs all + // FormData -- including the hidden _csrf input -- into the JSON body + // on top of the X-Csrf-Token header it sets, so confirmEmailSchema + // must tolerate _csrf turning up in the body too. + const token = await getTokenForUser() + const { response } = await user.doRequest('POST', { + url: '/user/emails/confirm', + json: { token, _csrf: user.csrfToken }, + }) + expect(response.statusCode).to.equal(200) + }) + + it('should still reject a non-string token', async function () { + const { response, body } = await user.doRequest('POST', { + url: '/user/emails/confirm', + json: { token: ['not', 'a', 'string'], _csrf: user.csrfToken }, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'token' + ) + }) + + it('should 404 for an invalid token (proving the request reaches the handler, not blocked by the schema)', async function () { + const { response, body } = await user.doRequest('POST', { + url: '/user/emails/confirm', + json: { token: 'not-a-real-token', _csrf: user.csrfToken }, + }) + expect(response.statusCode).to.equal(404) + expect(body.message).to.equal( + 'Sorry, your confirmation token is invalid or has expired. Please request a new email confirmation link.' + ) + }) +}) + +describe('Show confirm email page (UserEmailsController.showConfirm)', function () { + let user + + beforeEach('login', async function () { + user = new User() + await user.login() + }) + + it('should render the page with the token from the query string', async function () { + const { response, body } = await user.doRequest('GET', { + url: '/user/emails/confirm?token=mock-confirm-token', + }) + expect(response.statusCode).to.equal(200) + expect(body).to.include('value="mock-confirm-token"') + }) + + it('should reject an invalid token query value', async function () { + const { response, body } = await user.doRequest('GET', { + url: '/user/emails/confirm?token[foo]=bar', + json: true, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'token' + ) + }) +}) diff --git a/services/web/test/acceptance/src/DocumentControllerTests.mjs b/services/web/test/acceptance/src/DocumentControllerTests.mjs new file mode 100644 index 0000000000..8ab139aae5 --- /dev/null +++ b/services/web/test/acceptance/src/DocumentControllerTests.mjs @@ -0,0 +1,167 @@ +import { expect } from 'chai' +import UserHelper from './helpers/User.mjs' +import request from './helpers/request.js' +import { expectValidationErrorRaw } from '@overleaf/validation-tools/testUtils.js' + +const User = UserHelper.promises +const authedRequest = request.promises + +const auth = Buffer.from('overleaf:password').toString('base64') +function privateApiRequest(options) { + return authedRequest.request({ + ...options, + json: options.json === undefined ? true : options.json, + headers: { + ...options.headers, + Authorization: `Basic ${auth}`, + }, + }) +} + +describe('DocumentController', function () { + let owner, projectId, docId + + beforeEach(async function () { + owner = new User() + await owner.login() + projectId = await owner.createProject('document-controller-test', { + template: 'blank', + }) + const project = await owner.getProject(projectId) + docId = project.rootFolder[0].docs[0]._id.toString() + }) + + describe('GET /project/:Project_id/doc/:doc_id', function () { + it('should return the document as json', async function () { + const response = await privateApiRequest({ + method: 'get', + url: `/project/${projectId}/doc/${docId}`, + }) + expect(response.statusCode).to.equal(200) + expect(response.body.lines).to.be.an('array') + expect(response.body.version).to.be.a('number') + expect(response.body.pathname).to.equal('/main.tex') + }) + + it('should return the document as plain text when plain=true', async function () { + const response = await privateApiRequest({ + method: 'get', + url: `/project/${projectId}/doc/${docId}?plain=true`, + json: false, + }) + expect(response.statusCode).to.equal(200) + expect(response.body).to.be.a('string') + }) + + it('should reject a malformed project id with 404', async function () { + const response = await privateApiRequest({ + method: 'get', + url: `/project/not-an-object-id/doc/${docId}`, + }) + expectValidationErrorRaw(response, 404, 'Project_id') + }) + + it('should reject a malformed doc id with 404', async function () { + const response = await privateApiRequest({ + method: 'get', + url: `/project/${projectId}/doc/not-an-object-id`, + }) + expectValidationErrorRaw(response, 404, 'doc_id') + }) + }) + + describe('POST /project/:Project_id/doc/:doc_id', function () { + it('should update the document lines and version', async function () { + const response = await privateApiRequest({ + method: 'post', + url: `/project/${projectId}/doc/${docId}`, + json: { + lines: ['new content', 'second line'], + version: 2, + ranges: {}, + lastUpdatedAt: String(Date.now()), + lastUpdatedBy: owner._id.toString(), + }, + }) + expect(response.statusCode).to.equal(200) + + const getResponse = await privateApiRequest({ + method: 'get', + url: `/project/${projectId}/doc/${docId}`, + }) + expect(getResponse.body.lines).to.deep.equal([ + 'new content', + 'second line', + ]) + expect(getResponse.body.version).to.equal(2) + }) + + it('should accept a null lastUpdatedBy for system-initiated updates', async function () { + const response = await privateApiRequest({ + method: 'post', + url: `/project/${projectId}/doc/${docId}`, + json: { + lines: ['content'], + version: 2, + ranges: {}, + lastUpdatedAt: String(Date.now()), + lastUpdatedBy: null, + }, + }) + expect(response.statusCode).to.equal(200) + }) + + it('should reject a non-array lines field', async function () { + const response = await privateApiRequest({ + method: 'post', + url: `/project/${projectId}/doc/${docId}`, + json: { + lines: 'not-an-array', + version: 2, + ranges: {}, + }, + }) + expectValidationErrorRaw(response, 400, 'lines') + }) + + it('should reject a malformed lastUpdatedAt', async function () { + const response = await privateApiRequest({ + method: 'post', + url: `/project/${projectId}/doc/${docId}`, + json: { + lines: ['content'], + version: 2, + ranges: {}, + lastUpdatedAt: 'not-a-number', + lastUpdatedBy: owner._id.toString(), + }, + }) + expectValidationErrorRaw(response, 400, 'lastUpdatedAt') + }) + }) + + describe('POST /project/:Project_id/doc/:doc_id/changes/reject', function () { + it('should accept the request and return 204', async function () { + const response = await privateApiRequest({ + method: 'post', + url: `/project/${projectId}/doc/${docId}/changes/reject`, + json: { + rejectedChangeAuthorIds: [owner._id.toString()], + userId: owner._id.toString(), + }, + }) + expect(response.statusCode).to.equal(204) + }) + + it('should reject a non-ObjectId rejectedChangeAuthorIds entry', async function () { + const response = await privateApiRequest({ + method: 'post', + url: `/project/${projectId}/doc/${docId}/changes/reject`, + json: { + rejectedChangeAuthorIds: ['not-an-object-id'], + }, + }) + expectValidationErrorRaw(response, 400, 'rejectedChangeAuthorIds') + }) + }) +}) diff --git a/services/web/test/acceptance/src/DocumentUpdaterControllerTests.mjs b/services/web/test/acceptance/src/DocumentUpdaterControllerTests.mjs new file mode 100644 index 0000000000..76dcec7ccb --- /dev/null +++ b/services/web/test/acceptance/src/DocumentUpdaterControllerTests.mjs @@ -0,0 +1,40 @@ +import UserHelper from './helpers/User.mjs' +import { expectValidationErrorRaw } from '@overleaf/validation-tools/testUtils.js' + +const User = UserHelper.promises + +describe('DocumentUpdaterController', function () { + let owner, projectId + + beforeEach(async function () { + owner = new User() + await owner.login() + projectId = await owner.createProject('document-updater-controller-test', { + template: 'blank', + }) + }) + + describe('GET /Project/:Project_id/doc/:Doc_id/download', function () { + it('should reject a malformed project id with 404', async function () { + const { response, body } = await owner.doRequest('get', { + url: `/Project/not-an-object-id/doc/${projectId}/download`, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 404, + 'Project_id' + ) + }) + + it('should reject a malformed doc id with 404', async function () { + const { response, body } = await owner.doRequest('get', { + url: `/Project/${projectId}/doc/not-an-object-id/download`, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 404, + 'Doc_id' + ) + }) + }) +}) diff --git a/services/web/test/acceptance/src/FileStoreControllerTests.mjs b/services/web/test/acceptance/src/FileStoreControllerTests.mjs new file mode 100644 index 0000000000..f7308515b8 --- /dev/null +++ b/services/web/test/acceptance/src/FileStoreControllerTests.mjs @@ -0,0 +1,51 @@ +import { expect } from 'chai' +import UserHelper from './helpers/User.mjs' +import { expectValidationErrorRaw } from '@overleaf/validation-tools/testUtils.js' + +const User = UserHelper.promises + +describe('FileStoreController', function () { + let owner, projectId + + beforeEach(async function () { + owner = new User() + await owner.login() + projectId = await owner.createProject('filestore-controller-test', { + template: 'blank', + }) + }) + + describe('GET /Project/:Project_id/file/:File_id', function () { + it('should reject a malformed project id with 404', async function () { + const { response, body } = await owner.doRequest('get', { + url: `/Project/not-an-object-id/file/${projectId}`, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 404, + 'Project_id' + ) + }) + + it('should reject a malformed file id with 404', async function () { + const { response, body } = await owner.doRequest('get', { + url: `/Project/${projectId}/file/not-an-object-id`, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 404, + 'File_id' + ) + }) + }) + + describe('HEAD /Project/:Project_id/file/:File_id', function () { + it('should reject a malformed project id with 404', async function () { + const { response } = await owner.doRequest('head', { + url: `/Project/not-an-object-id/file/${projectId}`, + }) + // HEAD responses carry no body to assert a validation message against + expect(response.statusCode).to.equal(404) + }) + }) +}) diff --git a/services/web/test/acceptance/src/InactiveProjectControllerTests.mjs b/services/web/test/acceptance/src/InactiveProjectControllerTests.mjs new file mode 100644 index 0000000000..7b990fc061 --- /dev/null +++ b/services/web/test/acceptance/src/InactiveProjectControllerTests.mjs @@ -0,0 +1,44 @@ +import request from './helpers/request.js' +import { expectValidationErrorRaw } from '@overleaf/validation-tools/testUtils.js' + +const authedRequest = request.promises + +const auth = Buffer.from('overleaf:password').toString('base64') +function privateApiRequest(options) { + return authedRequest.request({ + ...options, + json: options.json === undefined ? true : options.json, + headers: { + ...options.headers, + Authorization: `Basic ${auth}`, + }, + }) +} + +describe('InactiveProjectController', function () { + describe('POST /internal/project/:project_id/deactivate', function () { + // The happy path (real archiving via docstore) isn't exercised here: + // MockDocstoreApi doesn't implement the archive endpoint this hits, so a + // full round trip 500s regardless of request validation. That behavior + // (mocked-manager only) is covered at the unit level instead; this file + // covers the request-validation wiring. + it('should reject a malformed project id with 404', async function () { + const response = await privateApiRequest({ + method: 'post', + url: `/internal/project/not-an-object-id/deactivate`, + }) + expectValidationErrorRaw(response, 404, 'project_id') + }) + }) + + describe('POST /internal/deactivateOldProjects', function () { + it('should reject a non-numeric numberOfProjectsToArchive', async function () { + const response = await privateApiRequest({ + method: 'post', + url: '/internal/deactivateOldProjects', + json: { numberOfProjectsToArchive: 'not-a-number' }, + }) + expectValidationErrorRaw(response, 400, 'numberOfProjectsToArchive') + }) + }) +}) diff --git a/services/web/test/acceptance/src/LinkedFilesTests.mjs b/services/web/test/acceptance/src/LinkedFilesTests.mjs index 5afe3b5d6a..14fb5faa87 100644 --- a/services/web/test/acceptance/src/LinkedFilesTests.mjs +++ b/services/web/test/acceptance/src/LinkedFilesTests.mjs @@ -5,21 +5,31 @@ import Settings from '@overleaf/settings' import UserHelper from './helpers/User.mjs' import express from 'express' import { plainTextResponse } from '../../../app/src/infrastructure/Response.mjs' +import { parseReq, z, handleValidationError } from '@overleaf/validation-tools' +import { expectValidationErrorRaw } from '@overleaf/validation-tools/testUtils.js' const User = UserHelper.promises +const linkedUrlProxySchema = z.object({ + query: z.object({ + url: z.string(), + }), +}) + const LinkedUrlProxy = express() LinkedUrlProxy.get('/', (req, res, next) => { - if (req.query.url === 'http://example.com/foo') { + const { query } = parseReq(req, linkedUrlProxySchema) + if (query.url === 'http://example.com/foo') { return plainTextResponse(res, 'foo foo foo') - } else if (req.query.url === 'http://example.com/bar') { + } else if (query.url === 'http://example.com/bar') { return plainTextResponse(res, 'bar bar bar') - } else if (req.query.url === 'http://example.com/large') { + } else if (query.url === 'http://example.com/large') { return plainTextResponse(res, 'x'.repeat(Settings.maxUploadSize + 1)) } else { return res.sendStatus(404) } }) +LinkedUrlProxy.use(handleValidationError) describe('LinkedFiles', function () { before(function () { @@ -207,6 +217,38 @@ describe('LinkedFiles', function () { expect(response.statusCode).to.equal(404) expect(body).to.equal('Source file not found') }) + + it('should reject a malformed file id when refreshing a linked file', async function () { + const { response, body } = await owner.doRequest('post', { + url: `/project/${projectOneId}/linked_file/not-an-object-id/refresh`, + json: true, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 404, + 'file_id' + ) + }) + + it('should reject a malformed source_project_id when creating a project_file linked file', async function () { + const { response, body } = await owner.doRequest('post', { + url: `/project/${projectOneId}/linked_file`, + json: { + name: 'test-link.txt', + parent_folder_id: projectOneRootFolderId, + provider: 'project_file', + data: { + source_project_id: 'not-an-object-id', + source_entity_path: `/${sourceDocName}`, + }, + }, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'source_project_id' + ) + }) }) describe('with a linked project_file from a v1 project that has not been imported', function () { @@ -423,6 +465,26 @@ describe('LinkedFiles', function () { expect(response.statusCode).to.equal(200) expect(body).to.equal('foo foo foo') }) + + it('should reject an unrecognized field in the linked file data', async function () { + const { response, body } = await owner.doRequest('post', { + url: `/project/${projectOneId}/linked_file`, + json: { + provider: 'url', + data: { + url: 'http://example.com/foo', + notARealField: 'nope', + }, + parent_folder_id: projectOneRootFolderId, + name: 'url-test-file-invalid', + }, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'notARealField' + ) + }) }) // TODO: Add test for asking for host that return ENOTFOUND @@ -489,6 +551,48 @@ describe('LinkedFiles', function () { expect(refreshedFile._id.toString()).to.equal(refreshedFileId.toString()) expect(refreshedFile.name).to.equal('test.pdf') }) + + it('should reject a malformed source_project_id when creating a project_output_file linked file', async function () { + const { response, body } = await owner.doRequest('post', { + url: `/project/${projectOneId}/linked_file`, + json: { + name: 'test.pdf', + parent_folder_id: projectOneRootFolderId, + provider: 'project_output_file', + data: { + source_project_id: 'not-an-object-id', + source_output_file_path: 'output.pdf', + build_id: '1234-abcd', + }, + }, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'source_project_id' + ) + }) + + it('should reject a malformed build_id when creating a project_output_file linked file', async function () { + const { response, body } = await owner.doRequest('post', { + url: `/project/${projectOneId}/linked_file`, + json: { + name: 'test.pdf', + parent_folder_id: projectOneRootFolderId, + provider: 'project_output_file', + data: { + source_project_id: projectTwoId, + source_output_file_path: 'output.pdf', + build_id: 'not-a-valid-build-id', + }, + }, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'build_id' + ) + }) }) describe('with a linked project_output_file from a v1 project that has not been imported', function () { diff --git a/services/web/test/acceptance/src/TemplatesControllerTests.mjs b/services/web/test/acceptance/src/TemplatesControllerTests.mjs new file mode 100644 index 0000000000..2e06991147 --- /dev/null +++ b/services/web/test/acceptance/src/TemplatesControllerTests.mjs @@ -0,0 +1,92 @@ +import UserHelper from './helpers/User.mjs' +import { expectValidationErrorRaw } from '@overleaf/validation-tools/testUtils.js' + +const User = UserHelper.promises + +describe('TemplatesController', function () { + let owner + + beforeEach(async function () { + owner = new User() + await owner.login() + }) + + describe('GET /project/new/template/:Template_version_id', function () { + it('should reject a non-numeric Template_version_id with 404', async function () { + const { response, body } = await owner.doRequest('get', { + url: '/project/new/template/not-a-number?id=123', + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 404, + 'Template_version_id' + ) + }) + + it('should reject a non-numeric id query param with 400', async function () { + const { response, body } = await owner.doRequest('get', { + url: '/project/new/template/456?id=not-a-number', + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'id' + ) + }) + + it('should reject a non-string templateName query param with 400', async function () { + const { response, body } = await owner.doRequest('get', { + url: '/project/new/template/456?id=123&templateName=a&templateName=b', + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'templateName' + ) + }) + + it('should reject a path-traversal-shaped brandVariationId query param with 400', async function () { + const { response, body } = await owner.doRequest('get', { + url: '/project/new/template/456?id=123&brandVariationId=1%2F..%2F..%2Fv1%2Fx', + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'brandVariationId' + ) + }) + }) + + describe('POST /project/new/template', function () { + it('should reject a non-numeric templateId with 400', async function () { + const { response, body } = await owner.doRequest('post', { + url: '/project/new/template', + json: { + templateId: 'not-a-number', + templateVersionId: '456', + }, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'templateId' + ) + }) + + it('should reject a path-traversal-shaped brandVariationId with 400', async function () { + const { response, body } = await owner.doRequest('post', { + url: '/project/new/template', + json: { + templateId: '123', + templateVersionId: '456', + brandVariationId: '1/../../v1/x', + }, + }) + expectValidationErrorRaw( + { statusCode: response.statusCode, body }, + 400, + 'brandVariationId' + ) + }) + }) +}) diff --git a/services/web/test/acceptance/src/TokenAccessTests.mjs b/services/web/test/acceptance/src/TokenAccessTests.mjs index eb66c9ef2e..7d27f101bc 100644 --- a/services/web/test/acceptance/src/TokenAccessTests.mjs +++ b/services/web/test/acceptance/src/TokenAccessTests.mjs @@ -7,6 +7,7 @@ import { db } from '../../../app/src/infrastructure/mongodb.mjs' import expectErrorResponse from './helpers/expectErrorResponse.mjs' import logger from '@overleaf/logger' import sinon from 'sinon' +import TokenAccessHandler from '../../../app/src/Features/TokenAccess/TokenAccessHandler.mjs' const tryEditorAccess = (user, projectId, test, callback) => async.series( @@ -85,8 +86,11 @@ const _doTryTokenAccess = ( return callback() } user.request.post( + // `token` here is a route param, not a body field -- the grant + // endpoints only read confirmedByUser/tokenHashPrefix from the body + // (see TokenAccessController.mjs's grantTokenAccessSchema). `${url}/grant`, - { json: { token } }, + { json: {} }, (err, response, body) => { if (err) { return callback(err) @@ -132,8 +136,10 @@ const _doTryTokenAccept = ( return callback() } user.request.post( + // `token` here is a route param, not a body field -- see the same + // note in _doTryTokenAccess above. `${url}/grant`, - { json: { token, confirmedByUser: true } }, + { json: { confirmedByUser: true } }, (err, response, body) => { if (err) { return callback(err) @@ -1057,7 +1063,6 @@ describe('TokenAccess', function () { `${this.tokens.readAndWrite}/grant`, { json: { - token: this.tokens.readAndWrite, tokenHashPrefix: urlFragment, }, }, @@ -1099,6 +1104,98 @@ describe('TokenAccess', function () { }) }) + describe('anonymous read-and-write token, enabled', function () { + let previousAnonymousReadAndWriteEnabled + beforeEach(function () { + previousAnonymousReadAndWriteEnabled = + TokenAccessHandler.ANONYMOUS_READ_AND_WRITE_ENABLED + // ANONYMOUS_READ_AND_WRITE_ENABLED is spread onto the default export + // (a copy) as well as kept on `promises` (the original object) -- + // both need to be flipped for the flag to take effect everywhere. + TokenAccessHandler.ANONYMOUS_READ_AND_WRITE_ENABLED = true + TokenAccessHandler.promises.ANONYMOUS_READ_AND_WRITE_ENABLED = true + }) + afterEach(function () { + TokenAccessHandler.ANONYMOUS_READ_AND_WRITE_ENABLED = + previousAnonymousReadAndWriteEnabled + TokenAccessHandler.promises.ANONYMOUS_READ_AND_WRITE_ENABLED = + previousAnonymousReadAndWriteEnabled + }) + + beforeEach(function (done) { + this.owner.createProject( + `token-anon-rw-enabled-test${Math.random()}`, + (err, projectId) => { + if (err != null) { + return done(err) + } + this.projectId = projectId + this.owner.makeTokenBased(this.projectId, err => { + if (err != null) { + return done(err) + } + this.owner.getProject(this.projectId, (err, project) => { + if (err != null) { + return done(err) + } + this.tokens = project.tokens + done() + }) + }) + } + ) + }) + + it('should grant the anonymous user read-and-write access and let them load the editor', function (done) { + async.series( + [ + cb => + tryEditorAccess( + this.anon, + this.projectId, + expectErrorResponse.restricted.html, + cb + ), + cb => + tryReadAndWriteTokenAccess( + this.anon, + this.tokens.readAndWrite, + (response, body) => { + expect(response.statusCode).to.equal(200) + }, + (response, body) => { + expect(response.statusCode).to.equal(200) + expect(body.redirect).to.equal(`/project/${this.projectId}`) + expect(body.grantAnonymousAccess).to.equal('readAndWrite') + }, + cb + ), + cb => + tryEditorAccess( + this.anon, + this.projectId, + (response, body) => { + expect(response.statusCode).to.equal(200) + }, + cb + ), + cb => + tryAnonContentAccess( + this.anon, + this.projectId, + this.tokens.readAndWrite, + (response, body) => { + expect(body.privilegeLevel).to.equal('readAndWrite') + expect(body.isRestrictedUser).to.equal(false) + }, + cb + ), + ], + done + ) + }) + }) + describe('private overleaf project', function () { beforeEach(function (done) { this.owner.createProject('overleaf-import', (err, projectId) => { diff --git a/services/web/test/acceptance/src/TpdsControllerTests.mjs b/services/web/test/acceptance/src/TpdsControllerTests.mjs new file mode 100644 index 0000000000..a51d151090 --- /dev/null +++ b/services/web/test/acceptance/src/TpdsControllerTests.mjs @@ -0,0 +1,79 @@ +import request from './helpers/request.js' +import { expectValidationErrorRaw } from '@overleaf/validation-tools/testUtils.js' + +const authedRequest = request.promises + +const auth = Buffer.from('overleaf:password').toString('base64') +function privateApiRequest(options) { + return authedRequest.request({ + ...options, + json: options.json === undefined ? true : options.json, + headers: { + ...options.headers, + Authorization: `Basic ${auth}`, + }, + }) +} + +describe('TpdsController', function () { + describe('POST /user/:user_id/project/new', function () { + it('should reject a malformed user_id with 404', async function () { + const response = await privateApiRequest({ + method: 'post', + url: '/user/not-an-object-id/project/new', + json: { projectName: 'foo' }, + }) + expectValidationErrorRaw(response, 404, 'user_id') + }) + }) + + describe('POST /user/:user_id/project/resolve', function () { + it('should reject a malformed user_id with 404', async function () { + const response = await privateApiRequest({ + method: 'post', + url: '/user/not-an-object-id/project/resolve', + json: { projectName: 'foo' }, + }) + expectValidationErrorRaw(response, 404, 'user_id') + }) + }) + + describe('POST /tpds/folder-update', function () { + it('should reject a malformed userId with 400', async function () { + const response = await privateApiRequest({ + method: 'post', + url: '/tpds/folder-update', + json: { userId: 'not-an-object-id', path: '/a/b' }, + }) + expectValidationErrorRaw(response, 400, 'userId') + }) + }) + + describe('POST /user/:user_id/update/:path(.+)', function () { + it('should reject a malformed user_id with 404', async function () { + const response = await privateApiRequest({ + method: 'post', + url: '/user/not-an-object-id/update/project-name/file.tex', + }) + expectValidationErrorRaw(response, 404, 'user_id') + }) + }) + + describe('POST /project/:project_id/contents/:path(.+)', function () { + it('should reject a malformed project_id with 404', async function () { + const response = await privateApiRequest({ + method: 'post', + url: '/project/not-an-object-id/contents/chapters/main.tex', + }) + expectValidationErrorRaw(response, 404, 'project_id') + }) + + it('should reject a path traversal payload with 404', async function () { + const response = await privateApiRequest({ + method: 'post', + url: '/project/507f191e810c19729de860ea/contents/../../../../etc/passwd', + }) + expectValidationErrorRaw(response, 404, 'params.path') + }) + }) +}) diff --git a/services/web/test/acceptance/src/mocks/MockDocUpdaterApi.mjs b/services/web/test/acceptance/src/mocks/MockDocUpdaterApi.mjs index 2926dd3d21..229439fec8 100644 --- a/services/web/test/acceptance/src/mocks/MockDocUpdaterApi.mjs +++ b/services/web/test/acceptance/src/mocks/MockDocUpdaterApi.mjs @@ -34,11 +34,13 @@ const insertOp = z.strictObject({ i: z.string(), p: z.number().int().min(0), u: z.boolean().optional(), + fixedRemoveChange: z.boolean().optional(), }) const deleteOp = z.strictObject({ d: z.string(), p: z.number().int().min(0), u: z.boolean().optional(), + fixedRemoveChange: z.boolean().optional(), }) const commentOp = z.strictObject({ c: z.string(), diff --git a/services/web/test/acceptance/src/mocks/MockDocstoreApi.mjs b/services/web/test/acceptance/src/mocks/MockDocstoreApi.mjs index 3e85876b8e..461ae260df 100644 --- a/services/web/test/acceptance/src/mocks/MockDocstoreApi.mjs +++ b/services/web/test/acceptance/src/mocks/MockDocstoreApi.mjs @@ -8,11 +8,13 @@ const insertOp = z.strictObject({ i: z.string(), p: z.number().int().min(0), u: z.boolean().optional(), + fixedRemoveChange: z.boolean().optional(), }) const deleteOp = z.strictObject({ d: z.string(), p: z.number().int().min(0), u: z.boolean().optional(), + fixedRemoveChange: z.boolean().optional(), }) const commentOp = z.strictObject({ c: z.string().optional(), diff --git a/services/web/test/unit/src/Analytics/AnalyticsController.test.mjs b/services/web/test/unit/src/Analytics/AnalyticsController.test.mjs index 1f67da7cdb..b6539f369a 100644 --- a/services/web/test/unit/src/Analytics/AnalyticsController.test.mjs +++ b/services/web/test/unit/src/Analytics/AnalyticsController.test.mjs @@ -1,5 +1,6 @@ -import { beforeEach, describe, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import sinon from 'sinon' +import { setReqValidationModeForTests } from '@overleaf/validation-tools' import MockResponse from '../helpers/MockResponse.mjs' const modulePath = new URL( '../../../../app/src/Features/Analytics/AnalyticsController.mjs', @@ -50,11 +51,17 @@ describe('AnalyticsController', function () { ctx.res = new MockResponse(vi) }) + afterEach(function () { + setReqValidationModeForTests(null) + }) + describe('updateEditingSession', function () { beforeEach(function (ctx) { + // projectId is validated as a Mongo ObjectId + ctx.projectId = '507f191e810c19729de860ea' ctx.req = { params: { - projectId: 'a project id', + projectId: ctx.projectId, }, session: {}, body: { @@ -75,7 +82,7 @@ describe('AnalyticsController', function () { sinon.assert.calledWith( ctx.AnalyticsManager.updateEditingSession, '1234', - 'a project id', + ctx.projectId, 'XY', { editorType: 'abc' } ) @@ -90,7 +97,6 @@ describe('AnalyticsController', function () { beforeEach(function (ctx) { const body = { foo: 'stuff', - _csrf: 'atoken123', } ctx.req = { params: { @@ -100,9 +106,6 @@ describe('AnalyticsController', function () { sessionID: 'sessionIDHere', session: {}, } - - ctx.expectedData = Object.assign({}, body) - delete ctx.expectedData._csrf }) it('should use the session', async function (ctx) { @@ -112,23 +115,24 @@ describe('AnalyticsController', function () { ctx.AnalyticsManager.recordEventForSession, ctx.req.session, ctx.req.params.event, - ctx.expectedData + ctx.req.body ) resolve() }) }) - it('should remove the CSRF token before sending to the manager', async function (ctx) { - await new Promise(resolve => { - ctx.controller.recordEvent(ctx.req, ctx.res) - sinon.assert.calledWith( - ctx.AnalyticsManager.recordEventForSession, - ctx.req.session, - ctx.req.params.event, - ctx.expectedData - ) - resolve() - }) + it('should reject an event name containing characters outside [a-z0-9-_]', function (ctx) { + setReqValidationModeForTests('enforce') + ctx.req.params.event = 'I.did:something,here/now;too' + expect(() => ctx.controller.recordEvent(ctx.req, ctx.res)).to.throw() + sinon.assert.notCalled(ctx.AnalyticsManager.recordEventForSession) + }) + + it('should reject an event name containing a disallowed character', function (ctx) { + setReqValidationModeForTests('enforce') + ctx.req.params.event = 'bad event!' + expect(() => ctx.controller.recordEvent(ctx.req, ctx.res)).to.throw() + sinon.assert.notCalled(ctx.AnalyticsManager.recordEventForSession) }) }) }) diff --git a/services/web/test/unit/src/Analytics/AnalyticsRegistrationSourceMiddleware.test.mjs b/services/web/test/unit/src/Analytics/AnalyticsRegistrationSourceMiddleware.test.mjs new file mode 100644 index 0000000000..42dc453f0e --- /dev/null +++ b/services/web/test/unit/src/Analytics/AnalyticsRegistrationSourceMiddleware.test.mjs @@ -0,0 +1,152 @@ +import { afterEach, beforeEach, describe, it, vi } from 'vitest' +import sinon from 'sinon' +import { setReqValidationModeForTests } from '@overleaf/validation-tools' +import MockRequest from '../helpers/MockRequest.mjs' +import MockResponse from '../helpers/MockResponse.mjs' + +const MODULE_PATH = new URL( + '../../../../app/src/Features/Analytics/AnalyticsRegistrationSourceMiddleware', + import.meta.url +).pathname + +describe('AnalyticsRegistrationSourceMiddleware', function () { + beforeEach(async function (ctx) { + ctx.req = new MockRequest(vi) + ctx.req.header = name => ctx.req.headers[name] + ctx.res = new MockResponse(vi) + ctx.next = sinon.stub() + + ctx.AnalyticsRegistrationSourceHelper = { + clearSource: sinon.stub(), + setInbound: sinon.stub(), + } + vi.doMock( + '../../../../app/src/Features/Analytics/AnalyticsRegistrationSourceHelper.mjs', + () => ({ default: ctx.AnalyticsRegistrationSourceHelper }) + ) + + ctx.SessionManager = { + isUserLoggedIn: sinon.stub().returns(false), + } + vi.doMock( + '../../../../app/src/Features/Authentication/SessionManager.mjs', + () => ({ default: ctx.SessionManager }) + ) + + ctx.AnalyticsRegistrationSourceMiddleware = ( + await import(MODULE_PATH) + ).default + }) + + afterEach(function () { + setReqValidationModeForTests(null) + }) + + describe('setInbound', function () { + beforeEach(function (ctx) { + ctx.req.url = '/project' + ctx.req.session = {} + ctx.middleware = ctx.AnalyticsRegistrationSourceMiddleware.setInbound() + }) + + describe('when the session already has an inbound referrer', function () { + beforeEach(function (ctx) { + ctx.req.session.inbound = { referrer: { medium: 'direct' } } + ctx.middleware(ctx.req, ctx.res, ctx.next) + }) + + it('does not overwrite it', function (ctx) { + sinon.assert.notCalled(ctx.AnalyticsRegistrationSourceHelper.setInbound) + }) + + it('calls next', function (ctx) { + sinon.assert.calledOnce(ctx.next) + }) + }) + + describe('when the user is already logged in', function () { + beforeEach(function (ctx) { + ctx.SessionManager.isUserLoggedIn.returns(true) + ctx.req.query = { utm_campaign: 'spring-sale' } + ctx.middleware(ctx.req, ctx.res, ctx.next) + }) + + it('does not record a referrer', function (ctx) { + sinon.assert.notCalled(ctx.AnalyticsRegistrationSourceHelper.setInbound) + }) + + it('calls next', function (ctx) { + sinon.assert.calledOnce(ctx.next) + }) + }) + + describe('with well-formed utm query params', function () { + beforeEach(function (ctx) { + ctx.req.query = { + utm_campaign: 'spring-sale', + utm_source: 'newsletter', + } + ctx.req.headers.referrer = 'https://example.com' + ctx.middleware(ctx.req, ctx.res, ctx.next) + }) + + it('forwards the parsed utm query to the helper', function (ctx) { + sinon.assert.calledWith( + ctx.AnalyticsRegistrationSourceHelper.setInbound, + ctx.req.session, + ctx.req.url, + { utm_campaign: 'spring-sale', utm_source: 'newsletter' }, + 'https://example.com' + ) + }) + + it('calls next', function (ctx) { + sinon.assert.calledOnce(ctx.next) + }) + }) + + describe('with a malformed utm query value', function () { + beforeEach(function (ctx) { + ctx.req.query = { utm_campaign: ['spring-sale', 'other'] } + }) + + describe('in the default log-only validation mode', function () { + beforeEach(function (ctx) { + setReqValidationModeForTests('log') + ctx.middleware(ctx.req, ctx.res, ctx.next) + }) + + it('falls back to the raw, unvalidated query', function (ctx) { + sinon.assert.calledWith( + ctx.AnalyticsRegistrationSourceHelper.setInbound, + ctx.req.session, + ctx.req.url, + ctx.req.query, + undefined + ) + }) + + it('calls next', function (ctx) { + sinon.assert.calledOnce(ctx.next) + }) + }) + + describe('with request validation enforced', function () { + beforeEach(function (ctx) { + setReqValidationModeForTests('enforce') + ctx.middleware(ctx.req, ctx.res, ctx.next) + }) + + it('fails silently without recording a referrer', function (ctx) { + sinon.assert.notCalled( + ctx.AnalyticsRegistrationSourceHelper.setInbound + ) + }) + + it('still calls next', function (ctx) { + sinon.assert.calledOnce(ctx.next) + }) + }) + }) + }) +}) diff --git a/services/web/test/unit/src/Analytics/AnalyticsUTMTrackingMiddleware.test.mjs b/services/web/test/unit/src/Analytics/AnalyticsUTMTrackingMiddleware.test.mjs index 4efcc835e4..7c63f11425 100644 --- a/services/web/test/unit/src/Analytics/AnalyticsUTMTrackingMiddleware.test.mjs +++ b/services/web/test/unit/src/Analytics/AnalyticsUTMTrackingMiddleware.test.mjs @@ -1,5 +1,6 @@ -import { assert, beforeEach, describe, it, vi } from 'vitest' +import { afterEach, assert, beforeEach, describe, it, vi } from 'vitest' import sinon from 'sinon' +import { setReqValidationModeForTests } from '@overleaf/validation-tools' import MockRequest from '../helpers/MockRequest.mjs' import MockResponse from '../helpers/MockResponse.mjs' @@ -44,6 +45,10 @@ describe('AnalyticsUTMTrackingMiddleware', function () { ctx.middleware = ctx.AnalyticsUTMTrackingMiddleware.recordUTMTags() }) + afterEach(function () { + setReqValidationModeForTests(null) + }) + describe('without UTM tags in query', function () { beforeEach(function (ctx) { ctx.req.url = '/project' @@ -216,4 +221,50 @@ describe('AnalyticsUTMTrackingMiddleware', function () { ) }) }) + + describe('with a non-string UTM parameter', function () { + beforeEach(function (ctx) { + ctx.req.url = '/project?utm_source=Organic&utm_source=Direct' + ctx.req.query = { + utm_source: ['Organic', 'Direct'], + } + ctx.req.headers = { + host: 'test-domain.overleaf.com', + } + }) + + it('still processes the request without throwing', function (ctx) { + setReqValidationModeForTests('log') + ctx.middleware(ctx.req, ctx.res, ctx.next) + + assert.isTrue(ctx.res.redirected) + assert.equal('/project', ctx.res.redirectedTo) + sinon.assert.calledWith( + ctx.AnalyticsManager.recordEventForSession, + ctx.req.session, + 'page-view', + { + path: '/project', + utm_source: ['Organic', 'Direct'], + domain: 'test-domain', + } + ) + }) + + describe('request validation', function () { + beforeEach(function (ctx) { + setReqValidationModeForTests('enforce') + }) + + it('does not throw', function (ctx) { + assert.doesNotThrow(() => { + ctx.middleware(ctx.req, ctx.res, ctx.next) + }) + + assert.isTrue(ctx.res.redirected) + assert.equal('/project', ctx.res.redirectedTo) + sinon.assert.notCalled(ctx.AnalyticsManager.recordEventForSession) + }) + }) + }) }) diff --git a/services/web/test/unit/src/Compile/ClsiCacheController.test.mjs b/services/web/test/unit/src/Compile/ClsiCacheController.test.mjs new file mode 100644 index 0000000000..0e591f995f --- /dev/null +++ b/services/web/test/unit/src/Compile/ClsiCacheController.test.mjs @@ -0,0 +1,155 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import sinon from 'sinon' +import MockRequest from '../helpers/MockRequest.mjs' +import MockResponse from '../helpers/MockResponse.mjs' +import { + NotFoundError, + ResourceGoneError, +} from '../../../../app/src/Features/Errors/Errors.js' + +const MODULE_PATH = + '../../../../app/src/Features/Compile/ClsiCacheController.mjs' + +// pin Errors.js to a single module instance so `instanceof` checks in the +// controller (loaded dynamically below, after other modules are mocked) +// see the same error classes this file imports statically +vi.mock('../../../../app/src/Features/Errors/Errors.js', () => + vi.importActual('../../../../app/src/Features/Errors/Errors.js') +) + +describe('ClsiCacheController', function () { + beforeEach(async function (ctx) { + // Project_id/editorBuildId are validated by the schema + ctx.projectId = '507f191e810c19729de860ea' + ctx.userId = '507f191e810c19729de860eb' + ctx.editorBuildId = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa-1111111-2222222' + + ctx.ClsiCacheHandler = { + isAllowedFilename: sinon.stub().returns(true), + getOutputFile: sinon.stub(), + getEgressLabel: sinon.stub().returns('output.pdf'), + } + ctx.ClsiCacheManager = { getLatestCompileResult: sinon.stub() } + ctx.CompileController = { + _getUserIdForCompile: sinon.stub().returns(ctx.userId), + _getSafeProjectName: sinon.stub().returns('project-name'), + _getSplitTestOptions: sinon.stub().resolves({ + pdfCachingMinChunkSize: 100, + pdfDownloadDomain: 'https://example.com', + }), + } + ctx.ProjectGetter = { + promises: { getProject: sinon.stub().resolves({ name: 'project' }) }, + } + + vi.doMock('../../../../app/src/Features/Compile/ClsiCacheHandler', () => ({ + default: ctx.ClsiCacheHandler, + })) + vi.doMock('../../../../app/src/Features/Compile/ClsiCacheManager', () => ({ + default: ctx.ClsiCacheManager, + })) + vi.doMock('../../../../app/src/Features/Compile/CompileController', () => ({ + default: ctx.CompileController, + })) + vi.doMock('../../../../app/src/Features/Project/ProjectGetter', () => ({ + default: ctx.ProjectGetter, + })) + + ctx.controller = (await import(MODULE_PATH)).default + ctx.req = new MockRequest(vi) + ctx.req.params = { Project_id: ctx.projectId } + ctx.res = new MockResponse(vi) + ctx.res.json = sinon.stub() + ctx.res.sendStatus = sinon.stub() + }) + + describe('getLatestBuildFromCache', function () { + beforeEach(function (ctx) { + ctx.result = { + zone: 'a', + outputFiles: [], + compileGroup: 'standard', + clsiServerId: 'server-1', + clsiCacheShard: 'shard-1', + options: {}, + stats: {}, + timings: {}, + } + }) + + it('should return the cached compile result as json', async function (ctx) { + ctx.ClsiCacheManager.getLatestCompileResult.resolves(ctx.result) + await ctx.controller.getLatestBuildFromCache(ctx.req, ctx.res) + ctx.ClsiCacheManager.getLatestCompileResult + .calledWith(ctx.projectId, ctx.userId) + .should.equal(true) + ctx.res.json + .calledWithMatch({ + fromCache: true, + status: 'success', + clsiServerId: 'server-1', + pdfDownloadDomain: 'https://example.com/zone/a', + }) + .should.equal(true) + }) + + it('should return 404 when the compile result is not found', async function (ctx) { + ctx.ClsiCacheManager.getLatestCompileResult.rejects(new NotFoundError()) + await ctx.controller.getLatestBuildFromCache(ctx.req, ctx.res) + ctx.res.sendStatus.calledWith(404).should.equal(true) + }) + + it('should return 410 when the compile result is gone', async function (ctx) { + ctx.ClsiCacheManager.getLatestCompileResult.rejects( + new ResourceGoneError() + ) + await ctx.controller.getLatestBuildFromCache(ctx.req, ctx.res) + ctx.res.sendStatus.calledWith(410).should.equal(true) + }) + + it('should reject a malformed project id', async function (ctx) { + ctx.req.params = { Project_id: 'not-an-object-id' } + await expect(ctx.controller.getLatestBuildFromCache(ctx.req, ctx.res)).to + .be.rejected + ctx.ClsiCacheManager.getLatestCompileResult.called.should.equal(false) + }) + }) + + describe('downloadFromCache', function () { + beforeEach(function (ctx) { + ctx.req.params = { + Project_id: ctx.projectId, + editorBuildId: ctx.editorBuildId, + filename: 'output.pdf', + } + }) + + it('should reject a malformed project id', async function (ctx) { + ctx.req.params.Project_id = 'not-an-object-id' + await expect(ctx.controller.downloadFromCache(ctx.req, ctx.res)).to.be + .rejected + ctx.ClsiCacheHandler.getOutputFile.called.should.equal(false) + }) + + it('should reject a malformed editorBuildId', async function (ctx) { + ctx.req.params.editorBuildId = 'not-a-valid-build-id' + await expect(ctx.controller.downloadFromCache(ctx.req, ctx.res)).to.be + .rejected + ctx.ClsiCacheHandler.getOutputFile.called.should.equal(false) + }) + + it('should reject a path traversal filename', async function (ctx) { + ctx.req.params.filename = '../../../etc/passwd' + await expect(ctx.controller.downloadFromCache(ctx.req, ctx.res)).to.be + .rejected + ctx.ClsiCacheHandler.getOutputFile.called.should.equal(false) + }) + + it('should reject a filename rejected by ClsiCacheHandler', async function (ctx) { + ctx.ClsiCacheHandler.isAllowedFilename.returns(false) + await expect(ctx.controller.downloadFromCache(ctx.req, ctx.res)).to.be + .rejected + ctx.ClsiCacheHandler.getOutputFile.called.should.equal(false) + }) + }) +}) diff --git a/services/web/test/unit/src/Compile/CompileController.test.mjs b/services/web/test/unit/src/Compile/CompileController.test.mjs index 5bf92e2b00..8fcfdb80a4 100644 --- a/services/web/test/unit/src/Compile/CompileController.test.mjs +++ b/services/web/test/unit/src/Compile/CompileController.test.mjs @@ -1,10 +1,11 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import sinon from 'sinon' import MockRequest from '../helpers/MockRequest.mjs' import MockResponse from '../helpers/MockResponse.mjs' import { Headers } from 'node-fetch' import { ReadableString } from '@overleaf/stream-utils' import { RequestFailedError } from '@overleaf/fetch-utils' +import { setReqValidationModeForTests } from '@overleaf/validation-tools' import { asZodError } from '@overleaf/validation-tools/testUtils.js' const modulePath = '../../../../app/src/Features/Compile/CompileController.mjs' @@ -390,7 +391,7 @@ describe('CompileController', function () { describe('with an editor id', function () { beforeEach(async function (ctx) { - ctx.req.body = { editorId: 'the-editor-id' } + ctx.req.body = { editorId: '550e8400-e29b-41d4-a716-446655440000' } await ctx.CompileController.compile(ctx.req, ctx.res, ctx.next) }) @@ -406,7 +407,7 @@ describe('CompileController', function () { enablePdfCaching: false, fileLineErrors: false, stopOnFirstError: false, - editorId: 'the-editor-id', + editorId: '550e8400-e29b-41d4-a716-446655440000', rootResourcePath: undefined, } ) @@ -657,6 +658,26 @@ describe('CompileController', function () { }) }) + describe('stopCompile', function () { + beforeEach(async function (ctx) { + ctx.CompileManager.promises.stopCompile = sinon.stub().resolves() + ctx.req.params = { Project_id: ctx.projectId } + ctx.req.session = {} + ctx.res.sendStatus = sinon.stub() + await ctx.CompileController.stopCompile(ctx.req, ctx.res, ctx.next) + }) + + it('should stop the compile for the project', function (ctx) { + ctx.CompileManager.promises.stopCompile + .calledWith(ctx.projectId, ctx.user_id) + .should.equal(true) + }) + + it('should return a 200', function (ctx) { + ctx.res.sendStatus.calledWith(200).should.equal(true) + }) + }) + describe('compileSubmission', function () { beforeEach(function (ctx) { ctx.submission_id = 'sub-1234' @@ -785,6 +806,22 @@ describe('CompileController', function () { }) }) + describe('with popupDownload', function () { + beforeEach(async function (ctx) { + ctx.req.query = { + clsiserverid: ctx.clsiServerId, + popupDownload: 'true', + } + await ctx.CompileController.downloadPdf(ctx.req, ctx.res, ctx.next) + }) + + it('should set the content-disposition to attachment', function (ctx) { + expect(ctx.res.setContentDisposition).toBeCalledWith('attachment', { + filename: 'test_namè__1.pdf', + }) + }) + }) + describe('anon', function () { beforeEach(async function (ctx) { ctx.SessionManager.getLoggedInUserId.returns(null) @@ -798,6 +835,26 @@ describe('CompileController', function () { }) }) + describe('with an enable_pdf_caching query param', function () { + beforeEach(async function (ctx) { + // output-files.ts appends this to the PDF download link whenever + // pdf caching is enabled. + ctx.req.query = { + clsiserverid: ctx.clsiServerId, + enable_pdf_caching: 'true', + } + await ctx.CompileController.downloadPdf(ctx.req, ctx.res, ctx.next) + }) + + it('should not reject the request', function (ctx) { + ctx.next.should.not.have.been.called + }) + + it('should proxy the PDF from the CLSI', function (ctx) { + ctx.fetchUtils.fetchStreamWithResponse.should.have.been.called + }) + }) + describe('when rate-limited', function () { beforeEach(async function (ctx) { ctx.rateLimiter.consume.rejects({ @@ -892,6 +949,55 @@ describe('CompileController', function () { ) }) }) + + describe('with the full query string buildFileList sends for the archive link', function () { + beforeEach(async function (ctx) { + // services/web/frontend/js/features/pdf-preview/util/file-list.ts + // appends editorId/compileGroup to the shared params object, then + // one `files` entry per archived file -- a single file arrives as + // a bare string, not a one-element array. enable_pdf_caching is + // appended separately (see output-files.ts) whenever pdf caching is + // enabled. + ctx.req.query = { + clsiserverid: ctx.clsiServerId, + editorId: 'e58b482a-3c9e-4de1-9a05-d5e2f1c8f8f0', + compileGroup: 'priority', + files: 'output.pdf', + enable_pdf_caching: 'true', + } + await ctx.CompileController.getOutputZipFromClsi( + ctx.req, + ctx.res, + ctx.next + ) + }) + + it('should not reject the request', function (ctx) { + ctx.next.should.not.have.been.called + }) + + it('should proxy to the CLSI', function (ctx) { + ctx.fetchUtils.fetchStreamWithResponse.should.have.been.called + }) + }) + + describe('with multiple files in the query string', function () { + beforeEach(async function (ctx) { + ctx.req.query = { + clsiserverid: ctx.clsiServerId, + files: ['output.pdf', 'output.log'], + } + await ctx.CompileController.getOutputZipFromClsi( + ctx.req, + ctx.res, + ctx.next + ) + }) + + it('should not reject the request', function (ctx) { + ctx.next.should.not.have.been.called + }) + }) }) describe('getFileForSubmissionFromClsi', function () { @@ -973,6 +1079,54 @@ describe('CompileController', function () { } ) }) + + it('should reject a buildId that does not match the hex-hyphen-hex shape', async function (ctx) { + ctx.req.query.buildId = 'not-a-valid-build-id' + ctx.next = sinon.stub() + await ctx.CompileController.proxySyncCode(ctx.req, ctx.res, ctx.next) + ctx.next.should.have.been.calledWithMatch({ + name: 'InvalidRequestError', + zodError: asZodError({ + origin: 'string', + code: 'invalid_format', + format: 'regex', + pattern: '/^[0-9a-f]+-[0-9a-f]+$/', + path: ['query', 'buildId'], + message: 'invalid buildId', + }), + }) + ctx.CompileManager.promises.syncTeX.should.have.been.calledOnce + }) + + describe('when the schema is in log-only mode', function () { + beforeEach(function () { + setReqValidationModeForTests('log') + }) + + afterEach(function () { + setReqValidationModeForTests(null) + }) + + it('should still reject a non-numeric line via the manual guard', async function (ctx) { + ctx.req.query.line = 'not-a-number' + ctx.next = sinon.stub() + await ctx.CompileController.proxySyncCode(ctx.req, ctx.res, ctx.next) + ctx.next.should.have.been.calledWithMatch({ + message: 'invalid line parameter', + }) + ctx.CompileManager.promises.syncTeX.should.have.been.calledOnce + }) + + it('should still reject a non-numeric column via the manual guard', async function (ctx) { + ctx.req.query.column = 'not-a-number' + ctx.next = sinon.stub() + await ctx.CompileController.proxySyncCode(ctx.req, ctx.res, ctx.next) + ctx.next.should.have.been.calledWithMatch({ + message: 'invalid column parameter', + }) + ctx.CompileManager.promises.syncTeX.should.have.been.calledOnce + }) + }) }) describe('proxySyncPdf', function () { @@ -1023,6 +1177,64 @@ describe('CompileController', function () { } ) }) + + it('should reject a buildId that does not match the hex-hyphen-hex shape', async function (ctx) { + ctx.req.query.buildId = 'not-a-valid-build-id' + ctx.next = sinon.stub() + await ctx.CompileController.proxySyncPdf(ctx.req, ctx.res, ctx.next) + ctx.next.should.have.been.calledWithMatch({ + name: 'InvalidRequestError', + zodError: asZodError({ + origin: 'string', + code: 'invalid_format', + format: 'regex', + pattern: '/^[0-9a-f]+-[0-9a-f]+$/', + path: ['query', 'buildId'], + message: 'invalid buildId', + }), + }) + ctx.CompileManager.promises.syncTeX.should.have.been.calledOnce + }) + + describe('when the schema is in log-only mode', function () { + beforeEach(function () { + setReqValidationModeForTests('log') + }) + + afterEach(function () { + setReqValidationModeForTests(null) + }) + + it('should still reject a non-numeric page via the manual guard', async function (ctx) { + ctx.req.query.page = 'not-a-number' + ctx.next = sinon.stub() + await ctx.CompileController.proxySyncPdf(ctx.req, ctx.res, ctx.next) + ctx.next.should.have.been.calledWithMatch({ + message: 'invalid page parameter', + }) + ctx.CompileManager.promises.syncTeX.should.have.been.calledOnce + }) + + it('should still reject a non-numeric h via the manual guard', async function (ctx) { + ctx.req.query.h = 'not-a-number' + ctx.next = sinon.stub() + await ctx.CompileController.proxySyncPdf(ctx.req, ctx.res, ctx.next) + ctx.next.should.have.been.calledWithMatch({ + message: 'invalid h parameter', + }) + ctx.CompileManager.promises.syncTeX.should.have.been.calledOnce + }) + + it('should still reject a non-numeric v via the manual guard', async function (ctx) { + ctx.req.query.v = 'not-a-number' + ctx.next = sinon.stub() + await ctx.CompileController.proxySyncPdf(ctx.req, ctx.res, ctx.next) + ctx.next.should.have.been.calledWithMatch({ + message: 'invalid v parameter', + }) + ctx.CompileManager.promises.syncTeX.should.have.been.calledOnce + }) + }) }) describe('getFileFromClsi', function () { @@ -1169,6 +1381,89 @@ describe('CompileController', function () { expect(ctx.res.statusCode).to.equal(404) }) }) + + describe('with a compileGroup query param', function () { + beforeEach(async function (ctx) { + // buildFileList (see file-list.ts) always appends this to every + // per-file download link it builds + ctx.req.query = { + clsiserverid: ctx.clsiServerId, + compileGroup: 'priority', + } + await ctx.CompileController.getFileFromClsi(ctx.req, ctx.res, ctx.next) + }) + + it('should not reject the request', function (ctx) { + ctx.next.should.not.have.been.called + }) + + it('should proxy to the CLSI', function (ctx) { + ctx.fetchUtils.fetchStreamWithResponse.should.have.been.called + }) + }) + + describe('with an enable_pdf_caching query param', function () { + beforeEach(async function (ctx) { + // output-files.ts appends this to per-file download links whenever + // pdf caching is enabled. + ctx.req.query = { + clsiserverid: ctx.clsiServerId, + enable_pdf_caching: 'true', + } + await ctx.CompileController.getFileFromClsi(ctx.req, ctx.res, ctx.next) + }) + + it('should not reject the request', function (ctx) { + ctx.next.should.not.have.been.called + }) + + it('should proxy to the CLSI', function (ctx) { + ctx.fetchUtils.fetchStreamWithResponse.should.have.been.called + }) + }) + + describe('with an invalid enable_pdf_caching query param', function () { + beforeEach(function (ctx) { + ctx.req.query = { + clsiserverid: ctx.clsiServerId, + enable_pdf_caching: 'notabool', + } + ctx.next = sinon.stub() + }) + + it('should reject the request', async function (ctx) { + await ctx.CompileController.getFileFromClsi(ctx.req, ctx.res, ctx.next) + ctx.next.should.have.been.calledWithMatch({ + name: 'InvalidRequestError', + zodError: asZodError({ + code: 'invalid_value', + expected: 'stringbool', + values: [ + 'true', + '1', + 'yes', + 'on', + 'y', + 'enabled', + 'false', + '0', + 'no', + 'off', + 'n', + 'disabled', + ], + path: ['query', 'enable_pdf_caching'], + message: + 'Invalid option: expected one of "true"|"1"|"yes"|"on"|"y"|"enabled"|"false"|"0"|"no"|"off"|"n"|"disabled"', + }), + }) + }) + + it('should not open a request to CLSI', async function (ctx) { + await ctx.CompileController.getFileFromClsi(ctx.req, ctx.res, ctx.next) + ctx.fetchUtils.fetchStreamWithResponse.should.not.have.been.called + }) + }) }) describe('deleteAuxFiles', function () { diff --git a/services/web/test/unit/src/FileStore/FileStoreController.test.mjs b/services/web/test/unit/src/FileStore/FileStoreController.test.mjs index 228bdb9950..1f38bda9c5 100644 --- a/services/web/test/unit/src/FileStore/FileStoreController.test.mjs +++ b/services/web/test/unit/src/FileStore/FileStoreController.test.mjs @@ -38,14 +38,15 @@ describe('FileStoreController', function () { ctx.controller = (await import(MODULE_PATH)).default ctx.stream = {} - ctx.projectId = '2k3j1lk3j21lk3j' - ctx.fileId = '12321kklj1lk3jk12' + // Project_id/File_id are validated as Mongo ObjectIds + ctx.projectId = '507f191e810c19729de860ea' + ctx.fileId = '507f191e810c19729de860eb' ctx.req = { params: { Project_id: ctx.projectId, File_id: ctx.fileId, }, - query: 'query string here', + query: { foo: 'bar' }, get(key) { return undefined }, diff --git a/services/web/test/unit/src/HelperFiles/UrlHelper.test.mjs b/services/web/test/unit/src/HelperFiles/UrlHelper.test.mjs index 688fbe931b..665af3ad8b 100644 --- a/services/web/test/unit/src/HelperFiles/UrlHelper.test.mjs +++ b/services/web/test/unit/src/HelperFiles/UrlHelper.test.mjs @@ -45,4 +45,26 @@ describe('UrlHelper', function () { ) }) }) + + describe('getCanonicalURL', function () { + it('strips the query string and hash', function (ctx) { + const req = { + headers: { host: '127.0.0.1:3000' }, + originalUrl: '/blog/some-post?foo=bar#section', + } + expect(ctx.UrlHelper.getCanonicalURL(req)).to.equal( + 'http://127.0.0.1:3000/blog/some-post' + ) + }) + + it('strips a trailing slash', function (ctx) { + const req = { + headers: { host: '127.0.0.1:3000' }, + originalUrl: '/blog/some-post/', + } + expect(ctx.UrlHelper.getCanonicalURL(req)).to.equal( + 'http://127.0.0.1:3000/blog/some-post' + ) + }) + }) }) diff --git a/services/web/test/unit/src/InactiveData/InactiveProjectController.test.mjs b/services/web/test/unit/src/InactiveData/InactiveProjectController.test.mjs new file mode 100644 index 0000000000..bb276a253a --- /dev/null +++ b/services/web/test/unit/src/InactiveData/InactiveProjectController.test.mjs @@ -0,0 +1,100 @@ +import { beforeEach, describe, it, vi } from 'vitest' +import sinon from 'sinon' +import MockRequest from '../helpers/MockRequest.mjs' +import MockResponse from '../helpers/MockResponse.mjs' + +const modulePath = + '../../../../app/src/Features/InactiveData/InactiveProjectController' + +describe('InactiveProjectController', function () { + beforeEach(async function (ctx) { + ctx.InactiveProjectManager = { + deactivateOldProjects: sinon.stub(), + deactivateProject: sinon.stub(), + } + vi.doMock( + '../../../../app/src/Features/InactiveData/InactiveProjectManager', + () => ({ + default: ctx.InactiveProjectManager, + }) + ) + ctx.controller = (await import(modulePath)).default + ctx.req = new MockRequest(vi) + ctx.res = new MockResponse(vi) + ctx.res.json = sinon.stub() + ctx.res.sendStatus = sinon.stub() + }) + + describe('deactivateOldProjects', function () { + it('should call the manager with the parsed numeric values', function (ctx) { + ctx.req.body = { numberOfProjectsToArchive: 5, ageOfProjects: 30 } + ctx.InactiveProjectManager.deactivateOldProjects.callsFake( + (limit, daysOld, cb) => cb(null, ['project-1']) + ) + ctx.controller.deactivateOldProjects(ctx.req, ctx.res) + ctx.InactiveProjectManager.deactivateOldProjects + .calledWith(5, 30) + .should.equal(true) + ctx.res.json.calledWith(['project-1']).should.equal(true) + }) + + it('should coerce numeric-looking strings sent as the body', function (ctx) { + // some private-API callers post numbers as strings + ctx.req.body = { numberOfProjectsToArchive: '5', ageOfProjects: '30' } + ctx.InactiveProjectManager.deactivateOldProjects.callsFake( + (limit, daysOld, cb) => cb(null, []) + ) + ctx.controller.deactivateOldProjects(ctx.req, ctx.res) + ctx.InactiveProjectManager.deactivateOldProjects + .calledWith(5, 30) + .should.equal(true) + }) + + it('should work without any body fields', function (ctx) { + ctx.req.body = {} + ctx.InactiveProjectManager.deactivateOldProjects.callsFake( + (limit, daysOld, cb) => cb(null, []) + ) + ctx.controller.deactivateOldProjects(ctx.req, ctx.res) + ctx.InactiveProjectManager.deactivateOldProjects + .calledWith(undefined, undefined) + .should.equal(true) + }) + + it('should return a 500 when the manager errors', function (ctx) { + ctx.req.body = {} + ctx.InactiveProjectManager.deactivateOldProjects.callsFake( + (limit, daysOld, cb) => cb(new Error('failed')) + ) + ctx.controller.deactivateOldProjects(ctx.req, ctx.res) + ctx.res.sendStatus.calledWith(500).should.equal(true) + }) + }) + + describe('deactivateProject', function () { + beforeEach(function (ctx) { + // project_id is validated as a Mongo ObjectId + ctx.projectId = '507f191e810c19729de860ea' + ctx.req.params = { project_id: ctx.projectId } + }) + + it('should call the manager and return 200', function (ctx) { + ctx.InactiveProjectManager.deactivateProject.callsFake((id, cb) => + cb(null) + ) + ctx.controller.deactivateProject(ctx.req, ctx.res) + ctx.InactiveProjectManager.deactivateProject + .calledWith(ctx.projectId) + .should.equal(true) + ctx.res.sendStatus.calledWith(200).should.equal(true) + }) + + it('should return a 500 when the manager errors', function (ctx) { + ctx.InactiveProjectManager.deactivateProject.callsFake((id, cb) => + cb(new Error('failed')) + ) + ctx.controller.deactivateProject(ctx.req, ctx.res) + ctx.res.sendStatus.calledWith(500).should.equal(true) + }) + }) +}) diff --git a/services/web/test/unit/src/LinkedFiles/LinkedFilesController.test.mjs b/services/web/test/unit/src/LinkedFiles/LinkedFilesController.test.mjs index b2ae3749cc..d8cd4077ee 100644 --- a/services/web/test/unit/src/LinkedFiles/LinkedFilesController.test.mjs +++ b/services/web/test/unit/src/LinkedFiles/LinkedFilesController.test.mjs @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import sinon from 'sinon' +import { setReqValidationModeForTests } from '@overleaf/validation-tools' const modulePath = '../../../../app/src/Features/LinkedFiles/LinkedFilesController.mjs' @@ -13,6 +14,10 @@ describe('LinkedFilesController', function () { ctx.clock.restore() }) + afterEach(function () { + setReqValidationModeForTests(null) + }) + beforeEach(async function (ctx) { ctx.userId = 'user-id' ctx.Agent = { @@ -25,9 +30,11 @@ describe('LinkedFilesController', function () { // ObjectIds, so use well-formed values throughout. ctx.projectId = '507f1f77bcf86cd799439011' ctx.parentFolderId = '507f191e810c19729de860eb' - ctx.provider = 'provider' + // must be one of the real, schema-validated provider literals; 'url' is + // the simplest shape (a single required `url` field in `data`). + ctx.provider = 'url' ctx.fileName = 'linked-file-name' - ctx.data = { customAgentData: 'foo' } + ctx.data = { url: 'https://example.com/foo' } ctx.LinkedFilesHandler = { promises: { getFileById: sinon.stub(), @@ -140,7 +147,7 @@ describe('LinkedFilesController', function () { expect(ctx.Agent.promises.createLinkedFile).to.have.been.calledWith( ctx.projectId, { - customAgentData: 'foo', + url: 'https://example.com/foo', provider: ctx.provider, importedAt: ctx.fakeTime.toISOString(), }, @@ -154,6 +161,43 @@ describe('LinkedFilesController', function () { ctx.LinkedFilesController.createLinkedFile(ctx.req, ctx.res, ctx.next) }) }) + + it('rejects a mendeley group_id containing a path separator without calling the agent', async function (ctx) { + setReqValidationModeForTests('enforce') + ctx.req.body.provider = 'mendeley' + ctx.req.body.data = { group_id: 'abcd/../../etc' } + await new Promise(resolve => { + ctx.next = sinon.stub().callsFake(() => resolve()) + ctx.res = { + json: () => resolve(), + sendStatus: () => resolve(), + } + ctx.LinkedFilesController.createLinkedFile(ctx.req, ctx.res, ctx.next) + }) + expect(ctx.next).to.have.been.calledOnce + expect(ctx.next.firstCall.args[0]?.name).to.equal('InvalidRequestError') + expect(ctx.Agent.promises.createLinkedFile).to.not.have.been.called + }) + + it('rejects a project_output_file build_id that is not in the hex-hyphen-hex shape without calling the agent', async function (ctx) { + setReqValidationModeForTests('enforce') + ctx.req.body.provider = 'project_output_file' + ctx.req.body.data = { + source_output_file_path: 'output.pdf', + build_id: 'not-a-valid-build-id', + } + await new Promise(resolve => { + ctx.next = sinon.stub().callsFake(() => resolve()) + ctx.res = { + json: () => resolve(), + sendStatus: () => resolve(), + } + ctx.LinkedFilesController.createLinkedFile(ctx.req, ctx.res, ctx.next) + }) + expect(ctx.next).to.have.been.calledOnce + expect(ctx.next.firstCall.args[0]?.name).to.equal('InvalidRequestError') + expect(ctx.Agent.promises.createLinkedFile).to.not.have.been.called + }) }) describe('refreshLinkedFiles', function () { beforeEach(function (ctx) { @@ -211,6 +255,7 @@ describe('LinkedFilesController', function () { }) it('rejects invalid params without calling the agent', async function (ctx) { + setReqValidationModeForTests('enforce') ctx.req.params.file_id = 'not-an-object-id' await new Promise(resolve => { ctx.next = sinon.stub().callsFake(() => resolve()) diff --git a/services/web/test/unit/src/PasswordReset/PasswordResetController.test.mjs b/services/web/test/unit/src/PasswordReset/PasswordResetController.test.mjs index e8db5c0af4..0645872447 100644 --- a/services/web/test/unit/src/PasswordReset/PasswordResetController.test.mjs +++ b/services/web/test/unit/src/PasswordReset/PasswordResetController.test.mjs @@ -121,6 +121,10 @@ describe('PasswordResetController', function () { }) describe('requestReset', function () { + beforeEach(function (ctx) { + ctx.req.body = { email: ctx.req.body.email } + }) + it('should tell the handler to process that email', async function (ctx) { ctx.PasswordResetHandler.promises.generateAndEmailResetToken.resolves( 'primary' diff --git a/services/web/test/unit/src/ServerAdmin/AdminController.test.mjs b/services/web/test/unit/src/ServerAdmin/AdminController.test.mjs new file mode 100644 index 0000000000..5edff686e4 --- /dev/null +++ b/services/web/test/unit/src/ServerAdmin/AdminController.test.mjs @@ -0,0 +1,177 @@ +import { beforeEach, describe, it, vi } from 'vitest' +import sinon from 'sinon' +import MockRequest from '../helpers/MockRequest.mjs' +import MockResponse from '../helpers/MockResponse.mjs' + +const modulePath = + '../../../../app/src/Features/ServerAdmin/AdminController.mjs' + +describe('AdminController', function () { + beforeEach(async function (ctx) { + ctx.Settings = {} + ctx.TpdsUpdateSender = { pollDropboxForUser: sinon.stub() } + ctx.TpdsProjectFlusher = { flushProjectToTpds: sinon.stub() } + ctx.EditorRealTimeController = { emitToAll: sinon.stub() } + ctx.SystemMessageManager = { + createMessage: sinon.stub(), + clearMessages: sinon.stub(), + promises: { + getMessagesFromDB: sinon.stub().resolves([]), + }, + } + ctx.ProjectGetter = { + promises: { findAllDebugProjects: sinon.stub().resolves([]) }, + } + ctx.Modules = { + promises: { hooks: { fire: sinon.stub().resolves([null]) } }, + } + ctx.Features = { hasFeature: sinon.stub().returns(false) } + + vi.doMock('@overleaf/settings', () => ({ default: ctx.Settings })) + vi.doMock( + '../../../../app/src/Features/ThirdPartyDataStore/TpdsUpdateSender', + () => ({ default: ctx.TpdsUpdateSender }) + ) + vi.doMock( + '../../../../app/src/Features/ThirdPartyDataStore/TpdsProjectFlusher', + () => ({ default: ctx.TpdsProjectFlusher }) + ) + vi.doMock( + '../../../../app/src/Features/Editor/EditorRealTimeController', + () => ({ default: ctx.EditorRealTimeController }) + ) + vi.doMock( + '../../../../app/src/Features/SystemMessages/SystemMessageManager', + () => ({ default: ctx.SystemMessageManager }) + ) + vi.doMock('../../../../app/src/Features/Project/ProjectGetter', () => ({ + default: ctx.ProjectGetter, + })) + vi.doMock('../../../../app/src/infrastructure/Modules', () => ({ + default: ctx.Modules, + })) + vi.doMock('../../../../app/src/infrastructure/Features', () => ({ + default: ctx.Features, + })) + + ctx.controller = (await import(modulePath)).default + ctx.req = new MockRequest(vi) + ctx.res = new MockResponse(vi) + ctx.res.redirect = sinon.stub() + ctx.res.sendStatus = sinon.stub() + ctx.res.render = sinon.stub() + ctx.next = sinon.stub() + }) + + describe('index', function () { + it('should render the admin page', async function (ctx) { + await ctx.controller.index(ctx.req, ctx.res, ctx.next) + ctx.res.render.calledWith('admin/index').should.equal(true) + }) + + it('should include debug projects when saas is enabled', async function (ctx) { + ctx.Features.hasFeature.returns(true) + await ctx.controller.index(ctx.req, ctx.res, ctx.next) + ctx.ProjectGetter.promises.findAllDebugProjects.called.should.equal(true) + ctx.res.render + .calledWithMatch('admin/index', { + debugProjects: [], + }) + .should.equal(true) + }) + }) + + describe('disconnectAllUsers', function () { + it('should use the default delay when none is given', function (ctx) { + ctx.req.query = {} + ctx.controller.disconnectAllUsers(ctx.req, ctx.res) + ctx.EditorRealTimeController.emitToAll + .calledWith('forceDisconnect', sinon.match.string, 10) + .should.equal(true) + ctx.res.redirect.calledWith('/admin#open-close-editor').should.equal(true) + }) + + it('should use the given delay', function (ctx) { + ctx.req.query = { delay: '5000' } + ctx.controller.disconnectAllUsers(ctx.req, ctx.res) + ctx.EditorRealTimeController.emitToAll + .calledWith('forceDisconnect', sinon.match.string, 5000) + .should.equal(true) + }) + }) + + describe('openEditor', function () { + it('should open the editor and redirect', function (ctx) { + ctx.controller.openEditor(ctx.req, ctx.res) + ctx.Settings.editorIsOpen.should.equal(true) + ctx.res.redirect.calledWith('/admin#open-close-editor').should.equal(true) + }) + }) + + describe('closeEditor', function () { + it('should close the editor and redirect', function (ctx) { + ctx.req.body = {} + ctx.controller.closeEditor(ctx.req, ctx.res) + Boolean(ctx.Settings.editorIsOpen).should.equal(false) + ctx.res.redirect.calledWith('/admin#open-close-editor').should.equal(true) + }) + }) + + describe('flushProjectToTpds', function () { + beforeEach(function (ctx) { + // project_id is validated as a Mongo ObjectId + ctx.projectId = '507f191e810c19729de860ea' + ctx.req.body = { project_id: ctx.projectId } + }) + + it('should flush the project and return 200', function (ctx) { + ctx.TpdsProjectFlusher.flushProjectToTpds.callsFake((id, cb) => cb()) + ctx.controller.flushProjectToTpds(ctx.req, ctx.res, ctx.next) + ctx.TpdsProjectFlusher.flushProjectToTpds + .calledWith(ctx.projectId) + .should.equal(true) + ctx.res.sendStatus.calledWith(200).should.equal(true) + }) + + it('should call next with an error on failure', function (ctx) { + const error = new Error('failed') + ctx.TpdsProjectFlusher.flushProjectToTpds.callsFake((id, cb) => cb(error)) + ctx.controller.flushProjectToTpds(ctx.req, ctx.res, ctx.next) + ctx.next.calledWith(error).should.equal(true) + }) + }) + + describe('pollDropboxForUser', function () { + it('should poll dropbox for the user', function (ctx) { + // user_id is validated as a Mongo ObjectId + ctx.userId = '507f191e810c19729de860eb' + ctx.req.body = { user_id: ctx.userId } + ctx.TpdsUpdateSender.pollDropboxForUser.callsFake((id, cb) => cb()) + ctx.controller.pollDropboxForUser(ctx.req, ctx.res) + ctx.TpdsUpdateSender.pollDropboxForUser + .calledWith(ctx.userId) + .should.equal(true) + ctx.res.sendStatus.calledWith(200).should.equal(true) + }) + }) + + describe('createMessage', function () { + it('should create the message and redirect', function (ctx) { + ctx.req.body = { content: 'a system message' } + ctx.SystemMessageManager.createMessage.callsFake((content, cb) => cb()) + ctx.controller.createMessage(ctx.req, ctx.res, ctx.next) + ctx.SystemMessageManager.createMessage + .calledWith('a system message') + .should.equal(true) + ctx.res.redirect.calledWith('/admin#system-messages').should.equal(true) + }) + }) + + describe('clearMessages', function () { + it('should clear messages and redirect', function (ctx) { + ctx.SystemMessageManager.clearMessages.callsFake(cb => cb()) + ctx.controller.clearMessages(ctx.req, ctx.res, ctx.next) + ctx.res.redirect.calledWith('/admin#system-messages').should.equal(true) + }) + }) +}) diff --git a/services/web/test/unit/src/SplitTests/SplitTestHandler.test.mjs b/services/web/test/unit/src/SplitTests/SplitTestHandler.test.mjs index c974020ed9..a91eb9bf6c 100644 --- a/services/web/test/unit/src/SplitTests/SplitTestHandler.test.mjs +++ b/services/web/test/unit/src/SplitTests/SplitTestHandler.test.mjs @@ -543,6 +543,57 @@ describe('SplitTestHandler', function () { }) }) + describe('malformed query override values', function () { + describe('getAssignment', function () { + beforeEach(function (ctx) { + ctx.AnalyticsManager.getIdsFromSession.returns({ + userId: 'abc123abc123', + }) + ctx.req.query = { 'active-test': { nested: 'oops' } } + }) + + it('falls through to the computed assignment', async function (ctx) { + const { variant } = await ctx.SplitTestHandler.promises.getAssignment( + ctx.req, + ctx.res, + 'active-test' + ) + expect(variant).to.equal('variant-1') + }) + }) + + describe('hasUserBeenAssignedToVariant', function () { + beforeEach(function (ctx) { + ctx.user = { + _id: new ObjectId(), + analyticsId: 'analytics-id', + splitTests: { + 'active-test': [ + { + variantName: 'variant-1', + versionNumber: 2, + assignedAt: 'active-test-assigned-at', + }, + ], + }, + } + ctx.SplitTestUserGetter.promises.getUser.resolves(ctx.user) + ctx.req.query = { 'active-test': { nested: 'oops' } } + }) + + it('still resolves the recorded assignment', async function (ctx) { + const assigned = + await ctx.SplitTestHandler.promises.hasUserBeenAssignedToVariant( + ctx.req, + ctx.user._id, + 'active-test', + 'variant-1' + ) + expect(assigned).to.be.true + }) + }) + }) + describe('variant user limits', function () { beforeEach(function (ctx) { ctx.AnalyticsManager.getIdsFromSession.returns({ diff --git a/services/web/test/unit/src/TokenAccess/TokenAccessController.test.mjs b/services/web/test/unit/src/TokenAccess/TokenAccessController.test.mjs index 22367c02dc..1d4d55a79d 100644 --- a/services/web/test/unit/src/TokenAccess/TokenAccessController.test.mjs +++ b/services/web/test/unit/src/TokenAccess/TokenAccessController.test.mjs @@ -1,6 +1,11 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import sinon from 'sinon' import mongodb from 'mongodb-legacy' +import { + InvalidParamsError, + InvalidRequestError, + setReqValidationModeForTests, +} from '@overleaf/validation-tools' import MockRequest from '../helpers/MockRequest.mjs' import MockResponse from '../helpers/MockResponse.mjs' import PrivilegeLevels from '../../../../app/src/Features/Authorization/PrivilegeLevels.mjs' @@ -277,6 +282,10 @@ describe('TokenAccessController', function () { ctx.TokenAccessController = (await import(MODULE_PATH)).default }) + afterEach(function () { + setReqValidationModeForTests(null) + }) + describe('grantTokenAccessReadAndWrite', function () { beforeEach(function (ctx) { ctx.LimitationsManager.promises.canAcceptEditCollaboratorInvite.resolves( @@ -1029,7 +1038,7 @@ describe('TokenAccessController', function () { describe('ensureUserCanUseSharingUpdatesConsentPage', function () { beforeEach(function (ctx) { - ctx.req.params = { Project_id: ctx.project._id } + ctx.req.params = { Project_id: ctx.project._id.toString() } }) describe('when not in link sharing changes test', function () { @@ -1082,10 +1091,10 @@ describe('TokenAccessController', function () { expect( ctx.CollaboratorsGetter.promises .isUserInvitedReadWriteMemberOfProject - ).to.have.been.calledWith(ctx.user._id, ctx.project._id) + ).to.have.been.calledWith(ctx.user._id, ctx.project._id.toString()) expect( ctx.CollaboratorsGetter.promises.userIsReadWriteTokenMember - ).to.have.been.calledWith(ctx.user._id, ctx.project._id) + ).to.have.been.calledWith(ctx.user._id, ctx.project._id.toString()) expect(ctx.next).to.have.been.calledOnce expect(ctx.next.firstCall.args[0]).to.not.exist }) @@ -1147,7 +1156,7 @@ describe('TokenAccessController', function () { describe('moveReadWriteToCollaborators', function () { beforeEach(function (ctx) { - ctx.req.params = { Project_id: ctx.project._id } + ctx.req.params = { Project_id: ctx.project._id.toString() } }) describe('when there are collaborator slots available', function () { @@ -1175,11 +1184,11 @@ describe('TokenAccessController', function () { it('sets the privilege level to read and write for the invited viewer', function (ctx) { expect( ctx.TokenAccessHandler.promises.removeReadAndWriteUserFromProject - ).to.have.been.calledWith(ctx.user._id, ctx.project._id) + ).to.have.been.calledWith(ctx.user._id, ctx.project._id.toString()) expect( ctx.CollaboratorsHandler.promises.addUserIdToProject ).to.have.been.calledWith( - ctx.project._id, + ctx.project._id.toString(), undefined, ctx.user._id, PrivilegeLevels.READ_AND_WRITE @@ -1214,11 +1223,11 @@ describe('TokenAccessController', function () { it('sets the privilege level to read only for the invited viewer (pendingEditor)', function (ctx) { expect( ctx.TokenAccessHandler.promises.removeReadAndWriteUserFromProject - ).to.have.been.calledWith(ctx.user._id, ctx.project._id) + ).to.have.been.calledWith(ctx.user._id, ctx.project._id.toString()) expect( ctx.CollaboratorsHandler.promises.addUserIdToProject ).to.have.been.calledWith( - ctx.project._id, + ctx.project._id.toString(), undefined, ctx.user._id, PrivilegeLevels.READ_ONLY, @@ -1232,7 +1241,7 @@ describe('TokenAccessController', function () { describe('moveReadWriteToReadOnly', function () { beforeEach(function (ctx) { - ctx.req.params = { Project_id: ctx.project._id } + ctx.req.params = { Project_id: ctx.project._id.toString() } }) describe('previously joined token access user moving to anonymous viewer', function () { @@ -1250,7 +1259,7 @@ describe('TokenAccessController', function () { it('removes them from read write token access refs and adds them to read only token access refs', function (ctx) { expect( ctx.TokenAccessHandler.promises.moveReadAndWriteUserToReadOnly - ).to.have.been.calledWith(ctx.user._id, ctx.project._id) + ).to.have.been.calledWith(ctx.user._id, ctx.project._id.toString()) expect(ctx.res.sendStatus).toHaveBeenCalledWith(204) }) @@ -1258,7 +1267,7 @@ describe('TokenAccessController', function () { expect( ctx.ProjectAuditLogHandler.promises.addEntry ).to.have.been.calledWith( - ctx.project._id, + ctx.project._id.toString(), 'readonly-via-sharing-updates', ctx.user._id, ctx.req.ip @@ -1266,4 +1275,96 @@ describe('TokenAccessController', function () { }) }) }) + + describe('request validation', function () { + beforeEach(function () { + setReqValidationModeForTests('enforce') + }) + + it('rejects a grant read-and-write request body with an unrecognized field', async function (ctx) { + ctx.req.params = { token: ctx.token } + ctx.req.body = { confirmedByUser: true, extraField: 'nope' } + await new Promise(resolve => { + ctx.TokenAccessController.grantTokenAccessReadAndWrite( + ctx.req, + ctx.res, + err => { + expect(err).to.be.instanceof(InvalidRequestError) + resolve() + } + ) + }) + }) + + it('rejects a grant read-only request body with an unrecognized field', async function (ctx) { + ctx.req.params = { token: ctx.token } + ctx.req.body = { confirmedByUser: true, extraField: 'nope' } + await new Promise(resolve => { + ctx.TokenAccessController.grantTokenAccessReadOnly( + ctx.req, + ctx.res, + err => { + expect(err).to.be.instanceof(InvalidRequestError) + resolve() + } + ) + }) + }) + + it('rejects a malformed Project_id when checking sharing-updates consent eligibility', async function (ctx) { + ctx.req.params = { Project_id: 'not-an-object-id' } + await new Promise(resolve => { + ctx.TokenAccessController.ensureUserCanUseSharingUpdatesConsentPage( + ctx.req, + ctx.res, + err => { + expect(err).to.be.instanceof(InvalidParamsError) + resolve() + } + ) + }) + }) + + it('rejects a malformed Project_id on the sharing-updates consent page', async function (ctx) { + ctx.req.params = { Project_id: 'not-an-object-id' } + await new Promise(resolve => { + ctx.TokenAccessController.sharingUpdatesConsent( + ctx.req, + ctx.res, + err => { + expect(err).to.be.instanceof(InvalidParamsError) + resolve() + } + ) + }) + }) + + it('rejects a malformed Project_id when moving to collaborators', async function (ctx) { + ctx.req.params = { Project_id: 'not-an-object-id' } + await new Promise(resolve => { + ctx.TokenAccessController.moveReadWriteToCollaborators( + ctx.req, + ctx.res, + err => { + expect(err).to.be.instanceof(InvalidParamsError) + resolve() + } + ) + }) + }) + + it('rejects a malformed Project_id when moving to read-only', async function (ctx) { + ctx.req.params = { Project_id: 'not-an-object-id' } + await new Promise(resolve => { + ctx.TokenAccessController.moveReadWriteToReadOnly( + ctx.req, + ctx.res, + err => { + expect(err).to.be.instanceof(InvalidParamsError) + resolve() + } + ) + }) + }) + }) }) diff --git a/yarn.lock b/yarn.lock index 2e46c284af..1270c18561 100644 --- a/yarn.lock +++ b/yarn.lock @@ -26527,7 +26527,7 @@ __metadata: languageName: node linkType: hard -"passport-local@npm:^1.0.0": +"passport-local@npm:1.0.0": version: 1.0.0 resolution: "passport-local@npm:1.0.0" dependencies: @@ -26536,6 +26536,15 @@ __metadata: languageName: node linkType: hard +"passport-local@patch:passport-local@npm%3A1.0.0#~/.yarn/patches/passport-local-npm-1.0.0-ed89961a0c.patch": + version: 1.0.0 + resolution: "passport-local@patch:passport-local@npm%3A1.0.0#~/.yarn/patches/passport-local-npm-1.0.0-ed89961a0c.patch::version=1.0.0&hash=35e215" + dependencies: + passport-strategy: "npm:1.x.x" + checksum: 10c0/9b8275673fb4f0edb31a82553c9901413c857669dd9e75df26de0958ab3760391ae762fa7596cd28956510b45cb37c05eb8c95f07c75e8fe6e07a77b9a89e988 + languageName: node + linkType: hard + "passport-oauth2@npm:1.6.1": version: 1.6.1 resolution: "passport-oauth2@npm:1.6.1"