diff --git a/.changeset/preview-url-resolver.md b/.changeset/preview-url-resolver.md new file mode 100644 index 0000000000..c573592580 --- /dev/null +++ b/.changeset/preview-url-resolver.md @@ -0,0 +1,32 @@ +--- +"nextly": patch +"create-nextly-app": patch +"@nextlyhq/admin": patch +"@nextlyhq/admin-css": patch +"@nextlyhq/blocks-engine": patch +"@nextlyhq/blocks-react": patch +"@nextlyhq/ui": patch +"@nextlyhq/adapter-drizzle": patch +"@nextlyhq/adapter-postgres": patch +"@nextlyhq/adapter-mysql": patch +"@nextlyhq/adapter-sqlite": patch +"@nextlyhq/storage-s3": patch +"@nextlyhq/storage-uploadthing": patch +"@nextlyhq/storage-vercel-blob": patch +"@nextlyhq/plugin-form-builder": patch +"@nextlyhq/plugin-page-builder": patch +"@nextlyhq/plugin-seo": patch +"@nextlyhq/plugin-sdk": patch +"@nextlyhq/eslint-config": patch +"@nextlyhq/prettier-config": patch +"@nextlyhq/telemetry": patch +"@nextlyhq/tsconfig": patch +"@nextlyhq/builder": patch +"@nextlyhq/module-specifiers": patch +--- + +Resolve an entry preview URL in one place, on the server. + +A collection declares its preview two ways and they are disjoint: code-first writes a function of the entry, a UI-created collection writes a template string. Both are now answered by one resolver, so the admin asks where an entry previews instead of deciding for itself. + +Resolving on the server is what makes the preview button reachable for editors and authors. The site URL sits behind a settings permission neither role holds, so a browser-side answer was unavailable to exactly the people who share previews. diff --git a/packages/admin/src/hooks/__tests__/useEntryPreview.test.ts b/packages/admin/src/hooks/__tests__/useEntryPreview.test.ts new file mode 100644 index 0000000000..e942d33983 --- /dev/null +++ b/packages/admin/src/hooks/__tests__/useEntryPreview.test.ts @@ -0,0 +1,393 @@ +import { renderHook, act } from "@testing-library/react"; +import { describe, expect, it, vi, beforeEach, afterEach } from "vitest"; + +import { useEntryPreview } from "../useEntryPreview"; + +const resolve = vi.hoisted(() => vi.fn()); + +vi.mock("@admin/services/previewUrlApi", () => ({ + previewUrlApi: { resolve }, +})); + +vi.mock("@admin/lib/preview/preview-data", () => ({ + storePreviewData: vi.fn(() => "preview-key"), + generatePreviewUrlWithData: vi.fn( + (url: string, key: string) => `${url}?__preview=${key}` + ), +})); + +/** A stand-in for the tab `window.open` hands back. */ +function fakeTab() { + return { location: { href: "" }, close: vi.fn(), opener: {} as unknown }; +} + +const collection = { + name: "posts", + admin: { preview: { hasPreview: true } }, +}; + +let openSpy: ReturnType; + +beforeEach(() => { + resolve.mockReset(); + openSpy = vi.spyOn(window, "open"); +}); + +afterEach(() => { + openSpy.mockRestore(); +}); + +describe("isPreviewAvailable", () => { + it("follows the stored boolean without asking the server", () => { + const { result } = renderHook(() => + useEntryPreview({ collection, entry: { id: "1" } }) + ); + + expect(result.current.isPreviewAvailable).toBe(true); + // Availability is a render-time question; a round trip here would make the + // button appear late on every entry that has one. + expect(resolve).not.toHaveBeenCalled(); + }); + + it("is false when the collection stores no preview", () => { + const { result } = renderHook(() => + useEntryPreview({ collection: { name: "posts" }, entry: { id: "1" } }) + ); + + expect(result.current.isPreviewAvailable).toBe(false); + }); + + it("is true from a stored template alone, with no boolean present", () => { + // A UI-created collection stores its template directly and never goes + // through the code-first sync that writes the boolean, so requiring the + // boolean would hide a preview the stored data plainly declares. Every row + // written before the boolean existed is this case. + const { result } = renderHook(() => + useEntryPreview({ + collection: { + name: "posts", + admin: { preview: { urlTemplate: "/preview/{slug}" } }, + }, + entry: { id: "1" }, + }) + ); + + expect(result.current.isPreviewAvailable).toBe(true); + }); + + it("is false for an empty stored template", () => { + // What a cleared field yields; it declares nothing. + const { result } = renderHook(() => + useEntryPreview({ + collection: { + name: "posts", + admin: { preview: { urlTemplate: "" } }, + }, + entry: { id: "1" }, + }) + ); + + expect(result.current.isPreviewAvailable).toBe(false); + }); + + it("is false when the collection stores hasPreview: false", () => { + const { result } = renderHook(() => + useEntryPreview({ + collection: { + name: "posts", + admin: { preview: { hasPreview: false } }, + }, + entry: { id: "1" }, + }) + ); + + expect(result.current.isPreviewAvailable).toBe(false); + }); +}); + +describe("openPreview", () => { + it("claims the tab BEFORE awaiting, so the popup blocker does not eat it", async () => { + const tab = fakeTab(); + const order: string[] = []; + openSpy.mockImplementation(() => { + order.push("open"); + return tab as unknown as Window; + }); + resolve.mockImplementation(() => { + order.push("resolve"); + return Promise.resolve({ status: "resolved", url: "https://s.dev/p/1" }); + }); + + const { result } = renderHook(() => + useEntryPreview({ collection, entry: { id: "1" } }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + // The whole point: a window opened after an await has lost the user-gesture + // context and Safari and Firefox block it. Asserting only that open() was + // called would pass on the broken ordering too. + expect(order).toEqual(["open", "resolve"]); + expect(tab.location.href).toBe("https://s.dev/p/1"); + }); + + it("opens the tab without noopener, then severs the reference by hand", async () => { + const tab = fakeTab(); + openSpy.mockReturnValue(tab as unknown as Window); + resolve.mockResolvedValue({ status: "resolved", url: "https://s.dev/p/1" }); + + const { result } = renderHook(() => + useEntryPreview({ collection, entry: { id: "1" } }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + // Passing "noopener" would make window.open return null and leave nothing to + // navigate, so the reference has to be cut manually instead. + const features = openSpy.mock.calls[0]?.[2]; + expect(features ?? "").not.toContain("noopener"); + expect(tab.opener).toBeNull(); + }); + + it("closes the claimed tab and reports why when no host is configured", async () => { + const tab = fakeTab(); + openSpy.mockReturnValue(tab as unknown as Window); + resolve.mockResolvedValue({ status: "noSiteUrl", path: "/p/1" }); + const onUnavailable = vi.fn(); + + const { result } = renderHook(() => + useEntryPreview({ collection, entry: { id: "1" }, onUnavailable }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + // Leaving a blank tab open would look like a preview that failed to load. + expect(tab.close).toHaveBeenCalled(); + expect(tab.location.href).toBe(""); + // Distinct from "unavailable": this one is fixed by an admin setting a site + // URL, not by the editor filling in a field. + expect(onUnavailable).toHaveBeenCalledWith("noSiteUrl"); + }); + + it("reports an entry that is not previewable yet", async () => { + const tab = fakeTab(); + openSpy.mockReturnValue(tab as unknown as Window); + resolve.mockResolvedValue({ status: "unavailable" }); + const onUnavailable = vi.fn(); + + const { result } = renderHook(() => + useEntryPreview({ collection, entry: { id: "1" }, onUnavailable }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + expect(onUnavailable).toHaveBeenCalledWith("unavailable"); + }); + + it("stays silent when the collection has no preview at all", async () => { + const tab = fakeTab(); + openSpy.mockReturnValue(tab as unknown as Window); + resolve.mockResolvedValue({ status: "notConfigured" }); + const onUnavailable = vi.fn(); + + const { result } = renderHook(() => + useEntryPreview({ collection, entry: { id: "1" }, onUnavailable }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + // The button should not have been offered, so an error here would describe a + // state the editor cannot act on. + expect(onUnavailable).not.toHaveBeenCalled(); + expect(tab.close).toHaveBeenCalled(); + }); + + it("closes the tab and reports when the request itself fails", async () => { + const tab = fakeTab(); + openSpy.mockReturnValue(tab as unknown as Window); + resolve.mockRejectedValue(new Error("network")); + const onUnavailable = vi.fn(); + + const { result } = renderHook(() => + useEntryPreview({ collection, entry: { id: "1" }, onUnavailable }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + expect(tab.close).toHaveBeenCalled(); + expect(onUnavailable).toHaveBeenCalledWith("failed"); + }); + + it("sends unsaved form values, not the saved row", async () => { + const tab = fakeTab(); + openSpy.mockReturnValue(tab as unknown as Window); + // Same-origin on purpose: this test is about WHICH VALUES reach the + // resolver, and a cross-origin URL would additionally drop the session-key + // handoff, mixing a second behaviour into the assertion. + resolve.mockResolvedValue({ + status: "resolved", + url: `${window.location.origin}/p/new`, + }); + + const { result } = renderHook(() => + useEntryPreview({ + collection, + entry: { id: "1", slug: "saved" }, + getFormValues: () => ({ slug: "edited" }), + }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + // Resolving against the saved row would open the previous URL and show the + // wrong page, which is worse than not offering the button. + expect(resolve).toHaveBeenCalledWith({ + collection: "posts", + entry: { id: "1", slug: "edited" }, + }); + expect(tab.location.href).toBe( + `${window.location.origin}/p/new?__preview=preview-key` + ); + }); + + it("stores unsaved data BEFORE opening the tab, not after", async () => { + const tab = fakeTab(); + const order: string[] = []; + const { storePreviewData } = await import( + "@admin/lib/preview/preview-data" + ); + vi.mocked(storePreviewData).mockImplementation(() => { + order.push("store"); + return "preview-key"; + }); + openSpy.mockImplementation(() => { + order.push("open"); + return tab as unknown as Window; + }); + resolve.mockResolvedValue({ status: "resolved", url: "https://s.dev/p/1" }); + + const { result } = renderHook(() => + useEntryPreview({ + collection, + entry: { id: "1" }, + getFormValues: () => ({ slug: "edited" }), + }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + // A new browsing context gets a COPY of session storage taken at creation. + // Written afterwards, the key stays in this window and the preview silently + // shows the last saved values instead of the edits on screen. + expect(order).toEqual(["store", "open"]); + }); + + it("reports a blocked popup instead of navigating the admin away", async () => { + // window.open returns null when the browser blocks it. + openSpy.mockReturnValue(null); + resolve.mockResolvedValue({ status: "resolved", url: "https://s.dev/p/1" }); + const onUnavailable = vi.fn(); + const before = window.location.href; + + const { result } = renderHook(() => + useEntryPreview({ collection, entry: { id: "1" }, onUnavailable }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + // Falling back to this window would take the editor off the form and + // discard every unsaved change — the opposite of what preview is for. + expect(window.location.href).toBe(before); + expect(onUnavailable).toHaveBeenCalledWith("popupBlocked"); + // And it must not even ask: the click cannot succeed either way. + expect(resolve).not.toHaveBeenCalled(); + }); + + it("drops the unsaved-data key cross-origin and says so, rather than sending a dead one", async () => { + const tab = fakeTab(); + openSpy.mockReturnValue(tab as unknown as Window); + // jsdom serves the admin from localhost; the site is elsewhere, which is + // what a configured site URL now routinely means. + resolve.mockResolvedValue({ + status: "resolved", + url: "https://site.example.com/p/1", + }); + const onUnsavedChangesNotSent = vi.fn(); + + const { result } = renderHook(() => + useEntryPreview({ + collection, + entry: { id: "1" }, + getFormValues: () => ({ slug: "edited" }), + onUnsavedChangesNotSent, + }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + // Session storage is partitioned per origin, so the key would name a + // payload the preview page cannot reach. Appending it would look like it + // worked while quietly rendering stale content. + expect(tab.location.href).toBe("https://site.example.com/p/1"); + expect(tab.location.href).not.toContain("__preview"); + // Reported, because silently previewing saved data is the failure mode this + // whole path exists to prevent. + expect(onUnsavedChangesNotSent).toHaveBeenCalled(); + }); + + it("still sends the key when the site is same-origin", async () => { + // The positive control for the drop above: without it, a hook that never + // appended the key would satisfy that test perfectly. + const tab = fakeTab(); + openSpy.mockReturnValue(tab as unknown as Window); + resolve.mockResolvedValue({ + status: "resolved", + url: `${window.location.origin}/p/1`, + }); + const onUnsavedChangesNotSent = vi.fn(); + + const { result } = renderHook(() => + useEntryPreview({ + collection, + entry: { id: "1" }, + getFormValues: () => ({ slug: "edited" }), + onUnsavedChangesNotSent, + }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + expect(tab.location.href).toContain("__preview=preview-key"); + expect(onUnsavedChangesNotSent).not.toHaveBeenCalled(); + }); + + it("navigates the current window when the collection opts out of a new tab", async () => { + resolve.mockResolvedValue({ status: "resolved", url: "https://s.dev/p/1" }); + + const { result } = renderHook(() => + useEntryPreview({ + collection: { + name: "posts", + admin: { preview: { hasPreview: true, openInNewTab: false } }, + }, + entry: { id: "1" }, + }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + expect(openSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/admin/src/hooks/index.ts b/packages/admin/src/hooks/index.ts index deb3c63912..8c2bac6a24 100644 --- a/packages/admin/src/hooks/index.ts +++ b/packages/admin/src/hooks/index.ts @@ -54,6 +54,7 @@ export { useEntryPreview } from "./useEntryPreview"; export type { PreviewConfig, PreviewCollection, + PreviewUnavailableReason, UseEntryPreviewOptions, UseEntryPreviewResult, } from "./useEntryPreview"; diff --git a/packages/admin/src/hooks/useEntryPreview.ts b/packages/admin/src/hooks/useEntryPreview.ts index 3f8de0e615..9dd71a6be9 100644 --- a/packages/admin/src/hooks/useEntryPreview.ts +++ b/packages/admin/src/hooks/useEntryPreview.ts @@ -1,14 +1,16 @@ "use client"; /** - * useEntryPreview Hook + * Opening an entry's preview. * - * Provides preview URL generation and preview opening functionality - * for entry forms. Supports both saved entries and unsaved changes - * via session storage. + * The URL is resolved by the server rather than here: a code-first collection + * declares its preview as a function that no column can hold, and the site URL + * the answer is based on sits behind a `settings` permission that editors and + * authors do not hold. So this hook decides WHETHER to offer the button, from a + * boolean the registry can store, and asks the server WHERE only when it is + * clicked. * * @module hooks/useEntryPreview - * @since 1.0.0 */ import { useCallback, useMemo } from "react"; @@ -17,81 +19,101 @@ import { storePreviewData, generatePreviewUrlWithData, } from "@admin/lib/preview/preview-data"; +import { previewUrlApi } from "@admin/services/previewUrlApi"; // ============================================================================ // Types // ============================================================================ /** - * Preview configuration from collection admin config. - * Supports both function-based (code-first) and template-based (UI) configs. + * The preview settings the admin reads back from the collection registry. + * + * Deliberately not the authored declaration: `url` is a function and + * `urlTemplate` is the server's resolution input, so neither belongs here. + * What the panel needs is whether to draw a button and how to label it. */ export interface PreviewConfig { /** - * Function to generate preview URL from entry data. - * Used by code-first collections. + * Whether this collection previews at all, decided when the config synced. + * + * Written only by the code-first sync, because that is the path whose + * declaration — a function — cannot itself be stored. */ - url?: (entry: Record) => string | null; - + hasPreview?: boolean; /** - * URL template with {fieldName} placeholders. - * Used by UI-created collections. + * A UI-created collection's stored template. + * + * Read here ONLY to answer whether a preview exists. The URL is never built + * from it in the browser: that is the resolver's job, and interpolating it + * here would be the second implementation this design exists to avoid. */ urlTemplate?: string; - - /** - * Whether to open preview in a new tab. - * @default true - */ + /** Whether to open the preview in a new tab. @default true */ openInNewTab?: boolean; - - /** - * Custom label for the preview button. - * @default "Preview" - */ + /** Custom label for the preview button. @default "Preview" */ label?: string; } -/** - * Collection configuration required for preview functionality. - */ +/** Collection configuration required for preview functionality. */ export interface PreviewCollection { - /** Collection slug/name */ + /** Collection slug. */ name: string; - /** Admin configuration including preview settings */ admin?: { preview?: PreviewConfig; }; } -/** - * Options for the useEntryPreview hook. - */ export interface UseEntryPreviewOptions { - /** Collection with preview configuration */ collection: PreviewCollection; - /** Existing entry data (for edit mode) */ + /** The saved entry, when editing an existing one. */ entry?: Record | null; + /** Current form values, so the preview reflects unsaved edits. */ + getFormValues?: () => Record; + /** Told why a click could not open anything. */ + onUnavailable?: (reason: PreviewUnavailableReason) => void; /** - * Function to get current form values (for unsaved changes). - * Called when opening preview to get the latest form state. + * Told when the preview OPENED but could not carry the editor's unsaved + * edits, so it is showing the last saved version instead. + * + * Deliberately not folded into `onUnavailable`. That reports a click which + * produced nothing; this reports one that produced something less than was + * asked for. Merging them would either suppress a real warning or label a + * working preview as a failure — and the editor's response differs: here + * they can save and click again. */ - getFormValues?: () => Record; + onUnsavedChangesNotSent?: () => void; } /** - * Return type for useEntryPreview hook. + * Why a preview click produced nothing. + * + * Separate from the button's availability because these are only discoverable + * on click: the entry's own values decide two of them, and the third is a + * deployment setting the panel cannot see. */ +export type PreviewUnavailableReason = + /** Declared, but not for this entry yet — no slug, wrong status. */ + | "unavailable" + /** + * No usable site URL is configured, so no host can be named. Covers an absent + * setting and one the browser would execute rather than navigate to. + */ + | "noSiteUrl" + /** + * The browser refused the new tab. Distinct from every other reason because + * nothing is wrong with the entry or the configuration — the editor can allow + * popups and click again. + */ + | "popupBlocked" + /** The request itself failed. */ + | "failed"; + export interface UseEntryPreviewResult { - /** Whether preview is available for this collection */ + /** Whether to offer the button at all. Known without a round trip. */ isPreviewAvailable: boolean; - /** Current preview URL based on saved entry (null if not available) */ - previewUrl: string | null; - /** Open the preview (uses unsaved data if available) */ - openPreview: () => void; - /** Get preview URL for specific data */ - getPreviewUrl: (data: Record) => string | null; - /** Label for the preview button */ + /** Resolve and open. Asynchronous: the URL comes from the server. */ + openPreview: () => Promise; + /** Label for the preview button. */ label: string; } @@ -100,53 +122,19 @@ export interface UseEntryPreviewResult { // ============================================================================ /** - * Interpolate URL template with entry data. - * - * Replaces {fieldName} placeholders with actual field values. - * Returns null if any required field is missing. + * Whether `url` is served from the origin this admin is running on. * - * @param template - URL template with {fieldName} placeholders - * @param data - Entry data to interpolate - * @returns Interpolated URL or null if interpolation fails + * Decides only whether the session-storage handoff can work: that storage is + * partitioned per origin, so a payload written here is unreachable from a + * preview page served anywhere else. Compared by parsed origin rather than by + * string prefix, which `https://site.example.com.evil.test` would satisfy. */ -function interpolateUrlTemplate( - template: string, - data: Record -): string | null { +function isSameOrigin(url: string): boolean { try { - let result = template; - const placeholders = template.match(/\{(\w+)\}/g) || []; - - for (const placeholder of placeholders) { - const fieldName = placeholder.slice(1, -1); // Remove { and } - const value = data[fieldName]; - - // If a required field is missing/null/undefined, can't generate URL - if (value === null || value === undefined || value === "") { - return null; - } - - // eslint-disable-next-line @typescript-eslint/no-base-to-string - result = result.replace(placeholder, String(value)); - } - - return result; + return new URL(url).origin === window.location.origin; } catch { - return null; - } -} - -/** - * Normalize a URL to be absolute if it's relative. - * - * @param url - URL that may be relative or absolute - * @returns Absolute URL - */ -function normalizeUrl(url: string): string { - if (url.startsWith("/")) { - return `${window.location.origin}${url}`; + return false; } - return url; } // ============================================================================ @@ -154,139 +142,143 @@ function normalizeUrl(url: string): string { // ============================================================================ /** - * useEntryPreview - Preview URL generation for entry forms - * - * Generates preview URLs based on collection configuration and entry data. - * Supports: - * - Function-based URL generation (code-first collections) - * - Template-based URL generation (UI-created collections) - * - Previewing unsaved changes via session storage + * useEntryPreview - open the site at the entry being edited. * - * @example Basic usage + * @example * ```tsx * const { isPreviewAvailable, openPreview, label } = useEntryPreview({ * collection, * entry, * getFormValues: () => form.getValues(), + * onUnavailable: reason => toast.error(PREVIEW_MESSAGES[reason]), * }); - * - * {isPreviewAvailable && ( - * - * )} - * ``` - * - * @example Check preview URL availability - * ```tsx - * const { previewUrl, isPreviewAvailable } = useEntryPreview({ - * collection, - * entry, - * }); - * - * // previewUrl is null if: - * // - Collection has no preview config - * // - Entry data is insufficient (e.g., missing slug) * ``` */ export function useEntryPreview({ collection, entry, getFormValues, + onUnavailable, + onUnsavedChangesNotSent, }: UseEntryPreviewOptions): UseEntryPreviewResult { const previewConfig = collection.admin?.preview; - // Check if preview is configured - const isPreviewAvailable = useMemo(() => { - if (!previewConfig) return false; - return !!(previewConfig.url || previewConfig.urlTemplate); - }, [previewConfig]); - - /** - * Generate preview URL for given data. - * Tries function-based first, then template-based. - */ - const getPreviewUrl = useCallback( - (data: Record): string | null => { - if (!previewConfig) return null; - - try { - // Try function-based URL first (code-first collections) - if (previewConfig.url) { - const url = previewConfig.url(data); - return url ? normalizeUrl(url) : null; - } - - // Try template-based URL (UI collections) - if (previewConfig.urlTemplate) { - const url = interpolateUrlTemplate(previewConfig.urlTemplate, data); - return url ? normalizeUrl(url) : null; - } - - return null; - } catch (error) { - console.error("Failed to generate preview URL:", error); - return null; - } - }, + // Answered from stored data, so the button does not flicker in after a fetch + // and does not appear for a collection that has no preview at all. + // + // EITHER signal counts, because the two authoring paths store different + // things: code-first syncs the boolean, since its function cannot be stored, + // while a UI-created collection has its template stored directly and may + // carry no boolean at all. Requiring the boolean alone would hide a preview + // that a stored template plainly declares — and every row written before the + // boolean existed is exactly that case. + const isPreviewAvailable = useMemo( + () => + previewConfig?.hasPreview === true || Boolean(previewConfig?.urlTemplate), [previewConfig] ); - /** - * Preview URL for the current saved entry. - */ - const previewUrl = useMemo(() => { - if (!entry) return null; - return getPreviewUrl(entry); - }, [entry, getPreviewUrl]); - - /** - * Open preview in a new tab/window. - * Uses unsaved form data if available via getFormValues. - */ - const openPreview = useCallback(() => { - // Get the data to preview (unsaved form values or saved entry) + const openPreview = useCallback(async () => { const unsavedData = getFormValues?.(); const dataToPreview = unsavedData ? { ...entry, ...unsavedData } : entry; - if (!dataToPreview) { - console.warn("No data available for preview"); + onUnavailable?.("unavailable"); return; } - // Generate base preview URL - let url = getPreviewUrl(dataToPreview); - if (!url) { - console.warn("Could not generate preview URL"); + const openInNewTab = previewConfig?.openInNewTab !== false; + + // BEFORE the tab is opened, because a new browsing context receives a COPY + // of session storage taken when it is created. A write afterwards stays in + // this window, the new tab never sees the key, and the preview silently + // renders the last saved values instead of what is on screen — the exact + // failure the unsaved-data path exists to prevent. + const previewKey = unsavedData + ? storePreviewData( + collection.name, + entry?.id as string | undefined, + dataToPreview + ) + : undefined; + + // Opened NOW, synchronously, while the click is still on the stack. A window + // opened after an `await` has lost the user-gesture context and Safari and + // Firefox block it, so the tab is claimed first and navigated once the URL + // arrives. + // + // `noopener` cannot be passed here: with it, `window.open` returns null and + // there is no handle left to navigate. The reference is severed by hand + // instead, while the tab is still `about:blank` and same-origin — after + // which it cannot reach back through `window.opener`. + const target = openInNewTab ? window.open("", "_blank") : null; + if (target) target.opener = null; + + // A blocked popup is NOT the same as a collection that asked to open in + // place. Falling back to navigating this window would take the editor off + // the form they are editing and discard everything unsaved, so it is + // reported instead — the browser's own blocked-popup affordance is what lets + // them retry. + if (openInNewTab && !target) { + onUnavailable?.("popupBlocked"); return; } - // If there's unsaved data, store it and append preview key - if (unsavedData) { - const entryId = entry?.id as string | undefined; - const previewKey = storePreviewData( - collection.name, - entryId, - dataToPreview - ); - url = generatePreviewUrlWithData(url, previewKey); - } + const abandon = (reason: PreviewUnavailableReason) => { + target?.close(); + onUnavailable?.(reason); + }; + + try { + const resolution = await previewUrlApi.resolve({ + collection: collection.name, + entry: dataToPreview, + }); + + if (resolution.status !== "resolved") { + // `notConfigured` is not reported: the button should not have been + // offered, so telling the editor a preview is unavailable would describe + // a state they cannot act on. The other two are theirs to fix — fill in + // the slug, or ask an admin to set the site URL. + if (resolution.status !== "notConfigured") abandon(resolution.status); + else target?.close(); + return; + } - // Open the preview - const openInNewTab = previewConfig?.openInNewTab !== false; - if (openInNewTab) { - window.open(url, "_blank", "noopener,noreferrer"); - } else { - window.location.href = url; + // Unsaved values travel through session storage rather than the URL, so + // the preview renders what is on screen instead of what was last saved. + // The payload was written above; only the key is appended here. + // + // Session storage is partitioned by ORIGIN, and a resolved preview URL is + // now routinely on a different one — that is what a configured site URL + // means. The key would then name a payload the preview page cannot reach, + // so it is omitted and the caller is told the preview shows saved content. + // Appending it anyway would look like it worked and quietly show stale + // data, which is the failure this whole path exists to prevent. + const sameOrigin = isSameOrigin(resolution.url); + const url = + previewKey === undefined || !sameOrigin + ? resolution.url + : generatePreviewUrlWithData(resolution.url, previewKey); + + if (previewKey !== undefined && !sameOrigin) onUnsavedChangesNotSent?.(); + + if (target) target.location.href = url; + else window.location.href = url; + } catch { + abandon("failed"); } - }, [collection.name, entry, getFormValues, getPreviewUrl, previewConfig]); + }, [ + collection.name, + entry, + getFormValues, + onUnavailable, + onUnsavedChangesNotSent, + previewConfig, + ]); return { isPreviewAvailable, - previewUrl, openPreview, - getPreviewUrl, label: previewConfig?.label || "Preview", }; } diff --git a/packages/admin/src/services/previewUrlApi.ts b/packages/admin/src/services/previewUrlApi.ts new file mode 100644 index 0000000000..9b9e9d0313 --- /dev/null +++ b/packages/admin/src/services/previewUrlApi.ts @@ -0,0 +1,50 @@ +/** + * Where an entry previews, answered by the server. + * + * The admin cannot compute this itself. A code-first collection declares its + * preview as a function of the entry, which exists only in the server's module + * graph and no column can hold — and the site URL the result is based on lives + * behind a `settings` permission that the `editor` and `author` roles do not + * have, which are exactly the roles that preview content. So the panel asks for + * a finished URL rather than the parts to build one. + * + * @module services/previewUrlApi + */ + +import { protectedApi } from "@admin/lib/api/protectedApi"; + +export interface PreviewUrlRequest { + collection: string; + /** + * The entry as it stands on screen, unsaved edits included. The server + * resolves against these values rather than the saved row, because an editor + * previews what they are looking at. + */ + entry: Record; +} + +/** + * The server's answer, kept as four cases rather than a nullable URL. + * + * Three of them mean "no preview to open", and collapsing them would lose the + * only distinction that matters to a caller trying to recover: `noSiteUrl` is + * the one where an origin is guessable and guessing yields the admin's own host, + * which produces a confident link to the wrong place. + */ +export type PreviewUrlResolution = + | { status: "resolved"; url: string } + | { status: "notConfigured" } + | { status: "unavailable" } + | { status: "noSiteUrl"; path: string }; + +export const previewUrlApi = { + /** + * Resolve the preview URL for one entry's current values. + * + * Requires `read` on the collection — weaker than minting a preview LINK, + * which hands out a bearer credential. This returns a URL and no credential, + * so it shows the caller only what their own session already permits. + */ + resolve: (request: PreviewUrlRequest): Promise => + protectedApi.post("/preview-url", request), +}; diff --git a/packages/nextly/src/api/general-settings.test.ts b/packages/nextly/src/api/general-settings.test.ts index d5de907c60..51c307ad2e 100644 --- a/packages/nextly/src/api/general-settings.test.ts +++ b/packages/nextly/src/api/general-settings.test.ts @@ -33,10 +33,7 @@ vi.mock("../di", () => ({ import { isErrorResponse, requireAnyPermission } from "../auth/middleware"; import { container } from "../di"; -import { - getGeneralSettings, - updateGeneralSettings, -} from "./general-settings"; +import { getGeneralSettings, updateGeneralSettings } from "./general-settings"; const SETTINGS = { applicationName: "Nextly", @@ -93,4 +90,56 @@ describe("updateGeneralSettings", () => { expect(json.message).toMatch(/updated/i); expect(json.item).toEqual(updated); }); + + it("refuses a site URL whose scheme the browser would execute", async () => { + // `z.string().url()` accepts every one of these — measured — because the + // WHATWG parser does. The value is concatenated into preview URLs the admin + // assigns to location.href, where such a scheme runs script in the admin's + // own origin rather than navigating. + const executable = [ + "javascript:alert(document.cookie)", + "data:text/html,", + "vbscript:msgbox(1)", + "file:///etc/passwd", + ]; + const updateSettings = vi.fn(); + (container.get as ReturnType).mockReturnValue({ + updateSettings, + }); + + expect(executable.length).toBeGreaterThan(0); + for (const siteUrl of executable) { + const res = await updateGeneralSettings( + new Request("http://x/api/nextly/general-settings", { + method: "PATCH", + body: JSON.stringify({ siteUrl }), + }) + ); + + expect(res.status).toBe(400); + } + + // Rejected before the write, not merely reported afterwards. + expect(updateSettings).not.toHaveBeenCalled(); + }); + + it("still accepts an ordinary http(s) site URL", async () => { + // The positive control for the refusal above: without it, a validator that + // rejected everything would satisfy that test perfectly. + const updated = { ...SETTINGS, siteUrl: "http://localhost:3000" }; + const updateSettings = vi.fn().mockResolvedValue(updated); + (container.get as ReturnType).mockReturnValue({ + updateSettings, + }); + + const res = await updateGeneralSettings( + new Request("http://x/api/nextly/general-settings", { + method: "PATCH", + body: JSON.stringify({ siteUrl: "http://localhost:3000" }), + }) + ); + + expect(res.status).toBe(200); + expect(updateSettings).toHaveBeenCalled(); + }); }); diff --git a/packages/nextly/src/api/general-settings.ts b/packages/nextly/src/api/general-settings.ts index 48dafc3fd1..661f36db33 100644 --- a/packages/nextly/src/api/general-settings.ts +++ b/packages/nextly/src/api/general-settings.ts @@ -32,10 +32,27 @@ async function getGeneralSettingsService(): Promise { const updateSettingsSchema = z.object({ applicationName: z.string().max(255).nullable().optional(), + // `.url()` alone accepts any scheme the WHATWG parser does, `javascript:` and + // `data:` included — and this value is concatenated into preview URLs the + // admin assigns to `location.href`, where such a scheme executes rather than + // navigates. The resolver refuses those too; this stops one being stored in + // the first place, so a row written before that check existed is the only way + // one can still be present. siteUrl: z .string() .url("Site URL must be a valid URL") .max(2048) + .refine( + value => { + try { + const { protocol } = new URL(value); + return protocol === "http:" || protocol === "https:"; + } catch { + return false; + } + }, + { message: "Site URL must start with http:// or https://" } + ) .nullable() .optional(), adminEmail: z diff --git a/packages/nextly/src/api/preview-url.ts b/packages/nextly/src/api/preview-url.ts new file mode 100644 index 0000000000..d16717dbf9 --- /dev/null +++ b/packages/nextly/src/api/preview-url.ts @@ -0,0 +1,126 @@ +/** + * Resolving where an entry previews. + * + * POST /api/nextly/preview-url -> the preview URL for one entry's data + * + * **Gated on `read` for the collection, which is deliberately weaker than the + * `update` that guards minting a preview LINK.** The two hand out different + * things. A link is a bearer credential: it carries a signed token that grants + * whoever holds it a read of a draft, so it is gated at the level of someone who + * may edit that draft. This endpoint returns a URL and no credential — opening + * it shows the caller exactly what their own session already permits, and shows + * a stranger nothing that was not already public. Requiring `update` here would + * hide the preview button from a reviewer who may read a collection but not + * change it, which is a role the workflow exists to serve. + * + * **The site URL travels in the response, and that is the point rather than a + * leak.** `settings` is a system resource the `editor` and `author` presets do + * not grant, so those roles cannot read the configured site URL directly — and + * they are exactly who previews content. Answering with a finished absolute URL + * is what lets them preview without being handed a settings read they should not + * have. The response discloses the site's own public address, which anyone who + * visits the site already knows. + * + * @module api/preview-url + */ + +import { z } from "zod"; + +import { container } from "../di"; +import type { NextlyServiceConfig } from "../di/register"; +import type { CollectionRegistryService } from "../domains/collections/services/collection-registry-service"; +import { + resolvePreviewUrl, + type PreviewDeclaration, +} from "../domains/collections/services/preview-url-resolver"; +import { getCachedNextly } from "../init"; +import type { GeneralSettingsService } from "../services/general-settings/general-settings-service"; + +import { respondData } from "./response-shapes"; +import { requireRouteCollectionAccess } from "./route-auth"; +import { withErrorHandler } from "./with-error-handler"; +import { nextlyValidationFromZod } from "./zod-to-nextly-error"; + +/** + * The entry travels in the request body rather than being loaded by id. + * + * An editor previews what is on screen, which includes edits not yet saved — so + * the values that decide the URL are the form's, not the row's. Loading the row + * here would resolve a URL for the last saved state and quietly show the wrong + * page, which is worse than not offering the button. + * + * The data is the caller's own: they are sending back what they are already + * looking at, and the resolved URL returns only to them. So this widens no read. + */ +const resolveSchema = z.object({ + collection: z.string().min(1), + entry: z.record(z.string(), z.unknown()), +}); + +async function settingsService(): Promise { + await getCachedNextly(); + return container.get("generalSettingsService"); +} + +/** + * Read the preview declaration for a collection from whichever authoring path + * defined it. + * + * A code-first collection holds a `url` function that exists only in the + * server's module graph; a UI-created one holds a `urlTemplate` string in the + * registry. Both are read here and handed to the one resolver, so the caller + * never branches on which kind of collection it is looking at. + * + * **The authored config is consulted FIRST, and the order is load-bearing.** A + * code-first collection is also synced into the registry, but the function + * cannot survive that trip — its stored record carries `hasPreview` and the + * presentation options and nothing that can produce a URL. Reading the registry + * first would therefore find a declaration for exactly those collections and + * find it empty, and the resolver would correctly report `notConfigured` for a + * collection whose preview works. + */ +async function previewDeclarationFor( + collection: string +): Promise { + await getCachedNextly(); + + const config = container.get("config"); + const authored = config?.collections?.find(c => c.slug === collection); + if (authored?.admin?.preview) return authored.admin.preview; + + const registry = container.get( + "collectionRegistryService" + ); + const stored = await registry.getCollectionBySlug(collection); + return stored?.admin?.preview; +} + +/** + * POST /api/nextly/preview-url + * + * Answers with one of the resolver's states, so a caller can tell "this + * collection has no preview" from "this entry is not previewable yet" from "no + * site URL is configured". Collapsing them into a nullable string is what let an + * earlier attempt fall back to the admin's own origin and hand out a link to the + * wrong host. + */ +export const resolveEntryPreviewUrl = withErrorHandler(async (req: Request) => { + const body: unknown = await req.json().catch(() => undefined); + const parsed = resolveSchema.safeParse(body); + if (!parsed.success) throw nextlyValidationFromZod(parsed.error); + const { collection, entry } = parsed.data; + + // Per COLLECTION, so naming a collection the caller cannot read does not + // resolve a URL into it. Row-level rules are not consulted: the entry data + // came from the caller, so no row they cannot already see is involved. + await requireRouteCollectionAccess(req, "read", collection); + + const [preview, settings] = await Promise.all([ + previewDeclarationFor(collection), + settingsService().then(service => service.getSettings()), + ]); + + return respondData( + resolvePreviewUrl({ preview, entry, siteUrl: settings.siteUrl }) + ); +}); diff --git a/packages/nextly/src/dispatcher/types.ts b/packages/nextly/src/dispatcher/types.ts index acd304d336..99f3831c15 100644 --- a/packages/nextly/src/dispatcher/types.ts +++ b/packages/nextly/src/dispatcher/types.ts @@ -25,6 +25,7 @@ export type ServiceType = | "webhooks" | "generalSettings" | "previewLinks" + | "previewUrl" | "imageSizes" | "dashboard" | "email" diff --git a/packages/nextly/src/domains/collections/services/__tests__/persisted-admin.test.ts b/packages/nextly/src/domains/collections/services/__tests__/persisted-admin.test.ts index 78b40ffd6d..c6cb5cb590 100644 --- a/packages/nextly/src/domains/collections/services/__tests__/persisted-admin.test.ts +++ b/packages/nextly/src/domains/collections/services/__tests__/persisted-admin.test.ts @@ -54,6 +54,33 @@ describe("toPersistedAdmin", () => { }); }); + it("stores whether a preview exists, never the function that decides it", () => { + const persisted = toPersistedAdmin({ + useAsTitle: "title", + preview: { + url: entry => `/posts/${String(entry.slug)}`, + label: "View post", + openInNewTab: false, + }, + }); + + expect(persisted?.preview).toEqual({ + hasPreview: true, + label: "View post", + openInNewTab: false, + }); + // The function is the thing no column can hold. If it ever appears here the + // row write will either fail or silently store null, and the admin will be + // reading a key that cannot mean anything. + expect(persisted?.preview).not.toHaveProperty("url"); + }); + + it("omits preview entirely when a collection declares none", () => { + // Distinct from hasPreview: false — nothing was declared, so there is no + // preview object to describe. + expect(toPersistedAdmin({ useAsTitle: "title" })?.preview).toBeUndefined(); + }); + it("returns undefined when a collection declares no admin options", () => { expect(toPersistedAdmin(undefined)).toBeUndefined(); }); diff --git a/packages/nextly/src/domains/collections/services/__tests__/preview-url-resolver.test.ts b/packages/nextly/src/domains/collections/services/__tests__/preview-url-resolver.test.ts new file mode 100644 index 0000000000..f15acd135a --- /dev/null +++ b/packages/nextly/src/domains/collections/services/__tests__/preview-url-resolver.test.ts @@ -0,0 +1,309 @@ +import { describe, expect, it } from "vitest"; + +import { + hasPreviewConfigured, + resolvePreviewUrl, +} from "../preview-url-resolver"; + +const SITE = "https://example.com"; + +describe("hasPreviewConfigured", () => { + it("is false for a collection that declares no preview", () => { + expect(hasPreviewConfigured(undefined)).toBe(false); + expect(hasPreviewConfigured({})).toBe(false); + }); + + it("is true for either authoring path", () => { + expect(hasPreviewConfigured({ url: () => "/p" })).toBe(true); + expect(hasPreviewConfigured({ urlTemplate: "/p/{slug}" })).toBe(true); + }); + + it("is false for an empty template rather than merely present", () => { + // A stored empty string is what a UI field yields when it is cleared, and + // treating it as "configured" would persist hasPreview: true for a + // collection whose button can never resolve. + expect(hasPreviewConfigured({ urlTemplate: "" })).toBe(false); + }); + + it("agrees with the resolver about what counts as configured", () => { + // The stored boolean and the resolution must not drift: anything this + // predicate calls configured must resolve to something other than + // notConfigured, and vice versa. + const cases: Parameters[0][] = [ + undefined, + {}, + { urlTemplate: "" }, + { url: () => "/p" }, + { urlTemplate: "/p/{slug}" }, + ]; + + expect(cases.length).toBeGreaterThan(0); + for (const preview of cases) { + const resolution = resolvePreviewUrl({ + preview, + entry: { slug: "hello" }, + siteUrl: SITE, + }); + expect(resolution.status === "notConfigured").toBe( + !hasPreviewConfigured(preview) + ); + } + }); +}); + +describe("resolvePreviewUrl", () => { + it("reports notConfigured when nothing declares a preview", () => { + expect( + resolvePreviewUrl({ preview: undefined, entry: {}, siteUrl: SITE }) + ).toEqual({ status: "notConfigured" }); + }); + + it("resolves a code-first function against the site URL", () => { + expect( + resolvePreviewUrl({ + preview: { url: entry => `/posts/${String(entry.slug)}` }, + entry: { slug: "hello" }, + siteUrl: SITE, + }) + ).toEqual({ status: "resolved", url: "https://example.com/posts/hello" }); + }); + + it("resolves a template against the site URL", () => { + expect( + resolvePreviewUrl({ + preview: { urlTemplate: "/preview/{slug}" }, + entry: { slug: "hello" }, + siteUrl: SITE, + }) + ).toEqual({ status: "resolved", url: "https://example.com/preview/hello" }); + }); + + it("prefers the function when a declaration somehow carries both", () => { + expect( + resolvePreviewUrl({ + preview: { url: () => "/from-function", urlTemplate: "/from-template" }, + entry: {}, + siteUrl: SITE, + }) + ).toEqual({ status: "resolved", url: "https://example.com/from-function" }); + }); + + it("reports unavailable when the authored function declines", () => { + expect( + resolvePreviewUrl({ + preview: { url: () => null }, + entry: {}, + siteUrl: SITE, + }) + ).toEqual({ status: "unavailable" }); + }); + + it("reports unavailable when the authored function throws", () => { + // User code runs inside a request here. A throw is the collection failing to + // produce a URL, not the server failing, so it must not escape as a 500. + expect( + resolvePreviewUrl({ + preview: { + url: () => { + throw new Error("author bug"); + }, + }, + entry: {}, + siteUrl: SITE, + }) + ).toEqual({ status: "unavailable" }); + }); + + it("reports unavailable when a template placeholder has no value yet", () => { + for (const slug of [undefined, null, ""]) { + expect( + resolvePreviewUrl({ + preview: { urlTemplate: "/preview/{slug}" }, + entry: { slug }, + siteUrl: SITE, + }) + ).toEqual({ status: "unavailable" }); + } + }); + + it("interpolates falsy-but-real values instead of suppressing the URL", () => { + // A truthiness test here would drop a legitimate id of 0, which reads as + // "this entry cannot be previewed" for the one entry that can. + expect( + resolvePreviewUrl({ + preview: { urlTemplate: "/p/{id}" }, + entry: { id: 0 }, + siteUrl: SITE, + }) + ).toEqual({ status: "resolved", url: "https://example.com/p/0" }); + }); + + it("refuses an object placeholder rather than emitting [object Object]", () => { + const resolution = resolvePreviewUrl({ + preview: { urlTemplate: "/p/{ref}" }, + entry: { ref: { id: 1 } }, + siteUrl: SITE, + }); + expect(resolution).toEqual({ status: "unavailable" }); + }); + + it("refuses value types with no meaningful string form", () => { + // A denylist that excluded objects would let each of these through, and the + // function case is the sharp one: String(fn) is the function's source text, + // which is long, valid-looking, and silently wrong in a URL. + const rejected: unknown[] = [ + () => "x", + Symbol("s"), + { id: 1 }, + [1, 2], + Number.NaN, + Number.POSITIVE_INFINITY, + ]; + + expect(rejected.length).toBeGreaterThan(0); + for (const ref of rejected) { + expect( + resolvePreviewUrl({ + preview: { urlTemplate: "/p/{ref}" }, + entry: { ref }, + siteUrl: SITE, + }) + ).toEqual({ status: "unavailable" }); + } + }); + + it("escapes interpolated values so they cannot alter the path shape", () => { + const resolution = resolvePreviewUrl({ + preview: { urlTemplate: "/p/{slug}" }, + entry: { slug: "a/../b" }, + siteUrl: SITE, + }); + expect(resolution).toEqual({ + status: "resolved", + url: "https://example.com/p/a%2F..%2Fb", + }); + }); + + // The separating property. Every case below produces "no usable URL", and a + // resolver that answered null for all of them would pass any test asserting + // only absence. What distinguishes this one is that a host IS guessable here + // and guessing yields the ADMIN's origin, which is confidently wrong — so the + // status must be its own value and must carry the path forward. + it("reports noSiteUrl, distinctly from unavailable, when no host is known", () => { + const resolution = resolvePreviewUrl({ + preview: { url: () => "/posts/hello" }, + entry: {}, + siteUrl: null, + }); + + expect(resolution).toEqual({ status: "noSiteUrl", path: "/posts/hello" }); + expect(resolution.status).not.toBe("unavailable"); + expect(resolution.status).not.toBe("resolved"); + }); + + it("refuses a site URL the browser would EXECUTE rather than navigate to", () => { + // The resolved string is assigned to location.href by the admin, and these + // schemes run script in the assigning document's origin — which for the + // preview tab is the admin's own. `z.string().url()` accepts every one of + // them, so a settings write would otherwise become script execution for + // whoever next clicks Preview. + const executable = [ + "javascript:alert(document.cookie)", + "JavaScript:alert(1)", + "data:text/html,", + "vbscript:msgbox(1)", + "file:///etc/passwd", + ]; + + expect(executable.length).toBeGreaterThan(0); + for (const siteUrl of executable) { + const resolution = resolvePreviewUrl({ + preview: { url: () => "/posts/hello" }, + entry: {}, + siteUrl, + }); + + expect(resolution).toEqual({ status: "noSiteUrl", path: "/posts/hello" }); + // Never `resolved`: a caller branching on that status navigates to it. + expect(resolution.status).not.toBe("resolved"); + } + }); + + it("refuses an executable URL returned by the authored function too", () => { + // The declaration is user code and may compute anything, so the same + // standard applies to what it returns as to the configured site. + const resolution = resolvePreviewUrl({ + preview: { url: () => "javascript:alert(1)" }, + entry: {}, + siteUrl: SITE, + }); + + // Not absolute by the navigable test, so it is joined under the site origin, + // where the scheme is inert as an ordinary path segment. + expect(resolution).toEqual({ + status: "resolved", + url: "https://example.com/javascript:alert(1)", + }); + }); + + it("accepts a site URL carrying a base path", () => { + expect( + resolvePreviewUrl({ + preview: { url: () => "/posts/hello" }, + entry: {}, + siteUrl: "https://example.com/site/", + }) + ).toEqual({ + status: "resolved", + url: "https://example.com/site/posts/hello", + }); + }); + + it("passes an absolute authored URL through even with no site configured", () => { + // An author who wrote a full URL named the host on purpose; re-basing it + // against the configured site would override a deliberate choice. + expect( + resolvePreviewUrl({ + preview: { url: () => "https://staging.example.org/p/1" }, + entry: {}, + siteUrl: null, + }) + ).toEqual({ status: "resolved", url: "https://staging.example.org/p/1" }); + }); + + it("joins base and path without doubling or dropping the separator", () => { + const cases = [ + { siteUrl: "https://example.com/", path: "/p" }, + { siteUrl: "https://example.com", path: "p" }, + { siteUrl: "https://example.com///", path: "p" }, + ]; + + expect(cases.length).toBeGreaterThan(0); + for (const { siteUrl, path } of cases) { + expect( + resolvePreviewUrl({ preview: { url: () => path }, entry: {}, siteUrl }) + ).toEqual({ status: "resolved", url: "https://example.com/p" }); + } + }); + + it("hands the authored function the entry it was given", () => { + // Observed rather than reconstructed: asserting on the argument the real + // call receives is what keeps this honest if the call site starts reshaping + // the entry before passing it on. + const seen: Record[] = []; + const entry = { slug: "hello", status: "draft" }; + + resolvePreviewUrl({ + preview: { + url: received => { + seen.push(received); + return "/p"; + }, + }, + entry, + siteUrl: SITE, + }); + + expect(seen).toEqual([entry]); + }); +}); diff --git a/packages/nextly/src/domains/collections/services/collection-sync-service.ts b/packages/nextly/src/domains/collections/services/collection-sync-service.ts index 20787a0049..c1b4998969 100644 --- a/packages/nextly/src/domains/collections/services/collection-sync-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-sync-service.ts @@ -66,6 +66,7 @@ import { type CodeFirstCollectionConfig, type SyncResult, } from "./collection-registry-service"; +import { hasPreviewConfigured } from "./preview-url-resolver"; /** * Options for the sync operation. @@ -259,12 +260,6 @@ export const ADMIN_KEYS_NOT_PERSISTED = { * authoring paths describe a collection in one place. See `resolveDescription`. */ description: "stored on the collection row rather than under `admin`", - /** - * Carries `url`, a function of the entry, which no column can hold. Serving it needs the - * admin to ASK the server to evaluate it rather than to read it back, which is a change to - * how the panel obtains config and is deliberately not folded in here. - */ - preview: "holds a function; needs server-side evaluation rather than storage", } as const; /** @@ -310,6 +305,17 @@ export function toPersistedAdmin(admin: CollectionConfig["admin"]) { limits: admin.pagination.limits, } : undefined, + // The preview declaration minus the part no column can hold. `url` is a function of the + // entry, so what is stored is the ANSWER to the only question the admin asks of it — is + // there a preview here — plus the two presentation options the button needs to render. + // The URL itself depends on the entry and is resolved per request instead. + preview: admin.preview + ? { + hasPreview: hasPreviewConfigured(admin.preview), + label: admin.preview.label, + openInNewTab: admin.preview.openInNewTab, + } + : undefined, // Include custom components for plugins (e.g., custom Edit views) components: admin.components, }; diff --git a/packages/nextly/src/domains/collections/services/preview-url-resolver.ts b/packages/nextly/src/domains/collections/services/preview-url-resolver.ts new file mode 100644 index 0000000000..5a12da01b6 --- /dev/null +++ b/packages/nextly/src/domains/collections/services/preview-url-resolver.ts @@ -0,0 +1,237 @@ +/** + * Where an entry previews: one answer, derived once. + * + * A collection may declare a preview two ways, and they are disjoint by + * construction. A code-first collection declares `url`, a function of the entry + * that only exists in the server's module graph. A UI-created collection + * declares `urlTemplate`, a string with `{field}` placeholders, because there is + * nowhere for it to write a function. Both answer the same question, so both are + * answered here rather than at each call site. + * + * The function is why this runs on the server at all. No column can hold it, so + * the admin cannot read it back from the registry the way it reads every other + * `admin` option — it has to ASK. That has a second consequence worth stating + * plainly, because it is the reason the site URL is read here and not in the + * browser: resolving in the admin would mean the admin reading site settings, + * and `settings` is a system resource that the `editor` and `author` presets do + * not grant. Those are precisely the roles that share preview links. Resolving + * server-side means the caller needs access to the COLLECTION and never to + * settings, so the permission a previewer already holds is the only one asked + * for. + * + * @module domains/collections/services/preview-url-resolver + */ + +/** + * A preview declaration, as either authoring path may write it. + * + * The two fields are alternatives rather than a pair: `url` comes from + * `CollectionPreviewConfig` (code-first, where it is required) and `urlTemplate` + * from the dynamic-collection admin config (UI-created, where no function can be + * stored). Both are optional here because this type is the union the resolver + * sees, not either authoring surface. + */ +export interface PreviewDeclaration { + /** Code-first: computes the URL from the entry, or declines by returning null. */ + url?: (entry: Record) => string | null; + /** UI-created: a path with `{fieldName}` placeholders. */ + urlTemplate?: string; +} + +/** + * What a preview resolution can say, including that it cannot answer. + * + * Four cases and not two, deliberately. Three of them render as "no preview + * button", so collapsing them into `null` is the tempting simplification — and + * it is the one that caused the defect this shape exists to prevent. A resolver + * that answers `null` for "the site URL is not configured" is indistinguishable + * from one answering `null` for "this collection has no preview", so a caller + * that wants to recover has nothing to branch on and reaches for the only origin + * it can see: its own. That is the admin's host, which is confidently wrong. + * + * `notConfigured` and `unavailable` are also worth separating even though both + * hide the button, because only one of them is a state the editor can leave: an + * entry with no slug yet becomes previewable once it has one, while a collection + * with no preview declaration never does. + */ +export type PreviewUrlResolution = + /** A complete, absolute URL. */ + | { status: "resolved"; url: string } + /** This collection declares no preview at all. */ + | { status: "notConfigured" } + /** + * A preview is declared, but not for this entry right now — the authored + * function returned null, or a template placeholder has no value yet. + */ + | { status: "unavailable" } + /** + * A path was produced and nothing can name a host for it. Distinct from every + * case above because a guess IS available here and is wrong. + * + * Covers a site URL that is absent AND one that is unusable — a scheme the + * browser would execute rather than navigate to, which is refused rather than + * returned. Both share one remedy: an administrator sets a real site URL. They + * are merged for that reason and not because they are the same event. + */ + | { status: "noSiteUrl"; path: string }; + +/** + * True when a collection declares a preview by either route. + * + * Exported because the admin needs the button's PRESENCE without a round trip, + * and a boolean is something the registry can store even though the function it + * is derived from is not. Both readers ask this one predicate: the projection + * that persists `hasPreview` and the resolver below. Deriving the stored boolean + * from the same expression that decides the resolution is what keeps a persisted + * `hasPreview: true` from outliving the declaration it was computed from. + */ +export function hasPreviewConfigured( + preview: PreviewDeclaration | undefined +): boolean { + if (!preview) return false; + return typeof preview.url === "function" || Boolean(preview.urlTemplate); +} + +/** + * The text a field value contributes to a URL, or null if it cannot contribute. + * + * An allowlist of the types that have a meaningful string form, rather than a + * denylist of the ones that do not. The two are not equivalent: excluding + * objects leaves functions through, and `String(fn)` is the function's SOURCE + * TEXT — a long, valid-looking path segment. Naming what is permitted cannot + * develop that kind of gap as the set of possible field values grows. + * + * Empty string is rejected alongside null and undefined because all three mean + * the same thing to an editor: the field has not been filled in, so no URL can + * be built from it yet. + */ +function asUrlSegment(value: unknown): string | null { + if (typeof value === "string") return value === "" ? null : value; + if (typeof value === "number") + return Number.isFinite(value) ? String(value) : null; + if (typeof value === "boolean") return String(value); + if (typeof value === "bigint") return value.toString(); + return null; +} + +/** + * The schemes a resolved preview URL may carry. + * + * The result of this module is assigned to `location.href` by the admin, so what + * comes back is not a string — it is something the browser will EXECUTE if the + * scheme says so. `javascript:` and `data:` both run script in the assigning + * document's origin, which for the preview tab is the admin's own. + * + * An allowlist rather than a check for the two known-bad schemes: `vbscript:`, + * `blob:` and whatever a future engine adds would each need their own entry, and + * the gap would be silent. Naming what may navigate cannot develop that gap. + */ +const NAVIGABLE_PROTOCOLS = new Set(["http:", "https:"]); + +/** + * Parse `value` as an absolute URL that is safe to navigate to, or null. + * + * One predicate for both places an absolute URL enters this module — the site + * URL read from settings, and a declaration that returned a full URL itself — so + * the two cannot end up holding different opinions about what is navigable. + * + * The site URL is the reason this exists. It is stored through an API whose + * schema is `z.string().url()`, and that accepts any scheme the WHATWG parser + * does: `javascript:alert(1)` validates. Without this, such a value would be + * concatenated with a path, returned as `resolved`, and assigned to a + * same-origin blank tab — turning a settings write into script execution in the + * admin for whoever next clicks Preview. + */ +function asNavigableUrl(value: string): URL | null { + let parsed: URL; + try { + parsed = new URL(value); + } catch { + return null; + } + return NAVIGABLE_PROTOCOLS.has(parsed.protocol) ? parsed : null; +} + +/** + * Substitute `{fieldName}` placeholders with entry values. + * + * Returns null when any placeholder has no usable value, which is the + * `unavailable` case: a template naming `{slug}` cannot produce a URL for an + * entry whose slug has not been filled in yet. Values are compared against + * null/undefined/empty-string rather than tested for truthiness, so a legitimate + * `0` or `false` interpolates instead of suppressing the whole URL. + */ +function interpolate( + template: string, + entry: Record +): string | null { + const placeholders = template.match(/\{(\w+)\}/g); + if (!placeholders) return template; + + let result = template; + for (const placeholder of placeholders) { + const field = placeholder.slice(1, -1); + const text = asUrlSegment(entry[field]); + if (text === null) return null; + result = result.replace(placeholder, encodeURIComponent(text)); + } + return result; +} + +/** + * Resolve the preview URL for one entry. + * + * `siteUrl` is where the reader's site is served, which is what turns the + * authored path into something that survives being pasted into an email. A + * declaration that already returns an absolute URL is passed through untouched — + * an author who writes a full URL has named the host deliberately, and + * re-basing it against the configured site would override that. + */ +export function resolvePreviewUrl({ + preview, + entry, + siteUrl, +}: { + preview: PreviewDeclaration | undefined; + entry: Record; + siteUrl: string | null; +}): PreviewUrlResolution { + if (!hasPreviewConfigured(preview)) return { status: "notConfigured" }; + + let path: string | null = null; + + if (typeof preview?.url === "function") { + // The authored function is user code running inside a request. It may throw, + // and a throw here is the collection declining to preview rather than a + // server fault, so it is reported as `unavailable` — the same answer as a + // deliberate `return null` — instead of failing the request. + try { + path = preview.url(entry); + } catch { + return { status: "unavailable" }; + } + } else if (preview?.urlTemplate) { + path = interpolate(preview.urlTemplate, entry); + } + + if (path === null || path === "") return { status: "unavailable" }; + + // An author who returned a full URL named the host deliberately, so it is not + // re-based against the configured site. It still has to be navigable: the + // declaration is user code and may compute anything. + const absolute = asNavigableUrl(path); + if (absolute) return { status: "resolved", url: path }; + + // A relative path cannot carry a scheme, so it needs no such check — a value + // like `javascript:...` fails to parse as absolute above and lands here, where + // joining it under the site's origin makes it an ordinary path segment. + const base = siteUrl === null ? null : asNavigableUrl(siteUrl); + if (!base) return { status: "noSiteUrl", path }; + + // Join without doubling or dropping the separator: the configured site may or + // may not carry a trailing slash, and an authored path may or may not lead + // with one. + const origin = `${base.origin}${base.pathname}`.replace(/\/+$/, ""); + const suffix = path.startsWith("/") ? path : `/${path}`; + return { status: "resolved", url: `${origin}${suffix}` }; +} diff --git a/packages/nextly/src/route-handler/__tests__/route-parser.preview-url.test.ts b/packages/nextly/src/route-handler/__tests__/route-parser.preview-url.test.ts new file mode 100644 index 0000000000..f933dae471 --- /dev/null +++ b/packages/nextly/src/route-handler/__tests__/route-parser.preview-url.test.ts @@ -0,0 +1,58 @@ +/** + * The preview-URL route. + * + * Two properties are worth pinning. The route takes no path parameters at all, + * because what is being asked about is form state rather than a saved row — so a + * deeper path is a mistake rather than a variant, and must not be answered. And + * it must stay distinct from `preview-links`, which sits one segment away and + * hands out a bearer credential rather than a URL: a parser that confused the + * two would answer a credential request with a plain URL, or worse the reverse. + */ +import { describe, it, expect } from "vitest"; + +import { parseRestRoute } from "../route-parser"; + +describe("preview-url routes", () => { + it("parses the resolve request", () => { + expect(parseRestRoute(["preview-url"], "POST")).toMatchObject({ + service: "previewUrl", + operation: "create", + method: "resolveEntryPreviewUrl", + }); + }); + + it("refuses a deeper path rather than ignoring the extra segments", () => { + // An unmatched route is the empty object here, as the webhook suite pins for + // the same shape. Left to fall through instead, a mistyped longer path would + // be answered by the bare route and read as working. + expect(parseRestRoute(["preview-url", "123"], "POST")).toEqual({}); + expect(parseRestRoute(["preview-url", "123", "extra"], "POST")).toEqual({}); + }); + + it("refuses every method but POST", () => { + // The entry travels in the body, so nothing else can carry a request. Left + // matching, a GET would reach the JSON-body handler rather than being + // answered method-not-allowed — the adjacent preview-links parser rejects + // non-POST on its first line for the same reason. + for (const method of ["GET", "PUT", "PATCH", "DELETE", "HEAD"]) { + expect(parseRestRoute(["preview-url"], method)).toEqual({}); + } + // Positive control: the same route with the right method still resolves, so + // the assertions above are about the METHOD and not a broken route. + expect(parseRestRoute(["preview-url"], "POST")).toMatchObject({ + service: "previewUrl", + }); + }); + + it("stays distinct from the preview-links routes", () => { + // Same first word, different resource, and they must not answer for each + // other: one returns a URL, the other mints a credential. + expect(parseRestRoute(["preview-links"], "POST")).toMatchObject({ + service: "previewLinks", + method: "mintPreviewLink", + }); + expect(parseRestRoute(["preview-url"], "POST")).toMatchObject({ + service: "previewUrl", + }); + }); +}); diff --git a/packages/nextly/src/route-handler/route-parser.ts b/packages/nextly/src/route-handler/route-parser.ts index 9b2a35ff26..12b44f68a4 100644 --- a/packages/nextly/src/route-handler/route-parser.ts +++ b/packages/nextly/src/route-handler/route-parser.ts @@ -1770,6 +1770,37 @@ function parsePreviewLinkRoutes( return null; } +/** + * `POST /api/nextly/preview-url` resolves where one entry previews. + * + * The entry travels in the body rather than the path, because an editor + * previews what is on screen — including values not yet saved — so there is no + * id that identifies what is being asked about. + * + * Anything deeper is refused rather than folded in. A trailing segment here + * would otherwise be ignored, and a caller who mistyped a longer path would get + * a confident answer to a route they did not ask for. + */ +function parsePreviewUrlRoutes( + id: string | undefined, + subresource: string | undefined, + httpMethod: string, + routeParams: Record +): ParsedRoute | null { + // The entry travels in the body, so only POST can carry a request at all. + // Matching regardless of method would hand a GET straight to the JSON-body + // handler rather than answering method-not-allowed. + if (httpMethod !== "POST") return null; + if (id !== undefined || subresource !== undefined) return null; + + return { + service: "previewUrl", + operation: "create", + method: "resolveEntryPreviewUrl", + routeParams, + }; +} + function parseApiKeyRoutes( id: string | undefined, httpMethod: string, @@ -2368,6 +2399,17 @@ export function parseRestRoute( if (result) return result; } + // Handle resolving where an entry previews + if (resource === "preview-url") { + const result = parsePreviewUrlRoutes( + id, + subresource, + httpMethod, + routeParams + ); + if (result) return result; + } + // Handle API Keys endpoints if (resource === "api-keys") { const result = parseApiKeyRoutes(id, httpMethod, routeParams); diff --git a/packages/nextly/src/routeHandler.ts b/packages/nextly/src/routeHandler.ts index 8dc5ae2b94..7418160fb5 100644 --- a/packages/nextly/src/routeHandler.ts +++ b/packages/nextly/src/routeHandler.ts @@ -55,6 +55,7 @@ import { deleteImageSize, } from "./api/image-sizes"; import { mintPreviewLink, revokePreviewLinks } from "./api/preview-links"; +import { resolveEntryPreviewUrl } from "./api/preview-url"; import { readOrGenerateRequestId, withRequestIdHeader } from "./api/request-id"; // canonical respondX wire shapes (spec §5.1) instead of the // hand-rolled `{ data: }` envelope. @@ -341,6 +342,7 @@ const DIRECT_DISPATCH_SERVICES = new Set([ "webhooks", "generalSettings", "previewLinks", + "previewUrl", "imageSizes", "dashboard", "schema", @@ -941,6 +943,13 @@ async function handleServiceRequest( : mintPreviewLink(req); } + // ==================== PREVIEW URL DIRECT DISPATCH ==================== + // Beside the handlers above and for the same reason: it parses its own JSON, + // and the shared body read below would leave the stream empty. + if (service === "previewUrl") { + return resolveEntryPreviewUrl(req); + } + // ==================== GENERAL SETTINGS DIRECT DISPATCH ==================== if (service === "generalSettings") { return handleGeneralSettingsRequest(req, httpMethod); diff --git a/packages/nextly/src/schemas/dynamic-collections/types.ts b/packages/nextly/src/schemas/dynamic-collections/types.ts index f558025ca9..9f4b6aa035 100644 --- a/packages/nextly/src/schemas/dynamic-collections/types.ts +++ b/packages/nextly/src/schemas/dynamic-collections/types.ts @@ -193,10 +193,27 @@ export interface CollectionAdminConfig { * ``` */ preview?: { + /** + * Whether this collection previews at all, decided when the config was synced. + * + * A code-first collection declares its preview as a FUNCTION of the entry, and no column can + * hold one — so the admin cannot read the declaration back the way it reads every other + * option. What it needs from the declaration is only whether a preview button belongs on the + * page, and that is a boolean, so the boolean is what gets stored. + * + * Derived by `hasPreviewConfigured`, which is also what the resolver consults, so a stored + * `true` and a resolution that reports `notConfigured` cannot disagree. The URL itself is + * never stored: it depends on the entry, so it is resolved per request. + */ + hasPreview?: boolean; + /** * URL template with field placeholders in {fieldName} format. * Used for UI-created collections where functions can't be stored. * + * Read by the server when resolving a preview URL, never by the admin: interpolating it in + * the browser would be a second implementation of a question the resolver already answers. + * * @example "/preview/{slug}", "/api/preview?id={id}" */ urlTemplate?: string;