diff --git a/.changeset/public-admin-meta-is-branding-only.md b/.changeset/public-admin-meta-is-branding-only.md new file mode 100644 index 0000000000..143bf80c37 --- /dev/null +++ b/.changeset/public-admin-meta-is-branding-only.md @@ -0,0 +1,28 @@ +--- +"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 +--- + +Serve only branding from the public `/api/admin-meta`. Plugin contributions, configured locales, custom sidebar groups and builder availability now come from the session-gated `/api/admin-meta/workspace`, so a plugin-declared permission slug is no longer readable before sign-in. The admin reads both and merges them, so no component changes. diff --git a/packages/admin/src/components/field-ui/usePluginClientConfig.ts b/packages/admin/src/components/field-ui/usePluginClientConfig.ts index 1a183e25f9..6e2f661862 100644 --- a/packages/admin/src/components/field-ui/usePluginClientConfig.ts +++ b/packages/admin/src/components/field-ui/usePluginClientConfig.ts @@ -26,8 +26,15 @@ export function usePluginClientConfig( pluginName: string ): Record | undefined { const branding = useBranding(); - return useMemo( - () => branding.plugins?.find(p => p.name === pluginName)?.clientConfig, - [branding.plugins, pluginName] - ); + return useMemo(() => { + // The installed list when it has arrived, the public channel otherwise. + // Both carry `clientConfig`, and the gated one is the richer record — so + // this prefers it rather than reading two sources and reconciling them. + // + // The fallback is what a plugin contributing to the SIGN-IN screen depends + // on: there is no session yet, so the installed list cannot exist, and its + // absence says nothing about whether the plugin declared a config. + const declared = branding.plugins ?? branding.pluginClientConfigs; + return declared?.find(p => p.name === pluginName)?.clientConfig; + }, [branding.plugins, branding.pluginClientConfigs, pluginName]); } diff --git a/packages/admin/src/context/providers/BrandingProvider.test.tsx b/packages/admin/src/context/providers/BrandingProvider.test.tsx index 224ad752cb..51fdfecee6 100644 --- a/packages/admin/src/context/providers/BrandingProvider.test.tsx +++ b/packages/admin/src/context/providers/BrandingProvider.test.tsx @@ -11,11 +11,16 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import type { AdminBranding } from "@admin/types/branding"; const get = vi.fn(); +const protectedGet = vi.fn(); vi.mock("@admin/lib/api/publicApi", () => ({ publicApi: { get: (...args: unknown[]) => get(...args) }, })); +vi.mock("@admin/lib/api/protectedApi", () => ({ + protectedApi: { get: (...args: unknown[]) => protectedGet(...args) }, +})); + import { BrandingProvider, useBrandingStatus, @@ -46,12 +51,14 @@ function status() { afterEach(() => { get.mockReset(); + protectedGet.mockReset(); vi.restoreAllMocks(); }); describe("useBrandingStatus", () => { - it("reports pending until the request answers", async () => { + it("reports pending until both requests answer", async () => { get.mockReturnValue(new Promise(() => {})); + protectedGet.mockReturnValue(new Promise(() => {})); renderProbe( new QueryClient({ defaultOptions: { queries: { retry: false } } }) @@ -60,8 +67,9 @@ describe("useBrandingStatus", () => { expect(status()).toBe("pending"); }); - it("reports answered once branding arrives", async () => { - get.mockResolvedValue({ plugins: [] } as AdminBranding); + it("reports answered once both halves arrive", async () => { + get.mockResolvedValue({ logoText: "Acme" } as AdminBranding); + protectedGet.mockResolvedValue({ plugins: [] } as AdminBranding); renderProbe( new QueryClient({ defaultOptions: { queries: { retry: false } } }) @@ -70,8 +78,45 @@ describe("useBrandingStatus", () => { await waitFor(() => expect(status()).toBe("answered")); }); - it("reports unavailable when the first request fails", async () => { + it("settles once the workspace half answers, even if branding is stalled", async () => { + // The ASYMMETRIC case, and the only one that separates the two halves. + // Every other case here moves both queries together, so a status + // combining them passes all of them — measured: reverting to + // `brandingPending || workspacePending` left the rest of this file green. + // + // Its reader draws a conclusion from a plugin being absent, so holding it + // on a loading state while the plugin list has arrived hides a settled + // answer behind a request that has nothing to do with the question. + get.mockReturnValue(new Promise(() => {})); + protectedGet.mockResolvedValue({ plugins: [] } as AdminBranding); + + renderProbe( + new QueryClient({ defaultOptions: { queries: { retry: false } } }) + ); + + await waitFor(() => expect(status()).toBe("answered")); + }); + + it("stays unavailable when only the branding half arrives", async () => { + // The property this hook exists for, and the one the split could have + // broken silently. Its reader concludes something from a plugin being + // ABSENT, and the plugin list lives in the session-gated half — so + // branding having answered says nothing about whether that conclusion is + // safe. Reporting the public query's state here would call it `answered` + // while the list had never been fetched. + get.mockResolvedValue({ logoText: "Acme" } as AdminBranding); + protectedGet.mockRejectedValue(new Error("unauthenticated")); + + renderProbe( + new QueryClient({ defaultOptions: { queries: { retry: false } } }) + ); + + await waitFor(() => expect(status()).toBe("unavailable")); + }); + + it("reports unavailable when the workspace request fails", async () => { get.mockRejectedValue(new Error("boom")); + protectedGet.mockRejectedValue(new Error("boom")); renderProbe( new QueryClient({ defaultOptions: { queries: { retry: false } } }) diff --git a/packages/admin/src/context/providers/BrandingProvider.tsx b/packages/admin/src/context/providers/BrandingProvider.tsx index ff3519a24e..7310673053 100644 --- a/packages/admin/src/context/providers/BrandingProvider.tsx +++ b/packages/admin/src/context/providers/BrandingProvider.tsx @@ -4,6 +4,7 @@ import { useQuery } from "@tanstack/react-query"; import type React from "react"; import { createContext, useContext, useEffect, useMemo } from "react"; +import { protectedApi } from "../../lib/api/protectedApi"; import { publicApi } from "../../lib/api/publicApi"; import { DEFAULT_MARK_PATHS, @@ -29,8 +30,22 @@ import type { */ interface BrandingState { branding: AdminBranding | undefined; - /** True until the admin-meta query settles, either way. */ + /** + * True until the WORKSPACE query settles, either way. + * + * Not the public half: this pairs with `isUnavailable` to answer whether a + * plugin being absent is a fact, and the plugin list is in the workspace + * half alone. + */ isPending: boolean; + /** + * True when the PUBLIC half never produced an answer. + * + * Separate from `isUnavailable` because the two halves fail independently + * and for different reasons: the workspace half fails routinely before + * sign-in, which says nothing about branding being readable. + */ + isBrandingUnavailable: boolean; /** * True when admin-meta has never produced an answer, so absence proves * nothing. @@ -66,6 +81,7 @@ export function useBrandingStatus(): Omit { return { isPending: state?.isPending ?? false, isUnavailable: state?.isUnavailable ?? false, + isBrandingUnavailable: state?.isBrandingUnavailable ?? false, }; } @@ -182,12 +198,11 @@ interface BrandingProviderProps { export function BrandingProvider({ children }: BrandingProviderProps) { const { - data: fetchedData, - isPending, + data: brandingData, // `isLoadingError`, not `isError`: the latter is also true when a // background refetch fails while a previous response is still cached, and // that cached response is a perfectly good answer. - isLoadingError, + isLoadingError: brandingUnavailable, } = useQuery({ queryKey: ["admin-meta"], queryFn: () => publicApi.get("/admin-meta"), @@ -197,16 +212,59 @@ export function BrandingProvider({ children }: BrandingProviderProps) { retry: false, }); - useColorInjection(fetchedData?.colors); - useFaviconInjection(fetchedData?.favicon); + // The half that describes the installation rather than its appearance. It + // comes from a session-gated route, so before sign-in this query fails and + // contributes nothing — which is correct, since no pre-session surface reads + // these fields. + const { + data: workspaceData, + isPending: workspacePending, + isLoadingError: workspaceUnavailable, + } = useQuery({ + queryKey: ["admin-meta", "workspace"], + queryFn: () => protectedApi.get("/admin-meta/workspace"), + staleTime: 5 * 60 * 1000, + retry: false, + }); - // Memoized because the value is now an object built here rather than the - // query's own stable `data` reference: without this every consumer of the + useColorInjection(brandingData?.colors); + useFaviconInjection(brandingData?.favicon); + + // Memoized because the value is an object built here rather than either + // query's stable `data` reference: without this every consumer of the // context re-renders on each render of this provider. - const value = useMemo( - () => ({ branding: fetchedData, isPending, isUnavailable: isLoadingError }), - [fetchedData, isPending, isLoadingError] - ); + // + // The two halves are merged so the shape consumers read is unchanged; the + // boundary that matters is the one on the server, which decides what an + // anonymous caller can be served at all. + const value = useMemo(() => { + const merged = + brandingData === undefined && workspaceData === undefined + ? undefined + : { ...brandingData, ...workspaceData }; + return { + branding: merged, + // The WORKSPACE query, matching `isUnavailable`. Both answer one + // question — is it safe to conclude something from a plugin being + // absent — and the plugin list is in that half. Combining the two + // reports "still loading" while the only relevant query has settled, so + // a stalled public request would hold the reader on a loading state + // indefinitely and hide a definitive workspace error behind it. + isPending: workspacePending, + // Reported from the WORKSPACE query. The reader this exists for treats a + // plugin's absence from the list as a fact about the project, and the + // plugin list lives in that half — so branding having arrived says + // nothing about whether that conclusion is safe to draw. + isUnavailable: workspaceUnavailable, + isBrandingUnavailable: brandingUnavailable, + }; + }, [ + brandingData, + workspaceData, + workspacePending, + workspaceUnavailable, + brandingUnavailable, + ]); return ( diff --git a/packages/admin/src/lib/api/refreshInterceptor.ts b/packages/admin/src/lib/api/refreshInterceptor.ts index b8491c91b5..ec3a87d5a4 100644 --- a/packages/admin/src/lib/api/refreshInterceptor.ts +++ b/packages/admin/src/lib/api/refreshInterceptor.ts @@ -65,13 +65,17 @@ export function setLoginRedirectPath(path: string): void { * 3. /admin/login mounts → PublicRoute checks setup-status, * sees no users → navigateTo("/admin/setup"). Bounce. */ -const NO_REDIRECT_PUBLIC_PATHS = new Set([ +export const NO_REDIRECT_PUBLIC_PATHS = new Set([ "/admin/login", "/admin/setup", "/admin/register", "/admin/forgot-password", "/admin/reset-password", "/admin/verify-email", + // An invited user reaching this page has no session yet, so a protected + // background query answers 401 exactly as it does on the others. Its absence + // bounced them to login and lost the invite token in the URL. + "/admin/accept-invite", ]); /** diff --git a/packages/admin/src/pages/dashboard/plugins/[slug].tsx b/packages/admin/src/pages/dashboard/plugins/[slug].tsx index 1d365fb92a..1dba6cca00 100644 --- a/packages/admin/src/pages/dashboard/plugins/[slug].tsx +++ b/packages/admin/src/pages/dashboard/plugins/[slug].tsx @@ -1,11 +1,7 @@ "use client"; import { Badge } from "@nextlyhq/ui"; -import { - useQuery, - useQueryClient, - useSuspenseQuery, -} from "@tanstack/react-query"; +import { useQuery, useSuspenseQuery } from "@tanstack/react-query"; import { Suspense } from "react"; import { @@ -24,10 +20,7 @@ import { } from "@admin/components/icons"; import { PageContainer } from "@admin/components/layout/page-container"; import { Breadcrumbs } from "@admin/components/shared"; -import { - PageErrorFallback, - SectionErrorFallback, -} from "@admin/components/shared/error-fallbacks"; +import { PageErrorFallback } from "@admin/components/shared/error-fallbacks"; import { PluginIcon } from "@admin/components/shared/plugin-icon"; import { QueryErrorBoundary } from "@admin/components/shared/query-error-boundary"; import { Link } from "@admin/components/ui/link"; @@ -46,6 +39,7 @@ import { } from "@admin/services/realPermissionsApi"; import type { PluginMetadata } from "@admin/types/branding"; +import { InstalledPluginsUnavailable } from "./components/InstalledPluginsUnavailable"; import { NotInstalledPlugin } from "./components/NotInstalledPlugin"; import { PluginPageLoading } from "./components/PluginPageLoading"; import { PluginStatusPill } from "./components/PluginsTable"; @@ -149,27 +143,6 @@ function UninstalledOrMissing({ activeSlug }: { activeSlug?: string }) { ); } -/** - * Shown when admin-meta failed, so whether this plugin is installed is - * unknown. - * - * Not the catalogue view: that one states the plugin is absent, which is a - * claim this page cannot make when the request that would have told it failed. - */ -function InstalledPluginsUnavailable() { - const queryClient = useQueryClient(); - - return ( - { - void queryClient.invalidateQueries({ queryKey: ["admin-meta"] }); - }} - /> - ); -} - function PluginDetailContent({ activeSlug }: { activeSlug?: string }) { const branding = useBranding(); const { isPending, isUnavailable } = useBrandingStatus(); diff --git a/packages/admin/src/pages/dashboard/plugins/browse.test.tsx b/packages/admin/src/pages/dashboard/plugins/browse.test.tsx index 7e5099ffe0..100a2cc5c9 100644 --- a/packages/admin/src/pages/dashboard/plugins/browse.test.tsx +++ b/packages/admin/src/pages/dashboard/plugins/browse.test.tsx @@ -14,8 +14,17 @@ import type { AdminBranding } from "@admin/types/branding"; let mockBranding: AdminBranding = { plugins: [] } as unknown as AdminBranding; -vi.mock("@admin/lib/api/publicApi", () => ({ - publicApi: { get: () => Promise.resolve(mockBranding) }, +// The provider, not the transport. Installed status comes from the +// session-gated half of admin-meta, which the page reads through this hook +// rather than by fetching for itself — mocking the public client would supply +// a payload the page no longer asks for and leave every entry uninstalled. +vi.mock("@admin/context/providers/BrandingProvider", () => ({ + useBranding: () => mockBranding, + useBrandingStatus: () => ({ + isPending: false, + isUnavailable: false, + isBrandingUnavailable: false, + }), })); import PluginBrowsePage from "./browse"; diff --git a/packages/admin/src/pages/dashboard/plugins/browse.tsx b/packages/admin/src/pages/dashboard/plugins/browse.tsx index 00c7872567..fe71e15668 100644 --- a/packages/admin/src/pages/dashboard/plugins/browse.tsx +++ b/packages/admin/src/pages/dashboard/plugins/browse.tsx @@ -21,16 +21,21 @@ import { PageErrorFallback } from "@admin/components/shared/error-fallbacks"; import { QueryErrorBoundary } from "@admin/components/shared/query-error-boundary"; import { SearchBar } from "@admin/components/shared/search-bar"; import { ROUTES } from "@admin/constants/routes"; -import { publicApi } from "@admin/lib/api/publicApi"; +import { + useBranding, + useBrandingStatus, +} from "@admin/context/providers/BrandingProvider"; import { resolveCataloguePresentation } from "@admin/lib/plugins/registry/resolve-catalogue-presentation"; import { shouldShowFeatured, staticRegistrySource, } from "@admin/lib/plugins/registry/static-source"; import type { RegistryPlugin } from "@admin/lib/plugins/registry/types"; -import type { AdminBranding, PluginMetadata } from "@admin/types/branding"; +import type { PluginMetadata } from "@admin/types/branding"; +import { InstalledPluginsUnavailable } from "./components/InstalledPluginsUnavailable"; import { PluginCard } from "./components/PluginCard"; +import { PluginPageLoading } from "./components/PluginPageLoading"; /** * Name, the RENDERED description, and tags. @@ -79,11 +84,14 @@ function PluginGrid({ function BrowseContent(): React.ReactElement { const [query, setQuery] = useState(""); - const { data: branding } = useSuspenseQuery({ - queryKey: ["admin-meta"], - queryFn: () => publicApi.get("/admin-meta"), - staleTime: 5 * 60 * 1000, - }); + // Read through the provider rather than a second query of its own. The + // plugin list is served by the session-gated route, and a duplicate reader + // pointed at the public one shares the same cache key while asking a + // question that route no longer answers — every installed plugin would read + // as uninstalled. + const branding = useBranding(); + const { isPending: pluginsPending, isUnavailable: pluginsUnavailable } = + useBrandingStatus(); const { data: entries } = useSuspenseQuery({ queryKey: ["plugin-registry"], queryFn: () => staticRegistrySource.list(), @@ -136,6 +144,17 @@ function BrowseContent(): React.ReactElement { [visible, featuredIdSet] ); + // Installed status is read from a list that arrives separately from the + // catalogue. Until it has, every entry looks uninstalled — which tells + // someone who already has a plugin to go and install it — so the directory + // waits rather than rendering a claim it cannot yet support. + if (pluginsPending) return ; + // A request that never answered is not an empty install list. Reporting + // every catalogue entry as uninstalled from an unanswered query tells + // someone who has the plugin to install it again, so the failure is shown + // instead of a claim derived from it. + if (pluginsUnavailable) return ; + return ( <>
diff --git a/packages/admin/src/pages/dashboard/plugins/components/InstalledPluginsUnavailable.tsx b/packages/admin/src/pages/dashboard/plugins/components/InstalledPluginsUnavailable.tsx new file mode 100644 index 0000000000..adb57e8af8 --- /dev/null +++ b/packages/admin/src/pages/dashboard/plugins/components/InstalledPluginsUnavailable.tsx @@ -0,0 +1,37 @@ +"use client"; + +import { useQueryClient } from "@tanstack/react-query"; +import type React from "react"; + +import { SectionErrorFallback } from "@admin/components/shared/error-fallbacks"; + +/** + * What a plugins page shows when the installed list never arrived. + * + * Distinct from the catalogue's "not installed" view, which STATES the plugin + * is absent — a claim no page can make from a request that failed. Every + * surface here reads installation from one list, so an unanswered request has + * to be reported rather than rendered as an empty result. + * + * The provider serves that list through `useBranding`, which neither suspends + * nor throws, so a surrounding `Suspense` or error boundary never sees the + * failure and each page shows this itself. + * + * @module pages/dashboard/plugins/components/InstalledPluginsUnavailable + */ +export function InstalledPluginsUnavailable(): React.ReactElement { + const queryClient = useQueryClient(); + + return ( + { + // Both halves: the list lives in the workspace query, and a caller + // retrying from here means "fetch the admin metadata again" rather + // than naming one of the two requests it is split across. + void queryClient.invalidateQueries({ queryKey: ["admin-meta"] }); + }} + /> + ); +} diff --git a/packages/admin/src/pages/dashboard/plugins/components/PluginsTable.tsx b/packages/admin/src/pages/dashboard/plugins/components/PluginsTable.tsx index 9c0d5ca17d..792105fc21 100644 --- a/packages/admin/src/pages/dashboard/plugins/components/PluginsTable.tsx +++ b/packages/admin/src/pages/dashboard/plugins/components/PluginsTable.tsx @@ -10,7 +10,6 @@ import { DropdownMenuSeparator, DropdownMenuTrigger, } from "@nextlyhq/ui"; -import { useSuspenseQuery } from "@tanstack/react-query"; import { useCallback, useEffect, useMemo, useState } from "react"; import { Columns } from "@admin/components/icons"; @@ -21,12 +20,18 @@ import type { NextlyColumn } from "@admin/components/ui/table/data-table"; import { ListShell } from "@admin/components/ui/table/list-shell"; import { ROUTES, buildRoute } from "@admin/constants/routes"; import { UI } from "@admin/constants/ui"; +import { + useBranding, + useBrandingStatus, +} from "@admin/context/providers/BrandingProvider"; import { useDebouncedValue } from "@admin/hooks/useDebouncedValue"; import { usePagination } from "@admin/hooks/usePagination"; -import { publicApi } from "@admin/lib/api/publicApi"; import { categoryLabel } from "@admin/lib/plugins/plugin-categories"; import { pluginSlug } from "@admin/lib/plugins/plugin-slug"; -import type { PluginMetadata, AdminBranding } from "@admin/types/branding"; +import type { PluginMetadata } from "@admin/types/branding"; + +import { InstalledPluginsUnavailable } from "./InstalledPluginsUnavailable"; +import { PluginsTableSkeleton } from "./PluginsTableSkeleton"; type PluginWithId = PluginMetadata & { id: string }; @@ -71,11 +76,18 @@ export function PluginStatusPill({ enabled }: { enabled: boolean }) { * the table exposes no mutation actions. */ export default function PluginsTable() { - const { data: branding } = useSuspenseQuery({ - queryKey: ["admin-meta"], - queryFn: () => publicApi.get("/admin-meta"), - staleTime: 5 * 60 * 1000, - }); + // Read through the provider rather than a second query of its own. The + // plugin list is served by the session-gated route, and a duplicate reader + // pointed at the public one shares the same cache key while asking a + // question that route no longer answers — the table would render empty. + const branding = useBranding(); + // `useBranding` neither suspends nor throws, so the Suspense boundary and + // the error boundary around this table can no longer show their + // fallbacks. Without these two branches an unanswered request renders the + // definitive empty state: momentarily on a slow load, permanently after a + // failure. + const { isPending: pluginsPending, isUnavailable: pluginsUnavailable } = + useBrandingStatus(); const [search, setSearch] = useState(""); const debouncedSearch = useDebouncedValue(search, UI.SEARCH_DEBOUNCE_MS); @@ -219,6 +231,12 @@ export default function PluginsTable() { [allColumns] ); + // Before the table, because its empty state is a STATEMENT: "no plugins + // installed" read from a request that has not answered is wrong while it is + // in flight and stays wrong after it fails. + if (pluginsPending) return ; + if (pluginsUnavailable) return ; + return ( { expect(screen.queryByText("Plugin not found")).toBeNull(); }); + /** + * A plugin declaring a PUBLIC client config is not a plugin the project has + * installed, and the two arrive from different requests. Holding them under + * one key let the public entry answer "is it installed" before the gated + * request had said anything — and because the page's pending and unavailable + * checks sit behind "did I find it", finding it there skipped both. + */ + it("does not treat a public client config as an installed plugin", async () => { + mockBranding = { + pluginClientConfigs: [ + { name: "@nextlyhq/plugin-seo", clientConfig: { a: 1 } }, + ], + } as unknown as AdminBranding; + mockBrandingStatus = { isPending: true, isUnavailable: false }; + + renderDetail("nextlyhq-plugin-seo"); + + expect(await screen.findByRole("status")).toBeInTheDocument(); + expect(screen.queryByText(INSTALL_COMMAND)).toBeNull(); + }); + + it("still reports the installed list unavailable despite a public config", async () => { + mockBranding = { + pluginClientConfigs: [ + { name: "@nextlyhq/plugin-seo", clientConfig: { a: 1 } }, + ], + } as unknown as AdminBranding; + mockBrandingStatus = { isPending: false, isUnavailable: true }; + + renderDetail("nextlyhq-plugin-seo"); + + expect( + await screen.findByText("Could not load your installed plugins") + ).toBeInTheDocument(); + }); + /** Same reasoning, permanent: a failed request is not evidence of absence. */ it("does not offer to install when the installed list could not be loaded", async () => { mockBrandingStatus = { isPending: false, isUnavailable: true }; diff --git a/packages/admin/src/types/branding.ts b/packages/admin/src/types/branding.ts index 2b5ec0ac5b..041ff61ad7 100644 --- a/packages/admin/src/types/branding.ts +++ b/packages/admin/src/types/branding.ts @@ -39,6 +39,19 @@ export interface PluginWidgetMeta { requiredPermission?: string; } +/** + * A plugin's public client configuration, served before a session exists. + * + * Separate from `plugins` because the two answer different questions: + * this says a plugin DECLARED a public config, while `plugins` says which + * plugins the project has installed. Merging them would let a plugin with + * a public config read as installed before the gated request answers. + */ +export interface PluginClientConfigMeta { + name: string; + clientConfig?: Record; +} + /** Plugin metadata returned by the `/admin-meta` API. */ export interface PluginMetadata { name: string; @@ -184,9 +197,25 @@ export interface AdminBranding { /** Runtime toggle for builder-related navigation visibility. */ showBuilder?: boolean; - /** Installed plugin metadata for sidebar rendering and plugin settings pages. */ + /** + * INSTALLED plugin metadata, for sidebar rendering and plugin settings pages. + * + * Comes from the session-gated half alone, so its absence means the list has + * not arrived rather than that the project has no plugins. Read + * `useBrandingStatus()` before concluding anything from a plugin missing here. + */ plugins?: PluginMetadata[]; + /** + * Public client configuration, readable before a session exists. + * + * Deliberately NOT merged into `plugins`: a plugin declaring a public config + * is not a plugin the project has installed, and a reader that found it in + * that list would have skipped the checks saying the installed list is still + * unavailable. Only `usePluginClientConfig` reads this. + */ + pluginClientConfigs?: PluginClientConfigMeta[]; + /** Custom sidebar groups created by the user for organizing collections/singles. */ customGroups?: Array<{ slug: string; name: string; icon?: string }>; diff --git a/packages/nextly/src/__tests__/routeHandler-admin-meta-workspace.test.ts b/packages/nextly/src/__tests__/routeHandler-admin-meta-workspace.test.ts index 7a86999df9..9dec6bd17a 100644 --- a/packages/nextly/src/__tests__/routeHandler-admin-meta-workspace.test.ts +++ b/packages/nextly/src/__tests__/routeHandler-admin-meta-workspace.test.ts @@ -32,11 +32,34 @@ afterAll(() => { else process.env.DB_DIALECT = ORIGINAL_DB_DIALECT; }); -function handlers() { +function handlers({ withPlugin = false }: { withPlugin?: boolean } = {}) { return createDynamicHandlers({ config: sanitizeConfig({ collections: [], admin: { branding: { logoText: "Acme" } }, + // A plugin contributing BOTH a public client config and a gated page, so + // the projection has something it must carry and something it must not. + plugins: withPlugin + ? ([ + { + name: "@acme/p", + version: "1.0.0", + nextly: "*", + contributes: { + admin: { + clientConfig: { providerId: "acme-sso" }, + pages: [ + { + path: "settings", + component: "AcmeSettings", + requiredPermission: "manage-acme", + }, + ], + }, + }, + }, + ] as never) + : undefined, }), }); } @@ -107,16 +130,88 @@ describe("admin-meta split over HTTP", () => { expect(response.headers.get("vary")).toBeNull(); }); - it("keeps the workspace fields on the public route for now", async () => { - // The admin still reads these from the public payload. Removing them - // before it is migrated would blank the sidebar rather than close - // anything, so this asserts the duplication is intact and is expected to - // be inverted once the client reads the authenticated route. + it("serves no field outside the branding vocabulary to an anonymous caller", async () => { + // An ALLOWLIST, deliberately. A list of fields to withhold has to be + // extended by whoever adds the next one, and plugin authors choose what a + // contribution carries — so the next sensitive field would be public by + // default and nothing here would notice. Asserting the whole key set + // instead means any addition to the public half fails this until someone + // decides it belongs there. const response = await handlers().GET( request("admin-meta"), ctx(["admin-meta"]) ); + const payload = (await response.json()) as Record; + + expect(Object.keys(payload).sort()).toEqual( + [ + "colors", + "favicon", + "logoText", + "logoUrl", + "logoUrlDark", + "logoUrlLight", + // Permitted, and narrowed by its own pair of cases below: the entries + // carry a name and a public client config and nothing else. + "pluginClientConfigs", + ].filter(key => key in payload) + ); + // The population clause: a payload that happened to be empty would satisfy + // the subset assertion above without proving anything was read. + expect(payload.logoText).toBe("Acme"); + }); + + it("serves a plugin's public client config before sign-in", async () => { + // A plugin may contribute components to the SIGN-IN screen, and those read + // their own config through the SDK before a session exists. That channel is + // public by declaration and holds no secrets, so withholding it removes the + // configuration those components render from. + const response = await handlers({ withPlugin: true }).GET( + request("admin-meta"), + ctx(["admin-meta"]) + ); + const payload = (await response.json()) as Record; + + expect(payload.pluginClientConfigs).toEqual([ + { name: "@acme/p", clientConfig: { providerId: "acme-sso" } }, + ]); + // Under its OWN key. Sharing `plugins` would let these entries stand in + // for the installed list on the client, where the two halves are merged. + expect(payload.plugins).toBeUndefined(); + }); + + it("withholds everything else about that plugin from the public route", async () => { + // The separating assertion, and the reason the projection NAMES its two + // fields rather than deleting the rest: an exact key set fails when a + // contribution field added later reaches the public payload, which a check + // for specific forbidden fields would not. + const response = await handlers({ withPlugin: true }).GET( + request("admin-meta"), + ctx(["admin-meta"]) + ); + const payload = (await response.json()) as Record; + const plugins = payload.pluginClientConfigs as Array< + Record + >; + + expect(Object.keys(plugins[0] ?? {}).sort()).toEqual([ + "clientConfig", + "name", + ]); + expect(JSON.stringify(payload)).not.toContain("manage-acme"); + expect(JSON.stringify(payload)).not.toContain("AcmeSettings"); + }); + + it("withholds plugin contributions from the public route", async () => { + const response = await handlers().GET( + request("admin-meta"), + ctx(["admin-meta"]) + ); + const payload = (await response.json()) as Record; - expect(await response.json()).toMatchObject({ showBuilder: true }); + expect(payload.plugins).toBeUndefined(); + expect(payload.showBuilder).toBeUndefined(); + expect(payload.locales).toBeUndefined(); + expect(payload.customGroups).toBeUndefined(); }); }); diff --git a/packages/nextly/src/__tests__/routeHandler-direct-branches.test.ts b/packages/nextly/src/__tests__/routeHandler-direct-branches.test.ts index df6280fd49..d301f2fb92 100644 --- a/packages/nextly/src/__tests__/routeHandler-direct-branches.test.ts +++ b/packages/nextly/src/__tests__/routeHandler-direct-branches.test.ts @@ -204,7 +204,11 @@ describe("handleAdminMetaRequest (GET /admin-meta)", () => { expect(json).not.toHaveProperty("data"); // Bare body shape: branding fields live at the top level. expect(json.logoText).toBe("Test Co."); - expect(json.showBuilder).toBe(false); + // `showBuilder` describes the installation rather than its appearance, so + // it moved to the session-gated route with the rest of the workspace half. + // Asserted as absent HERE rather than dropped, so the public payload's + // boundary stays covered from this suite as well. + expect(json.showBuilder).toBeUndefined(); }); }); diff --git a/packages/nextly/src/routeHandler.ts b/packages/nextly/src/routeHandler.ts index 2c769a2ef9..8dc5ae2b94 100644 --- a/packages/nextly/src/routeHandler.ts +++ b/packages/nextly/src/routeHandler.ts @@ -1294,6 +1294,31 @@ async function buildAdminMeta(): Promise<{ const plugins = buildPluginAdminMeta(config?.plugins ?? [], pluginOverrides); if (plugins.length > 0) { workspace.plugins = plugins; + + // A plugin may contribute components to the SIGN-IN screen, and those + // read their own `clientConfig` through the plugin SDK before a session + // exists. That channel is public by declaration — it never holds secrets + // — so it is projected here rather than withheld. + // + // Built by naming the two fields it carries rather than by removing the + // rest: a contribution field added later is then absent from this + // projection by construction, which is the same reason the public + // payload is a separate half rather than a filtered copy of the whole. + // + // Under its OWN key, never `plugins`. The client merges the two halves, + // so sharing a key would let these entries stand in for the installed + // list before the gated request answers — and a reader that finds a + // plugin there has already skipped the checks that would have told it + // the list is not available yet. + const publicPlugins = plugins + .filter(plugin => plugin.clientConfig !== undefined) + .map(plugin => ({ + name: plugin.name, + clientConfig: plugin.clientConfig, + })); + if (publicPlugins.length > 0) { + branding.pluginClientConfigs = publicPlugins; + } } // Override config branding with DB values when available @@ -1364,15 +1389,15 @@ function withSessionCacheHeaders(response: Response): Response { * Public — no authentication required, because the sign-in screen renders * before a session exists. * - * Still carries the workspace half, which `GET /api/admin-meta/workspace` - * now also serves. The duplication is deliberate and temporary: the admin - * reads these fields from here until it is migrated to the authenticated - * route, and removing them before then would blank the sidebar rather than - * close anything. + * Branding ONLY. What an anonymous caller may read is decided by which half + * of `buildAdminMeta` is serialized here, rather than by a list of fields to + * withhold — so a contribution field added later is private by default. A + * filter would have to be extended by whoever adds it, and plugin authors + * choose those fields rather than this package. */ async function handleAdminMetaRequest(): Promise { - const { branding, workspace } = await buildAdminMeta(); - return respondAdminMeta({ ...branding, ...workspace }); + const { branding } = await buildAdminMeta(); + return respondAdminMeta(branding); } /**