diff --git a/.changeset/migrate-auth0.md b/.changeset/migrate-auth0.md index d1caf948b9..927125efad 100644 --- a/.changeset/migrate-auth0.md +++ b/.changeset/migrate-auth0.md @@ -2,4 +2,4 @@ "clerk": minor --- -Add `clerk migrate export auth0`, and let `clerk migrate import` read Auth0 exports, including the NDJSON file from Auth0's bulk export job. `clerk migrate` is still experimental and needs `CLERK_EXPERIMENTAL=migrate`. +Add `clerk migrate export auth0`, and let `clerk migrate import` read Auth0 exports, including the NDJSON file from Auth0's bulk export job. diff --git a/.changeset/migrate-authjs.md b/.changeset/migrate-authjs.md index d349e4a441..e5d7f27671 100644 --- a/.changeset/migrate-authjs.md +++ b/.changeset/migrate-authjs.md @@ -2,4 +2,4 @@ "clerk": minor --- -Add `clerk migrate export authjs`, and let `clerk migrate import` read Auth.js exports. The export reads an Auth.js database over `--db-url` (Postgres, MySQL, libsql/Turso or SQLite), finding the user table whether Prisma or Drizzle named it. Auth.js stores no passwords, so imported users sign in with OAuth or an email link, or set a password. The export reads each user's OAuth providers, so the import can offer to turn them on, and warns that Auth.js leaves OAuth users' emails unverified; `--trust-oauth-emails` marks them verified. `clerk migrate` is still experimental and needs `CLERK_EXPERIMENTAL=migrate`. +Add `clerk migrate export authjs`, and let `clerk migrate import` read Auth.js exports. The export reads an Auth.js database over `--db-url` (Postgres, MySQL, libsql/Turso or SQLite), finding the user table whether Prisma or Drizzle named it. Auth.js stores no passwords, so imported users sign in with OAuth or an email link, or set a password. The export reads each user's OAuth providers, so the import can offer to turn them on, and warns that Auth.js leaves OAuth users' emails unverified; `--trust-oauth-emails` marks them verified. diff --git a/.changeset/migrate-betterauth.md b/.changeset/migrate-betterauth.md index dc242bb401..e55120b8fa 100644 --- a/.changeset/migrate-betterauth.md +++ b/.changeset/migrate-betterauth.md @@ -2,4 +2,4 @@ "clerk": minor --- -Add `clerk migrate export betterauth`, and let `clerk migrate import` read Better Auth exports. The export reads a Better Auth database over `--db-url` (Postgres, MySQL, libsql/Turso or SQLite), including schemas Drizzle generated with snake_case columns or plural tables, and detects plugin columns. Better Auth's scrypt, bcrypt and argon2 passwords carry across, and anonymous users are skipped. A user any source skips, such as Supabase's soft-deleted users, no longer stops the import: it's recorded as skipped, and `--allow-partial` is only for real rejects. `clerk migrate` is still experimental and needs `CLERK_EXPERIMENTAL=migrate`. +Add `clerk migrate export betterauth`, and let `clerk migrate import` read Better Auth exports. The export reads a Better Auth database over `--db-url` (Postgres, MySQL, libsql/Turso or SQLite), including schemas Drizzle generated with snake_case columns or plural tables, and detects plugin columns. Better Auth's scrypt, bcrypt and argon2 passwords carry across, and anonymous users are skipped. A user any source skips, such as Supabase's soft-deleted users, no longer stops the import: it's recorded as skipped, and `--allow-partial` is only for real rejects. diff --git a/.changeset/migrate-custom-sources.md b/.changeset/migrate-custom-sources.md index fb9542f26d..153606ec08 100644 --- a/.changeset/migrate-custom-sources.md +++ b/.changeset/migrate-custom-sources.md @@ -2,4 +2,4 @@ "clerk": minor --- -Add `clerk migrate sources`, and let `clerk migrate import --source` take the path to a source you wrote. `clerk migrate sources` lists the built-in sources with the passwords, MFA and metadata each brings across, and `clerk migrate sources ` shows where each field lands, how to export, and any caveats. A source you wrote is a `.ts` or `.js` file in your project, checked before use, so a platform with no built-in source can be imported without rebuilding the CLI. `clerk migrate` is still experimental and needs `CLERK_EXPERIMENTAL=migrate`. +Add `clerk migrate sources`, and let `clerk migrate import --source` take the path to a source you wrote. `clerk migrate sources` lists the built-in sources with the passwords, MFA and metadata each brings across, and `clerk migrate sources ` shows where each field lands, how to export, and any caveats. A source you wrote is a `.ts` or `.js` file in your project, checked before use, so a platform with no built-in source can be imported without rebuilding the CLI. diff --git a/.changeset/migrate-export.md b/.changeset/migrate-export.md index df70f88daa..e2b3b2d991 100644 --- a/.changeset/migrate-export.md +++ b/.changeset/migrate-export.md @@ -2,4 +2,4 @@ "clerk": minor --- -Add `clerk migrate export clerk` and `clerk migrate export supabase`, and let `clerk migrate import` take the export's run ID in place of a file and `--source`. `clerk migrate` is still experimental and needs `CLERK_EXPERIMENTAL=migrate`. +Add `clerk migrate export clerk` and `clerk migrate export supabase`, and let `clerk migrate import` take the export's run ID in place of a file and `--source`. diff --git a/.changeset/migrate-firebase.md b/.changeset/migrate-firebase.md index bef7d6bf18..12e5a4b8f2 100644 --- a/.changeset/migrate-firebase.md +++ b/.changeset/migrate-firebase.md @@ -2,4 +2,4 @@ "clerk": minor --- -Add `clerk migrate export firebase`, and let `clerk migrate import` read Firebase exports with their password hashes. The project's hash parameters come from the export, the `--firebase-*` flags, or `CLERK_FIREBASE_SIGNER_KEY`, `CLERK_FIREBASE_SALT_SEPARATOR`, `CLERK_FIREBASE_ROUNDS` and `CLERK_FIREBASE_MEM_COST`. `clerk migrate` is still experimental and needs `CLERK_EXPERIMENTAL=migrate`. +Add `clerk migrate export firebase`, and let `clerk migrate import` read Firebase exports with their password hashes. The project's hash parameters come from the export, the `--firebase-*` flags, or `CLERK_FIREBASE_SIGNER_KEY`, `CLERK_FIREBASE_SALT_SEPARATOR`, `CLERK_FIREBASE_ROUNDS` and `CLERK_FIREBASE_MEM_COST`. diff --git a/.changeset/migrate-runs-undo.md b/.changeset/migrate-runs-undo.md index 2c498fb32e..8f3d4dd307 100644 --- a/.changeset/migrate-runs-undo.md +++ b/.changeset/migrate-runs-undo.md @@ -2,6 +2,6 @@ "clerk": minor --- -Add `clerk migrate runs` and `clerk migrate undo`, and let a re-run of `clerk migrate import` continue where a stopped one left off. `clerk migrate` is still experimental and needs `CLERK_EXPERIMENTAL=migrate`. +Add `clerk migrate runs` and `clerk migrate undo`, and let a re-run of `clerk migrate import` continue where a stopped one left off. Every user an import creates now carries `clerkMigrateRun` (the run's ID) in its `private_metadata`, replacing any value the source had under that key. A re-run reads it to adopt a user whose create was cut off. Nothing removes it afterwards, and your backend can read it. diff --git a/.changeset/migrate-workos.md b/.changeset/migrate-workos.md index 1232d1da84..9001cc5d4e 100644 --- a/.changeset/migrate-workos.md +++ b/.changeset/migrate-workos.md @@ -2,4 +2,4 @@ "clerk": minor --- -Add `clerk migrate export workos`, and let `clerk migrate import` read WorkOS exports. WorkOS returns no password hashes, so imported users reset their password or sign in with SSO. `clerk init` now also flags a WorkOS install and links the migration guide. `clerk migrate` is still experimental and needs `CLERK_EXPERIMENTAL=migrate`. +Add `clerk migrate export workos`, and let `clerk migrate import` read WorkOS exports. WorkOS returns no password hashes, so imported users reset their password or sign in with SSO. `clerk init` now also flags a WorkOS install and links the migration guide. diff --git a/.changeset/migrate.md b/.changeset/migrate.md new file mode 100644 index 0000000000..f8bf153c32 --- /dev/null +++ b/.changeset/migrate.md @@ -0,0 +1,9 @@ +--- +"clerk": minor +--- + +Make `clerk migrate` available without `CLERK_EXPERIMENTAL=migrate`: it now shows in `clerk --help` and shell completion. + +- `clerk migrate export` gets users out of Clerk, Supabase, Firebase, Auth0, WorkOS, Better Auth or Auth.js into a run, and `clerk migrate import` checks every user against your instance, asks before it writes, and continues a run that stopped partway. +- `clerk migrate runs` shows what each run did, `clerk migrate undo` deletes the users an import created, and `clerk migrate sources` shows what each source brings across. `--source ./my-source.ts` imports with a source you wrote. +- `clerk migrate import --reserve-unverified` creates the emails and phones a source never verified as reserved (usable for sign-in, locked to the user) instead of unverified. At a terminal, the import asks. diff --git a/README.md b/README.md index d9c3523e5b..d6bbeb4d74 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,7 @@ Commands: init [options] Initialize Clerk in your project link [options] Link this project to a Clerk application mcp Manage the Clerk remote MCP server connection for AI editors and CLIs + migrate Migrate users into Clerk from another auth provider or another Clerk instance open Open Clerk resources in your browser telemetry Control CLI usage telemetry (status, disable, enable) unlink [options] Unlink this project from its Clerk application diff --git a/packages/cli-core/src/commands/migrate/README.md b/packages/cli-core/src/commands/migrate/README.md index da82d4516f..c9facb42e3 100644 --- a/packages/cli-core/src/commands/migrate/README.md +++ b/packages/cli-core/src/commands/migrate/README.md @@ -1,22 +1,11 @@ # `clerk migrate` -> **Experimental.** `clerk migrate` is hidden from help and completion, and -> refuses to run (exit 2, `experiment_disabled`), unless `CLERK_EXPERIMENTAL` -> includes `migrate`: -> -> ```sh -> CLERK_EXPERIMENTAL=migrate clerk migrate import --help -> ``` -> -> `CLERK_EXPERIMENTAL` is a comma-separated list. Names are trimmed and -> case-insensitive, and unknown names are ignored. - Migrate users into a Clerk instance from another auth provider, or from another Clerk instance. ``` -clerk migrate export [source] [-o ] [--json] -clerk migrate import [--source ] [--dry-run] [--allow-partial] [--new-run] [--yes] [--json] +clerk migrate export [platform] [-o ] [--json] +clerk migrate import [file|export-run-id] [--source ] [--dry-run] [--allow-partial] [--new-run] [--yes] [--json] clerk migrate runs [run-id] [--json] clerk migrate undo [--dry-run] [--yes] [--include-signed-in] [--json] clerk migrate sources [source] [--json] @@ -32,8 +21,8 @@ clerk migrate import 20260929-141502-a1b2 --yes # 3. import it ``` `clerk migrate undo ` takes an import back out, and `clerk migrate runs` -shows what every run did. Every subcommand takes `--runs-dir ` (or -`CLERK_MIGRATE_DIR`) to keep its runs somewhere else. +shows what every run did. Every subcommand but `sources` takes `--runs-dir ` +(or `CLERK_MIGRATE_DIR`) to keep its runs somewhere else. `clerk migrate` on its own is a group name, not a command: it prints its help. ## The rules @@ -310,7 +299,7 @@ keep `--runs-dir`: the import looks the run ID up there. | Flag | Platforms | Description | | -------------------------- | ---------------------------------- | ----------------------------------------------------------------- | | `-o, --output ` | all | Write the export here instead of the run folder | -| `-y, --yes` | all | Do not prompt: fail on a bad credential | +| `-y, --yes` | all | Do not prompt: fail on a rejected credential | | `--json` | all | Print the result as JSON; never prompts | | `--runs-dir ` | all | Where runs are kept (see [the run store](#the-run-store)) | | `--db-url ` | `supabase`, `authjs`, `betterauth` | Connection string (Postgres only for `supabase`) | @@ -436,7 +425,9 @@ only opens local files and `@libsql/client` ships native optional dependencies. Nothing native ships in the binary — that is the whole reason the `engines.bun` floor exists. Resolution is `--db-url`, then `SUPABASE_DB_URL` / `AUTHJS_DB_URL` / `BETTERAUTH_DB_URL`, then a masked prompt, since a connection string carries the password inline. A password -pasted unencoded (`#`, `@`, `/` and the like) is percent-encoded for you. +pasted unencoded (`#`, `@`, `/` and the like) is percent-encoded for you. A libsql +token comes from `?authToken=` on the URL, or from `TURSO_AUTH_TOKEN` / +`LIBSQL_AUTH_TOKEN`, and is redacted like a password. **Connection strings are redacted everywhere.** Errors show `postgres://***@host/db`, including when the password itself contains an @@ -697,6 +688,7 @@ clerk migrate import users.json --source clerk --new-run --yes clerk migrate import users.json --source clerk --json --yes clerk migrate import users.json --source clerk --require-password --yes clerk migrate import users.json --source clerk --skip-legal-checks --yes +clerk migrate import users.json --source workos --reserve-unverified --yes clerk migrate import users.json --source firebase --firebase-signer-key SIGNER_KEY \ --firebase-salt-separator SALT_SEPARATOR --firebase-rounds 8 --firebase-mem-cost 14 --yes clerk migrate import users.json --source clerk --runs-dir ./runs --yes @@ -714,6 +706,7 @@ clerk migrate import # a human is asked | `--new-run` | Start a new run instead of [continuing](#re-running) an earlier one | | `--require-password` | Import only users that carry a password digest | | `--skip-legal-checks` | Import users with no legal acceptance into an instance requiring it | +| `--reserve-unverified` | Create [unverified identifiers](#verified-vs-unverified-identifiers) reserved, not unverified | | `--firebase-signer-key ` | Firebase base64 signer key (overrides the export file), or `CLERK_FIREBASE_SIGNER_KEY` (does not) | | `--firebase-salt-separator ` | Firebase base64 salt separator, or `CLERK_FIREBASE_SALT_SEPARATOR` | | `--firebase-rounds ` | Firebase scrypt rounds, or `CLERK_FIREBASE_ROUNDS` | @@ -843,10 +836,11 @@ after them. They sort the users four ways: any other user, with a warning. Spaces around a phone don't count, as Clerk trims them. Phones the instance has off are stripped anyway, so they are not checked - - it lacks an identifier the instance requires. An email or phone counts - only when it is verified, because an unverified one is attached after the - user exists. A required phone dropped for not being in E.164 form is named - as such + - it lacks an identifier the instance requires. An email or phone counts when + it is verified, or when it is created reserved (`--reserve-unverified`, or + an adopted user's recorded mode). An unverified one attached after the user + exists does not. A required phone dropped for not being in E.164 form is + named as such - it has no identifier left once the emails and phones Clerk would refuse are stripped: those of an instance that neither has them on nor signs in or does MFA with them. A username is kept, as Clerk stores it with @@ -871,9 +865,9 @@ after them. They sort the users four ways: to 15, `scrypt_firebase`, `argon2i`/`argon2id` and `scrypt_werkzeug` are checked; other hashers are not, and Clerk refuses a bad one at create) - Supabase: its only providers are ones Clerk has off, or doesn't offer at all - (Figma, Kakao, Keycloak, WorkOS, Zoom, Fly), and it has no verified email or - phone the instance signs in with by code or link. The checks offer to turn - on the first kind; nothing can turn on the second + (Figma, Kakao, Keycloak, WorkOS, Zoom, Fly), and it has no verified or + reserved email or phone the instance signs in with by code or link. The + checks offer to turn on the first kind; nothing can turn on the second - its source ID, primary email, primary phone or username repeats an earlier user in the file that passes the checks above. Only what the create sends counts: an extra email is attached after it, and a stripped @@ -954,6 +948,12 @@ email: the create is retried without the phone. The user counts as imported, and the summary lists them under "Imported without their phone", by Clerk's reason (`result.warnings` in `--json`). +With [reserved](#verified-vs-unverified-identifiers) phones on the create, the +refused one may be any of them, so the verified phone is retried alone first. +Reserved phones the refusal kept off, when Clerk's answer can't say which was +refused, are attached one by one once the user exists, unverified: only the +refused one is lost. + #### Throughput Defaults follow Clerk's documented `POST /v1/users` limits: 100 req/s for @@ -1326,6 +1326,35 @@ unconfirmed address there would silently promote it. A Clerk export keeps an unverified primary email or phone unverified. +**Unverified or reserved.** By default an unverified identifier is attached +after the user exists (`POST /v1/email_addresses` or `POST /v1/phone_numbers`, +`verified: false`). The user +cannot sign in with it, and another user can claim it by verifying it first. +`--reserve-unverified`, or a yes at the prompt a human gets when the file has +any, creates them **reserved** instead, on `POST /v1/users` through +`email_address_identification_status` / `phone_number_identification_status`. +A reserved identifier is unverified, but usable for sign-in and locked to the +user, and becomes verified the first time the user signs in with it. That is +how most source platforms treat an unconfirmed address, but it lets the user +sign in with one nobody proved they own, so it is opt-in. `-y`, `--json` and +agent mode never ask, and keep them unverified without the flag. + +A run records its choice (`reserveUnverified` in `run.json`), and a continued +run reuses it without asking, so every user of one run is created the same way. +A run recorded before the choice existed asks once, and records the answer. +`--reserve-unverified` on a continue of a run that kept them unverified is +refused; pass `--new-run` to change modes. A user a run adopts keeps the mode +its first create recorded (`reserved` on its `creating` line). When Clerk +refused a phone and the retry went out without the reserved phones, that line +lists them as `pending`, and a continue that adopts the user attaches them, +unverified (one Clerk refuses again fails with a note). + +Reserved, an unverified identifier is checked for duplicates like a verified +one, because it goes on the user's create: one that an instance user or an +earlier record in the file that passes the checks already holds rejects the +whole user, where without reserving only that address would fail to attach, with +a note. If that rejects users you want, import without reserving. + ### Firebase hash parameters Firebase uses a modified scrypt, so Clerk needs the project's four parameters diff --git a/packages/cli-core/src/commands/migrate/gate.test.ts b/packages/cli-core/src/commands/migrate/gate.test.ts deleted file mode 100644 index 7179fc22a2..0000000000 --- a/packages/cli-core/src/commands/migrate/gate.test.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { useIntegrationTestHarness, clerk } from "../../test/integration/lib/harness.ts"; -import { generateCompletions } from "../completion/__complete.ts"; -import { createProgram } from "../../cli-program.ts"; - -useIntegrationTestHarness(); - -let saved: string | undefined; -beforeEach(() => { - saved = process.env.CLERK_EXPERIMENTAL; - delete process.env.CLERK_EXPERIMENTAL; -}); -afterEach(() => { - if (saved === undefined) delete process.env.CLERK_EXPERIMENTAL; - else process.env.CLERK_EXPERIMENTAL = saved; -}); - -describe("with CLERK_EXPERIMENTAL unset", () => { - test("clerk --help leaves migrate out", () => { - expect(createProgram().helpInformation()).not.toContain("migrate"); - }); - - test.each([ - [["migrate"]], - [["migrate", "--help"]], - [["migrate", "import", "users.json", "--yes"]], - [["migrate", "--not-a-flag"]], - [["help", "migrate"]], - [["--verbose", "migrate", "import", "x.json"]], - ])("clerk %p exits 2 with experiment_disabled", async (args) => { - const result = await clerk.raw("--mode", "agent", ...args); - expect(result.exitCode).toBe(2); - expect(result.stderr).toContain('"code":"experiment_disabled"'); - expect(result.stderr).toContain("CLERK_EXPERIMENTAL=migrate"); - }); - - test("completion offers no subcommands under migrate", () => { - const names = generateCompletions(createProgram(), ["migrate", ""]).completions.map( - (c) => c.name, - ); - expect(names.filter((name) => !name.startsWith("-"))).toEqual([]); - }); -}); - -describe("with CLERK_EXPERIMENTAL=migrate", () => { - test("clerk --help lists migrate", () => { - process.env.CLERK_EXPERIMENTAL = "migrate"; - expect(createProgram().helpInformation()).toContain("migrate"); - }); -}); diff --git a/packages/cli-core/src/commands/migrate/import-users.test.ts b/packages/cli-core/src/commands/migrate/import-users.test.ts index 9a0af5d242..fd15e6db34 100644 --- a/packages/cli-core/src/commands/migrate/import-users.test.ts +++ b/packages/cli-core/src/commands/migrate/import-users.test.ts @@ -84,6 +84,42 @@ describe("buildCreateUserBody", () => { ]); }); + // Only the create can make a reserved identifier, so it goes here, after the + // verified primary, with a status per address in the same order. + test("sends unverified identifiers reserved when asked, in order", () => { + const target = user({ + email: "a@x.dev", + unverifiedEmailAddresses: ["c@x.dev"], + unverifiedPhoneNumbers: ["+15555550101"], + }); + expect(buildCreateUserBody(target, splitIdentifiers(target), true, true)).toMatchObject({ + email_address: ["a@x.dev", "c@x.dev"], + email_address_identification_status: ["verified", "reserved"], + phone_number: ["+15555550101"], + phone_number_identification_status: ["reserved"], + }); + }); + + test("makes a reserved email the primary when there is no verified one", () => { + const target = user({ email: undefined, unverifiedEmailAddresses: ["c@x.dev"] }); + expect(buildCreateUserBody(target, splitIdentifiers(target), true, true)).toMatchObject({ + email_address: ["c@x.dev"], + email_address_identification_status: ["reserved"], + }); + }); + + test("sends no status arrays by default, or with nothing to reserve", () => { + const unverified = user({ unverifiedEmailAddresses: ["c@x.dev"] }); + const body = buildCreateUserBody(unverified, splitIdentifiers(unverified), true); + expect(body.email_address).toEqual(["a@x.dev"]); + expect(body).not.toHaveProperty("email_address_identification_status"); + + const verified = user(); + expect( + buildCreateUserBody(verified, splitIdentifiers(verified), true, true), + ).not.toHaveProperty("email_address_identification_status"); + }); + // Allowlists and blocklists police sign-ups; these users already signed up. test("skips the instance's sign-up restrictions", () => { expect(buildCreateUserBody(user(), splitIdentifiers(user()), true)).toMatchObject({ @@ -333,6 +369,29 @@ describe("importUsers", () => { expect(requests.filter((r) => r.url.endsWith("/v1/phone_numbers"))).toHaveLength(1); }); + test("creates unverified identifiers reserved rather than attaching them", async () => { + stub(() => ok("user_created")); + + await importUsers({ + users: [user({ email: ["a@x.dev", "b@x.dev"], unverifiedEmailAddresses: ["c@x.dev"] })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + reserveUnverified: true, + }); + + expect(requests.find((r) => r.url.endsWith("/v1/users"))?.body).toMatchObject({ + email_address: ["a@x.dev", "c@x.dev"], + email_address_identification_status: ["verified", "reserved"], + }); + expect( + requests.filter((r) => r.url.endsWith("/v1/email_addresses")).map((r) => r.body), + ).toEqual([ + { user_id: "user_created", email_address: "b@x.dev", primary: false, verified: true }, + ]); + expect(lines.at(-1)).not.toHaveProperty("pending"); + }); + test("marks a user whose password the source dropped", async () => { stub(() => ok("user_created")); @@ -558,6 +617,85 @@ describe("importUsers", () => { expect(lines.at(-1)).toMatchObject({ clerkId: "user_found", status: "created" }); }); + // The `creating` line says what the create did, so a continued run that + // adopts the user knows what is left to attach. + test("marks a creating line reserved only when the create reserved something", async () => { + stub(() => ok("user_created")); + + await importUsers({ + users: [ + user({ userId: "u1", unverifiedEmailAddresses: ["c@x.dev"] }), + user({ userId: "u2", email: "b@x.dev" }), + ], + secretKey: "sk_test_x", + limits: LIMITS, + record, + reserveUnverified: true, + }); + + expect(allLines.filter((line) => line.status === "creating")).toEqual([ + { sourceId: "u1", status: "creating", reserved: true }, + { sourceId: "u2", status: "creating" }, + ]); + }); + + // Adopted, the user is not created again, so its create's mode decides what + // is still missing, not this run's flag. + test("attaches an adopted user's unverified email its create left out, even with the flag", async () => { + stub(() => ok("idn_1")); + + await importUsers({ + users: [user({ unverifiedEmailAddresses: ["c@x.dev"] })], + adopted: new Map([["u1", "user_found"]]), + secretKey: "sk_test_x", + limits: LIMITS, + record, + reserveUnverified: true, + }); + + expect(requests.map((r) => r.body)).toEqual([ + { user_id: "user_found", email_address: "c@x.dev", primary: false, verified: false }, + ]); + }); + + test("attaches nothing for an adopted user whose create reserved them, even without the flag", async () => { + stub(() => ok("idn_1")); + + await importUsers({ + users: [user({ unverifiedEmailAddresses: ["c@x.dev"] })], + adopted: new Map([["u1", "user_found"]]), + adoptedReserved: new Set(["u1"]), + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(requests).toEqual([]); + }); + + // The retry that dropped them was cut off before its answer: the reserved + // phones it left out are attached, unverified, as they would have been. + test("attaches the reserved phones an adopted user's retried create left out", async () => { + stub(() => ok("idn_1")); + + await importUsers({ + users: [ + user({ unverifiedEmailAddresses: ["c@x.dev"], unverifiedPhoneNumbers: ["+31612345678"] }), + ], + adopted: new Map([["u1", "user_found"]]), + adoptedReserved: new Set(["u1"]), + adoptedPending: new Map([["u1", ["+31612345678"]]]), + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + // The email went on the create reserved, so it is not attached again. + expect(requests.map((r) => r.body)).toEqual([ + { user_id: "user_found", phone_number: "+31612345678", primary: false, verified: false }, + ]); + }); + // Shapes from clerk_go's apierror: the country error carries its own code // and no param_name; the E.164 error is a form error on phone_number. test.each([ @@ -604,6 +742,138 @@ describe("importUsers", () => { ); }); + test("drops a refused phone's statuses with it", async () => { + stub((url, attempt) => + url.endsWith("/v1/users") && attempt === 1 + ? new Response( + JSON.stringify({ + errors: [ + { + code: "form_param_format_invalid", + message: "bad phone", + meta: { param_name: "phone_number" }, + }, + ], + }), + { status: 422 }, + ) + : ok("user_created"), + ); + + await importUsers({ + users: [user({ unverifiedPhoneNumbers: ["+31612345678"] })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + reserveUnverified: true, + }); + + expect(requests[0]?.body).toHaveProperty("phone_number_identification_status"); + expect(requests[1]?.body).not.toHaveProperty("phone_number"); + expect(requests[1]?.body).not.toHaveProperty("phone_number_identification_status"); + expect(lines.at(-1)?.error).toContain("Failed to add phone +31612345678"); + }); + + describe("a refused phone among reserved ones", () => { + const refusePhone = (refusals: number) => + stub((url, attempt) => + url.endsWith("/v1/users") && attempt <= refusals + ? new Response( + JSON.stringify({ + errors: [ + { + code: "form_param_format_invalid", + message: "bad phone", + meta: { param_name: "phone_number" }, + }, + ], + }), + { status: 422 }, + ) + : ok("user_created"), + ); + const withPhones = (fields: Partial = {}) => + user({ phone: "+15555550100", unverifiedPhoneNumbers: ["+31612345678"], ...fields }); + + // Any of the phones may be the refused one; the verified one goes alone first. + test("keeps the verified phone, dropping only the reserved ones", async () => { + refusePhone(1); + await importUsers({ + users: [withPhones()], + secretKey: "sk_test_x", + limits: LIMITS, + record, + reserveUnverified: true, + }); + + expect(requests[1]?.body).toMatchObject({ phone_number: ["+15555550100"] }); + expect(requests[1]?.body).not.toHaveProperty("phone_number_identification_status"); + // Recorded before the retry goes out, so a continue can attach them. + const creating = allLines.filter((line) => line.status === "creating"); + expect(creating[0]).not.toHaveProperty("pending"); + expect(creating[1]).toMatchObject({ + reserved: true, + pending: [{ kind: "phone", value: "+31612345678", verified: false }], + }); + expect(lines.at(-1)?.error).toContain("Failed to add phone +31612345678:"); + expect(lines.at(-1)?.error).not.toContain("+15555550100"); + }); + + test("creates a user with no email on its verified phone", async () => { + refusePhone(1); + const summary = await importUsers({ + users: [withPhones({ email: undefined })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + reserveUnverified: true, + }); + + expect(summary).toMatchObject({ successful: 1, failed: 0 }); + }); + + // The first refusal may have been the verified phone's alone. + test("drops the verified phone refused alone, and attaches the reserved ones after", async () => { + refusePhone(2); + await importUsers({ + users: [withPhones()], + secretKey: "sk_test_x", + limits: LIMITS, + record, + reserveUnverified: true, + }); + + expect(requests[2]?.body).not.toHaveProperty("phone_number"); + expect(lines.at(-1)?.error).toContain("Failed to add phone +15555550100:"); + expect(requests[3]).toMatchObject({ + url: expect.stringContaining("/v1/phone_numbers"), + body: { phone_number: "+31612345678", verified: false }, + }); + }); + + // Any one of them may be the refused one, so none is lost with it. + test("attaches each reserved phone on its own when it can't tell which was refused", async () => { + refusePhone(1); + await importUsers({ + users: [withPhones({ unverifiedPhoneNumbers: ["+31612345678", "+31612345679"] })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + reserveUnverified: true, + }); + + expect(requests[1]?.body).toMatchObject({ phone_number: ["+15555550100"] }); + const attached = requests + .filter((request) => request.url.includes("/v1/phone_numbers")) + .map((request) => request.body); + expect(attached).toEqual([ + expect.objectContaining({ phone_number: "+31612345678", verified: false }), + expect.objectContaining({ phone_number: "+31612345679", verified: false }), + ]); + expect(lines.at(-1)?.error ?? "").not.toContain("Failed to add phone"); + }); + }); + test("does not retry without the phone when it is the only identifier", async () => { stub( () => diff --git a/packages/cli-core/src/commands/migrate/import-users.ts b/packages/cli-core/src/commands/migrate/import-users.ts index 67c37867b8..f08acce91d 100644 --- a/packages/cli-core/src/commands/migrate/import-users.ts +++ b/packages/cli-core/src/commands/migrate/import-users.ts @@ -75,23 +75,59 @@ export function splitIdentifiers(user: User): Identifiers { }; } +/** + * The values and parallel statuses for one identifier kind on `POST /v1/users`. + * + * A reserved identifier is unverified, but usable for sign-in and locked to + * this user. Only the create can make one, so with `reserve` the unverified + * identifiers go here instead of being attached afterwards. + */ +function createIdentifiers( + verified: string | undefined, + unverified: string[], + reserve: boolean, +): { values: string[]; statuses?: string[] } { + const values = [...(verified ? [verified] : []), ...(reserve ? unverified : [])]; + if (!reserve || unverified.length === 0) return { values }; + return { + values, + statuses: values.map((_, index) => (verified && index === 0 ? "verified" : "reserved")), + }; +} + /** * Builds the `POST /v1/users` request body. * * Optional fields are omitted rather than sent as null so Clerk applies its own * defaults for anything the source platform did not record. + * + * @param reserveUnverified - Create the identifiers the source never verified + * as reserved, in this request, rather than attaching them unverified. */ export function buildCreateUserBody( user: User, identifiers: Identifiers, skipPasswordRequirement: boolean, + reserveUnverified = false, ): Record { // The instance's allowlist, blocklist, disposable-email and subaddress rules // police sign-ups. These users already signed up, on the source platform. const body: Record = { external_id: user.userId, skip_restriction_checks: true }; - if (identifiers.primaryEmail) body.email_address = [identifiers.primaryEmail]; - if (identifiers.primaryPhone) body.phone_number = [identifiers.primaryPhone]; + const emails = createIdentifiers( + identifiers.primaryEmail, + identifiers.unverifiedEmails, + reserveUnverified, + ); + if (emails.values.length > 0) body.email_address = emails.values; + if (emails.statuses) body.email_address_identification_status = emails.statuses; + const phones = createIdentifiers( + identifiers.primaryPhone, + identifiers.unverifiedPhones, + reserveUnverified, + ); + if (phones.values.length > 0) body.phone_number = phones.values; + if (phones.statuses) body.phone_number_identification_status = phones.statuses; if (user.firstName) body.first_name = user.firstName; if (user.lastName) body.last_name = user.lastName; if (user.username) body.username = user.username; @@ -137,8 +173,23 @@ type CreateContext = { halt: AbortSignal; /** Aborted by a Ctrl-C, the quota or a halt. No create goes out after it. */ stop: AbortSignal; + /** Unverified identifiers go on the create as reserved, not attached after. */ + reserveUnverified: boolean; }; +/** + * True when Clerk refused the create for a phone. The country error names no + * parameter, only its own code. A phone another user holds also names + * `phone_number`, and fails the user. + */ +function isPhoneRefusal(error: unknown): error is BapiError { + return ( + error instanceof BapiError && + (error.code === "unsupported_country_code" || + (error.code === "form_param_format_invalid" && error.meta?.param_name === "phone_number")) + ); +} + /** A request a stop kept from going out: neither a failure nor unknown. */ class NotSentError extends Error {} @@ -153,8 +204,19 @@ export function outcomeUnknown(error: unknown): boolean { return true; } -/** The extra identifiers a user carries, in the order they are attached. */ -export function pendingIdentifiers(identifiers: Identifiers): PendingIdentifier[] { +/** + * The extra identifiers a user carries, in the order they are attached. + * + * @param reserveUnverified - The unverified ones went on the create as + * reserved, so there is nothing to attach for them. + */ +export function pendingIdentifiers( + identifiers: Identifiers, + reserveUnverified = false, +): PendingIdentifier[] { + if (reserveUnverified) { + identifiers = { ...identifiers, unverifiedEmails: [], unverifiedPhones: [] }; + } return [ ...identifiers.additionalEmails.map((value) => ({ kind: "email" as const, @@ -244,28 +306,37 @@ async function attachAll( }; } +type CreatedUser = { + clerkUserId: string; + notes: string[]; + /** Reserved phones a refusal kept off the create, to attach one by one. */ + attachLater?: string[]; + phoneRefusal?: string; +}; + /** * Creates one user, retrying without a phone Clerk refuses. * * @param sending - Called as each `POST /v1/users` goes out, so the run * records the user only once a create may actually land. - * @returns The Clerk ID, a note when the phone was dropped, and Clerk's - * reason for refusing it. + * @returns The Clerk ID, a note when the phone was dropped, Clerk's reason + * for refusing it, and the reserved phones still to attach. */ async function createUser( ctx: CreateContext, user: User, identifiers: Identifiers, skipPasswordRequirement: boolean, - sending: () => void, -): Promise<{ clerkUserId: string; notes: string[]; phoneRefusal?: string }> { + /** Called as each attempt goes out, with what it sends. */ + sending: (body: Record) => void, +): Promise { // A Ctrl-C or a full instance hands the slot on to queued creates; none // of them is sent, and none waits for a paced turn to find that out. const create = async (body: Record) => ctx.schedule( async () => { if (ctx.stop.aborted) throw new NotSentError(); - sending(); + sending(body); return bapiRequest({ method: "POST", path: "/v1/users", @@ -276,7 +347,12 @@ async function createUser( { stop: ctx.stop }, ); - const body = buildCreateUserBody(user, identifiers, skipPasswordRequirement); + const body = buildCreateUserBody( + user, + identifiers, + skipPasswordRequirement, + ctx.reserveUnverified, + ); // The run's marker, with whatever private metadata the source brought: // without it a create cut off mid-flight could never be told from a user // someone else made with the same external_id. It replaces a marker the @@ -289,23 +365,48 @@ async function createUser( } const notes: string[] = []; let phoneRefusal: string | undefined; + let attachLater: string[] = []; let response; try { response = await create(body); } catch (error) { // A phone Clerk refuses (a country it does not support, a number that is // not E.164) should not cost a user who has an email to be created under. - // The country error names no parameter, only its own code. A phone another - // user holds also names `phone_number`, and fails the user. - const phoneRefused = - error instanceof BapiError && - (error.code === "unsupported_country_code" || - (error.code === "form_param_format_invalid" && error.meta?.param_name === "phone_number")); - if (!phoneRefused || !identifiers.primaryEmail) throw error; - const { phone_number: _dropped, ...withoutPhone } = body; - response = await create(withoutPhone); - phoneRefusal = error.longMessage ?? error.message; - notes.push(`Failed to add phone ${identifiers.primaryPhone}: ${phoneRefusal}`); + if (!isPhoneRefusal(error)) throw error; + const { + phone_number: sent, + phone_number_identification_status: statuses, + ...withoutPhone + } = body; + let refusal = error; + let dropped = sent as string[]; + // With reserved phones on the create, any one of them may be the refused + // one: the verified phone alone goes first, so it is not lost with them. + if (statuses && identifiers.primaryPhone) { + try { + response = await create({ ...withoutPhone, phone_number: [identifiers.primaryPhone] }); + dropped = dropped.filter((phone) => phone !== identifiers.primaryPhone); + } catch (retryError) { + if (!isPhoneRefusal(retryError)) throw retryError; + refusal = retryError; + } + } + if (!response) { + if (!body.email_address) throw refusal; + response = await create(withoutPhone); + } + // Which reserved phone was refused is unknown when there are several, or + // when the verified one was refused alone: each is attached on its own once + // the user exists, unverified, so only a refused one is lost. + const reservedDropped = dropped.filter((phone) => phone !== identifiers.primaryPhone); + if (statuses && (refusal !== error || reservedDropped.length > 1)) { + attachLater = reservedDropped; + dropped = dropped.filter((phone) => !attachLater.includes(phone)); + } + if (dropped.length > 0) { + phoneRefusal = refusal.longMessage ?? refusal.message; + notes.push(`Failed to add phone ${dropped.join(", ")}: ${phoneRefusal}`); + } } // Untracked, the user could never be undone. Thrown, the outcome is unknown, @@ -314,7 +415,12 @@ async function createUser( if (typeof clerkUserId !== "string" || !clerkUserId) { throw new Error("Clerk answered POST /v1/users without a user ID"); } - return { clerkUserId, notes, ...(phoneRefusal ? { phoneRefusal } : {}) }; + return { + clerkUserId, + notes, + attachLater, + ...(phoneRefusal ? { phoneRefusal } : {}), + }; } export type ImportUsersOptions = { @@ -336,8 +442,22 @@ export type ImportUsersOptions = { * not created again; only their extra identifiers are sent. */ adopted?: Map; + /** + * The adopted users whose create reserved their unverified identifiers, from + * its `creating` line. The rest had them attached unverified, whatever this + * run's `reserveUnverified` says. + */ + adoptedReserved?: Set; + /** + * Reserved phones an adopted user's create left out, from its `creating` + * line: the retry after a refused phone sends without them. Attached once + * the user is adopted, unverified, as they would have been. + */ + adoptedPending?: Map; /** Allow users that carry no password. */ skipPasswordRequirement?: boolean; + /** Create the identifiers the source never verified as reserved. */ + reserveUnverified?: boolean; /** Carried into the summary so the report covers the whole file. */ validationFailed?: number; /** Receives the counts as each user finishes. */ @@ -360,7 +480,10 @@ export async function importUsers(options: ImportUsersOptions): Promise(), + adoptedReserved = new Set(), + adoptedPending = new Map(), skipPasswordRequirement = true, + reserveUnverified = false, validationFailed = 0, progress: report, } = options; @@ -396,6 +519,7 @@ export async function importUsers(options: ImportUsersOptions): Promise => { const retries: string[] = []; const identifiers = splitIdentifiers(user); - let created: { clerkUserId: string; notes: string[]; phoneRefusal?: string }; + let created: CreatedUser; const adoptedId = adopted.get(user.userId); + // What the create did with the unverified identifiers decides what is left + // to attach: an adopted user's create may have run in the other mode. + const reserved = adoptedId + ? adoptedReserved.has(user.userId) + : reserveUnverified && + (identifiers.unverifiedEmails.length > 0 || identifiers.unverifiedPhones.length > 0); let sent = false; try { created = adoptedId - ? { clerkUserId: adoptedId, notes: [] } + ? { clerkUserId: adoptedId, notes: [], attachLater: adoptedPending.get(user.userId) ?? [] } : await retryOn429( async () => - createUser(ctx, user, identifiers, skipPasswordRequirement, () => { - record({ sourceId: user.userId, status: "creating" }); + createUser(ctx, user, identifiers, skipPasswordRequirement, (body) => { + // A retry after a refused phone sends without the reserved + // ones. Cut off before its answer, the user would be adopted + // as reserved, those phones never attached. + const sentPhones = new Set((body.phone_number as string[] | undefined) ?? []); + const leftOut = reserved + ? identifiers.unverifiedPhones.filter((phone) => !sentPhones.has(phone)) + : []; + record({ + sourceId: user.userId, + status: "creating", + ...(reserved ? { reserved: true } : {}), + ...(leftOut.length > 0 + ? { + pending: leftOut.map((value) => ({ + kind: "phone" as const, + value, + verified: false, + })), + } + : {}), + }); sent = true; }), { @@ -512,7 +662,12 @@ export async function importUsers(options: ImportUsersOptions): Promise ({ + kind: "phone" as const, + value, + verified: false, + })); + const toAttach = [...pendingIdentifiers(identifiers, reserved), ...retried]; record(toAttach.length > 0 ? { ...line, pending: toAttach } : line); if (created.phoneRefusal) { const reason = normalizeErrorMessage(created.phoneRefusal); diff --git a/packages/cli-core/src/commands/migrate/index.test.ts b/packages/cli-core/src/commands/migrate/index.test.ts index d3e5857c8f..b0565fb50e 100644 --- a/packages/cli-core/src/commands/migrate/index.test.ts +++ b/packages/cli-core/src/commands/migrate/index.test.ts @@ -1,4 +1,4 @@ -import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { beforeAll, describe, expect, test } from "bun:test"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -7,16 +7,6 @@ import { createProgram } from "../../cli-program.ts"; import { exportPlatformKeys } from "./export/registry.ts"; import { isAssumeYes, setAssumeYes } from "./lib/assume-yes.ts"; -let saved: string | undefined; -beforeAll(() => { - saved = process.env.CLERK_EXPERIMENTAL; - process.env.CLERK_EXPERIMENTAL = "migrate"; -}); -afterAll(() => { - if (saved === undefined) delete process.env.CLERK_EXPERIMENTAL; - else process.env.CLERK_EXPERIMENTAL = saved; -}); - function findCommand(names: string[]) { let current = createProgram().commands.find((cmd) => cmd.name() === names[0]); for (const name of names.slice(1)) { @@ -55,6 +45,7 @@ describe("registerMigrate", () => { "--new-run", "--require-password", "--skip-legal-checks", + "--reserve-unverified", "--firebase-signer-key", "--firebase-salt-separator", "--firebase-rounds", diff --git a/packages/cli-core/src/commands/migrate/index.ts b/packages/cli-core/src/commands/migrate/index.ts index 0e27c76d2b..c39ea2a157 100644 --- a/packages/cli-core/src/commands/migrate/index.ts +++ b/packages/cli-core/src/commands/migrate/index.ts @@ -1,5 +1,4 @@ import type { Program } from "../../cli-program.ts"; -import { isExperimentEnabled, requireExperiment } from "../../lib/experimental.ts"; import { parseIntegerOption } from "../../lib/option-parsers.ts"; import { setMode } from "../../mode.ts"; import { setAssumeYes } from "./lib/assume-yes.ts"; @@ -10,27 +9,7 @@ import { runs } from "./runs.ts"; import { undo } from "./undo.ts"; import { list as sources } from "./sources/list.ts"; -export function registerMigrate(program: Program, env: NodeJS.ProcessEnv = process.env): void { - if (!isExperimentEnabled("migrate", env)) { - // A hidden stub rather than no command at all: `clerk migrate` then says how - // to turn it on instead of "unknown command". It has no subcommands, so - // completion has nothing to walk, and it accepts anything, so every - // invocation reaches the refusal. - const stub = program - .command("migrate", { hidden: true }) - .helpOption(false) - .allowUnknownOption() - .allowExcessArguments() - .argument("[args...]") - .action(() => requireExperiment("migrate", env)); - // `clerk help migrate` renders help without running the action. - stub.helpInformation = () => { - requireExperiment("migrate", env); - return ""; - }; - return; - } - +export function registerMigrate(program: Program): void { const migrateCommand = program .command("migrate") .description("Migrate users into Clerk from another auth provider or another Clerk instance") @@ -94,6 +73,10 @@ export function registerMigrate(program: Program, env: NodeJS.ProcessEnv = proce "--skip-legal-checks", "Import users with no legal acceptance on record into an instance that requires it", ) + .option( + "--reserve-unverified", + "Create emails and phones the source never verified as reserved (usable for sign-in, locked to the user) instead of unverified", + ) .option( "--firebase-signer-key ", "Firebase base64 signer key (overrides the export file), or CLERK_FIREBASE_SIGNER_KEY (does not)", diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 261627e175..74b09851b4 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -237,6 +237,49 @@ describe("rejects", () => { ); }); + // --reserve-unverified puts unverified identifiers on the create, so they + // clash there as a primary would; without it they are attached after. + test.each([ + [ + true, + { + b: "email is also used by an earlier user in the file, which is kept", + c: "email is already used by a user in the instance", + }, + ], + [false, {}], + ])( + "with reserveUnverified %p, unverified emails that clash are duplicates", + async (reserveUnverified, expected) => { + existing = [{ id: "user_1", email_addresses: [{ email_address: "taken@x.dev" }] }]; + expect( + await reasonsOf({ + reserveUnverified, + users: [ + user("a", { unverifiedEmailAddresses: ["shared@x.dev"] }), + user("b", { unverifiedEmailAddresses: ["shared@x.dev"] }), + user("c", { unverifiedEmailAddresses: ["taken@x.dev"] }), + ], + }), + ).toEqual(expected); + }, + ); + + // An adopted user's create already ran without reserving, so its unverified + // email is attached later, whatever this run's flag says: no clash. + test("an adopted user's unverified email counts by its own create's mode", async () => { + expect( + await reasonsOf({ + reserveUnverified: true, + adoptedSourceIds: new Set(["b"]), + users: [ + user("a", { unverifiedEmailAddresses: ["shared@x.dev"] }), + user("b", { unverifiedEmailAddresses: ["shared@x.dev"] }), + ], + }), + ).toEqual({}); + }); + // The source's order decides which duplicate survives, so the dry run says. test("a duplicate names the earlier user kept in its place", async () => { const checks = await checkImport( @@ -272,6 +315,18 @@ describe("rejects", () => { }); }); + // Reserved is usable for sign-in, so it meets the requirement (checked live + // against the E2E test app, which requires an email). + test("a reserved email meets an email requirement", async () => { + expect( + await reasonsOf({ + settings: EMAIL_REQUIRED, + reserveUnverified: true, + users: [user("unverified", { email: undefined, unverifiedEmailAddresses: ["u@x.dev"] })], + }), + ).toEqual({}); + }); + test("a password that is not the shape its hasher says", async () => { expect( await reasonsOf({ @@ -409,6 +464,44 @@ describe("rejects", () => { ).toEqual({ neither: "only signs in with Discord, which is not enabled in Clerk" }); }); + // The create would strip it, so it can't be what saves the user. + test("an email Clerk refuses is no way in for a supabase user on a disabled provider", async () => { + expect( + await reasonsOf({ + settings: settings({ + email_address: { + enabled: true, + used_for_first_factor: true, + first_factors: ["email_code"], + }, + username: { enabled: true }, + }), + supabaseRows: [{ id: "u", raw_app_meta_data: { providers: ["discord"] } }], + users: [user("u", { email: "u@x.test", username: "uname" })], + }), + ).toEqual({ u: "only signs in with Discord, which is not enabled in Clerk" }); + }); + + test("a phone not in E.164 form is no way in for a supabase user on a disabled provider", async () => { + expect( + await reasonsOf({ + settings: { + ...settings({ + phone_number: { + enabled: true, + used_for_first_factor: true, + first_factors: ["phone_code"], + }, + username: { enabled: true }, + }), + username_settings: { min_length: 4, max_length: 64, allow_numeric_usernames: true }, + } as unknown as UserSettingsJSON, + supabaseRows: [{ id: "u", raw_app_meta_data: { providers: ["discord"] } }], + users: [user("u", { email: undefined, username: "12345", phone: "(555) 123-4567" })], + }), + ).toEqual({ u: "only signs in with Discord, which is not enabled in Clerk" }); + }); + test("an unverified email is no way in for a supabase user on a disabled provider", async () => { expect( await reasonsOf({ @@ -425,6 +518,24 @@ describe("rejects", () => { ).toEqual({ u: "only signs in with Discord, which is not enabled in Clerk" }); }); + // A reserved identifier signs in by code, and is verified the first time. + test("a reserved email is a way in for a supabase user on a disabled provider", async () => { + expect( + await reasonsOf({ + settings: settings({ + email_address: { + enabled: true, + used_for_first_factor: true, + first_factors: ["email_link"], + }, + }), + reserveUnverified: true, + supabaseRows: [{ id: "u", raw_app_meta_data: { providers: ["discord"] } }], + users: [user("u", { email: undefined, unverifiedEmailAddresses: ["u@x.dev"] })], + }), + ).toEqual({}); + }); + // Clerk can't turn on a provider it doesn't offer, so none is suggested. test("a provider Clerk doesn't offer is named as such, with no fix", async () => { const rows = [ diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 17b434b570..d8edc97216 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -121,6 +121,13 @@ export type CheckInput = { * sending `skip_legal_checks`. Without it they are rejected. */ skipLegalChecks?: boolean; + /** Unverified identifiers are created reserved, so they meet a requirement. */ + reserveUnverified?: boolean; + /** + * Source IDs of adopted users whose create reserved their unverified + * identifiers: those exist reserved, whatever this run's flag says. + */ + reservedSourceIds?: Set; /** * Clerk IDs a continued run found behind its own in-flight creates: finding * them in the instance is expected. @@ -129,7 +136,9 @@ export type CheckInput = { /** * Source IDs of those adopted users. Each already exists in Clerk, so none * is rejected: the checks stop creates, and these need none. They count - * toward no quota either; the instance's live count has them already. + * toward no quota either; the instance's live count has them already. Their + * create ran in the mode its `creating` line records, so this run's flag + * does not decide what they reserved. */ adoptedSourceIds?: Set; spinner?: SpinnerControls; @@ -187,21 +196,32 @@ export function hashShapeProblem(password: string, hasher: string): string | und * * An email or phone counts only when it is verified: an unverified one is * attached after the user exists, so it cannot satisfy a sign-up requirement. + * With `reserveUnverified` it goes on the create as reserved, which does. */ function missingRequiredIdentifier( user: User, settings: UserSettingsJSON | null, + reserveUnverified = false, ): string | undefined { if (!settings) return undefined; const required = (attribute: AttributeName) => isRequired(settings, attribute); const identifiers = splitIdentifiers(user); + const reserved = (unverified: string[]) => reserveUnverified && unverified.length > 0; - if (required("email_address") && !identifiers.primaryEmail) { + if ( + required("email_address") && + !identifiers.primaryEmail && + !reserved(identifiers.unverifiedEmails) + ) { return identifiers.unverifiedEmails.length > 0 ? "only has an unverified email, and this instance requires an email" : "no email, which this instance requires"; } - if (required("phone_number") && !identifiers.primaryPhone) { + if ( + required("phone_number") && + !identifiers.primaryPhone && + !reserved(identifiers.unverifiedPhones) + ) { return identifiers.unverifiedPhones.length > 0 ? "only has an unverified phone number, and this instance requires one" : "no phone number, which this instance requires"; @@ -560,7 +580,10 @@ const DUPLICATE_SOURCE_ID = "duplicate source ID in the file"; * * @returns Each duplicate's reason, and the earlier user kept in its place. */ -function findFileDuplicates(users: User[]): { +function findFileDuplicates( + users: User[], + reserves: (user: User) => boolean = () => false, +): { reasons: Map; keptBy: Map; } { @@ -580,9 +603,7 @@ function findFileDuplicates(users: User[]): { } seenIds.add(user.userId); - const { primaryEmail, primaryPhone } = splitIdentifiers(user); - const ownEmails = primaryEmail ? [primaryEmail] : []; - const ownPhones = primaryPhone ? [primaryPhone] : []; + const { emails: ownEmails, phones: ownPhones } = sentIdentifiers(user, reserves(user)); const emailOwner = ownEmails.map((email) => emails.get(email.toLowerCase())).find(Boolean); const phoneOwner = ownPhones.map((phone) => phones.get(phoneKey(phone))).find(Boolean); @@ -614,8 +635,38 @@ function findFileDuplicates(users: User[]): { } /** - * Users the instance already holds, found by source ID, primary email, - * primary phone or username. + * Whether a user's unverified identifiers are (or will be) created reserved: + * an adopted user's by what its create recorded, everyone else's by this + * run's flag. + */ +function reservesFor(input: CheckInput): (user: User) => boolean { + return (user) => + input.adoptedSourceIds?.has(user.userId) + ? Boolean(input.reservedSourceIds?.has(user.userId)) + : Boolean(input.reserveUnverified); +} + +/** + * The emails and phones `POST /v1/users` sends for a user: the verified + * primary, plus the unverified ones when they are created reserved. + */ +function sentIdentifiers(user: User, reserveUnverified: boolean) { + const { primaryEmail, primaryPhone, unverifiedEmails, unverifiedPhones } = splitIdentifiers(user); + return { + emails: [ + ...(primaryEmail ? [primaryEmail] : []), + ...(reserveUnverified ? unverifiedEmails : []), + ], + phones: [ + ...(primaryPhone ? [primaryPhone] : []), + ...(reserveUnverified ? unverifiedPhones : []), + ], + }; +} + +/** + * Users the instance already holds, found by source ID, the emails and phones + * the create sends, or username. * * Only what `POST /v1/users` itself carries is looked up: an extra email that * collides is attached after the user exists, fails on its own, and is noted @@ -631,10 +682,10 @@ async function findInstanceDuplicates( const byUsername = new Map(); for (const user of users) { - const identifiers = splitIdentifiers(user); + const sent = sentIdentifiers(user, reservesFor(input)(user)); byExternalId.set(user.userId, user.userId); - if (identifiers.primaryEmail) byEmail.set(identifiers.primaryEmail.toLowerCase(), user.userId); - if (identifiers.primaryPhone) byPhone.set(phoneKey(identifiers.primaryPhone), user.userId); + for (const email of sent.emails) byEmail.set(email.toLowerCase(), user.userId); + for (const phone of sent.phones) byPhone.set(phoneKey(phone), user.userId); if (typeof user.username === "string" && user.username) { byUsername.set(user.username.toLowerCase(), user.userId); } @@ -710,16 +761,21 @@ function findDisabledProviderRejects(input: CheckInput): Map { const { excludedIds } = findUsersWithOnlyDisabledProviders(input.supabaseRows, disabled); // A disabled provider only strands a user with no other way in: a verified - // email or phone the instance signs in with by code or link still works. + // or reserved email or phone the instance signs in with by code or link + // still works. const strategies = firstFactorStrategies(input.settings); const usersById = new Map(input.users.map((user) => [user.userId, user])); + const reserves = reservesFor(input); const canSignInOtherwise = (id: string) => { const user = usersById.get(id); if (!user) return false; - const { primaryEmail, primaryPhone } = splitIdentifiers(user); + // An email Clerk refuses, or a phone not in E.164 form with numeric + // usernames on, is stripped before the create, so it is no way in. + const sent = dropLoosePhones(dropRefusedEmails(user).user, input.settings).user; + const { emails, phones } = sentIdentifiers(sent, reserves(user)); return ( - (Boolean(primaryEmail) && (strategies.has("email_code") || strategies.has("email_link"))) || - (Boolean(primaryPhone) && strategies.has("phone_code")) + (emails.length > 0 && (strategies.has("email_code") || strategies.has("email_link"))) || + (phones.length > 0 && strategies.has("phone_code")) ); }; // Each reject names only that user's own providers. @@ -862,10 +918,13 @@ function buildFixes(input: CheckInput, users: User[]): Fix[] { // An unverified email does not satisfy a required one, so a file of only // unverified addresses flags the requirement even when every user has one. - const unverifiedOnly = users.some((user) => { - const identifiers = splitIdentifiers(user); - return !identifiers.primaryEmail && identifiers.unverifiedEmails.length > 0; - }); + // Created reserved, it does, and making email optional fixes nothing. + const unverifiedOnly = + !input.reserveUnverified && + users.some((user) => { + const identifiers = splitIdentifiers(user); + return !identifiers.primaryEmail && identifiers.unverifiedEmails.length > 0; + }); const flagged = report.blocking.slice(); if ( unverifiedOnly && @@ -1056,6 +1115,7 @@ export async function checkImport(input: CheckInput): Promise { user.password && user.passwordHasher ? hashShapeProblem(user.password, user.passwordHasher) : undefined; + const reserve = reservesFor(input)(user); const reason = adopted(original) ? undefined : (original.skipReason ?? @@ -1068,11 +1128,11 @@ export async function checkImport(input: CheckInput): Promise { : undefined) ?? // A required phone the file has, but only in a form Clerk refuses here. (phoned.dropped && - missingRequiredIdentifier(user, input.settings) && - !missingRequiredIdentifier(emailed, input.settings) + missingRequiredIdentifier(user, input.settings, reserve) && + !missingRequiredIdentifier(emailed, input.settings, reserve) ? "its phones aren't in E.164 form, and this instance requires a phone number (numeric usernames are on)" : undefined) ?? - missingRequiredIdentifier(user, input.settings) ?? + missingRequiredIdentifier(user, input.settings, reserve) ?? // Stripping the identifiers the instance has off can leave nothing to // sign in with; Clerk would still create the user. (!hasAnyIdentifier(sent) @@ -1107,10 +1167,10 @@ export async function checkImport(input: CheckInput): Promise { // An adopted user already holds its email, phone and username in Clerk, so // it claims them first, wherever it sits in the file: a fresh record sharing // one is the create Clerk would refuse, and is not kept in its place. - const { reasons: fileDuplicates, keptBy } = findFileDuplicates([ - ...passed.filter(adopted), - ...passed.filter((user) => !adopted(user)), - ]); + const { reasons: fileDuplicates, keptBy } = findFileDuplicates( + [...passed.filter(adopted), ...passed.filter((user) => !adopted(user))], + reservesFor(input), + ); let candidates: User[] = []; for (const user of passed) { const kept = keptBy.get(user); diff --git a/packages/cli-core/src/commands/migrate/lib/run-store.ts b/packages/cli-core/src/commands/migrate/lib/run-store.ts index 25f63fd4a5..f9932b17e3 100644 --- a/packages/cli-core/src/commands/migrate/lib/run-store.ts +++ b/packages/cli-core/src/commands/migrate/lib/run-store.ts @@ -103,6 +103,11 @@ export type RunRecord = { * not everyone, so it finishes `partial` and importing it says so. */ truncated?: boolean; + /** + * Whether an import created unverified emails and phones reserved. A + * continue reuses it, so every user of one run is created the same way. + */ + reserveUnverified?: boolean; /** The import run an undo reverses. */ undoes?: string; /** The undo run that reversed this one. */ @@ -128,6 +133,11 @@ export type UserLine = { /** Failed: the HTTP status, or "unknown". Skipped: a machine reason, NO_PASSWORD_CODE or IN_FILE_CODE. */ code?: string; passwordDropped?: boolean; + /** + * On a `creating` line: the create sent this user's unverified identifiers + * reserved. A continued run that adopts the user attaches none of them. + */ + reserved?: boolean; }; /** What `runs` shows for a run: its stored status, or how it stopped. */ diff --git a/packages/cli-core/src/commands/migrate/readme.test.ts b/packages/cli-core/src/commands/migrate/readme.test.ts index 88e5f6af44..c39c1cf697 100644 --- a/packages/cli-core/src/commands/migrate/readme.test.ts +++ b/packages/cli-core/src/commands/migrate/readme.test.ts @@ -9,19 +9,10 @@ * be written down — so neither renaming a flag nor adding one passes silently. */ -import { afterAll, describe, expect, test } from "bun:test"; +import { describe, expect, test } from "bun:test"; import type { Command } from "commander"; import { createProgram } from "../../cli-program.ts"; -// Without it `migrate` is the gate's stub, with no subcommands to check. Set at -// load, because the cases below are built from the command tree. -const savedExperimental = process.env.CLERK_EXPERIMENTAL; -process.env.CLERK_EXPERIMENTAL = "migrate"; -afterAll(() => { - if (savedExperimental === undefined) delete process.env.CLERK_EXPERIMENTAL; - else process.env.CLERK_EXPERIMENTAL = savedExperimental; -}); - const README = await Bun.file(new URL("./README.md", import.meta.url)).text(); /** Fenced blocks only, so prose that merely mentions a flag is not parsed. */ diff --git a/packages/cli-core/src/commands/migrate/run-interactive.test.ts b/packages/cli-core/src/commands/migrate/run-interactive.test.ts index 34b985a6aa..1c5cd18ea7 100644 --- a/packages/cli-core/src/commands/migrate/run-interactive.test.ts +++ b/packages/cli-core/src/commands/migrate/run-interactive.test.ts @@ -30,6 +30,8 @@ const mockPassword = mock(async () => "" as unknown); let confirmAnswer = true; /** User settings the instance serves, or none (the settings read fails). */ let instanceSettings: Record | undefined; +/** The answer to the reserved-identifiers question, apart from consent. */ +let reserveAnswer = true; /** Every confirmation the run put up, in order — the wording is the assertion. */ let confirmMessages: string[] = []; let originalMode: string | undefined; @@ -44,7 +46,7 @@ mock.module("../../lib/listage.ts", () => ({ mock.module("../../lib/prompts.ts", () => ({ confirm: async ({ message }: { message: string }) => { confirmMessages.push(message); - return confirmAnswer; + return message.includes("never verified") ? reserveAnswer : confirmAnswer; }, multiselect: async () => [], text: (...args: unknown[]) => mockText(...(args as [])), @@ -66,7 +68,7 @@ let workDir: string; let configDir: string; let originalCwd: string; let originalFetch: typeof globalThis.fetch; -let requests: { method: string; url: string }[]; +let requests: { method: string; url: string; body: unknown }[]; const EXPORT = [ { id: "u1", primary_email_address: "a@x.dev" }, @@ -102,6 +104,7 @@ beforeEach(() => { requests = []; confirmAnswer = true; instanceSettings = undefined; + reserveAnswer = true; confirmMessages = []; mockSelect.mockReset(); mockText.mockReset(); @@ -114,7 +117,11 @@ beforeEach(() => { globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { const url = new URL(input.toString()); const method = init?.method ?? "GET"; - requests.push({ method, url: url.toString() }); + requests.push({ + method, + url: url.toString(), + body: init?.body ? JSON.parse(init.body as string) : null, + }); if (url.pathname === "/v1/instance") { return Response.json({ object: "instance", id: "ins_1", environment_type: "development" }); } @@ -384,3 +391,125 @@ describe("legal consent", () => { expect(created()).toHaveLength(0); }); }); + +describe("unverified identifiers", () => { + const withUnverified = [ + { id: "u1", primary_email_address: "a@x.dev" }, + { id: "u2", primary_email_address: "b@x.dev", unverified_email_addresses: "c@x.dev" }, + ]; + const statuses = () => + created().map((r) => (r.body as Record)?.email_address_identification_status); + + beforeEach(() => { + fs.writeFileSync(path.join(workDir, "export.json"), JSON.stringify(withUnverified)); + }); + + test("asks whether to reserve them, before consent, and a yes creates them reserved", async () => { + await run(importOptions); + + expect(confirmMessages).toEqual([ + "1 user has an email or phone the source never verified. Create them reserved (usable for sign-in, locked to the user) instead of unverified? A reserved one another user already holds rejects its user, not just that address.", + "Import 2 users?", + ]); + expect(statuses()).toEqual([undefined, ["verified", "reserved"]]); + }); + + /** Rewrites the only run as one a crash stopped after creating u1, and returns its folder. */ + const interruptAfterU1 = (patch: Record = {}): string => { + const [id] = fs.readdirSync(runsDir()); + const dir = path.join(runsDir(), id!); + const record = JSON.parse(fs.readFileSync(path.join(dir, "run.json"), "utf-8")); + delete record.finishedAt; + fs.writeFileSync(path.join(dir, "run.json"), JSON.stringify({ ...record, ...patch })); + const lines = fs.readFileSync(path.join(dir, "users.ndjson"), "utf-8").split("\n"); + fs.writeFileSync( + path.join(dir, "users.ndjson"), + lines.filter((line) => line.includes('"u1"')).join("\n") + "\n", + ); + requests = []; + confirmMessages = []; + return dir; + }; + + // A human said yes, and the continue runs under -y, which never asks. + test("a continued run creates the rest in the mode it recorded, without asking", async () => { + await run(importOptions); + interruptAfterU1(); + + await run({ ...importOptions, yes: true }); + + expect(confirmMessages).toEqual([]); + expect(statuses()).toEqual([["verified", "reserved"]]); + }); + + test("refuses --reserve-unverified on a continue of a run that kept them unverified", async () => { + reserveAnswer = false; + await run(importOptions); + interruptAfterU1(); + + await expect(run({ ...importOptions, yes: true, reserveUnverified: true })).rejects.toThrow( + /created its users' unverified emails and phones unverified/, + ); + expect(created()).toHaveLength(0); + }); + + test("a run recorded before the mode was asks again", async () => { + await run(importOptions); + interruptAfterU1({ reserveUnverified: undefined }); + + await run(importOptions); + + expect(confirmMessages.some((message) => message.includes("never verified"))).toBe(true); + // Recorded, so a later `-y` continue reserves the rest as this one did. + expect(listRuns(runsDir())[0]?.reserveUnverified).toBe(true); + }); + + test("a no keeps them unverified, attached after the user exists", async () => { + reserveAnswer = false; + + await run(importOptions); + + expect(statuses()).toEqual([undefined, undefined]); + expect(requests.some((r) => new URL(r.url).pathname === "/v1/email_addresses")).toBe(true); + }); + + // `-y` is consent to write, not a yes to making unconfirmed addresses usable. + test("--yes does not ask, and keeps them unverified", async () => { + await run({ ...importOptions, yes: true }); + + expect(confirmMessages).toEqual([]); + expect(statuses()).toEqual([undefined, undefined]); + }); + + // A yes at the prompt is a choice; the re-run the refusal suggests must + // keep it, or it would reject the users it let through. + test("a yes carries into the command a refusal suggests", async () => { + fs.writeFileSync( + path.join(workDir, "export.json"), + JSON.stringify([ + ...withUnverified, + { + id: "u3", + primary_email_address: "d@x.dev", + password_digest: "d", + password_hasher: "rot13", + }, + ]), + ); + + const error = (await run(importOptions).catch((caught: unknown) => caught)) as { + examples?: { command: string }[]; + }; + + expect(error.examples?.map((example) => example.command)).toEqual([ + "clerk migrate import export.json --source clerk --reserve-unverified --secret-key --allow-partial --yes", + ]); + }); + + test("--reserve-unverified does not ask, and creates them reserved", async () => { + await run({ ...importOptions, reserveUnverified: true }); + + expect(confirmMessages).toEqual(["Import 2 users?"]); + expect(statuses()).toEqual([undefined, ["verified", "reserved"]]); + }); +}); diff --git a/packages/cli-core/src/commands/migrate/run.test.ts b/packages/cli-core/src/commands/migrate/run.test.ts index b17001ca0b..36df08f1cd 100644 --- a/packages/cli-core/src/commands/migrate/run.test.ts +++ b/packages/cli-core/src/commands/migrate/run.test.ts @@ -1172,6 +1172,23 @@ describe("run", () => { expect(error.examples ?? []).toEqual([]); }); + // A continue of a run that kept them unverified refuses the flag. + test("--new-run's continue keeps the earlier run's reservation mode", async () => { + stubClerk({ failing: new Set(["u2"]) }); + await run(baseOptions); + const [first] = listRuns(runsDir()); + expect(first?.reserveUnverified).toBe(false); + captured.clear(); + stubClerk({ existing: [{ id: "user_u1", external_id: "u1" }] }); + + const error = (await run({ ...baseOptions, newRun: true, reserveUnverified: true }).catch( + (caught: unknown) => caught, + )) as CliError; + + expect(error.message).toContain(`Drop --new-run to continue run ${first!.id}.`); + expect(error.examples?.[0]?.command).not.toContain("--reserve-unverified"); + }); + // The continue would carry --require-password and leave the same users out. test("--new-run --require-password doesn't offer a continue that imports nobody", async () => { await run({ ...baseOptions, requirePassword: true }); @@ -1395,6 +1412,30 @@ describe("run", () => { ); }); + // Reserved meets the requirement, so neither the reject nor the fix applies. + test("with --reserve-unverified, an unverified-only user is neither rejected nor a fix", async () => { + stubClerk({ + settings: { + attributes: { email_address: { enabled: true, required: true } }, + // A way in besides a password, so users without one import. + enterprise_sso: { enabled: true }, + }, + }); + fs.writeFileSync( + path.join(workDir, "export.json"), + JSON.stringify([ + { id: "u1", primary_email_address: "a@x.dev" }, + { id: "u2", unverified_email_addresses: "b@x.dev" }, + ]), + ); + + await run({ ...baseOptions, dryRun: true, reserveUnverified: true }); + + expect(captured.err).not.toContain("only has an unverified email"); + expect(captured.err).not.toContain("required_for_sign_up"); + expect(process.exitCode).toBe(0); + }); + test("legal consent: refused without --skip-legal-checks, sent with skip_legal_checks with it", async () => { stubClerk({ settings: { @@ -1964,6 +2005,128 @@ describe("run", () => { expect(readRun(runsDir(), first!.id)?.status).toBe("complete"); }); + // The `creating` line records what the create did; a continued run reads it + // back rather than guessing from this run's flags. + test("an adopted user whose create reserved its unverified email gets no attach for it", async () => { + fs.writeFileSync( + path.join(workDir, "export.json"), + JSON.stringify([ + { id: "u1", primary_email_address: "a@x.dev" }, + { id: "u2", primary_email_address: "b@x.dev", unverified_email_addresses: "c@x.dev" }, + ]), + ); + stubClerk({ failing: new Set(["u2"]) }); + await run(baseOptions); + const [first] = listRuns(runsDir()); + fs.appendFileSync( + path.join(runsDir(), first!.id, "users.ndjson"), + `${JSON.stringify({ sourceId: "u2", status: "creating", reserved: true })}\n`, + ); + interrupt(first!.id); + + requests = []; + process.exitCode = 0; + stubClerk({ + existing: [ + { id: "user_found", external_id: "u2", private_metadata: { clerkMigrateRun: first!.id } }, + ], + }); + await run(baseOptions); + + expect(created()).toEqual([]); + expect(requests.filter((r) => r.url.endsWith("/v1/email_addresses"))).toEqual([]); + expect(latestUserLines(runsDir(), first!.id).get("u2")).toMatchObject({ + status: "created", + clerkId: "user_found", + }); + }); + + // The retry after a refused phone sent without the reserved phones, and was + // cut off: they are attached, the reserved email isn't sent again. + test("an adopted user gets the reserved phones its cut-off retry left out", async () => { + fs.writeFileSync( + path.join(workDir, "export.json"), + JSON.stringify([ + { id: "u1", primary_email_address: "a@x.dev" }, + { + id: "u2", + primary_email_address: "b@x.dev", + unverified_email_addresses: "c@x.dev", + unverified_phone_numbers: "+31612345678", + }, + ]), + ); + stubClerk({ failing: new Set(["u2"]) }); + await run(baseOptions); + const [first] = listRuns(runsDir()); + fs.appendFileSync( + path.join(runsDir(), first!.id, "users.ndjson"), + `${JSON.stringify({ + sourceId: "u2", + status: "creating", + reserved: true, + pending: [{ kind: "phone", value: "+31612345678", verified: false }], + })}\n`, + ); + interrupt(first!.id); + + requests = []; + process.exitCode = 0; + stubClerk({ + existing: [ + { id: "user_found", external_id: "u2", private_metadata: { clerkMigrateRun: first!.id } }, + ], + }); + await run(baseOptions); + + expect(created()).toEqual([]); + expect(requests.filter((r) => r.url.endsWith("/v1/email_addresses"))).toEqual([]); + expect( + requests.filter((r) => r.url.endsWith("/v1/phone_numbers")).map((r) => r.body), + ).toEqual([ + { user_id: "user_found", phone_number: "+31612345678", primary: false, verified: false }, + ]); + }); + + // Created reserved by the run that stopped: its unverified email exists + // reserved, and meets the requirement, though this run has no flag. + test("an adopted user created reserved meets a required email without the flag", async () => { + const settings = { + attributes: { email_address: { enabled: true, required: true } }, + enterprise_sso: { enabled: true }, + }; + fs.writeFileSync( + path.join(workDir, "export.json"), + JSON.stringify([ + { id: "u1", primary_email_address: "a@x.dev" }, + { id: "u2", unverified_email_addresses: "c@x.dev" }, + ]), + ); + stubClerk({ settings, failing: new Set(["u2"]) }); + await run({ ...baseOptions, reserveUnverified: true }); + const [first] = listRuns(runsDir()); + fs.appendFileSync( + path.join(runsDir(), first!.id, "users.ndjson"), + `${JSON.stringify({ sourceId: "u2", status: "creating", reserved: true })}\n`, + ); + interrupt(first!.id); + + requests = []; + process.exitCode = 0; + stubClerk({ + settings, + existing: [ + { id: "user_found", external_id: "u2", private_metadata: { clerkMigrateRun: first!.id } }, + ], + }); + await run(baseOptions); + + expect(latestUserLines(runsDir(), first!.id).get("u2")).toMatchObject({ + status: "created", + clerkId: "user_found", + }); + }); + // A reject would stop a create; an adopted user needs none. Rejected, it // stayed `creating` and the run could never finish. test("an adopted user a check would reject still finishes the run", async () => { diff --git a/packages/cli-core/src/commands/migrate/run.ts b/packages/cli-core/src/commands/migrate/run.ts index b865662129..0b6e7109ab 100644 --- a/packages/cli-core/src/commands/migrate/run.ts +++ b/packages/cli-core/src/commands/migrate/run.ts @@ -45,7 +45,7 @@ import { NEXT_STEPS, printAgentNextSteps } from "../../lib/next-steps.ts"; import { confirm } from "../../lib/prompts.ts"; import { interruptedExitCode } from "../../lib/signals.ts"; import { withGutter, withSpinner } from "../../lib/spinner.ts"; -import { importUsers } from "./import-users.ts"; +import { importUsers, splitIdentifiers } from "./import-users.ts"; import { ALREADY_IMPORTED, checkImport, type ImportChecks } from "./lib/checks.ts"; import { fetchInstanceSettings, fetchUserCount } from "./lib/clerk-config.ts"; import { readEnvelope, type ExportEnvelope } from "./lib/export-file.ts"; @@ -127,6 +127,12 @@ export type MigrateRunOptions = { * Without it, a prompt asks; where nobody can be asked, they are rejected. */ skipLegalChecks?: boolean; + /** + * Create the emails and phones the source never verified as reserved, not + * unverified. Without it, a prompt asks; where nobody can be asked, they + * stay unverified. + */ + reserveUnverified?: boolean; /** Check against the instance, report, and write nothing. */ dryRun?: boolean; /** Import the users that pass, and record the rest as skipped. */ @@ -780,6 +786,7 @@ function commandFor( if ((!fromExport || options.envelopeSource) && source) parts.push("--source", quoteArg(source)); if (options.newRun) parts.push("--new-run"); parts.push(...importFlags(options)); + if (options.reserveUnverified) parts.push("--reserve-unverified"); // Names, not `<…>`: pasted as is, a shell reads `` as a redirect. if (options.firebaseSignerKey) parts.push("--firebase-signer-key", "SIGNER_KEY"); if (options.firebaseSaltSeparator !== undefined) @@ -1073,15 +1080,38 @@ async function runImport(rawOptions: MigrateRunOptions, lock: ImportLock): Promi // checked or sent again. Those whose extra identifiers never attached // get just the attaches. const continued = resume.kind === "continue" ? resume.record : undefined; + // The users this run created were made in its reserve mode, so the rest + // are too: a continue reuses the mode rather than asking, and refuses a + // flag that asks for the other. A run from before this was recorded has + // none, and continues as it did. + const recordedReserve = continued?.reserveUnverified; + if (recordedReserve !== undefined) { + if (options.reserveUnverified && !recordedReserve) { + throwUsageError( + `Run ${continued!.id} created its users' unverified emails and phones unverified, so --reserve-unverified would create the rest differently. Nothing was imported.\n` + + "Continue it without --reserve-unverified, or pass --new-run to start a new run.", + ); + } + options = { ...options, reserveUnverified: recordedReserve }; + } // What the plan below is built from; checked again once the run's lock // is held, since a prompt can wait while an undo or a continue writes. const plannedLines = continued ? readUserLines(runsDir, continued.id).length : undefined; const done = new Map(); const attachOnly: UserLine[] = []; const inFlight: string[] = []; + const inFlightReserved = new Set(); + const inFlightPending = new Map(); if (continued) { for (const line of latestUserLines(runsDir, continued.id).values()) { if (line.status === "creating") inFlight.push(line.sourceId); + if (line.status === "creating" && line.reserved) inFlightReserved.add(line.sourceId); + if (line.status === "creating" && line.pending?.length) { + inFlightPending.set( + line.sourceId, + line.pending.filter((entry) => entry.kind === "phone").map((entry) => entry.value), + ); + } if (line.status !== "created" || !line.clerkId) continue; done.set(line.sourceId, line.clerkId); if (line.pending?.length) attachOnly.push(line); @@ -1187,10 +1217,36 @@ async function runImport(rawOptions: MigrateRunOptions, lock: ImportLock): Promi options = { ...options, skipLegalChecks }; } + // Unverified stays the default: reserved makes an address the source never + // confirmed usable for sign-in, which is the operator's call to make. + let reserveUnverified = options.reserveUnverified ?? false; + const withUnverified = users.filter((user) => { + const { unverifiedEmails, unverifiedPhones } = splitIdentifiers(user); + return unverifiedEmails.length > 0 || unverifiedPhones.length > 0; + }).length; + // `-y` is consent to write, not a yes to this, so it does not ask. + if ( + !reserveUnverified && + recordedReserve === undefined && + withUnverified > 0 && + !options.dryRun && + !options.yes && + canPrompt(options) + ) { + reserveUnverified = await confirm({ + message: `${plural(withUnverified, "user")} ${withUnverified === 1 ? "has" : "have"} an email or phone the source never verified. Create them reserved (usable for sign-in, locked to the user) instead of unverified? A reserved one another user already holds rejects its user, not just that address.`, + default: false, + }); + } + // The answers carry into every command printed from here on, so a + // suggested re-run does not quietly drop what the operator chose. + options = { ...options, skipLegalChecks, reserveUnverified }; + const checks = await withSpinner("Checking users against the instance...", async (spinner) => checkImport({ users, skipLegalChecks, + reserveUnverified, failures, unknownFields: loaded.unknownFields, ...(supabaseRows ? { supabaseRows } : {}), @@ -1203,6 +1259,8 @@ async function runImport(rawOptions: MigrateRunOptions, lock: ImportLock): Promi schedule, adoptedClerkIds: new Set(adopted.values()), adoptedSourceIds: new Set(adopted.keys()), + // Their `creating` line says how the create ran, not this run's flag. + reservedSourceIds: new Set([...adopted.keys()].filter((id) => inFlightReserved.has(id))), spinner, }), ); @@ -1263,6 +1321,9 @@ async function runImport(rawOptions: MigrateRunOptions, lock: ImportLock): Promi newRun: false, allowPartial: options.allowPartial || recorded.allowPartial, skipLegalChecks: options.skipLegalChecks || recorded.skipLegalChecks, + // Its own mode: a continue refuses --reserve-unverified on a run + // that kept them unverified. + reserveUnverified: earlierRun.reserveUnverified ?? options.reserveUnverified, }, input.fromExport, ["--yes"], @@ -1380,7 +1441,13 @@ async function runImport(rawOptions: MigrateRunOptions, lock: ImportLock): Promi const flags = pickImportFlags(options); const run = continued ? continueWithExport( - continueRun(runsDir, withAttempt(continued, target, flags), plannedLines, carry), + // The answer is recorded, so a later `-y` continue keeps it. + continueRun( + runsDir, + { ...withAttempt(continued, target, flags), reserveUnverified }, + plannedLines, + carry, + ), input.fromExport, ) : startRun(runsDir, { @@ -1392,6 +1459,7 @@ async function runImport(rawOptions: MigrateRunOptions, lock: ImportLock): Promi ...(input.fromExport ? { fromExport: input.fromExport } : {}), ...(options.envelopeSource ? { envelopeSource: options.envelopeSource } : {}), ...(firebaseHash ? { firebaseHash } : {}), + reserveUnverified, flags, }); // Printed now, not on the way out: on a Ctrl-C the signal handler exits @@ -1421,7 +1489,10 @@ async function runImport(rawOptions: MigrateRunOptions, lock: ImportLock): Promi runId: run.record.id, attachOnly, adopted, + adoptedReserved: inFlightReserved, + adoptedPending: inFlightPending, skipPasswordRequirement: !options.requirePassword, + reserveUnverified, progress, }), ) diff --git a/packages/cli-core/src/lib/errors.ts b/packages/cli-core/src/lib/errors.ts index d18a4af766..7c93230356 100644 --- a/packages/cli-core/src/lib/errors.ts +++ b/packages/cli-core/src/lib/errors.ts @@ -106,8 +106,6 @@ export const ERROR_CODE = { INSTALLER_NOT_FOUND: "installer_not_found", /** The npm registry was unreachable. */ REGISTRY_UNREACHABLE: "registry_unreachable", - /** The command is experimental and its `CLERK_EXPERIMENTAL` name isn't set. */ - EXPERIMENT_DISABLED: "experiment_disabled", /** Production instance was created but came back without a domain. */ DEPLOY_DOMAIN_MISSING: "deploy_domain_missing", /** diff --git a/packages/cli-core/src/lib/experimental.test.ts b/packages/cli-core/src/lib/experimental.test.ts deleted file mode 100644 index 0351f3e09c..0000000000 --- a/packages/cli-core/src/lib/experimental.test.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { describe, expect, test } from "bun:test"; -import { CliError, EXIT_CODE } from "./errors.ts"; -import { isExperimentEnabled, requireExperiment } from "./experimental.ts"; - -describe("isExperimentEnabled", () => { - test.each([ - { value: undefined, expected: false }, - { value: "", expected: false }, - { value: "migrate", expected: true }, - { value: " Migrate ", expected: true }, - { value: "other,migrate", expected: true }, - { value: "other, MIGRATE ,x", expected: true }, - { value: "migrates", expected: false }, - { value: "other", expected: false }, - ])("CLERK_EXPERIMENTAL=$value → $expected", ({ value, expected }) => { - expect(isExperimentEnabled("migrate", { CLERK_EXPERIMENTAL: value })).toBe(expected); - }); -}); - -describe("requireExperiment", () => { - test("throws a usage-level CliError naming the variable", () => { - try { - requireExperiment("migrate", {}); - throw new Error("expected throw"); - } catch (error) { - expect(error).toBeInstanceOf(CliError); - expect((error as CliError).code).toBe("experiment_disabled"); - expect((error as CliError).exitCode).toBe(EXIT_CODE.USAGE); - expect((error as CliError).message).toContain("CLERK_EXPERIMENTAL=migrate"); - } - }); - - test("passes when enabled", () => { - expect(() => requireExperiment("migrate", { CLERK_EXPERIMENTAL: "migrate" })).not.toThrow(); - }); -}); diff --git a/packages/cli-core/src/lib/experimental.ts b/packages/cli-core/src/lib/experimental.ts deleted file mode 100644 index 7fa4b03c54..0000000000 --- a/packages/cli-core/src/lib/experimental.ts +++ /dev/null @@ -1,27 +0,0 @@ -/** - * Commands that ship to `main` before they're finished stay behind a name in - * `CLERK_EXPERIMENTAL`, a comma-separated list. Names are trimmed and - * case-insensitive, and unknown names are ignored. - */ -import { CliError, ERROR_CODE, EXIT_CODE } from "./errors.ts"; - -function enabledExperiments(env: NodeJS.ProcessEnv): Set { - return new Set( - (env.CLERK_EXPERIMENTAL ?? "") - .split(",") - .map((name) => name.trim().toLowerCase()) - .filter(Boolean), - ); -} - -export function isExperimentEnabled(name: string, env: NodeJS.ProcessEnv = process.env): boolean { - return enabledExperiments(env).has(name.toLowerCase()); -} - -export function requireExperiment(name: string, env: NodeJS.ProcessEnv = process.env): void { - if (isExperimentEnabled(name, env)) return; - throw new CliError( - `\`clerk ${name}\` is experimental. Set CLERK_EXPERIMENTAL=${name} to use it.`, - { code: ERROR_CODE.EXPERIMENT_DISABLED, exitCode: EXIT_CODE.USAGE }, - ); -} diff --git a/packages/cli-core/src/test/integration/completion.test.ts b/packages/cli-core/src/test/integration/completion.test.ts index 11bc5e2200..3d06a6abf5 100644 --- a/packages/cli-core/src/test/integration/completion.test.ts +++ b/packages/cli-core/src/test/integration/completion.test.ts @@ -221,28 +221,21 @@ describe("generateCompletions", () => { }); test("completes --source with the built-in migrate sources", () => { - const saved = process.env.CLERK_EXPERIMENTAL; - process.env.CLERK_EXPERIMENTAL = "migrate"; - try { - const names = generateCompletions(createProgram(), [ - "migrate", - "import", - "--source", - "", - ]).completions.map((c) => c.name); - expect(names).toEqual([ - "clerk", - "auth0", - "authjs", - "betterauth", - "firebase", - "supabase", - "workos", - ]); - } finally { - if (saved === undefined) delete process.env.CLERK_EXPERIMENTAL; - else process.env.CLERK_EXPERIMENTAL = saved; - } + const names = generateCompletions(createProgram(), [ + "migrate", + "import", + "--source", + "", + ]).completions.map((c) => c.name); + expect(names).toEqual([ + "clerk", + "auth0", + "authjs", + "betterauth", + "firebase", + "supabase", + "workos", + ]); }); test("returns empty for options with unknown values (file paths)", () => { @@ -298,20 +291,13 @@ describe("generateCompletions", () => { }); test("migrate sources: suggests the built-in sources", () => { - const saved = process.env.CLERK_EXPERIMENTAL; - process.env.CLERK_EXPERIMENTAL = "migrate"; - try { - const names = generateCompletions(createProgram(), [ - "migrate", - "sources", - "", - ]).completions.map((c) => c.name); - expect(names).toContain("supabase"); - expect(names).toContain("workos"); - } finally { - if (saved === undefined) delete process.env.CLERK_EXPERIMENTAL; - else process.env.CLERK_EXPERIMENTAL = saved; - } + const names = generateCompletions(createProgram(), [ + "migrate", + "sources", + "", + ]).completions.map((c) => c.name); + expect(names).toContain("supabase"); + expect(names).toContain("workos"); }); test("open dashboard: filters subpaths by prefix", () => { diff --git a/test/e2e/migrate.test.ts b/test/e2e/migrate.test.ts index 1fc5f97e7e..ce216a922d 100644 --- a/test/e2e/migrate.test.ts +++ b/test/e2e/migrate.test.ts @@ -11,6 +11,9 @@ * - A user whose only email is unverified, imported into an instance that * requires an email, is refused by Clerk. The import's checks reject that * user up front on the strength of this test, so it checks both halves. + * - The same user imported with `--reserve-unverified` is created, with that + * email reserved and primary. The checks let them through on the strength + * of this test. * * Requires `CLERK_PLATFORM_API_KEY` and `CLERK_CLI_TEST_APP_ID`. Locally, run * via `bun run test:e2e:op` so 1Password resolves both in-memory. @@ -36,7 +39,6 @@ async function cli(args: string[]) { ...process.env, CLERK_CONFIG_DIR: join(workDir, "config"), CLERK_MIGRATE_DIR: join(workDir, "runs"), - CLERK_EXPERIMENTAL: "migrate", CLERK_TELEMETRY_DISABLED: "1", }) .cwd(workDir) @@ -279,3 +281,36 @@ test("a user whose only email is unverified is refused where email is required", }), ]); }, 60_000); + +test("--reserve-unverified creates an unverified-only user with the email reserved", async () => { + const hex = randomBytes(6).toString("hex"); + const email = `e2e-${hex}+clerk_test@clerkcookie.com`; + const file = writeExport([{ id: `sb_${hex}`, email }]); + + const imported = await cli([ + "migrate", + "import", + file, + "--source", + "supabase", + "--reserve-unverified", + "--yes", + "--json", + ]); + const { run } = JSON.parse(imported.stdout.toString()) as { run: { id: string } }; + const [line] = latestLines(run.id); + expect(line).toMatchObject({ status: "created" }); + + const fetched = await cli(["api", `/users/${line?.clerkId as string}`]); + const user = JSON.parse(fetched.stdout.toString()) as { + primary_email_address_id: string; + email_addresses: { id: string; email_address: string; reserved: boolean }[]; + }; + expect(user.email_addresses).toEqual([ + expect.objectContaining({ + id: user.primary_email_address_id, + email_address: email, + reserved: true, + }), + ]); +}, 60_000);