From 4be4056dcb201478f239dd6c4fe21c9a6a713cc7 Mon Sep 17 00:00:00 2001 From: St0rmz1 Date: Fri, 14 Aug 2026 11:42:26 -0700 Subject: [PATCH] feat(exports): enable user data exports for all users --- .../(app)/components/SidebarUserFooter.tsx | 11 ++--- .../src/app/(app)/data-exports/page.test.ts | 42 +++++++++++++++++++ apps/web/src/app/(app)/data-exports/page.tsx | 2 +- .../src/routers/user-exports-router.test.ts | 4 +- apps/web/src/routers/user-exports-router.ts | 26 ++++++------ 5 files changed, 61 insertions(+), 24 deletions(-) create mode 100644 apps/web/src/app/(app)/data-exports/page.test.ts diff --git a/apps/web/src/app/(app)/components/SidebarUserFooter.tsx b/apps/web/src/app/(app)/components/SidebarUserFooter.tsx index 3a3482ef45..916b223423 100644 --- a/apps/web/src/app/(app)/components/SidebarUserFooter.tsx +++ b/apps/web/src/app/(app)/components/SidebarUserFooter.tsx @@ -18,7 +18,6 @@ type User = { google_user_name: string; google_user_email: string; google_user_image_url: string; - is_admin: boolean; }; type SidebarUserFooterProps = { @@ -86,12 +85,10 @@ export default function SidebarUserFooter({ user, isLoading }: SidebarUserFooter Connected Accounts - {user.is_admin && ( - router.push('/data-exports')}> - - Request data export - - )} + router.push('/data-exports')}> + + Request data export + router.push('/install')}> Install diff --git a/apps/web/src/app/(app)/data-exports/page.test.ts b/apps/web/src/app/(app)/data-exports/page.test.ts new file mode 100644 index 0000000000..d7af7ce9ca --- /dev/null +++ b/apps/web/src/app/(app)/data-exports/page.test.ts @@ -0,0 +1,42 @@ +import React from 'react'; + +const mockGetUserFromAuth = jest.fn(); +const mockNotFound = jest.fn(() => { + throw new Error('NEXT_NOT_FOUND'); +}); + +(globalThis as typeof globalThis & { React: typeof React }).React = React; + +jest.mock('@/lib/user/server', () => ({ getUserFromAuth: mockGetUserFromAuth })); +jest.mock('next/navigation', () => ({ notFound: mockNotFound })); +jest.mock('@/components/PageLayout', () => ({ PageLayout: () => null })); +jest.mock('./DataExportsClient', () => ({ DataExportsClient: () => null })); +jest.mock('./RequestDataDeletionCard', () => ({ RequestDataDeletionCard: () => null })); + +// `.test.ts`, not `.test.tsx`: jest's testMatch is `**/src/**/*.test.ts`, so a `.tsx` +// suite is silently never collected. An earlier version of this file was a `.tsx` and +// never ran, which is how its deletion went unnoticed. +describe('DataExportsPage', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('authenticates the visitor without requiring Kilo staff', async () => { + mockGetUserFromAuth.mockResolvedValue({ user: null }); + const { default: DataExportsPage } = await import('./page'); + + await expect(DataExportsPage()).rejects.toThrow('NEXT_NOT_FOUND'); + // `adminOnly: false` is the whole guard: signed out is still refused, but a signed-in + // non-staff user reaches the page. Asserted explicitly so a regression to + // `adminOnly: true` fails here rather than silently hiding the page from users. + expect(mockGetUserFromAuth).toHaveBeenCalledWith({ adminOnly: false }); + }); + + it('renders for a signed-in user who is not a Kilo admin', async () => { + mockGetUserFromAuth.mockResolvedValue({ user: { id: 'user-1', is_admin: false } }); + const { default: DataExportsPage } = await import('./page'); + + await expect(DataExportsPage()).resolves.toBeTruthy(); + expect(mockNotFound).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/src/app/(app)/data-exports/page.tsx b/apps/web/src/app/(app)/data-exports/page.tsx index 06c0aef5fe..9b192337c0 100644 --- a/apps/web/src/app/(app)/data-exports/page.tsx +++ b/apps/web/src/app/(app)/data-exports/page.tsx @@ -5,7 +5,7 @@ import { DataExportsClient } from './DataExportsClient'; import { RequestDataDeletionCard } from './RequestDataDeletionCard'; export default async function DataExportsPage() { - const { user } = await getUserFromAuth({ adminOnly: true }); + const { user } = await getUserFromAuth({ adminOnly: false }); if (!user) notFound(); return ( diff --git a/apps/web/src/routers/user-exports-router.test.ts b/apps/web/src/routers/user-exports-router.test.ts index 71e9c28b06..7b3997fa34 100644 --- a/apps/web/src/routers/user-exports-router.test.ts +++ b/apps/web/src/routers/user-exports-router.test.ts @@ -129,9 +129,9 @@ describe('user exports router guards and serialization', () => { expect(() => __test__.requireWebSession(false)).not.toThrow(); }); - it('rejects non-admin users', async () => { + it('allows non-admin users to list their exports', async () => { const caller = await createCallerForUser(stranger.id); - await expect(caller.userExports.list()).rejects.toMatchObject({ code: 'FORBIDDEN' }); + await expect(caller.userExports.list()).resolves.toEqual({ exports: [], nextCursor: null }); }); it('normalizes database timestamp text into strict UTC ISO strings', () => { diff --git a/apps/web/src/routers/user-exports-router.ts b/apps/web/src/routers/user-exports-router.ts index 7e3a05cadb..88b079127b 100644 --- a/apps/web/src/routers/user-exports-router.ts +++ b/apps/web/src/routers/user-exports-router.ts @@ -17,7 +17,7 @@ import { } from '@/lib/auth/data-export-download-codes'; import { db } from '@/lib/drizzle'; import { sendDataExportDownloadCodeEmail } from '@/lib/email'; -import { adminProcedure, createTRPCRouter, type TRPCContext } from '@/lib/trpc/init'; +import { baseProcedure, createTRPCRouter, type TRPCContext } from '@/lib/trpc/init'; import { ORGANIZATION_EXPORT_ROLES, organizationExportAccess, @@ -165,10 +165,9 @@ async function exportableOrganizations(userId: string): Promise<{ id: string; na * role on it. * * The shared predicate, not `ensureOrganizationAccess`, which every other organization - * router uses. That helper grants `owner` to any `is_admin` caller, so on this router — - * where every procedure is `adminProcedure` — it would authorise on staff status rather - * than on membership, and nobody may export another person's or another organization's - * data. + * router uses. That helper grants `owner` to any `is_admin` caller, so it would authorise + * a Kilo staff member on staff status rather than on membership, and nobody — staff + * included — may export another person's or another organization's data. * * Shared with the Worker, which re-checks independently, because the two must reach the * same verdict. They once did not, and an export generated, showed as ready, and then @@ -246,8 +245,7 @@ async function createExportRequest(subject: { FROM user_data_exports exports WHERE ${scopeFilter} AND exports.status <> 'failed' - -- TEMPORARY: throttle lowered to 5 minutes for pre-launch testing; restore to 24 hours before going live. - AND exports.requested_at > now() - interval '5 minutes' + AND exports.requested_at > now() - interval '24 hours' ORDER BY exports.requested_at DESC LIMIT 1 `); @@ -421,13 +419,13 @@ function downloadCodeError(result: Exclude): TR } export const userExportsRouter = createTRPCRouter({ - request: adminProcedure.mutation(async ({ ctx }) => { + request: baseProcedure.mutation(async ({ ctx }) => { requireWebSession(ctx.authViaToken); const row = await createExportRequest({ kiloUserId: ctx.user.id, organizationId: null }); return serialize(row); }), - requestOrganization: adminProcedure + requestOrganization: baseProcedure .input(OrganizationExportSchema) .mutation(async ({ ctx, input }) => { requireWebSession(ctx.authViaToken); @@ -443,18 +441,18 @@ export const userExportsRouter = createTRPCRouter({ }), /** Organizations the caller may export, for rendering the organization buttons. */ - exportableOrganizations: adminProcedure.query(async ({ ctx }) => { + exportableOrganizations: baseProcedure.query(async ({ ctx }) => { requireWebSession(ctx.authViaToken); return { organizations: await exportableOrganizations(ctx.user.id) }; }), - list: adminProcedure.input(ListInputSchema).query(async ({ ctx, input }) => { + list: baseProcedure.input(ListInputSchema).query(async ({ ctx, input }) => { requireWebSession(ctx.authViaToken); // Two independent reasons an export is visible. // // Anything the caller requested, whatever its subject. Whoever pressed the button // always sees the result, even if the access that admitted the request is not - // access this query can reproduce — a Kilo staff elevation, for instance. + // access this query can still reproduce — an export role revoked afterwards, say. // // Plus every export belonging to an organization they may export, so a second // admin is not refused by the one-active-export-per-org constraint while being @@ -508,7 +506,7 @@ export const userExportsRouter = createTRPCRouter({ * held web session alone cannot reach the artifact without also reaching the * inbox. */ - requestDownloadCode: adminProcedure.input(ExportIdSchema).mutation(async ({ ctx, input }) => { + requestDownloadCode: baseProcedure.input(ExportIdSchema).mutation(async ({ ctx, input }) => { requireWebSession(ctx.authViaToken); await requireDownloadableExport(ctx, input.exportId); const email = requireDownloadCodeRecipient(ctx.user.google_user_email); @@ -539,7 +537,7 @@ export const userExportsRouter = createTRPCRouter({ }), /** Step 2 of the download: redeem the emailed code for one signed URL. */ - createDownload: adminProcedure.input(DownloadInputSchema).mutation(async ({ ctx, input }) => { + createDownload: baseProcedure.input(DownloadInputSchema).mutation(async ({ ctx, input }) => { requireWebSession(ctx.authViaToken); await requireDownloadableExport(ctx, input.exportId); const email = requireDownloadCodeRecipient(ctx.user.google_user_email);