Skip to content

feat(data exports): enable user data exports for all users - #5280

Open
St0rmz1 wants to merge 1 commit into
mainfrom
feat/enable-data-exports-for-all-users
Open

feat(data exports): enable user data exports for all users#5280
St0rmz1 wants to merge 1 commit into
mainfrom
feat/enable-data-exports-for-all-users

Conversation

@St0rmz1

@St0rmz1 St0rmz1 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Data exports were restricted to Kilo staff while the feature was being built. This removes the is_admin gate so any signed in user can request and download an export of their own data.

The gate came off in three places: the /data-exports page, the user menu entry that links to it, and all six userExports tRPC procedures. Those procedures move from adminProcedure to baseProcedure, which still requires an authenticated session, so the endpoints are open to signed in users rather than public.

The request throttle also goes back to 24 hours. It had been lowered to 5 minutes for pre-launch testing, with a comment saying to restore it before going live, while the message shown to users already said 24 hours. The code and the copy now agree.

Changes

  • page.tsx: getUserFromAuth({ adminOnly: true }) becomes adminOnly: false. A signed out visitor still gets notFound().
  • SidebarUserFooter.tsx: the user menu entry renders for everyone. The is_admin field is dropped from the component's local User type, since nothing else used it.
  • user-exports-router.ts: request, requestOrganization, exportableOrganizations, list, requestDownloadCode and createDownload move to baseProcedure.
  • user-exports-router.ts: throttle restored from interval '5 minutes' to interval '24 hours', and the TEMPORARY comment removed.
  • user-exports-router.ts: two comments reworded. One justified this router doing its own membership check by stating that every procedure here is adminProcedure, which is no longer true. The other cited a Kilo staff elevation as the reason a requester might see an export whose access the list query cannot reproduce, which can no longer happen, so it now cites a revoked export role. The behavior both comments describe is unchanged.
  • user-exports-router.test.ts: rejects non-admin users becomes allows non-admin users to list their exports, asserting an empty list instead of FORBIDDEN.
  • page.test.ts: new. Covers the page guard, asserting adminOnly: false, that a signed out visitor is refused, and that a signed in non-admin user renders.

What this does not change

  • Organization exports still require an owner or admin role on that organization, checked by requireExportableOrganization and re-checked independently by the export Worker. Kilo staff status does not grant access to another organization's data, and the test covering that still passes.
  • API token and extension callers are still refused by requireWebSession.
  • The emailed download code step is unchanged.

Verification

No manual testing was run. The dev stack for this change is owned by the reviewer, so verification here was automated only: pnpm --filter web test -- data-export user-exports-router passes 164 tests across 14 suites, alongside format:check, lint and typecheck. The boxes below are the manual paths still worth walking.

  • Sign in as a non-admin user, confirm Request data export appears in the user menu and the page loads
  • Request an export as a non-admin user, confirm a second request inside 24 hours is refused
  • Confirm a non-admin user who is not an owner or admin of any organization sees no organization export buttons

Visual Changes

The user menu entry linking to /data-exports is now visible to all signed in users, where it previously rendered only for is_admin accounts. Screenshots to add: the user menu on a non-admin account, before and after.

Before After

Reviewer Notes

  • baseProcedure still requires an authenticated session. createTRPCContext throws UNAUTHORIZED when there is no user, so this widens access from staff to signed in users, not to anonymous callers.
  • Dropping adminProcedure also stops emitAdminAccessEvent firing for these procedures. That is intended, since they are no longer an admin surface, but it does mean these calls no longer appear in the admin access log.
  • The new page test is page.test.ts, not .tsx. Jest's testMatch in apps/web/jest.config.ts is **/src/**/*.test.ts, so a .tsx suite is never collected. A page.test.tsx existed on an earlier branch and was deleted in fix(user-data-export): show expired downloads as disabled with a tooltip #5202 without any failure, because it had never run.

@kilo-code-bot

kilo-code-bot Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The admin-to-base-procedure switch is safe: createTRPCContext already rejects unauthenticated callers, all export authorization remains membership-scoped (requireExportableOrganization / requireDownloadableExport), the 24h throttle restoration matches the user-facing messages, and the new page test's mocks and file extension align with the real modules and jest testMatch.

Files Reviewed (5 files)
  • apps/web/src/app/(app)/components/SidebarUserFooter.tsx
  • apps/web/src/app/(app)/data-exports/page.test.ts
  • apps/web/src/app/(app)/data-exports/page.tsx
  • apps/web/src/routers/user-exports-router.test.ts
  • apps/web/src/routers/user-exports-router.ts

Verified additionally: baseProcedure authentication guarantee in lib/trpc/init.ts, getUserFromAuth semantics in lib/user/server.ts, FOOTER_MENU_ROUTES sync in AppSidebar.tsx, and DataExportsClient only calling the now-public userExports procedures. No memory-leak vectors (subscriptions, timers, listeners, unbounded caches) introduced by the diff.


Reviewed by kimi-k3 · Input: 191.4K · Output: 10K · Cached: 480.8K

Review guidance: REVIEW.md from base branch main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant