diff --git a/docs/content/docs/concepts/cli.mdx b/docs/content/docs/concepts/cli.mdx index 0dd0cc30f7..42b4ebdfc9 100644 --- a/docs/content/docs/concepts/cli.mdx +++ b/docs/content/docs/concepts/cli.mdx @@ -43,7 +43,7 @@ npx auth@latest migrate plan npx auth@latest migrate plan --json > better-auth-migration-plan.json ``` -The JSON plan includes the adapter, dialect, selected and detected account identity strategies, sorted schema changes, blockers, and any release migration actions. A blocked plan exits with status `1`. +The JSON plan includes the adapter, dialect, selected and detected account identity strategies, account counts, issuer-resolution coverage, projected collisions, sorted schema changes, blockers, and any release migration actions. A blocked plan exits with status `1`. ### Apply a migration diff --git a/docs/content/docs/guides/1-7-upgrade-guide.mdx b/docs/content/docs/guides/1-7-upgrade-guide.mdx index e6032f041c..76a5b6b0ea 100644 --- a/docs/content/docs/guides/1-7-upgrade-guide.mdx +++ b/docs/content/docs/guides/1-7-upgrade-guide.mdx @@ -63,7 +63,7 @@ npx auth migrate plan npx auth migrate apply ``` -`migrate plan` only reads the database. It prints the selected and detected account identity strategies, schema changes, irreversible release actions, and blockers. Each blocker has a stable code and links to the section of this guide that resolves it. On a populated 1.6 account table, an omitted strategy produces `account-identity-strategy-required`; set the option and run the plan again. `migrate apply` derives what it can from your configuration, asks about the decisions it cannot make, and saves the reviewed answers to `better-auth-migration.json`. The file identifies the `1.6-to-1.7` transition and records the effective persisted namespaces from your configuration, so applying it under a changed configuration produces a blocker instead of silently changing account identity. A blocked run leaves the database unchanged. +`migrate plan` only reads the database. It prints the selected and detected account identity strategies, account counts, projected collisions, automatic namespace resolution, providers that need manual review, schema changes, irreversible release actions, and blockers. Each blocker has a stable code and links to the section of this guide that resolves it. On a populated 1.6 account table, an omitted strategy produces `account-identity-strategy-required`; set the option and run the plan again. `migrate apply` derives what it can from your configuration, asks about the decisions it cannot make, and saves the reviewed answers to `better-auth-migration.json`. The file identifies the `1.6-to-1.7` transition and records the effective persisted namespaces from your configuration, so applying it under a changed configuration produces a blocker instead of silently changing account identity. A blocked run leaves the database unchanged. Pass the saved file positionally to review or apply the same decisions in another environment: diff --git a/e2e/adapter/test/drizzle-adapter/migration-1-6-to-1-7.test.ts b/e2e/adapter/test/drizzle-adapter/migration-1-6-to-1-7.test.ts index 2997df1e45..ebf29d1d4a 100644 --- a/e2e/adapter/test/drizzle-adapter/migration-1-6-to-1-7.test.ts +++ b/e2e/adapter/test/drizzle-adapter/migration-1-6-to-1-7.test.ts @@ -145,6 +145,7 @@ it("plans an explicit provider-id upgrade against Drizzle snake_case columns", a expect(inspection.accountIdentity).toMatchObject({ selectedStrategy: "provider-id", detectedStrategy: "provider-id", + totalAccounts: 1, }); expect(inspection.migrationBlockers).not.toContainEqual( expect.objectContaining({ diff --git a/packages/better-auth/src/db/get-migration.test.ts b/packages/better-auth/src/db/get-migration.test.ts index afd124fb76..9c36e6f653 100644 --- a/packages/better-auth/src/db/get-migration.test.ts +++ b/packages/better-auth/src/db/get-migration.test.ts @@ -1392,6 +1392,74 @@ describe("get-migration: 1.6 account issuer resolution", () => { ); }); + it.each([ + { + expected: { + automaticNamespaceResolution: { resolved: 2, total: 2 }, + externalAccounts: 2, + manualReviewProviders: [], + projectedCollisions: 0, + totalAccounts: 3, + }, + strategy: "provider-id" as const, + }, + { + expected: { + automaticNamespaceResolution: { resolved: 2, total: 2 }, + externalAccounts: 2, + manualReviewProviders: [], + projectedCollisions: 0, + totalAccounts: 3, + }, + strategy: "issuer" as const, + }, + ])("reports $strategy namespace readiness", async ({ + expected, + strategy, + }) => { + const db = new DatabaseSync(":memory:"); + createLegacyAccountTable(db); + const config: BetterAuthOptions = { + account: { identityStrategy: strategy }, + database: db, + socialProviders: socialConfig, + }; + + const migration = await getMigrations(config, { throwOnUnsafe: false }); + + expect(migration.accountIdentity).toMatchObject(expected); + }); + + it("reports distinct projected issuer identity collisions", async () => { + const db = new DatabaseSync(":memory:"); + db.exec( + `CREATE TABLE "account" ( + "id" text primary key not null, + "accountId" text not null, + "issuer" text, + "providerId" text not null, + "userId" text not null, + "createdAt" date not null, + "updatedAt" date not null + )`, + ); + db.exec( + `INSERT INTO "account" ("id", "accountId", "issuer", "providerId", "userId", "createdAt", "updatedAt") + VALUES + ('a1', 'subject', 'https://issuer.example.com', 'custom-a', 'u1', '2020-01-01', '2020-01-01'), + ('a2', 'subject', 'https://issuer.example.com', 'custom-b', 'u2', '2020-01-01', '2020-01-01'), + ('a3', 'subject', 'https://issuer.example.com', 'custom-c', 'u3', '2020-01-01', '2020-01-01')`, + ); + const config: BetterAuthOptions = { + account: { identityStrategy: "issuer" }, + database: db, + }; + + const migration = await getMigrations(config, { throwOnUnsafe: false }); + + expect(migration.accountIdentity.projectedCollisions).toBe(1); + }); + it("keeps an already-migrated v1.7 issuer database unchanged by default", async () => { const db = new DatabaseSync(":memory:"); const publishedV17Config: BetterAuthOptions = { @@ -1862,6 +1930,45 @@ describe("get-migration: 1.6 account issuer resolution", () => { }, ]); }); + + it("reports an unconfigured SIWE provider as requiring issuer review", async () => { + const db = new DatabaseSync(":memory:"); + db.exec( + `CREATE TABLE "account" ( + "id" text primary key not null, + "accountId" text not null, + "providerId" text not null, + "userId" text not null, + "createdAt" date not null, + "updatedAt" date not null + )`, + ); + db.exec( + `INSERT INTO "account" ("id", "accountId", "providerId", "userId", "createdAt", "updatedAt") + VALUES ('a1', '0x1234', 'siwe', 'u1', '2020-01-01', '2020-01-01')`, + ); + const config: BetterAuthOptions = { + account: { identityStrategy: "issuer" }, + database: db, + }; + + const migration = await getMigrations(config, { throwOnUnsafe: false }); + + expect(migration.accountIdentity).toMatchObject({ + automaticNamespaceResolution: { resolved: 0, total: 1 }, + externalAccounts: 1, + manualReviewProviders: ["siwe"], + }); + await expect(validateMigrationFrom16(config, {})).resolves.toEqual([ + { + accountCount: 1, + code: "issuer-required", + providerId: "siwe", + reason: "unconfigured-provider", + table: "account", + }, + ]); + }); }); describe("get-migration: 1.6 plugin provider issuer resolution", () => { diff --git a/packages/better-auth/src/db/release-migration.ts b/packages/better-auth/src/db/release-migration.ts index 24f0e22e8a..645dbefa5d 100644 --- a/packages/better-auth/src/db/release-migration.ts +++ b/packages/better-auth/src/db/release-migration.ts @@ -612,6 +612,13 @@ export interface AccountIdentityMigrationAssessment { | undefined; migrationRequired: boolean; requiresRekey: boolean; + totalAccounts: number; + externalAccounts: number; + automaticNamespaceResolution?: + | { resolved: number; total: number } + | undefined; + projectedCollisions: number; + manualReviewProviders: string[]; } /** Inspects the configured account-identity path without changing the database. */ @@ -633,6 +640,10 @@ export async function inspectAccountIdentityMigration( physicalSchema, migrationRequired: false, requiresRekey: false, + totalAccounts: 0, + externalAccounts: 0, + projectedCollisions: 0, + manualReviewProviders: [], }); if (!accountSchema) return createEmptyAssessment(); const physicalSchema = { @@ -675,26 +686,28 @@ export async function inspectAccountIdentityMigration( ).length; const physicalSchemaComplete = columns.has(issuerColumn) && accountsWithIssuer === accounts.length; + const configuredIdentity = await resolveConfiguredIssuerState( + config, + "issuer", + ); + const resolveProviderKind = (providerId: string): ProviderIdentityKind => + configuredIdentity.providerKinds[providerId] ?? + (providerId === "credential" ? "local" : "external"); let detectedStrategy: AccountIdentityMigrationAssessment["detectedStrategy"]; if (!physicalSchemaComplete) { detectedStrategy = accountsWithIssuer === 0 ? "provider-id" : "mixed"; } else { - const configured = await resolveConfiguredIssuerState(config, "issuer"); let providerScopedEvidence = false; let issuerScopedEvidence = false; for (const account of accounts) { const storedIssuer = readStoredIssuer(account.issuer); if (!storedIssuer) continue; - const identityKind = - configured.providerKinds[account.providerId] ?? - (account.providerId === "credential" || account.providerId === "siwe" - ? "local" - : "external"); + const identityKind = resolveProviderKind(account.providerId); const providerScopedIssuer = createProviderScopedMigrationIssuer( account.providerId, identityKind, ); - const configuredIssuer = configured.issuers[account.providerId]; + const configuredIssuer = configuredIdentity.issuers[account.providerId]; if (configuredIssuer === providerScopedIssuer) continue; if (storedIssuer === providerScopedIssuer) { providerScopedEvidence = true; @@ -714,12 +727,60 @@ export async function inspectAccountIdentityMigration( const requiresRekey = physicalSchemaComplete && (detectedStrategy === "mixed" || detectedStrategy !== selectedStrategy); + const externalAccountRows = accounts.filter( + (account) => resolveProviderKind(account.providerId) === "external", + ); + const resolveProjectedNamespace = (account: (typeof accounts)[number]) => { + if (selectedStrategy === "provider-id") { + return createProviderScopedMigrationIssuer( + account.providerId, + resolveProviderKind(account.providerId), + ); + } + return ( + readStoredIssuer(account.issuer) || + configuredIdentity.issuers[account.providerId] + ); + }; + const projectedIdentities = new Set(); + const projectedCollisionIdentities = new Set(); + for (const account of accounts) { + const namespace = resolveProjectedNamespace(account); + if (!namespace) continue; + const identity = JSON.stringify([namespace, account.providerAccountId]); + if (projectedIdentities.has(identity)) { + projectedCollisionIdentities.add(identity); + } else { + projectedIdentities.add(identity); + } + } + const manualReviewProviders = + selectedStrategy === "issuer" + ? [ + ...new Set( + externalAccountRows + .filter((account) => !resolveProjectedNamespace(account)) + .map((account) => account.providerId), + ), + ].sort() + : []; + const automaticallyResolvedExternalAccounts = externalAccountRows.filter( + (account) => Boolean(resolveProjectedNamespace(account)), + ).length; return { selectedStrategy, detectedStrategy, physicalSchema, migrationRequired: !physicalSchemaComplete || requiresRekey, requiresRekey, + totalAccounts: accounts.length, + externalAccounts: externalAccountRows.length, + automaticNamespaceResolution: { + resolved: automaticallyResolvedExternalAccounts, + total: externalAccountRows.length, + }, + projectedCollisions: projectedCollisionIdentities.size, + manualReviewProviders, }; } diff --git a/packages/cli/src/commands/migrate.ts b/packages/cli/src/commands/migrate.ts index add7385496..5f3ec86e79 100644 --- a/packages/cli/src/commands/migrate.ts +++ b/packages/cli/src/commands/migrate.ts @@ -198,6 +198,20 @@ export function printHumanMigrationPlan( console.log( `Account identity: ${migrationPlan.accountIdentity.selectedStrategy} (database: ${migrationPlan.accountIdentity.detectedStrategy})`, ); + console.log( + `Accounts: ${migrationPlan.accountIdentity.totalAccounts} total, ${migrationPlan.accountIdentity.externalAccounts} external`, + ); + if (migrationPlan.accountIdentity.automaticIssuerResolution) { + console.log( + `Automatic issuer resolution: ${migrationPlan.accountIdentity.automaticIssuerResolution.resolved}/${migrationPlan.accountIdentity.automaticIssuerResolution.total}`, + ); + } + console.log( + `Projected collisions: ${migrationPlan.accountIdentity.projectedCollisions}`, + ); + console.log( + `Providers requiring manual issuer review: ${migrationPlan.accountIdentity.manualReviewProviders.join(", ") || "none"}`, + ); console.log( `Blockers: ${migrationPlan.blockers.map(({ code }) => code).join(", ") || "none"}`, ); diff --git a/packages/cli/src/commands/migration-interview.ts b/packages/cli/src/commands/migration-interview.ts index 4a393232c4..abce5c01dd 100644 --- a/packages/cli/src/commands/migration-interview.ts +++ b/packages/cli/src/commands/migration-interview.ts @@ -185,7 +185,7 @@ export function describeIrreversibleReleaseActions( } if ( accountIdentity.migrationRequired && - accountIdentity.detectedStrategy !== "empty" + accountIdentity.totalAccounts > 0 ) { actions.push( "write the 1.7 account identity onto every existing account row", diff --git a/packages/cli/src/commands/migration-plan.test.ts b/packages/cli/src/commands/migration-plan.test.ts index b1ede2082b..8c5f66f194 100644 --- a/packages/cli/src/commands/migration-plan.test.ts +++ b/packages/cli/src/commands/migration-plan.test.ts @@ -1,4 +1,5 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; +import { printHumanMigrationPlan } from "./migrate"; import { createMigrationPlan } from "./migration-plan"; describe("createMigrationPlan", () => { @@ -9,6 +10,11 @@ describe("createMigrationPlan", () => { detectedStrategy: "issuer", migrationRequired: true, requiresRekey: true, + totalAccounts: 1, + externalAccounts: 1, + automaticNamespaceResolution: { resolved: 1, total: 1 }, + projectedCollisions: 0, + manualReviewProviders: [], }, hasChanges: false, migrationBlockers: [ @@ -43,6 +49,11 @@ describe("createMigrationPlan", () => { detectedStrategy: "provider-id", migrationRequired: true, requiresRekey: false, + totalAccounts: 3, + externalAccounts: 2, + automaticNamespaceResolution: { resolved: 2, total: 2 }, + projectedCollisions: 0, + manualReviewProviders: [], }, hasChanges: true, migrationBlockers: [], @@ -69,4 +80,120 @@ describe("createMigrationPlan", () => { }, }); }); + + it("prints issuer namespace readiness and manual-review providers", () => { + const plan = createMigrationPlan({ + accountIdentity: { + selectedStrategy: "issuer", + detectedStrategy: "provider-id", + migrationRequired: true, + requiresRekey: false, + totalAccounts: 17, + externalAccounts: 3, + automaticNamespaceResolution: { resolved: 2, total: 3 }, + projectedCollisions: 1, + manualReviewProviders: ["custom-oidc"], + }, + hasChanges: true, + migrationBlockers: [], + migrationTarget: { adapter: "prisma", dialect: "postgres" }, + toBeAdded: [], + toBeAddedIndexes: [], + toBeCreated: [], + }); + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + + try { + printHumanMigrationPlan(plan, [], [], []); + const output = log.mock.calls + .map((parts) => parts.map(String).join(" ")) + .join("\n"); + expect(output).toContain( + "Account identity strategy: issuer (database: provider-id)", + ); + expect(output).toContain("Automatic namespace resolution: 2/3"); + expect(output).toContain("Projected collisions: 1"); + expect(output).toContain( + "Providers requiring manual issuer review: custom-oidc", + ); + expect(output).not.toContain("Persisted namespace:"); + expect(output).toContain("No database changes were applied."); + } finally { + log.mockRestore(); + } + }); + + it("prints the deterministic provider namespace branch", () => { + const plan = createMigrationPlan({ + accountIdentity: { + selectedStrategy: "provider-id", + detectedStrategy: "provider-id", + migrationRequired: true, + requiresRekey: false, + totalAccounts: 4, + externalAccounts: 3, + automaticNamespaceResolution: { resolved: 3, total: 3 }, + projectedCollisions: 0, + manualReviewProviders: [], + }, + hasChanges: true, + migrationBlockers: [], + migrationTarget: { adapter: "drizzle", dialect: "sqlite" }, + toBeAdded: [], + toBeAddedIndexes: [], + toBeCreated: [], + }); + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + + try { + printHumanMigrationPlan(plan, [], [], []); + const output = log.mock.calls + .map((parts) => parts.map(String).join(" ")) + .join("\n"); + expect(output).toContain( + "Account identity strategy: provider-id (database: provider-id)", + ); + expect(output).toContain("Automatic namespace resolution: 3/3"); + expect(output).toContain("Projected collisions: 0"); + expect(output).toContain( + "Persisted namespace: deterministic provider namespace", + ); + expect(output).not.toContain("Providers requiring manual issuer review:"); + } finally { + log.mockRestore(); + } + }); + + it("omits automatic namespace resolution when there are no external accounts", () => { + const plan = createMigrationPlan({ + accountIdentity: { + selectedStrategy: "issuer", + detectedStrategy: "provider-id", + migrationRequired: true, + requiresRekey: false, + totalAccounts: 2, + externalAccounts: 0, + automaticNamespaceResolution: { resolved: 0, total: 0 }, + projectedCollisions: 0, + manualReviewProviders: [], + }, + hasChanges: true, + migrationBlockers: [], + migrationTarget: { adapter: "drizzle", dialect: "sqlite" }, + toBeAdded: [], + toBeAddedIndexes: [], + toBeCreated: [], + }); + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + + try { + printHumanMigrationPlan(plan, [], [], []); + const output = log.mock.calls + .map((parts) => parts.map(String).join(" ")) + .join("\n"); + expect(output).not.toContain("Automatic namespace resolution:"); + } finally { + log.mockRestore(); + } + }); }); diff --git a/packages/cli/test/migrate.test.ts b/packages/cli/test/migrate.test.ts index 3939dcd405..9624a63fbe 100644 --- a/packages/cli/test/migrate.test.ts +++ b/packages/cli/test/migrate.test.ts @@ -322,6 +322,10 @@ describe("migration plan status", () => { detectedStrategy: "provider-id", migrationRequired: false, requiresRekey: false, + totalAccounts: 1, + externalAccounts: 0, + projectedCollisions: 0, + manualReviewProviders: [], }, hasChanges: false, migrationBlockers: [], @@ -345,6 +349,10 @@ describe("migration plan status", () => { detectedStrategy: "provider-id", migrationRequired: true, requiresRekey: false, + totalAccounts: 1, + externalAccounts: 0, + projectedCollisions: 0, + manualReviewProviders: [], }, hasChanges: true, migrationBlockers: [ @@ -1045,37 +1053,6 @@ describe("migrate command modes", () => { }); describe("plan every unresolved 1.6.30 release decision", () => { - it("reports the provider-id issuer backfill and an unknown SCIM inventory accurately", async () => { - const db = new Database(":memory:"); - await createReleaseDecisionFixture(db, { - identityStrategy: "provider-id", - }); - const consoleLog = vi.spyOn(console, "log").mockImplementation(() => {}); - - try { - await migrateAction({ - cwd: process.cwd(), - mode: "plan", - outputFormat: "json", - }); - - const jsonPlan = JSON.parse(String(consoleLog.mock.calls[0]?.[0])) as { - releaseMigration?: { actions: string[] }; - }; - expect(jsonPlan.releaseMigration?.actions).toContain( - "write the 1.7 account identity onto every existing account row", - ); - expect(jsonPlan.releaseMigration?.actions).toContain( - "retire 1 SCIM provider, confirm the complete provisioned-account retirement inventory, and require a full reprovision of every SCIM connection", - ); - expect(jsonPlan.releaseMigration?.actions).not.toContain( - "retire 1 SCIM provider, delete 0 provisioned accounts, and require a full reprovision of every SCIM connection", - ); - } finally { - process.exitCode = undefined; - } - }); - it("reports the consent strategy and SCIM inventory in one run", async () => { const db = new Database(":memory:"); const { scimAccountId } = await createReleaseDecisionFixture(db, {