diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index c85a7c84f..ad5592ec4 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -67,3 +67,26 @@ jobs: - name: Lint run: npm run lint + + # The build the release tarball and a manual install use, with its guard at + # the end. Docker and the Lite images build another way (webpack, export), so + # nothing else would notice this one breaking before a release does. + standalone-build: + name: Standalone build and guard + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - name: Checkout + uses: actions/checkout@v7 + + - name: Set up Node + uses: actions/setup-node@v7 + with: + node-version: 22 + cache: npm + + - name: Install dependencies + run: npm ci + + - name: Build + run: npm run build diff --git a/components/email/__tests__/rich-text-editor-quote-caret.test.tsx b/components/email/__tests__/rich-text-editor-quote-caret.test.tsx new file mode 100644 index 000000000..37de365d6 --- /dev/null +++ b/components/email/__tests__/rich-text-editor-quote-caret.test.tsx @@ -0,0 +1,72 @@ +import { act, render, waitFor } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; +import React from 'react'; +import type { Editor } from '@tiptap/core'; + +import { RichTextEditor } from '../rich-text-editor'; +import { serializeEditorContent } from '../quoted-html'; + +// A reply's quoted inline images load while the caret already waits in the +// body, and swapping them in re-sets the editor's content. That threw the caret +// onto the quote as a node, where the first key typed replaced the whole quote +// and the reply went out without the original or its images, or below the quote. + +const PLACEHOLDER = 'data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7'; +const IMAGE = 'data:image/png;base64,iVBORw0KGgo='; +const body = (src: string, below: string) => `

On Tuesday, C wrote:

Original text

