From 6c9842249ca93aa66bf0fbf25c0a0a9443637dfb Mon Sep 17 00:00:00 2001
From: Omar Elshopky <omarhelshopky@gmail.com>
Date: Wed, 26 Aug 2026 17:57:08 +0300
Subject: [PATCH 1/4] fix(upgrade): reconcile staff two-factor storage during
 upgrade

Lunar 1.5 renamed the Filament two-factor columns to the app-authentication
names (two_factor_secret -> app_authentication_secret, two_factor_recovery_codes
-> app_authentication_recovery_codes, two_factor_confirmed_at dropped) via an
admin-package migration, and the v2 panel (spec 0049, #2558) matches Filament's
app-authentication: the secret behind an `encrypted` cast read with decryptString
(no unserialize), recovery codes behind `encrypted:array` verified bcrypt-hashed.

But lunar:upgrade carries the staff columns across verbatim, reconciling neither:

- A store upgraded from a pre-1.5 line never ran the rename, so it lands with the
  old column names and the v2 Staff model (which reads app_authentication_*) sees
  no 2FA at all.
- v1 stored the secret as encrypt(serialize(...)) and recovery codes as plaintext.
  Read back through v2's non-serializing `encrypted` cast, the secret decrypts to
  the serialization wrapper (s:16:"JBSW...";) and google2fa throws "Invalid
  characters in the base32 string"; recovery codes json_decode to null. So staff
  cannot pass the 2FA challenge after upgrading.

Add an upgrade step that renames the columns when a pre-1.5 store still carries
them (a 1.5+ store already has the v2 names and is skipped), then re-encodes the
secret without the serialize wrapper and bcrypt-hashes each plaintext recovery
code, matching the `encrypted` / `encrypted:array` casts. Idempotent (already-v2
data is left untouched); an undecryptable record is skipped rather than aborting
the whole upgrade. One-way, like the other upgrade data steps.
---
 ...6_01_000016_reconcile_staff_two_factor.php | 182 ++++++++++++++
 .../Feature/ReconcileStaffTwoFactorTest.php   | 231 ++++++++++++++++++
 2 files changed, 413 insertions(+)
 create mode 100644 packages/upgrade/database/migrations/2026_06_01_000016_reconcile_staff_two_factor.php
 create mode 100644 tests/upgrade/Feature/ReconcileStaffTwoFactorTest.php

diff --git a/packages/upgrade/database/migrations/2026_06_01_000016_reconcile_staff_two_factor.php b/packages/upgrade/database/migrations/2026_06_01_000016_reconcile_staff_two_factor.php
new file mode 100644
index 0000000000..1d213b54e2
--- /dev/null
+++ b/packages/upgrade/database/migrations/2026_06_01_000016_reconcile_staff_two_factor.php
@@ -0,0 +1,182 @@
+<?php
+
+use Illuminate\Contracts\Encryption\DecryptException;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Crypt;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Hash;
+use Illuminate\Support\Facades\Schema;
+use Lunar\Core\Database\Migration;
+
+/**
+ * v1 → v2 upgrade data step: reconcile staff two-factor storage.
+ *
+ * Two gaps on the staff table, handled in order:
+ *
+ * 1. Column names. Lunar 1.5 renamed the two-factor columns to Filament's
+ *    app-authentication names — two_factor_secret → app_authentication_secret,
+ *    two_factor_recovery_codes → app_authentication_recovery_codes, and dropped
+ *    the redundant two_factor_confirmed_at — via an admin-package migration. A
+ *    store upgraded straight from a pre-1.5 line never ran it, so it lands with the
+ *    old names and the v2 Staff model (which reads app_authentication_*) sees no
+ *    2FA. Apply the rename here when the v1 columns are still present; a 1.5+ store
+ *    already has the v2 names and is left untouched.
+ *
+ * 2. Encoding. v1 stored the secret and recovery codes as encrypt(serialize(...))
+ *    with plaintext recovery codes. v2's panel (spec 0049, #2558) matches
+ *    Filament's app-authentication: the secret behind an `encrypted` cast read with
+ *    decryptString (no unserialize), recovery codes behind `encrypted:array`
+ *    verified bcrypt-hashed. Carried across verbatim, the secret then decrypts to
+ *    the serialization wrapper (s:16:"JBSW...";) and google2fa throws "Invalid
+ *    characters in the base32 string", while the recovery value json_decodes to
+ *    null. Unwrap the serialized secret and re-encrypt it without the wrapper, and
+ *    bcrypt-hash each still-plaintext recovery code — matching the `encrypted` /
+ *    `encrypted:array` casts so the Staff model reads them back natively.
+ *
+ * Idempotent: the rename is guarded on the v1 columns' presence, a secret already a
+ * plain base32 string fails the serialize probe, a recovery value already a JSON
+ * array is left alone, and an already-hashed code is not re-hashed. One-way, no
+ * down().
+ */
+return new class extends Migration
+{
+    public function up(): void
+    {
+        $staff = $this->prefix.'staff';
+
+        if (! Schema::hasTable($staff)) {
+            return;
+        }
+
+        $this->renameLegacyColumns($staff);
+
+        if (! Schema::hasColumn($staff, 'app_authentication_secret')
+            || ! Schema::hasColumn($staff, 'app_authentication_recovery_codes')) {
+            return;
+        }
+
+        DB::table($staff)
+            ->where(function ($query): void {
+                $query->whereNotNull('app_authentication_secret')
+                    ->orWhereNotNull('app_authentication_recovery_codes');
+            })
+            ->orderBy('id')
+            ->chunkById(500, function ($rows) use ($staff): void {
+                foreach ($rows as $row) {
+                    try {
+                        $update = [];
+
+                        $secret = $this->reencodeSecret($row->app_authentication_secret);
+
+                        if ($secret !== null) {
+                            $update['app_authentication_secret'] = $secret;
+                        }
+
+                        $codes = $this->reencodeRecoveryCodes($row->app_authentication_recovery_codes);
+
+                        if ($codes !== null) {
+                            $update['app_authentication_recovery_codes'] = $codes;
+                        }
+
+                        if ($update !== []) {
+                            DB::table($staff)->where('id', $row->id)->update($update);
+                        }
+                    } catch (DecryptException) {
+                        // A 2FA record this database's APP_KEY cannot decrypt — a stale
+                        // value from a prior key rotation, say — is left as-is rather
+                        // than aborting the whole upgrade; that staff member re-enrols.
+                        // A key wrong for the entire database surfaces earlier in the
+                        // upgrade, not just here.
+                    }
+                }
+            });
+    }
+
+    /**
+     * Apply Lunar 1.5's two-factor column rename when a pre-1.5 store upgraded
+     * without it. A 1.5+ store already has the v2 names and is left untouched.
+     */
+    private function renameLegacyColumns(string $staff): void
+    {
+        // Guard each column independently: Fortify's migration adds and renames the
+        // pair together, but a store part-way through a manual fix could carry only
+        // one, and renaming a missing column would throw.
+        $renameSecret = Schema::hasColumn($staff, 'two_factor_secret');
+        $renameRecoveryCodes = Schema::hasColumn($staff, 'two_factor_recovery_codes');
+        $dropConfirmedAt = Schema::hasColumn($staff, 'two_factor_confirmed_at');
+
+        if (! $renameSecret && ! $renameRecoveryCodes && ! $dropConfirmedAt) {
+            return;
+        }
+
+        Schema::table($staff, function (Blueprint $table) use ($renameSecret, $renameRecoveryCodes, $dropConfirmedAt): void {
+            if ($renameSecret) {
+                $table->renameColumn('two_factor_secret', 'app_authentication_secret');
+            }
+
+            if ($renameRecoveryCodes) {
+                $table->renameColumn('two_factor_recovery_codes', 'app_authentication_recovery_codes');
+            }
+
+            if ($dropConfirmedAt) {
+                $table->dropColumn('two_factor_confirmed_at');
+            }
+        });
+    }
+
+    /**
+     * Returns the re-encrypted secret when the stored value is v1's serialized
+     * wrapper, or null when it is already the plain v2 form (or absent).
+     */
+    private function reencodeSecret(?string $encrypted): ?string
+    {
+        if ($encrypted === null || $encrypted === '') {
+            return null;
+        }
+
+        $plain = @unserialize(Crypt::decryptString($encrypted), ['allowed_classes' => false]);
+
+        return is_string($plain) ? Crypt::encryptString($plain) : null;
+    }
+
+    /**
+     * Returns the re-encrypted, bcrypt-hashed recovery-code set when the stored
+     * value is v1's serialized/plaintext form, or null when it is already the v2
+     * `encrypted:array` form (or absent).
+     */
+    private function reencodeRecoveryCodes(?string $encrypted): ?string
+    {
+        if ($encrypted === null || $encrypted === '') {
+            return null;
+        }
+
+        $decrypted = Crypt::decryptString($encrypted);
+
+        // The v2 `encrypted:array` form decrypts straight to a JSON array; leave it.
+        // This relies on v1 always carrying the serialize wrapper (Fortify stored
+        // `encrypt(serialize(json_encode($codes)))`), so a v1 value never decrypts to
+        // bare JSON — otherwise plaintext codes could be mistaken for a done set.
+        if (is_array(json_decode($decrypted, true))) {
+            return null;
+        }
+
+        $json = @unserialize($decrypted, ['allowed_classes' => false]);
+
+        if (! is_string($json)) {
+            return null;
+        }
+
+        $codes = json_decode($json, true);
+
+        if (! is_array($codes)) {
+            return null;
+        }
+
+        $hashed = array_values(array_map(
+            fn ($code): string => Hash::isHashed((string) $code) ? (string) $code : Hash::make((string) $code),
+            $codes,
+        ));
+
+        return Crypt::encryptString(json_encode($hashed));
+    }
+};
diff --git a/tests/upgrade/Feature/ReconcileStaffTwoFactorTest.php b/tests/upgrade/Feature/ReconcileStaffTwoFactorTest.php
new file mode 100644
index 0000000000..bba344c8e0
--- /dev/null
+++ b/tests/upgrade/Feature/ReconcileStaffTwoFactorTest.php
@@ -0,0 +1,231 @@
+<?php
+
+declare(strict_types=1);
+
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Crypt;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Hash;
+use Illuminate\Support\Facades\Schema;
+use Lunar\Tests\Upgrade\TestCase;
+
+uses(TestCase::class);
+
+/**
+ * Isolated prefix so this stands up its own throwaway schema without touching
+ * the shared lunar_* tables.
+ */
+const STAFF_2FA_UPG_PREFIX = 'upg2fa_';
+
+beforeEach(function () {
+    config(['lunar.database.table_prefix' => STAFF_2FA_UPG_PREFIX]);
+});
+
+afterEach(function () {
+    Schema::dropIfExists(STAFF_2FA_UPG_PREFIX.'staff');
+});
+
+function staffTwoFactorMigration(): object
+{
+    $path = glob(dirname(__DIR__, 3).'/packages/upgrade/database/migrations/*reconcile_staff_two_factor.php');
+
+    return require $path[0];
+}
+
+/**
+ * Stand up a staff table. `renamed` picks the shape: a pre-1.5 store still has the
+ * two_factor_* columns (plus the redundant confirmed_at); a 1.5+ store already has
+ * the app_authentication_* names.
+ */
+function createStaffTable(bool $renamed): void
+{
+    Schema::create(STAFF_2FA_UPG_PREFIX.'staff', function (Blueprint $table) use ($renamed) {
+        $table->id();
+        $table->string('email')->unique();
+        $table->string('password');
+
+        if ($renamed) {
+            $table->text('app_authentication_secret')->nullable();
+            $table->text('app_authentication_recovery_codes')->nullable();
+        } else {
+            $table->text('two_factor_secret')->nullable();
+            $table->text('two_factor_recovery_codes')->nullable();
+            $table->timestamp('two_factor_confirmed_at')->nullable();
+        }
+
+        $table->timestamps();
+    });
+}
+
+/**
+ * Insert a staff row whose 2FA columns carry v1's encoding: Crypt::encrypt()
+ * serializes by default, and recovery codes are stored plaintext inside the JSON.
+ *
+ * @param  array<int, string>  $plaintextCodes
+ */
+function insertLegacyStaff(int $id, string $secretColumn, string $recoveryColumn, string $secret, array $plaintextCodes): void
+{
+    DB::table(STAFF_2FA_UPG_PREFIX.'staff')->insert([
+        'id' => $id,
+        'email' => "staff{$id}@example.com",
+        'password' => bcrypt('secret'),
+        $secretColumn => Crypt::encrypt($secret),
+        $recoveryColumn => Crypt::encrypt(json_encode($plaintextCodes)),
+        'created_at' => now(),
+        'updated_at' => now(),
+    ]);
+}
+
+/**
+ * Insert a staff row already in the v2 shape (renamed columns): the secret behind
+ * encryptString (no serialize) and an encrypted:array of bcrypt-hashed codes.
+ *
+ * @param  array<int, string>  $plaintextCodes
+ */
+function insertV2Staff(int $id, string $secret, array $plaintextCodes): void
+{
+    DB::table(STAFF_2FA_UPG_PREFIX.'staff')->insert([
+        'id' => $id,
+        'email' => "staff{$id}@example.com",
+        'password' => bcrypt('secret'),
+        'app_authentication_secret' => Crypt::encryptString($secret),
+        'app_authentication_recovery_codes' => Crypt::encryptString(json_encode(
+            array_map(fn (string $code): string => Hash::make($code), $plaintextCodes),
+        )),
+        'created_at' => now(),
+        'updated_at' => now(),
+    ]);
+}
+
+test('it renames pre-1.5 columns and re-encodes the secret and recovery codes', function () {
+    createStaffTable(renamed: false);
+    insertLegacyStaff(1, 'two_factor_secret', 'two_factor_recovery_codes', 'JBSWY3DPEHPK3PXP', ['aaaa111111-bbbb222222', 'cccc333333-dddd444444']);
+
+    staffTwoFactorMigration()->up();
+
+    $table = STAFF_2FA_UPG_PREFIX.'staff';
+
+    // Columns renamed to the v2 shape; the redundant confirmation timestamp dropped.
+    expect(Schema::hasColumn($table, 'app_authentication_secret'))->toBeTrue()
+        ->and(Schema::hasColumn($table, 'app_authentication_recovery_codes'))->toBeTrue()
+        ->and(Schema::hasColumn($table, 'two_factor_secret'))->toBeFalse()
+        ->and(Schema::hasColumn($table, 'two_factor_confirmed_at'))->toBeFalse();
+
+    $row = DB::table($table)->find(1);
+
+    // Secret re-encrypted without the serialize wrapper: the `encrypted` cast (decryptString) now reads clean base32.
+    expect(Crypt::decryptString($row->app_authentication_secret))->toBe('JBSWY3DPEHPK3PXP');
+
+    // Recovery codes now a JSON array of bcrypt hashes the `encrypted:array` cast reads natively.
+    $codes = json_decode(Crypt::decryptString($row->app_authentication_recovery_codes), true);
+    expect($codes)->toBeArray()->toHaveCount(2)
+        ->and(Hash::check('aaaa111111-bbbb222222', $codes[0]))->toBeTrue()
+        ->and(Hash::check('cccc333333-dddd444444', $codes[1]))->toBeTrue();
+});
+
+test('it re-encodes an already-renamed (1.5) store', function () {
+    createStaffTable(renamed: true);
+    insertLegacyStaff(1, 'app_authentication_secret', 'app_authentication_recovery_codes', 'JBSWY3DPEHPK3PXP', ['aaaa111111-bbbb222222']);
+
+    staffTwoFactorMigration()->up();
+
+    $row = DB::table(STAFF_2FA_UPG_PREFIX.'staff')->find(1);
+
+    expect(Crypt::decryptString($row->app_authentication_secret))->toBe('JBSWY3DPEHPK3PXP');
+
+    $codes = json_decode(Crypt::decryptString($row->app_authentication_recovery_codes), true);
+    expect($codes)->toBeArray()->toHaveCount(1)
+        ->and(Hash::check('aaaa111111-bbbb222222', $codes[0]))->toBeTrue();
+});
+
+test('it is idempotent on an already-reconciled row', function () {
+    createStaffTable(renamed: true);
+    insertLegacyStaff(1, 'app_authentication_secret', 'app_authentication_recovery_codes', 'JBSWY3DPEHPK3PXP', ['aaaa111111-bbbb222222']);
+
+    staffTwoFactorMigration()->up();
+    $afterFirst = DB::table(STAFF_2FA_UPG_PREFIX.'staff')->find(1);
+
+    // A reconciled secret fails the serialize probe and a hashed recovery set is
+    // already a JSON array, so a second run skips both — the stored ciphertext is
+    // left byte-identical, not re-encrypted.
+    staffTwoFactorMigration()->up();
+    $afterSecond = DB::table(STAFF_2FA_UPG_PREFIX.'staff')->find(1);
+
+    expect($afterSecond->app_authentication_secret)->toBe($afterFirst->app_authentication_secret)
+        ->and($afterSecond->app_authentication_recovery_codes)->toBe($afterFirst->app_authentication_recovery_codes);
+});
+
+test('it renames the columns but leaves a staff row without 2FA untouched', function () {
+    createStaffTable(renamed: false);
+    DB::table(STAFF_2FA_UPG_PREFIX.'staff')->insert([
+        'id' => 1,
+        'email' => 'nobody@example.com',
+        'password' => bcrypt('secret'),
+        'created_at' => now(),
+        'updated_at' => now(),
+    ]);
+
+    staffTwoFactorMigration()->up();
+
+    $row = DB::table(STAFF_2FA_UPG_PREFIX.'staff')->find(1);
+
+    expect(Schema::hasColumn(STAFF_2FA_UPG_PREFIX.'staff', 'app_authentication_secret'))->toBeTrue()
+        ->and($row->app_authentication_secret)->toBeNull()
+        ->and($row->app_authentication_recovery_codes)->toBeNull();
+});
+
+test('it reconciles a mixed set in one pass, leaving already-v2 and empty rows untouched', function () {
+    createStaffTable(renamed: true);
+
+    // v1-encoded, both columns -> re-encoded.
+    insertLegacyStaff(1, 'app_authentication_secret', 'app_authentication_recovery_codes', 'JBSWY3DPEHPK3PXP', ['aaaa111111-bbbb222222']);
+    // Already v2 -> skipped, left byte-identical.
+    insertV2Staff(2, 'MFRGGZDFMZTWQ2LK', ['cccc333333-dddd444444']);
+    // No 2FA -> untouched.
+    DB::table(STAFF_2FA_UPG_PREFIX.'staff')->insert([
+        'id' => 3, 'email' => 'staff3@example.com', 'password' => bcrypt('secret'), 'created_at' => now(), 'updated_at' => now(),
+    ]);
+    // v1 secret only, recovery null -> secret re-encoded, recovery stays null.
+    DB::table(STAFF_2FA_UPG_PREFIX.'staff')->insert([
+        'id' => 4, 'email' => 'staff4@example.com', 'password' => bcrypt('secret'),
+        'app_authentication_secret' => Crypt::encrypt('NBSWY3DPEB3W64TMMQ'),
+        'created_at' => now(), 'updated_at' => now(),
+    ]);
+
+    $v2Before = DB::table(STAFF_2FA_UPG_PREFIX.'staff')->find(2);
+
+    staffTwoFactorMigration()->up();
+
+    $table = STAFF_2FA_UPG_PREFIX.'staff';
+
+    expect(Crypt::decryptString(DB::table($table)->find(1)->app_authentication_secret))->toBe('JBSWY3DPEHPK3PXP');
+
+    $v2After = DB::table($table)->find(2);
+    expect($v2After->app_authentication_secret)->toBe($v2Before->app_authentication_secret)
+        ->and($v2After->app_authentication_recovery_codes)->toBe($v2Before->app_authentication_recovery_codes);
+
+    expect(DB::table($table)->find(3)->app_authentication_secret)->toBeNull();
+
+    $four = DB::table($table)->find(4);
+    expect(Crypt::decryptString($four->app_authentication_secret))->toBe('NBSWY3DPEB3W64TMMQ')
+        ->and($four->app_authentication_recovery_codes)->toBeNull();
+});
+
+test('it renames a pre-1.5 store that already lacks two_factor_confirmed_at', function () {
+    Schema::create(STAFF_2FA_UPG_PREFIX.'staff', function (Blueprint $table) {
+        $table->id();
+        $table->string('email')->unique();
+        $table->string('password');
+        $table->text('two_factor_secret')->nullable();
+        $table->text('two_factor_recovery_codes')->nullable();
+        $table->timestamps();
+    });
+    insertLegacyStaff(1, 'two_factor_secret', 'two_factor_recovery_codes', 'JBSWY3DPEHPK3PXP', ['aaaa111111-bbbb222222']);
+
+    staffTwoFactorMigration()->up();
+
+    $table = STAFF_2FA_UPG_PREFIX.'staff';
+    expect(Schema::hasColumn($table, 'app_authentication_secret'))->toBeTrue()
+        ->and(Schema::hasColumn($table, 'two_factor_secret'))->toBeFalse()
+        ->and(Crypt::decryptString(DB::table($table)->find(1)->app_authentication_secret))->toBe('JBSWY3DPEHPK3PXP');
+});

From 9135cdd35e47735cfd01d67ae34002a629e803a0 Mon Sep 17 00:00:00 2001
From: Omar Elshopky <omarhelshopky@gmail.com>
Date: Sun, 30 Aug 2026 00:59:51 +0300
Subject: [PATCH 2/4] fix(upgrade): discard unconfirmed staff 2FA enrolments;
 reconcile columns independently
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Address review on the staff two-factor reconcile step:

- Blocker: a Fortify-derived pre-1.5 store writes the secret when enrolment starts
  and only stamps two_factor_confirmed_at once the user verifies a code — until then
  2FA is off. Carrying an unconfirmed secret across promoted it to an active v2 secret
  with no email fallback, a hard lockout. Clear the secret / recovery columns where
  two_factor_confirmed_at IS NULL before the rename.
- Handle the two app_authentication_* columns independently in the data pass, matching
  the per-column rename guard, so a part-way-fixed secret-only store is re-encoded
  instead of skipped.
- Drop the redundant ->orderBy('id') before chunkById() (it applies its own).

Adds tests for the unconfirmed-enrolment and secret-only-column cases.
---
 ...6_01_000016_reconcile_staff_two_factor.php | 74 ++++++++++++++-----
 .../Feature/ReconcileStaffTwoFactorTest.php   | 52 ++++++++++++-
 2 files changed, 104 insertions(+), 22 deletions(-)

diff --git a/packages/upgrade/database/migrations/2026_06_01_000016_reconcile_staff_two_factor.php b/packages/upgrade/database/migrations/2026_06_01_000016_reconcile_staff_two_factor.php
index 1d213b54e2..082ffbd7e9 100644
--- a/packages/upgrade/database/migrations/2026_06_01_000016_reconcile_staff_two_factor.php
+++ b/packages/upgrade/database/migrations/2026_06_01_000016_reconcile_staff_two_factor.php
@@ -20,7 +20,10 @@
  *    store upgraded straight from a pre-1.5 line never ran it, so it lands with the
  *    old names and the v2 Staff model (which reads app_authentication_*) sees no
  *    2FA. Apply the rename here when the v1 columns are still present; a 1.5+ store
- *    already has the v2 names and is left untouched.
+ *    already has the v2 names and is left untouched. A half-finished enrolment
+ *    (secret written but two_factor_confirmed_at still null) was 2FA-off in v1, so
+ *    its columns are cleared before the rename rather than promoted to an active v2
+ *    secret the user never confirmed.
  *
  * 2. Encoding. v1 stored the secret and recovery codes as encrypt(serialize(...))
  *    with plaintext recovery codes. v2's panel (spec 0049, #2558) matches
@@ -50,32 +53,44 @@ public function up(): void
 
         $this->renameLegacyColumns($staff);
 
-        if (! Schema::hasColumn($staff, 'app_authentication_secret')
-            || ! Schema::hasColumn($staff, 'app_authentication_recovery_codes')) {
+        // Handle the two columns independently: a store part-way through a manual fix
+        // may carry only one of them, and selecting or updating a missing column would throw.
+        $hasSecret = Schema::hasColumn($staff, 'app_authentication_secret');
+        $hasRecoveryCodes = Schema::hasColumn($staff, 'app_authentication_recovery_codes');
+
+        if (! $hasSecret && ! $hasRecoveryCodes) {
             return;
         }
 
         DB::table($staff)
-            ->where(function ($query): void {
-                $query->whereNotNull('app_authentication_secret')
-                    ->orWhereNotNull('app_authentication_recovery_codes');
+            ->where(function ($query) use ($hasSecret, $hasRecoveryCodes): void {
+                if ($hasSecret) {
+                    $query->orWhereNotNull('app_authentication_secret');
+                }
+
+                if ($hasRecoveryCodes) {
+                    $query->orWhereNotNull('app_authentication_recovery_codes');
+                }
             })
-            ->orderBy('id')
-            ->chunkById(500, function ($rows) use ($staff): void {
+            ->chunkById(500, function ($rows) use ($staff, $hasSecret, $hasRecoveryCodes): void {
                 foreach ($rows as $row) {
                     try {
                         $update = [];
 
-                        $secret = $this->reencodeSecret($row->app_authentication_secret);
+                        if ($hasSecret) {
+                            $secret = $this->reencodeSecret($row->app_authentication_secret);
 
-                        if ($secret !== null) {
-                            $update['app_authentication_secret'] = $secret;
+                            if ($secret !== null) {
+                                $update['app_authentication_secret'] = $secret;
+                            }
                         }
 
-                        $codes = $this->reencodeRecoveryCodes($row->app_authentication_recovery_codes);
+                        if ($hasRecoveryCodes) {
+                            $codes = $this->reencodeRecoveryCodes($row->app_authentication_recovery_codes);
 
-                        if ($codes !== null) {
-                            $update['app_authentication_recovery_codes'] = $codes;
+                            if ($codes !== null) {
+                                $update['app_authentication_recovery_codes'] = $codes;
+                            }
                         }
 
                         if ($update !== []) {
@@ -103,13 +118,36 @@ private function renameLegacyColumns(string $staff): void
         // one, and renaming a missing column would throw.
         $renameSecret = Schema::hasColumn($staff, 'two_factor_secret');
         $renameRecoveryCodes = Schema::hasColumn($staff, 'two_factor_recovery_codes');
-        $dropConfirmedAt = Schema::hasColumn($staff, 'two_factor_confirmed_at');
+        $hasConfirmedAt = Schema::hasColumn($staff, 'two_factor_confirmed_at');
 
-        if (! $renameSecret && ! $renameRecoveryCodes && ! $dropConfirmedAt) {
+        if (! $renameSecret && ! $renameRecoveryCodes && ! $hasConfirmedAt) {
             return;
         }
 
-        Schema::table($staff, function (Blueprint $table) use ($renameSecret, $renameRecoveryCodes, $dropConfirmedAt): void {
+        // Discard half-finished v1 enrolments before they become active v2 2FA. The
+        // Fortify-derived plugin writes the secret and recovery codes when enrolment
+        // STARTS and only stamps two_factor_confirmed_at once the user verifies a TOTP
+        // code — until then v1 treats 2FA as off. v2's AppAuthentication::isEnabled()
+        // is just filled($secret), and the challenge offers no email fallback once a
+        // secret exists, so carrying an unconfirmed secret across would lock that staff
+        // member out with a TOTP they never finished setting up.
+        if ($hasConfirmedAt) {
+            $clear = [];
+
+            if ($renameSecret) {
+                $clear['two_factor_secret'] = null;
+            }
+
+            if ($renameRecoveryCodes) {
+                $clear['two_factor_recovery_codes'] = null;
+            }
+
+            if ($clear !== []) {
+                DB::table($staff)->whereNull('two_factor_confirmed_at')->update($clear);
+            }
+        }
+
+        Schema::table($staff, function (Blueprint $table) use ($renameSecret, $renameRecoveryCodes, $hasConfirmedAt): void {
             if ($renameSecret) {
                 $table->renameColumn('two_factor_secret', 'app_authentication_secret');
             }
@@ -118,7 +156,7 @@ private function renameLegacyColumns(string $staff): void
                 $table->renameColumn('two_factor_recovery_codes', 'app_authentication_recovery_codes');
             }
 
-            if ($dropConfirmedAt) {
+            if ($hasConfirmedAt) {
                 $table->dropColumn('two_factor_confirmed_at');
             }
         });
diff --git a/tests/upgrade/Feature/ReconcileStaffTwoFactorTest.php b/tests/upgrade/Feature/ReconcileStaffTwoFactorTest.php
index bba344c8e0..034becb029 100644
--- a/tests/upgrade/Feature/ReconcileStaffTwoFactorTest.php
+++ b/tests/upgrade/Feature/ReconcileStaffTwoFactorTest.php
@@ -62,10 +62,11 @@ function createStaffTable(bool $renamed): void
  * serializes by default, and recovery codes are stored plaintext inside the JSON.
  *
  * @param  array<int, string>  $plaintextCodes
+ * @param  array<string, mixed>  $extra  further columns (e.g. two_factor_confirmed_at)
  */
-function insertLegacyStaff(int $id, string $secretColumn, string $recoveryColumn, string $secret, array $plaintextCodes): void
+function insertLegacyStaff(int $id, string $secretColumn, string $recoveryColumn, string $secret, array $plaintextCodes, array $extra = []): void
 {
-    DB::table(STAFF_2FA_UPG_PREFIX.'staff')->insert([
+    DB::table(STAFF_2FA_UPG_PREFIX.'staff')->insert(array_merge([
         'id' => $id,
         'email' => "staff{$id}@example.com",
         'password' => bcrypt('secret'),
@@ -73,7 +74,7 @@ function insertLegacyStaff(int $id, string $secretColumn, string $recoveryColumn
         $recoveryColumn => Crypt::encrypt(json_encode($plaintextCodes)),
         'created_at' => now(),
         'updated_at' => now(),
-    ]);
+    ], $extra));
 }
 
 /**
@@ -99,7 +100,7 @@ function insertV2Staff(int $id, string $secret, array $plaintextCodes): void
 
 test('it renames pre-1.5 columns and re-encodes the secret and recovery codes', function () {
     createStaffTable(renamed: false);
-    insertLegacyStaff(1, 'two_factor_secret', 'two_factor_recovery_codes', 'JBSWY3DPEHPK3PXP', ['aaaa111111-bbbb222222', 'cccc333333-dddd444444']);
+    insertLegacyStaff(1, 'two_factor_secret', 'two_factor_recovery_codes', 'JBSWY3DPEHPK3PXP', ['aaaa111111-bbbb222222', 'cccc333333-dddd444444'], ['two_factor_confirmed_at' => now()]);
 
     staffTwoFactorMigration()->up();
 
@@ -229,3 +230,46 @@ function insertV2Staff(int $id, string $secret, array $plaintextCodes): void
         ->and(Schema::hasColumn($table, 'two_factor_secret'))->toBeFalse()
         ->and(Crypt::decryptString(DB::table($table)->find(1)->app_authentication_secret))->toBe('JBSWY3DPEHPK3PXP');
 });
+
+test('it discards an unconfirmed pre-1.5 enrolment instead of promoting it to active 2FA', function () {
+    createStaffTable(renamed: false);
+    // Confirmed enrolment (two_factor_confirmed_at set) -> carried across and re-encoded.
+    insertLegacyStaff(1, 'two_factor_secret', 'two_factor_recovery_codes', 'JBSWY3DPEHPK3PXP', ['aaaa111111-bbbb222222'], ['two_factor_confirmed_at' => now()]);
+    // Enrolment started but never confirmed (confirmed_at null) -> 2FA was OFF in v1.
+    insertLegacyStaff(2, 'two_factor_secret', 'two_factor_recovery_codes', 'MFRGGZDFMZTWQ2LK', ['cccc333333-dddd444444'], ['two_factor_confirmed_at' => null]);
+
+    staffTwoFactorMigration()->up();
+
+    $table = STAFF_2FA_UPG_PREFIX.'staff';
+
+    // The confirmed member keeps working 2FA.
+    expect(Crypt::decryptString(DB::table($table)->find(1)->app_authentication_secret))->toBe('JBSWY3DPEHPK3PXP');
+
+    // The unconfirmed member comes out with NO 2FA (they re-enrol in v2) — not a filled
+    // secret they never confirmed, which v2 would treat as active with no email fallback,
+    // locking them out.
+    $unconfirmed = DB::table($table)->find(2);
+    expect($unconfirmed->app_authentication_secret)->toBeNull()
+        ->and($unconfirmed->app_authentication_recovery_codes)->toBeNull();
+});
+
+test('it re-encodes a store that carries only the secret column', function () {
+    // A part-way-fixed store with only the secret column present (no recovery column):
+    // the data pass must handle it independently rather than bail because its pair is absent.
+    Schema::create(STAFF_2FA_UPG_PREFIX.'staff', function (Blueprint $table) {
+        $table->id();
+        $table->string('email')->unique();
+        $table->string('password');
+        $table->text('app_authentication_secret')->nullable();
+        $table->timestamps();
+    });
+    DB::table(STAFF_2FA_UPG_PREFIX.'staff')->insert([
+        'id' => 1, 'email' => 'staff1@example.com', 'password' => bcrypt('secret'),
+        'app_authentication_secret' => Crypt::encrypt('JBSWY3DPEHPK3PXP'),
+        'created_at' => now(), 'updated_at' => now(),
+    ]);
+
+    staffTwoFactorMigration()->up();
+
+    expect(Crypt::decryptString(DB::table(STAFF_2FA_UPG_PREFIX.'staff')->find(1)->app_authentication_secret))->toBe('JBSWY3DPEHPK3PXP');
+});

From 3ebd16fd1c3e43950f9034632bd53834b74a0a32 Mon Sep 17 00:00:00 2001
From: Omar Elshopky <omarhelshopky@gmail.com>
Date: Sun, 30 Aug 2026 01:39:48 +0300
Subject: [PATCH 3/4] chore(upgrade): renumber staff two-factor step to 000017

---
 ...actor.php => 2026_06_01_000017_reconcile_staff_two_factor.php} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename packages/upgrade/database/migrations/{2026_06_01_000016_reconcile_staff_two_factor.php => 2026_06_01_000017_reconcile_staff_two_factor.php} (100%)

diff --git a/packages/upgrade/database/migrations/2026_06_01_000016_reconcile_staff_two_factor.php b/packages/upgrade/database/migrations/2026_06_01_000017_reconcile_staff_two_factor.php
similarity index 100%
rename from packages/upgrade/database/migrations/2026_06_01_000016_reconcile_staff_two_factor.php
rename to packages/upgrade/database/migrations/2026_06_01_000017_reconcile_staff_two_factor.php

From 30d2d0add95f89925e4bfa4227606a62423916dd Mon Sep 17 00:00:00 2001
From: Omar Elshopky <omarhelshopky@gmail.com>
Date: Sat, 5 Sep 2026 04:27:14 +0300
Subject: [PATCH 4/4] fix(upgrade): guard staff 2FA rename against an existing
 target column

Address further review on the staff two-factor reconcile step:

- renameLegacyColumns() gated each rename only on the v1 source column being
  present, not on the v2 target being absent. A store that manually COPIED
  two_factor_secret into app_authentication_secret (rather than renaming it)
  carries both columns, at which point renameColumn() throws and aborts the whole
  upgrade. Guard each rename on the target also being absent; when both exist the
  rename is skipped and the data pass reconciles the v2 column directly.
- The DecryptException branch now logs a warning with the affected staff id: the
  staff member cannot self-serve a re-enrolment (the encrypted cast throws when
  their challenge reads the stale secret), so an operator must clear the columns.
  Reworded the comment, which had overpromised that they "re-enrol".

Adds tests for the both-columns (copied, not renamed) shape: one whose v2 column
still carries the v1 wrapper and is reconciled, and one already reconciled by hand
that the data pass leaves byte-identical.
---
 ...6_01_000017_reconcile_staff_two_factor.php | 48 +++++++---
 .../Feature/ReconcileStaffTwoFactorTest.php   | 91 +++++++++++++++++++
 2 files changed, 124 insertions(+), 15 deletions(-)

diff --git a/packages/upgrade/database/migrations/2026_06_01_000017_reconcile_staff_two_factor.php b/packages/upgrade/database/migrations/2026_06_01_000017_reconcile_staff_two_factor.php
index 082ffbd7e9..d21b3a51ad 100644
--- a/packages/upgrade/database/migrations/2026_06_01_000017_reconcile_staff_two_factor.php
+++ b/packages/upgrade/database/migrations/2026_06_01_000017_reconcile_staff_two_factor.php
@@ -5,6 +5,7 @@
 use Illuminate\Support\Facades\Crypt;
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Facades\Hash;
+use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Schema;
 use Lunar\Core\Database\Migration;
 
@@ -19,8 +20,10 @@
  *    the redundant two_factor_confirmed_at — via an admin-package migration. A
  *    store upgraded straight from a pre-1.5 line never ran it, so it lands with the
  *    old names and the v2 Staff model (which reads app_authentication_*) sees no
- *    2FA. Apply the rename here when the v1 columns are still present; a 1.5+ store
- *    already has the v2 names and is left untouched. A half-finished enrolment
+ *    2FA. Apply the rename here when a v1 column is still present and its v2 target
+ *    is absent; a 1.5+ store (or one part-way through a manual fix that already has
+ *    the v2 column) is left untouched, the data pass reconciling the v2 column
+ *    directly. A half-finished enrolment
  *    (secret written but two_factor_confirmed_at still null) was 2FA-off in v1, so
  *    its columns are cleared before the rename rather than promoted to an active v2
  *    secret the user never confirmed.
@@ -36,10 +39,10 @@
  *    bcrypt-hash each still-plaintext recovery code — matching the `encrypted` /
  *    `encrypted:array` casts so the Staff model reads them back natively.
  *
- * Idempotent: the rename is guarded on the v1 columns' presence, a secret already a
- * plain base32 string fails the serialize probe, a recovery value already a JSON
- * array is left alone, and an already-hashed code is not re-hashed. One-way, no
- * down().
+ * Idempotent: the rename is guarded on the v1 source being present and the v2
+ * target absent, a secret already a plain base32 string fails the serialize probe,
+ * a recovery value already a JSON array is left alone, and an already-hashed code is
+ * not re-hashed. One-way, no down().
  */
 return new class extends Migration
 {
@@ -98,10 +101,18 @@ public function up(): void
                         }
                     } catch (DecryptException) {
                         // A 2FA record this database's APP_KEY cannot decrypt — a stale
-                        // value from a prior key rotation, say — is left as-is rather
-                        // than aborting the whole upgrade; that staff member re-enrols.
-                        // A key wrong for the entire database surfaces earlier in the
-                        // upgrade, not just here.
+                        // value from a prior key rotation, say — is left untouched rather
+                        // than aborting the whole upgrade. Clearing it would be a silent
+                        // 2FA downgrade, and it was equally unusable in v1. The staff
+                        // member cannot self-serve a re-enrolment (the encrypted cast
+                        // throws when their challenge reads the stale secret), so an admin
+                        // must clear the columns — warn so the operator knows who needs
+                        // attention. A key wrong for the entire database surfaces earlier
+                        // in the upgrade, not just here.
+                        Log::warning(
+                            'Skipped staff two-factor reconciliation: the stored value could not be decrypted with the current APP_KEY.',
+                            ['staff_id' => $row->id],
+                        );
                     }
                 }
             });
@@ -113,11 +124,18 @@ public function up(): void
      */
     private function renameLegacyColumns(string $staff): void
     {
-        // Guard each column independently: Fortify's migration adds and renames the
-        // pair together, but a store part-way through a manual fix could carry only
-        // one, and renaming a missing column would throw.
-        $renameSecret = Schema::hasColumn($staff, 'two_factor_secret');
-        $renameRecoveryCodes = Schema::hasColumn($staff, 'two_factor_recovery_codes');
+        // Guard each rename on the v1 (source) column being present AND the v2
+        // (target) column being absent. Fortify's migration renames the pair
+        // together, but a store part-way through a manual fix could carry only one —
+        // or could have COPIED (not renamed) two_factor_secret into
+        // app_authentication_secret, leaving both columns present, at which point
+        // renameColumn() would throw and abort the whole upgrade. When both exist,
+        // skipping the rename is enough: the data pass below reconciles whatever is
+        // already in the v2 column.
+        $renameSecret = Schema::hasColumn($staff, 'two_factor_secret')
+            && ! Schema::hasColumn($staff, 'app_authentication_secret');
+        $renameRecoveryCodes = Schema::hasColumn($staff, 'two_factor_recovery_codes')
+            && ! Schema::hasColumn($staff, 'app_authentication_recovery_codes');
         $hasConfirmedAt = Schema::hasColumn($staff, 'two_factor_confirmed_at');
 
         if (! $renameSecret && ! $renameRecoveryCodes && ! $hasConfirmedAt) {
diff --git a/tests/upgrade/Feature/ReconcileStaffTwoFactorTest.php b/tests/upgrade/Feature/ReconcileStaffTwoFactorTest.php
index 034becb029..afc0dabd38 100644
--- a/tests/upgrade/Feature/ReconcileStaffTwoFactorTest.php
+++ b/tests/upgrade/Feature/ReconcileStaffTwoFactorTest.php
@@ -273,3 +273,94 @@ function insertV2Staff(int $id, string $secret, array $plaintextCodes): void
 
     expect(Crypt::decryptString(DB::table(STAFF_2FA_UPG_PREFIX.'staff')->find(1)->app_authentication_secret))->toBe('JBSWY3DPEHPK3PXP');
 });
+
+test('it skips the rename when a store carries both the v1 and v2 columns, reconciling the v2 column', function () {
+    // A store part-way through a manual fix that COPIED (not renamed) two_factor_secret
+    // into app_authentication_secret carries both columns. renameColumn() would throw on
+    // the collision and abort the upgrade, so the rename is skipped and the data pass
+    // reconciles whatever sits in the v2 column.
+    Schema::create(STAFF_2FA_UPG_PREFIX.'staff', function (Blueprint $table) {
+        $table->id();
+        $table->string('email')->unique();
+        $table->string('password');
+        $table->text('two_factor_secret')->nullable();
+        $table->text('two_factor_recovery_codes')->nullable();
+        $table->text('app_authentication_secret')->nullable();
+        $table->text('app_authentication_recovery_codes')->nullable();
+        $table->timestamps();
+    });
+
+    // The v1 encoding was copied into both columns, so the v2 column still carries the
+    // serialize wrapper that needs re-encoding.
+    $legacySecret = Crypt::encrypt('JBSWY3DPEHPK3PXP');
+    $legacyCodes = Crypt::encrypt(json_encode(['aaaa111111-bbbb222222']));
+    DB::table(STAFF_2FA_UPG_PREFIX.'staff')->insert([
+        'id' => 1, 'email' => 'staff1@example.com', 'password' => bcrypt('secret'),
+        'two_factor_secret' => $legacySecret,
+        'two_factor_recovery_codes' => $legacyCodes,
+        'app_authentication_secret' => $legacySecret,
+        'app_authentication_recovery_codes' => $legacyCodes,
+        'created_at' => now(), 'updated_at' => now(),
+    ]);
+
+    // Must not throw on the rename collision.
+    staffTwoFactorMigration()->up();
+
+    $table = STAFF_2FA_UPG_PREFIX.'staff';
+
+    // The rename was skipped: the v1 columns are left in place (renaming onto the
+    // existing v2 columns is what would have aborted the upgrade).
+    expect(Schema::hasColumn($table, 'app_authentication_secret'))->toBeTrue()
+        ->and(Schema::hasColumn($table, 'two_factor_secret'))->toBeTrue();
+
+    // The data pass reconciled the v2 column: its serialize wrapper is now plain base32.
+    $row = DB::table($table)->find(1);
+    expect(Crypt::decryptString($row->app_authentication_secret))->toBe('JBSWY3DPEHPK3PXP');
+
+    $codes = json_decode(Crypt::decryptString($row->app_authentication_recovery_codes), true);
+    expect($codes)->toBeArray()->toHaveCount(1)
+        ->and(Hash::check('aaaa111111-bbbb222222', $codes[0]))->toBeTrue();
+});
+
+test('it leaves an already-reconciled v2 column untouched when a store carries both columns', function () {
+    // Both columns present, but the operator has ALREADY reconciled the v2 column by hand
+    // to the v2 encoding (secret behind encryptString, recovery an array of bcrypt hashes).
+    // The rename is skipped (both exist) and the data pass must be a no-op — the idempotency
+    // guards (a plain secret fails the serialize probe, a hashed recovery set is already a
+    // JSON array) leave the ciphertext byte-identical rather than re-encrypting it.
+    Schema::create(STAFF_2FA_UPG_PREFIX.'staff', function (Blueprint $table) {
+        $table->id();
+        $table->string('email')->unique();
+        $table->string('password');
+        $table->text('two_factor_secret')->nullable();
+        $table->text('two_factor_recovery_codes')->nullable();
+        $table->text('app_authentication_secret')->nullable();
+        $table->text('app_authentication_recovery_codes')->nullable();
+        $table->timestamps();
+    });
+
+    // v1 encoding still in the old columns; the v2 column is already in v2 form.
+    DB::table(STAFF_2FA_UPG_PREFIX.'staff')->insert([
+        'id' => 1, 'email' => 'staff1@example.com', 'password' => bcrypt('secret'),
+        'two_factor_secret' => Crypt::encrypt('JBSWY3DPEHPK3PXP'),
+        'two_factor_recovery_codes' => Crypt::encrypt(json_encode(['aaaa111111-bbbb222222'])),
+        'app_authentication_secret' => Crypt::encryptString('JBSWY3DPEHPK3PXP'),
+        'app_authentication_recovery_codes' => Crypt::encryptString(json_encode([Hash::make('aaaa111111-bbbb222222')])),
+        'created_at' => now(), 'updated_at' => now(),
+    ]);
+
+    $before = DB::table(STAFF_2FA_UPG_PREFIX.'staff')->find(1);
+
+    staffTwoFactorMigration()->up();
+
+    $after = DB::table(STAFF_2FA_UPG_PREFIX.'staff')->find(1);
+
+    // The already-reconciled v2 columns are byte-identical — not re-encrypted.
+    expect($after->app_authentication_secret)->toBe($before->app_authentication_secret)
+        ->and($after->app_authentication_recovery_codes)->toBe($before->app_authentication_recovery_codes);
+
+    // …and still read back correctly in the v2 form.
+    expect(Crypt::decryptString($after->app_authentication_secret))->toBe('JBSWY3DPEHPK3PXP');
+    $codes = json_decode(Crypt::decryptString($after->app_authentication_recovery_codes), true);
+    expect(Hash::check('aaaa111111-bbbb222222', $codes[0]))->toBeTrue();
+});