diff --git a/e2e/adapter/test/kysely-adapter/migration-1-6-to-1-7.test.ts b/e2e/adapter/test/kysely-adapter/migration-1-6-to-1-7.test.ts index fa4462e3bf..a244058311 100644 --- a/e2e/adapter/test/kysely-adapter/migration-1-6-to-1-7.test.ts +++ b/e2e/adapter/test/kysely-adapter/migration-1-6-to-1-7.test.ts @@ -1373,7 +1373,10 @@ it("retires published 1.6.30 SCIM credentials with a custom account ID column an } }); -it("checkpoints MySQL legacy tables before retiring SCIM accounts", { +/** + * @see https://github.com/better-auth/better-auth/pull/10575#discussion_r3831145554 + */ +it("checkpoints MySQL legacy tables and rolls back interrupted SCIM retirement", { timeout: 60_000, }, async () => { const databaseName = createDatabaseName().replace("postgres", "scim_mysql"); @@ -1417,12 +1420,6 @@ it("checkpoints MySQL legacy tables before retiring SCIM accounts", { emailDomain: "mysql.example.com", }); - const auth17 = betterAuth({ - baseURL: "http://localhost:3000", - database: { db: currentDatabase, transaction: true, type: "mysql" }, - emailAndPassword: { enabled: true }, - plugins: [createCurrentScimPlugin(legacyScimAccount.userId)], - }); const options = { accountIssuers: { workforce: "local:retired-scim:workforce", @@ -1432,6 +1429,12 @@ it("checkpoints MySQL legacy tables before retiring SCIM accounts", { providers: "reprovision" as const, }, }; + const auth17 = betterAuth({ + baseURL: "http://localhost:3000", + database: { db: currentDatabase, transaction: true, type: "mysql" }, + emailAndPassword: { enabled: true }, + plugins: [createCurrentScimPlugin(legacyScimAccount.userId)], + }); await expect(migrateFrom16(auth17.options, options)).rejects.toThrow( "forced SCIM account retirement interruption", ); @@ -1454,13 +1457,18 @@ it("checkpoints MySQL legacy tables before retiring SCIM accounts", { "SELECT COUNT(*) AS count FROM `account` WHERE `id` = ?", [legacyScimAccount.id], ); - expect(Number(remainingAccounts[0]?.count)).toBe(0); + expect(Number(remainingAccounts[0]?.count)).toBe(1); const migration = await migrateFrom16(auth17.options, options); expect(migration).toMatchObject({ accounts: { migrated: 0, providers: {} }, scim: { - identities: [], + identities: [ + { + providerId: "workforce", + userId: legacyScimAccount.userId, + }, + ], reprovisionRequired: true, retiredProviders: 1, }, diff --git a/packages/better-auth/src/db/get-migration.ts b/packages/better-auth/src/db/get-migration.ts index 3f5ccdc542..ab7f8e09ec 100644 --- a/packages/better-auth/src/db/get-migration.ts +++ b/packages/better-auth/src/db/get-migration.ts @@ -1631,8 +1631,21 @@ export async function migrateFrom16( options, releaseData, ); - await inspectScimAccountsFrom16(config, options, releaseData); + const inspectedScimAccounts = await inspectScimAccountsFrom16( + config, + options, + releaseData, + ); const migrationDatabase = await getMigrationDatabase(config); + if ( + migrationDatabase.databaseType === "mysql" && + inspectedScimAccounts.length > 0 && + !migrationDatabase.transaction + ) { + throw new BetterAuthError( + `The ${migrationDatabase.adapterId} adapter must expose a transaction-scoped migration connection before Better Auth can safely retire populated 1.6 SCIM accounts on MySQL.`, + ); + } const initialMigration = await getMigrationsWithDatabase( config, { diff --git a/packages/better-auth/src/db/release-migration.test.ts b/packages/better-auth/src/db/release-migration.test.ts index a2ebcc10a4..9cae7c457e 100644 --- a/packages/better-auth/src/db/release-migration.test.ts +++ b/packages/better-auth/src/db/release-migration.test.ts @@ -1,6 +1,7 @@ import { DatabaseSync } from "node:sqlite"; import type { BetterAuthOptions } from "@better-auth/core"; import { describe, expect, it } from "vitest"; +import { getMigrationDatabase } from "./migration-database"; import type { LegacyReleaseDataState, MigrateFrom16Options, @@ -47,6 +48,32 @@ function createScimRetirementFixture() { } describe("1.6 SCIM account retirement", () => { + /** + * @see https://github.com/better-auth/better-auth/pull/10575#discussion_r3831145554 + */ + it("requires a transaction before deleting MySQL SCIM accounts", async () => { + const { config, database, options, state } = createScimRetirementFixture(); + const migrationDatabase = await getMigrationDatabase(config); + const inspectedAccounts = await inspectScimAccountsFrom16( + config, + options, + state, + ); + + await expect( + retireScimAccountsFrom16(config, options, state, inspectedAccounts, { + ...migrationDatabase, + databaseType: "mysql", + transaction: undefined, + }), + ).rejects.toThrow( + "must expose a transaction-scoped migration connection before Better Auth can safely retire populated 1.6 SCIM accounts on MySQL", + ); + expect( + database.prepare(`SELECT "id" FROM "account" ORDER BY "id"`).all(), + ).toEqual([{ id: "a1" }]); + }); + /** * @see https://github.com/better-auth/better-auth/pull/10575 */ diff --git a/packages/better-auth/src/db/release-migration.ts b/packages/better-auth/src/db/release-migration.ts index 8b53cfbd18..0b3ded7c73 100644 --- a/packages/better-auth/src/db/release-migration.ts +++ b/packages/better-auth/src/db/release-migration.ts @@ -1382,20 +1382,22 @@ async function readScimAccountsFrom16( (table) => table.name === accountTable, ); if (!accountTableMetadata) return { accounts: [], accountTable }; - const accounts = - providerIds.size === 0 - ? [] - : ( - await sql` - SELECT - ${sql.ref(idColumn)} AS "id", - ${sql.ref(accountIdColumn)} AS "providerAccountId", - ${sql.ref(providerIdColumn)} AS "providerId", - ${sql.ref(userIdColumn)} AS "userId" - FROM ${sql.table(accountTable)} - WHERE ${sql.ref(providerIdColumn)} IN (${sql.join([...providerIds])}) - `.execute(kysely) - ).rows; + if (providerIds.size === 0) return { accounts: [], accountTable }; + const accountQuery = sql` + SELECT + ${sql.ref(idColumn)} AS "id", + ${sql.ref(accountIdColumn)} AS "providerAccountId", + ${sql.ref(providerIdColumn)} AS "providerId", + ${sql.ref(userIdColumn)} AS "userId" + FROM ${sql.table(accountTable)} + WHERE ${sql.ref(providerIdColumn)} IN (${sql.join([...providerIds])}) + `; + const lockedAccountQuery = + migrationDatabase?.databaseType === "mysql" && + migrationDatabase.inTransaction + ? sql`${accountQuery} FOR UPDATE` + : accountQuery; + const accounts = (await lockedAccountQuery.execute(kysely)).rows; return { accounts: [...accounts], accountTable }; } @@ -1467,12 +1469,13 @@ export async function retireScimAccountsFrom16( migrationDatabase?: MigrationDatabase, ): Promise { if (!state.scimProvider || !options.scim) return []; + const database = migrationDatabase ?? (await getMigrationDatabase(config)); const accounts = await inspectScimAccountsFrom16( config, options, state, undefined, - migrationDatabase, + database, ); const freshAccountIds = new Set(accounts.map((account) => account.id)); if ( @@ -1485,7 +1488,17 @@ export async function retireScimAccountsFrom16( ); } if (accounts.length === 0) return []; - const { kysely } = migrationDatabase ?? (await getMigrationDatabase(config)); + if (database.databaseType === "mysql" && !database.inTransaction) { + if (!database.transaction) { + throw new BetterAuthError( + `The ${database.adapterId} adapter must expose a transaction-scoped migration connection before Better Auth can safely retire populated 1.6 SCIM accounts on MySQL.`, + ); + } + return database.transaction((transaction) => + retireScimAccountsFrom16(config, options, state, accounts, transaction), + ); + } + const { kysely } = database; const accountSchema = getAuthTables(config).account; if (!accountSchema) return []; const accountTable = accountSchema.modelName || "account"; @@ -1500,7 +1513,7 @@ export async function retireScimAccountsFrom16( const { accounts: remainingAccounts } = await readScimAccountsFrom16( config, state, - migrationDatabase, + database, ); if (remainingAccounts.length > 0) { reportMigrationDecisionBlocker(undefined, {