diff --git a/src/__tests__/trpc/userWalletsAuth.test.ts b/src/__tests__/trpc/userWalletsAuth.test.ts new file mode 100644 index 00000000..7e79f2ec --- /dev/null +++ b/src/__tests__/trpc/userWalletsAuth.test.ts @@ -0,0 +1,115 @@ +import { beforeAll, beforeEach, describe, expect, it, jest } from "@jest/globals"; + +import { makeAnonymousCtx, makeSessionCtx, makeWalletCtx } from "./helpers"; + +jest.mock("superjson", () => ({ + __esModule: true, + default: { + serialize: (value: unknown) => value, + deserialize: (value: unknown) => value, + }, +})); + +jest.mock("@/server/auth", () => ({ + __esModule: true, + getServerAuthSession: jest.fn(), +})); + +let createCaller: typeof import("@/server/api/root").createCaller; + +const SIGNER = "addr_test1signer"; +const OTHER = "addr_test1other"; + +const walletRow = { + id: "wallet-1", + name: "Treasury", + description: "Board treasury", + signersAddresses: [SIGNER, OTHER], + signersDescriptions: ["Alice", "Bob"], +}; + +const makeMockDb = () => ({ + wallet: { + findMany: jest.fn(async (_args: unknown) => [walletRow]), + }, +}); + +/** + * wallet.getUserWallets lists every wallet that names an address as a signer. + * Signer addresses are public on-chain, so the address alone proves nothing: + * the caller must hold a session for it. + */ +describe("wallet.getUserWallets authorization", () => { + beforeAll(async () => { + ({ createCaller } = await import("@/server/api/root")); + }); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("refuses an anonymous caller without touching the database", async () => { + const db = makeMockDb(); + const caller = createCaller(makeAnonymousCtx(db) as any); + + await expect( + caller.wallet.getUserWallets({ address: SIGNER }), + ).rejects.toMatchObject({ code: "UNAUTHORIZED" }); + + expect(db.wallet.findMany).not.toHaveBeenCalled(); + }); + + it("refuses a wallet session asking for someone else's address", async () => { + const db = makeMockDb(); + const caller = createCaller(makeWalletCtx(OTHER, db) as any); + + await expect( + caller.wallet.getUserWallets({ address: SIGNER }), + ).rejects.toMatchObject({ code: "FORBIDDEN" }); + + expect(db.wallet.findMany).not.toHaveBeenCalled(); + }); + + it("refuses a NextAuth session asking for someone else's address", async () => { + const db = makeMockDb(); + const caller = createCaller(makeSessionCtx(OTHER, db) as any); + + await expect( + caller.wallet.getUserWallets({ address: SIGNER }), + ).rejects.toMatchObject({ code: "FORBIDDEN" }); + + expect(db.wallet.findMany).not.toHaveBeenCalled(); + }); + + it("lists the wallets of the address the wallet session holds", async () => { + const db = makeMockDb(); + const caller = createCaller(makeWalletCtx(SIGNER, db) as any); + + await expect( + caller.wallet.getUserWallets({ address: SIGNER }), + ).resolves.toEqual([walletRow]); + + expect(db.wallet.findMany).toHaveBeenCalledWith({ + where: { signersAddresses: { has: SIGNER } }, + }); + }); + + it("lists the wallets for any address in a multi-wallet session", async () => { + const db = makeMockDb(); + const ctx = { ...makeWalletCtx(OTHER, db), sessionWallets: [OTHER, SIGNER] }; + const caller = createCaller(ctx as any); + + await expect( + caller.wallet.getUserWallets({ address: SIGNER }), + ).resolves.toEqual([walletRow]); + }); + + it("lists the wallets of a NextAuth user's own address", async () => { + const db = makeMockDb(); + const caller = createCaller(makeSessionCtx(SIGNER, db) as any); + + await expect( + caller.wallet.getUserWallets({ address: SIGNER }), + ).resolves.toEqual([walletRow]); + }); +}); diff --git a/src/components/pages/homepage/wallets/index.tsx b/src/components/pages/homepage/wallets/index.tsx index 0be9ab03..1a50051f 100644 --- a/src/components/pages/homepage/wallets/index.tsx +++ b/src/components/pages/homepage/wallets/index.tsx @@ -29,8 +29,32 @@ import { ImportTransferDialog } from "./import-transfer-dialog"; import BotManagementCard from "@/components/pages/user/BotManagementCard"; +/** + * Shown to a connected wallet that has not signed the session nonce yet. The + * wallet list is only served to a session for the address, so without this + * the page would claim the user has no wallets at all. + */ +function AuthorizeToSeeWallets() { + const requestReauth = useUserStore((state) => state.requestReauth); + return ( +
+

+ Authorize your connected wallet to see the multisig wallets it signs + for. +

+ +
+ ); +} + export default function PageWallets() { - const { wallets, isLoading: isLoadingWallets } = useUserWallets(); + const { + wallets, + isLoading: isLoadingWallets, + needsAuthorization, + } = useUserWallets(); const [showArchived, setShowArchived] = useState(false); const userAddress = useUserStore((state) => state.userAddress); const address = userAddress ?? ""; @@ -43,18 +67,17 @@ export default function PageWallets() { refetchOnWindowFocus: false, }, ); + // Both wallet-creation lists are protectedProcedures: without a session for + // this address they can only answer 401/403. const isAuthorized = walletSession?.authorized ?? false; - const sessionWallets = walletSession?.wallets ?? []; - // If there's no wallet-session constraint, public read queries can run without authorization - const canQueryPublicWalletData = sessionWallets.length === 0 || isAuthorized; const { data: newPendingWallets, isLoading: isLoadingNewWallets } = api.wallet.getUserNewWallets.useQuery( { address }, { // Only enable query when user is authorized (prevents 403 errors) - enabled: address.length > 0 && canQueryPublicWalletData, + enabled: address.length > 0 && isAuthorized, retry: (failureCount, error) => { - // Don't retry on authorization errors (403) + // Don't retry on authorization errors (401/403) if (error && typeof error === "object") { const err = error as { code?: string; @@ -65,8 +88,11 @@ export default function PageWallets() { const errorMessage = err.message || err.shape?.message || ""; const isAuthError = err.code === "FORBIDDEN" || + err.code === "UNAUTHORIZED" || err.data?.code === "FORBIDDEN" || + err.data?.code === "UNAUTHORIZED" || err.data?.httpStatus === 403 || + err.data?.httpStatus === 401 || err.shape?.code === "FORBIDDEN" || errorMessage.includes("Address mismatch"); if (isAuthError) return false; @@ -81,9 +107,9 @@ export default function PageWallets() { { address }, { // Only enable query when user is authorized (prevents 403 errors) - enabled: address.length > 0 && canQueryPublicWalletData, + enabled: address.length > 0 && isAuthorized, retry: (failureCount, error) => { - // Don't retry on authorization errors (403) + // Don't retry on authorization errors (401/403) if (error && typeof error === "object") { const err = error as { code?: string; @@ -94,8 +120,11 @@ export default function PageWallets() { const errorMessage = err.message || err.shape?.message || ""; const isAuthError = err.code === "FORBIDDEN" || + err.code === "UNAUTHORIZED" || err.data?.code === "FORBIDDEN" || + err.data?.code === "UNAUTHORIZED" || err.data?.httpStatus === 403 || + err.data?.httpStatus === 401 || err.shape?.code === "FORBIDDEN" || errorMessage.includes("Address mismatch"); if (isAuthError) return false; @@ -147,9 +176,13 @@ export default function PageWallets() { )} - {!isLoadingWallets && address.length > 0 && (!wallets || wallets.length === 0) && ( - + {!isLoadingWallets && needsAuthorization && ( + )} + {!isLoadingWallets && + !needsAuthorization && + address.length > 0 && + (!wallets || wallets.length === 0) && } {!isLoadingWallets && wallets && wallets .filter((wallet) => showArchived || !wallet.isArchived) diff --git a/src/hooks/useUserWallets.ts b/src/hooks/useUserWallets.ts index adde9bc1..e9e01271 100644 --- a/src/hooks/useUserWallets.ts +++ b/src/hooks/useUserWallets.ts @@ -11,27 +11,32 @@ export default function useUserWallets() { const address = userAddress ?? ""; // Check wallet session authorization before enabling queries - const { data: walletSession } = api.auth.getWalletSession.useQuery( - { address }, - { - enabled: address.length > 0, - refetchOnWindowFocus: false, - }, - ); + const { data: walletSession, isError: sessionCheckFailed } = + api.auth.getWalletSession.useQuery( + { address }, + { + enabled: address.length > 0, + refetchOnWindowFocus: false, + }, + ); + // The wallet list is only served to a caller with a session for `address` + // (the server answers 401/403 otherwise), so wait for the authorization + // step instead of firing a request that is bound to fail. The layout + // refetches the session after authorizing, which flips this on. const isAuthorized = walletSession?.authorized ?? false; - const sessionWallets = walletSession?.wallets ?? []; - // If there's no wallet-session constraint, public read queries can run without authorization - const canQueryPublicWalletData = sessionWallets.length === 0 || isAuthorized; - - const { data: wallets, isLoading } = api.wallet.getUserWallets.useQuery( + + const { + data: wallets, + isLoading, + error: walletsError, + } = api.wallet.getUserWallets.useQuery( { address }, { - // Only enable query when user is authorized (prevents 403 errors) - enabled: address.length > 0 && canQueryPublicWalletData, + enabled: address.length > 0 && isAuthorized, staleTime: 1 * 60 * 1000, // 1 minute (user/wallet data) gcTime: 5 * 60 * 1000, // 5 minutes retry: (failureCount, error) => { - // Don't retry on authorization errors (403) + // Don't retry on authorization errors (401/403) if (error && typeof error === "object") { const err = error as { code?: string; @@ -42,8 +47,11 @@ export default function useUserWallets() { const errorMessage = err.message || err.shape?.message || ""; const isAuthError = err.code === "FORBIDDEN" || + err.code === "UNAUTHORIZED" || err.data?.code === "FORBIDDEN" || + err.data?.code === "UNAUTHORIZED" || err.data?.httpStatus === 403 || + err.data?.httpStatus === 401 || err.shape?.code === "FORBIDDEN" || errorMessage.includes("Address mismatch"); if (isAuthError) return false; @@ -61,5 +69,25 @@ export default function useUserWallets() { .map((wallet) => buildWallet(wallet, network)); }, [wallets, network]); - return { wallets: mappedWallets, isLoading }; + // Until the session question is answered the list is effectively loading; + // once it is answered "no", callers should ask the user to authorize rather + // than render "you have no wallets". + // A failed session check is not a "no": the auth modal only opens on an + // answer, so prompting then would offer a button that does nothing. A list + // request refused with 401 is a "no" (the cookie expired since the session + // check was cached). + const isCheckingSession = + address.length > 0 && walletSession === undefined && !sessionCheckFailed; + const listRefused = + walletsError?.data?.code === "UNAUTHORIZED" || + walletsError?.data?.httpStatus === 401; + const needsAuthorization = + address.length > 0 && + ((walletSession !== undefined && !isAuthorized) || listRefused); + + return { + wallets: mappedWallets, + isLoading: isLoading || isCheckingSession, + needsAuthorization, + }; } diff --git a/src/server/api/routers/wallets.ts b/src/server/api/routers/wallets.ts index d3714ae0..ca892148 100644 --- a/src/server/api/routers/wallets.ts +++ b/src/server/api/routers/wallets.ts @@ -223,7 +223,14 @@ function walletDataFromPayload( } export const walletRouter = createTRPCRouter({ - // Read operations stay public but validate signer membership by address param + // Lists every wallet that names `address` as a signer, so the caller must + // have proved control of that address (wallet session or NextAuth user). + // + // This used to skip the check when there was no session at all, which let + // anyone look up any address — signer addresses are public on-chain — and + // read back full wallet rows: names, descriptions, the signers' own labels + // and import bodies. getWallet already refuses anonymous callers; the list + // must not hand out more than the single-wallet read does. getUserWallets: publicProcedure .input(z.object({ address: z.string() })) .query(async ({ ctx, input }) => { @@ -233,9 +240,10 @@ export const walletRouter = createTRPCRouter({ : ctx.sessionAddress ? [ctx.sessionAddress] : []; - // If user has an active session, validate that the requested address is authorized - // Throw error if address doesn't match (security: prevent unauthorized access) - if (addresses.length > 0 && !addresses.includes(input.address)) { + if (addresses.length === 0) { + throw new TRPCError({ code: "UNAUTHORIZED" }); + } + if (!addresses.includes(input.address)) { throw new TRPCError({ code: "FORBIDDEN", message: "Address mismatch" }); } // Query wallets where the user is a signer