History

Fix SSR crash from the admin nav and add org API key revoke

The admin nav hook read orgMembersAtom, which is wrapped in refreshOnWindowFocus — a browser-only atom whose signal touches window at read time. Evaluating it during SSR threw ReferenceError synchronously in render, above the routed outlet, so the whole authenticated document fell back to client rendering and the landing page was blank on first paint. Give the atom an explicit server value so its read never runs on the server; the role reads as unknown during SSR and resolves client-side, and the nav still fails closed. Org API keys could be minted and listed but never revoked. Add DELETE /account/org-api-keys/:apiKeyId with the same admin gate as the mint, resolving ownership against the org's own keys so a member's personal key or a sibling org's key cannot be destroyed through it. Self-host and cloudflare refuse it as they already refuse the mint.

Rhys Sullivan committed 3 months ago

Commit 445a4cd1 parent 8bca26215 files · +454 −8Browse files

Showing the changes to packages (5 of 15 files). Show all

Unified · Split

5 files
  1. packages/core/api/src/account/api.ts +12 −0
  2. packages/core/api/src/account/handlers.ts +6 −0
  3. packages/core/api/src/account/service.ts +4 −0
  4. packages/react/src/api/account-atoms.tsx +19 −5
  5. packages/react/src/multiplayer/use-admin-nav.tsx +7 −0
packages/core/api/src/account/api.ts+12−0
@@ -219,6 +219,18 @@ export const AccountApi = HttpApiGroup.make("account") · 218 unmodified lines
219 error: [AccountError, AccountUnauthorized, AccountForbidden, AccountNoOrganization],219 error: [AccountError, AccountUnauthorized, AccountForbidden, AccountNoOrganization],
220 }),220 }),
221 )221 )
222 // Revoke is its own route rather than a reuse of `revokeApiKey`: that one
223 // resolves ownership against the caller's PERSONAL keys, so an org key id is
224 // never in its owned set and revoking one there always fails. An org key is
225 // the most privileged credential in the product (it reads the tenant-wide
226 // `/admin/*` plane), so it must be revocable — admin-gated, like the mint.
227 .add(
228 HttpApiEndpoint.delete("revokeOrgApiKey", "/account/org-api-keys/:apiKeyId", {
229 params: ApiKeyParams,
230 success: SuccessResponse,
231 error: [AccountError, AccountUnauthorized, AccountForbidden, AccountNoOrganization],
232 }),
233 )
222 .add(234 .add(
223 HttpApiEndpoint.get("listMembers", "/account/members", {235 HttpApiEndpoint.get("listMembers", "/account/members", {
224 success: OrgMembersResponse,236 success: OrgMembersResponse,
packages/core/api/src/account/handlers.ts+6−0
@@ -56,6 +56,12 @@ export const AccountHandlers = HttpApiBuilder.group(AccountHttpApi, "account", ( · 55 unmodified lines
56 return yield* (yield* AccountProvider).createOrgApiKey(headers, payload.name);56 return yield* (yield* AccountProvider).createOrgApiKey(headers, payload.name);
57 }),57 }),
58 )58 )
59 .handle("revokeOrgApiKey", ({ params }) =>
60 Effect.gen(function* () {
61 const headers = yield* requestHeaders;
62 return yield* (yield* AccountProvider).revokeOrgApiKey(headers, params.apiKeyId);
63 }),
64 )
59 .handle("listMembers", () =>65 .handle("listMembers", () =>
60 Effect.gen(function* () {66 Effect.gen(function* () {
61 const headers = yield* requestHeaders;67 const headers = yield* requestHeaders;
packages/core/api/src/account/service.ts+4−0
@@ -57,6 +57,10 @@ export interface AccountProviderShape { · 56 unmodified lines
57 headers: AccountHeaders,57 headers: AccountHeaders,
58 name: string,58 name: string,
59 ) => OrgScoped<CreatedApiKey, AccountForbidden>;59 ) => OrgScoped<CreatedApiKey, AccountForbidden>;
60 readonly revokeOrgApiKey: (
61 headers: AccountHeaders,
62 apiKeyId: string,
63 ) => OrgScoped<Success, AccountForbidden>;
60 readonly listMembers: (headers: AccountHeaders) => OrgScoped<Members>;64 readonly listMembers: (headers: AccountHeaders) => OrgScoped<Members>;
61 readonly listRoles: (headers: AccountHeaders) => OrgScoped<Roles>;65 readonly listRoles: (headers: AccountHeaders) => OrgScoped<Roles>;
62 readonly inviteMember: (66 readonly inviteMember: (
packages/react/src/api/account-atoms.tsx+19−5
@@ -28,11 +28,25 @@ export const revokeApiKey = AccountApiClient.mutation("account", "revokeApiKey") · 27 unmodified lines
28 28
29// ── Organization members ─────────────────────────────────────────────────────29// ── Organization members ─────────────────────────────────────────────────────
30 30
31export const orgMembersAtom = Atom.refreshOnWindowFocus(31// `refreshOnWindowFocus` is BROWSER-ONLY: its signal atom subscribes to
32 AccountApiClient.query("account", "listMembers", {32// `window`/`document.visibilityState` the first time the atom is read, so
33 timeToLive: "30 seconds",33// evaluating it on a server throws `ReferenceError: window is not defined` —
34 reactivityKeys: [ReactivityKey.orgMembers],34// synchronously, in render, where no Suspense boundary can absorb it.
35 }),35//
36// `withServerValueInitial` gives the atom an explicit server snapshot
37// (`AsyncResult.initial(true)`) that `useAtomValue`'s `getServerSnapshot` reads
38// INSTEAD of evaluating the atom, so the read function never runs server-side.
39// Every reader already handles the initial state (it is the ordinary loading
40// state), and the client refetches on mount. Without this, any SSR'd tree that
41// reads the member list — cloud's shell does, to decide the admin nav — fails
42// the whole document render.
43export const orgMembersAtom = Atom.withServerValueInitial(
44 Atom.refreshOnWindowFocus(
45 AccountApiClient.query("account", "listMembers", {
46 timeToLive: "30 seconds",
47 reactivityKeys: [ReactivityKey.orgMembers],
48 }),
49 ),
36);50);
37 51
38export const orgRolesAtom = AccountApiClient.query("account", "listRoles", {52export const orgRolesAtom = AccountApiClient.query("account", "listRoles", {
packages/react/src/multiplayer/use-admin-nav.tsx+7−0
@@ -19,6 +19,13 @@ import type { ShellNavItem } from "./shell"; · 18 unmodified lines
19// matters. Failing closed here is deliberate: while the member list is loading19// matters. Failing closed here is deliberate: while the member list is loading
20// or unavailable, showing nothing is better than flashing a section that will20// or unavailable, showing nothing is better than flashing a section that will
21// refuse.21// refuse.
22//
23// That fail-closed default is also what makes this hook SSR-safe. `orgMembersAtom`
24// carries an explicit server value (see `api/account-atoms`), so on the server it
25// reads as initial without evaluating — the role is simply UNKNOWN during SSR,
26// which lands on `false` here and resolves client-side once the list arrives.
27// The hook must never suspend or throw: it runs above the routed outlet, so
28// anything it raises takes down the whole server-rendered document.
22// ---------------------------------------------------------------------------29// ---------------------------------------------------------------------------
23 30
24/** Whether the current member reads as an admin/owner of this tenant. */31/** Whether the current member reads as an admin/owner of this tenant. */

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t