fix(upgrade): reconcile staff two-factor storage during upgrade

[omarelshopky]

Summary

Fixes #2651. Reconciles the staff two-factor columns from v1 to v2 during lunar:upgrade — both the column names and the encoding, which the upgrade package previously left untouched (it had no staff/two-factor migration at all). See the
issue for the two failure modes.

Changes

Adds packages/upgrade/database/migrations/2026_06_01_000016_reconcile_staff_two_factor.php:

• Rename (gated): when a pre-1.5 store still carries two_factor_secret, rename two_factor_* → app_authentication_* and drop two_factor_confirmed_at. A 1.5+ store already has the v2 names and is skipped.
• Re-encode: unwrap v1's encrypt(serialize(...)) secret and re-encrypt with encryptString, and bcrypt-hash each plaintext recovery code, storing encryptString(json_encode([hashes])) — matching the encrypted / encrypted:array casts on the v2 Staff model.
• Idempotent (already-v2 data fails the serialize probe / is a JSON array → skipped; hashed codes are not re-hashed). An undecryptable record (e.g. a stale value from a prior key rotation) is skipped rather than aborting the whole upgrade. One-way, no down().

Tests

Adds tests/upgrade/Feature/ReconcileStaffTwoFactorTest.php: pre-1.5 rename + re-encode, already-renamed (1.5) re-encode, idempotent re-run, a no-2FA row, a mixed single pass (v1 + already-v2 + null + secret-only), and the two_factor_confirmed_at-absent branch.

[glennjacobs]

Thanks for this — the approach is right and the encoding probes hold up in every shape I checked (pre-1.5 stephenjude/Fortify-style, 1.5+ Filament v4 native, already-v2). One blocker before this can go in though.

Unconfirmed v1 enrolments get promoted to active 2FA, which can lock staff out.

The pre-1.5 plugin is Fortify-derived: the secret and recovery codes are written when enrolment starts, and two_factor_confirmed_at is only set once the user confirms a TOTP code — until then v1 treats 2FA as off. renameLegacyColumns() drops two_factor_confirmed_at without reading it, so a half-finished enrolment (secret set, confirmed_at NULL) comes out of the upgrade with a filled app_authentication_secret. In v2, AppAuthentication::isEnabled() is just filled($secret), and the challenge controller offers no email fallback once a secret exists — only TOTP or a recovery code, neither of which that staff member ever finished setting up. That's a hard lockout, fixable only by manually nulling the columns.

Could you have renameLegacyColumns() null two_factor_secret / two_factor_recovery_codes where two_factor_confirmed_at IS NULL (when that column is present) before renaming/dropping, and add a test for the unconfirmed-enrolment row? The 1.5 rename migration had the same latent flaw, but for 1.5+ stores the column is already gone — the pre-1.5 path is the one chance to get this right.

Two minor things while you're in there (take or leave):

• After the rename, the data pass bails unless both app_authentication_* columns exist, but renameLegacyColumns() deliberately guards each column independently for part-way-fixed stores — so a secret-only store gets renamed but never re-encoded. Either handle the columns independently in the data pass too, or simplify the rename guard to match.
• The ->orderBy('id') before chunkById() is redundant — chunkById() applies its own id ordering.

The test coverage is otherwise really solid — thanks for the mixed-pass and idempotency cases in particular.

[glennjacobs]

Thanks for this — it's a really well put-together fix. The gating, idempotency probes, and the decision to discard unconfirmed enrolments before the rename are all spot on, and the test coverage is thorough. I've run the upgrade suite, PHPStan, and Pint against the branch locally and everything passes.

One change I'd like before merging, plus a smaller optional one:

Guard the rename against the target column already existing. In renameLegacyColumns() each rename is gated on the source column being present, but not on the target being absent. The migration already caters to stores part-way through a manual fix — and a store that manually copied two_factor_secret into app_authentication_secret (rather than renaming it) will carry both columns, at which point renameColumn() throws and aborts the whole upgrade. Could you guard each rename with && ! Schema::hasColumn($staff, 'app_authentication_secret') (and the same for the recovery-codes pair)? When both exist, skipping the rename is enough — the data pass will still reconcile whatever's in the new column. A test for that shape would be great too.

Optional: surface skipped undecryptable rows. The DecryptException comment says the staff member "re-enrols", but in practice they can't self-serve that — the encrypted cast throws when the challenge reads the stale secret, so they're stuck until an admin clears the columns. Leaving the data untouched is the right call (that row was equally broken in v1, and clearing it would be a silent 2FA downgrade), but a Log::warning with the affected row id would let the operator know who needs manual attention. At minimum, rewording the comment would avoid overpromising.