mirror of
https://github.com/Luchexx/wacrm.git
synced 2026-11-02 09:19:51 +00:00
fix(auth): load account context without an embedded FK join
getCurrentAccount() and the client AuthProvider resolved the caller's
account through a PostgREST embedded join (`account:accounts!inner(...)`)
on profiles. That embed requires PostgREST to have the
profiles.account_id -> accounts.id relationship in its schema cache.
When the cache is stale -- a common Supabase state right after a
migration adds the FK, or when migrations are applied out of band -- the
embed fails hard with PGRST200 and the whole account context collapses.
It is the only query shape in the app that depends on relationship
inference (everything else reads accounts with plain `.eq('id', ...)`
filters), which is why the dashboard kept working while Settings ->
API keys / Team members returned 403 "Could not load account context"
and the Overview's cards hung loading (issue #294).
Replace the embed with a plain account-by-id lookup in both the server
helper and the client AuthProvider. Same accounts RLS, no relationship
inference, graceful degradation. The client now also keeps account_id /
account_role even if the account-name lookup itself fails. Adds a
regression test for getCurrentAccount.
Fixes #294
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
0779932664
commit
738db87847
@ -131,12 +131,7 @@ export function AuthProvider({ children }: { children: ReactNode }) {
|
||||
const { data, error } = await supabase
|
||||
.from("profiles")
|
||||
.select(
|
||||
// `account:accounts!inner(id, name)` — explicit join on the
|
||||
// single FK profiles.account_id → accounts.id. `!inner` so a
|
||||
// missing account collapses to null rather than a half-
|
||||
// populated row (shouldn't happen post-017 NOT NULL, but
|
||||
// belt-and-braces against forks running older schemas).
|
||||
"id, full_name, email, avatar_url, role, beta_features, account_id, account_role, account:accounts!inner(id, name, default_currency)",
|
||||
"id, full_name, email, avatar_url, role, beta_features, account_id, account_role",
|
||||
)
|
||||
.eq("user_id", userId)
|
||||
.maybeSingle();
|
||||
@ -152,27 +147,40 @@ export function AuthProvider({ children }: { children: ReactNode }) {
|
||||
}
|
||||
|
||||
if (data) {
|
||||
// Supabase's typed client surfaces an embedded `!inner` row
|
||||
// as either an object or a single-element array depending on
|
||||
// the schema's inferred cardinality — normalise to the object
|
||||
// form before reading.
|
||||
const accountRaw = Array.isArray(data.account)
|
||||
? data.account[0] ?? null
|
||||
: (data.account as {
|
||||
id: string;
|
||||
name: string;
|
||||
default_currency: string | null;
|
||||
} | null);
|
||||
// Narrow default_currency defensively: forks running pre-021
|
||||
// schemas won't have the column, so a missing/null value reads
|
||||
// as the safe USD fallback rather than crashing the picker.
|
||||
const accountRow: AccountSummary | null = accountRaw
|
||||
? {
|
||||
id: accountRaw.id,
|
||||
name: accountRaw.name,
|
||||
default_currency: accountRaw.default_currency ?? DEFAULT_CURRENCY,
|
||||
}
|
||||
: null;
|
||||
// Load the account with a plain lookup by id instead of an
|
||||
// embedded FK join. The embed (`account:accounts!inner(...)`)
|
||||
// forces PostgREST to resolve the profiles.account_id →
|
||||
// accounts.id relationship from its schema cache; a stale cache
|
||||
// (common right after a migration adds the FK) makes it fail
|
||||
// hard with PGRST200 and blanks the whole profile — the user
|
||||
// then loses account context everywhere (issue #294). A point
|
||||
// lookup by id needs no relationship inference, so the profile
|
||||
// (with account_id / account_role) still resolves even if the
|
||||
// account name lookup itself can't.
|
||||
let accountRow: AccountSummary | null = null;
|
||||
if (data.account_id) {
|
||||
const { data: account, error: accountErr } = await supabase
|
||||
.from("accounts")
|
||||
// default_currency added in migration 021; narrowed to the
|
||||
// USD fallback below for older schemas where it reads null.
|
||||
.select("id, name, default_currency")
|
||||
.eq("id", data.account_id)
|
||||
.maybeSingle();
|
||||
if (accountErr) {
|
||||
console.error("[AuthProvider] fetchAccount error:", {
|
||||
message: accountErr.message,
|
||||
details: accountErr.details,
|
||||
hint: accountErr.hint,
|
||||
code: accountErr.code,
|
||||
});
|
||||
} else if (account) {
|
||||
accountRow = {
|
||||
id: account.id,
|
||||
name: account.name,
|
||||
default_currency: account.default_currency ?? DEFAULT_CURRENCY,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
// Narrow the DB enum into our AccountRole union. The DB
|
||||
// constraint should make this unconditional, but a future
|
||||
|
||||
176
src/lib/auth/account.test.ts
Normal file
176
src/lib/auth/account.test.ts
Normal file
@ -0,0 +1,176 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
// getCurrentAccount resolves the caller's account context. The
|
||||
// regression this file guards (issue #294): account loading must NOT
|
||||
// depend on a PostgREST embedded FK join (`accounts!inner`), because a
|
||||
// stale schema cache makes that embed fail hard and blanks the whole
|
||||
// context. It must instead read the profile and then the account with
|
||||
// two plain point queries.
|
||||
|
||||
// ------------------------------------------------------------
|
||||
// Chainable Supabase query-builder mock. Each `.from(table)` hands back
|
||||
// a thenable builder pre-loaded with the result queued for that table,
|
||||
// so we can assert which tables were queried and with what filters.
|
||||
// ------------------------------------------------------------
|
||||
interface BuilderCall {
|
||||
table: string;
|
||||
columns?: string;
|
||||
eqArgs: [string, unknown][];
|
||||
}
|
||||
|
||||
function makeClient(opts: {
|
||||
user: { id: string } | null;
|
||||
userErr?: unknown;
|
||||
byTable: Record<string, { data: unknown; error: unknown }>;
|
||||
}) {
|
||||
const calls: BuilderCall[] = [];
|
||||
|
||||
const from = (table: string) => {
|
||||
const call: BuilderCall = { table, eqArgs: [] };
|
||||
calls.push(call);
|
||||
const builder = {
|
||||
select(columns: string) {
|
||||
call.columns = columns;
|
||||
return builder;
|
||||
},
|
||||
eq(col: string, val: unknown) {
|
||||
call.eqArgs.push([col, val]);
|
||||
return builder;
|
||||
},
|
||||
maybeSingle() {
|
||||
return Promise.resolve(
|
||||
opts.byTable[table] ?? { data: null, error: null },
|
||||
);
|
||||
},
|
||||
};
|
||||
return builder;
|
||||
};
|
||||
|
||||
return {
|
||||
calls,
|
||||
client: {
|
||||
auth: {
|
||||
getUser: () =>
|
||||
Promise.resolve({
|
||||
data: { user: opts.user },
|
||||
error: opts.userErr ?? null,
|
||||
}),
|
||||
},
|
||||
from,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
const createClient = vi.fn();
|
||||
vi.mock("@/lib/supabase/server", () => ({
|
||||
createClient: () => createClient(),
|
||||
}));
|
||||
|
||||
const { getCurrentAccount, UnauthorizedError, ForbiddenError } = await import(
|
||||
"./account"
|
||||
);
|
||||
|
||||
afterEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe("getCurrentAccount", () => {
|
||||
it("resolves context via a plain accounts lookup, not an embedded join", async () => {
|
||||
const { client, calls } = makeClient({
|
||||
user: { id: "user-1" },
|
||||
byTable: {
|
||||
profiles: {
|
||||
data: { account_id: "acct-1", account_role: "owner" },
|
||||
error: null,
|
||||
},
|
||||
accounts: { data: { id: "acct-1", name: "Acme" }, error: null },
|
||||
},
|
||||
});
|
||||
createClient.mockReturnValue(client);
|
||||
|
||||
const ctx = await getCurrentAccount();
|
||||
|
||||
expect(ctx).toMatchObject({
|
||||
userId: "user-1",
|
||||
accountId: "acct-1",
|
||||
role: "owner",
|
||||
account: { id: "acct-1", name: "Acme" },
|
||||
});
|
||||
|
||||
// Two queries: profiles by user_id, then accounts by id. Neither
|
||||
// selects an embedded relationship — the regression guard.
|
||||
expect(calls.map((c) => c.table)).toEqual(["profiles", "accounts"]);
|
||||
expect(calls[0].columns).not.toMatch(/accounts!/);
|
||||
expect(calls[0].eqArgs).toEqual([["user_id", "user-1"]]);
|
||||
expect(calls[1].columns).not.toMatch(/accounts!/);
|
||||
expect(calls[1].eqArgs).toEqual([["id", "acct-1"]]);
|
||||
});
|
||||
|
||||
it("throws UnauthorizedError when there is no session", async () => {
|
||||
const { client } = makeClient({ user: null, byTable: {} });
|
||||
createClient.mockReturnValue(client);
|
||||
await expect(getCurrentAccount()).rejects.toBeInstanceOf(UnauthorizedError);
|
||||
});
|
||||
|
||||
it("maps a profiles query error to 'Could not load account context'", async () => {
|
||||
const { client } = makeClient({
|
||||
user: { id: "user-1" },
|
||||
byTable: {
|
||||
profiles: { data: null, error: { code: "PGRST200" } },
|
||||
},
|
||||
});
|
||||
createClient.mockReturnValue(client);
|
||||
await expect(getCurrentAccount()).rejects.toThrow(
|
||||
"Could not load account context",
|
||||
);
|
||||
});
|
||||
|
||||
it("maps an accounts query error to 'Could not load account context'", async () => {
|
||||
// The exact #294 shape if the embed were still in play, but now on
|
||||
// the decoupled accounts lookup: profile resolves, account read errors.
|
||||
const { client } = makeClient({
|
||||
user: { id: "user-1" },
|
||||
byTable: {
|
||||
profiles: {
|
||||
data: { account_id: "acct-1", account_role: "admin" },
|
||||
error: null,
|
||||
},
|
||||
accounts: { data: null, error: { code: "PGRST200" } },
|
||||
},
|
||||
});
|
||||
createClient.mockReturnValue(client);
|
||||
const err = await getCurrentAccount().catch((e) => e);
|
||||
expect(err).toBeInstanceOf(ForbiddenError);
|
||||
expect(err.message).toBe("Could not load account context");
|
||||
});
|
||||
|
||||
it("rejects a profile not linked to an account", async () => {
|
||||
const { client } = makeClient({
|
||||
user: { id: "user-1" },
|
||||
byTable: {
|
||||
profiles: { data: { account_id: null, account_role: null }, error: null },
|
||||
},
|
||||
});
|
||||
createClient.mockReturnValue(client);
|
||||
await expect(getCurrentAccount()).rejects.toThrow(
|
||||
"Profile is not linked to an account",
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects an account_id that resolves to no readable account", async () => {
|
||||
const { client } = makeClient({
|
||||
user: { id: "user-1" },
|
||||
byTable: {
|
||||
profiles: {
|
||||
data: { account_id: "acct-1", account_role: "viewer" },
|
||||
error: null,
|
||||
},
|
||||
accounts: { data: null, error: null },
|
||||
},
|
||||
});
|
||||
createClient.mockReturnValue(client);
|
||||
await expect(getCurrentAccount()).rejects.toThrow(
|
||||
"Profile is not linked to an account",
|
||||
);
|
||||
});
|
||||
});
|
||||
@ -114,14 +114,9 @@ export async function getCurrentAccount(): Promise<AccountContext> {
|
||||
throw new UnauthorizedError();
|
||||
}
|
||||
|
||||
// Selecting through the FK gives us the account name in one
|
||||
// query — `account:accounts!inner(id,name)` is Supabase's
|
||||
// explicit-join syntax. `!inner` so a NULL account_id (which
|
||||
// shouldn't exist) yields no row and trips the guard below
|
||||
// rather than silently returning a half-populated profile.
|
||||
const { data, error } = await supabase
|
||||
.from("profiles")
|
||||
.select("account_id, account_role, account:accounts!inner(id, name)")
|
||||
.select("account_id, account_role")
|
||||
.eq("user_id", user.id)
|
||||
.maybeSingle();
|
||||
|
||||
@ -129,7 +124,7 @@ export async function getCurrentAccount(): Promise<AccountContext> {
|
||||
console.error("[getCurrentAccount] profile fetch error:", error);
|
||||
throw new ForbiddenError("Could not load account context");
|
||||
}
|
||||
if (!data || !data.account_id || !data.account_role || !data.account) {
|
||||
if (!data || !data.account_id || !data.account_role) {
|
||||
// Pre-migration profile, or a manual insert that skipped the
|
||||
// signup trigger. The user is authenticated but the app has
|
||||
// no way to scope their queries — treat as forbidden.
|
||||
@ -142,16 +137,38 @@ export async function getCurrentAccount(): Promise<AccountContext> {
|
||||
throw new ForbiddenError(`Unknown account role: ${data.account_role}`);
|
||||
}
|
||||
|
||||
// Supabase's typed client returns related rows as an array even
|
||||
// for `!inner` single-record joins; normalise to a single object.
|
||||
const accountRow = Array.isArray(data.account) ? data.account[0] : data.account;
|
||||
// Load the account with a plain point lookup by id rather than an
|
||||
// embedded FK join (`account:accounts!inner(...)`). The embed forces
|
||||
// PostgREST to resolve the profiles.account_id → accounts.id
|
||||
// relationship from its schema cache; when that cache is stale — a
|
||||
// common Supabase state right after a migration adds the FK, or when
|
||||
// migrations are applied out of band — the embed fails hard with
|
||||
// PGRST200 ("could not find a relationship … in the schema cache")
|
||||
// and takes down the entire account context (issue #294). A lookup by
|
||||
// id needs no relationship inference and is gated by the same accounts
|
||||
// RLS, so it stays robust against cache staleness and older schemas.
|
||||
const { data: account, error: accountErr } = await supabase
|
||||
.from("accounts")
|
||||
.select("id, name")
|
||||
.eq("id", data.account_id)
|
||||
.maybeSingle();
|
||||
|
||||
if (accountErr) {
|
||||
console.error("[getCurrentAccount] account fetch error:", accountErr);
|
||||
throw new ForbiddenError("Could not load account context");
|
||||
}
|
||||
if (!account) {
|
||||
// account_id points at no readable account row — orphaned profile
|
||||
// or an RLS gap. Same "can't scope this user" outcome as above.
|
||||
throw new ForbiddenError("Profile is not linked to an account");
|
||||
}
|
||||
|
||||
return {
|
||||
supabase,
|
||||
userId: user.id,
|
||||
accountId: data.account_id,
|
||||
role: data.account_role,
|
||||
account: { id: accountRow.id, name: accountRow.name },
|
||||
account: { id: account.id, name: account.name },
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user