diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 5563fc6a59..20c971013c 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -4153,18 +4153,36 @@ App::get('/v1/account/mfa/factors') ]) ->inject('response') ->inject('user') - ->action(function (Response $response, Document $user) { + ->inject('session') + ->action(function (Response $response, Document $user, Document $session) { $mfaRecoveryCodes = $user->getAttribute('mfaRecoveryCodes', []); - $recoveryCodeEnabled = \is_array($mfaRecoveryCodes) && \count($mfaRecoveryCodes) > 0; + $isRecoveryCodeEnabled = \is_array($mfaRecoveryCodes) && \count($mfaRecoveryCodes) > 0; $totp = TOTP::getAuthenticatorFromUser($user); + + $isTotpEnabled = $totp !== null && $totp->getAttribute('verified', false); + $isEmailEnabled = $user->getAttribute('email', false) && $user->getAttribute('emailVerification', false); + $isPhoneEnabled = $user->getAttribute('phone', false) && $user->getAttribute('phoneVerification', false); + + // Disallow email or phone as 2nd factor, if it was used as 1st factor already + // This is just for informative purposes, actual protection lies in unique check when adding a factor + if(!is_null($session)) { + $existingFactors = $session->getAttribute('factors', []); + + if(\in_array(Type::EMAIL, $existingFactors) && $isEmailEnabled) { + $isEmailEnabled = false; + } + if(\in_array(Type::PHONE, $existingFactors) && $isPhoneEnabled) { + $isPhoneEnabled = false; + } + } $factors = new Document([ - Type::TOTP => $totp !== null && $totp->getAttribute('verified', false), - Type::EMAIL => $user->getAttribute('email', false) && $user->getAttribute('emailVerification', false), - Type::PHONE => $user->getAttribute('phone', false) && $user->getAttribute('phoneVerification', false), - Type::RECOVERY_CODE => $recoveryCodeEnabled + Type::TOTP => $isTotpEnabled, + Type::EMAIL => $isEmailEnabled, + Type::PHONE => $isPhoneEnabled, + Type::RECOVERY_CODE => $isRecoveryCodeEnabled ]); $response->dynamic($factors, Response::MODEL_MFA_FACTORS); diff --git a/app/controllers/shared/api.php b/app/controllers/shared/api.php index 959ee77b7d..7b4a8d295b 100644 --- a/app/controllers/shared/api.php +++ b/app/controllers/shared/api.php @@ -2,6 +2,7 @@ use Appwrite\Auth\Auth; use Appwrite\Auth\Key; +use Appwrite\Auth\MFA\Type; use Appwrite\Auth\MFA\Type\TOTP; use Appwrite\Event\Audit; use Appwrite\Event\Build; @@ -397,7 +398,23 @@ App::init() $hasVerifiedEmail = $user->getAttribute('emailVerification', false); $hasVerifiedPhone = $user->getAttribute('phoneVerification', false); $hasVerifiedAuthenticator = TOTP::getAuthenticatorFromUser($user)?->getAttribute('verified') ?? false; - $hasMoreFactors = $hasVerifiedEmail || $hasVerifiedPhone || $hasVerifiedAuthenticator; + + $availableFactors = 0; + + if($hasVerifiedAuthenticator) { + $availableFactors++; + } + + $usedFactors = \is_null($session) ? [] : $session->getAttribute('factors', []); + + if($hasVerifiedEmail && !\in_array(Type::EMAIL, $usedFactors)) { + $availableFactors++; + } + if($hasVerifiedPhone && !\in_array(Type::PHONE, $usedFactors)) { + $availableFactors++; + } + + $hasMoreFactors = $availableFactors > 0; $minimumFactors = ($mfaEnabled && $hasMoreFactors) ? 2 : 1; if (!in_array('mfa', $route->getGroups())) {