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 new file mode 100644 index 0000000000..d21b3a51ad --- /dev/null +++ b/packages/upgrade/database/migrations/2026_06_01_000017_reconcile_staff_two_factor.php @@ -0,0 +1,238 @@ +prefix.'staff'; + + if (! Schema::hasTable($staff)) { + return; + } + + $this->renameLegacyColumns($staff); + + // 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) use ($hasSecret, $hasRecoveryCodes): void { + if ($hasSecret) { + $query->orWhereNotNull('app_authentication_secret'); + } + + if ($hasRecoveryCodes) { + $query->orWhereNotNull('app_authentication_recovery_codes'); + } + }) + ->chunkById(500, function ($rows) use ($staff, $hasSecret, $hasRecoveryCodes): void { + foreach ($rows as $row) { + try { + $update = []; + + if ($hasSecret) { + $secret = $this->reencodeSecret($row->app_authentication_secret); + + if ($secret !== null) { + $update['app_authentication_secret'] = $secret; + } + } + + if ($hasRecoveryCodes) { + $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 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], + ); + } + } + }); + } + + /** + * 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 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) { + return; + } + + // 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'); + } + + if ($renameRecoveryCodes) { + $table->renameColumn('two_factor_recovery_codes', 'app_authentication_recovery_codes'); + } + + if ($hasConfirmedAt) { + $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..afc0dabd38 --- /dev/null +++ b/tests/upgrade/Feature/ReconcileStaffTwoFactorTest.php @@ -0,0 +1,366 @@ + 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 $plaintextCodes + * @param array $extra further columns (e.g. two_factor_confirmed_at) + */ +function insertLegacyStaff(int $id, string $secretColumn, string $recoveryColumn, string $secret, array $plaintextCodes, array $extra = []): void +{ + DB::table(STAFF_2FA_UPG_PREFIX.'staff')->insert(array_merge([ + '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(), + ], $extra)); +} + +/** + * 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 $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'], ['two_factor_confirmed_at' => now()]); + + 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'); +}); + +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'); +}); + +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(); +});