bert.hausmans/crewli
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: setupTotp() clobbering mfa_confirmed_at breaks login MFA check

Browse Source 
When MFA was already enabled and the user clicked "Opnieuw instellen"
on the TOTP card, setupTotp() unconditionally set mfa_confirmed_at to
null. If the user then cancelled the dialog without confirming, the
login controller's check `mfa_enabled && mfa_confirmed_at` evaluated
to false (true && null), silently skipping the MFA challenge.

Fix: only set mfa_method and mfa_confirmed_at when MFA is not yet
enabled (first-time setup). For re-setup or adding TOTP as a second
method, only rotate the mfa_secret — matching the guard already
applied to setupEmail().

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
bert.hausmans
2026-04-15 23:02:37 +02:00
parent a9c84ee0a6
commit 63a13c0ce9
1 changed files with 11 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
api/app/Services/MfaService.php
View File

@@ -34,16 +34,22 @@ final class MfaService
/**
* Begin TOTP setup: generate a secret and return QR code data.
* MFA is NOT yet active — user must confirm with a valid code.
* If MFA is already enabled (re-setup or adding as second method),
* only the secret is rotated — confirmed_at and preferred method
* are preserved so a cancelled setup doesn't break the login flow.
*/
public function setupTotp(User $user): array
{
$secret = $this->google2fa->generateSecretKey(32);
$user->update([
'mfa_secret' => encrypt($secret),
'mfa_method' => MfaMethod::TOTP->value,
'mfa_confirmed_at' => null,
]);
$updateData = ['mfa_secret' => encrypt($secret)];
if (! $user->mfa_enabled) {
$updateData['mfa_method'] = MfaMethod::TOTP->value;
$updateData['mfa_confirmed_at'] = null;
}
$user->update($updateData);
$qrCodeUrl = $this->google2fa->getQRCodeUrl(
company: 'Crewli',