History

Render not-found for an org slug the session does not belong to (#1825)

Rhys Sullivan committed 2 months ago

Commit 06a7b751 parent a9bf6234 files · +102 −21Browse files

Unified · Split

4 files
  1. .changeset/foreign-org-slug-not-found.md +13 −0
  2. apps/cloud/src/routes/__root.tsx +25 −7
  3. e2e/scenarios/org-slug-routing.test.ts +59 −13
  4. packages/react/src/multiplayer/org-slug-gate.tsx +5 −1
.changeset/foreign-org-slug-not-found.md+13−0
@@ -0,0 +1,13 @@
1---
2"executor": patch
3---
4
5**A console URL naming an organization you cannot see is a not-found page, every time**
6
7Opening `/<some-other-slug>/policies` sometimes rendered the full authenticated console — sidebar, org switcher showing your OWN organization, page chrome — under an address naming an organization you are not a member of. The page body was a failing org-scoped query with a Retry button, so the workspace on screen belonged to nobody and the URL belonged to someone else.
8
9The shell's not-found only fired once `/account/me` had answered for the URL's slug. Until then the console read its identity from the auth-hint cookie, which always names the organization the session last landed in, never the one in the address bar. So the first paint answered a question about a different organization and built a whole workspace out of it, and whether you ever saw that depended on how fast the server replied.
10
11The shell is now built only from an answer that names the organization the URL names. A slug the current answer does not cover renders nothing at all until `/account/me` resolves for that slug, and then either the workspace or — for an organization this session cannot see — the not-found page. The URL is never rewritten: a wrong address stays a wrong address.
12
13The ordinary cold load is untouched. The hint names the slug already in the URL, so it matches on the very first paint and the shell renders with no round trip. Only a slug the hint does not name waits: a foreign one, and the single frame after switching organizations, which now paints the organization the URL asked for instead of briefly showing the previous one.
apps/cloud/src/routes/__root.tsx+25−7
@@ -262,15 +262,33 @@ function AuthGate() { · 261 unmodified lines
262 return urlOrgSlug ? <NotFoundPage /> : <BlankScreen />;262 return urlOrgSlug ? <NotFoundPage /> : <BlankScreen />;
263 }263 }
264 264
265 // The authenticated answer must NAME the org the URL names before any shell
266 // is built from it. `auth.organization` is the auth-hint cookie until
267 // `/account/me` lands, and the hint always names the session's OWN org — so
268 // on a foreign slug it is an answer about a different organization, and
269 // rendering the shell from it puts the user in a workspace the URL never
270 // named. `/account/me` is scoped by the URL's slug (getActiveOrgSlug), so
271 // once it resolves this can only agree or be null; a disagreement is
272 // therefore always an unresolved answer, never a verdict. Blank, not
273 // not-found: the 404 above is the only thing entitled to declare a wrong
274 // address, and it waits for the server.
275 //
276 // The legitimate cold load is untouched: the hint names the slug in the URL,
277 // so this matches on the very first paint and the shell renders with no
278 // round trip. Only a slug the hint does not name pays the wait — a foreign
279 // slug (which then 404s) and the frame after an org switch (which then
280 // renders the org the URL asked for, instead of flashing the previous one).
281 if (pathnameOrgSlug != null && auth.organization.slug !== pathnameOrgSlug) {
282 return <BlankScreen />;
283 }
284
265 const activeSlug = auth.organization.slug;285 const activeSlug = auth.organization.slug;
266 // The org context's slug feeds the connect card's `/<slug>/mcp` install URL.286 // The org context's slug feeds the connect card's `/<slug>/mcp` install URL.
267 // Prefer the URL's slug over the session's: on first paint `auth.organization`287 // Source it from the URL, which is the actual request scope and is correct on
268 // comes from the SSR auth-hint (the COOKIE's org), so a multi-org user viewing288 // the very first paint. The gate above has already made the two agree
269 // /<orgB> while their cookie still points at orgA would briefly render orgA's289 // whenever the URL names a slug at all, so this is the same value stated in
270 // slug in the copyable URL before /account/me (URL-scoped) corrects it. The290 // the terms the rest of the tree is keyed on. VALIDATED (pathnameOrgSlug, not
271 // URL slug is the actual request scope and is correct on the very first paint,291 // the raw route param): the `{-$orgSlug}` param also captures reserved
272 // so sourcing it from there removes that flash. VALIDATED (pathnameOrgSlug,
273 // not the raw route param): the `{-$orgSlug}` param also captures reserved
274 // console roots ("/integrations" → orgSlug "integrations"), which are not292 // console roots ("/integrations" → orgSlug "integrations"), which are not
275 // org scopes. Falls back to the auth org on a bare/reserved URL (which293 // org scopes. Falls back to the auth org on a bare/reserved URL (which
276 // OrgSlugGate canonicalizes onto it below).294 // OrgSlugGate canonicalizes onto it below).
e2e/scenarios/org-slug-routing.test.ts+59−13
@@ -5,9 +5,10 @@ · 4 unmodified lines
5//5//
6// - /account/me advertises the org's URL slug (valid grammar)6// - /account/me advertises the org's URL slug (valid grammar)
7// - a bare deep link (/policies) canonicalizes to /<slug>/policies7// - a bare deep link (/policies) canonicalizes to /<slug>/policies
8// - an unknown slug (/zz-no-such-org/policies) is a wrong address — a8// - an unknown slug (/zz-no-such-org/policies) is a wrong address on a
9// not-found page, never a silent redirect into a workspace the URL9// multi-tenant host — a not-found page with the console shell never built
10// didn't name10// at any point, never a silent redirect into a workspace the URL didn't
11// name (a single-tenant host canonicalizes it instead; both are pinned)
11// - in-shell navigation keeps the slug prefix on every link12// - in-shell navigation keeps the slug prefix on every link
12//13//
13// Cloud's switch-into-another-org-by-URL behavior is covered separately by14// Cloud's switch-into-another-org-by-URL behavior is covered separately by
@@ -20,6 +21,14 @@ import { scenario } from "../src/scenario"; · 6 unmodified lines
20import { Api, Browser, Target } from "../src/services";21import { Api, Browser, Target } from "../src/services";
21import { visit } from "../src/surfaces/browser";22import { visit } from "../src/surfaces/browser";
22 23
24declare global {
25 interface Window {
26 /** Set by this scenario's init script the first time a console shell
27 * sidebar is attached to the document. Test-only. */
28 __executorShellEverMounted?: boolean;
29 }
30}
31
23scenario(32scenario(
24 "Org URLs · console paths carry the organization slug",33 "Org URLs · console paths carry the organization slug",
25 {},34 {},
@@ -46,17 +55,54 @@ scenario( · 20 unmodified lines
46 await page.getByText("Policies").first().waitFor();55 await page.getByText("Policies").first().waitFor();
47 });56 });
48 57
49 // The "unknown slug is a 404" contract is multi-tenant only. Selfhost is58 // What an unknown slug MEANS depends on tenancy, so both answers are
50 // single-tenant: /account/me always returns the instance org regardless of59 // pinned rather than one being skipped:
51 // the URL segment, so the slug is cosmetic and an unknown one canonicalizes60 //
52 // onto the shell rather than 404ing. Cloud enforces the not-found; selfhost61 // multi-tenant (cloud) — the slug is a real request scope. /account/me
53 // legitimately does not.62 // returns no organization for one this session cannot see, so the
54 if (!target.name.startsWith("selfhost")) {63 // address is simply wrong: a not-found page, the URL left exactly as
55 await step("An unknown org slug is a wrong address, not a redirect", async () => {64 // typed, and no console shell built at any point along the way.
56 await visit(page, "/zz-no-such-org/policies");65 // single-tenant (selfhost) — /account/me always returns the instance
66 // org regardless of the URL segment, so the slug is cosmetic and
67 // OrgSlugGate canonicalizes it onto the shell instead.
68 const singleTenant = target.name.startsWith("selfhost");
69
70 // Waiting for the not-found text on its own is not enough. The shell used
71 // to render first from the auth-hint cookie — which names the session's
72 // OWN org, never the URL's — and correct itself only once /account/me
73 // answered, so a run that flashed an entire workspace under a foreign
74 // address still ended on "Page not found" and passed. Record the shell
75 // from before the page's own scripts run, so a sidebar that exists for a
76 // single frame is still caught long after it is gone.
77 await page.addInitScript(() => {
78 window.__executorShellEverMounted = false;
79 const look = () => {
80 if (document.querySelector('aside, button[aria-label="Open navigation"]')) {
81 window.__executorShellEverMounted = true;
82 }
83 };
84 new MutationObserver(look).observe(document, { childList: true, subtree: true });
85 look();
86 });
87
88 await step("An unknown org slug resolves by tenancy, never by redirect", async () => {
89 await visit(page, "/zz-no-such-org/policies");
90 if (singleTenant) {
91 await page.waitForURL((url) => url.pathname === `/${slug}/policies`, { timeout: 30_000 });
92 } else {
57 await page.getByText("Page not found").waitFor({ timeout: 30_000 });93 await page.getByText("Page not found").waitFor({ timeout: 30_000 });
58 });94 }
59 }95
96 const landed = {
97 pathname: new URL(page.url()).pathname,
98 shellEverMounted: await page.evaluate(() => window.__executorShellEverMounted === true),
99 };
100 expect(landed, "an unknown slug never lands in a workspace the URL did not name").toEqual(
101 singleTenant
102 ? { pathname: `/${slug}/policies`, shellEverMounted: true }
103 : { pathname: "/zz-no-such-org/policies", shellEverMounted: false },
104 );
105 });
60 106
61 await step("In-shell navigation keeps the slug prefix", async () => {107 await step("In-shell navigation keeps the slug prefix", async () => {
62 await visit(page, `/${slug}`);108 await visit(page, `/${slug}`);
packages/react/src/multiplayer/org-slug-gate.tsx+5−1
@@ -16,7 +16,11 @@ import { useNavigate, useParams, useRouterState } from "@tanstack/react-router"; · 15 unmodified lines
16// membership and resolves data for that org — same as the MCP URL-pinned org.16// membership and resolves data for that org — same as the MCP URL-pinned org.
17// So a foreign slug never reaches this gate as "active" on a multi-org host:17// So a foreign slug never reaches this gate as "active" on a multi-org host:
18// the server returns no organization for an org the caller can't see, and the18// the server returns no organization for an org the caller can't see, and the
19// shell 404s upstream. That makes two browser tabs on different orgs fully19// shell 404s upstream. The host is what enforces that — it must not build this
20// subtree until the authenticated answer NAMES the slug in the URL, or an
21// optimistic answer about the session's own org (cloud's auth-hint cookie)
22// would mount the shell under a foreign address before the server ever
23// replies. That makes two browser tabs on different orgs fully
20// independent — no shared "active org" to steal. On a single-org host (e.g.24// independent — no shared "active org" to steal. On a single-org host (e.g.
21// self-host) every slug resolves to the same org server-side, so a bogus slug25// self-host) every slug resolves to the same org server-side, so a bogus slug
22// (e.g. `/totally-bogus`) would otherwise fuzzy-match a route and render26// (e.g. `/totally-bogus`) would otherwise fuzzy-match a route and render