From e43c006bb170c5890c9cd9c2b1f1c878cb92fcf1 Mon Sep 17 00:00:00 2001 From: igm Date: Mon, 12 Jan 2026 17:07:14 +0800 Subject: [PATCH] Fix merge conflicts and add withTransaction helper - Add withTransaction helper that gracefully handles nested transactions (reuses existing transaction in tests, starts new one otherwise) - Update auth procedures to use withTransaction instead of direct .transaction() - Add email config to all e2e test contexts (required by merged code) - Remove duplicate verification token code from signup procedure Co-Authored-By: Claude Opus 4.5 --- .../src/__tests__/e2e/admin.test.ts | 6 ++++ .../api-server/src/__tests__/e2e/auth.test.ts | 6 ++++ apps/api-server/src/__tests__/e2e/me.test.ts | 6 ++++ .../api-server/src/__tests__/e2e/orgs.test.ts | 6 ++++ .../src/__tests__/e2e/webauthn.test.ts | 11 ++++++ .../src/procedures/auth/forgot-password.ts | 3 +- .../src/procedures/auth/login-if-completed.ts | 7 ++-- apps/api-server/src/procedures/auth/signup.ts | 19 ++--------- packages/db/src/helpers/with-transaction.ts | 34 +++++++++++++++++++ packages/db/src/index.ts | 4 +++ 10 files changed, 82 insertions(+), 20 deletions(-) create mode 100644 packages/db/src/helpers/with-transaction.ts diff --git a/apps/api-server/src/__tests__/e2e/admin.test.ts b/apps/api-server/src/__tests__/e2e/admin.test.ts index 0ee9692..7d68b5b 100644 --- a/apps/api-server/src/__tests__/e2e/admin.test.ts +++ b/apps/api-server/src/__tests__/e2e/admin.test.ts @@ -39,6 +39,7 @@ import { uniqueTestId, withTestTransaction, } from "@reviq/test-helpers"; +import { createLoggingEmailClient } from "@reviq/emails"; import { router } from "../../router.js"; import { COOKIE_NAMES } from "../../utils/cookies.js"; import { hashToken } from "../../utils/crypto.js"; @@ -75,6 +76,11 @@ function createAPIContext( rpName: TEST_RP.rpName, reqHeaders, resHeaders: new Headers(), + email: { + client: createLoggingEmailClient(), + fromAddress: "test@example.com", + baseUrl: TEST_RP.origin, + }, }; } diff --git a/apps/api-server/src/__tests__/e2e/auth.test.ts b/apps/api-server/src/__tests__/e2e/auth.test.ts index 5654241..cd238f6 100644 --- a/apps/api-server/src/__tests__/e2e/auth.test.ts +++ b/apps/api-server/src/__tests__/e2e/auth.test.ts @@ -50,6 +50,7 @@ import { uniqueTestId, withTestTransaction, } from "@reviq/test-helpers"; +import { createLoggingEmailClient } from "@reviq/emails"; import { VirtualAuthenticator } from "@reviq/virtual-authenticator"; import { router } from "../../router.js"; import { COOKIE_NAMES } from "../../utils/cookies.js"; @@ -100,6 +101,11 @@ function createAPIContext( rpName: TEST_RP.rpName, reqHeaders, resHeaders: new Headers(), + email: { + client: createLoggingEmailClient(), + fromAddress: "test@example.com", + baseUrl: TEST_RP.origin, + }, }; } diff --git a/apps/api-server/src/__tests__/e2e/me.test.ts b/apps/api-server/src/__tests__/e2e/me.test.ts index 61ecb62..55c750d 100644 --- a/apps/api-server/src/__tests__/e2e/me.test.ts +++ b/apps/api-server/src/__tests__/e2e/me.test.ts @@ -32,6 +32,7 @@ import { uniqueTestId, withTestTransaction, } from "@reviq/test-helpers"; +import { createLoggingEmailClient } from "@reviq/emails"; import { router } from "../../router.js"; import { COOKIE_NAMES } from "../../utils/cookies.js"; import { hashToken } from "../../utils/crypto.js"; @@ -82,6 +83,11 @@ function createAPIContext( rpName: TEST_RP.rpName, reqHeaders, resHeaders: new Headers(), + email: { + client: createLoggingEmailClient(), + fromAddress: "test@example.com", + baseUrl: TEST_RP.origin, + }, }; } diff --git a/apps/api-server/src/__tests__/e2e/orgs.test.ts b/apps/api-server/src/__tests__/e2e/orgs.test.ts index 40ea8a2..cfa91e3 100644 --- a/apps/api-server/src/__tests__/e2e/orgs.test.ts +++ b/apps/api-server/src/__tests__/e2e/orgs.test.ts @@ -23,6 +23,7 @@ import { uniqueTestId, withTestTransaction, } from "@reviq/test-helpers"; +import { createLoggingEmailClient } from "@reviq/emails"; import { router } from "../../router.js"; import { COOKIE_NAMES } from "../../utils/cookies.js"; import { hashToken } from "../../utils/crypto.js"; @@ -58,6 +59,11 @@ function createAPIContext( rpName: TEST_RP.rpName, reqHeaders, resHeaders: new Headers(), + email: { + client: createLoggingEmailClient(), + fromAddress: "test@example.com", + baseUrl: TEST_RP.origin, + }, }; } diff --git a/apps/api-server/src/__tests__/e2e/webauthn.test.ts b/apps/api-server/src/__tests__/e2e/webauthn.test.ts index d007150..59ad8c9 100644 --- a/apps/api-server/src/__tests__/e2e/webauthn.test.ts +++ b/apps/api-server/src/__tests__/e2e/webauthn.test.ts @@ -23,6 +23,7 @@ import { uniqueTestId, withTestTransaction, } from "@reviq/test-helpers"; +import { createLoggingEmailClient } from "@reviq/emails"; import { VirtualAuthenticator } from "@reviq/virtual-authenticator"; import { router } from "../../router.js"; import { COOKIE_NAMES } from "../../utils/cookies.js"; @@ -51,6 +52,11 @@ function createAPIContext( rpName: TEST_RP.rpName, reqHeaders, resHeaders: new Headers(), + email: { + client: createLoggingEmailClient(), + fromAddress: "test@example.com", + baseUrl: TEST_RP.origin, + }, }; } @@ -133,6 +139,11 @@ function createLoginRequestContext( rpName: TEST_RP.rpName, reqHeaders, resHeaders: new Headers(), + email: { + client: createLoggingEmailClient(), + fromAddress: "test@example.com", + baseUrl: TEST_RP.origin, + }, }; } diff --git a/apps/api-server/src/procedures/auth/forgot-password.ts b/apps/api-server/src/procedures/auth/forgot-password.ts index d4c2d54..d7b0453 100644 --- a/apps/api-server/src/procedures/auth/forgot-password.ts +++ b/apps/api-server/src/procedures/auth/forgot-password.ts @@ -6,6 +6,7 @@ * This prevents attackers from determining which emails are registered */ +import { withTransaction } from "@reviq/db"; import { sendPasswordResetEmail } from "@reviq/emails"; import { TOKEN_DURATIONS } from "../../utils/cookies.js"; import { @@ -37,7 +38,7 @@ export const forgotPassword = os.auth.forgotPassword.handler( const expiresAt = generateExpiry(TOKEN_DURATIONS.PASSWORD_RESET); // Delete old tokens and insert new one in transaction - await context.db.transaction().execute(async (trx) => { + await withTransaction(context.db, async (trx) => { // Delete any existing password reset tokens for this user (security measure) await trx .deleteFrom("password_resets") diff --git a/apps/api-server/src/procedures/auth/login-if-completed.ts b/apps/api-server/src/procedures/auth/login-if-completed.ts index adf8856..28ca67d 100644 --- a/apps/api-server/src/procedures/auth/login-if-completed.ts +++ b/apps/api-server/src/procedures/auth/login-if-completed.ts @@ -16,6 +16,7 @@ * e. Return { status: 'completed', redirectTo: '/dashboard' or '/auth/trust-device' } */ +import { withTransaction } from "@reviq/db"; import { COOKIE_NAMES, COOKIE_OPTIONS, @@ -90,9 +91,9 @@ export const loginIfRequestIsCompleted = const userAgent = getUserAgent(context.reqHeaders); // Create session in transaction (atomic: device upsert + session + login_request delete) - const { session, deviceTrusted } = await context.db - .transaction() - .execute(async (trx) => { + const { session, deviceTrusted } = await withTransaction( + context.db, + async (trx) => { // Upsert user device const deviceId = await upsertUserDevice( trx, diff --git a/apps/api-server/src/procedures/auth/signup.ts b/apps/api-server/src/procedures/auth/signup.ts index ecfc1bf..a8f9d0a 100644 --- a/apps/api-server/src/procedures/auth/signup.ts +++ b/apps/api-server/src/procedures/auth/signup.ts @@ -10,6 +10,7 @@ import type { import type { Kysely } from "kysely"; import type { RPInfo } from "../../utils/webauthn.js"; import { ORPCError } from "@orpc/server"; +import { withTransaction } from "@reviq/db"; import { sendVerificationEmail } from "@reviq/emails"; import { verifyRegistrationResponse } from "@simplewebauthn/server"; import { @@ -159,7 +160,7 @@ export async function signupWithPasskey( // Create user and passkey in a transaction (handle race condition if concurrent signup) try { - const result = await db.transaction().execute(async (trx) => { + const result = await withTransaction(db, async (trx) => { // Create user const user = await trx .insertInto("users") @@ -276,7 +277,7 @@ export const signup = os.auth.signup.handler(async ({ input, context }) => { ); // Create session and email verification in transaction - const session = await context.db.transaction().execute(async (trx) => { + const session = await withTransaction(context.db, async (trx) => { // Create session (7 days, trusted mode false initially, no device) const newSession = await createSession(trx, { userId, @@ -307,20 +308,6 @@ export const signup = os.auth.signup.handler(async ({ input, context }) => { COOKIE_OPTIONS.session, ); - // Generate verification token - const verificationToken = generateSecureBase58Token(); - const expiresAt = generateExpiry(TOKEN_DURATIONS.EMAIL_VERIFICATION); - - // Store verification token (store raw token, not hash - it's already high-entropy) - await context.db - .insertInto("email_verifications") - .values({ - user_id: userId, - token: verificationToken, - expires_at: expiresAt, - }) - .execute(); - // Send verification email await sendVerificationEmail({ client: context.email.client, diff --git a/packages/db/src/helpers/with-transaction.ts b/packages/db/src/helpers/with-transaction.ts new file mode 100644 index 0000000..21137fb --- /dev/null +++ b/packages/db/src/helpers/with-transaction.ts @@ -0,0 +1,34 @@ +import type { Database } from "@reviq/db-schema"; +import type { Kysely, Transaction } from "kysely"; + +/** + * Type for a database connection that could be either a Kysely instance or a Transaction + */ +export type DbConnection = Kysely | Transaction; + +/** + * Execute a callback within a transaction, handling nested transaction scenarios. + * + * If the provided db is already a transaction, the callback is executed directly + * without starting a new transaction (since Kysely doesn't support nested transactions). + * + * If the provided db is a regular Kysely instance, a new transaction is started. + * + * @param db - Database connection (Kysely instance or Transaction) + * @param callback - Function to execute within the transaction + * @returns The result of the callback + */ +export async function withTransaction( + db: DbConnection, + callback: (trx: Transaction) => Promise, +): Promise { + // Check if db is already a transaction + // Kysely Transaction objects have isTransaction = true + if ("isTransaction" in db && db.isTransaction) { + // Already in a transaction, execute callback directly + return callback(db as Transaction); + } + + // Not in a transaction, start one + return (db as Kysely).transaction().execute(callback); +} diff --git a/packages/db/src/index.ts b/packages/db/src/index.ts index 7571b38..8e370e2 100644 --- a/packages/db/src/index.ts +++ b/packages/db/src/index.ts @@ -33,6 +33,10 @@ export { parseToken, TOKEN_PREFIX, } from "./helpers/token.js"; +export { + type DbConnection, + withTransaction, +} from "./helpers/with-transaction.js"; /** * Export model operations */