fix(cli): pages get and pull read public pages from other organizations (SUPER-3431)

packages/cli/src/commands/pages/get/command.ts

@@ -1,11 +1,20 @@
 import { positional } from "@superset/cli-framework";
 import { command } from "../../../lib/command";
 import { pageRefFromArg } from "../pageRef";
+import { publicFallback } from "../publicPage";
 
 export default command({
 	description: "Show a page",
-	args: [positional("page").required().desc("Page id or slug")],
-	run: async ({ ctx, args }) => ({
-		data: await ctx.api.page.get.query(pageRefFromArg(args.page as string)),
-	}),
+	args: [positional("page").required().desc("Page id, slug or link")],
+	run: async ({ ctx, args }) => {
+		const ref = pageRefFromArg(args.page as string);
+		try {
+			return { data: await ctx.api.page.get.query(ref) };
+		} catch (error) {
+			const page = await publicFallback(ref, error, (slug) =>
+				ctx.api.page.publicView.query({ slug }),
+			);
+			return { data: { ...page, access: "public" as const } };
+		}
+	},
 });

packages/cli/src/commands/pages/pageRef.test.ts

@@ -0,0 +1,31 @@
+import { describe, expect, test } from "bun:test";
+import { pageLinkVersion, pageRefFromArg } from "./pageRef";
+
+const ID = "60c2d931-0edc-4681-949c-344ed9fa6265";
+
+describe("pageRefFromArg", () => {
+	test("reads the slug out of a page link", () => {
+		expect(
+			pageRefFromArg("https://app.superset.sh/page/q3-pipeline-ab12cd/?v=2"),
+		).toEqual({ slug: "q3-pipeline-ab12cd" });
+	});
+
+	test("keeps ids and bare slugs as they are", () => {
+		expect(pageRefFromArg(ID)).toEqual({ id: ID });
+		expect(pageRefFromArg("q3-pipeline")).toEqual({ slug: "q3-pipeline" });
+	});
+});
+
+describe("pageLinkVersion", () => {
+	test("is the version a page link asks for, as the web viewer reads it", () => {
+		expect(pageLinkVersion("https://app.superset.sh/page/q3?v=2")).toBe(2);
+	});
+
+	test("is undefined without a usable version", () => {
+		expect(pageLinkVersion("https://app.superset.sh/page/q3")).toBeUndefined();
+		expect(
+			pageLinkVersion("https://app.superset.sh/page/q3?v=x"),
+		).toBeUndefined();
+		expect(pageLinkVersion("q3?v=2")).toBeUndefined();
+	});
+});

packages/cli/src/commands/pages/pageRef.ts

@@ -1,7 +1,21 @@
 const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
+const PAGE_PATH = /^\/page\/([^/]+)\/?$/;
+
+function pageLink(value: string): { slug: string; url: URL } | null {
+	if (!URL.canParse(value)) return null;
+	const url = new URL(value);
+	const match = PAGE_PATH.exec(url.pathname);
+	return match?.[1] ? { slug: decodeURIComponent(match[1]), url } : null;
+}
 
 export function pageRefFromArg(
 	value: string,
 ): { id: string } | { slug: string } {
-	return UUID.test(value) ? { id: value } : { slug: value };
+	const ref = pageLink(value)?.slug ?? value;
+	return UUID.test(ref) ? { id: ref } : { slug: ref };
+}
+
+export function pageLinkVersion(value: string): number | undefined {
+	const version = Number(pageLink(value)?.url.searchParams.get("v"));
+	return Number.isInteger(version) && version > 0 ? version : undefined;
 }

packages/cli/src/commands/pages/publicPage.test.ts

@@ -0,0 +1,71 @@
+import { describe, expect, test } from "bun:test";
+import { TRPCClientError } from "@trpc/client";
+import { publicFallback } from "./publicPage";
+
+const PUBLIC_PAGE = { slug: "q3", version: 1 };
+
+function trpcError(code: string, i18nKey: string | null) {
+	return new TRPCClientError("refused", {
+		result: {
+			error: { message: "refused", code: -1, data: { code, i18nKey } },
+		},
+	});
+}
+
+const pageMissing = () =>
+	trpcError("NOT_FOUND", "serverError.page.pageNotFound");
+
+function reader(result: typeof PUBLIC_PAGE | null) {
+	const asked: string[] = [];
+	const read = async (slug: string) => {
+		asked.push(slug);
+		return result;
+	};
+	return { read, asked };
+}
+
+describe("publicFallback", () => {
+	test("reads the public page when the organization lookup misses it", async () => {
+		const { read, asked } = reader(PUBLIC_PAGE);
+		expect(await publicFallback({ slug: "q3" }, pageMissing(), read)).toEqual(
+			PUBLIC_PAGE,
+		);
+		expect(asked).toEqual(["q3"]);
+	});
+
+	test("keeps the original error when the page is not public", async () => {
+		const { read } = reader(null);
+		const missing = pageMissing();
+		await expect(publicFallback({ slug: "q3" }, missing, read)).rejects.toBe(
+			missing,
+		);
+	});
+
+	test("keeps the switch-organization error for a member of the page's organization", async () => {
+		const { read, asked } = reader(PUBLIC_PAGE);
+		const otherOrg = trpcError("FORBIDDEN", null);
+		await expect(publicFallback({ slug: "q3" }, otherOrg, read)).rejects.toBe(
+			otherOrg,
+		);
+		expect(asked).toEqual([]);
+	});
+
+	test("does not hide a missing version behind the public one", async () => {
+		const { read, asked } = reader(PUBLIC_PAGE);
+		const noVersion = trpcError("NOT_FOUND", null);
+		await expect(publicFallback({ slug: "q3" }, noVersion, read)).rejects.toBe(
+			noVersion,
+		);
+		expect(asked).toEqual([]);
+	});
+
+	test("keeps the original error when the public read fails", async () => {
+		const missing = pageMissing();
+		const read = async () => {
+			throw new Error("No procedure found");
+		};
+		await expect(publicFallback({ slug: "q3" }, missing, read)).rejects.toBe(
+			missing,
+		);
+	});
+});

