From 118c3a225ee400208822c4123beb907a5c8c8b6f Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Wed, 3 Jun 2026 05:36:34 -0500 Subject: [PATCH 1/4] fix(onboarding): restore token-gated cloud bootstrap --- .../api/cloud/__tests__/bootstrap.test.ts | 185 +++++++++++++++++ apps/web/src/routes/api/cloud/bootstrap.ts | 186 ++++++++++++++++++ 2 files changed, 371 insertions(+) create mode 100644 apps/web/src/routes/api/cloud/__tests__/bootstrap.test.ts create mode 100644 apps/web/src/routes/api/cloud/bootstrap.ts diff --git a/apps/web/src/routes/api/cloud/__tests__/bootstrap.test.ts b/apps/web/src/routes/api/cloud/__tests__/bootstrap.test.ts new file mode 100644 index 000000000..b76fd030c --- /dev/null +++ b/apps/web/src/routes/api/cloud/__tests__/bootstrap.test.ts @@ -0,0 +1,185 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' + +const hoisted = vi.hoisted(() => { + const updateWhere = vi.fn().mockResolvedValue(undefined) + return { + insertValues: vi.fn().mockResolvedValue(undefined), + updateWhere, + updateSet: vi.fn(() => ({ where: updateWhere })), + } +}) + +vi.mock('@/lib/server/db', () => ({ + db: { + query: { + principal: { findFirst: vi.fn() }, + settings: { findFirst: vi.fn() }, + postStatuses: { findFirst: vi.fn() }, + }, + insert: vi.fn(() => ({ values: hoisted.insertValues })), + update: vi.fn(() => ({ set: hoisted.updateSet })), + }, + settings: { id: 'settings.id' }, + principal: { role: 'principal.role', userId: 'principal.userId', type: 'principal.type' }, + postStatuses: {}, + eq: vi.fn((left, right) => ({ left, right })), + and: vi.fn((...conditions) => ({ and: conditions })), + DEFAULT_STATUSES: [], +})) + +vi.mock('@/lib/server/auth', () => ({ + getAuth: vi.fn(), +})) + +vi.mock('@/lib/server/auth/magic-link-mint', () => ({ + mintMagicLinkUrl: vi.fn(), +})) + +vi.mock('@/lib/server/domains/settings/settings.helpers', () => ({ + invalidateSettingsCache: vi.fn().mockResolvedValue(undefined), +})) + +vi.mock('@/lib/server/domains/settings', () => ({ + DEFAULT_AUTH_CONFIG: { oauth: { google: true, github: true, password: true }, openSignup: false }, + DEFAULT_PORTAL_CONFIG: { oauth: { password: true, magicLink: true }, features: {} }, +})) + +vi.mock('@/lib/shared/utils', () => ({ + slugify: (value: string) => + value + .toLowerCase() + .replace(/\s+/g, '-') + .replace(/[^a-z0-9-]/g, ''), +})) + +vi.mock('@opencoven-feedback/ids', () => ({ + generateId: (prefix: string) => `${prefix}_test`, +})) + +import { getAuth } from '@/lib/server/auth' +import { mintMagicLinkUrl } from '@/lib/server/auth/magic-link-mint' +import { db } from '@/lib/server/db' +import { handleCloudBootstrap } from '../bootstrap' + +const dbMock = db as unknown as { + query: { + principal: { findFirst: ReturnType } + settings: { findFirst: ReturnType } + postStatuses: { findFirst: ReturnType } + } + insert: ReturnType + update: ReturnType +} + +function makeReq(opts: { body?: unknown; authHeader?: string | null } = {}): Request { + const headers: Record = { 'Content-Type': 'application/json' } + if (opts.authHeader !== null) headers.Authorization = opts.authHeader ?? 'Bearer test-token' + + return new Request('http://acme.example.com/api/cloud/bootstrap', { + method: 'POST', + headers, + body: + opts.body === undefined + ? JSON.stringify({ email: 'Founder@Acme.com', workspaceName: 'Acme Feedback' }) + : typeof opts.body === 'string' + ? opts.body + : JSON.stringify(opts.body), + }) +} + +describe('POST /api/cloud/bootstrap', () => { + beforeEach(() => { + vi.clearAllMocks() + process.env.CLOUD_BOOTSTRAP_TOKEN = 'test-token' + dbMock.query.principal.findFirst.mockResolvedValue(undefined) + dbMock.query.settings.findFirst.mockResolvedValue(undefined) + dbMock.query.postStatuses.findFirst.mockResolvedValue(undefined) + vi.mocked(getAuth).mockResolvedValue({ + api: { signUpEmail: vi.fn().mockResolvedValue({ user: { id: 'user_admin' } }) }, + } as never) + vi.mocked(mintMagicLinkUrl).mockResolvedValue('https://acme.example.com/verify?token=claim') + }) + + afterEach(() => { + delete process.env.CLOUD_BOOTSTRAP_TOKEN + }) + + it('404s when CLOUD_BOOTSTRAP_TOKEN is unset', async () => { + delete process.env.CLOUD_BOOTSTRAP_TOKEN + const res = await handleCloudBootstrap({ request: makeReq() }) + expect(res.status).toBe(404) + }) + + it('401s when bearer token is missing or incorrect', async () => { + await expect( + handleCloudBootstrap({ request: makeReq({ authHeader: null }) }) + ).resolves.toHaveProperty('status', 401) + await expect( + handleCloudBootstrap({ request: makeReq({ authHeader: 'Bearer wrong-token' }) }) + ).resolves.toHaveProperty('status', 401) + }) + + it('400s when required fields are missing', async () => { + const res = await handleCloudBootstrap({ + request: makeReq({ body: { email: 'founder@acme.com' } }), + }) + expect(res.status).toBe(400) + }) + + it('409s when a different admin already exists', async () => { + dbMock.query.principal.findFirst.mockResolvedValueOnce({ + role: 'admin', + user: { id: 'user_other', email: 'other@example.com' }, + }) + + const res = await handleCloudBootstrap({ request: makeReq() }) + + expect(res.status).toBe(409) + expect(vi.mocked(getAuth)).not.toHaveBeenCalled() + expect(vi.mocked(mintMagicLinkUrl)).not.toHaveBeenCalled() + }) + + it('creates the intended admin and returns a bounded claim URL', async () => { + const signUpEmail = vi.fn().mockResolvedValue({ user: { id: 'user_admin' } }) + vi.mocked(getAuth).mockResolvedValue({ api: { signUpEmail } } as never) + + const res = await handleCloudBootstrap({ request: makeReq() }) + + expect(res.status).toBe(200) + await expect(res.json()).resolves.toEqual({ + claimUrl: 'https://acme.example.com/verify?token=claim', + expiresInDays: 7, + userId: 'user_admin', + }) + expect(signUpEmail).toHaveBeenCalledWith({ + body: expect.objectContaining({ + email: 'founder@acme.com', + name: 'Acme Feedback', + password: expect.any(String), + }), + headers: expect.any(Headers), + }) + expect(vi.mocked(mintMagicLinkUrl)).toHaveBeenCalledWith({ + email: 'founder@acme.com', + portalUrl: 'https://acme.example.com', + callbackPath: '/admin/feedback', + errorCallbackPath: '/admin/login', + expiresInSeconds: 604800, + }) + expect(hoisted.updateSet).toHaveBeenCalledWith({ role: 'admin' }) + }) + + it('is idempotent for the same admin email', async () => { + dbMock.query.principal.findFirst.mockResolvedValueOnce({ + role: 'admin', + user: { id: 'user_existing', email: 'founder@acme.com' }, + }) + + const res = await handleCloudBootstrap({ request: makeReq() }) + + expect(res.status).toBe(200) + await expect(res.json()).resolves.toMatchObject({ userId: 'user_existing' }) + expect(vi.mocked(getAuth)).toHaveBeenCalledOnce() + expect(vi.mocked(mintMagicLinkUrl)).toHaveBeenCalledOnce() + }) +}) diff --git a/apps/web/src/routes/api/cloud/bootstrap.ts b/apps/web/src/routes/api/cloud/bootstrap.ts new file mode 100644 index 000000000..fdc015c36 --- /dev/null +++ b/apps/web/src/routes/api/cloud/bootstrap.ts @@ -0,0 +1,186 @@ +import { createFileRoute } from '@tanstack/react-router' +import type { StatusId } from '@opencoven-feedback/ids' +import { generateId } from '@opencoven-feedback/ids' +import { db, settings, principal, postStatuses, eq, and, DEFAULT_STATUSES } from '@/lib/server/db' +import type { SetupState } from '@/lib/server/db' +import { getAuth } from '@/lib/server/auth' +import { mintMagicLinkUrl } from '@/lib/server/auth/magic-link-mint' +import { invalidateSettingsCache } from '@/lib/server/domains/settings/settings.helpers' +import { DEFAULT_AUTH_CONFIG, DEFAULT_PORTAL_CONFIG } from '@/lib/server/domains/settings' +import { slugify } from '@/lib/shared/utils' + +const CLAIM_EXPIRES_IN_DAYS = 7 +const CLAIM_EXPIRES_IN_SECONDS = CLAIM_EXPIRES_IN_DAYS * 24 * 60 * 60 + +/** + * Control-plane bootstrap for externally provisioned tenants. + * + * Self-hosted instances do not set CLOUD_BOOTSTRAP_TOKEN, so this + * endpoint returns 404 there. Managed/external provisioners can set a + * per-tenant token and call this once with the intended owner email to + * bind first-admin claim to that email instead of leaving the public + * onboarding form as first-user-wins. + */ +export async function handleCloudBootstrap({ request }: { request: Request }): Promise { + const expected = process.env.CLOUD_BOOTSTRAP_TOKEN + if (!expected) return new Response('Not Found', { status: 404 }) + + const provided = request.headers.get('authorization') + if (provided !== `Bearer ${expected}`) { + return Response.json({ error: 'Unauthorized' }, { status: 401 }) + } + + let body: unknown + try { + body = await request.json() + } catch { + return Response.json({ error: 'Invalid JSON body' }, { status: 400 }) + } + + const parsed = parseBody(body) + if (!parsed) { + return Response.json( + { error: 'Missing required fields: email, workspaceName' }, + { status: 400 } + ) + } + + const { email, workspaceName } = parsed + const slug = slugify(workspaceName) + if (slug.length < 2) { + return Response.json( + { error: 'workspaceName must produce a slug of at least 2 characters' }, + { status: 400 } + ) + } + + const adminUserId = await ensureAdminUser({ email, workspaceName, request }) + if (adminUserId === 'CONFLICT') { + return Response.json({ error: 'A different admin is already configured' }, { status: 409 }) + } + + await Promise.all([ensureCompleteSettings(workspaceName, slug), ensureDefaultStatuses()]) + await invalidateSettingsCache() + + const claimUrl = await mintMagicLinkUrl({ + email, + portalUrl: workspacePortalUrl(request), + callbackPath: '/admin/feedback', + errorCallbackPath: '/admin/login', + expiresInSeconds: CLAIM_EXPIRES_IN_SECONDS, + }) + + return Response.json({ claimUrl, expiresInDays: CLAIM_EXPIRES_IN_DAYS, userId: adminUserId }) +} + +interface ParsedBody { + email: string + workspaceName: string +} + +function parseBody(body: unknown): ParsedBody | null { + if (!body || typeof body !== 'object') return null + const input = body as Record + const email = typeof input.email === 'string' ? input.email.trim().toLowerCase() : '' + const workspaceName = typeof input.workspaceName === 'string' ? input.workspaceName.trim() : '' + if (!email || !workspaceName) return null + return { email, workspaceName } +} + +async function ensureAdminUser({ + email, + workspaceName, + request, +}: { + email: string + workspaceName: string + request: Request +}): Promise { + const existingAdmin = await db.query.principal.findFirst({ + where: and(eq(principal.role, 'admin'), eq(principal.type, 'user')), + with: { user: { columns: { email: true, id: true } } }, + }) + + if (existingAdmin) { + if (existingAdmin.user?.email?.toLowerCase() === email) return existingAdmin.user.id + return 'CONFLICT' + } + + const auth = await getAuth() + const throwawayPassword = `${crypto.randomUUID()}${crypto.randomUUID()}` + const signedUp = await auth.api.signUpEmail({ + body: { email, name: workspaceName, password: throwawayPassword }, + headers: request.headers, + }) + + await db + .update(principal) + .set({ role: 'admin' }) + .where(eq(principal.userId, signedUp.user.id as never)) + + return signedUp.user.id +} + +async function ensureCompleteSettings(workspaceName: string, slug: string): Promise { + const existing = await db.query.settings.findFirst() + const completeSetup: SetupState = { + version: 1, + steps: { core: true, workspace: true, boards: true }, + completedAt: new Date().toISOString(), + } + const portalConfigDefault = JSON.stringify(DEFAULT_PORTAL_CONFIG) + const authConfigDefault = JSON.stringify({ + ...DEFAULT_AUTH_CONFIG, + oauth: { ...DEFAULT_AUTH_CONFIG.oauth, password: false, magicLink: true }, + openSignup: false, + }) + + if (existing) { + await db + .update(settings) + .set({ + name: workspaceName, + slug, + setupState: JSON.stringify(completeSetup), + portalConfig: existing.portalConfig ?? portalConfigDefault, + authConfig: existing.authConfig ?? authConfigDefault, + }) + .where(eq(settings.id, existing.id)) + return + } + + await db.insert(settings).values({ + id: generateId('workspace'), + name: workspaceName, + slug, + createdAt: new Date(), + setupState: JSON.stringify(completeSetup), + portalConfig: portalConfigDefault, + authConfig: authConfigDefault, + }) +} + +async function ensureDefaultStatuses(): Promise { + const existing = await db.query.postStatuses.findFirst() + if (existing) return + await db.insert(postStatuses).values( + DEFAULT_STATUSES.map((status) => ({ + id: generateId('status') as StatusId, + ...status, + createdAt: new Date(), + })) + ) +} + +function workspacePortalUrl(request: Request): string { + const url = new URL(request.url) + return `https://${url.host}` +} + +export const Route = createFileRoute('/api/cloud/bootstrap')({ + server: { + handlers: { + POST: handleCloudBootstrap, + }, + }, +}) From d4d3747ca1bad4437ce1b72597d361529ae2aea5 Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Wed, 3 Jun 2026 06:51:56 -0500 Subject: [PATCH 2/4] fix(onboarding): guard admin bootstrap --- .../functions/__tests__/onboarding.test.ts | 219 ++++++++++++++++++ .../src/lib/server/functions/onboarding.ts | 60 +++-- 2 files changed, 248 insertions(+), 31 deletions(-) create mode 100644 apps/web/src/lib/server/functions/__tests__/onboarding.test.ts diff --git a/apps/web/src/lib/server/functions/__tests__/onboarding.test.ts b/apps/web/src/lib/server/functions/__tests__/onboarding.test.ts new file mode 100644 index 000000000..ea8310d49 --- /dev/null +++ b/apps/web/src/lib/server/functions/__tests__/onboarding.test.ts @@ -0,0 +1,219 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { state, principalTable, settingsTable, postStatusesTable, userTable } = vi.hoisted(() => ({ + state: { + sessionUserId: 'user_attacker', + currentSettings: null as null | { + id: string + name: string + slug: string + setupState: string + portalConfig?: string | null + authConfig?: string | null + managedFieldPaths?: string[] | null + }, + principals: [] as Array<{ + id: string + userId: string + role: string + type: string + createdAt?: Date + }>, + inserts: [] as Array<{ table: string; values: unknown }>, + updates: [] as Array<{ table: string; values: Record; where: unknown }>, + statusesExist: true, + }, + principalTable: { + userId: 'principal.userId', + role: 'principal.role', + type: 'principal.type', + tableName: 'principal', + }, + settingsTable: { id: 'settings.id', tableName: 'settings' }, + postStatusesTable: { tableName: 'postStatuses' }, + userTable: { id: 'user.id', tableName: 'user' }, +})) + +vi.mock('@tanstack/react-start', () => ({ + createServerFn: () => { + const chain = { + inputValidator: () => chain, + handler: (fn: unknown) => fn, + } + return chain + }, +})) + +vi.mock('@opencoven-feedback/ids', () => ({ + generateId: (prefix: string) => `${prefix}_generated`, +})) + +vi.mock('@/lib/server/auth/session', () => ({ + getSession: async () => ({ user: { id: state.sessionUserId } }), +})) + +vi.mock('@/lib/server/functions/workspace', () => ({ + getSettings: async () => state.currentSettings, +})) + +vi.mock('@/lib/server/config-file/managed-guard', () => ({ + assertNotManaged: async () => undefined, +})) + +vi.mock('@/lib/server/config-file/managed-paths', () => ({ + isPathManaged: () => false, +})) + +vi.mock('@/lib/server/domains/settings/settings.helpers', () => ({ + invalidateSettingsCache: async () => undefined, +})) + +vi.mock('@/lib/server/domains/settings', () => ({ + DEFAULT_AUTH_CONFIG: { openSignup: false }, + DEFAULT_PORTAL_CONFIG: { oauth: {}, features: {} }, +})) + +vi.mock('@/lib/server/domains/principals/principal.service', () => ({ + syncPrincipalProfile: async () => undefined, +})) + +vi.mock('@/lib/server/domains/boards/board.service', () => ({ + listBoards: async () => [], +})) + +function matchesWhere(row: Record, where: unknown): boolean { + if (!where || typeof where !== 'object') return false + const clause = where as { op?: string; col?: string; value?: unknown; conditions?: unknown[] } + if (clause.op === 'and') { + return clause.conditions?.every((condition) => matchesWhere(row, condition)) ?? false + } + if (clause.col === principalTable.userId) return row.userId === clause.value + if (clause.col === principalTable.role) return row.role === clause.value + if (clause.col === principalTable.type) return row.type === clause.value + if (clause.col === settingsTable.id) return row.id === clause.value + return false +} + +vi.mock('@/lib/server/db', () => ({ + USE_CASE_TYPES: ['feedback', 'roadmap', 'changelog'], + DEFAULT_STATUSES: [], + principal: principalTable, + settings: settingsTable, + postStatuses: postStatusesTable, + user: userTable, + eq: (col: string, value: unknown) => ({ op: 'eq', col, value }), + and: (...conditions: unknown[]) => ({ op: 'and', conditions }), + db: { + query: { + principal: { + findFirst: async ({ where }: { where: unknown }) => + state.principals.find((row) => matchesWhere(row, where)) ?? null, + }, + postStatuses: { + findFirst: async () => (state.statusesExist ? { id: 'status_existing' } : null), + }, + }, + insert: (table: { tableName?: string }) => ({ + values: (values: unknown) => { + const tableName = table.tableName ?? 'principal' + state.inserts.push({ table: tableName, values }) + if (table === principalTable) { + state.principals.push({ + ...(values as { id: string; userId: string; role: string }), + type: 'user', + }) + } + return { + returning: async () => { + if (table === settingsTable) { + state.currentSettings = values as NonNullable + return [state.currentSettings] + } + return [values] + }, + } + }, + }), + update: (table: { tableName?: string }) => ({ + set: (values: Record) => ({ + where: (where: unknown) => { + const tableName = table.tableName ?? 'principal' + state.updates.push({ table: tableName, values, where }) + if (table === principalTable) { + const row = state.principals.find((principal) => matchesWhere(principal, where)) + if (row) Object.assign(row, values) + } + if (table === settingsTable && state.currentSettings) { + state.currentSettings = { ...state.currentSettings, ...values } + } + return { + returning: async () => [state.currentSettings], + } + }, + }), + }), + }, +})) + +beforeEach(() => { + state.sessionUserId = 'user_attacker' + state.currentSettings = { + id: 'settings_existing', + name: 'Existing', + slug: 'existing', + setupState: JSON.stringify({ + version: 1, + steps: { core: true, workspace: false, boards: false }, + }), + portalConfig: null, + authConfig: null, + managedFieldPaths: null, + } + state.principals = [ + { id: 'principal_owner', userId: 'user_owner', role: 'admin', type: 'user' }, + { id: 'principal_attacker', userId: 'user_attacker', role: 'user', type: 'user' }, + ] + state.inserts = [] + state.updates = [] + state.statusesExist = true +}) + +describe('onboarding authorization', () => { + it('does not promote a non-admin caller during an existing mid-onboarding setup', async () => { + const { setupWorkspaceFn } = await import('../onboarding') + + await expect( + setupWorkspaceFn({ data: { workspaceName: 'Pwned Workspace', useCase: 'feedback' } }) + ).rejects.toThrow('Only admin can complete setup') + + expect(state.principals.find((row) => row.userId === 'user_attacker')?.role).toBe('user') + expect(state.inserts).toEqual([]) + expect(state.updates).toEqual([]) + }) + + it('does not let saveUseCaseFn mutate settings when another human admin exists', async () => { + const { saveUseCaseFn } = await import('../onboarding') + + await expect(saveUseCaseFn({ data: { useCase: 'roadmap' } })).rejects.toThrow( + 'Only admin can complete setup' + ) + + expect(JSON.parse(state.currentSettings!.setupState).useCase).toBeUndefined() + expect(state.principals.find((row) => row.userId === 'user_attacker')?.role).toBe('user') + expect(state.inserts).toEqual([]) + expect(state.updates).toEqual([]) + }) + + it('still bootstraps the first authenticated human user when no admin exists', async () => { + state.principals = [ + { id: 'principal_first', userId: 'user_attacker', role: 'user', type: 'user' }, + ] + + const { saveUseCaseFn } = await import('../onboarding') + + await saveUseCaseFn({ data: { useCase: 'feedback' } }) + + expect(state.principals.find((row) => row.userId === 'user_attacker')?.role).toBe('admin') + expect(JSON.parse(state.currentSettings!.setupState).useCase).toBe('feedback') + }) +}) diff --git a/apps/web/src/lib/server/functions/onboarding.ts b/apps/web/src/lib/server/functions/onboarding.ts index cb8a0ad51..be6b502dc 100644 --- a/apps/web/src/lib/server/functions/onboarding.ts +++ b/apps/web/src/lib/server/functions/onboarding.ts @@ -8,7 +8,7 @@ import { getSession } from '@/lib/server/auth/session' import { getSettings } from './workspace' import { syncPrincipalProfile } from '@/lib/server/domains/principals/principal.service' import { listBoards } from '@/lib/server/domains/boards/board.service' -import { db, settings, principal, user, postStatuses, eq, DEFAULT_STATUSES } from '@/lib/server/db' +import { db, settings, principal, user, postStatuses, and, eq, DEFAULT_STATUSES } from '@/lib/server/db' import { invalidateSettingsCache } from '@/lib/server/domains/settings/settings.helpers' import { DEFAULT_AUTH_CONFIG, DEFAULT_PORTAL_CONFIG } from '@/lib/server/domains/settings' import { assertNotManaged } from '@/lib/server/config-file/managed-guard' @@ -16,23 +16,34 @@ import { isPathManaged } from '@/lib/server/config-file/managed-paths' import { slugify } from '@/lib/shared/utils' import { getSetupState } from '@/lib/shared/db-types' -/** Onboarding promotes the acting user to admin in two server fns - * (saveUseCaseFn, setupWorkspaceFn). Same DB shape, same intent — - * insert when missing, upgrade when present-but-not-admin. */ -async function ensureAdminPrincipal(userId: UserId, logLabel: string): Promise { +async function ensureOnboardingAdminPrincipal(userId: UserId, logLabel: string): Promise { const existing = await db.query.principal.findFirst({ where: eq(principal.userId, userId), }) + + if (existing && isAdmin(existing.role)) { + return + } + + const existingAdmin = await db.query.principal.findFirst({ + where: and(eq(principal.role, 'admin'), eq(principal.type, 'user')), + columns: { id: true }, + }) + + if (existingAdmin) { + throw new Error('Only admin can complete setup') + } + if (!existing) { - console.log(`[fn:onboarding] ${logLabel}: creating admin member for user`) + console.log(`[fn:onboarding] ${logLabel}: creating admin principal for first onboarding user`) await db.insert(principal).values({ id: generateId('principal'), userId, role: 'admin', createdAt: new Date(), }) - } else if (!isAdmin(existing.role)) { - console.log(`[fn:onboarding] ${logLabel}: upgrading user to admin`) + } else { + console.log(`[fn:onboarding] ${logLabel}: upgrading first onboarding user to admin`) await db.update(principal).set({ role: 'admin' }).where(eq(principal.userId, userId)) } } @@ -77,10 +88,11 @@ export interface SetupWorkspaceResult { /** * Setup workspace during onboarding. * Creates settings and default statuses. - * Requires authentication. For fresh installs (no settings), makes the user admin. + * Requires authentication. For fresh installs (no settings and no human admin), + * bootstraps the first user as admin. * * NOTE: Cannot use requireAuth() here because it requires settings to exist, - * but we're creating settings. We manually check auth and handle member creation. + * but we're creating settings. We manually check auth and handle principal creation. */ export const setupWorkspaceFn = createServerFn({ method: 'POST' }) .inputValidator(setupWorkspaceSchema) @@ -117,21 +129,9 @@ export const setupWorkspaceFn = createServerFn({ method: 'POST' }) let setupState: SetupState | null = getSetupState(existingSettings?.setupState ?? null) - // Fresh install (no settings): first authenticated user becomes admin. - // Settings exist + workspace step done: require existing admin. - // Settings exist + workspace step not done: ensure user becomes admin. - if (!existingSettings) { - await ensureAdminPrincipal(session.user.id as UserId, 'setupWorkspaceFn') - } else if (setupState?.steps?.workspace) { - const principalRecord = await db.query.principal.findFirst({ - where: eq(principal.userId, session.user.id as UserId), - }) - if (!principalRecord || !isAdmin(principalRecord.role)) { - throw new Error('Only admin can complete setup') - } - } else { - await ensureAdminPrincipal(session.user.id as UserId, 'setupWorkspaceFn') - } + // First authenticated user may bootstrap an install, but once a human admin + // exists, onboarding mutations require the acting user to already be admin. + await ensureOnboardingAdminPrincipal(session.user.id as UserId, 'setupWorkspaceFn') // Check if onboarding is already complete if (setupState?.steps?.core && setupState?.steps?.workspace && setupState?.steps?.boards) { @@ -345,6 +345,8 @@ export const saveUseCaseFn = createServerFn({ method: 'POST' }) steps: { core: true, workspace: false, boards: false }, } + await ensureOnboardingAdminPrincipal(session.user.id as UserId, 'saveUseCaseFn') + const updatedState: SetupState = { ...setupState, useCase: data.useCase } await db @@ -352,10 +354,6 @@ export const saveUseCaseFn = createServerFn({ method: 'POST' }) .set({ setupState: JSON.stringify(updatedState) }) .where(eq(settings.id, existingSettings.id)) - if (!setupState.steps.workspace) { - await ensureAdminPrincipal(session.user.id as UserId, 'saveUseCaseFn') - } - await invalidateSettingsCache() console.log(`[fn:onboarding] saveUseCaseFn: saved useCase=${data.useCase}`) } else { @@ -372,6 +370,8 @@ export const saveUseCaseFn = createServerFn({ method: 'POST' }) useCase: data.useCase, } + await ensureOnboardingAdminPrincipal(session.user.id as UserId, 'saveUseCaseFn') + await db.insert(settings).values({ id: generateId('workspace'), name: 'My Workspace', // Placeholder, will be updated in workspace step @@ -380,8 +380,6 @@ export const saveUseCaseFn = createServerFn({ method: 'POST' }) setupState: JSON.stringify(setupState), }) - await ensureAdminPrincipal(session.user.id as UserId, 'saveUseCaseFn') - await invalidateSettingsCache() console.log( `[fn:onboarding] saveUseCaseFn: created initial settings with useCase=${data.useCase}` From 3cf9d15ad5a3e68c240529f63adf1483730b7f80 Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Fri, 5 Jun 2026 16:58:30 -0700 Subject: [PATCH 3/4] fix(onboarding): restrict bootstrap admin promotion --- .../__tests__/onboarding-security.test.ts | 199 ++++++++++++++++++ .../src/lib/server/functions/onboarding.ts | 99 ++++++--- 2 files changed, 265 insertions(+), 33 deletions(-) create mode 100644 apps/web/src/lib/server/functions/__tests__/onboarding-security.test.ts diff --git a/apps/web/src/lib/server/functions/__tests__/onboarding-security.test.ts b/apps/web/src/lib/server/functions/__tests__/onboarding-security.test.ts new file mode 100644 index 000000000..978624abf --- /dev/null +++ b/apps/web/src/lib/server/functions/__tests__/onboarding-security.test.ts @@ -0,0 +1,199 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +type AnyHandler = (args: { data: Record }) => Promise + +vi.mock('@tanstack/react-start', () => ({ + createServerFn: () => ({ + inputValidator: () => ({ + handler: (fn: AnyHandler) => fn, + }), + handler: (fn: AnyHandler) => fn, + }), +})) + +const hoisted = vi.hoisted(() => ({ + mockGetSession: vi.fn(), + mockGetSettings: vi.fn(), + mockAssertNotManaged: vi.fn(), + mockInvalidateSettingsCache: vi.fn(), + mockPrincipalFindFirst: vi.fn(), + mockTxExecute: vi.fn(), + mockUpdateSet: vi.fn(), + mockUpdateWhere: vi.fn(), + mockInsertValues: vi.fn(), + mockTransaction: vi.fn(), + mockEq: vi.fn((column: string, value: unknown) => ({ op: 'eq', column, value })), + mockAnd: vi.fn((...conditions: unknown[]) => ({ op: 'and', conditions })), + mockNe: vi.fn((column: string, value: unknown) => ({ op: 'ne', column, value })), + mockGenerateId: vi.fn(), +})) + +vi.mock('@/lib/server/auth/session', () => ({ + getSession: hoisted.mockGetSession, +})) + +vi.mock('../workspace', () => ({ + getSettings: hoisted.mockGetSettings, +})) + +vi.mock('@/lib/server/config-file/managed-guard', () => ({ + assertNotManaged: hoisted.mockAssertNotManaged, +})) + +vi.mock('@/lib/server/domains/settings/settings.helpers', () => ({ + invalidateSettingsCache: hoisted.mockInvalidateSettingsCache, +})) + +vi.mock('@opencoven-feedback/ids', () => ({ + generateId: hoisted.mockGenerateId, +})) + +vi.mock('@/lib/server/domains/principals/principal.service', () => ({ + syncPrincipalProfile: vi.fn(), +})) + +vi.mock('@/lib/server/domains/boards/board.service', () => ({ + listBoards: vi.fn(), +})) + +vi.mock('@/lib/server/domains/settings', () => ({ + DEFAULT_AUTH_CONFIG: {}, + DEFAULT_PORTAL_CONFIG: {}, +})) + +vi.mock('@/lib/server/config-file/managed-paths', () => ({ + isPathManaged: vi.fn(() => false), +})) + +vi.mock('@/lib/server/db', () => ({ + USE_CASE_TYPES: ['saas', 'consumer', 'marketplace', 'internal'], + DEFAULT_STATUSES: [], + settings: { id: 'settings.id' }, + principal: { + id: 'principal.id', + userId: 'principal.userId', + role: 'principal.role', + type: 'principal.type', + }, + user: { id: 'user.id' }, + postStatuses: {}, + eq: hoisted.mockEq, + and: hoisted.mockAnd, + ne: hoisted.mockNe, + sql: (strings: TemplateStringsArray, ...values: unknown[]) => ({ strings, values }), + db: { + query: { + principal: { + findFirst: hoisted.mockPrincipalFindFirst, + }, + postStatuses: { + findFirst: vi.fn(), + }, + }, + update: vi.fn(() => ({ + set: hoisted.mockUpdateSet.mockImplementation(() => ({ + where: hoisted.mockUpdateWhere.mockResolvedValue(undefined), + returning: vi.fn().mockResolvedValue([]), + })), + })), + insert: vi.fn(() => ({ + values: hoisted.mockInsertValues.mockResolvedValue(undefined), + })), + transaction: hoisted.mockTransaction.mockImplementation(async (fn: (tx: unknown) => unknown) => + fn({ + execute: hoisted.mockTxExecute.mockResolvedValue(undefined), + query: { + principal: { + findFirst: hoisted.mockPrincipalFindFirst, + }, + }, + update: vi.fn(() => ({ + set: hoisted.mockUpdateSet.mockImplementation(() => ({ + where: hoisted.mockUpdateWhere.mockResolvedValue(undefined), + })), + })), + insert: vi.fn(() => ({ + values: hoisted.mockInsertValues.mockResolvedValue(undefined), + })), + }) + ), + }, +})) + +const { saveUseCaseFn } = await import('../onboarding') + +const incompleteSettings = { + id: 'workspace_existing', + setupState: JSON.stringify({ + version: 1, + steps: { core: true, workspace: false, boards: false }, + }), +} + +describe('saveUseCaseFn onboarding admin promotion', () => { + beforeEach(() => { + vi.clearAllMocks() + hoisted.mockGetSession.mockResolvedValue({ user: { id: 'user_attacker' } }) + hoisted.mockGetSettings.mockResolvedValue(incompleteSettings) + hoisted.mockAssertNotManaged.mockResolvedValue(undefined) + hoisted.mockInvalidateSettingsCache.mockResolvedValue(undefined) + hoisted.mockGenerateId.mockReturnValue('principal_new_admin') + }) + + it('does not promote an existing non-admin principal when a human admin already exists', async () => { + hoisted.mockPrincipalFindFirst + .mockResolvedValueOnce({ id: 'principal_attacker', userId: 'user_attacker', role: 'user' }) + .mockResolvedValueOnce({ + id: 'principal_admin', + userId: 'user_admin', + role: 'admin', + type: 'user', + }) + + await expect(saveUseCaseFn({ data: { useCase: 'saas' } })).rejects.toThrow( + 'Only admin can complete setup' + ) + + expect(hoisted.mockUpdateSet).not.toHaveBeenCalled() + expect(hoisted.mockInsertValues).not.toHaveBeenCalled() + }) + + it('does not promote a non-admin principal when another human principal already exists', async () => { + hoisted.mockPrincipalFindFirst + .mockResolvedValueOnce({ id: 'principal_attacker', userId: 'user_attacker', role: 'user' }) + .mockResolvedValueOnce(null) + .mockResolvedValueOnce({ id: 'principal_other', userId: 'user_other', role: 'user' }) + + await expect(saveUseCaseFn({ data: { useCase: 'saas' } })).rejects.toThrow( + 'Only admin can complete setup' + ) + + expect(hoisted.mockUpdateSet).not.toHaveBeenCalled() + expect(hoisted.mockInsertValues).not.toHaveBeenCalled() + }) + + it('still bootstraps an admin principal when no other human principal exists', async () => { + hoisted.mockPrincipalFindFirst + .mockResolvedValueOnce(null) + .mockResolvedValueOnce(null) + .mockResolvedValueOnce(null) + + await saveUseCaseFn({ data: { useCase: 'saas' } }) + + expect(hoisted.mockInsertValues).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'principal_new_admin', + userId: 'user_attacker', + role: 'admin', + }) + ) + expect(hoisted.mockUpdateSet).toHaveBeenCalledWith({ + setupState: JSON.stringify({ + version: 1, + steps: { core: true, workspace: false, boards: false }, + useCase: 'saas', + }), + }) + expect(hoisted.mockInvalidateSettingsCache).toHaveBeenCalledOnce() + }) +}) diff --git a/apps/web/src/lib/server/functions/onboarding.ts b/apps/web/src/lib/server/functions/onboarding.ts index 5038b2c28..f463f5591 100644 --- a/apps/web/src/lib/server/functions/onboarding.ts +++ b/apps/web/src/lib/server/functions/onboarding.ts @@ -14,8 +14,10 @@ import { principal, user, postStatuses, - and, eq, + and, + ne, + sql, DEFAULT_STATUSES, } from '@/lib/server/db' import { invalidateSettingsCache } from '@/lib/server/domains/settings/settings.helpers' @@ -25,36 +27,53 @@ import { isPathManaged } from '@/lib/server/config-file/managed-paths' import { slugify } from '@/lib/shared/utils' import { getSetupState } from '@/lib/shared/db-types' -async function ensureOnboardingAdminPrincipal(userId: UserId, logLabel: string): Promise { - const existing = await db.query.principal.findFirst({ - where: eq(principal.userId, userId), - }) - - if (existing && isAdmin(existing.role)) { - return - } - - const existingAdmin = await db.query.principal.findFirst({ - where: and(eq(principal.role, 'admin'), eq(principal.type, 'user')), - columns: { id: true }, - }) +/** Onboarding promotes the acting user to admin during bootstrap only. + * Once another human principal exists, onboarding endpoints must not + * promote a different non-admin principal. */ +async function ensureAdminPrincipal(userId: UserId, logLabel: string): Promise { + await db.transaction(async (tx) => { + await tx.execute( + sql`SELECT pg_advisory_xact_lock(hashtext('quackback:onboarding_bootstrap_admin'))` + ) + + const existing = await tx.query.principal.findFirst({ + where: eq(principal.userId, userId), + }) + if (existing && isAdmin(existing.role)) { + return + } - if (existingAdmin) { - throw new Error('Only admin can complete setup') - } + const existingAdmin = await tx.query.principal.findFirst({ + where: and(eq(principal.role, 'admin'), eq(principal.type, 'user')), + columns: { id: true }, + }) + if (existingAdmin) { + throw new Error('Only admin can complete setup') + } - if (!existing) { - console.log(`[fn:onboarding] ${logLabel}: creating admin principal for first onboarding user`) - await db.insert(principal).values({ - id: generateId('principal'), - userId, - role: 'admin', - createdAt: new Date(), + const existingHumanPrincipal = await tx.query.principal.findFirst({ + where: existing + ? and(eq(principal.type, 'user'), ne(principal.id, existing.id)) + : eq(principal.type, 'user'), + columns: { id: true }, }) - } else { - console.log(`[fn:onboarding] ${logLabel}: upgrading first onboarding user to admin`) - await db.update(principal).set({ role: 'admin' }).where(eq(principal.userId, userId)) - } + if (existingHumanPrincipal) { + throw new Error('Only admin can complete setup') + } + + if (!existing) { + console.log(`[fn:onboarding] ${logLabel}: creating admin principal for first onboarding user`) + await tx.insert(principal).values({ + id: generateId('principal'), + userId, + role: 'admin', + createdAt: new Date(), + }) + } else { + console.log(`[fn:onboarding] ${logLabel}: upgrading first onboarding user to admin`) + await tx.update(principal).set({ role: 'admin' }).where(eq(principal.userId, userId)) + } + }) } /** @@ -138,9 +157,21 @@ export const setupWorkspaceFn = createServerFn({ method: 'POST' }) let setupState: SetupState | null = getSetupState(existingSettings?.setupState ?? null) - // First authenticated user may bootstrap an install, but once a human admin - // exists, onboarding mutations require the acting user to already be admin. - await ensureOnboardingAdminPrincipal(session.user.id as UserId, 'setupWorkspaceFn') + // Fresh install (no settings): first authenticated user becomes admin. + // Settings exist + workspace step done: require existing admin. + // Settings exist + workspace step not done: ensure user becomes admin. + if (!existingSettings) { + await ensureAdminPrincipal(session.user.id as UserId, 'setupWorkspaceFn') + } else if (setupState?.steps?.workspace) { + const principalRecord = await db.query.principal.findFirst({ + where: eq(principal.userId, session.user.id as UserId), + }) + if (!principalRecord || !isAdmin(principalRecord.role)) { + throw new Error('Only admin can complete setup') + } + } else { + await ensureAdminPrincipal(session.user.id as UserId, 'setupWorkspaceFn') + } // Check if onboarding is already complete if (setupState?.steps?.core && setupState?.steps?.workspace && setupState?.steps?.boards) { @@ -354,7 +385,9 @@ export const saveUseCaseFn = createServerFn({ method: 'POST' }) steps: { core: true, workspace: false, boards: false }, } - await ensureOnboardingAdminPrincipal(session.user.id as UserId, 'saveUseCaseFn') + if (!setupState.steps.workspace) { + await ensureAdminPrincipal(session.user.id as UserId, 'saveUseCaseFn') + } const updatedState: SetupState = { ...setupState, useCase: data.useCase } @@ -379,7 +412,7 @@ export const saveUseCaseFn = createServerFn({ method: 'POST' }) useCase: data.useCase, } - await ensureOnboardingAdminPrincipal(session.user.id as UserId, 'saveUseCaseFn') + await ensureAdminPrincipal(session.user.id as UserId, 'saveUseCaseFn') await db.insert(settings).values({ id: generateId('workspace'), From db2033c3a23fa42cc6240fa1fc9f339903e4eadd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 6 Jun 2026 13:47:19 +0000 Subject: [PATCH 4/4] fix(onboarding): serialize bootstrap admin checks and gate fresh use-case insert --- .../__tests__/onboarding-security.test.ts | 14 +++++++ .../functions/__tests__/onboarding.test.ts | 38 +++++++++++++------ .../src/lib/server/functions/onboarding.ts | 2 + 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/apps/web/src/lib/server/functions/__tests__/onboarding-security.test.ts b/apps/web/src/lib/server/functions/__tests__/onboarding-security.test.ts index 978624abf..00f9bcff9 100644 --- a/apps/web/src/lib/server/functions/__tests__/onboarding-security.test.ts +++ b/apps/web/src/lib/server/functions/__tests__/onboarding-security.test.ts @@ -196,4 +196,18 @@ describe('saveUseCaseFn onboarding admin promotion', () => { }) expect(hoisted.mockInvalidateSettingsCache).toHaveBeenCalledOnce() }) + + it('rejects before inserting settings on fresh install when another human principal exists', async () => { + hoisted.mockGetSettings.mockResolvedValue(null) + hoisted.mockPrincipalFindFirst + .mockResolvedValueOnce({ id: 'principal_attacker', userId: 'user_attacker', role: 'user' }) + .mockResolvedValueOnce(null) + .mockResolvedValueOnce({ id: 'principal_other', userId: 'user_other', role: 'user' }) + + await expect(saveUseCaseFn({ data: { useCase: 'saas' } })).rejects.toThrow( + 'Only admin can complete setup' + ) + + expect(hoisted.mockInsertValues).not.toHaveBeenCalled() + }) }) diff --git a/apps/web/src/lib/server/functions/__tests__/onboarding.test.ts b/apps/web/src/lib/server/functions/__tests__/onboarding.test.ts index f00567f6a..a6cc8a51e 100644 --- a/apps/web/src/lib/server/functions/__tests__/onboarding.test.ts +++ b/apps/web/src/lib/server/functions/__tests__/onboarding.test.ts @@ -24,6 +24,7 @@ const { state, principalTable, settingsTable, postStatusesTable, userTable } = v statusesExist: true, }, principalTable: { + id: 'principal.id', userId: 'principal.userId', role: 'principal.role', type: 'principal.type', @@ -87,23 +88,20 @@ function matchesWhere(row: Record, where: unknown): boolean { if (clause.op === 'and') { return clause.conditions?.every((condition) => matchesWhere(row, condition)) ?? false } + if (clause.op === 'ne') { + if (clause.col === principalTable.id) return row.id !== clause.value + return false + } if (clause.col === principalTable.userId) return row.userId === clause.value + if (clause.col === principalTable.id) return row.id === clause.value if (clause.col === principalTable.role) return row.role === clause.value if (clause.col === principalTable.type) return row.type === clause.value if (clause.col === settingsTable.id) return row.id === clause.value return false } -vi.mock('@/lib/server/db', () => ({ - USE_CASE_TYPES: ['saas', 'consumer', 'marketplace', 'internal'], - DEFAULT_STATUSES: [], - principal: principalTable, - settings: settingsTable, - postStatuses: postStatusesTable, - user: userTable, - eq: (col: string, value: unknown) => ({ op: 'eq', col, value }), - and: (...conditions: unknown[]) => ({ op: 'and', conditions }), - db: { +vi.mock('@/lib/server/db', () => { + const dbMock = { query: { principal: { findFirst: async ({ where }: { where: unknown }) => @@ -152,8 +150,24 @@ vi.mock('@/lib/server/db', () => ({ }, }), }), - }, -})) + execute: async () => undefined, + transaction: async (fn: (tx: unknown) => unknown) => fn(dbMock), + } + + return { + USE_CASE_TYPES: ['saas', 'consumer', 'marketplace', 'internal'], + DEFAULT_STATUSES: [], + principal: principalTable, + settings: settingsTable, + postStatuses: postStatusesTable, + user: userTable, + eq: (col: string, value: unknown) => ({ op: 'eq', col, value }), + and: (...conditions: unknown[]) => ({ op: 'and', conditions }), + ne: (col: string, value: unknown) => ({ op: 'ne', col, value }), + sql: (strings: TemplateStringsArray, ...values: unknown[]) => ({ strings, values }), + db: dbMock, + } +}) beforeEach(() => { state.sessionUserId = 'user_attacker' diff --git a/apps/web/src/lib/server/functions/onboarding.ts b/apps/web/src/lib/server/functions/onboarding.ts index f463f5591..f4689b299 100644 --- a/apps/web/src/lib/server/functions/onboarding.ts +++ b/apps/web/src/lib/server/functions/onboarding.ts @@ -406,6 +406,8 @@ export const saveUseCaseFn = createServerFn({ method: 'POST' }) // (same rationale as setupWorkspaceFn): no settings row yet to // read managedFieldPaths from. The reconciler will overwrite on // its next tick if the file owns these fields. + await ensureAdminPrincipal(session.user.id as UserId, 'saveUseCaseFn') + const setupState: SetupState = { version: 1, steps: { core: true, workspace: false, boards: false },