diff --git a/backend/src/config/env.js b/backend/src/config/env.js index 3496f354..d8385610 100644 --- a/backend/src/config/env.js +++ b/backend/src/config/env.js @@ -101,6 +101,7 @@ module.exports = { ONLYOFFICE_LANG: process.env.ONLYOFFICE_LANG?.trim() || 'en', ONLYOFFICE_FORCE_SAVE: normalizeBoolean(process.env.ONLYOFFICE_FORCE_SAVE) || false, ONLYOFFICE_FILE_EXTENSIONS: process.env.ONLYOFFICE_FILE_EXTENSIONS || '', + ONLYOFFICE_DOWNLOAD_ORIGINS: process.env.ONLYOFFICE_DOWNLOAD_ORIGINS || '', // Collabora (WOPI) COLLABORA_URL: process.env.COLLABORA_URL?.trim() || null, diff --git a/backend/src/config/index.js b/backend/src/config/index.js index e06df8fa..703e6f66 100644 --- a/backend/src/config/index.js +++ b/backend/src/config/index.js @@ -164,7 +164,7 @@ if (env.PUBLIC_URL) { // They are considered valid so accessing the app that way doesn't raise the // public-URL mismatch warning, and they're accepted by CORS. PUBLIC_URL remains // the canonical URL used to build absolute links (shares, OIDC callbacks, WOPI). -const parseOriginList = (value) => +const parseOriginList = (value, variableName = 'INTERNAL_URL') => (typeof value === 'string' ? value.split(',') : []) .map((entry) => entry.trim()) .filter(Boolean) @@ -172,7 +172,7 @@ const parseOriginList = (value) => try { return new URL(entry).origin; } catch (err) { - console.warn(`[Config] Invalid INTERNAL_URL entry: ${entry}`); + console.warn(`[Config] Invalid ${variableName} entry: ${entry}`); return null; } }) @@ -323,6 +323,10 @@ const onlyoffice = { extensions: env.ONLYOFFICE_FILE_EXTENSIONS.split(',') .map((s) => s.trim().toLowerCase()) .filter(Boolean), + // Where a saved document may be fetched from, beyond the Document Server's + // own address: it sometimes reports itself under another host than the one it + // is called on, behind a proxy or inside a container network. + downloadOrigins: parseOriginList(env.ONLYOFFICE_DOWNLOAD_ORIGINS, 'ONLYOFFICE_DOWNLOAD_ORIGINS'), }; // Silent JWT mismatches surface to the user as "Document security token is not @@ -335,6 +339,16 @@ if (onlyoffice.serverUrl && !env.ONLYOFFICE_SECRET) { ); } +// --- Thumbnail access --- +// Thumbnails are served from /static, outside the authentication middleware, so +// the URL has to carry its own proof that somebody was cleared to see it. +// Derived from the session secret, so it lasts as long as that does; when even +// that could not be stored, a restart only means already-loaded pages fetch +// their thumbnails again through the API, which re-runs the access check. +const thumbnailAccess = { + secret: deriveSecret('thumbnails'), +}; + // --- Collabora (WOPI) --- const collaboraBaseUrl = env.COLLABORA_URL?.replace(/\/$/, '') || null; const collaboraDiscoveryUrl = @@ -633,6 +647,7 @@ module.exports = { }, thumbnails: { size: 200, quality: 70 }, + thumbnailAccess, uploads, onlyoffice, collabora, diff --git a/backend/src/routes/collabora.js b/backend/src/routes/collabora.js index f806803e..28ec3e7e 100644 --- a/backend/src/routes/collabora.js +++ b/backend/src/routes/collabora.js @@ -13,8 +13,15 @@ const asyncHandler = require('../utils/asyncHandler'); const logger = require('../utils/logger'); const { getDiscoveryActionsByExt } = require('../services/collaboraDiscoveryService'); const lockService = require('../services/wopiLockService'); +const versions = require('../services/versions/operations'); const { ValidationError, UnauthorizedError, ForbiddenError } = require('../errors/AppError'); +/** A Collabora save header, under its current name or the one older servers still send. */ +const wopiSaveHeader = (req, name) => + String(req.headers[`x-cool-wopi-${name}`] ?? req.headers[`x-lool-wopi-${name}`] ?? '') + .trim() + .toLowerCase(); + const router = express.Router(); const toExt = (filename = '') => { @@ -282,18 +289,33 @@ router.post( return res.status(409).end(); } - const dir = path.dirname(abs); - const tmp = path.join(dir, `.${path.basename(abs)}.wopi-tmp-${process.pid}-${Date.now()}`); - - const writeStream = fs.createWriteStream(tmp); - await new Promise((resolve, reject) => { - req.pipe(writeStream); - writeStream.on('finish', resolve); - writeStream.on('error', reject); - req.on('error', reject); - }); - - await fsp.rename(tmp, abs); + await versions.saveFile( + abs, + (temporaryPath) => + new Promise((resolve, reject) => { + const writeStream = fs.createWriteStream(temporaryPath, { flags: 'wx' }); + req.pipe(writeStream); + writeStream.on('finish', resolve); + writeStream.on('error', reject); + req.on('error', reject); + }), + { + purpose: 'wopi', + author: { id: tokenPayload.userId || null, label: tokenPayload.userName || null }, + source: 'collabora', + session: { + // One lock per open document, held by everyone editing it together: + // the session its saves belong to. Without one every save would stand + // as a state of its own. + key: requestLock ? `wopi:${requestLock}` : null, + startedAt: Number.isFinite(tokenPayload.iat) ? tokenPayload.iat * 1000 : null, + }, + // Collabora saves on its own every few minutes; the ones somebody asked + // for, and the one made on closing the document, are states worth + // keeping. + explicit: wopiSaveHeader(req, 'isautosave') !== 'true', + } + ); const stat = await fsp.stat(abs); res.setHeader('Cache-Control', 'no-store'); diff --git a/backend/src/routes/onlyoffice.js b/backend/src/routes/onlyoffice.js index 5566d866..8c88b9dc 100644 --- a/backend/src/routes/onlyoffice.js +++ b/backend/src/routes/onlyoffice.js @@ -3,6 +3,7 @@ const path = require('path'); const fs = require('fs'); const fsp = require('fs/promises'); const crypto = require('crypto'); +const { pipeline } = require('stream/promises'); const axios = require('axios'); const jwt = require('jsonwebtoken'); @@ -10,6 +11,7 @@ const { onlyoffice, public: publicConfig, mimeTypes } = require('../config/index const { normalizeRelativePath } = require('../utils/pathUtils'); const { ensureDir } = require('../utils/fsUtils'); const { resolvePathWithAccess } = require('../services/accessManager'); +const versions = require('../services/versions/operations'); const logger = require('../utils/logger'); const asyncHandler = require('../utils/asyncHandler'); const { ValidationError, UnauthorizedError, ForbiddenError } = require('../errors/AppError'); @@ -33,6 +35,89 @@ const getDocumentType = (ext) => { const resolveMime = (ext) => mimeTypes[ext] || 'application/octet-stream'; +/** + * The addresses a saved document may be fetched from. + * + * The Document Server's own, and any declared beside it: behind a proxy or + * inside a container network it sometimes reports itself under a host other + * than the one it is called on. + */ +const buildAllowedDownloadOrigins = () => { + const origins = new Set(); + const add = (value) => { + if (!value) return; + try { + origins.add(new URL(value).origin); + } catch { + // Ignore malformed configuration entries. + } + }; + add(onlyoffice.serverUrl); + (onlyoffice.downloadOrigins || []).forEach(add); + return origins; +}; + +/** + * The callback says where to fetch the saved document from, and the server + * fetched whatever it was told to: an address on the machine itself, or inside + * the network the container sits in, reached by anyone who can reach the + * callback. It has to come from the Document Server we sent the document to. + */ +const ensureAllowedDownloadUrl = (rawUrl) => { + let parsed; + try { + parsed = new URL(String(rawUrl)); + } catch { + throw new ValidationError('The document URL is not a valid URL.'); + } + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { + throw new ValidationError('The document URL must use HTTP or HTTPS.'); + } + const allowed = buildAllowedDownloadOrigins(); + if (!allowed.has(parsed.origin)) { + logger.warn( + { origin: parsed.origin, allowed: Array.from(allowed) }, + 'ONLYOFFICE callback rejected: document URL origin is not allowed. Add it to ONLYOFFICE_DOWNLOAD_ORIGINS if the Document Server reports a different host.' + ); + throw new ForbiddenError('The document URL does not come from the configured Document Server.'); + } + return parsed.toString(); +}; + +/** + * Pull the saved document into a file of its own, next to the document. + * + * Never into the document itself: it used to be truncated to nothing before + * the Document Server had answered, so a slow network, a restart or a refused + * download left an empty file where the work had been. The versions place the + * temporary file over the document once it is whole. + */ +const fetchDocumentInto = async (downloadUrl, temporaryPath, mode) => { + const response = await axios.get(downloadUrl, { responseType: 'stream', timeout: 30000 }); + await pipeline(response.data, fs.createWriteStream(temporaryPath, { flags: 'wx', mode })); + // The write stream's mode is subject to the umask; this is not. + await fsp.chmod(temporaryPath, mode); +}; + +/** + * Who a callback is saving for. + * + * The Document Server names the people whose changes are in this save; the + * last of them is the one to credit. It says nothing when a save carries no + * history — the first one of a session — and then the account the editing + * session was opened for is the answer. Somebody who came through a share link + * is credited as the link: there is no account to name. + */ +const authorFromCallback = (body, backendCtx) => { + const changes = Array.isArray(body?.history?.changes) ? body.history.changes : []; + const user = changes.length ? changes[changes.length - 1]?.user : null; + const id = user?.id ? String(user.id) : backendCtx?.userId || null; + if ((id && id.startsWith('guest_')) || (!id && backendCtx?.guestSessionId)) { + return { id: null, label: 'share-link' }; + } + return { id, label: user?.name ? String(user.name) : null }; +}; + const getDsJwtFromReq = (req) => { const auth = (req.headers['authorization'] || req.headers['authorizationjwt'] || '').toString(); if (auth.toLowerCase().startsWith('bearer ')) { @@ -294,6 +379,8 @@ router.post( const status = Number(body.status); // See ONLYOFFICE callback statuses: 2 - Save, 6 - Force Save if ((status === 2 || status === 6) && body.url) { + // Only the Document Server we handed the document to may be fetched from. + const downloadUrl = ensureAllowedDownloadUrl(body.url); let abs = null; if (backendCtx && typeof backendCtx.absolutePath === 'string' && backendCtx.absolutePath) { abs = backendCtx.absolutePath; @@ -308,16 +395,41 @@ router.post( abs = resolved.absolutePath; } await ensureDir(path.dirname(abs)); - // Download updated file from Document Server - const response = await axios.get(body.url, { responseType: 'stream' }); - await fsp.writeFile(abs, Buffer.from([])); // ensure file exists / truncate - const writeStream = fs.createWriteStream(abs); - await new Promise((resolve, reject) => { - response.data.pipe(writeStream); - writeStream.on('finish', resolve); - writeStream.on('error', reject); - }); - logger.debug({ path: relativePath }, 'ONLYOFFICE file updated'); + + // Keep the permissions the document already had; one the editor is + // creating starts private. + let mode = 0o600; + try { + const previous = await fsp.stat(abs); + if (previous.isFile()) mode = previous.mode & 0o777; + } catch { + // A document that is not there yet has nothing to keep. + } + + // A save on purpose — the editor's own Save, or the last one made once + // everybody has left the document — is a state worth keeping. The + // automatic saves in between are not, beyond the checkpoint the + // versions take of a session that runs long. + const explicit = status === 2 || Number(body.forcesavetype) === 1; + + await versions.saveFile( + abs, + (temporaryPath) => fetchDocumentInto(downloadUrl, temporaryPath, mode), + { + purpose: 'onlyoffice', + author: authorFromCallback(body, backendCtx), + source: 'onlyoffice', + session: { + // Everybody editing together shares the document key: it is the + // session, and its saves belong to it rather than each standing + // as a state of its own. + key: typeof body.key === 'string' && body.key ? body.key : null, + startedAt: Number.isFinite(backendCtx?.iat) ? backendCtx.iat * 1000 : null, + }, + explicit, + } + ); + logger.debug({ path: relativePath, status }, 'ONLYOFFICE file updated'); // MUST return {error:0} according to ONLYOFFICE spec return res.json({ error: 0 }); } diff --git a/backend/src/routes/thumbnails.js b/backend/src/routes/thumbnails.js index 5d12879a..dd0af824 100644 --- a/backend/src/routes/thumbnails.js +++ b/backend/src/routes/thumbnails.js @@ -6,6 +6,7 @@ const { normalizeRelativePath } = require('../utils/pathUtils'); const { extensions } = require('../config/index'); const { getThumbnail } = require('../services/thumbnailService'); const { resolvePathWithAccess } = require('../services/accessManager'); +const { withThumbnailToken } = require('../utils/thumbnailTokens'); const logger = require('../utils/logger'); const asyncHandler = require('../utils/asyncHandler'); const { ValidationError, NotFoundError } = require('../errors/AppError'); @@ -77,6 +78,9 @@ router.get( throw new ValidationError('Thumbnails are not available for this file type.'); } + // The check above is the only one this thumbnail will get: the picture + // itself is served from /static, outside the authentication middleware. The + // token carries that decision to the handler there. let thumbnail = ''; try { thumbnail = await getThumbnail(absolutePath); @@ -96,7 +100,7 @@ router.get( return res.json({ thumbnail: previewUrl }); } - res.json({ thumbnail: thumbnail || '' }); + res.json({ thumbnail: withThumbnailToken(thumbnail || '') }); }) ); diff --git a/backend/src/utils/staticServer.js b/backend/src/utils/staticServer.js index 5522578b..d8978bcd 100644 --- a/backend/src/utils/staticServer.js +++ b/backend/src/utils/staticServer.js @@ -1,15 +1,51 @@ const path = require('path'); const fs = require('fs'); const express = require('express'); -const { directories } = require('../config/index'); +const { auth, directories } = require('../config/index'); const logger = require('./logger'); +/** + * A thumbnail is served from /static, which the authentication middleware does + * not cover, and its cache name is derived from the file's path — so anybody + * who can guess a path can ask for the picture of it, and a 200 against a 404 + * answers "does this file exist" besides. + * + * A session would not settle it either: it says who is asking, not what they + * were cleared to see, so a visitor holding a valid session for one share could + * name a thumbnail belonging to another share or to a private folder. + * + * The decision is made by /api/thumbnails, which runs the real access check and + * signs the one filename it just cleared. This reads that signature back — no + * database, no session, nothing else to get wrong. + */ +const requireThumbnailToken = (req, res, next) => { + if (auth.enabled === false) return next(); + + // The cache is flat, so a request names one file and nothing else. Taking the + // basename would let a token for "x.webp" unlock "sub/dir/x.webp". + let filename; + try { + filename = decodeURIComponent((req.path || '').replace(/^\/+/, '')); + } catch { + // A malformed escape throws, and matches no thumbnail either way. + return res.status(401).end(); + } + const token = typeof req.query?.t === 'string' ? req.query.t : ''; + + // eslint-disable-next-line global-require + const { verifyThumbnailToken } = require('./thumbnailTokens'); + if (filename && !filename.includes('/') && verifyThumbnailToken(filename, token)) return next(); + + logger.debug({ filename }, 'Thumbnail request without a valid token'); + return res.status(401).end(); +}; + /** * Configures static file serving for thumbnails, logos, and frontend */ const configureStaticFiles = (app) => { // Serve thumbnails - app.use('/static/thumbnails', express.static(directories.thumbnails)); + app.use('/static/thumbnails', requireThumbnailToken, express.static(directories.thumbnails)); logger.debug('Mounted /static/thumbnails'); // Serve custom logos diff --git a/backend/src/utils/thumbnailTokens.js b/backend/src/utils/thumbnailTokens.js new file mode 100644 index 00000000..a7eda2f3 --- /dev/null +++ b/backend/src/utils/thumbnailTokens.js @@ -0,0 +1,68 @@ +const crypto = require('crypto'); + +const { thumbnailAccess } = require('../config/index'); + +/** + * Thumbnails are served from /static, outside the auth middleware, so the URL + * has to carry its own proof of access. + * + * A session cookie could not do that job: it says who the caller is, not what + * they were allowed to see. A share visitor with a valid session for share A + * could ask for any cached filename, including one belonging to share B or to + * a private folder — the cache name is derived from the path, so it is + * guessable. This token is minted only by /api/thumbnails, which runs the full + * access check first, and it names the one file it unlocks. + */ +// Matches the guest session lifetime: a page left open longer than this +// refetches its thumbnails through /api, which re-runs the access check. +const TTL_MS = 24 * 60 * 60 * 1000; + +const sign = (filename, expiresAt) => + crypto + .createHmac('sha256', thumbnailAccess.secret) + .update(`${filename}:${expiresAt}`) + .digest('base64url'); + +const createThumbnailToken = (filename, now = Date.now()) => { + const expiresAt = now + TTL_MS; + return `${expiresAt}.${sign(filename, expiresAt)}`; +}; + +/** + * @returns {boolean} true when the token was issued for this exact filename + * and has not expired. + */ +const verifyThumbnailToken = (filename, token, now = Date.now()) => { + if (typeof filename !== 'string' || typeof token !== 'string') return false; + + const separator = token.indexOf('.'); + if (separator <= 0) return false; + + const expiresAt = Number(token.slice(0, separator)); + if (!Number.isFinite(expiresAt) || expiresAt < now) return false; + + const provided = Buffer.from(token.slice(separator + 1), 'utf8'); + const expected = Buffer.from(sign(filename, expiresAt), 'utf8'); + if (provided.length !== expected.length) return false; + + return crypto.timingSafeEqual(provided, expected); +}; + +/** + * Append the token to a /static/thumbnails URL produced by the service layer. + * Anything that is not such a URL (an empty string, a /api/preview fallback) + * is returned untouched. + */ +const withThumbnailToken = (url) => { + if (typeof url !== 'string' || !url.startsWith('/static/thumbnails/')) return url; + const filename = url.slice('/static/thumbnails/'.length); + if (!filename || filename.includes('/') || filename.includes('?')) return url; + return `${url}?t=${createThumbnailToken(filename)}`; +}; + +module.exports = { + TTL_MS, + createThumbnailToken, + verifyThumbnailToken, + withThumbnailToken, +}; diff --git a/backend/tests/routes/collabora-save.test.js b/backend/tests/routes/collabora-save.test.js new file mode 100644 index 00000000..65530917 --- /dev/null +++ b/backend/tests/routes/collabora-save.test.js @@ -0,0 +1,162 @@ +import fs from 'node:fs/promises'; +import path from 'node:path'; +import express from 'express'; +import jwt from 'jsonwebtoken'; +import request from 'supertest'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { modulePath, setupTestEnv } from '../helpers/env-test-utils.js'; + +/** + * A save made through Collabora. + * + * Collabora hands the document over itself, so nothing here could destroy a + * file the way a download that stopped arriving could. What was missing is the + * other half: the state each save replaced was dropped, so a document edited + * all afternoon had no history at all — while the same document opened in the + * text editor did. + * + * The saves now go through the versions, which keep what they replace. The + * editor saves on its own every few minutes, and those automatic saves belong + * to the session rather than standing as states of their own: the lock every + * co-editor holds is what says which session a save belongs to. + */ + +const SECRET = 'collabora-save-secret'; +const DOCUMENT = 'Projects/report.docx'; +const FILE_ID = 'file-1'; + +let env; +let app; +let alice; + +const load = (relative) => require(modulePath(relative)); + +const absolute = () => path.join(env.volumeDir, ...DOCUMENT.split('/')); + +beforeEach(async () => { + env = await setupTestEnv({ + tag: 'collabora-save-', + env: { + PUBLIC_URL: 'https://files.example.com', + COLLABORA_URL: 'https://collabora.example.com', + COLLABORA_SECRET: SECRET, + }, + }); + + alice = await load('src/services/users').createLocalUser({ + email: 'alice@example.com', + username: 'alice', + displayName: 'Alice', + password: 'secret123', + roles: ['user'], + }); + + await fs.mkdir(path.dirname(absolute()), { recursive: true }); + await fs.writeFile(absolute(), 'first'); + + app = express(); + app.use('/api', load('src/routes/collabora')); + app.use(load('src/middleware/errorHandler').errorHandler); +}); + +afterEach(async () => { + load('src/services/trash/maintenance').stop(); + await env.cleanup(); +}); + +const accessToken = () => + jwt.sign( + { + fileId: FILE_ID, + absolutePath: absolute(), + canWrite: true, + userId: alice.id, + userName: 'Alice', + }, + SECRET, + { algorithm: 'HS256', expiresIn: 60 } + ); + +/** As Collabora saves: the whole document in the body, and how it came to save. */ +const put = (content, { autosave = false, lock = null } = {}) => { + const call = request(app) + .post(`/api/collabora/wopi/files/${FILE_ID}/contents`) + .query({ access_token: accessToken() }) + .set('Content-Type', 'application/octet-stream'); + if (autosave) call.set('X-COOL-WOPI-IsAutosave', 'true'); + if (lock) call.set('X-WOPI-Lock', lock); + return call.send(Buffer.from(content)); +}; + +const historyOf = async () => { + const db = await load('src/services/db').getDb(); + const store = load('src/services/versions/store'); + const file = store.listFiles(db)[0] || null; + return { + file, + versions: file ? store.listVersionsOfFile(db, file.id) : [], + }; +}; + +const contentOf = async (version) => { + const operations = load('src/services/versions/operations'); + const located = await operations.locateVersion(version.id); + return fs.readFile(located.absolutePath, 'utf8'); +}; + +describe('a document saved from Collabora', () => { + it('is written, and what it replaced is kept', async () => { + const response = await put('second'); + + expect(response.status).toBe(200); + expect(await fs.readFile(absolute(), 'utf8')).toBe('second'); + + const { versions } = await historyOf(); + expect(versions).toHaveLength(1); + expect(await contentOf(versions[0])).toBe('first'); + }); + + it('records who saved it, and through which editor', async () => { + await put('second'); + + const { file } = await historyOf(); + expect(file.currentSource).toBe('collabora'); + expect(file.currentAuthorId).toBe(alice.id); + expect(file.currentAuthorLabel).toBe('Alice'); + }); + + /** + * The editor's own timer, rather than somebody asking: a state on the way to + * the next one. What keeps it from filling the history is the session, and + * the session is the lock every co-editor of the document holds. + */ + it('marks a save the editor made by itself, under the session that made it', async () => { + await put('second', { autosave: true, lock: 'lock-abc' }); + + const { file } = await historyOf(); + expect(file.currentExplicit).toBe(false); + expect(file.currentSession).toBe('wopi:lock-abc'); + }); + + it('marks a save somebody asked for as one', async () => { + await put('second', { lock: 'lock-abc' }); + + const { file } = await historyOf(); + expect(file.currentExplicit).toBe(true); + }); + + /** + * Saved on purpose, then saved again by the timer: the state somebody chose + * is kept, rather than being swallowed by the automatic save that follows it. + */ + it('keeps the state somebody asked for when the timer saves over it', async () => { + await put('asked for', { lock: 'lock-abc' }); + await put('by the timer', { autosave: true, lock: 'lock-abc' }); + + const { versions } = await historyOf(); + const contents = await Promise.all(versions.map(contentOf)); + expect(contents).toContain('asked for'); + expect(await fs.readFile(absolute(), 'utf8')).toBe('by the timer'); + }); +}); diff --git a/backend/tests/routes/onlyoffice-save.test.js b/backend/tests/routes/onlyoffice-save.test.js new file mode 100644 index 00000000..7ca9b6a7 --- /dev/null +++ b/backend/tests/routes/onlyoffice-save.test.js @@ -0,0 +1,276 @@ +import fs from 'node:fs/promises'; +import http from 'node:http'; +import path from 'node:path'; +import express from 'express'; +import jwt from 'jsonwebtoken'; +import request from 'supertest'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { modulePath, setupTestEnv } from '../helpers/env-test-utils.js'; + +/** + * A save made through ONLYOFFICE. + * + * The Document Server does not send the document: it sends a URL to fetch it + * from, and the answer to that is what replaces the file. The file used to be + * emptied before the fetch began, so a slow network, a restart or a refused + * download left nothing at all where the work had been — and the state the save + * replaced was gone either way, since nothing kept it. + * + * Both are the same change: the document is fetched into a file of its own and + * handed to the versions, which keep what it replaces and put it in place only + * once it is whole. A document nobody could save is therefore still the + * document, and the history the engine has been keeping since it arrived is + * finally written by somebody. + */ + +const SECRET = 'onlyoffice-save-secret'; +const DOCUMENT = 'Projects/report.docx'; + +let env; +let app; +let alice; +let documentServer; +let serverUrl; +/** The same server under another address, as one behind a proxy reports itself. */ +let mirror; +let mirrorUrl; +/** Somewhere else entirely, answering perfectly well: what must not be fetched. */ +let stranger; +let strangerUrl; +/** What the fake Document Server hands out, and how badly it fails to. */ +let saved; + +const load = (relative) => require(modulePath(relative)); + +const absolute = () => path.join(env.volumeDir, ...DOCUMENT.split('/')); + +beforeEach(async () => { + saved = { content: 'second', refuse: false, dieHalfway: false }; + + documentServer = http.createServer((req, res) => { + if (saved.refuse) { + res.statusCode = 500; + res.end('no'); + return; + } + res.setHeader('Content-Type', 'application/octet-stream'); + if (saved.dieHalfway) { + // Answered, then gone: the network drops, the Document Server restarts. + // This is the shape that used to destroy the document, since the file was + // already empty by the time the stream stopped arriving. + res.setHeader('Content-Length', String(saved.content.length + 100)); + res.write(saved.content.slice(0, 3)); + setTimeout(() => res.destroy(), 20); + return; + } + res.end(saved.content); + }); + await new Promise((resolve) => documentServer.listen(0, '127.0.0.1', resolve)); + serverUrl = `http://127.0.0.1:${documentServer.address().port}`; + + stranger = http.createServer((req, res) => { + res.setHeader('Content-Type', 'application/octet-stream'); + res.end('stolen'); + }); + await new Promise((resolve) => stranger.listen(0, '127.0.0.1', resolve)); + strangerUrl = `http://127.0.0.1:${stranger.address().port}`; + + mirror = http.createServer((req, res) => { + res.setHeader('Content-Type', 'application/octet-stream'); + res.end(saved.content); + }); + await new Promise((resolve) => mirror.listen(0, '127.0.0.1', resolve)); + mirrorUrl = `http://127.0.0.1:${mirror.address().port}`; + + env = await setupTestEnv({ + tag: 'onlyoffice-save-', + env: { + PUBLIC_URL: 'https://files.example.com', + ONLYOFFICE_URL: serverUrl, + ONLYOFFICE_SECRET: SECRET, + ONLYOFFICE_DOWNLOAD_ORIGINS: mirrorUrl, + }, + }); + + alice = await load('src/services/users').createLocalUser({ + email: 'alice@example.com', + username: 'alice', + displayName: 'Alice', + password: 'secret123', + roles: ['user'], + }); + + await fs.mkdir(path.dirname(absolute()), { recursive: true }); + await fs.writeFile(absolute(), 'first'); + + app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + req.user = alice; + next(); + }); + app.use('/api', load('src/routes/onlyoffice')); + app.use(load('src/middleware/errorHandler').errorHandler); +}); + +afterEach(async () => { + load('src/services/trash/maintenance').stop(); + await new Promise((resolve) => documentServer.close(resolve)); + await new Promise((resolve) => mirror.close(resolve)); + await new Promise((resolve) => stranger.close(resolve)); + await env.cleanup(); +}); + +/** As the Document Server calls back: its own token, and the document to fetch. */ +const callback = (body) => + request(app) + .post('/api/onlyoffice/callback') + .query({ path: DOCUMENT }) + .set('Authorization', `Bearer ${jwt.sign({ any: true }, SECRET)}`) + .send({ url: `${serverUrl}/saved.docx`, key: 'session-key-1', ...body }); + +/** The versions kept for the only document these tests touch. */ +const versionsKept = async () => { + const db = await load('src/services/db').getDb(); + const store = load('src/services/versions/store'); + const files = store.listFiles(db); + if (!files.length) return []; + return store.listVersionsOfFile(db, files[0].id); +}; + +/** What the history says about the content the document holds now. */ +const currentState = async () => { + const db = await load('src/services/db').getDb(); + const store = load('src/services/versions/store'); + return store.listFiles(db)[0] || null; +}; + +const contentOf = async (version) => { + const operations = load('src/services/versions/operations'); + const located = await operations.locateVersion(version.id); + return fs.readFile(located.absolutePath, 'utf8'); +}; + +describe('the document the Document Server hands back', () => { + it('replaces the file, and what it replaced is kept', async () => { + const response = await callback({ status: 2 }); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ error: 0 }); + expect(await fs.readFile(absolute(), 'utf8')).toBe('second'); + + const versions = await versionsKept(); + expect(versions).toHaveLength(1); + expect(await contentOf(versions[0])).toBe('first'); + // Written before the application ever saw the file, so nobody is named: + // what the save itself is credited with is on the state it wrote. + expect(versions[0].source).toBe('external'); + }); + + it('leaves the document alone when the server refuses to hand it over', async () => { + saved.refuse = true; + + const response = await callback({ status: 2 }); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ error: 1 }); + expect(await fs.readFile(absolute(), 'utf8')).toBe('first'); + expect(await versionsKept()).toHaveLength(0); + }); + + /** + * The failure the old order could not survive. The document was emptied + * before the download began, so a stream that stopped arriving — a dropped + * network, a Document Server restarting mid-answer — left an empty file where + * the work had been. Nothing is written until the whole document is here. + */ + it('leaves the document alone when the download dies halfway', async () => { + saved.dieHalfway = true; + + const response = await callback({ status: 2 }); + + expect(response.body).toEqual({ error: 1 }); + expect(await fs.readFile(absolute(), 'utf8')).toBe('first'); + expect(await versionsKept()).toHaveLength(0); + }); + + it('credits the person whose changes it carries', async () => { + await callback({ + status: 2, + history: { changes: [{ user: { id: alice.id, name: 'Alice' } }] }, + }); + + const state = await currentState(); + expect(state.currentSource).toBe('onlyoffice'); + expect(state.currentAuthorId).toBe(alice.id); + expect(state.currentAuthorLabel).toBe('Alice'); + + // And once a second save replaces it, the state she wrote is kept as hers. + saved.content = 'third'; + await callback({ status: 2, key: 'session-key-2' }); + + const versions = await versionsKept(); + const hers = versions.find((version) => version.authorId === alice.id); + expect(hers, 'the save Alice made was not kept as hers').toBeTruthy(); + expect(hers.source).toBe('onlyoffice'); + expect(await contentOf(hers)).toBe('second'); + }); + + /** + * A save from a share link has no account behind it. It used to be credited + * to the guest session's own identifier, which names nobody once the session + * is over. + */ + it('credits a save made through a share link to the link', async () => { + await callback({ + status: 2, + history: { changes: [{ user: { id: 'guest_abc', name: 'Guest User' } }] }, + }); + + const state = await currentState(); + expect(state.currentAuthorId).toBeNull(); + expect(state.currentAuthorLabel).toBe('share-link'); + }); + + /** Force save (6) is the same save as far as the file is concerned. */ + it('saves a forced save too', async () => { + saved.content = 'forced'; + + const response = await callback({ status: 6, forcesavetype: 1 }); + + expect(response.body).toEqual({ error: 0 }); + expect(await fs.readFile(absolute(), 'utf8')).toBe('forced'); + expect(await versionsKept()).toHaveLength(1); + }); + + /** + * The callback says where the document is, and the server used to fetch + * whatever it was told to — an address on the machine itself, or one inside + * the container's network, reached by anybody who can reach the callback. + */ + it('fetches nothing from a server that is not the Document Server', async () => { + // Answering, and answering well: a refusal here cannot be the network. + const response = await callback({ status: 2, url: `${strangerUrl}/saved.docx` }); + + expect(response.body).toEqual({ error: 1 }); + expect(await fs.readFile(absolute(), 'utf8')).toBe('first'); + expect(await versionsKept()).toHaveLength(0); + }); + + /** A Document Server behind a proxy reports itself under the declared host. */ + it('fetches from an address declared beside the Document Server', async () => { + const response = await callback({ status: 2, url: `${mirrorUrl}/saved.docx` }); + + expect(response.body).toEqual({ error: 0 }); + expect(await fs.readFile(absolute(), 'utf8')).toBe('second'); + }); + + it('acknowledges a status that saves nothing, and writes nothing', async () => { + const response = await callback({ status: 1 }); + + expect(response.body).toEqual({ error: 0 }); + expect(await fs.readFile(absolute(), 'utf8')).toBe('first'); + expect(await versionsKept()).toHaveLength(0); + }); +}); diff --git a/backend/tests/routes/thumbnails-token.test.js b/backend/tests/routes/thumbnails-token.test.js new file mode 100644 index 00000000..54c0e305 --- /dev/null +++ b/backend/tests/routes/thumbnails-token.test.js @@ -0,0 +1,80 @@ +import fs from 'node:fs/promises'; +import path from 'node:path'; +import express from 'express'; +import request from 'supertest'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { modulePath, setupTestEnv } from '../helpers/env-test-utils.js'; + +/** + * The whole way a thumbnail is fetched, end to end. + * + * `/api/thumbnails` decides — it resolves the path, checks the access — and + * then hands back a URL under `/static`, which the authentication middleware + * does not cover. The decision has to travel with that URL, or the picture is + * readable by anybody who can guess a file's path. + * + * So this walks it: ask the API, follow the URL it gives, and ask for the same + * picture without what the API added. + */ + +/** A real one-pixel PNG: the thumbnailer has to be able to read it. */ +const ONE_PIXEL_PNG = Buffer.from( + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==', + 'base64' +); + +const PICTURE = 'Pictures/dot.png'; + +let env; +let app; + +const load = (relative) => require(modulePath(relative)); + +beforeEach(async () => { + env = await setupTestEnv({ tag: 'thumbnails-token-route-' }); + + const alice = await load('src/services/users').createLocalUser({ + email: 'alice@example.com', + username: 'alice', + displayName: 'Alice', + password: 'secret123', + roles: ['user'], + }); + + const absolute = path.join(env.volumeDir, ...PICTURE.split('/')); + await fs.mkdir(path.dirname(absolute), { recursive: true }); + await fs.writeFile(absolute, ONE_PIXEL_PNG); + + app = express(); + app.use((req, _res, next) => { + req.user = alice; + req.session = {}; + next(); + }); + app.use('/api', load('src/routes/thumbnails')); + load('src/utils/staticServer').configureStaticFiles(app); + app.use(load('src/middleware/errorHandler').errorHandler); +}); + +afterEach(async () => { + await env.cleanup(); +}); + +describe('asking for a thumbnail', () => { + it('hands back a URL that opens, and only with what it handed back', async () => { + const asked = await request(app).get(`/api/thumbnails/${PICTURE}`); + + expect(asked.status).toBe(200); + const url = asked.body.thumbnail; + expect(url, 'no thumbnail was made for a one-pixel PNG').toMatch(/^\/static\/thumbnails\//); + expect(url).toContain('?t='); + + const [pathname, query] = url.split('?'); + expect((await request(app).get(pathname).query(query)).status).toBe(200); + + // The same picture, asked for without the proof the API attached: this is + // every request that did not go through the access check. + expect((await request(app).get(pathname)).status).toBe(401); + }); +}); diff --git a/backend/tests/utils/static-thumbnails.test.js b/backend/tests/utils/static-thumbnails.test.js new file mode 100644 index 00000000..a789cacf --- /dev/null +++ b/backend/tests/utils/static-thumbnails.test.js @@ -0,0 +1,155 @@ +import { describe, it, expect, afterEach } from 'vitest'; +import path from 'node:path'; +import fs from 'node:fs/promises'; +import express from 'express'; +import cookieParser from 'cookie-parser'; +import request from 'supertest'; +import { setupTestEnv } from '../helpers/env-test-utils.js'; + +/** + * Thumbnails are served from /static, outside the auth middleware, so the URL + * carries its own proof: a signature minted by /api/thumbnails once the real + * access check passed, naming the one file it unlocks. + * + * A session cookie cannot do that job — it identifies the caller without + * saying what they were cleared to see, so a share visitor could ask for a + * filename belonging to another share. These pin that the signature is + * required, file-scoped, and time-limited. + */ + +let currentEnv; + +afterEach(async () => { + if (currentEnv) { + await currentEnv.cleanup(); + currentEnv = null; + } +}); + +const MODULES = ['src/config/env', 'src/config/index', 'src/utils/staticServer']; + +const buildApp = (env) => { + const { configureStaticFiles } = env.requireFresh('src/utils/staticServer'); + const app = express(); + app.use(cookieParser()); + // express-session normally provides this; only the shape matters here. + app.use((req, _res, next) => { + req.session = req.headers['x-signed-in'] ? { localUserId: 'user-1' } : {}; + next(); + }); + configureStaticFiles(app); + return app; +}; + +const seedThumbnail = async (env, name = 'v3-abc.webp') => { + const dir = path.join(env.cacheDir, 'thumbnails'); + await fs.mkdir(dir, { recursive: true }); + await fs.writeFile(path.join(dir, name), Buffer.from('fake-webp')); +}; + +describe('Thumbnail access', () => { + it('refuses a request with no token, signed in or not', async () => { + const env = await setupTestEnv({ tag: 'thumbnails-no-token-', modules: MODULES }); + currentEnv = env; + await seedThumbnail(env); + const app = buildApp(env); + + const anonymous = await request(app).get('/static/thumbnails/v3-abc.webp'); + expect(anonymous.status).toBe(401); + + // Being signed in is not the point: this endpoint answers to the token + // issued for one file, not to whoever happens to hold a session. + const signedIn = await request(app) + .get('/static/thumbnails/v3-abc.webp') + .set('x-signed-in', '1'); + expect(signedIn.status).toBe(401); + }); + + it('serves the file the token was issued for', async () => { + const env = await setupTestEnv({ tag: 'thumbnails-token-', modules: MODULES }); + currentEnv = env; + await seedThumbnail(env); + + const { createThumbnailToken } = env.requireFresh('src/utils/thumbnailTokens'); + const response = await request(buildApp(env)) + .get('/static/thumbnails/v3-abc.webp') + .query({ t: createThumbnailToken('v3-abc.webp') }); + + expect(response.status).toBe(200); + }); + + it('refuses a token issued for another file', async () => { + const env = await setupTestEnv({ tag: 'thumbnails-other-file-', modules: MODULES }); + currentEnv = env; + await seedThumbnail(env, 'v3-mine.webp'); + await seedThumbnail(env, 'v3-someone-else.webp'); + + const { createThumbnailToken } = env.requireFresh('src/utils/thumbnailTokens'); + // This is the case a session check could never catch: a legitimate visitor + // reusing their own credential against a filename they were never cleared + // for. + const response = await request(buildApp(env)) + .get('/static/thumbnails/v3-someone-else.webp') + .query({ t: createThumbnailToken('v3-mine.webp') }); + + expect(response.status).toBe(401); + }); + + it('refuses an expired or tampered token', async () => { + const env = await setupTestEnv({ tag: 'thumbnails-expired-', modules: MODULES }); + currentEnv = env; + await seedThumbnail(env); + + const { createThumbnailToken, TTL_MS } = env.requireFresh('src/utils/thumbnailTokens'); + const app = buildApp(env); + + const expired = createThumbnailToken('v3-abc.webp', Date.now() - TTL_MS - 1000); + expect( + (await request(app).get('/static/thumbnails/v3-abc.webp').query({ t: expired })).status + ).toBe(401); + + // Pushing the expiry out by hand invalidates the signature. + const valid = createThumbnailToken('v3-abc.webp'); + const forged = `${Date.now() + 10 * 60 * 1000}.${valid.split('.')[1]}`; + expect( + (await request(app).get('/static/thumbnails/v3-abc.webp').query({ t: forged })).status + ).toBe(401); + }); + + it('does not let a token unlock a nested path with the same basename', async () => { + const env = await setupTestEnv({ tag: 'thumbnails-nested-', modules: MODULES }); + currentEnv = env; + const dir = path.join(env.cacheDir, 'thumbnails', 'sub'); + await fs.mkdir(dir, { recursive: true }); + await fs.writeFile(path.join(dir, 'v3-abc.webp'), Buffer.from('fake-webp')); + + const { createThumbnailToken } = env.requireFresh('src/utils/thumbnailTokens'); + const response = await request(buildApp(env)) + .get('/static/thumbnails/sub/v3-abc.webp') + .query({ t: createThumbnailToken('v3-abc.webp') }); + + expect(response.status).toBe(401); + }); + + it('answers a malformed URL with 401 rather than a crash', async () => { + const env = await setupTestEnv({ tag: 'thumbnails-malformed-', modules: MODULES }); + currentEnv = env; + await seedThumbnail(env); + + const response = await request(buildApp(env)).get('/static/thumbnails/%zz.webp'); + expect(response.status).toBe(401); + }); + + it('serves everything when authentication is disabled', async () => { + const env = await setupTestEnv({ + tag: 'thumbnails-no-auth-', + env: { AUTH_MODE: 'disabled' }, + modules: MODULES, + }); + currentEnv = env; + await seedThumbnail(env); + + const response = await request(buildApp(env)).get('/static/thumbnails/v3-abc.webp'); + expect(response.status).toBe(200); + }); +}); diff --git a/docs/configuration/environment.md b/docs/configuration/environment.md index 439d117e..9c06de09 100644 --- a/docs/configuration/environment.md +++ b/docs/configuration/environment.md @@ -143,6 +143,7 @@ The sharing system (toolbar **Share** button, guest links such as `/share/:token | ----------------------------- | ------------------ | --------------------------------------------------------------------------------------------------------------------------------------- | | `ONLYOFFICE_URL` | _none_ | Public URL for Document Server (must reach your app's `PUBLIC_URL`). | | `ONLYOFFICE_SECRET` | _none_ | JWT secret shared with OnlyOffice Document Server for `/api/onlyoffice` calls. | +| `ONLYOFFICE_DOWNLOAD_ORIGINS` | _none_ | Comma-separated extra origins a saved document may be fetched from. Set it when the Document Server reports itself under another host than `ONLYOFFICE_URL`; that one is always allowed. | | `ONLYOFFICE_LANG` | `en` | Language code for the editor UI. | | `ONLYOFFICE_FORCE_SAVE` | `false` | When true, OnlyOffice forces users to save via the editor UI. | | `ONLYOFFICE_FILE_EXTENSIONS` | _default list_ | Extra file extensions to surface to the Document Server. |