logo
${below}`; + +async function openReply(below: string) { + let editor: Editor | null = null; + const element = (content: string) => ( + {}} onEditorReady={(ed) => { editor = ed as unknown as Editor; }} /> + ); + const view = render(element(body(PLACEHOLDER, below))); + await waitFor(() => expect(editor).not.toBeNull()); + const ed = editor as unknown as Editor; + // Where focus('start') puts it; focus itself waits for a frame. + act(() => { ed.commands.setTextSelection(1); }); + return { editor: ed, setContent: (content: string) => view.rerender(element(content)) }; +} + +const type = (editor: Editor, text: string) => act(() => { editor.view.dispatch(editor.state.tr.insertText(text)); }); + +describe.each([ + ['with a line below the quote', '

'], + ['with the quote at the very end', ''], +])('typing in a reply once its inline images have loaded, %s', (_name, below) => { + it('goes above the quote and keeps it and its image', async () => { + const { editor, setContent } = await openReply(below); + + setContent(body(IMAGE, below)); + type(editor, 'My answer'); + + const sent = serializeEditorContent(editor); + expect(editor.state.doc.child(0).textContent).toBe('My answer'); + expect(sent).toContain('Original text'); + expect(sent).toContain(IMAGE); + }); + + it('carries on where the user was already typing', async () => { + const { editor, setContent } = await openReply(below); + type(editor, 'Hel'); + + setContent(body(IMAGE, `${below}`).replace('

', '

Hel

')); + type(editor, 'lo'); + + expect(editor.state.doc.child(0).textContent).toBe('Hello'); + expect(serializeEditorContent(editor)).toContain('Original text'); + }); +}); + +describe('a body replaced by a shorter one', () => { + it('leaves the caret inside it', async () => { + const { editor, setContent } = await openReply('

A long line below the quote

'); + act(() => { editor.commands.setTextSelection(editor.state.doc.content.size - 2); }); + + setContent('

Short

'); + type(editor, '!'); + + expect(editor.state.doc.textContent).toContain('Short'); + expect(editor.state.doc.textContent).toContain('!'); + }); +}); diff --git a/components/email/quoted-html.ts b/components/email/quoted-html.ts index d1aa476df..1cef8c354 100644 --- a/components/email/quoted-html.ts +++ b/components/email/quoted-html.ts @@ -23,6 +23,22 @@ const QUOTED_REMOTE_ALLOWED_ATTR = "data-quoted-remote"; // Atom blocks typing must never replace just because they were left selected. const ATOM_BLOCKS = new Set(["quotedHtml", "signatureBlock"]); +/** + * Replacing the editor's content throws the caret to where the old selection + * maps to: the quote as a node, where the next key typed replaces the whole + * quote, or the line below it. There is no focus event for the guard below to + * catch: a reply's inline images finish loading while the caret already waits + * in the body. It goes back to where it was, or to the start if that is gone. + */ +export function restoreCaretAfterContentSync(editor: Editor, caret: number): void { + const { doc } = editor.state; + const near = Selection.near(doc.resolve(Math.min(caret, doc.content.size))); + const target = near instanceof TextSelection ? near : Selection.atStart(doc); + if (target instanceof TextSelection && !target.eq(editor.state.selection)) { + editor.view.dispatch(editor.state.tr.setSelection(target)); + } +} + // Reusable style for the quote bar when quoting email text (like in a reply). const QUOTE_BAR_STYLE = "border-left:2px solid #c5c5c5;padding-left:12px;margin-top:8px;"; diff --git a/components/email/rich-text-editor.tsx b/components/email/rich-text-editor.tsx index 1eb7decca..3448897cd 100644 --- a/components/email/rich-text-editor.tsx +++ b/components/email/rich-text-editor.tsx @@ -24,7 +24,7 @@ import { TableRow } from "@tiptap/extension-table-row"; import { TableHeader } from "@tiptap/extension-table-header"; import { TableCell } from "@tiptap/extension-table-cell"; import { ReplyQuoteCollapse } from "@/components/email/reply-quote-collapse"; -import { QuotedHtml, serializeEditorContent } from "@/components/email/quoted-html"; +import { QuotedHtml, restoreCaretAfterContentSync, serializeEditorContent } from "@/components/email/quoted-html"; import { keepLoadedParagraphSpacing, paragraphStyleWithoutSpacing, trailingParagraphBreakRule } from "@/components/email/paragraph-breaks"; import { SignatureBlock } from "@/components/email/signature-block"; import { styledBlockAttributes } from "@/components/email/styled-block-attributes"; @@ -614,7 +614,9 @@ export function RichTextEditor({ if (!editor) return; const loaded = keepLoadedParagraphSpacing(content); if (loaded !== serializeEditorContent(editor)) { + const caret = editor.state.selection.from; editor.commands.setContent(loaded, { emitUpdate: false }); + restoreCaretAfterContentSync(editor, caret); } }, [content, editor]); diff --git a/components/files/__tests__/wopi-base-path.test.tsx b/components/files/__tests__/wopi-base-path.test.tsx index c1facd996..0e2296298 100644 --- a/components/files/__tests__/wopi-base-path.test.tsx +++ b/components/files/__tests__/wopi-base-path.test.tsx @@ -2,6 +2,7 @@ import { render, renderHook, waitFor, cleanup } from '@testing-library/react'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { useWopiStatus } from '@/hooks/use-wopi-status'; import { WopiEditor } from '@/components/files/wopi-editor'; +import { useAuthStore } from '@/stores/auth-store'; // Behind a reverse proxy at a sub-path (e.g. /webmail) a bare // fetch('/api/...') reaches the mail server instead of the webmail and comes @@ -26,9 +27,9 @@ afterEach(() => { describe('WOPI requests under a sub-path mount', () => { it('probes the editor status under the mount prefix', async () => { + useAuthStore.setState({ isAuthenticated: true, client: {} as never }); renderHook(() => useWopiStatus(true)); - await waitFor(() => expect(fetchMock).toHaveBeenCalled()); - expect(fetchMock.mock.calls[0][0]).toBe('/webmail/api/wopi/status'); + await waitFor(() => expect(fetchMock).toHaveBeenCalledWith('/webmail/api/wopi/status', undefined)); }); it('launches the editor under the mount prefix', async () => { diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index 0a37a0835..21626921c 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -3214,6 +3214,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { attachments: [payload.attachment], headers: labelSourceHeaders(email.headers), originalFrom: email.from, + // The login holding the message, whose blob this attachment is. + accountId: email.sourceClientAccountId ?? viewingAccountId ?? undefined, }, }); setComposerMode('forward'); diff --git a/components/pro/__tests__/pro-email-reply-account.test.tsx b/components/pro/__tests__/pro-email-reply-account.test.tsx new file mode 100644 index 000000000..d0d514717 --- /dev/null +++ b/components/pro/__tests__/pro-email-reply-account.test.tsx @@ -0,0 +1,137 @@ +import { render, waitFor } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import React from 'react'; + +/** + * A reply, forward or forward-as-attachment from a Pro message view names the + * login holding the message. The composer defaults its From to that account and + * copies the original's parts from it; without the login it takes the message + * for the active account's, so the reply goes out from the wrong account, or + * fails on a blobId that account doesn't have (#1511). + */ + +const viewerProps = vi.hoisted(() => ({ current: null as Record | null })); + +vi.mock('@/components/email/email-viewer', () => ({ + EmailViewer: (props: Record) => { + viewerProps.current = props; + return null; + }, +})); +vi.mock('@/components/error', () => ({ + ErrorBoundary: ({ children }: { children: React.ReactNode }) => React.createElement(React.Fragment, null, children), + EmailViewerErrorFallback: () => null, +})); +vi.mock('next-intl', () => ({ useTranslations: () => (key: string) => key })); +vi.mock('@/stores/toast-store', () => ({ toast: { error: () => {}, success: () => {} } })); +vi.mock('@/hooks/use-pro-multi-account-identities', async (importOriginal) => ({ + ...await importOriginal(), + useProMultiAccountIdentities: () => ({ enabled: true, groups: [], allIdentities: [] }), +})); + +const { openComposeTab } = vi.hoisted(() => ({ openComposeTab: vi.fn() })); + +function selectorStore(state: T) { + const hook = (sel?: (s: T) => unknown) => (typeof sel === 'function' ? sel(state) : state); + hook.getState = () => state; + return hook; +} + +const auth = vi.hoisted(() => ({ + state: { client: null as unknown, activeAccountId: 'login-a', getClientForAccount: () => undefined }, +})); +vi.mock('@/stores/auth-store', () => ({ useAuthStore: selectorStore(auth.state) })); + +const MESSAGE = vi.hoisted(() => ({ + id: 'eaaaaab', + blobId: 'blob-of-message', + threadId: 't1', + mailboxIds: { inbox: true }, + keywords: { $seen: true }, + from: [{ email: 'sender@example.com', name: 'Sender' }], + to: [{ email: 'b@example.org' }], + subject: 'Hello', + receivedAt: '2026-01-01T00:00:00Z', + bodyValues: { '1': { value: '' } }, + textBody: [], + htmlBody: [{ partId: '1', type: 'text/html' }], + attachments: [{ blobId: 'blob-of-logo', name: 'logo.png', type: 'image/png', size: 10, cid: 'logo', disposition: 'inline' }], +})); + +vi.mock('@/stores/email-store', () => ({ + resolveEmailBlobContext: (_email: unknown, client: unknown) => ({ client, clientAccountId: auth.state.activeAccountId }), + useEmailStore: selectorStore({ + fetchEmailContent: async () => MESSAGE, + deleteEmail: async () => {}, + markAsRead: async () => {}, + toggleStar: async () => {}, + moveToMailbox: async () => {}, + setEmailKeywordsLocal: () => {}, + mailboxes: [], + emails: [], + selectedEmail: null, + }), +})); +vi.mock('@/stores/identity-store', () => ({ useIdentityStore: selectorStore({ identities: [] }) })); +vi.mock('@/stores/settings-store', () => ({ useSettingsStore: selectorStore({ emailDownloadTemplate: '' }) })); +vi.mock('@/stores/pro-tab-store', () => ({ + useProTabStore: selectorStore({ openComposeTab, closeTab: () => {}, updateTabTitle: () => {} }), +})); + +import { ProEmailView } from '../pro-email-tab-body'; + +const ACTIONS = ['onReply', 'onReplyAll', 'onForward', 'onForwardAsAttachment'] as const; + +async function open(props: { clientAccountId?: string; accountId?: string; override?: boolean }) { + const client = { getEmail: vi.fn().mockResolvedValue(MESSAGE), getUsername: () => 'b@example.org', getAccountId: () => 'jmap-b' }; + const { override = true, ...rest } = props; + if (!override) auth.state.client = client; + const view = render(); + await waitFor(() => expect(viewerProps.current?.email).toBeTruthy()); + return { ...view, client }; +} + +function composeFrom(action: typeof ACTIONS[number]) { + openComposeTab.mockClear(); + (viewerProps.current![action] as () => void)(); + expect(openComposeTab).toHaveBeenCalledTimes(1); + return openComposeTab.mock.calls[0][0].replyTo as Record; +} + +describe('compose actions of a Pro message view', () => { + beforeEach(() => { + viewerProps.current = null; + openComposeTab.mockClear(); + auth.state.activeAccountId = 'login-a'; + auth.state.client = null; + }); + + it.each(ACTIONS)('%s of another login names that login', async (action) => { + await open({ clientAccountId: 'login-b' }); + + expect(composeFrom(action).accountId).toBe('login-b'); + }); + + it.each(ACTIONS)('%s of a shared folder of another login names the login, not the owner', async (action) => { + await open({ clientAccountId: 'login-b', accountId: 'jmap-team' }); + + expect(composeFrom(action).accountId).toBe('login-b'); + }); + + it.each(ACTIONS)('%s of the active login names the active login', async (action) => { + await open({ override: false }); + + expect(composeFrom(action).accountId).toBe('login-a'); + }); + + it('keeps naming the login the message was read in after the active account changes', async () => { + // A message of b isn't foreign while b is active, so it carries no stamp. + auth.state.activeAccountId = 'login-b'; + const { rerender, client } = await open({ clientAccountId: 'login-b' }); + + auth.state.activeAccountId = 'login-a'; + rerender(); + + expect(composeFrom('onReply').accountId).toBe('login-b'); + }); +}); diff --git a/components/pro/pro-email-tab-body.tsx b/components/pro/pro-email-tab-body.tsx index 024742440..bc8d0fbfc 100644 --- a/components/pro/pro-email-tab-body.tsx +++ b/components/pro/pro-email-tab-body.tsx @@ -111,7 +111,17 @@ export function stampProSource(email: Email, source: EmailBlobContext, loginId: return { ...email, sourceClientAccountId: login, sourceAccountId: account }; } -function buildReplyContext(email: Email): ProReplyContext { +/** + * The login holding the message, for ProReplyContext.accountId: the composer + * defaults its From to that account and copies the original's parts from it + * (#1104, #1511). email.accountId can't say: it is for display, and a message + * read through client.getEmail() has none. + */ +function replyLogin(email: Email, source: EmailWriteSource): string | undefined { + return email.sourceClientAccountId ?? source.clientAccountId; +} + +function buildReplyContext(email: Email, source: EmailWriteSource): ProReplyContext { return { from: email.from, replyToAddresses: email.replyTo, @@ -122,7 +132,7 @@ function buildReplyContext(email: Email): ProReplyContext { subject: email.subject, ...getQuoteBodies(email), receivedAt: email.receivedAt, - accountId: email.accountId, + accountId: replyLogin(email, source), attachments: email.attachments, messageId: email.messageId, inReplyTo: email.inReplyTo, @@ -285,7 +295,7 @@ export function ProEmailView({ emailId, client: clientOverride, clientAccountId, if (!email) return; openComposeTab({ mode: 'reply', - replyTo: buildReplyContext(email), + replyTo: buildReplyContext(email, writeSource), sourceEmailId: email.id, sourceEmailSource: writeSource, initialDraftText: draftText, @@ -305,7 +315,7 @@ export function ProEmailView({ emailId, client: clientOverride, clientAccountId, if (!email) return; openComposeTab({ mode: 'replyAll', - replyTo: buildReplyContext(email), + replyTo: buildReplyContext(email, writeSource), sourceEmailId: email.id, sourceEmailSource: writeSource, title: buildReplySubject(email.subject || t('email_composer.new_message'), t('email_composer.prefix.reply')), @@ -316,7 +326,7 @@ export function ProEmailView({ emailId, client: clientOverride, clientAccountId, if (!email) return; openComposeTab({ mode: 'forward', - replyTo: buildReplyContext(email), + replyTo: buildReplyContext(email, writeSource), sourceEmailId: email.id, sourceEmailSource: writeSource, title: buildForwardSubject(email.subject || t('email_composer.new_message'), t('email_composer.prefix.forward')), @@ -355,6 +365,7 @@ export function ProEmailView({ emailId, client: clientOverride, clientAccountId, attachments: [payload.attachment], headers: labelSourceHeaders(email.headers), originalFrom: email.from, + accountId: replyLogin(email, writeSource), }, sourceEmailId: email.id, sourceEmailSource: writeSource, diff --git a/hooks/__tests__/use-wopi-status.test.ts b/hooks/__tests__/use-wopi-status.test.ts new file mode 100644 index 000000000..6e80b974f --- /dev/null +++ b/hooks/__tests__/use-wopi-status.test.ts @@ -0,0 +1,140 @@ +import { renderHook, waitFor, cleanup } from '@testing-library/react'; +import { describe, it, expect, vi, beforeEach, afterEach, type Mock } from 'vitest'; + +// /api/wopi/status needs the sign-in context cookie. The mail and files views +// can mount before it exists, so a 401 there says nothing about the editor and +// must not leave office editing off until the next page load (#1518). + +const ENABLED = { enabled: true, editExtensions: ['docx', 'odt'], viewExtensions: ['pdf'], newExtensions: ['docx'] }; + +const answer = (status: number, body: unknown = {}) => ({ ok: status >= 200 && status < 300, status, json: async () => body }); + +let fetchMock: Mock<(url: string, init?: RequestInit) => Promise>; +let useWopiStatus: typeof import('@/hooks/use-wopi-status').useWopiStatus; +let useAuthStore: typeof import('@/stores/auth-store').useAuthStore; + +const signIn = (client: object) => useAuthStore.setState({ isAuthenticated: true, client: client as never }); + +beforeEach(async () => { + fetchMock = vi.fn(); + // Only the status route counts: the auth store asks for other things on import. + vi.stubGlobal('fetch', (url: string, init?: RequestInit) => ( + url.includes('/api/wopi/status') ? fetchMock(url, init) : Promise.resolve(answer(404)) + )); + // The hook shares its answer through module state, like a page load does. + vi.resetModules(); + ({ useWopiStatus } = await import('@/hooks/use-wopi-status')); + ({ useAuthStore } = await import('@/stores/auth-store')); + useAuthStore.setState({ isAuthenticated: false, client: null }); +}); + +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); +}); + +describe('useWopiStatus', () => { + it('asks nothing before sign-in, and asks once sign-in is done', async () => { + fetchMock.mockResolvedValue(answer(200, ENABLED)); + const { result } = renderHook(() => useWopiStatus(true)); + + await Promise.resolve(); + expect(fetchMock).not.toHaveBeenCalled(); + + signIn({}); + await waitFor(() => expect(result.current?.enabled).toBe(true)); + expect(result.current?.editExtensions).toContain('docx'); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('asks nothing while the persisted flag says signed in but no client is connected yet', async () => { + fetchMock.mockResolvedValue(answer(200, ENABLED)); + useAuthStore.setState({ isAuthenticated: true, client: null }); + renderHook(() => useWopiStatus(true)); + + await Promise.resolve(); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('turns editing on when a 401 is followed by the real client taking over', async () => { + // A local copy is on screen before the restore writes the context cookie. + fetchMock.mockResolvedValueOnce(answer(401)).mockResolvedValue(answer(200, ENABLED)); + signIn({ kind: 'local copy' }); + const { result } = renderHook(() => useWopiStatus(true)); + await waitFor(() => expect(result.current).not.toBeNull()); + expect(result.current?.enabled).toBe(false); + + signIn({ kind: 'real' }); + await waitFor(() => expect(result.current?.enabled).toBe(true)); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it('does not share a 401 still in flight with the client that took over', async () => { + let finishFirst: (value: unknown) => void = () => {}; + fetchMock + .mockReturnValueOnce(new Promise((resolve) => { finishFirst = resolve; })) + .mockResolvedValue(answer(200, ENABLED)); + signIn({ kind: 'local copy' }); + const { result } = renderHook(() => useWopiStatus(true)); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); + + signIn({ kind: 'real' }); + await waitFor(() => expect(result.current?.enabled).toBe(true)); + finishFirst(answer(401)); + + // The late refusal does not take the answer back, for this view or the next. + await new Promise((resolve) => setTimeout(resolve)); + expect(result.current?.enabled).toBe(true); + const next = renderHook(() => useWopiStatus(true)); + await waitFor(() => expect(next.result.current?.enabled).toBe(true)); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it.each([401, 500, 502])('asks again on the next mount after a %i', async (status) => { + fetchMock.mockResolvedValueOnce(answer(status)).mockResolvedValue(answer(200, ENABLED)); + signIn({}); + const first = renderHook(() => useWopiStatus(true)); + await waitFor(() => expect(first.result.current).not.toBeNull()); + expect(first.result.current?.enabled).toBe(false); + + const second = renderHook(() => useWopiStatus(true)); + await waitFor(() => expect(second.result.current?.enabled).toBe(true)); + }); + + it('keeps a real answer for the page, across views and account switches', async () => { + fetchMock.mockResolvedValue(answer(200, ENABLED)); + signIn({ account: 'a' }); + const first = renderHook(() => useWopiStatus(true)); + await waitFor(() => expect(first.result.current?.enabled).toBe(true)); + + const second = renderHook(() => useWopiStatus(true)); + await waitFor(() => expect(second.result.current?.enabled).toBe(true)); + signIn({ account: 'b' }); + await waitFor(() => expect(useAuthStore.getState().client).toEqual({ account: 'b' })); + await new Promise((resolve) => setTimeout(resolve)); + + expect(first.result.current?.enabled).toBe(true); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('keeps a real "no editor" answer too, so mail views do not keep asking', async () => { + fetchMock.mockResolvedValue(answer(200, { enabled: false })); + signIn({}); + const first = renderHook(() => useWopiStatus(true)); + await waitFor(() => expect(first.result.current?.enabled).toBe(false)); + + const second = renderHook(() => useWopiStatus(true)); + await waitFor(() => expect(second.result.current?.enabled).toBe(false)); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('asks nothing while the caller has it switched off', async () => { + fetchMock.mockResolvedValue(answer(200, ENABLED)); + signIn({}); + const { result } = renderHook(() => useWopiStatus(false)); + + await Promise.resolve(); + expect(fetchMock).not.toHaveBeenCalled(); + expect(result.current).toBeNull(); + }); +}); diff --git a/hooks/use-wopi-status.ts b/hooks/use-wopi-status.ts index 4d6dd014a..c3b575edf 100644 --- a/hooks/use-wopi-status.ts +++ b/hooks/use-wopi-status.ts @@ -3,11 +3,12 @@ import { useEffect, useState } from "react"; import { IS_LITE } from "@/lib/lite"; import { apiFetch } from "@/lib/browser-navigation"; +import { useAuthStore } from "@/stores/auth-store"; /** * Whether a WOPI document editor is configured for this deployment and which - * file extensions it handles (#425). Fetched once per page load and shared - * across consumers via a module-level promise. + * file extensions it handles (#425). The answer is the same for every account, + * so the first real one is kept for the page load and shared across consumers. */ export interface WopiStatus { enabled: boolean; @@ -19,41 +20,61 @@ export interface WopiStatus { const DISABLED: WopiStatus = { enabled: false, editExtensions: [], viewExtensions: [], newExtensions: [] }; -let statusPromise: Promise | null = null; +let known: WopiStatus | null = null; +let pending: Promise | null = null; +let pendingClient: unknown = null; -function fetchWopiStatus(): Promise { +function fetchWopiStatus(client: unknown): Promise { // The WOPI bridge is a server feature; the static build never has an editor. if (IS_LITE) return Promise.resolve(DISABLED); - if (!statusPromise) { - statusPromise = apiFetch("/api/wopi/status") - .then((res) => (res.ok ? res.json() : DISABLED)) - .then((data) => ({ + if (known) return Promise.resolve(known); + // A request sent for an earlier client may be refused for lack of the sign-in + // context this one brought, so only the same client shares it. + if (pending && pendingClient === client) return pending; + + const request: Promise = apiFetch("/api/wopi/status") + .then((res) => { + if (!res.ok) throw new Error(`HTTP ${res.status}`); + return res.json(); + }) + .then((data) => { + known = { enabled: !!data?.enabled, editExtensions: Array.isArray(data?.editExtensions) ? data.editExtensions : [], viewExtensions: Array.isArray(data?.viewExtensions) ? data.viewExtensions : [], newExtensions: Array.isArray(data?.newExtensions) ? data.newExtensions : [], - })) - .catch(() => { - statusPromise = null; // allow a retry on the next mount - return DISABLED; - }); - } - return statusPromise; + }; + return known; + }) + // A refusal or an outage says nothing about the editor (#1518): show no + // editor for now and ask again on the next mount or client. + .catch(() => DISABLED) + .finally(() => { + if (pending === request) pending = null; + }); + pending = request; + pendingClient = client; + return request; } export function useWopiStatus(enabled: boolean): WopiStatus | null { const [status, setStatus] = useState(null); + // The route needs the sign-in context cookie, which is written before the + // store holds a connected client. `isAuthenticated` alone comes back from + // storage on load, and a local copy's client gives way to the real one once + // the restore wrote the cookie, so this follows the client itself (#1518). + const client = useAuthStore((state) => (state.isAuthenticated ? state.client : null)); useEffect(() => { - if (!enabled) return; + if (!enabled || !client) return; let cancelled = false; - fetchWopiStatus().then((s) => { + fetchWopiStatus(client).then((s) => { if (!cancelled) setStatus(s); }); return () => { cancelled = true; }; - }, [enabled]); + }, [enabled, client]); return status; } diff --git a/lib/desktop/__tests__/trust-flag.test.ts b/lib/desktop/__tests__/trust-flag.test.ts index c40bd5846..0edd41532 100644 --- a/lib/desktop/__tests__/trust-flag.test.ts +++ b/lib/desktop/__tests__/trust-flag.test.ts @@ -3,7 +3,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { readFileSync } from 'node:fs'; import path from 'node:path'; import { build } from 'esbuild'; -import { desktopBuildFlag } from '../../../scripts/desktop-build-flag.mjs'; +import { DESKTOP_TRUST_MARKER, desktopBuildFlag } from '../../../scripts/desktop-build-flag.mjs'; const ROOT = path.resolve(__dirname, '../../..'); const TRUST_KEY = Symbol.for('bulwark.desktop.trustedJmapOrigins'); @@ -112,6 +112,28 @@ describe('the flag is fixed at build time', () => { } }); + /** Compiles the trust check alone, kept as an export the way Turbopack keeps it. */ + async function trustCheckOfBuild(flag: string): Promise { + const result = await build({ + entryPoints: [path.join(ROOT, 'lib/desktop/trusted-servers.ts')], + bundle: true, + write: false, + minify: true, + format: 'esm', + platform: 'node', + define: { 'process.env.NEXT_PUBLIC_BULWARK_DESKTOP': JSON.stringify(flag) }, + }); + return result.outputFiles[0].text; + } + + it('a server build holds no trust path, even when a bundler keeps the unused export', async () => { + expect(await trustCheckOfBuild('')).not.toContain(DESKTOP_TRUST_MARKER); + }); + + it('a desktop build still holds it', async () => { + expect(await trustCheckOfBuild('1')).toContain(DESKTOP_TRUST_MARKER); + }); + it('no image build sets it', () => { for (const file of ['Dockerfile', 'Dockerfile.lite']) { expect(readFileSync(path.join(ROOT, file), 'utf8')).not.toContain('NEXT_PUBLIC_BULWARK_DESKTOP'); diff --git a/lib/desktop/trusted-servers.ts b/lib/desktop/trusted-servers.ts index e2cf034b5..8fa60c651 100644 --- a/lib/desktop/trusted-servers.ts +++ b/lib/desktop/trusted-servers.ts @@ -7,8 +7,9 @@ * or the environment can write it, so script in a page can't aim the login * routes at the LAN. * - * Callers guard this with IS_DESKTOP_BUILD, so a server build compiles the - * whole thing away. scripts/standalone-guard.mjs checks that it did. + * Callers check the flag too, but Turbopack keeps an imported export after the + * call is gone. So the check also starts here, spelled out: every bundler then + * drops the body from a server build. scripts/standalone-guard.mjs checks it did. */ type TrustedOrigins = ReadonlySet; @@ -25,6 +26,7 @@ function originOf(url: string): string | null { } export function isDesktopTrustedServer(url: string | null | undefined): boolean { + if (process.env.NEXT_PUBLIC_BULWARK_DESKTOP !== '1') return false; if (!url) return false; const origin = originOf(url); if (!origin) return false;