diff --git a/libraries/overleaf-editor-core/lib/schemas.js b/libraries/overleaf-editor-core/lib/schemas.js new file mode 100644 index 0000000000..495c0ad227 --- /dev/null +++ b/libraries/overleaf-editor-core/lib/schemas.js @@ -0,0 +1,89 @@ +// @ts-check +'use strict' + +const { z, zz } = require('@overleaf/validation-tools') + +const rawTrackingProps = z.strictObject({ + type: z.enum(['insert', 'delete']), + userId: zz.objectId(), + ts: z.iso.datetime(), +}) + +const rawClearTrackingProps = z.strictObject({ + type: z.literal('none'), +}) + +const rawInsertOp = z.union([ + z.strictObject({ + i: z.string(), + commentIds: z.array(zz.objectId()).optional(), + tracking: rawTrackingProps.optional(), + }), + z.string(), +]) + +const rawRemoveOp = z.number().int().max(-1) + +const rawRetainOp = z.union([ + z.strictObject({ + r: z.number().int().min(1), + commentIds: z.array(zz.objectId()).optional(), + tracking: rawTrackingProps.or(rawClearTrackingProps).optional(), + }), + z.number().int().min(1), +]) + +const rawScanOp = z.union([rawInsertOp, rawRemoveOp, rawRetainOp]) + +const rawTextOperation = z.strictObject({ + textOperation: z.array(rawScanOp).min(1), + contentHash: z.string().optional(), +}) + +const rawRange = z.strictObject({ + pos: z.number().int().min(0), + length: z.number().int().min(0), +}) + +const rawAddCommentOperation = z.strictObject({ + commentId: zz.objectId(), + ranges: z.array(rawRange).min(1), + resolved: z.boolean().optional(), +}) + +const rawSetCommentStateOperation = z.strictObject({ + commentId: zz.objectId(), + resolved: z.boolean(), +}) + +const rawDeleteCommentOperation = z.strictObject({ + deleteComment: zz.objectId(), +}) + +const rawEditNoOperation = z.strictObject({ + noOp: z.literal(true), +}) + +const rawEditOperation = z.union([ + rawTextOperation, + rawAddCommentOperation, + rawDeleteCommentOperation, + rawSetCommentStateOperation, + rawEditNoOperation, +]) + +module.exports = { + rawTrackingProps, + rawClearTrackingProps, + rawInsertOp, + rawRemoveOp, + rawRetainOp, + rawScanOp, + rawTextOperation, + rawRange, + rawAddCommentOperation, + rawSetCommentStateOperation, + rawDeleteCommentOperation, + rawEditNoOperation, + rawEditOperation, +} diff --git a/libraries/overleaf-editor-core/lib/types.ts b/libraries/overleaf-editor-core/lib/types.ts index 84db04c5cd..eb81c0dd23 100644 --- a/libraries/overleaf-editor-core/lib/types.ts +++ b/libraries/overleaf-editor-core/lib/types.ts @@ -1,6 +1,8 @@ -import Blob from './blob' -import TrackingProps from './file_data/tracking_props' -import ClearTrackingProps from './file_data/clear_tracking_props' +import type Blob from './blob' +import type TrackingProps from './file_data/tracking_props' +import type ClearTrackingProps from './file_data/clear_tracking_props' +import type { z } from '@overleaf/validation-tools' +import type * as schemas from './schemas' export type BlobStore = { getBlob(hash: string): Promise @@ -17,10 +19,7 @@ export type RangesBlob = { trackedChanges: TrackedChangeRawData[] } -export type RawRange = { - pos: number - length: number -} +export type RawRange = z.infer export type CommentRawData = { id: string @@ -33,15 +32,11 @@ export type TrackedChangeRawData = { tracking: TrackingPropsRawData } -export type TrackingPropsRawData = { - type: 'insert' | 'delete' - userId: string - ts: string -} +export type TrackingPropsRawData = z.infer -export type ClearTrackingPropsRawData = { - type: 'none' -} +export type ClearTrackingPropsRawData = z.infer< + typeof schemas.rawClearTrackingProps +> export type TrackingDirective = TrackingProps | ClearTrackingProps @@ -112,55 +107,32 @@ export type RawHollowStringFileData = { stringLength: number } export type RawV2DocVersions = Record -export type RawInsertOp = - | { - i: string - commentIds?: string[] - tracking?: TrackingPropsRawData - } - | string +export type RawInsertOp = z.infer -export type RawRemoveOp = number -export type RawRetainOp = - | { - r: number - commentIds?: string[] - tracking?: TrackingPropsRawData | ClearTrackingPropsRawData - } - | number +export type RawRemoveOp = z.infer +export type RawRetainOp = z.infer -export type RawScanOp = RawInsertOp | RawRemoveOp | RawRetainOp +export type RawScanOp = z.infer -export type RawTextOperation = { - textOperation: RawScanOp[] - contentHash?: string -} +export type RawTextOperation = z.infer -export type RawAddCommentOperation = { - commentId: string - ranges: RawRange[] - resolved?: boolean -} +export type RawAddCommentOperation = z.infer< + typeof schemas.rawAddCommentOperation +> -export type RawDeleteCommentOperation = { deleteComment: string } +export type RawDeleteCommentOperation = z.infer< + typeof schemas.rawDeleteCommentOperation +> -export type RawSetCommentStateOperation = { - commentId: string - resolved: boolean -} +export type RawSetCommentStateOperation = z.infer< + typeof schemas.rawSetCommentStateOperation +> -export type RawEditNoOperation = { - noOp: true -} +export type RawEditNoOperation = z.infer export type RawEditFileOperation = RawEditOperation & { pathname: string } -export type RawEditOperation = - | RawTextOperation - | RawAddCommentOperation - | RawDeleteCommentOperation - | RawSetCommentStateOperation - | RawEditNoOperation +export type RawEditOperation = z.infer export type LinkedFileData = { importedAt: string diff --git a/libraries/overleaf-editor-core/package.json b/libraries/overleaf-editor-core/package.json index 1d71610875..1ac4cec4cd 100644 --- a/libraries/overleaf-editor-core/package.json +++ b/libraries/overleaf-editor-core/package.json @@ -26,6 +26,7 @@ }, "dependencies": { "@overleaf/o-error": "workspace:*", + "@overleaf/validation-tools": "workspace:*", "check-types": "^5.1.0", "lodash": "^4.18.1", "p-map": "^4.0.0", diff --git a/services/history-v1/api/schema.js b/services/history-v1/api/schema.js index a150a16705..b462373a02 100644 --- a/services/history-v1/api/schema.js +++ b/services/history-v1/api/schema.js @@ -39,6 +39,8 @@ const operationSchema = z hash: z.string(), }) .optional(), + // TODO: Use rawEditOperation from overleaf-editor-core/lib/schemas.js + // instead textOperation: z.array(z.any()).optional(), file: fileSchema.optional(), contentHash: z.string().optional(), diff --git a/services/real-time/app/js/Router.js b/services/real-time/app/js/Router.js index 7d68a6911a..b4c671641b 100644 --- a/services/real-time/app/js/Router.js +++ b/services/real-time/app/js/Router.js @@ -11,6 +11,7 @@ import Errors from './Errors.js' import { z, zz } from '@overleaf/validation-tools' import { isZodErrorLike } from 'zod-validation-error' import os from 'node:os' +import schemas from 'overleaf-editor-core/lib/schemas.js' const { UnexpectedArgumentsError } = Errors @@ -60,7 +61,8 @@ const applyOtUpdateSchema = z.object({ }) ) ) - .min(1), + .min(1) + .or(z.array(schemas.rawEditOperation).min(1)), v: z.number().int().min(0), }), }) diff --git a/services/real-time/app/js/WebsocketController.js b/services/real-time/app/js/WebsocketController.js index 83d7beb34b..07871d7413 100644 --- a/services/real-time/app/js/WebsocketController.js +++ b/services/real-time/app/js/WebsocketController.js @@ -8,6 +8,7 @@ import ConnectedUsersManager from './ConnectedUsersManager.js' import WebsocketLoadBalancer from './WebsocketLoadBalancer.js' import RoomManager from './RoomManager.js' import Errors from './Errors.js' +import { EditOperationBuilder } from 'overleaf-editor-core' const { CodedError, @@ -658,14 +659,42 @@ export default WebsocketController = { ) }, + _isHistoryOTUpdate(update) { + for (const op of update.op) { + if (!EditOperationBuilder.isValid(op)) { + return false + } + } + + return true + }, + _assertClientCanApplyUpdate(client, docId, update, callback) { - if (WebsocketController._isCommentUpdate(update)) { + if (WebsocketController._isHistoryOTUpdate(update)) { + return WebsocketController._assertClientCanApplyHistoryUpdate( + client, + docId, + update, + callback + ) + } else { + return WebsocketController._assertClientCanApplyShareJsUpdate( + client, + docId, + update, + callback + ) + } + }, + + _assertClientCanApplyShareJsUpdate(client, docId, update, callback) { + if (WebsocketController._isShareJsCommentUpdate(update)) { return AuthorizationManager.assertClientCanViewProjectAndDoc( client, docId, callback ) - } else if (update.meta?.tc) { + } else if (WebsocketController._isShareJsTrackedChangesUpdate(update)) { return AuthorizationManager.assertClientCanReviewProjectAndDoc( client, docId, @@ -680,10 +709,7 @@ export default WebsocketController = { } }, - _isCommentUpdate(update) { - if (!(update && update.op instanceof Array)) { - return false - } + _isShareJsCommentUpdate(update) { for (const op of update.op) { if (!op.c) { return false @@ -691,4 +717,73 @@ export default WebsocketController = { } return true }, + + _isShareJsTrackedChangesUpdate(update) { + return Boolean(update.meta?.tc) + }, + + _assertClientCanApplyHistoryUpdate(client, docId, update, callback) { + if (WebsocketController._isHistoryOTCommentUpdate(update)) { + return AuthorizationManager.assertClientCanViewProjectAndDoc( + client, + docId, + callback + ) + } else if (WebsocketController._isHistoryOTTrackedChangesUpdate(update)) { + return AuthorizationManager.assertClientCanReviewProjectAndDoc( + client, + docId, + callback + ) + } else { + return AuthorizationManager.assertClientCanEditProjectAndDoc( + client, + docId, + callback + ) + } + }, + + _isHistoryOTCommentUpdate(update) { + for (const op of update.op) { + if (!WebsocketController._isHistoryOTAddCommentOperation(op)) { + return false + } + } + return true + }, + + _isHistoryOTAddCommentOperation(op) { + return typeof op === 'object' && 'commentId' in op && 'ranges' in op + }, + + _isHistoryOTTrackedChangesUpdate(update) { + for (const op of update.op) { + if (!('textOperation' in op)) { + return false + } + for (const scanOp of op.textOperation) { + // Plain retains + if (typeof scanOp === 'number' && scanOp > 0) { + continue + } + // Plain insertions + if (typeof scanOp !== 'object') { + return false + } + // Tracked insertions + if ('i' in scanOp && 'tracking' in scanOp) { + continue + } + // Tracked ranges (we don't accepting by clearing tracking) + if ('r' in scanOp && scanOp.tracking?.type !== 'none') { + continue + } + + return false + } + } + + return true + }, } diff --git a/services/real-time/package.json b/services/real-time/package.json index e80c8d23d3..531c54e829 100644 --- a/services/real-time/package.json +++ b/services/real-time/package.json @@ -32,6 +32,7 @@ "express": "4.22.1", "express-session": "^1.17.1", "lodash": "^4.18.1", + "overleaf-editor-core": "workspace:*", "proxy-addr": "^2.0.7", "socket.io": "github:overleaf/socket.io#0.9.19-overleaf-12", "socket.io-client": "https://github.com/overleaf/socket.io-client/archive/refs/tags/0.9.17-overleaf-6.tar.gz", diff --git a/services/real-time/test/unit/js/WebsocketController.test.js b/services/real-time/test/unit/js/WebsocketController.test.js index ea0926a55c..b4c449caae 100644 --- a/services/real-time/test/unit/js/WebsocketController.test.js +++ b/services/real-time/test/unit/js/WebsocketController.test.js @@ -1788,5 +1788,107 @@ describe('WebsocketController', function () { }) }) }) + + describe('with a history-OT client', function () { + const tracking = { + insert: { type: 'insert', userId: 'user-id-123', ts: '2026-07-15' }, + delete: { type: 'delete', userId: 'user-id-123', ts: '2026-07-15' }, + none: { type: 'none' }, + } + + function expectRoutedTo(ctx, target, update) { + return new Promise(resolve => { + for (const name of [ + 'assertClientCanViewProjectAndDoc', + 'assertClientCanReviewProjectAndDoc', + 'assertClientCanEditProjectAndDoc', + ]) { + ctx.AuthorizationManager[name].yields(new Error('not authorized')) + } + ctx.AuthorizationManager[target].yields(null) + ctx.WebsocketController._assertClientCanApplyUpdate( + ctx.client, + ctx.doc_id, + update, + error => { + expect(error).to.be.null + expect(ctx.AuthorizationManager[target].calledOnce).to.be.true + resolve() + } + ) + }) + } + + it('should route an add-comment op to view access', async function (ctx) { + await expectRoutedTo(ctx, 'assertClientCanViewProjectAndDoc', { + op: [{ commentId: 'comment-id', ranges: [{ pos: 0, length: 3 }] }], + }) + }) + + it('should route a tracked insertion to review access', async function (ctx) { + await expectRoutedTo(ctx, 'assertClientCanReviewProjectAndDoc', { + op: [{ textOperation: [5, { i: 'x', tracking: tracking.insert }] }], + }) + }) + + it('should route a tracked deletion to review access', async function (ctx) { + await expectRoutedTo(ctx, 'assertClientCanReviewProjectAndDoc', { + op: [{ textOperation: [{ r: 3, tracking: tracking.delete }] }], + }) + }) + + it('should route accepting a tracked change (clearing tracking) to edit access', async function (ctx) { + await expectRoutedTo(ctx, 'assertClientCanEditProjectAndDoc', { + op: [{ textOperation: [{ r: 3, tracking: tracking.none }] }], + }) + }) + + it('should route an untracked insertion to edit access', async function (ctx) { + await expectRoutedTo(ctx, 'assertClientCanEditProjectAndDoc', { + op: [{ textOperation: ['plain'] }], + }) + }) + + it('should route an untracked deletion to edit access', async function (ctx) { + await expectRoutedTo(ctx, 'assertClientCanEditProjectAndDoc', { + op: [{ textOperation: [-3] }], + }) + }) + + it('should route a mix of tracked and untracked ops to edit access', async function (ctx) { + await expectRoutedTo(ctx, 'assertClientCanEditProjectAndDoc', { + op: [ + { textOperation: [{ i: 'x', tracking: tracking.insert }] }, + { textOperation: ['plain'] }, + ], + }) + }) + + it('should route a resolve-comment op to edit access', async function (ctx) { + // Only add-comment ops (commentId + ranges) are recognised as comment + // updates; resolving a comment currently requires edit access. + await expectRoutedTo(ctx, 'assertClientCanEditProjectAndDoc', { + op: [{ commentId: 'comment-id', resolved: true }], + }) + }) + + it('should route a permanent history-OT insertion carrying meta.tc to edit access', async function (ctx) { + // meta.tc is a legacy (sharejs) tracked-changes id seed and is + // meaningless for history-OT, where tracking is carried by the op. + // A review-only client must not be able to make a permanent edit by + // attaching it. + await expectRoutedTo(ctx, 'assertClientCanEditProjectAndDoc', { + op: [{ textOperation: ['permanent untracked text'] }], + meta: { tc: 'tracked-change-id' }, + }) + }) + + it('should route a permanent history-OT deletion carrying meta.tc to edit access', async function (ctx) { + await expectRoutedTo(ctx, 'assertClientCanEditProjectAndDoc', { + op: [{ textOperation: [-3] }], + meta: { tc: 'tracked-change-id' }, + }) + }) + }) }) }) diff --git a/services/web/frontend/js/features/ide-react/editor/share-js-doc.ts b/services/web/frontend/js/features/ide-react/editor/share-js-doc.ts index 9116062eb0..d101c4c786 100644 --- a/services/web/frontend/js/features/ide-react/editor/share-js-doc.ts +++ b/services/web/frontend/js/features/ide-react/editor/share-js-doc.ts @@ -24,7 +24,7 @@ import { EditOperationBuilder, CommentList, } from 'overleaf-editor-core' -import { +import type { StringFileRawData, RawEditOperation, } from 'overleaf-editor-core/lib/types' diff --git a/services/web/frontend/js/features/ide-react/editor/types/document.ts b/services/web/frontend/js/features/ide-react/editor/types/document.ts index f6e5f6aebb..a2a6bac3a4 100644 --- a/services/web/frontend/js/features/ide-react/editor/types/document.ts +++ b/services/web/frontend/js/features/ide-react/editor/types/document.ts @@ -1,6 +1,6 @@ import { StringFileData } from 'overleaf-editor-core' import { AnyOperation } from '../../../../../../types/change' -import { RawEditOperation } from 'overleaf-editor-core/lib/types' +import type { RawEditOperation } from 'overleaf-editor-core/lib/types' export type Version = number diff --git a/services/web/frontend/js/infrastructure/project-snapshot.ts b/services/web/frontend/js/infrastructure/project-snapshot.ts index 55ac882cd3..2db8d34dd1 100644 --- a/services/web/frontend/js/infrastructure/project-snapshot.ts +++ b/services/web/frontend/js/infrastructure/project-snapshot.ts @@ -1,6 +1,6 @@ import pLimit from 'p-limit' import { Change, Chunk, Snapshot, File } from 'overleaf-editor-core' -import { RawChange, RawChunk } from 'overleaf-editor-core/lib/types' +import type { RawChange, RawChunk } from 'overleaf-editor-core/lib/types' import { FetchError, getJSON, postJSON } from '@/infrastructure/fetch-json' import path from 'path-browserify' diff --git a/yarn.lock b/yarn.lock index 3a046249a1..590d2b64d4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7187,6 +7187,7 @@ __metadata: mocha: "npm:^11.1.0" mocha-junit-reporter: "npm:^2.2.1" mocha-multi-reporters: "npm:^1.5.1" + overleaf-editor-core: "workspace:*" proxy-addr: "npm:^2.0.7" sinon: "npm:^9.2.4" sinon-chai: "npm:3.7.0" @@ -25255,6 +25256,7 @@ __metadata: resolution: "overleaf-editor-core@workspace:libraries/overleaf-editor-core" dependencies: "@overleaf/o-error": "workspace:*" + "@overleaf/validation-tools": "workspace:*" "@types/check-types": "npm:^7.3.7" "@types/path-browserify": "npm:^1.0.3" chai: "npm:^3.3.0"