From 1d3a73e36536beac73b5b6692ced29eda9fefc77 Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Fri, 28 Aug 2026 16:41:35 +0200 Subject: [PATCH] =?UTF-8?q?=E2=AC=86=EF=B8=8F(collaboration)=20upgrade=20y?= =?UTF-8?q?hub=20to=200.8.0=20and=20adopt=20its=20permission=20model?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit yhub 0.8.0 retires the 'r' | 'rw' | null access vocabulary. The auth plugin now answers a typed permission object stating, facet by facet, what a subject may do with a document, and yhub enforces every facet itself - on the websocket and on the REST routes alike. Three rules we wanted but could not express under the old vocabulary become one-line facets. Our whole access policy now lives in src/yhub-server/permissions.js, apart from the server so that it can be read and tested without standing up redis and postgres. Read-only users no longer share their cursor #2544. A read-only connection could still propagate awareness updates to everyone else in the document, even though its document updates were already dropped. Presence is now a permission of its own, separate from the right to edit: a reader receives it and never publishes it. The collaboration server enforces that rather than trusting the editor to stay quiet, so a modified or stale client changes nothing. The frontend has to know it too - the http fallback provider has no receive-only mode, so a reader's provider is built with no awareness instance at all, or its first PATCH would take a 403 and close it for good. The browser is granted only the two routes it uses, the websocket and ydoc for the http fallback. Everything else - history, rollback, prune, and every backend-internal endpoint - is refused to it, as is any endpoint a future release adds, because the grant names no wildcard. create-ydoc in particular was reachable by any signed-in editor and is now the backend's alone. Anonymous visitors are given the userid "anonymous" rather than no identity at all, which is what lets them keep editing public documents: yhub refuses the upgrade of a caller that holds the write but cannot be attributed. Their edits share one author. Room is DocRef throughout, which is a rename of object keys and not only of types: the worker event payload and the stream message lookup both carry it, and both fail silently rather than loudly when missed. Signed-off-by: Kevin Jahns --- CHANGELOG.md | 27 ++ documentation/collaboration.md | 19 ++ .../docs/doc-editor/components/DocEditor.tsx | 4 +- .../docs/doc-editor/hook/useCollaboration.tsx | 48 ++- .../__tests__/useProviderStore.test.tsx | 48 ++- .../stores/useProviderStore.tsx | 26 +- src/yhub-server/README.md | 113 +++++-- src/yhub-server/migration.js | 68 ++-- src/yhub-server/package-lock.json | 16 +- src/yhub-server/package.json | 5 +- src/yhub-server/permissions.js | 81 +++++ src/yhub-server/permissions.test.js | 162 +++++++++ src/yhub-server/server.js | 318 ++++++++++-------- 13 files changed, 712 insertions(+), 223 deletions(-) create mode 100644 src/yhub-server/permissions.js create mode 100644 src/yhub-server/permissions.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a090079b..59e72fc53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,35 @@ and this project adheres to ## [Unreleased] +### Security + +- šŸ”’ļø(collaboration) stop read-only users from sharing their cursor #2544. A + read-only connection could still propagate awareness updates ā€” the cursor and + the selection ā€” to everyone else in the document, even though its document + updates were already dropped. Presence is now a permission of its own, + separate from the right to edit: a reader receives it and never publishes it. + The collaboration server enforces it rather than trusting the editor to stay + quiet, dropping a read-only connection's presence on the websocket and + refusing the awareness field of an http fallback request, so a modified or + stale client changes nothing + ### Added +- āœØ(collaboration) grant the browser only the two routes it uses. The + collaboration server now answers what a caller may do with a document as a + permission object, facet by facet, and enforces every facet itself. A browser + is granted the websocket and the document route the http fallback polls, and + nothing else ā€” the history, activity, changeset, rollback and prune routes, + every backend-internal endpoint, and any endpoint a future release adds are + refused to it. `create-ydoc` in particular was reachable by any signed-in + editor and is now the backend's alone. The backend's admin token keeps full + access, minus the irreversible content erasure that the new version exposes + over http for the first time + +- āœØ(collaboration) let anonymous visitors edit public documents again under the + new permission model, with their changes attributed to a shared `anonymous` + author + - āœØ(frontend) fall back to http polling when the websocket cannot be opened. Some networks refuse a websocket upgrade ā€” corporate proxies, captive portals ā€” and a browser is told nothing more than "the connection closed"; those users diff --git a/documentation/collaboration.md b/documentation/collaboration.md index 3df7b70b4..3a99d4955 100644 --- a/documentation/collaboration.md +++ b/documentation/collaboration.md @@ -87,6 +87,25 @@ the whole document, so a large document polled by many clients is real egress. I editing, not a replacement for the socket ā€” and the socket keeps being retried underneath, so a client that fell back during an outage returns to it on its own. +A reader on the fallback sees no cursors at all. Read-only clients may not publish presence (see +below), the provider has no receive-only setting for it, and a reader that tried to publish would +be refused and stop polling altogether ā€” so it is built without awareness and only ever reads the +document. On the websocket a reader still sees everyone else's cursors. + Documents are never in conflict either way: both transports publish from the same Yjs document, and Yjs merges. Before the fallback existed, users who could not open a websocket edited a document that was saved wholesale and erased each other's modifications; that is what this removes. + +## Who may share a cursor + +Presence ā€” the coloured cursors and selections of the other people in a document ā€” is a permission +of its own, separate from the right to edit. A **reader receives presence but never publishes it**: +they see who else is in the document and where, and nobody sees them. + +The collaboration server enforces this itself rather than trusting the editor to be quiet. It drops +a read-only connection's presence message on the websocket, and refuses the `awareness` field of a +fallback request, so a modified or stale client changes nothing. See the access-control section of +`src/yhub-server/README.md` for the permission tables this comes from. + +Note this is deliberately stricter than the collaboration server's own default, which lets +read-only connections broadcast cursors. diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/DocEditor.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/components/DocEditor.tsx index 79bedea1f..f9b526cfc 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/components/DocEditor.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/DocEditor.tsx @@ -83,9 +83,11 @@ interface DocEditorProps { } export const DocEditor = ({ doc }: DocEditorProps) => { - useCollaboration(doc.id); const isDeletedDoc = !!doc.deleted_at; const readOnly = !doc.abilities.partial_update || isDeletedDoc; + // the collaboration providers are built from this too: a reader publishes no + // presence, on either transport + useCollaboration(doc.id, readOnly); const { trackEvent } = useAnalytics(); const [hasTracked, setHasTracked] = useState(false); const { authenticated } = useAuth(); diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/hook/useCollaboration.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/hook/useCollaboration.tsx index aa574204d..ddad61886 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/hook/useCollaboration.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/hook/useCollaboration.tsx @@ -1,5 +1,5 @@ import { useQueryClient } from '@tanstack/react-query'; -import { useEffect } from 'react'; +import { useEffect, useRef } from 'react'; import { useCollaborationUrl, useConfig } from '@/core/config'; import { KEY_DOC } from '@/docs/doc-management/api/useDoc'; @@ -7,7 +7,15 @@ import { useProviderStore } from '@/docs/doc-management/stores/useProviderStore' import { useIsOffline } from '@/features/service-worker/hooks/useOffline'; import { useBroadcastStore } from '@/stores/useBroadcastStore'; -export const useCollaboration = (room: string) => { +/** + * `readOnly` is the editor's own predicate, and it reaches this far because the + * providers are built with it: a reader publishes no presence, which the http + * fallback can only express by carrying no awareness instance at all (see + * `createProvider`). It is allowed to be stricter than the collaboration + * server's own verdict ā€” that direction only declines presence we would have + * been permitted to send ā€” but never looser. + */ +export const useCollaboration = (room: string, readOnly = false) => { const collaborationUrl = useCollaborationUrl(room); const { addTask } = useBroadcastStore(); const queryClient = useQueryClient(); @@ -108,9 +116,41 @@ export const useCollaboration = (room: string) => { return; } - const newProvider = createProvider(collaborationUrl, room); + const newProvider = createProvider(collaborationUrl, room, undefined, { + readOnly, + }); setBroadcastProvider(newProvider); - }, [provider, collaborationUrl, createProvider, room, setBroadcastProvider]); + }, [ + provider, + collaborationUrl, + createProvider, + room, + readOnly, + setBroadcastProvider, + ]); + + /** + * Rebuild the providers when the access changes under us. + * + * The effect above builds them once and `reconnect` only reopens the socket, so + * the `readOnly` decision baked into the http fallback would otherwise outlive + * the access it was made from. Demotion is the direction that bites: an editor + * turned reader would keep an awareness-publishing fallback, whose first + * `PATCH` takes a 403 and closes it for good. A permission change already + * forces a full re-auth (the server closes with 4401, the document is + * refetched, the connection is made again), so tearing the providers down here + * is in keeping rather than an extra disruption ā€” the document itself lives on + * the server, and the editor re-renders from the fresh sync. + */ + const builtReadOnly = useRef(readOnly); + useEffect(() => { + if (!provider || builtReadOnly.current === readOnly) { + return; + } + builtReadOnly.current = readOnly; + cleanupBroadcast(); + destroyProvider(); + }, [readOnly, provider, cleanupBroadcast, destroyProvider]); /** * Destroy the provider when the component is unmounted diff --git a/src/frontend/apps/impress/src/features/docs/doc-management/stores/__tests__/useProviderStore.test.tsx b/src/frontend/apps/impress/src/features/docs/doc-management/stores/__tests__/useProviderStore.test.tsx index 153a1f026..61aa273d8 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-management/stores/__tests__/useProviderStore.test.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-management/stores/__tests__/useProviderStore.test.tsx @@ -99,10 +99,16 @@ const createWebsocketFallback = vi.fn( }, ); -vi.mock('@y/yhub-http-fallback', () => ({ - HttpProvider: vi.fn(function () { +// hoisted alongside the `vi.mock` below, which is lifted above every `const` in +// this file ā€” the store builds it with `new`, so it stays a function expression +const { HttpProviderMock } = vi.hoisted(() => ({ + HttpProviderMock: vi.fn(function (..._args: unknown[]) { return httpProvider; }), +})); + +vi.mock('@y/yhub-http-fallback', () => ({ + HttpProvider: HttpProviderMock, createWebsocketFallback: (primary: unknown, secondary: unknown) => createWebsocketFallback( primary as FakeProvider, @@ -119,6 +125,7 @@ describe('useProviderStore', () => { httpProvider = new FakeHttpProvider(); stopFallback = vi.fn(); createWebsocketFallback.mockClear(); + HttpProviderMock.mockClear(); // the store is a module-level singleton: put it back to its defaults, or // a test reads what the one before it left behind useProviderStore.getState().destroyProvider(); @@ -231,6 +238,43 @@ describe('useProviderStore', () => { expect(useProviderStore.getState().isSynced).toBe(true); }); + it('lets an editor publish presence over the http fallback', () => { + // the shared Awareness instance: one client id, one set of clocks, whichever + // transport is carrying it + expect(HttpProviderMock).toHaveBeenCalledTimes(1); + expect(HttpProviderMock.mock.calls[0][3]).toMatchObject({ + awareness: provider.awareness, + }); + }); + + it('gives a read-only document no awareness on the http fallback', () => { + useProviderStore.getState().destroyProvider(); + HttpProviderMock.mockClear(); + + useProviderStore + .getState() + .createProvider( + 'ws://localhost/collaboration/ws/v1/docs', + 'doc-id', + undefined, + { + readOnly: true, + }, + ); + + /** + * A reader may not publish presence, and the provider has no receive-only + * setting. Left enabled, its first round would PATCH awareness, take a 403 + * from the collaboration server, and ā€” a 4xx being permanent ā€” close before + * it ever issued a GET, leaving a reader whose websocket is blocked in front + * of an empty document. `null` is what keeps the round to a plain poll. + */ + expect(HttpProviderMock).toHaveBeenCalledTimes(1); + expect(HttpProviderMock.mock.calls[0][3]).toMatchObject({ + awareness: null, + }); + }); + it('tears everything down with the document', () => { useProviderStore.getState().destroyProvider(); diff --git a/src/frontend/apps/impress/src/features/docs/doc-management/stores/useProviderStore.tsx b/src/frontend/apps/impress/src/features/docs/doc-management/stores/useProviderStore.tsx index ba65927a4..bd63c7a8d 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-management/stores/useProviderStore.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-management/stores/useProviderStore.tsx @@ -6,11 +6,22 @@ import { create } from 'zustand'; import { collaborationHttpTarget } from '@/core/config/hooks/useCollaborationUrl'; import { Base64 } from '@/docs/doc-management'; +/** + * `readOnly` decides whether this client may publish presence. It has to be known + * when the providers are built, not merely when the editor renders: the http + * fallback either carries an awareness instance or does not, and there is no + * middle setting (see `createProvider`). + */ +export interface CreateProviderOptions { + readOnly?: boolean; +} + export interface UseCollaborationStore { createProvider: ( providerUrl: string, storeId: string, initialDoc?: Base64, + options?: CreateProviderOptions, ) => WebsocketProvider; destroyProvider: () => void; setReady: (value: boolean) => void; @@ -80,7 +91,7 @@ const suspendFallback = (httpProvider: HttpProvider | undefined) => { export const useProviderStore = create((set, get) => ({ ...defaultValues, - createProvider: (wsUrl, storeId, initialDoc) => { + createProvider: (wsUrl, storeId, initialDoc, { readOnly = false } = {}) => { const doc = new Y.Doc({ guid: storeId, }); @@ -111,6 +122,17 @@ export const useProviderStore = create((set, get) => ({ * sync. It shares the `Awareness` instance for the same reason ā€” awareness state is keyed * by `doc.clientID`, so two instances would advertise the same client id with independent * clocks and fight over the local state. + * + * A reader gets no awareness instance at all. It may not publish presence (the + * collaboration server refuses the `awareness` field of a `PATCH`, and the endpoint grant + * makes the whole route read-only for it), and the provider has no receive-only setting: + * one flag both publishes the local state and applies the remote one. Left enabled, the + * first round would `PATCH` presence, take a 403, and ā€” a 4xx being permanent ā€” close the + * provider for good *before* it ever issued its first `GET`, so a reader on a network that + * blocks websockets would sit in front of an empty document indefinitely. Disabled, the + * round carries no body at all and degrades to exactly the poll a reader needs. The cost is + * that a reader on the fallback sees no remote cursors; at a 10s poll they would be a + * postcard from the past anyway. */ const target = collaborationHttpTarget(wsUrl); const httpProvider = target @@ -119,7 +141,7 @@ export const useProviderStore = create((set, get) => ({ target.serverUrl, { org: target.org, docid: storeId }, { - awareness: provider.awareness, + awareness: readOnly ? null : provider.awareness, // createWebsocketFallback owns the connection state connect: false, // Docs users are served the garbage-collected document; a full-history request is diff --git a/src/yhub-server/README.md b/src/yhub-server/README.md index 267617327..3e63bfc2a 100644 --- a/src/yhub-server/README.md +++ b/src/yhub-server/README.md @@ -27,8 +27,8 @@ It is not a fork of yhub ā€” it is a thin wrapper: `X-User-Id` header), for the Django backend to re-check the authorization of a document's connected clients when permissions change (backend wiring pending) ā€” authenticated with an RS256 admin JWT issued by Django and - verified against its JWKS (`/api/v1.0/jwks`); the `reset-connections` - purpose is granted only to that admin token, never to regular users, + verified against its JWKS (`/api/v1.0/jwks`); this route is named by no + browser grant, so it is reachable by that admin token alone, - exposes `POST /collaboration/create-ydoc/v1/{org}/{docid}` (optional `X-User-Id` header naming the user the initial content is attributed to), which seeds a document's initial Yjs state from a raw binary update @@ -36,11 +36,12 @@ It is not a fork of yhub ā€” it is a thin wrapper: `application/octet-stream`), so the Django backend can create documents server-side. The built-in `PATCH .../ydoc/` takes the same update, but this one is a **strict create** ā€” 409 when the document already has content ā€” and - it credits the content to `X-User-Id` instead of to the caller. Guarded by - standard document write access (the admin JWT, or a user session with update - ability). Reading needs neither, and goes through the built-in `GET - .../ydoc/`, which since 0.5.0 answers JSON (the update base64 encoded) to a - request sending `Accept: application/json`, + it credits the content to `X-User-Id` instead of to the caller. Admin JWT + only: it is a backend route, and under yhub 0.7 it was the one custom + endpoint a signed-in editor could also reach, because it declared no + `accessPurpose`. Reading goes through the built-in `GET .../ydoc/`, which + since 0.5.0 answers JSON (the update base64 encoded) to a request sending + `Accept: application/json`, - exposes `POST /collaboration/migrate/v1/{org}/{docid}`, which replays a document's **full** legacy version history out of the S3 media bucket (see "Full migration" below) ā€” admin JWT only, like `reset-connections`, @@ -79,17 +80,76 @@ It is not a fork of yhub ā€” it is a thin wrapper: - mirrors the environment conventions used elsewhere in this repository (`*_FILE` secret indirection, `COLLABORATION_SERVER_ORIGIN` allowlist, ā€¦). -Public exposure: route the whole `/collaboration/` prefix to this server ā€” -the websocket and the built-in document APIs (`ydoc`, `rollback`, `prune`, -`changeset`, `activity`) are all guarded by the same cookie-based document -authorization and are meant to be reachable by browsers, as is -`/collaboration/jwks/v1`, which carries public keys and nothing else. The one -exception is `/collaboration/reset-connections/`, `/collaboration/migrate/`, -`/collaboration/restore-ydoc/` and `/collaboration/reset-ydoc/`, which are -backend-internal and should not be routed through the public ingress. The two -probes are not worth publishing either ā€” kubelet calls them from inside ā€” and -the helm chart's ingress lists what it routes rather than what it hides, so -they stay in-cluster on their own. +Public exposure: the browser needs two routes, the websocket +`/collaboration/ws/` and `/collaboration/ydoc/` for the http fallback, plus +`/collaboration/jwks/v1`, which carries public keys and nothing else. Every +other route this server serves ā€” `rollback`, `prune`, `changeset`, `activity`, +`reset-connections`, `migrate`, `create-ydoc`, `restore-ydoc`, `reset-ydoc` ā€” +is now refused to a browser by the permission tables themselves (see "Access +control" below), so publishing one is no longer the security boundary it was +under yhub 0.7. Keep them off the +public ingress all the same: an endpoint that cannot be reached cannot be +probed. The two probes are not worth publishing either ā€” kubelet calls them from +inside ā€” and the helm chart's ingress lists what it routes rather than what it +hides, so they stay in-cluster on their own. + +### Access control + +yhub 0.8 replaced the `'r' | 'rw' | null` access vocabulary with **permission +objects**: the auth plugin answers, per facet, what a subject may do with one +document, and yhub enforces every facet itself ā€” on the websocket and on the +REST routes alike. Docs' whole policy is three tables in `permissions.js`, kept +out of `server.js` so they can be read and tested without redis and postgres. +`permissions.test.js` asks them the same questions yhub's gates ask; run it with +`npm test`. + +Masks are positional `crud` strings where `-` denies, so `'-r--'` is read-only. + +| Facet | Reader | Editor | Admin token | +|---|---|---|---| +| `ydoc` | `-r--` | `-ru-` | `cru-` | +| `awareness` | `-r--` | `-ru-` | `-ru-` | +| `history` | ā€” | ā€” | `from: 0` | +| `delete` | ā€” | ā€” | `['soft']` | +| `endpoint.ws` | `-r--` | `-ru-` | `crud` (`'*'`) | +| `endpoint.ydoc` | `-r--` | `-ru-` | `crud` (`'*'`) | +| every other endpoint | ā€” | ā€” | `crud` (`'*'`) | + +Reader and editor are the same document permission, `browserDocumentPermissions`, +switched on the backend's `abilities.update`; `abilities.retrieve` decided +whether there is any access at all before that. + +Four of those cells are decisions rather than transcriptions: + +- **`awareness: '-r--'` for a reader.** A reader receives presence and never + publishes it ā€” [suitenumerique/docs#2544](https://github.com/suitenumerique/docs/pull/2544), + where a read-only connection was found to still propagate cursors even though + its document updates were dropped. yhub enforces it on both transports: it + drops a read-only connection's awareness message on the socket, and refuses the + `awareness` field of `PATCH /ydoc`. This is a deliberate departure from yhub's + own default, which grants a reader `'-ru-'` and documents read-only cursors as + a feature. The frontend has to know it too: the http fallback provider has no + receive-only setting, so a reader's `HttpProvider` is built with no awareness + instance at all, or its first `PATCH` would take a 403 and close it for good. +- **No `'*'` endpoint fallback for the browser.** Only `ws` and `ydoc` are named, + so everything else is denied ā€” including any endpoint a future yhub release + adds. Under 0.7 this fence was a `purpose != null` check, which `create-ydoc` + slipped through by declaring no purpose. +- **No `history` facet for the browser.** This is what makes yhub refuse a + `gc=false` connection with a 403; Docs users are served the garbage-collected + document, and the full history is the backend's business. It also keeps + `activity` and `changeset` closed even if their endpoints were ever granted. +- **`delete: ['soft']` and not `'hard'` for the admin.** yhub 0.8 made + `DELETE /ydoc?hard=true` reachable over REST for the first time. Docs keeps + irreversible erasure programmatic, behind `reset-ydoc` (see "Deletion"). + +Identity is separate from permission. `authenticate` establishes who is asking; +returning `null` there means *anonymous*, not *denied*, so every rejection in +this server is a thrown `apiError(401, ā€¦)`. An unauthenticated visitor is given +the userid `anonymous`, which is what lets them edit a public document at all: +yhub refuses the upgrade of a caller that holds the write but has no identity, +because attributions carry the userid. Every anonymous edit is therefore +attributed to that one shared author. ### Origins and cors @@ -98,7 +158,7 @@ server from, and it is passed to yhub as its `cors` configuration: yhub applies it to the websocket upgrade *and* to every REST route, refusing a cross-origin request from anywhere else with a `403` before authentication runs. A request carrying no `Origin` at all is same-origin or is not a browser, and is gated by -the session cookie alone ā€” which is why `readAuthInfo` no longer checks the +the session cookie alone ā€” which is why `authenticate` no longer checks the origin itself: doing it twice would refuse exactly the requests the http fallback makes, since a same-origin `fetch` GET sends no `Origin` header. @@ -332,12 +392,15 @@ so the document comes back with its whole history. Restoring one that is not deleted answers 200 and changes nothing, which is what lets the backend restore a subtree without asking what became of each document in it. -Erasing the content for good is a third operation (`YHub.deleteDoc(room, { -hard: true })`), reachable from inside this process only ā€” yhub deliberately -keeps it off the REST API. It is not what deleting a document in Docs does: a -soft-deleted one simply stops being restorable after `TRASHBIN_CUTOFF_DAYS`, -and its content is kept. Note that a hard deletion is final for that room ā€” the -docid can never be written again, and `restore-ydoc` answers 409 for it. +Erasing the content for good is a third operation (`YHub.deleteDoc(docRef, { +hard: true })`). yhub 0.8 exposes it over REST as `DELETE .../ydoc?hard=true`, +gated by the `delete` facet ā€” and the admin token is granted `['soft']` only, so +in Docs that request is refused and the erasure stays reachable from inside this +process alone, through `reset-ydoc` below. It is not what deleting a document in +Docs does: a soft-deleted one simply stops being restorable after +`TRASHBIN_CUTOFF_DAYS`, and its content is kept. Note that a hard deletion is +final for that room ā€” the docid can never be written again, and `restore-ydoc` +answers 409 for it. ### Resetting (`POST /collaboration/reset-ydoc/v1/{org}/{docid}`) diff --git a/src/yhub-server/migration.js b/src/yhub-server/migration.js index cf59ce686..f895d9ef0 100644 --- a/src/yhub-server/migration.js +++ b/src/yhub-server/migration.js @@ -129,8 +129,8 @@ export const migrationLog = logger.child({ module: 'soft-migration' }); // drift from the room keys, and both sit outside its scanned `:room:*` pattern. // // One seeder per room: -const migrateLockKey = (yhub, room) => - `${yhub.stream.prefix}:softmigrate:${room.org}:${room.docid}:${room.branch}`; +const migrateLockKey = (yhub, docRef) => + `${yhub.stream.prefix}:softmigrate:${docRef.org}:${docRef.docid}:${docRef.branch}`; // Documents whose version history has been replayed into postgres. Membership // is permanent: a second replay of the same versions would attribute the same // content twice (see fullMigrate). @@ -265,15 +265,15 @@ const listLegacyVersions = async (docid) => { // the stream but carry no content) ā€” then the SELECT again, which closes the // store-before-trim compaction race (and the worst case of a miss is only a // redundant, idempotent re-seed). -const ydocExists = async (yhub, room) => { - if ((await yhub.persistence.retrieveDoc(room, {})).lastClock !== '0') { +const ydocExists = async (yhub, docRef) => { + if ((await yhub.persistence.retrieveDoc(docRef, {})).lastClock !== '0') { return true; } - const streams = await yhub.stream.getMessages([{ room, clock: '0' }]); + const streams = await yhub.stream.getMessages([{ docRef, clock: '0' }]); if ((streams[0]?.messages ?? []).some((m) => m.type === 'ydoc:update:v1')) { return true; } - return (await yhub.persistence.retrieveDoc(room, {})).lastClock !== '0'; + return (await yhub.persistence.retrieveDoc(docRef, {})).lastClock !== '0'; }; // Per-docid migration verdicts, in-memory (per replica). 'exists' is monotone @@ -321,18 +321,18 @@ const rememberVerdict = ( const inflightMigrations = new Map(); // docid -> Promise let activeSeeds = 0; -const migrate = async (yhub, room) => { +const migrate = async (yhub, docRef) => { // A fully migrated room holds a single row at clock 0, which leaves // `lastClock` at '0' ā€” so ydocExists cannot see it and would seed on top of a // complete history. Harmless (the seed's attributions are excluded as already // known) but a pointless S3 round-trip per document during a backfill. - if (await yhub.stream.redis.sIsMember(migratedSetKey(yhub), room.docid)) { + if (await yhub.stream.redis.sIsMember(migratedSetKey(yhub), docRef.docid)) { return 'exists'; } - if (await ydocExists(yhub, room)) return 'exists'; + if (await ydocExists(yhub, docRef)) return 'exists'; // collapse cross-replica herds: one seeder per room, the rest wait and // re-probe - const lockKey = migrateLockKey(yhub, room); + const lockKey = migrateLockKey(yhub, docRef); const lockToken = randomUUID(); const redis = yhub.stream.redis; const acquired = await redis.set(lockKey, lockToken, { @@ -349,7 +349,7 @@ const migrate = async (yhub, room) => { while (Date.now() < deadline && (await redis.exists(lockKey)) === 1) { await new Promise((resolve) => setTimeout(resolve, 300)); } - if (await ydocExists(yhub, room)) return 'exists'; + if (await ydocExists(yhub, docRef)) return 'exists'; } if (activeSeeds >= MAX_CONCURRENT_SEEDS) { // fail fast under a herd of distinct cold docs ā€” the client's retry @@ -363,10 +363,10 @@ const migrate = async (yhub, room) => { activeSeeds++; try { const start = Date.now(); - const update = await fetchLegacyDoc(room.docid); + const update = await fetchLegacyDoc(docRef.docid); if (update == null) { migrationLog.info( - { event: 'seed.empty', docid: room.docid }, + { event: 'seed.empty', docid: docRef.docid }, 'no legacy s3 object; room starts empty', ); return 'empty'; @@ -381,7 +381,7 @@ const migrate = async (yhub, room) => { err.permanent = true; throw err; } - await yhub.stream.addMessage(room, { + await yhub.stream.addMessage(docRef, { type: 'ydoc:update:v1', // Deliberately no insertAt/deleteAt. A lazy seed is not an editing // event: stamping it would put a second, meaningless timestamp on @@ -409,7 +409,7 @@ const migrate = async (yhub, room) => { migrationLog.info( { event: 'seed.ok', - docid: room.docid, + docid: docRef.docid, bytes: update.byteLength, durationMs: Date.now() - start, }, @@ -436,17 +436,17 @@ const migrate = async (yhub, room) => { // Resolves when the room is usable (already known, freshly seeded, or // legitimately empty); rejects to deny access. Idempotent and safe to // re-enter ā€” it also runs on rechecks and default-purpose REST calls. -export const maybeMigrate = async (yhub, room) => { - const cached = verdicts.get(room.docid); +export const maybeMigrate = async (yhub, docRef) => { + const cached = verdicts.get(docRef.docid); if (cached != null && cached.expires > Date.now()) { if (cached.verdict === 'failed') throw cached.error; return; } - let migration = inflightMigrations.get(room.docid); + let migration = inflightMigrations.get(docRef.docid); if (migration == null) { - migration = migrate(yhub, room) + migration = migrate(yhub, docRef) .then( - (verdict) => rememberVerdict(room.docid, verdict), + (verdict) => rememberVerdict(docRef.docid, verdict), (err) => { // The one place the *cause* is recorded, once per attempt rather // than per access: a cached verdict re-raises this error without @@ -456,10 +456,10 @@ export const maybeMigrate = async (yhub, room) => { { event: 'seed.failed', err, - docid: room.docid, + docid: docRef.docid, permanent, bucket: LEGACY_S3_BUCKET_NAME, - key: `${room.docid}/file`, + key: `${docRef.docid}/file`, }, permanent ? 'soft migration is not possible for this legacy object' @@ -469,7 +469,7 @@ export const maybeMigrate = async (yhub, room) => { // a retryable failure is remembered only briefly, so a hiccup // cannot lock a document out for the full poison-object window rememberVerdict( - room.docid, + docRef.docid, 'failed', err, permanent ? VERDICT_TTL_MS.failed : TRANSIENT_TTL_MS, @@ -478,8 +478,8 @@ export const maybeMigrate = async (yhub, room) => { throw err; }, ) - .finally(() => inflightMigrations.delete(room.docid)); - inflightMigrations.set(room.docid, migration); + .finally(() => inflightMigrations.delete(docRef.docid)); + inflightMigrations.set(docRef.docid, migration); } return migration; }; @@ -500,17 +500,17 @@ export const maybeMigrate = async (yhub, room) => { // messages a live editor is writing; and the history is genuinely the oldest // thing in the room. The next compact task merges the row into the room's // normal state and drops it ā€” yhub needs no special case for any of this. -export const fullMigrate = async (yhub, room, { force = false } = {}) => { +export const fullMigrate = async (yhub, docRef, { force = false } = {}) => { const start = Date.now(); const redis = yhub.stream.redis; // Membership is the guard against attributing the same content twice: once // compaction has folded the clock-0 row into a normal one and deleted it, // a second replay would insert a second contentmap for ids that already // carry one, and the two timestamps would both survive the merge. - if (!force && (await redis.sIsMember(migratedSetKey(yhub), room.docid))) { + if (!force && (await redis.sIsMember(migratedSetKey(yhub), docRef.docid))) { return { status: 'already' }; } - const { versions, dropped } = await listLegacyVersions(room.docid); + const { versions, dropped } = await listLegacyVersions(docRef.docid); if (versions.length === 0) { return { status: 'empty', versions: 0, durationMs: Date.now() - start }; } @@ -525,7 +525,7 @@ export const fullMigrate = async (yhub, room, { force = false } = {}) => { let skipped = 0; try { for (const version of versions) { - const update = await fetchLegacyDoc(room.docid, version.versionId); + const update = await fetchLegacyDoc(docRef.docid, version.versionId); if (update == null) continue; // deleted between listing and read bytes += update.byteLength; try { @@ -543,7 +543,7 @@ export const fullMigrate = async (yhub, room, { force = false } = {}) => { { event: 'full.version-skipped', err, - docid: room.docid, + docid: docRef.docid, versionId: version.versionId, }, 'legacy s3 version is not a valid yjs update; skipping it', @@ -592,9 +592,9 @@ export const fullMigrate = async (yhub, room, { force = false } = {}) => { }; } const nongcDoc = Y.encodeStateAsUpdate(ydoc); - await yhub.persistence.store(room, { + await yhub.persistence.store(docRef, { lastClock: '0', - gcDoc: await yhub.computePool.mergeUpdates(true, [nongcDoc], { room }), + gcDoc: await yhub.computePool.mergeUpdates(true, [nongcDoc], { docRef }), nongcDoc, contentmap: Y.encodeContentMap(Y.mergeContentMaps(contentmaps)), contentids: Y.encodeContentIds(seen), @@ -602,7 +602,7 @@ export const fullMigrate = async (yhub, room, { force = false } = {}) => { } finally { ydoc.destroy(); } - await redis.sAdd(migratedSetKey(yhub), room.docid); + await redis.sAdd(migratedSetKey(yhub), docRef.docid); const result = { status: 'ok', versions: versions.length, @@ -613,7 +613,7 @@ export const fullMigrate = async (yhub, room, { force = false } = {}) => { durationMs: Date.now() - start, }; migrationLog.info( - { event: 'full.ok', docid: room.docid, ...result }, + { event: 'full.ok', docid: docRef.docid, ...result }, 'stored document history from s3 versions', ); return result; diff --git a/src/yhub-server/package-lock.json b/src/yhub-server/package-lock.json index c9914474e..62822fae9 100644 --- a/src/yhub-server/package-lock.json +++ b/src/yhub-server/package-lock.json @@ -7,7 +7,7 @@ "name": "yhub-server", "dependencies": { "@aws-sdk/client-s3": "3.1110.0", - "@y/hub": "0.7.0", + "@y/hub": "0.8.0", "@y/y": "14.0.0-rc.24", "jose": "6.2.8" }, @@ -504,15 +504,15 @@ "license": "ISC" }, "node_modules/@y/hub": { - "version": "0.7.0", - "resolved": "https://registry.npmjs.org/@y/hub/-/hub-0.7.0.tgz", - "integrity": "sha512-IPZKFEgJfuLBFDzJAa2ikPwbQHO7WTQgCSFsxqZa9HJdLn4Nbx8HJyLs9hZYtoquTG3MhMOASLifp60EYrghbw==", + "version": "0.8.0", + "resolved": "https://registry.npmjs.org/@y/hub/-/hub-0.8.0.tgz", + "integrity": "sha512-omBq00oLl7NVmB2qPMlKmTL/6ZTxP8hYZZHOX0RMsSKL+53JlbLJn9WVX04xtXYaM+Q81aLsy1/BRUwBYf4yOw==", "license": "AGPL-3.0 OR PROPRIETARY", "dependencies": { "@y-crdt/yn": "^0.1.4", "@y/protocols": "^1.0.6-rc.1", "@y/y": "^14.0.0-rc.24", - "lib0": "^1.0.0-rc.25", + "lib0": "^1.0.0-rc.27", "minio": "^8.0.6", "pino": "^10.3.1", "postgres": "^3.4.3", @@ -944,9 +944,9 @@ } }, "node_modules/lib0": { - "version": "1.0.0-rc.25", - "resolved": "https://registry.npmjs.org/lib0/-/lib0-1.0.0-rc.25.tgz", - "integrity": "sha512-UVxr56D1kVTx8P2Om5cAWXg2Pawk7JiOkCio+GLwlHcMko3FRE5xklOKB+t+fZSLA4ZLcVTZUKEIXPd9cRhEJg==", + "version": "1.0.0-rc.27", + "resolved": "https://registry.npmjs.org/lib0/-/lib0-1.0.0-rc.27.tgz", + "integrity": "sha512-bUaxkb+GjIKrPIw0OYU+NALy6sPY46wi86F13kJ9N9nMGiwUfk/6Kf5YTgdPFLfkjgIDIGhHzuInE+pMHyUgzQ==", "license": "MIT", "bin": { "0ecdsa-generate-keypair": "src/bin/0ecdsa-generate-keypair.js", diff --git a/src/yhub-server/package.json b/src/yhub-server/package.json index 2f9de5ce6..bb86fcd65 100644 --- a/src/yhub-server/package.json +++ b/src/yhub-server/package.json @@ -5,11 +5,12 @@ "scripts": { "start": "node server.js", "dev": "nodemon --delay 1 server.js", - "init-db": "node node_modules/@y/hub/bin/init-db.js" + "init-db": "node node_modules/@y/hub/bin/init-db.js", + "test": "node --test" }, "dependencies": { "@aws-sdk/client-s3": "3.1110.0", - "@y/hub": "0.7.0", + "@y/hub": "0.8.0", "@y/y": "14.0.0-rc.24", "jose": "6.2.8" }, diff --git a/src/yhub-server/permissions.js b/src/yhub-server/permissions.js new file mode 100644 index 000000000..9c819f3ea --- /dev/null +++ b/src/yhub-server/permissions.js @@ -0,0 +1,81 @@ +/** + * Docs' access policy, as yhub 0.8 permission objects. + * + * Kept apart from `server.js` so it can be read ā€” and tested ā€” without standing + * up redis and postgres: these three tables *are* the policy, and they are the + * only thing between a reader and someone else's document. + * + * A permission object states, per facet, what a subject may do with one + * document; yhub enforces every facet itself, on the websocket and on the REST + * routes alike. Masks are positional `crud` strings where `-` denies, so + * `'-r--'` is read-only and `'----'` grants nothing. + */ + +/** + * What a browser may do with a document, from the backend's verdict on it. + * `canEdit` is `abilities.update`; read access was settled by `abilities.retrieve` + * before this is reached. + * + * `awareness` is why Docs moved to yhub 0.8: a reader receives presence but + * never publishes it (suitenumerique/docs#2544 ā€” a read-only connection used to + * propagate cursors even though its document updates were dropped). yhub enforces + * it on both transports: it drops a read-only connection's awareness message on + * the socket, and refuses the `awareness` field of `PATCH /ydoc`. Note this is a + * deliberate departure from yhub's own default, which grants a reader `'-ru-'` + * and documents read-only cursors as a feature. + * + * `ydoc` withholds `c`, which yhub's migration table would grant an editor: `c` + * is reserved for "may populate the initial content", and in Docs that is + * `create-ydoc` with the admin token. `u` alone already creates the document on + * first write. + * + * No `history` facet, and that is load-bearing rather than an omission: it is + * what makes yhub refuse a `gc=false` connection with a 403. Docs users are + * served the garbage-collected document; the full history is the backend's. + * + * No `delete` facet: deleting a document is Django's, through the admin token. + * + * No `'*'` endpoint fallback, so everything not named here is denied. The browser + * calls exactly two routes ā€” the websocket, and `ydoc` for the http fallback. + * `activity`, `changeset`, `rollback`, `prune` and every custom endpoint are + * closed to it. Under 0.7 this fence was a `purpose != null` check in + * `getAccessType`, which `create-ydoc` slipped through by declaring no purpose. + */ +export const browserDocumentPermissions = (canEdit) => ({ + type: 'permissions:document:v1', + ydoc: canEdit ? '-ru-' : '-r--', + awareness: canEdit ? '-ru-' : '-r--', + endpoint: { + // `r` opens the socket, `u` admits document updates over it + ws: canEdit ? '-ru-' : '-r--', + // GET is `r` and PATCH is `u`; DELETE (`d`) stays out ā€” see `delete` above + ydoc: canEdit ? '-ru-' : '-r--', + }, +}); + +/** + * Django's admin token: everything, with one deliberate hole. `delete: ['soft']` + * and not `'hard'` ā€” yhub 0.8 made `DELETE /ydoc?hard=true` reachable over REST + * for the first time, and Docs keeps irreversible erasure programmatic, behind + * `reset-ydoc`, exactly as `yhub_services.delete_ydoc` describes. + */ +export const adminDocumentPermissions = { + type: 'permissions:document:v1', + ydoc: 'cru-', + awareness: '-ru-', + history: { from: 0 }, + delete: ['soft'], + endpoint: { '*': 'crud' }, +}; + +/** + * The global-scoped routes, served to anyone: the JWKS, which carries public keys + * and which the backend must read before it can authenticate anything we send it, + * and the two probes, which kubernetes calls with no cookie and no token. Read + * only, and named individually ā€” a global endpoint added later is denied until it + * is listed here. + */ +export const publicGlobalPermissions = { + type: 'permissions:global:v1', + endpoint: { ping: '-r--', ready: '-r--', jwks: '-r--' }, +}; diff --git a/src/yhub-server/permissions.test.js b/src/yhub-server/permissions.test.js new file mode 100644 index 000000000..072a42f97 --- /dev/null +++ b/src/yhub-server/permissions.test.js @@ -0,0 +1,162 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; + +// Reached by path rather than by name on purpose. `@y/hub`'s export map exposes +// only the package index, which pulls in uws, redis and postgres ā€” and the two +// functions this test needs to run yhub's real pipeline (normalize the plugin's +// answer, then ask it the gate's question) are not on it. Importing the module +// directly keeps the test to plain objects, and if yhub ever moves the file the +// failure is loud rather than silently vacuous. +import { + createDocumentPermissions, + hasPermissions, + normalizePermissions, +} from './node_modules/@y/hub/src/permissions.js'; + +import { + adminDocumentPermissions, + browserDocumentPermissions, + publicGlobalPermissions, +} from './permissions.js'; + +/** + * These tables are Docs' entire access policy, and yhub reads them literally ā€” + * a wrong character in a mask is a silent grant. So the assertions below are + * written as the questions yhub itself asks at each gate (`hasPermissions`), + * not as a snapshot of the objects: a table may be respelled freely, but it may + * not start answering a question differently. + */ +const reader = normalizePermissions(browserDocumentPermissions(false)); +const editor = normalizePermissions(browserDocumentPermissions(true)); +const admin = normalizePermissions(adminDocumentPermissions); + +const grants = (permissions, required) => + hasPermissions(permissions, createDocumentPermissions(required)); + +describe('read-only users and presence (suitenumerique/docs#2544)', () => { + it('lets a reader receive presence', () => { + assert.equal(grants(reader, { awareness: '-r--' }), true); + }); + + it('never lets a reader broadcast presence', () => { + // the requirement this migration exists for. yhub's own default grants a + // reader '-ru-' here; Docs deliberately does not. + assert.equal(grants(reader, { awareness: '--u-' }), false); + }); + + it('lets an editor broadcast presence', () => { + assert.equal(grants(editor, { awareness: '--u-' }), true); + }); +}); + +describe('the document itself', () => { + it('lets a reader read but not write', () => { + assert.equal(grants(reader, { ydoc: '-r--' }), true); + assert.equal(grants(reader, { ydoc: '--u-' }), false); + }); + + it('lets an editor write', () => { + assert.equal(grants(editor, { ydoc: '--u-' }), true); + }); + + it('opens the socket to a reader but admits no update over it', () => { + assert.equal(grants(reader, { endpoint: { ws: '-r--' } }), true); + assert.equal(grants(reader, { endpoint: { ws: '--u-' } }), false); + }); + + it('lets an editor send updates over the socket', () => { + assert.equal(grants(editor, { endpoint: { ws: '--u-' } }), true); + }); +}); + +describe('the http fallback route', () => { + it('lets a reader GET the document and no more', () => { + assert.equal(grants(reader, { endpoint: { ydoc: '-r--' } }), true); + assert.equal(grants(reader, { endpoint: { ydoc: '--u-' } }), false); + }); + + it('lets an editor GET and PATCH', () => { + assert.equal(grants(editor, { endpoint: { ydoc: '-r--' } }), true); + assert.equal(grants(editor, { endpoint: { ydoc: '--u-' } }), true); + }); + + it('never lets a browser DELETE the document', () => { + // deletion is Django's, through the admin token + assert.equal(grants(editor, { endpoint: { ydoc: '---d' } }), false); + assert.equal(grants(editor, { delete: ['soft'] }), false); + }); +}); + +describe('everything the browser must not reach', () => { + // there is no '*' fallback in the browser grant, so an endpoint yhub adds in + // a future release is denied until it is named ā€” this is the property that + // replaced 0.7's `purpose != null` check + for (const name of [ + 'activity', + 'changeset', + 'rollback', + 'prune', + 'create-ydoc', + 'migrate', + 'reset-connections', + 'restore-ydoc', + 'reset-ydoc', + 'some-endpoint-added-later', + ]) { + it(`refuses ${name} to an editor`, () => { + assert.equal(grants(editor, { endpoint: { [name]: '-r--' } }), false); + assert.equal(grants(editor, { endpoint: { [name]: 'c---' } }), false); + }); + } + + it('withholds history, which is what refuses a gc=false connection', () => { + assert.equal(grants(editor, { history: { from: 0 } }), false); + }); +}); + +describe('the admin token', () => { + it('reaches every endpoint, named or not', () => { + assert.equal( + grants(admin, { endpoint: { 'create-ydoc': 'crud', migrate: 'crud' } }), + true, + ); + }); + + it('reads and writes the document, with its full history', () => { + assert.equal(grants(admin, { ydoc: '-ru-' }), true); + assert.equal(grants(admin, { history: { from: 0 } }), true); + }); + + it('soft-deletes but never hard-deletes over REST', () => { + // DELETE /ydoc?hard=true became reachable over REST in yhub 0.8; Docs keeps + // irreversible erasure programmatic, behind reset-ydoc + assert.equal(grants(admin, { delete: ['soft'] }), true); + assert.equal(grants(admin, { delete: ['hard'] }), false); + }); + + it('is not granted rollback or prune', () => { + // 0.8 stopped implying them from write access; Docs does not use them + assert.equal(grants(admin, { history: { from: 0, rollback: true } }), false); + assert.equal(grants(admin, { history: { from: 0, prune: true } }), false); + }); +}); + +describe('the public global routes', () => { + const globalPerms = normalizePermissions(publicGlobalPermissions); + const globalGrants = (required) => + hasPermissions(globalPerms, { + type: 'permissions:global:v1', + ...required, + }); + + for (const name of ['ping', 'ready', 'jwks']) { + it(`serves ${name} to anyone, read only`, () => { + assert.equal(globalGrants({ endpoint: { [name]: '-r--' } }), true); + assert.equal(globalGrants({ endpoint: { [name]: '--u-' } }), false); + }); + } + + it('refuses a global endpoint it does not name', () => { + assert.equal(globalGrants({ endpoint: { anything: '-r--' } }), false); + }); +}); diff --git a/src/yhub-server/server.js b/src/yhub-server/server.js index 228e9ec52..cc068cb02 100644 --- a/src/yhub-server/server.js +++ b/src/yhub-server/server.js @@ -1,10 +1,12 @@ -import { createHash, createPublicKey, randomUUID } from 'node:crypto'; -import { readFileSync } from 'node:fs'; +import { createPublicKey } from 'node:crypto'; import { apiError, + checkPermissions, createApiEndpoint, createAuthPlugin, + createAuthorize, + createDocumentPermissions, createYHub, logger, } from '@y/hub'; @@ -19,6 +21,12 @@ import { } from 'jose'; import { secret } from './env.js'; +// Docs' access policy as yhub permission objects ā€” see permissions.test.js +import { + adminDocumentPermissions, + browserDocumentPermissions, + publicGlobalPermissions, +} from './permissions.js'; // legacy Django/S3 document store ā€” see migration.js and README.md import { SOFT_MIGRATION, @@ -116,16 +124,14 @@ const YHUB_S3_REGION_NAME = process.env.YHUB_S3_REGION_NAME; // URL scheme Docs already routes to the collaboration server. Hardcoded like // the audiences: the backend builds its urls with the same prefix. const API_PREFIX = 'collaboration'; -// Paths of the routes declared in `api` that are served to anyone: the JWKS, -// which carries public keys and which the backend must read before it can -// authenticate anything we send it, and the two probes, which kubernetes calls -// with no cookie and no token. readAuthInfo reads the raw request, without any -// route context, hence the duplication of the paths here. -const PUBLIC_PATHS = new Set([ - `/${API_PREFIX}/jwks/v1`, - `/${API_PREFIX}/ping/v1`, - `/${API_PREFIX}/ready/v1`, -]); +// The identity of a caller who is not signed in. It is a userid rather than the +// absence of one on purpose: yhub refuses the websocket upgrade of a caller that +// holds the write but has no identity (401 `unauthenticated`), because +// attributions carry the userid ā€” so without this an anonymous visitor could not +// edit a public document at all. Every anonymous edit is therefore attributed to +// one shared author; yhub's own userids are opaque strings and Docs' are UUIDs, +// so this cannot collide with a real one. +const ANONYMOUS_USERID = 'anonymous'; // What the readiness check gives a store before reporting it unreachable. Short // on purpose: the point of the probe is to answer, and answering "not ready" // early is more useful than holding the connection until kubelet times out. @@ -237,7 +243,9 @@ const JWKS = createRemoteJWKSet( const backendFetch = async (path, { cookie, origin }) => { const res = await fetch(`${COLLABORATION_BACKEND_BASE_URL}${path}`, { headers: { - cookie, + // an anonymous caller may have no session at all; `cookie: undefined` would + // reach the backend as the literal string "undefined" + ...(cookie ? { cookie } : {}), // a same-origin request carries no `Origin` ā€” forwarded when there is one, omitted // rather than sent empty, which is not a value the header is allowed to take ...(origin ? { origin } : {}), @@ -268,61 +276,65 @@ const backendFetch = async (path, { cookie, origin }) => { // the legacy store could not be reached (timeout, network, backpressure) ā€” // the same request later may well succeed, so it answers 503 rather than // silently starting an empty document on top of content that exists. -const seedFromLegacyStore = async (room) => { +const seedFromLegacyStore = async (docRef) => { try { // `yhub` is declared at the bottom of this file ā€” safe: auth callbacks only // fire once the server is up, i.e. after that assignment - await maybeMigrate(yhub, room); + await maybeMigrate(yhub, docRef); } catch (err) { if (!isPermanentFailure(err)) { throw apiError(503, 'Legacy document store is unavailable'); } // why it failed was logged once, at the attempt, inside maybeMigrate migrationLog.warn( - { event: 'seed.skipped', docid: room.docid, err: err?.message }, + { event: 'seed.skipped', docid: docRef.docid, err: err?.message }, 'admitting caller to a document that could not be migrated; it opens as new', ); } }; const auth = createAuthPlugin({ - // uws req is only valid synchronously ā€” read headers AND query before first await. - async readAuthInfo(req) { - const url = req.getUrl(); + /** + * Who is asking. Returning `null` here would mean "an anonymous caller" ā€” it is + * not a refusal, and `authorize` is still asked ā€” so every rejection below is a + * thrown `apiError(401)` and every unauthenticated caller gets an identity. + * Under 0.7 this callback denied by returning `null`, which is the one change + * in this file that would fail open rather than closed if it were missed. + */ + async authenticate(req) { + // uws req is only valid synchronously ā€” read headers AND the url before the + // first await. const authorization = req.getHeader('authorization'); const cookie = req.getHeader('cookie'); const origin = req.getHeader('origin'); - const gcOff = req.getQuery('gc') === 'false'; - // The JWKS and the probes are served to anyone (see PUBLIC_PATHS). This - // identity is granted their purposes and nothing else - // (getGlobalAccessType), and the check is on their exact paths. - if (PUBLIC_PATHS.has(url)) { - return { userid: 'anonymous' }; - } + // 0.8 dropped `accessPurpose`, and `authorize` is told the scope and the + // resource but not the route. The seed below needs to know one thing about + // the route ā€” whether this is the `migrate` call ā€” so the endpoint name is + // read off the path here, where the request still exists, and carried on the + // identity. `/collaboration/{name}/{version}/...` ā†’ index 2. + const endpoint = req.getUrl().split('/')[2] ?? ''; + if (authorization !== '') { // backend-to-server call: RS256 JWT signed by Django, verified against // its JWKS. A browser cannot attach an Authorization header to a ws // upgrade or a credentialed cross-origin fetch, so this never shadows a - // real user session. present-but-invalid fails here (401) instead of - // falling through to the cookie flow, which would mask a - // misconfiguration as an origin error. + // real user session. present-but-invalid is rejected here (401) instead of + // falling through to the cookie flow, which would mask a misconfiguration + // as an anonymous browse. const token = authorization.startsWith('Bearer ') ? authorization.slice('Bearer '.length) : authorization; + let payload; try { // clockTolerance absorbs Django's cache-at-exp race (the admin token // is cached for exactly its lifetime, so it can arrive here moments // after exp) plus small clock skew ā€” without it a kick would be // silently dropped as a 401. - const { payload } = await jwtVerify(token, JWKS, { + ({ payload } = await jwtVerify(token, JWKS, { algorithms: ['RS256'], audience: YHUB_AUDIENCE, clockTolerance: 5, - }); - // admin tokens act as the "system" user (no per-user admin identities yet) - return payload.admin === true - ? { userid: 'system', admin: true } - : null; + })); } catch (err) { // jose tags every token-validation failure with an `ERR_Jā€¦` code (bad // signature, expired, wrong or missing audience) ā€” those are permanent, @@ -337,22 +349,34 @@ const auth = createAuthPlugin({ ) { throw apiError(503, 'Token verification keys are unavailable'); } - return null; + throw apiError(401, 'Invalid token'); } + if (payload.admin !== true) { + // a valid token that is not an admin one is a credential we refuse, not + // an anonymous caller: returning null here ā€” which is what 0.7 did ā€” + // would now silently downgrade a service call into a public browse + throw apiError(401, 'Not an admin token'); + } + // admin tokens act as the "system" user (no per-user admin identities yet) + return { userid: 'system', admin: true, endpoint }; } - if (gcOff) return null; // full-history connections: not for Docs users // No origin check here: `server.cors` below is the allowlist, and yhub applies it to the // websocket upgrade and to every REST request before this runs. Checking it a second time // would also refuse the http fallback's polls ā€” a same-origin `fetch` GET carries no // `Origin` header at all, so on a deployment where the page and /collaboration/ share a // host every round would 401 while the PATCH beside it succeeded. - if (!cookie) return null; // was 4001 'No cookies' + // + // A caller with no cookie at all is anonymous too: the probes arrive that way, and so does + // a public document opened in a fresh browser. What they may do is `authorize`'s answer. + if (!cookie) { + return { userid: ANONYMOUS_USERID, origin, endpoint }; + } try { const user = await backendFetch('/api/v1.0/users/me/', { cookie, origin, }); - return { userid: String(user.id), cookie, origin }; // MUST be string (yhub server.js:667) + return { userid: String(user.id), cookie, origin, endpoint }; } catch (err) { // Only a genuine "not signed in" falls back to the anonymous identity. // On backend failure (5xx/network) still refuse to admit the connection ā€” @@ -363,82 +387,75 @@ const auth = createAuthPlugin({ if (err?.status !== 401 && err?.status !== 403) { throw apiError(503, 'Authentication backend is unavailable'); } - // anonymous (public docs): stable per-session id ā€” random ids would mint a new - // permanent attribution identity per reconnect - const anon = createHash('sha256') - .update(cookie) - .digest('base64url') - .slice(0, 16); - return { userid: `anon:${anon}`, cookie, origin }; + // the cookie is kept: it is a session the backend still answers document + // questions about, which is how a public document resolves below + return { userid: ANONYMOUS_USERID, cookie, origin, endpoint }; } }, - // Authorizes the global-scoped endpoints: the JWKS and the two probes, all - // of them read-only and public. Anything else is refused here. - async getGlobalAccessType(authInfo, purpose) { - return purpose === 'jwks' || purpose === 'ping' || purpose === 'ready' - ? 'r' - : null; - }, - async getAccessType(authInfo, { org, docid, branch }, purpose) { - if (authInfo.admin === true) { - // Django's admin token: full access. It still goes through the legacy - // seed, on the same terms as a user (default purpose only, so a - // `migrate` call is not seeded out from under fullMigrate). Without it a - // backend read of an unmigrated document would answer with an *empty* - // doc, and a create-ydoc against one would write a second lineage next - // to the legacy content the first user access is about to seed in. - // Access itself is never in question here ā€” the token already granted it. - // The same org/branch fence the user path applies below. The admin token - // is the only identity that can name an arbitrary org or branch, and the - // legacy store is branchless ā€” `{docid}/file` *is* main ā€” so seeding any - // other room would write main's content into an orphan room, and the - // per-docid verdict cache would then report that docid as done and leave - // the real room empty. - if ( - SOFT_MIGRATION && - purpose == null && - org === ORG && - branch === 'main' && - UUID4.test(docid) - ) { - await seedFromLegacyStore({ org, docid, branch }); + /** + * What that caller may do. One handler per scope; a scope without a handler + * denies, which is what closes `org` and `branch` here. Denial is a value ā€” + * returning `null`, never throwing: an error thrown during a websocket recheck + * disconnects with the transient close code 1013 instead of the revoke code + * 4401. + */ + authorize: createAuthorize({ + async document({ org, docid, branch }, user) { + if (user?.admin === true) { + // Django's admin token: full access. It still goes through the legacy + // seed, on the same terms as a user, but not for `migrate` ā€” that call + // replays the whole version history itself, and seeding first would put + // the newest version in the room underneath it. Without the seed a + // backend read of an unmigrated document would answer with an *empty* + // doc, and a create-ydoc against one would write a second lineage next + // to the legacy content the first user access is about to seed in. + // Access itself is never in question here ā€” the token already granted it. + // The same org/branch fence the user path applies below. The admin token + // is the only identity that can name an arbitrary org or branch, and the + // legacy store is branchless ā€” `{docid}/file` *is* main ā€” so seeding any + // other room would write main's content into an orphan room, and the + // per-docid verdict cache would then report that docid as done and leave + // the real room empty. + if ( + SOFT_MIGRATION && + user.endpoint !== 'migrate' && + org === ORG && + branch === 'main' && + UUID4.test(docid) + ) { + await seedFromLegacyStore({ org, docid, branch }); + } + return adminDocumentPermissions; } - return 'rw'; - } - // Regular users only get access for the default purpose ā€” custom-endpoint - // purposes (reset-connections, migrate) are backend-internal. Loose != on - // purpose: ws upgrades and rechecks pass undefined, built-in rest - // endpoints null. - if ( - org !== ORG || - branch !== 'main' || - !UUID4.test(docid) || - purpose != null - ) { - return null; - } - let doc; - try { - doc = await backendFetch(`/api/v1.0/documents/${docid}/`, authInfo); - } catch (err) { - // the backend answered "no": a real, permanent denial (403 Forbidden) - if (err?.status === 401 || err?.status === 403 || err?.status === 404) { + if (org !== ORG || branch !== 'main' || !UUID4.test(docid)) { return null; } - // it did not answer at all ā€” say so, so the caller retries instead of - // reading a 5xx or a network blip as a permission decision - throw apiError(503, 'Document authorization backend is unavailable'); - } - if (!doc.abilities?.retrieve) { - return null; - } - // the backend has already decided the caller may read this document; the - // seed only decides what is in it - if (SOFT_MIGRATION) { - await seedFromLegacyStore({ org, docid, branch }); - } - return doc.abilities.update ? 'rw' : 'r'; - }, + let doc; + try { + doc = await backendFetch(`/api/v1.0/documents/${docid}/`, user); + } catch (err) { + // the backend answered "no": a real, permanent denial (403 Forbidden) + if (err?.status === 401 || err?.status === 403 || err?.status === 404) { + return null; + } + // it did not answer at all ā€” say so, so the caller retries instead of + // reading a 5xx or a network blip as a permission decision + throw apiError(503, 'Document authorization backend is unavailable'); + } + if (!doc.abilities?.retrieve) { + return null; + } + // the backend has already decided the caller may read this document; the + // seed only decides what is in it + if (SOFT_MIGRATION) { + await seedFromLegacyStore({ org, docid, branch }); + } + return browserDocumentPermissions(doc.abilities.update === true); + }, + async global() { + return publicGlobalPermissions; + }, + }), }); // Mimic the old y-provider REST responses (JSON, not yhub's lib0-any @@ -452,9 +469,9 @@ const jsonResponse = (status, body) => // Does the room hold anything? Covers persisted rows and the messages still on // the stream, which is what makes it an answer about the content rather than // about the storage. -const hasContent = async (yhub, room) => { +const hasContent = async (yhub, docRef) => { const { gcDoc } = await yhub.getDoc( - room, + docRef, { gc: true, nongc: false }, { gcOnMerge: false }, ); @@ -474,9 +491,9 @@ const hasContent = async (yhub, room) => { // yhub has no such operation, a hard deletion is final for the room and even // `restoreDoc` refuses it. Here the document id belongs to a Django document // that goes on living, so the room has to be usable again. -const eraseContent = async (yhub, room, by) => { - await yhub.deleteDoc(room, { hard: true, by }); - await yhub.persistence.deleteTombstone(room); +const eraseContent = async (yhub, docRef, by) => { + await yhub.deleteDoc(docRef, { hard: true, by }); + await yhub.persistence.deleteTombstone(docRef); }; const readyLog = logger.child({ module: 'readiness' }); @@ -512,7 +529,6 @@ const api = [ // holds perfectly good websocket connections every time a store blinks. createApiEndpoint('ping', { scope: 'global', - accessPurpose: 'ping', get: { handler: () => jsonResponse(200, { status: 'pong' }), }, @@ -524,7 +540,6 @@ const api = [ // is the whole difference with the liveness probe above. createApiEndpoint('ready', { scope: 'global', - accessPurpose: 'ready', get: { handler: async (req) => { // both at once: a probe is not the place to add the latency of one @@ -551,7 +566,6 @@ const api = [ // the other's key, so either can be rolled without the other being changed. createApiEndpoint('jwks', { scope: 'global', - accessPurpose: 'jwks', get: { // an empty set when no key is configured: honest, and the backend // refuses our (equally absent) tokens rather than trusting anything @@ -563,11 +577,10 @@ const api = [ }), // POST /collaboration/reset-connections/v1/{org}/{docid} ā€” replaces // y-provider's /collaboration/api/reset-connections/?room=. Doc-scoped, so - // the room comes from the path; access is gated to the admin token via the - // 'reset-connections' purpose in getAccessType. uws routes are exact: a - // trailing slash 404s. + // the docRef comes from the path; access is the admin token's alone, because + // the browser's grant names no `reset-connections` endpoint and has no `'*'` + // fallback. uws routes are exact: a trailing slash 404s. createApiEndpoint('reset-connections', { - accessPurpose: 'reset-connections', post: { handler: async (req) => { const userId = req.headers['x-user-id'] || null; @@ -577,10 +590,10 @@ const api = [ if (!UUID4.test(req.docid)) { return jsonResponse(400, { error: 'Room name is invalid' }); } - // in-place recheck: every yhub server re-runs getAccessType per + // in-place recheck: every yhub server re-runs `authorize` per // matching connection and closes 4401 only when the access changed ā€” // no reconnect churn for unaffected clients - await req.yhub.recheckAuth(req.room, { + await req.yhub.recheckAuth(req.docRef, { users: userId ? [userId] : null, }); return jsonResponse(200, { message: 'Connections reset' }); @@ -593,7 +606,6 @@ const api = [ // 'migrate' access purpose, since it writes history and reads the legacy // store. createApiEndpoint('migrate', { - accessPurpose: 'migrate', post: { handler: async (req) => { if (req.org !== ORG) { @@ -614,7 +626,7 @@ const api = [ // Only safe while its clock-0 row is still there: once compaction has // folded that row away, a second replay attributes the same content a // second time and the activity timestamps become ambiguous. - const { status, ...stats } = await fullMigrate(req.yhub, req.room, { + const { status, ...stats } = await fullMigrate(req.yhub, req.docRef, { force: req.query.force === 'true', }); // `status` is what a backfill driver records per document: 'ok', @@ -653,6 +665,14 @@ const api = [ createApiEndpoint('create-ydoc', { post: { handler: async (req) => { + // The endpoint facet already admitted this caller ā€” only the admin + // token names this route ā€” but the write itself is a separate facet, and + // yhub's contract is that a handler states the facets it touches. Cheap, + // and it keeps the grant honest if the tables above ever widen. + checkPermissions( + req.permissions, + createDocumentPermissions({ ydoc: '--u-' }), + ); if (req.org !== ORG) { return jsonResponse(400, { error: 'Unknown org' }); } @@ -660,7 +680,7 @@ const api = [ return jsonResponse(400, { error: 'Room name is invalid' }); } if (req.branch !== 'main') { - // cookie users are main-only via getAccessType, but the admin + // cookie users are main-only via `authorize`, but the admin // token bypasses it ā€” reject explicitly so an admin create can't // seed an orphan non-main room (and dodge the 409 check, which is // branch-scoped) @@ -693,7 +713,7 @@ const api = [ // content then appears twice. Accepted: Django creates each doc // once, and a duplicated seed is user-fixable, unlike corruption. const { gcDoc } = await req.yhub.getDoc( - req.room, + req.docRef, { gc: true, nongc: false }, { gcOnMerge: false }, ); @@ -718,7 +738,7 @@ const api = [ userid, customAttributions: [], }, - { room: req.room }, + { docRef: req.docRef }, ); } catch { // a malformed update makes the compute worker throw (yhub logs @@ -735,7 +755,7 @@ const api = [ } // on a fresh room this creates the stream, schedules compaction, and // fans out to any live subscribers ā€” nothing else to do - await req.yhub.stream.addMessage(req.room, { + await req.yhub.stream.addMessage(req.docRef, { type: 'ydoc:update:v1', contentmap: result.contentmap, update: result.update, @@ -753,7 +773,6 @@ const api = [ // 'restore' purpose ā€” a document leaves the trashbin because the backend // says so, never because an editor asked. createApiEndpoint('restore-ydoc', { - accessPurpose: 'restore', post: { handler: async (req) => { if (req.org !== ORG) { @@ -764,14 +783,14 @@ const api = [ } if (req.branch !== 'main') { // as in create-ydoc: the admin token is not fenced to main by - // getAccessType, and a deletion is recorded per branch + // `authorize`, and a deletion is recorded per branch return jsonResponse(400, { error: 'Unknown branch' }); } // read the deletion before undoing it: `restoreDoc` throws a plain // Error for a document whose content was erased, and that is a // conflict to report as one ā€” catching around the call would turn // every failure alike, a database outage included, into the same answer - const tombstone = await req.yhub.persistence.retrieveTombstone(req.room); + const tombstone = await req.yhub.persistence.retrieveTombstone(req.docRef); if (tombstone == null) { // not an error: the backend restores a whole subtree, of which only // the part that was deleted with it has anything to put back @@ -783,7 +802,7 @@ const api = [ if (tombstone.hard || tombstone.purgedAt != null) { return jsonResponse(409, { error: 'Document content was erased' }); } - await req.yhub.restoreDoc(req.room); + await req.yhub.restoreDoc(req.docRef); return jsonResponse(200, { message: 'Document restored', restored: true, @@ -801,7 +820,6 @@ const api = [ // and admin-only, like the deletions it is built on: this destroys content // with no way back. createApiEndpoint('reset-ydoc', { - accessPurpose: 'reset', post: { handler: async (req) => { if (req.org !== ORG) { @@ -814,22 +832,28 @@ const api = [ return jsonResponse(400, { error: 'Unknown branch' }); } const by = req.headers['x-user-id'] || req.authInfo.userid; + // No `checkPermissions` here, deliberately. The honest facet for what + // follows would be `delete: ['hard']`, and the admin grant withholds + // `'hard'` on purpose so that `DELETE /ydoc?hard=true` stays refused over + // REST. The erasure below reaches `deleteDoc` programmatically, which the + // `delete` facet does not gate; the endpoint facet ā€” this route is named + // by no browser grant ā€” is what admits the caller. // Nothing compacts this room while the erasure runs: this drops the // task already waiting for it and refuses to enqueue another, which // leaves one writer to race with ā€” a task a worker had claimed before // this call. The tombstone barrier covers it right up to the moment // the room is made writable again, so it can only land after that, // and the second pass below is what picks it up. - await req.yhub.stream.disableCompaction(req.room); + await req.yhub.stream.disableCompaction(req.docRef); try { - await eraseContent(req.yhub, req.room, by); - if (await hasContent(req.yhub, req.room)) { + await eraseContent(req.yhub, req.docRef, by); + if (await hasContent(req.yhub, req.docRef)) { resetLog.warn( { docid: req.docid }, 'content came back while it was being erased, erasing again', ); - await eraseContent(req.yhub, req.room, by); - if (await hasContent(req.yhub, req.room)) { + await eraseContent(req.yhub, req.docRef, by); + if (await hasContent(req.yhub, req.docRef)) { // saying it is erased when it is not is the one answer this // endpoint must never give return jsonResponse(500, { @@ -840,7 +864,7 @@ const api = [ } finally { // even on failure: leaving compaction off would freeze the room for // every later edit, a worse state than the one we came to fix - await req.yhub.stream.enableCompaction(req.room); + await req.yhub.stream.enableCompaction(req.docRef); } return jsonResponse(200, { message: 'Document content erased' }); }, @@ -876,13 +900,17 @@ const touchDocument = async (docid) => { // traffic of someone merely opening a document never reaches it. Since yhub // 0.5.0 it is handed the room of the task alongside the merged document. const workerEvents = { - docUpdate: ({ room }) => { + docUpdate: ({ docRef }) => { // Django knows the documents of this org, on the main branch, by their uuid - if (room.org !== ORG || room.branch !== 'main' || !UUID4.test(room.docid)) { + if ( + docRef.org !== ORG || + docRef.branch !== 'main' || + !UUID4.test(docRef.docid) + ) { return; } // deliberately not awaited: a slow backend must not hold the worker - touchDocument(room.docid); + touchDocument(docRef.docid); }, }; @@ -965,7 +993,7 @@ const yhub = await createYHub({ api, apiPrefix: API_PREFIX, // What a browser may reach this server from, applied by yhub to the websocket upgrade - // and to every REST route ā€” the only origin check there is, `readAuthInfo` no longer + // and to every REST route ā€” the only origin check there is, `authenticate` no longer // does its own. `credentials` is what lets the http fallback send the session cookie // on a cross-origin `fetch`; it is also why the list has to be concrete, browsers // refusing "*" together with Access-Control-Allow-Credentials.