packages/cli/src/commands/pages/publicPage.ts

@@ -0,0 +1,22 @@
+import { TRPCClientError } from "@trpc/client";
+
+function pageNotFound(error: unknown): boolean {
+	if (!(error instanceof TRPCClientError)) return false;
+	const data = error.data as { i18nKey?: string | null } | null;
+	return data?.i18nKey === "serverError.page.pageNotFound";
+}
+
+/**
+ * A page anyone with the link can see is often outside the caller's
+ * organizations, so the organization lookup misses it.
+ */
+export async function publicFallback<T>(
+	ref: { id: string } | { slug: string },
+	error: unknown,
+	read: (slug: string) => Promise<T | null>,
+): Promise<T> {
+	if (!("slug" in ref) || !pageNotFound(error)) throw error;
+	const page = await read(ref.slug).catch(() => null);
+	if (!page) throw error;
+	return page;
+}

packages/cli/src/commands/pages/pull/command.ts

@@ -1,6 +1,7 @@
 import { CLIError, number, positional } from "@superset/cli-framework";
-import { command } from "../../../lib/command";
-import { pageRefFromArg } from "../pageRef";
+import { type CliContext, command } from "../../../lib/command";
+import { pageLinkVersion, pageRefFromArg } from "../pageRef";
+import { publicFallback } from "../publicPage";
 
 const DOWNLOAD_TIMEOUT_MS = 30_000;
 
@@ -14,38 +15,67 @@ function downloadFailed(version: number, cause: unknown): CLIError {
 	return new CLIError(`Could not download version ${version}`, detail);
 }
 
+async function download(url: string, version: number): Promise<ArrayBuffer> {
+	try {
+		const response = await fetch(url, {
+			signal: AbortSignal.timeout(DOWNLOAD_TIMEOUT_MS),
+		});
+		if (!response.ok) {
+			throw new CLIError(
+				`Could not download version ${version}`,
+				`The blob store answered ${response.status}`,
+			);
+		}
+		return await response.arrayBuffer();
+	} catch (error) {
+		if (error instanceof CLIError) throw error;
+		throw downloadFailed(version, error);
+	}
+}
+
+async function locateVersion(
+	ctx: CliContext,
+	arg: string,
+	requested: number | undefined,
+): Promise<{ url: string; version: number }> {
+	const ref = pageRefFromArg(arg);
+	try {
+		const version = await ctx.api.page.pull.query({
+			...ref,
+			...(requested ? { version: requested } : {}),
+		});
+		return { url: version.downloadUrl, version: version.version };
+	} catch (error) {
+		const page = await publicFallback(ref, error, (slug) =>
+			ctx.api.page.publicPull.query({ slug }),
+		);
+		if (requested && requested !== page.version) {
+			throw new CLIError(
+				`Version ${requested} is not public`,
+				`From outside its organization, only the version this page serves (${page.version}) can be read`,
+			);
+		}
+		return { url: page.downloadUrl, version: page.version };
+	}
+}
+
 export default command({
 	description: "Write a published version's HTML to stdout",
-	args: [positional("page").required().desc("Page id or slug")],
+	args: [positional("page").required().desc("Page id, slug or link")],
 	options: {
 		version: number()
 			.alias("v")
-			.desc("Version to fetch (defaults to the one currently served)"),
+			.desc(
+				"Version to fetch (defaults to a link's ?v=, else the one currently served)",
+			),
 	},
 	run: async ({ ctx, args, options }) => {
-		const version = await ctx.api.page.pull.query({
-			...pageRefFromArg(args.page as string),
-			...(options.version ? { version: options.version } : {}),
-		});
-
-		let body: ArrayBuffer;
-		try {
-			const response = await fetch(version.downloadUrl, {
-				signal: AbortSignal.timeout(DOWNLOAD_TIMEOUT_MS),
-			});
-			if (!response.ok) {
-				throw new CLIError(
-					`Could not download version ${version.version}`,
-					`The blob store answered ${response.status}`,
-				);
-			}
-			body = await response.arrayBuffer();
-		} catch (error) {
-			if (error instanceof CLIError) throw error;
-			throw downloadFailed(version.version, error);
-		}
-
-		process.stdout.write(Buffer.from(body));
+		const { url, version } = await locateVersion(
+			ctx,
+			args.page as string,
+			options.version ?? pageLinkVersion(args.page as string),
+		);
+		process.stdout.write(Buffer.from(await download(url, version)));
 		return undefined;
 	},
 });