Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions dev/docs/yjs.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,12 @@ sequenceDiagram
G-->>HP: user identity
HP->>G: GET drives/{driveId}/items/{itemId}/permissions
G-->>HP: allowed actions
alt 401 / 403 / 404
HP-->>C: reject - access denied
alt 401
HP-->>C: refuse - token-invalid
else 403 / 404
HP-->>C: refuse - access-denied
else other failure
HP-->>C: refuse - server-error
else
alt write action present
HP-->>C: accept (read-write)
Expand Down
41 changes: 33 additions & 8 deletions packages/web-pkg/src/composables/yjs/useYjsSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,19 @@ export const YjsStatus = {

export type YjsStatus = (typeof YjsStatus)[keyof typeof YjsStatus]

/**
* Why the Yjs server refused the handshake. Mirrors `DeniedReason` in
* `services/yjs/src/server.ts`, keep the two in sync.
*/
export const YjsDeniedReason = {
TokenInvalid: 'token-invalid',
AccessDenied: 'access-denied',
MalformedDocument: 'malformed-document',
ServerError: 'server-error'
} as const

export type YjsDeniedReason = (typeof YjsDeniedReason)[keyof typeof YjsDeniedReason]

export interface YjsSessionOptions {
/** The file the session is bound to. Its id forms the room name, its etag drives staleness detection. */
resource: MaybeRefOrGetter<Resource>
Expand Down Expand Up @@ -723,6 +736,22 @@ export function useYjsSession(options: YjsSessionOptions): YjsSession {
}
type ContentReporter = ReturnType<typeof createContentReporter>

/** The user-facing side of a refused handshake. */
function deniedMessage(reason: string): string {
switch (reason) {
case YjsDeniedReason.TokenInvalid:
return $gettext(
'Your session expired and collaborative editing stopped. Reload the page to collaborate again.'
)
case YjsDeniedReason.AccessDenied:
return $gettext('Collaborative editing is not available for this file.')
default:
return $gettext(
'The collaboration server refused the connection. Editing continues without collaboration.'
)
}
}

/** Connects a Hocuspocus provider and arms the connect timeout. */
function connectRemote(doc: Y.Doc, name: string, serverUrl: string) {
let connectTimer: number | undefined
Expand All @@ -741,11 +770,8 @@ export function useYjsSession(options: YjsSessionOptions): YjsSession {
},
onAuthenticationFailed({ reason }) {
console.error('[yjs] auth failed:', reason)
// Surfaced as an error so the user sees the reason rather than a
// silent disconnect.
error.value = new Error(reason || $gettext('authentication failed'))
isLockedForReload.value = true
clearConnectTimer()
error.value = new Error(deniedMessage(reason))

// Stop retrying: `permissionDeniedHandler` leaves `shouldConnect`
// true, so the socket layer keeps reconnecting - and a later attempt
Expand All @@ -754,10 +780,9 @@ export function useYjsSession(options: YjsSessionOptions): YjsSession {
stopProvider(prov)

// Hydrate and release the loading gate, but only for a failed
// *opening* connect. A token expiring mid-session leaves a live,
// populated document; re-running the hydration checks there could
// plant a stale flag with a claim this now read-only client will
// never act on.
// *opening* connect. Mid-session the document is already live and
// populated; re-running the hydration checks there could plant a
// stale flag against a room this client has just left.
if (unref(isReady)) return
void onProviderSynced(doc, null, prov.awareness!)
},
Expand Down
54 changes: 48 additions & 6 deletions packages/web-pkg/tests/unit/composables/yjs/useYjsSession.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type { Resource } from '@opencloud-eu/web-client'
import {
buildYjsRoomName,
useYjsSession,
YjsDeniedReason,
type YjsAdapter,
type YjsSession
} from '../../../../src/composables/yjs'
Expand Down Expand Up @@ -516,18 +517,59 @@ describe('useYjsSession — remote mode (yjsServerUrl set)', () => {
expect(unref(s.session.isReady)).toBe(true)
})

it('surfaces an auth failure as an error, locks read-only and releases the loading gate', async () => {
it('surfaces an auth failure as an error and releases the loading gate', async () => {
silenceConsoleError()
const s = setupSession({ yjsServerUrl: 'wss://example.test/yjs' })
await flushPromises()
providerInstances[0].triggerAuthFailed('token expired')
providerInstances[0].triggerAuthFailed(YjsDeniedReason.TokenInvalid)
await flushPromises()

expect(unref(s.session.error)?.message).toBe('token expired')
expect(unref(s.session.isLockedForReload)).toBe(true)
expect(unref(s.session.error)?.message).toContain('Your session expired')
expect(unref(s.session.isReady)).toBe(true)
})

// A refused room is not a verdict on write permission: the caller derives
// read-only from the resource's own permissions, and the server's Graph
// probe can disagree with it (a version skew between the two is enough).
// Locking the editor over that took a working document away from the user.
it.each([
YjsDeniedReason.TokenInvalid,
YjsDeniedReason.AccessDenied,
YjsDeniedReason.MalformedDocument,
YjsDeniedReason.ServerError,
'something-we-never-shipped'
])('continues locally instead of locking read-only on %s', async (reason) => {
silenceConsoleError()
const s = setupSession({
yjsServerUrl: 'wss://example.test/yjs',
currentContent: 'my important notes'
})
await flushPromises()
providerInstances[0].triggerAuthFailed(reason)
await flushPromises()

expect(unref(s.session.isLockedForReload)).toBe(false)
expect(unref(s.session.error)).not.toBeNull()
expect(s.ydoc!.getText(SHARED_TEXT_KEY).toString()).toBe('my important notes')
})

// The reason codes exist so the message can name the actual cause. An
// expired token is a "reload me", a denial is not.
it('tells an expired token apart from a denied document', async () => {
silenceConsoleError()
const expired = setupSession({ yjsServerUrl: 'wss://example.test/yjs' })
await flushPromises()
providerInstances[0].triggerAuthFailed(YjsDeniedReason.TokenInvalid)
await flushPromises()

const denied = setupSession({ yjsServerUrl: 'wss://example.test/yjs' })
await flushPromises()
providerInstances[1].triggerAuthFailed(YjsDeniedReason.AccessDenied)
await flushPromises()

expect(unref(expired.session.error)?.message).not.toBe(unref(denied.session.error)?.message)
})

// Regression: the gate was released on an empty Y.Doc, so an expired token
// rendered the user's document as a blank page next to a toast. They had
// just fetched the file over WebDAV, so showing it is neither a leak nor a
Expand Down Expand Up @@ -659,13 +701,13 @@ describe('useYjsSession — unreachable Yjs server', () => {
silenceConsoleError()
const s = setupSession({ yjsServerUrl })
await flushPromises()
providerInstances[0].triggerAuthFailed('token expired')
providerInstances[0].triggerAuthFailed(YjsDeniedReason.TokenInvalid)
vi.advanceTimersByTime(20_000)
await flushPromises()

// The auth reason survives: the timeout must not overwrite it with its own
// "server could not be reached" message.
expect(unref(s.session.error)?.message).toBe('token expired')
expect(unref(s.session.error)?.message).toContain('Your session expired')
// Auth failure disconnects too (see below), so exactly one disconnect - the
// timeout did not add a second.
expect(providerInstances[0].disconnect).toHaveBeenCalledOnce()
Expand Down
146 changes: 104 additions & 42 deletions services/yjs/src/server.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Server } from '@hocuspocus/server'
import type { onAuthenticatePayload } from '@hocuspocus/server'

const port = parseInt(process.env.PORT ?? '1234', 10)
const opencloudUrl = (process.env.OPENCLOUD_URL ?? '').replace(/\/$/, '')
Expand Down Expand Up @@ -35,6 +36,32 @@ const MAX_DOCUMENT_NAME_LENGTH = 512
// handshakes hanging in `onAuthenticate` indefinitely.
const GRAPH_TIMEOUT_MS = 10_000

// Machine-readable codes for a refused handshake. Hocuspocus relays
// `error.reason` to the client's `onAuthenticationFailed` and substitutes
// "permission-denied" for a plain Error.
const DeniedReason = {
/** The token was rejected. A renewed one may well be accepted. */
TokenInvalid: 'token-invalid',
/** The user does not have this file, or it does not exist. */
AccessDenied: 'access-denied',
/** The room name does not resolve to a file id. */
MalformedDocument: 'malformed-document',
/** The probe itself failed: OpenCloud unreachable, 5xx, unusable body. */
ServerError: 'server-error'
} as const

type DeniedReason = (typeof DeniedReason)[keyof typeof DeniedReason]

/** An Error carrying the `reason` code Hocuspocus relays to the client. */
function refuse(reason: DeniedReason, detail: string): Error {
return Object.assign(new Error(`${reason}: ${detail}`), { reason })
}

/** True for an error that already carries a client-facing `reason` code. */
function isRefusal(e: unknown): e is Error & { reason: DeniedReason } {
return e instanceof Error && typeof (e as { reason?: unknown }).reason === 'string'
}

function hslToHex(h: number, s: number, l: number): string {
const a = s * Math.min(l, 1 - l)
function channel(n: number): string {
Expand Down Expand Up @@ -62,7 +89,8 @@ async function validateTokenAgainstOpenCloud(token: string): Promise<GraphUser>
})
if (!res.ok) {
const detail = await res.text().catch(() => '')
throw new Error(`graph /me returned ${res.status}: ${detail.slice(0, 200)}`)
const reason = res.status === 401 ? DeniedReason.TokenInvalid : DeniedReason.ServerError
throw refuse(reason, `graph /me returned ${res.status}: ${detail.slice(0, 200)}`)
}
return res.json() as Promise<GraphUser>
}
Expand Down Expand Up @@ -95,19 +123,24 @@ function parseDocumentId(documentName: string): { driveId: string; itemId: strin
const fileId = versionSep >= 0 ? scopedFileId.slice(0, versionSep) : scopedFileId
const sep = fileId.indexOf('!')
if (sep <= 0 || sep === fileId.length - 1) {
throw new Error(`malformed documentName=${JSON.stringify(documentName)}`)
throw refuse(
DeniedReason.MalformedDocument,
`documentName=${JSON.stringify(documentName)} is not a file id`
)
}
return { driveId: fileId.slice(0, sep), itemId: fileId }
}

// Probes OC's Graph API for the user's effective access to the file. Returns
// `{ canWrite }` on success; `null` when OC denies access (401/403/404).
// `{ canWrite }`, or refuses with the `reason` the client needs to tell a
// stale token (401) from a file it may not touch (403/404) - the two call for
// opposite reactions there, and a single "denied" would blur them.
//
// The permissions endpoint reports the effective action set (top-level
// `@libre.graph.permissions.actions.allowedValues`, the merged PermissionSet
// that also backs WebDAV's `oc:permissions`) and 404s for a file the user
// cannot see, which is what makes it the authorization gate.
async function probeFileAccess(token: string, documentName: string): Promise<FileAccess | null> {
async function probeFileAccess(token: string, documentName: string): Promise<FileAccess> {
const { driveId, itemId } = parseDocumentId(documentName)
// `$select` on the action set is what keeps this cheap: the endpoint returns
// right after resolving the effective actions instead of also listing user,
Expand All @@ -123,12 +156,25 @@ async function probeFileAccess(token: string, documentName: string): Promise<Fil
signal: AbortSignal.timeout(GRAPH_TIMEOUT_MS)
})

if (res.status === 401 || res.status === 403 || res.status === 404) {
return null
}
if (!res.ok) {
const detail = await res.text().catch(() => '')
throw new Error(`graph permissions returned ${res.status}: ${detail.slice(0, 200)}`)
let reason: DeniedReason
switch (res.status) {
case 401:
reason = DeniedReason.TokenInvalid
break
case 403:
case 404:
reason = DeniedReason.AccessDenied
break
default:
reason = DeniedReason.ServerError
}
throw refuse(
reason,
`graph permissions returned ${res.status} for document=` +
`${JSON.stringify(documentName)}: ${detail.slice(0, 200)}`
)
}

const body = (await res.json()) as Record<string, unknown>
Expand All @@ -138,6 +184,45 @@ async function probeFileAccess(token: string, documentName: string): Promise<Fil
return { canWrite: allowed.includes(WRITE_ACTION) }
}

/**
* The handshake's authentication and authorization: who is connecting, and
* what they may do to this file. Refusals carry a `DeniedReason`.
*/
async function authenticate({ token, documentName, connectionConfig }: onAuthenticatePayload) {
if (!token) {
throw refuse(DeniedReason.TokenInvalid, 'no token in the handshake')
}
if (documentName.length > MAX_DOCUMENT_NAME_LENGTH) {
throw refuse(DeniedReason.MalformedDocument, `documentName too long (${documentName.length})`)
}

const me = await validateTokenAgainstOpenCloud(token)
const id = me.id ?? me.userPrincipalName ?? me.mail ?? 'unknown'

// Authorization: does this user have the file at all, and may they write it.
const access = await probeFileAccess(token, documentName)

const readOnly = !access.canWrite

// Writes are gated on `connectionConfig.readOnly`, which Hocuspocus reads
// when it builds the Connection. The hook's return value only feeds
// `context`, so setting it there would leave the connection writable.
connectionConfig.readOnly = readOnly

console.log(
`[onAuthenticate] document=${JSON.stringify(documentName)} user="${me.displayName ?? id}" ` +
`id="${id}" readOnly=${readOnly}`
)
return {
readOnly,
user: {
id,
displayName: me.displayName ?? me.userPrincipalName ?? id,
color: deterministicColor(id)
}
}
}

const server = new Server({
port,
address: '0.0.0.0',
Expand Down Expand Up @@ -173,41 +258,18 @@ const server = new Server({
throw undefined
},

async onAuthenticate({ token, documentName, connectionConfig }) {
if (!token) {
throw new Error('missing token')
}
if (documentName.length > MAX_DOCUMENT_NAME_LENGTH) {
throw new Error(`documentName too long (${documentName.length})`)
}

const me = await validateTokenAgainstOpenCloud(token)
const id = me.id ?? me.userPrincipalName ?? me.mail ?? 'unknown'

// Authorization: does this user have the file at all, and may they write it.
const access = await probeFileAccess(token, documentName)
if (access === null) {
throw new Error(`access denied for document=${JSON.stringify(documentName)}`)
}

const readOnly = !access.canWrite

// Writes are gated on `connectionConfig.readOnly`, which Hocuspocus reads
// when it builds the Connection. The hook's return value only feeds
// `context`, so setting it there would leave the connection writable.
connectionConfig.readOnly = readOnly

console.log(
`[onAuthenticate] document=${JSON.stringify(documentName)} user="${me.displayName ?? id}" ` +
`id="${id}" readOnly=${readOnly}`
)
return {
readOnly,
user: {
id,
displayName: me.displayName ?? me.userPrincipalName ?? id,
color: deterministicColor(id)
async onAuthenticate(payload) {
try {
return await authenticate(payload)
} catch (e) {
const doc = JSON.stringify(payload.documentName)
if (isRefusal(e)) {
const log = e.reason === DeniedReason.ServerError ? console.error : console.warn
log(`[onAuthenticate] refused document=${doc} reason=${e.reason}: ${e.message}`)
throw e
}
console.error(`[onAuthenticate] unexpected error document=${doc}:`, e)
throw refuse(DeniedReason.ServerError, e instanceof Error ? e.message : String(e))
}
},

Expand Down