diff --git a/backend/src/routes/preview.routes.ts b/backend/src/routes/preview.routes.ts index 295d63a..8623ce6 100644 --- a/backend/src/routes/preview.routes.ts +++ b/backend/src/routes/preview.routes.ts @@ -5,6 +5,7 @@ import buildExecutor from '../services/buildExecutor.js'; import { ensurePreviewBundle } from '../services/previewBundleService.js'; import { injectLatestPreTeXtLayoutFix } from '../services/previewTransformService.js'; import { getProofdeskDataPath } from '../utils/dataPaths.js'; +import authSessionStore from '../services/authSessionStore.js'; const getPreviewMimeType = (ext: string): string => { const mimeTypes: Record = { @@ -76,14 +77,66 @@ const versionLivePreviewAssets = (html: string, version?: string): string => { export const createPreviewRouter = (): Router => { const router = Router(); - router.get('/:sessionId/*', async (req: Request, res: Response): Promise => { + /** + * Gate preview access on an authenticated session (issue #51). + * + * Applied on the route rather than at the `app.use('/preview', ...)` mount, which matters: + * `req.params` is empty at mount level because `:sessionId` belongs to this route, so a check + * placed there reads `sessionId` as undefined and waves everything through. + * + * Authentication comes from the session cookie, which is also the only mechanism available to + * the caller that matters: a browser cannot attach an Authorization header to an iframe + * navigation, and the preview is loaded in an iframe. The + * cookie is `SameSite=Lax`, and since cookies are scoped by host rather than origin, it is still + * sent when the app and the API differ only by port or subdomain. + * + * On ownership: when a build session is on record and names a creator, only that user may read + * its preview. When no record exists the request is allowed through for any authenticated user, + * and that is a deliberate compromise rather than an oversight. Build sessions live in memory, so + * every preview on disk becomes an unowned orphan after a restart; failing closed there would + * break previews on every deploy. The reported hole — anyone on the internet reading a preview + * without logging in — is closed either way, and an orphan still requires guessing a 16-hex + * identifier. + */ + const requirePreviewAccess = async ( + req: Request, + res: Response, + next: () => void + ): Promise => { const { sessionId } = req.params; - const filePath = req.params[0] || 'overview.html'; if (!/^[0-9a-f]{16}$/.test(sessionId)) { return res.status(400).send('Invalid session ID'); } + // Only a real session cookie counts here, and a bearer token deliberately does not. + // + // `extractAccessToken` returns any bearer value it finds without checking it, setting + // `req.authSession` to null. On routes like /user that is harmless, because the token is + // forwarded to GitHub and a forged one fails there. This route never uses the token for + // anything, so presence would be the entire gate and `Authorization: Bearer anything` would + // walk straight through. Resolving the cookie directly means a request either carries a session + // this server issued or it does not. + const session = await authSessionStore.getSessionFromRequest(req); + const login = session?.user?.login; + + if (!login) { + return res.status(401).send('Authentication required'); + } + + const owner = buildExecutor.getSession(sessionId)?.creatorLogin; + + if (owner && owner !== login) { + return res.status(403).send('Access denied'); + } + + return next(); + }; + + router.get('/:sessionId/*', requirePreviewAccess, async (req: Request, res: Response): Promise => { + const { sessionId } = req.params; + const filePath = req.params[0] || 'overview.html'; + const activeSession = buildExecutor.getSession(sessionId); const outputPath = activeSession ? path.resolve(activeSession.outputPath) diff --git a/backend/tests/server.routes.test.js b/backend/tests/server.routes.test.js index 0c9307b..c373500 100644 --- a/backend/tests/server.routes.test.js +++ b/backend/tests/server.routes.test.js @@ -425,7 +425,9 @@ describe('active backend routes', () => { }); it('rewrites nested preview asset paths for knowls and shared CSS', async () => { - const knowlResponse = await request(app).get(`/preview/${previewSessionId}/knowl/sample.html`); + const knowlResponse = await request(app) + .get(`/preview/${previewSessionId}/knowl/sample.html`) + .set('Cookie', await previewCookie()); assert.equal(knowlResponse.status, 200); assert.match(knowlResponse.text, /src="\/preview\/aaaaaaaaaaaaaaaa\/images\/important\.svg"/); assert.match(knowlResponse.text, /href="\/preview\/aaaaaaaaaaaaaaaa\/figure-images\/sample\.png"/); @@ -433,13 +435,17 @@ describe('active backend routes', () => { assert.ok(knowlResponse.text.includes("replaceDelimitedCommands(tex, ['syseq','spalignsys'], convertSpAlign)")); assert.match(knowlResponse.text, /inlineMath:\[\[/); - const cssResponse = await request(app).get(`/preview/${previewSessionId}/css/ila.css`); + const cssResponse = await request(app) + .get(`/preview/${previewSessionId}/css/ila.css`) + .set('Cookie', await previewCookie()); assert.equal(cssResponse.status, 200); assert.match(cssResponse.text, /url\("\/preview\/aaaaaaaaaaaaaaaa\/fonts\/CharterBT-Roman\.woff"\)/); }); it('injects the MathBox loader cleanup into preview HTML', async () => { - const response = await request(app).get(`/preview/${previewSessionId}/demo.html`); + const response = await request(app) + .get(`/preview/${previewSessionId}/demo.html`) + .set('Cookie', await previewCookie()); assert.equal(response.status, 200); assert.match(response.text, /id="mathbox-loader-preview-fix"/); assert.match(response.text, /proofdesk-loader-hidden/); @@ -447,12 +453,53 @@ describe('active backend routes', () => { }); it('cache-busts local live preview JavaScript and CSS references', async () => { - const response = await request(app).get(`/preview/${previewSessionId}/demo.html?t=live-123`); + const response = await request(app) + .get(`/preview/${previewSessionId}/demo.html?t=live-123`) + .set('Cookie', await previewCookie()); assert.equal(response.status, 200); assert.match(response.text, /href="styles\.css\?proofdeskLive=live-123"/); assert.match(response.text, /src="js\/demo\.js\?proofdeskLive=live-123"/); }); + /** A session cookie from the local demo login — the only credential previews accept. */ + const previewCookie = async () => { + const auth = await request(app).get('/auth/local-test'); + return auth.headers['set-cookie']; + }; + + it('refuses a preview request carrying an unverified bearer token', async () => { + // A bearer value is never validated on this path, so accepting one would make the header + // itself the credential. Only a session this server issued may pass. + const response = await request(app) + .get(`/preview/${previewSessionId}/demo.html`) + .set('Authorization', 'Bearer not-a-real-token'); + assert.equal(response.status, 401); + }); + + it('refuses a preview request with no credentials (issue #51)', async () => { + // The reported hole: anyone who knew a session id could read the compiled output. + const response = await request(app).get(`/preview/${previewSessionId}/demo.html`); + assert.equal(response.status, 401); + }); + + it('still rejects a malformed session id before asking for credentials', async () => { + const response = await request(app).get('/preview/not-a-session/demo.html'); + assert.equal(response.status, 400); + }); + + it('accepts a preview request carrying the session cookie', async () => { + // A browser cannot put an Authorization header on an iframe navigation, so the cookie path is + // the one that actually matters in the product. + const auth = await request(app).get('/auth/local-test'); + const cookie = auth.headers['set-cookie']; + assert.ok(cookie, 'the local demo login did not set a session cookie'); + + const response = await request(app) + .get(`/preview/${previewSessionId}/demo.html`) + .set('Cookie', cookie); + assert.equal(response.status, 200); + }); + it('serves the Prometheus metrics data', async () => { const response = await request(app).get('/metrics'); assert.equal(response.status, 200);