From ac41efa6cedf14a7966116c08dfcc4a08fb3bb78 Mon Sep 17 00:00:00 2001 From: Marten Klitzke Date: Wed, 7 Oct 2026 17:43:21 +0200 Subject: [PATCH 1/4] refactor: token helpers the bio and the org flow share --- lib/bio.ts | 29 +++++++++-------------------- lib/tokens.ts | 21 +++++++++++++++++++++ 2 files changed, 30 insertions(+), 20 deletions(-) create mode 100644 lib/tokens.ts diff --git a/lib/bio.ts b/lib/bio.ts index f24edd5..f60edb7 100644 --- a/lib/bio.ts +++ b/lib/bio.ts @@ -1,6 +1,8 @@ +import { appendToken, removeAppendedToken } from "./tokens"; + export const BIO_MAX_LENGTH = 1024; -export const VERIFICATION_TOKEN_PATTERN = /^FLEETYARDS-[A-Z0-9]{10}$/; +export { VERIFICATION_TOKEN_PATTERN } from "./tokens"; const ENTITIES: Record = { amp: "&", @@ -72,28 +74,15 @@ export function parseBio(html: string): string | null { export class BioTooLongError extends Error {} -export function withToken( - bio: string, - token: string, - maxLength = BIO_MAX_LENGTH -) { - if (bio.includes(token)) return { bio, added: false }; - - const next = bio ? `${bio}\n\n${token}` : token; - if (next.length > maxLength) throw new BioTooLongError(); +export function withToken(bio: string, token: string) { + const { text, changed } = appendToken(bio, token); + if (text.length > BIO_MAX_LENGTH) throw new BioTooLongError(); - return { bio: next, added: true }; + return { bio: text, added: changed }; } -// Only the token as withToken appended it: one the user put elsewhere in their -// bio themselves is theirs to remove. export function withoutToken(bio: string, token: string) { - if (bio === token) return { bio: "", removed: true }; - - const suffix = `\n\n${token}`; - if (bio.endsWith(suffix)) { - return { bio: bio.slice(0, -suffix.length), removed: true }; - } + const { text, changed } = removeAppendedToken(bio, token); - return { bio, removed: false }; + return { bio: text, removed: changed }; } diff --git a/lib/tokens.ts b/lib/tokens.ts new file mode 100644 index 0000000..0ee1f4a --- /dev/null +++ b/lib/tokens.ts @@ -0,0 +1,21 @@ +export const VERIFICATION_TOKEN_PATTERN = /^FLEETYARDS-[A-Z0-9]{10}$/; + +// A verification token goes after the text, a blank line apart, and only that +// is ever taken out again: the same token placed elsewhere by hand is the +// owner's to remove. +export function appendToken(text: string, token: string) { + if (text.includes(token)) return { text, changed: false }; + + return { text: text ? `${text}\n\n${token}` : token, changed: true }; +} + +export function removeAppendedToken(text: string, token: string) { + if (text === token) return { text: "", changed: true }; + + const suffix = `\n\n${token}`; + if (text.endsWith(suffix)) { + return { text: text.slice(0, -suffix.length), changed: true }; + } + + return { text, changed: false }; +} From 333c6759c9f986e5f13a1568a908cabd33769f24 Mon Sep 17 00:00:00 2001 From: Marten Klitzke Date: Wed, 7 Oct 2026 17:43:21 +0200 Subject: [PATCH 2/4] fix(org): compare an org's draft and live page by section and markup Text alone missed an unpublished image, link or formatting change, and a Textile div block cut the comparison short. A section one page leaves out counts as empty. --- __tests__/org.test.ts | 67 ++++++++++++++++++++++++++++++--- lib/org.ts | 86 ++++++++++++++++++++++++++++++++----------- 2 files changed, 126 insertions(+), 27 deletions(-) diff --git a/__tests__/org.test.ts b/__tests__/org.test.ts index 8e23e0e..1cfd269 100644 --- a/__tests__/org.test.ts +++ b/__tests__/org.test.ts @@ -1,8 +1,9 @@ import { describe, it, expect } from "vitest"; import { - contentBlocks, + contentSections, hasPendingChanges, isAccessDenied, + pageShowsToken, parseDraftField, } from "@/lib/org"; @@ -47,19 +48,32 @@ describe("parseDraftField", () => { }); }); -describe("contentBlocks", () => { - it("reads every text block as plain text without tokens", () => { +describe("contentSections", () => { + it("reads every section by name, without tokens", () => { expect( - contentBlocks( + contentSections( orgPage( '

Our board.

\n\n

FLEETYARDS-ABCDEFGHIJ

' ) ) - ).toEqual(["Welcome aboard.", "Our board.", "Our manifesto."]); + ).toEqual( + new Map([ + ["introduction", "

Welcome aboard.

"], + ["history", "

Our board.

"], + ["manifesto", "

Our manifesto.

"], + ]) + ); + }); + + it("reads a section past the divs Textile nests in it", () => { + expect( + contentSections(orgPage("

Inner

After

")) + ?.get("history") + ).toBe('

Inner

After

'); }); it("refuses a page without text blocks", () => { - expect(contentBlocks("Access denied")).toBeNull(); + expect(contentSections("Access denied")).toBeNull(); }); }); @@ -82,7 +96,48 @@ describe("hasPendingChanges", () => { ).toBe(true); }); + it("sees a change that only touches markup", () => { + expect( + hasPendingChanges( + orgPage('

Our board.

'), + orgPage('

Our board.

') + ) + ).toBe(true); + }); + + it("sees a change after a nested div", () => { + expect( + hasPendingChanges( + orgPage("

Same

Edited

"), + orgPage("

Same

Original

") + ) + ).toBe(true); + }); + + it("reads a section one page leaves out as empty", () => { + const withEmptyCharter = `${orgPage("

Our board.

")}
`; + + expect( + hasPendingChanges(withEmptyCharter, orgPage("

Our board.

")) + ).toBe(false); + }); + it("refuses to compare a page it cannot read", () => { expect(hasPendingChanges("", orgPage("

x

"))).toBeNull(); }); }); + +describe("pageShowsToken", () => { + it("finds a token Textile split with a span", () => { + expect( + pageShowsToken( + '

FLEETYARDS-ABCDEFGHIJ

', + "FLEETYARDS-ABCDEFGHIJ" + ) + ).toBe(true); + }); + + it("finds nothing on a page without it", () => { + expect(pageShowsToken("

Our board.

", "FLEETYARDS-ABCDEFGHIJ")).toBe(false); + }); +}); diff --git a/lib/org.ts b/lib/org.ts index e51e42b..b543216 100644 --- a/lib/org.ts +++ b/lib/org.ts @@ -7,7 +7,6 @@ export const ORG_VERIFICATION_FIELD = "history"; // As the site and RSI print an org's SID: up to ten capitals and digits. export const SID_PATTERN = /^[A-Z0-9]{1,10}$/; -const TOKEN_IN_TEXT = /FLEETYARDS-[A-Z0-9]{10}/g; // Without content rights RSI still answers 200, with a page titled so. export function isAccessDenied(html: string) { @@ -28,36 +27,81 @@ export function parseDraftField(html: string, field: string): string | null { return decodeEntities(textarea[1]!.replace(/^\r?\n/, "")); } -// The org's text blocks as a reader sees them, for comparing two renderings -// of the page: tags and every FleetYards token dropped, whitespace collapsed. -// Null when the page has none, which is markup this cannot read. -export function contentBlocks(html: string): string[] | null { - const blocks = [ - ...html.matchAll(/
([\s\S]*?)<\/div>/g), - ].map((match) => - match[1]! - .replace(/<[^>]*>/g, "") - .replace(/&[^;\s]+;/g, (entity) => decodeEntities(entity) ?? entity) - .replace(TOKEN_IN_TEXT, "") - .replace(/\s+/g, " ") - .trim() - ); +// Every way a rendered page shows a FleetYards token: Textile wraps the +// capitals in a span. +const TOKEN_IN_MARKUP = + /(?:)?FLEETYARDS(?:<\/span>)?-[A-Z0-9]{10}/g; + +const BLOCK_START = '
'; + +// A block's inner markup up to its own closing tag: Textile's `div.` blocks +// nest divs inside it. +function blockAt(html: string, start: number) { + const tags = //g; + tags.lastIndex = start; + let depth = 1; + + for (let tag = tags.exec(html); tag; tag = tags.exec(html)) { + depth += tag[0] === "
" ? -1 : 1; + if (depth === 0) return html.slice(start, tag.index); + } + + return null; +} + +// Which org text a block belongs to: the content tab it sits in, or the +// introduction above the tabs. +function sectionOf(html: string, index: number) { + const tab = [...html.slice(0, index).matchAll(/id="tab-([a-z]+)"/g)].pop(); - return blocks.length > 0 ? blocks : null; + return tab ? tab[1]! : "introduction"; +} + +// The org's text sections as rendered, by name: markup kept, so an image, a +// link or formatting counts as a change too; every FleetYards token, the +// paragraphs left empty by taking one out, and whitespace dropped. Null when +// the page has none, which is markup this cannot read. +export function contentSections(html: string): Map | null { + const sections = new Map(); + + for ( + let index = html.indexOf(BLOCK_START); + index !== -1; + index = html.indexOf(BLOCK_START, index + 1) + ) { + const inner = blockAt(html, index + BLOCK_START.length); + if (inner === null) return null; + + sections.set( + sectionOf(html, index), + inner + .replace(TOKEN_IN_MARKUP, "") + .replace(/

\s*<\/p>/g, "") + .replace(/\s+/g, " ") + .trim() + ); + } + + return sections.size > 0 ? sections : null; } // Whether the org's draft holds changes besides FleetYards tokens: RSI // publishes the whole draft at once, so writing then would publish them too. +// A section one page leaves out counts as empty. export function hasPendingChanges( previewHtml: string, publicHtml: string ): boolean | null { - const draft = contentBlocks(previewHtml); - const live = contentBlocks(publicHtml); + const draft = contentSections(previewHtml); + const live = contentSections(publicHtml); if (!draft || !live) return null; - return ( - draft.length !== live.length || - draft.some((block, index) => block !== live[index]) + return [...new Set([...draft.keys(), ...live.keys()])].some( + (section) => (draft.get(section) ?? "") !== (live.get(section) ?? "") ); } + +// Whether a rendered org page shows the token, Textile's markup aside. +export function pageShowsToken(html: string, token: string) { + return html.replace(/<[^>]*>/g, "").includes(token); +} From 2485c4cbe370dcfa80ca528119b89cbeda334a32 Mon Sep 17 00:00:00 2001 From: Marten Klitzke Date: Wed, 7 Oct 2026 17:43:21 +0200 Subject: [PATCH 3/4] fix(org): finish an interrupted verification instead of skipping it Saving and publishing are decided apart, from the draft and the live page, so a run that saved but failed to publish is completed by the next. The draft is compared again right before publishing and put back if another edit arrived. Requests for one org, and for the bio, run one at a time, so a remove cannot overtake a slow write. A token the extension did not place is reported, not skipped. --- __tests__/message-handler.test.ts | 93 +++++++++++++++++++ lib/message-handler.ts | 146 ++++++++++++++++++++---------- 2 files changed, 191 insertions(+), 48 deletions(-) diff --git a/__tests__/message-handler.test.ts b/__tests__/message-handler.test.ts index 14ccc0b..5f0831a 100644 --- a/__tests__/message-handler.test.ts +++ b/__tests__/message-handler.test.ts @@ -621,6 +621,99 @@ describe("onMessage org verify actions", () => { expect(fetch).not.toHaveBeenCalled(); }); + it("publishes a token a failed run left in the draft", async () => { + const fetch = mockRsi({ + content: contentPage(`Our board.\n\n${verificationToken}`), + }); + + const result = await send({ action: "org-verify-write", sid: "MARU", token: verificationToken }); + + expect(result.payload).toEqual({ sid: "MARU", changed: true }); + expect(posted(fetch, "/api/orgs/saveDraft")).toEqual([]); + expect(posted(fetch, "/api/orgs/publishDraft")).toEqual([{ symbol: "MARU" }]); + }); + + it("does nothing for a token already in the draft and live", async () => { + const fetch = mockRsi({ + content: contentPage(`Our board.\n\n${verificationToken}`), + live: orgPage(`

Our board.

${verificationToken}

`), + preview: orgPage(`

Our board.

${verificationToken}

`), + }); + + const result = await send({ action: "org-verify-write", sid: "MARU", token: verificationToken }); + + expect(result.payload).toEqual({ sid: "MARU", changed: false }); + expect(posted(fetch, "/api/orgs/publishDraft")).toEqual([]); + }); + + it("puts the draft back when another edit arrives before publishing", async () => { + let previewReads = 0; + const fetch = vi.spyOn(globalThis, "fetch").mockImplementation(async (url) => { + const target = String(url); + if (target.endsWith("/admin/content")) return new Response(contentPage("Our board.")); + if (target.endsWith("/admin/preview")) { + previewReads += 1; + return new Response( + previewReads === 1 + ? orgPage("

Our board.

") + : orgPage("

Our board.

", "Someone else's draft.") + ); + } + if (target.endsWith("/en/orgs/MARU")) return new Response(orgPage("

Our board.

")); + return json({ success: 1 }); + }); + + const result = await send({ action: "org-verify-write", sid: "MARU", token: verificationToken }); + + expect(result.code).toBe(409); + expect(posted(fetch, "/api/orgs/saveDraft")).toEqual([ + { symbol: "MARU", history: `Our board.\n\n${verificationToken}` }, + { symbol: "MARU", history: "Our board." }, + ]); + expect(posted(fetch, "/api/orgs/publishDraft")).toEqual([]); + }); + + it("answers 409 for a token it did not place", async () => { + mockRsi({ + content: contentPage(`${verificationToken}\n\nOur board.`), + live: orgPage(`

${verificationToken}

Our board.

`), + }); + + const result = await send({ action: "org-verify-remove", sid: "MARU", token: verificationToken }); + + expect(result.code).toBe(409); + }); + + it("lets a remove wait for a write to the same org", async () => { + const order: string[] = []; + let releaseWrite: () => void = () => {}; + let draft = "Our board."; + vi.spyOn(globalThis, "fetch").mockImplementation(async (url, init) => { + const target = String(url); + if (target.endsWith("/admin/content")) return new Response(contentPage(draft)); + if (target.endsWith("/admin/preview")) return new Response(orgPage("

Our board.

")); + if (target.endsWith("/en/orgs/MARU")) return new Response(orgPage("

Our board.

")); + if (target.endsWith("/api/orgs/saveDraft")) { + const { history } = JSON.parse(String(init?.body)); + order.push(history.includes(verificationToken) ? "save-token" : "save-clean"); + if (history.includes(verificationToken)) { + await new Promise((resolve) => (releaseWrite = resolve)); + } + draft = history; + } + return json({ success: 1 }); + }); + + const write = send({ action: "org-verify-write", sid: "MARU", token: verificationToken }); + await vi.waitFor(() => expect(order).toEqual(["save-token"])); + const remove = send({ action: "org-verify-remove", sid: "MARU", token: verificationToken }); + releaseWrite(); + await write; + await remove; + + expect(order).toEqual(["save-token", "save-clean"]); + }); + it("removes the token it appended and publishes again", async () => { const fetch = mockRsi({ content: contentPage(`Our board.\n\n${verificationToken}`), diff --git a/lib/message-handler.ts b/lib/message-handler.ts index 4ff3236..dbc2ff6 100644 --- a/lib/message-handler.ts +++ b/lib/message-handler.ts @@ -17,8 +17,10 @@ import { SID_PATTERN, hasPendingChanges, isAccessDenied, + pageShowsToken, parseDraftField, } from "./org"; +import { appendToken, removeAppendedToken } from "./tokens"; import { BioTooLongError, VERIFICATION_TOKEN_PATTERN, @@ -161,10 +163,44 @@ async function storePricing(token: string) { }; } +// Requests for the same org (or the same bio) wait for each other: a remove +// sent while a slow write is still out would read the draft before the write +// lands, find nothing, and leave the token the write then publishes. +const queues = new Map>(); + +function oneAtATime(key: string, task: () => Promise): Promise { + const run = (queues.get(key) ?? Promise.resolve()).then(task, task); + const settled = run.catch(() => undefined); + queues.set(key, settled); + void settled.then(() => { + if (queues.get(key) === settled) queues.delete(key); + }); + + return run; +} + +async function orgPages(rsiToken: string, sid: string) { + const [content, preview, live] = await Promise.all([ + fetchOrgAdminPage(rsiToken, sid, "content"), + fetchOrgAdminPage(rsiToken, sid, "preview"), + fetchOrgPage(sid), + ]); + + return { + content, + previewHtml: preview.ok ? await preview.text() : null, + liveHtml: live.ok ? await live.text() : null, + }; +} + // Writes the token into the org's history and publishes it, for the org the // signed-in account can edit. Only while nothing else waits in the org's // draft: publishing takes the whole draft live, another officer's half-done // edit included. +// +// Saving and publishing are decided apart, from the draft and from the live +// page: a run that saved but failed to publish is finished by the next one +// instead of found "already done". async function verifyOrg( action: OrgVerifyAction, sid: unknown, @@ -181,62 +217,77 @@ async function verifyOrg( return { code: 400, action, error: "Invalid SID" }; } - const content = await fetchOrgAdminPage(rsiToken, sid, "content"); - if (!content.ok) { - return { code: content.status, action, error: "Org unreadable", payload: { sid } }; - } + const failed = (code: number, error: string) => ({ + code, + action, + error, + payload: { sid }, + }); + + const { content, previewHtml, liveHtml } = await orgPages(rsiToken, sid); + if (!content.ok) return failed(content.status, "Org unreadable"); const contentHtml = await content.text(); - if (isAccessDenied(contentHtml)) { - return { code: 403, action, error: "No rights for this org", payload: { sid } }; - } + if (isAccessDenied(contentHtml)) return failed(403, "No rights for this org"); const draft = parseDraftField(contentHtml, ORG_VERIFICATION_FIELD); - if (draft === null) { - return { code: 422, action, error: "Org unreadable", payload: { sid } }; - } - - const [preview, live] = await Promise.all([ - fetchOrgAdminPage(rsiToken, sid, "preview"), - fetchOrgPage(sid), - ]); const pending = - preview.ok && live.ok - ? hasPendingChanges(await preview.text(), await live.text()) - : null; - if (pending === null) { - return { code: 422, action, error: "Org unreadable", payload: { sid } }; + previewHtml && liveHtml ? hasPendingChanges(previewHtml, liveHtml) : null; + if (draft === null || pending === null || !liveHtml) { + return failed(422, "Org unreadable"); } - if (pending) { - return { code: 409, action, error: "Unpublished changes", payload: { sid } }; + if (pending) return failed(409, "Unpublished changes"); + + const writing = action === "org-verify-write"; + const { text: next, changed: needsSave } = writing + ? appendToken(draft, verificationToken) + : removeAppendedToken(draft, verificationToken); + + // Somewhere other than where the extension puts it: not the extension's to + // take out, and still public. + if (!writing && !needsSave && next.includes(verificationToken)) { + return failed(409, "Token placed by hand"); } - let next: string; - let changed: boolean; - if (action === "org-verify-write") { - ({ bio: next, added: changed } = withToken(draft, verificationToken, Infinity)); - } else { - ({ bio: next, removed: changed } = withoutToken(draft, verificationToken)); + const needsPublish = writing !== pageShowsToken(liveHtml, verificationToken); + + const succeeded = async (response: Response) => + response.ok && (await reportsSuccess(response)); + + if (needsSave) { + const saved = await saveOrgDraft(rsiToken, sid, ORG_VERIFICATION_FIELD, next); + if (!(await succeeded(saved))) { + return failed(saved.ok ? 502 : saved.status, "Org update failed"); + } } - if (changed) { - for (const request of [ - () => saveOrgDraft(rsiToken, sid, ORG_VERIFICATION_FIELD, next), - () => publishOrgDraft(rsiToken, sid), - ]) { - const response = await request(); - if (!response.ok || !(await reportsSuccess(response))) { - return { - code: response.ok ? 502 : response.status, - action, - error: "Org update failed", - payload: { sid }, - }; + if (needsPublish) { + // Another officer can save a draft while this one runs. Asked again right + // before publishing, so their edit is not what goes live with the token. + const again = await orgPages(rsiToken, sid); + const stillAlone = + again.previewHtml && again.liveHtml + ? hasPendingChanges(again.previewHtml, again.liveHtml) === false + : false; + + if (!stillAlone) { + if (needsSave) { + await saveOrgDraft(rsiToken, sid, ORG_VERIFICATION_FIELD, draft); } + return failed(409, "Unpublished changes"); + } + + const published = await publishOrgDraft(rsiToken, sid); + if (!(await succeeded(published))) { + return failed(published.ok ? 502 : published.status, "Org update failed"); } } - return { code: 200, action, payload: { sid, changed } }; + return { + code: 200, + action, + payload: { sid, changed: needsSave || needsPublish }, + }; } export async function onMessage( @@ -269,7 +320,9 @@ export async function onMessage( JSON.stringify({ code: 401, action: message.action, error: "No RSI session" }) ); } else { - const result = await verifyBio(message.action, message.token, token).catch( + const result = await oneAtATime("bio", () => + verifyBio(message.action, message.token, token) + ).catch( (error) => { console.error("FY Sync: Bio update failed", error); @@ -291,11 +344,8 @@ export async function onMessage( JSON.stringify({ code: 401, action: message.action, error: "No RSI session" }) ); } else { - const result = await verifyOrg( - message.action, - message.sid, - message.token, - token + const result = await oneAtATime(`org:${message.sid}`, () => + verifyOrg(message.action, message.sid, message.token, token) ).catch((error) => { console.error("FY Sync: Org update failed", error); From df8663747ae471322a690ce1514ba3d922ed2c14 Mon Sep 17 00:00:00 2001 From: Marten Klitzke Date: Wed, 7 Oct 2026 17:57:40 +0200 Subject: [PATCH 4/4] fix(org): never overwrite an officer's history edit, and judge the token by its own section Putting the draft back after a late conflict re-reads the history first and restores it only while it still holds exactly what this request saved; a refused restore is reported, with the token marked as possibly left in the draft. Whether the token is live is read from the history section alone: a token in another section is one the extension never wrote, so a removal refuses it instead of publishing and reporting success. The re-check before publishing no longer fetches the admin form. --- __tests__/message-handler.test.ts | 83 +++++++++++++++++++++++++++++-- __tests__/org.test.ts | 27 ++++++---- lib/message-handler.ts | 59 +++++++++++++++++----- lib/org.ts | 57 +++++++++++++++------ 4 files changed, 187 insertions(+), 39 deletions(-) diff --git a/__tests__/message-handler.test.ts b/__tests__/message-handler.test.ts index 5f0831a..d8a2d93 100644 --- a/__tests__/message-handler.test.ts +++ b/__tests__/message-handler.test.ts @@ -510,7 +510,7 @@ describe("onMessage org verify actions", () => { const verificationToken = "FLEETYARDS-ABCDEFGHIJ"; const orgPage = (history: string, manifesto = "Ours.") => - `

Intro.

${history}

${manifesto}

`; + `

Intro.

${history}

${manifesto}

`; const contentPage = (history: string) => `Description - Admin`; @@ -648,9 +648,14 @@ describe("onMessage org verify actions", () => { it("puts the draft back when another edit arrives before publishing", async () => { let previewReads = 0; - const fetch = vi.spyOn(globalThis, "fetch").mockImplementation(async (url) => { + let draft = "Our board."; + const fetch = vi.spyOn(globalThis, "fetch").mockImplementation(async (url, init) => { const target = String(url); - if (target.endsWith("/admin/content")) return new Response(contentPage("Our board.")); + if (target.endsWith("/admin/content")) return new Response(contentPage(draft)); + if (target.endsWith("/api/orgs/saveDraft")) { + draft = JSON.parse(String(init?.body)).history; + return json({ success: 1 }); + } if (target.endsWith("/admin/preview")) { previewReads += 1; return new Response( @@ -684,6 +689,78 @@ describe("onMessage org verify actions", () => { expect(result.code).toBe(409); }); + it("leaves an officer's history edit made during the run alone", async () => { + let contentReads = 0; + let previewReads = 0; + const fetch = vi.spyOn(globalThis, "fetch").mockImplementation(async (url) => { + const target = String(url); + if (target.endsWith("/admin/content")) { + contentReads += 1; + return new Response( + contentPage( + contentReads === 1 ? "Our board." : `Our new board.\n\n${verificationToken}` + ) + ); + } + if (target.endsWith("/admin/preview")) { + previewReads += 1; + return new Response( + orgPage(previewReads === 1 ? "

Our board.

" : "

Our new board.

") + ); + } + if (target.endsWith("/en/orgs/MARU")) return new Response(orgPage("

Our board.

")); + return json({ success: 1 }); + }); + + const result = await send({ action: "org-verify-write", sid: "MARU", token: verificationToken }); + + expect(result).toMatchObject({ code: 409, payload: { changed: true } }); + expect(posted(fetch, "/api/orgs/saveDraft")).toHaveLength(1); + expect(posted(fetch, "/api/orgs/publishDraft")).toEqual([]); + }); + + it("says so when putting the draft back was refused", async () => { + let previewReads = 0; + let saves = 0; + vi.spyOn(globalThis, "fetch").mockImplementation(async (url, init) => { + const target = String(url); + if (target.endsWith("/admin/content")) { + return new Response( + contentPage(saves === 0 ? "Our board." : `Our board.\n\n${verificationToken}`) + ); + } + if (target.endsWith("/admin/preview")) { + previewReads += 1; + return new Response( + orgPage("

Our board.

", previewReads === 1 ? "Ours." : "Half done.") + ); + } + if (target.endsWith("/en/orgs/MARU")) return new Response(orgPage("

Our board.

")); + if (target.endsWith("/api/orgs/saveDraft")) { + saves += 1; + return json(saves === 1 ? { success: 1 } : { success: 0, msg: "ErrCsrf" }); + } + return json({ success: 1 }); + }); + + const result = await send({ action: "org-verify-write", sid: "MARU", token: verificationToken }); + + expect(result).toMatchObject({ code: 502, payload: { changed: true } }); + }); + + it("refuses to remove a token another section shows", async () => { + const fetch = mockRsi({ + content: contentPage(`Our board.\n\n${verificationToken}`), + live: orgPage(`

Our board.

${verificationToken}

`, verificationToken), + preview: orgPage(`

Our board.

${verificationToken}

`, verificationToken), + }); + + const result = await send({ action: "org-verify-remove", sid: "MARU", token: verificationToken }); + + expect(result.code).toBe(409); + expect(posted(fetch, "/api/orgs/saveDraft")).toEqual([]); + }); + it("lets a remove wait for a write to the same org", async () => { const order: string[] = []; let releaseWrite: () => void = () => {}; diff --git a/__tests__/org.test.ts b/__tests__/org.test.ts index 1cfd269..b3a09a4 100644 --- a/__tests__/org.test.ts +++ b/__tests__/org.test.ts @@ -3,8 +3,8 @@ import { contentSections, hasPendingChanges, isAccessDenied, - pageShowsToken, parseDraftField, + tokenOnPage, } from "@/lib/org"; // Trimmed from a live org page. @@ -127,17 +127,26 @@ describe("hasPendingChanges", () => { }); }); -describe("pageShowsToken", () => { - it("finds a token Textile split with a span", () => { +describe("tokenOnPage", () => { + const token = "FLEETYARDS-ABCDEFGHIJ"; + + it("finds a token Textile split with a span in the history", () => { expect( - pageShowsToken( - '

FLEETYARDS-ABCDEFGHIJ

', - "FLEETYARDS-ABCDEFGHIJ" + tokenOnPage( + orgPage('

Our board.

FLEETYARDS-ABCDEFGHIJ

'), + token, + "history" ) - ).toBe(true); + ).toEqual({ inField: true, elsewhere: false }); + }); + + it("tells a token in another section apart", () => { + expect( + tokenOnPage(orgPage("

Our board.

", token), token, "history") + ).toEqual({ inField: false, elsewhere: true }); }); - it("finds nothing on a page without it", () => { - expect(pageShowsToken("

Our board.

", "FLEETYARDS-ABCDEFGHIJ")).toBe(false); + it("refuses a page it cannot read", () => { + expect(tokenOnPage("", token, "history")).toBeNull(); }); }); diff --git a/lib/message-handler.ts b/lib/message-handler.ts index dbc2ff6..4d0f903 100644 --- a/lib/message-handler.ts +++ b/lib/message-handler.ts @@ -17,8 +17,8 @@ import { SID_PATTERN, hasPendingChanges, isAccessDenied, - pageShowsToken, parseDraftField, + tokenOnPage, } from "./org"; import { appendToken, removeAppendedToken } from "./tokens"; import { @@ -179,20 +179,36 @@ function oneAtATime(key: string, task: () => Promise): Promise { return run; } -async function orgPages(rsiToken: string, sid: string) { - const [content, preview, live] = await Promise.all([ - fetchOrgAdminPage(rsiToken, sid, "content"), +// The draft as it will be published, and the org page as it is now. +async function comparedPages(rsiToken: string, sid: string) { + const [preview, live] = await Promise.all([ fetchOrgAdminPage(rsiToken, sid, "preview"), fetchOrgPage(sid), ]); return { - content, previewHtml: preview.ok ? await preview.text() : null, liveHtml: live.ok ? await live.text() : null, }; } +async function orgPages(rsiToken: string, sid: string) { + const [content, compared] = await Promise.all([ + fetchOrgAdminPage(rsiToken, sid, "content"), + comparedPages(rsiToken, sid), + ]); + + return { content, ...compared }; +} + +async function draftField(rsiToken: string, sid: string) { + const content = await fetchOrgAdminPage(rsiToken, sid, "content"); + + return content.ok + ? parseDraftField(await content.text(), ORG_VERIFICATION_FIELD) + : null; +} + // Writes the token into the org's history and publishes it, for the org the // signed-in account can edit. Only while nothing else waits in the org's // draft: publishing takes the whole draft live, another officer's half-done @@ -217,11 +233,13 @@ async function verifyOrg( return { code: 400, action, error: "Invalid SID" }; } - const failed = (code: number, error: string) => ({ + // `changed` on a failure: the token may sit in the draft now, so the page + // that asked should still have it taken out. + const failed = (code: number, error: string, changed = false) => ({ code, action, error, - payload: { sid }, + payload: { sid, changed }, }); const { content, previewHtml, liveHtml } = await orgPages(rsiToken, sid); @@ -238,6 +256,9 @@ async function verifyOrg( } if (pending) return failed(409, "Unpublished changes"); + const onPage = tokenOnPage(liveHtml, verificationToken, ORG_VERIFICATION_FIELD); + if (!onPage) return failed(422, "Org unreadable"); + const writing = action === "org-verify-write"; const { text: next, changed: needsSave } = writing ? appendToken(draft, verificationToken) @@ -245,11 +266,14 @@ async function verifyOrg( // Somewhere other than where the extension puts it: not the extension's to // take out, and still public. - if (!writing && !needsSave && next.includes(verificationToken)) { + if ( + !writing && + ((!needsSave && next.includes(verificationToken)) || onPage.elsewhere) + ) { return failed(409, "Token placed by hand"); } - const needsPublish = writing !== pageShowsToken(liveHtml, verificationToken); + const needsPublish = writing !== onPage.inField; const succeeded = async (response: Response) => response.ok && (await reportsSuccess(response)); @@ -264,16 +288,27 @@ async function verifyOrg( if (needsPublish) { // Another officer can save a draft while this one runs. Asked again right // before publishing, so their edit is not what goes live with the token. - const again = await orgPages(rsiToken, sid); + const again = await comparedPages(rsiToken, sid); const stillAlone = again.previewHtml && again.liveHtml ? hasPendingChanges(again.previewHtml, again.liveHtml) === false : false; if (!stillAlone) { - if (needsSave) { - await saveOrgDraft(rsiToken, sid, ORG_VERIFICATION_FIELD, draft); + // A remove that cannot publish leaves the draft without the token, which + // is where it should end up anyway; the live page still shows it. + if (!writing || !needsSave) return failed(409, "Unpublished changes"); + + // Put back only what this request wrote: an officer who saved the + // history since keeps their edit, and the token with it. + const current = await draftField(rsiToken, sid); + if (current !== next) return failed(409, "Unpublished changes", true); + + const restored = await saveOrgDraft(rsiToken, sid, ORG_VERIFICATION_FIELD, draft); + if (!(await succeeded(restored))) { + return failed(restored.ok ? 502 : restored.status, "Draft left with token", true); } + return failed(409, "Unpublished changes"); } diff --git a/lib/org.ts b/lib/org.ts index b543216..1ec3701 100644 --- a/lib/org.ts +++ b/lib/org.ts @@ -57,11 +57,9 @@ function sectionOf(html: string, index: number) { return tab ? tab[1]! : "introduction"; } -// The org's text sections as rendered, by name: markup kept, so an image, a -// link or formatting counts as a change too; every FleetYards token, the -// paragraphs left empty by taking one out, and whitespace dropped. Null when -// the page has none, which is markup this cannot read. -export function contentSections(html: string): Map | null { +// Each org text section's inner markup as rendered, by name. Null when the +// page has none, or a block never closes: markup this cannot read. +function rawSections(html: string): Map | null { const sections = new Map(); for ( @@ -72,17 +70,30 @@ export function contentSections(html: string): Map | null { const inner = blockAt(html, index + BLOCK_START.length); if (inner === null) return null; - sections.set( - sectionOf(html, index), + sections.set(sectionOf(html, index), inner); + } + + return sections.size > 0 ? sections : null; +} + +// The org's text sections for comparing two renderings of the page: markup +// kept, so an image, a link or formatting counts as a change too; every +// FleetYards token, the paragraphs left empty by taking one out, and +// whitespace dropped. +export function contentSections(html: string): Map | null { + const sections = rawSections(html); + if (!sections) return null; + + return new Map( + [...sections].map(([name, inner]) => [ + name, inner .replace(TOKEN_IN_MARKUP, "") .replace(/

\s*<\/p>/g, "") .replace(/\s+/g, " ") - .trim() - ); - } - - return sections.size > 0 ? sections : null; + .trim(), + ]) + ); } // Whether the org's draft holds changes besides FleetYards tokens: RSI @@ -101,7 +112,23 @@ export function hasPendingChanges( ); } -// Whether a rendered org page shows the token, Textile's markup aside. -export function pageShowsToken(html: string, token: string) { - return html.replace(/<[^>]*>/g, "").includes(token); +// Where a rendered org page shows the token, Textile's markup aside: in the +// section the extension writes, or anywhere else, which it never wrote and +// cannot take out. Null for a page it cannot read. +export function tokenOnPage( + html: string, + token: string, + field: string +): { inField: boolean; elsewhere: boolean } | null { + const sections = rawSections(html); + if (!sections) return null; + + const shows = (markup: string) => markup.replace(/<[^>]*>/g, "").includes(token); + + return { + inField: shows(sections.get(field) ?? ""), + elsewhere: [...sections].some( + ([name, markup]) => name !== field && shows(markup) + ), + }; }