From 09ec48af36e8a5b8af1927f49a018ed79d310fd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20de=20la=20Pen=CC=83a?= Date: Tue, 22 Sep 2026 21:04:14 +0100 Subject: [PATCH 1/4] feat: Remove the admin controls for legacy MFA Security questions and authenticator devices are no longer reset from the user edit form, because that MFA now lives in module-multi-factor-auth. --- .../language/english/admin_accounts_lang.php | 10 ---- admin/views/Accounts/edit/inc-mfa-device.php | 17 ------- .../views/Accounts/edit/inc-mfa-question.php | 17 ------- src/Auth/Admin/User/Tab/Security.php | 50 +------------------ src/Model/User.php | 44 ++-------------- 5 files changed, 6 insertions(+), 132 deletions(-) delete mode 100644 admin/views/Accounts/edit/inc-mfa-device.php delete mode 100644 admin/views/Accounts/edit/inc-mfa-question.php diff --git a/admin/language/english/admin_accounts_lang.php b/admin/language/english/admin_accounts_lang.php index 8c10343c..737789ef 100644 --- a/admin/language/english/admin_accounts_lang.php +++ b/admin/language/english/admin_accounts_lang.php @@ -41,16 +41,6 @@ $lang['accounts_edit_password_legend'] = 'Password'; -$lang['accounts_edit_mfa_question_legend'] = 'Multi Factor Authentication: Questions'; -$lang['accounts_edit_mfa_question_field_reset_label'] = 'Set new qustions on next log in'; -$lang['accounts_edit_mfa_question_field_reset_yes'] = 'Yes, require user to set new security questions on next log in.'; -$lang['accounts_edit_mfa_question_field_reset_no'] = 'No, do not require user to set new security questions on next log in.'; - -$lang['accounts_edit_mfa_device_legend'] = 'Multi Factor Authentication: Device'; -$lang['accounts_edit_mfa_device_field_reset_label'] = 'Setup a new device on next log in'; -$lang['accounts_edit_mfa_device_field_reset_yes'] = 'Yes, require user to setup a new security device on next log in.'; -$lang['accounts_edit_mfa_device_field_reset_no'] = 'No, do not require user to setup a new security device on next log in.'; - $lang['accounts_edit_basic_legend'] = 'Basic Information'; $lang['accounts_edit_basic_field_first_placeholder'] = 'The user\'s first name'; $lang['accounts_edit_basic_field_last_placeholder'] = 'The user\'s surname'; diff --git a/admin/views/Accounts/edit/inc-mfa-device.php b/admin/views/Accounts/edit/inc-mfa-device.php deleted file mode 100644 index b3f4974f..00000000 --- a/admin/views/Accounts/edit/inc-mfa-device.php +++ /dev/null @@ -1,17 +0,0 @@ - 'reset_mfa_device', - 'label' => lang('accounts_edit_mfa_device_field_reset_label'), - 'options' => [ - [ - 'value' => true, - 'label' => lang('accounts_edit_mfa_device_field_reset_yes'), - 'selected' => set_radio('reset_mfa_device') ? true : false, - ], - [ - 'value' => false, - 'label' => lang('accounts_edit_mfa_device_field_reset_no'), - 'selected' => !set_radio('reset_mfa_device') ? true : false, - ], - ], -]); diff --git a/admin/views/Accounts/edit/inc-mfa-question.php b/admin/views/Accounts/edit/inc-mfa-question.php deleted file mode 100644 index 7d1f530c..00000000 --- a/admin/views/Accounts/edit/inc-mfa-question.php +++ /dev/null @@ -1,17 +0,0 @@ - 'reset_mfa_question', - 'label' => lang('accounts_edit_mfa_question_field_reset_label'), - 'options' => [ - [ - 'value' => true, - 'label' => lang('accounts_edit_mfa_question_field_reset_yes'), - 'selected' => set_radio('reset_mfa_question') ? true : false, - ], - [ - 'value' => false, - 'label' => lang('accounts_edit_mfa_question_field_reset_no'), - 'selected' => !set_radio('reset_mfa_question') ? true : false, - ], - ], -]); diff --git a/src/Auth/Admin/User/Tab/Security.php b/src/Auth/Admin/User/Tab/Security.php index 79c6a379..ceb64a86 100644 --- a/src/Auth/Admin/User/Tab/Security.php +++ b/src/Auth/Admin/User/Tab/Security.php @@ -7,7 +7,6 @@ use Nails\Auth\Model\User\Password; use Nails\Auth\Resource\User; use Nails\Common\Exception\ValidationException; -use Nails\Common\Service\Config; use Nails\Common\Service\Input; use Nails\Common\Service\View; use Nails\Factory; @@ -59,20 +58,12 @@ public function getBody(User $oUser): string { /** @var View $oView */ $oView = Factory::service('View'); - /** @var Config $oConfig */ - $oConfig = Factory::service('Config'); /** @var Password $oUserPasswordModel */ $oUserPasswordModel = Factory::model('UserPassword', Constants::MODULE_SLUG); - $oConfig->load('auth/auth'); - return $oView ->load( - array_filter([ - 'Accounts/edit/inc-password', - $oConfig->item('authTwoFactorMode') == 'QUESTION' ? 'Accounts/edit/inc-mfa-question' : null, - $oConfig->item('authTwoFactorMode') == 'DEVICE' ? 'Accounts/edit/inc-mfa-device' : null, - ]), + ['Accounts/edit/inc-password'], [ 'oUser' => $oUser, 'sPasswordRules' => $oUserPasswordModel->getRulesAsString($oUser->group_id), @@ -109,13 +100,9 @@ public function getValidationRules(User $oUser): array { /** @var Input $oInput */ $oInput = Factory::service('Input'); - /** @var Config $oConfig */ - $oConfig = Factory::service('Config'); /** @var Password $oUserPasswordModel */ $oUserPasswordModel = Factory::model('UserPassword', Constants::MODULE_SLUG); - $oConfig->load('auth/auth'); - $aRules = [ 'temp_pw' => [], ]; @@ -130,15 +117,6 @@ function ($sPassword) use ($oUser, $oUserPasswordModel) { ]; } - switch ($oConfig->item('authTwoFactorMode')) { - case 'QUESTION': - $aRules['reset_mfa_question'] = []; - break; - case 'DEVICE': - $aRules['reset_mfa_device'] = []; - break; - } - return $aRules; } @@ -154,33 +132,9 @@ function ($sPassword) use ($oUser, $oUserPasswordModel) { */ public function getPostData(User $oUser, array $aPost): array { - /** @var Config $oConfig */ - $oConfig = Factory::service('Config'); - - $aData = [ + return [ 'password' => getFromArray('password', $aPost), 'temp_pw' => (bool) getFromArray('temp_pw', $aPost), ]; - - switch ($oConfig->item('authTwoFactorMode')) { - case 'QUESTION': - $aData = array_merge( - $aData, - [ - 'reset_mfa_question' => (bool) getFromArray('reset_mfa_question', $aPost), - ] - ); - break; - case 'DEVICE': - $aData = array_merge( - $aData, - [ - 'reset_mfa_device' => (bool) getFromArray('reset_mfa_device', $aPost), - ] - ); - break; - } - - return $aData; } } diff --git a/src/Model/User.php b/src/Model/User.php index 47373929..324b202a 100644 --- a/src/Model/User.php +++ b/src/Model/User.php @@ -984,12 +984,10 @@ public function update($iUserId = null, array $aData = []): bool unset($aData['password']); // Set the data - $aDataUser = []; - $aDataMeta = []; - $sDataEmail = ''; - $sDataUsername = ''; - $bResetMfaQuestion = false; - $bResetMfaDevice = false; + $aDataUser = []; + $aDataMeta = []; + $sDataEmail = ''; + $sDataUsername = ''; foreach ($aData as $key => $val) { @@ -1023,10 +1021,6 @@ public function update($iUserId = null, array $aData = []): bool $sDataEmail = strtolower(trim($val)); } elseif ($key == 'username') { $sDataUsername = strtolower(trim($val)); - } elseif ($key == 'reset_mfa_question') { - $bResetMfaQuestion = $val; - } elseif ($key == 'reset_mfa_device') { - $bResetMfaDevice = $val; } else { $aDataMeta[$key] = $val; } @@ -1064,36 +1058,6 @@ public function update($iUserId = null, array $aData = []): bool // -------------------------------------------------------------------------- - // Resetting 2FA? - if ($bResetMfaQuestion || $bResetMfaDevice) { - - /** @var \Nails\Common\Service\Config $oConfig */ - $oConfig = Factory::service('Config'); - $oConfig->load('auth/auth'); - $sTwoFactorMode = $oConfig->item('authTwoFactorMode'); - - if ($sTwoFactorMode == 'QUESTION' && $bResetMfaQuestion) { - - $oDb->where('user_id', $iUserId); - if (!$oDb->delete(Config::get('NAILS_DB_PREFIX') . 'user_auth_two_factor_question')) { - $oDb->transaction()->rollback(); - $this->setError('Could not reset user\'s Multi Factor Authentication questions.'); - return false; - } - - } elseif ($sTwoFactorMode == 'DEVICE' && $bResetMfaDevice) { - - $oDb->where('user_id', $iUserId); - if (!$oDb->delete(Config::get('NAILS_DB_PREFIX') . 'user_auth_two_factor_device_secret')) { - $oDb->transaction()->rollback(); - $this->setError('Could not reset user\'s Multi Factor Authentication device.'); - return false; - } - } - } - - // -------------------------------------------------------------------------- - // Update the user table $oDb->where('id', $iUserId); $oDb->set('last_update', 'NOW()', false); From 9847f7b1b230d101bc8a17938de85b764ea5295f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20de=20la=20Pen=CC=83a?= Date: Tue, 22 Sep 2026 21:04:21 +0100 Subject: [PATCH 2/4] feat: Remove legacy question and device MFA from auth Login and password reset no longer challenge with auth's own security questions or authenticator devices, and the abandoned Google Authenticator dependency goes with them. --- auth/config/auth.php | 24 - auth/config/auth.twofactor.php | 56 -- auth/controllers/Login.php | 75 --- auth/controllers/MfaDevice.php | 196 ------- auth/controllers/MfaQuestion.php | 260 --------- auth/controllers/PasswordForgotten.php | 256 +-------- auth/controllers/PasswordReset.php | 99 +--- auth/controllers/Register.php | 1 - auth/language/english/auth_lang.php | 25 +- auth/views/mfa/device/ask.php | 55 -- auth/views/mfa/device/setup.php | 61 --- auth/views/mfa/question/ask.php | 62 --- auth/views/mfa/question/set.php | 126 ----- auth/views/password/change_temp.php | 46 -- .../password/forgotten_security_question.php | 41 -- composer.json | 1 - src/Controller/BaseMfa.php | 197 ------- src/Exception/Login/RequiresMfaException.php | 14 - src/Housekeeping/TwoFactorTokens.php | 87 --- src/Routes.php | 2 - src/Service/Authentication.php | 511 +----------------- tests/RoutesTest.php | 8 +- 22 files changed, 26 insertions(+), 2177 deletions(-) delete mode 100644 auth/config/auth.twofactor.php delete mode 100644 auth/controllers/MfaDevice.php delete mode 100644 auth/controllers/MfaQuestion.php delete mode 100644 auth/views/mfa/device/ask.php delete mode 100644 auth/views/mfa/device/setup.php delete mode 100644 auth/views/mfa/question/ask.php delete mode 100644 auth/views/mfa/question/set.php delete mode 100644 auth/views/password/forgotten_security_question.php delete mode 100644 src/Controller/BaseMfa.php delete mode 100644 src/Exception/Login/RequiresMfaException.php delete mode 100644 src/Housekeeping/TwoFactorTokens.php diff --git a/auth/config/auth.php b/auth/config/auth.php index eff4e53d..575d9060 100644 --- a/auth/config/auth.php +++ b/auth/config/auth.php @@ -36,27 +36,3 @@ * On login show the last known IP of the user */ $config['authShowLastIpOnLogin'] = false; - -// -------------------------------------------------------------------------- - -/** - * Auth sub config files - * Load both versions, app version overrides Nails version - */ -$sAppPath = NAILS_APP_PATH . 'application/modules/auth/config/'; -$sNailsPath = NAILS_PATH . 'module-auth/auth/config/'; - -$aFiles = [ - 'auth.twofactor.php', -]; - -foreach ($aFiles as $sFile) { - - if (file_exists($sNailsPath . $sFile)) { - include $sNailsPath . $sFile; - } - - if (file_exists($sAppPath . $sFile)) { - include $sAppPath . $sFile; - } -} diff --git a/auth/config/auth.twofactor.php b/auth/config/auth.twofactor.php deleted file mode 100644 index d63f8426..00000000 --- a/auth/config/auth.twofactor.php +++ /dev/null @@ -1,56 +0,0 @@ - [ - // The number of system questions a user must have - 'numQuestions' => 1, - - // The number of user questions a user must have - 'numUserQuestions' => 0, - - // The questions the system can use - 'questions' => [ - 'What was your childhood nickname? ', - 'In what city did you meet your spouse/significant other?', - 'What is the name of your favorite childhood friend? ', - 'What is the middle name of your oldest child?', - 'What is your oldest sibling\'s middle name?', - 'What was your childhood phone number including area code?', - 'What is your oldest cousin\'s first and last name?', - 'What was the name of your first stuffed animal?', - 'In what city or town did your mother and father meet? ', - 'Where were you when you had your first kiss? ', - 'What is the first name of the boy or girl that you first kissed?', - 'In what city does your nearest sibling live? ', - 'What is your oldest sibling\'s birthday month and year? (e.g., January 1900) ', - 'What is your oldest brother\'s birthday month and year? (e.g., January 1900) ', - 'What is your oldest sister\'s birthday month and year? (e.g., January 1900) ', - 'What is your maternal grandmother\'s maiden name?', - 'In what city or town was your first job?', - 'What is the name of the place your wedding reception was held?', - 'What is the name of a college or university you applied to but didn\'t attend?', - 'Where were you when you first heard about 9/11?', - ], - ], - 'DEVICE' => [], -]; diff --git a/auth/controllers/Login.php b/auth/controllers/Login.php index 03c1502c..bd2283b1 100644 --- a/auth/controllers/Login.php +++ b/auth/controllers/Login.php @@ -14,7 +14,6 @@ use Nails\Auth\Controller\Base; use Nails\Auth\Exception\AuthException; use Nails\Auth\Exception\Login\NoUserException; -use Nails\Auth\Exception\Login\RequiresMfaException; use Nails\Auth\Exception\Login\RequiresPasswordResetExpiredException; use Nails\Auth\Exception\Login\RequiresPasswordResetTempException; use Nails\Auth\Model\User\Password; @@ -151,9 +150,6 @@ public function index() } catch (NoUserException $e) { $this->oUserFeedback->error($e->getMessage()); - } catch (RequiresMfaException $e) { - $this->handleMfa($oUser); - } catch (RequiresPasswordResetTempException $e) { $this->handlePasswordReset($oUser, $bRemember, 'TEMP'); @@ -217,8 +213,6 @@ protected function handleLogin(Resource\User $oUser, bool $bRemember = false, st $oConfig = Factory::service('Config'); /** @var Password $oUserPasswordModel */ $oUserPasswordModel = Factory::model('UserPassword', Constants::MODULE_SLUG); - /** @var Authentication $oAuthService */ - $oAuthService = Factory::service('Authentication', Constants::MODULE_SLUG); if (!empty($oUser->temp_pw)) { @@ -228,10 +222,6 @@ protected function handleLogin(Resource\User $oUser, bool $bRemember = false, st $this->handlePasswordReset($oUser, $bRemember, 'EXPIRED'); - } elseif ($oConfig->item('authTwoFactorMode')) { - - $this->handleMfa($oUser); - } else { // Finally! Send this user on their merry way... @@ -283,8 +273,6 @@ protected function handleLogin(Resource\User $oUser, bool $bRemember = false, st /** * Whether to offer this user a passkey before sending them on their way * - * An MFA-challenged login never returns through here. - * * @throws FactoryException */ protected function shouldNudgeForPasskey(Resource\User $oUser): bool @@ -308,69 +296,6 @@ protected function shouldNudgeForPasskey(Resource\User $oUser): bool // -------------------------------------------------------------------------- - /** - * Handle MFA redirect - * - * @param Resource\User $oUser The user who requires MFA - * @param bool $bRemember Whether to set the rememberMe cookie or not - * - * @throws AuthException - * @throws FactoryException - */ - protected function handleMfa(Resource\User $oUser, bool $bRemember = false): void - { - /** @var Authentication $oAuthService */ - $oAuthService = Factory::service('Authentication', Constants::MODULE_SLUG); - /** @var Config $oConfig */ - $oConfig = Factory::service('Config'); - - $aTwoFactorToken = $oAuthService->mfaTokenGenerate($oUser->id); - - if (!$aTwoFactorToken) { - throw new AuthException( - 'A user tried to login and the system failed to generate a two-factor auth token.' - ); - } - - // Is there any query data? - $aQuery = array_filter([ - 'return_to' => $this->data['return_to'] ?: null, - 'remember' => $bRemember, - ]); - - $sQuery = !empty($aQuery) ? '?' . http_build_query($aQuery) : ''; - - // Where we sending the user? - switch ($oConfig->item('authTwoFactorMode')) { - - case 'QUESTION': - $sController = 'mfa/question'; - break; - - case 'DEVICE': - $sController = 'mfa/device'; - break; - - default: - throw new AuthException('"' . $oConfig->item('authTwoFactorMode') . '" is not a valid MFA Mode'); - break; - } - - // Compile the URL - $aUrl = [ - 'auth', - $sController, - $oUser->id, - $aTwoFactorToken['salt'], - $aTwoFactorToken['token'], - ]; - - // Login was successful, redirect to the appropriate MFA page - redirect(implode('/', $aUrl) . $sQuery); - } - - // -------------------------------------------------------------------------- - /** * @param Resource\User $oUser The user who is resetting their password * @param bool $bRemember Whether to set the rememberMe cookie or not diff --git a/auth/controllers/MfaDevice.php b/auth/controllers/MfaDevice.php deleted file mode 100644 index 68fb126d..00000000 --- a/auth/controllers/MfaDevice.php +++ /dev/null @@ -1,196 +0,0 @@ -authMfaMode == 'DEVICE') { - $this->index(); - } else { - show404(); - } - } - - // -------------------------------------------------------------------------- - - /** - * Remaps requests to the correct method - * - * @throws FactoryException - */ - public function index() - { - // Validates the request token and generates a new one for the next request - $this->validateToken(); - - // -------------------------------------------------------------------------- - - // Has this user already set up an MFA? - /** @var Authentication $oAuthService */ - $oAuthService = Factory::service('Authentication', Constants::MODULE_SLUG); - $oMfaDevice = $oAuthService->mfaDeviceSecretGet($this->mfaUser->id); - - if ($oMfaDevice) { - $this->requestCode(); - } else { - $this->setupDevice(); - } - } - - // -------------------------------------------------------------------------- - - /** - * Sets up a new MFA device - * - * @throws FactoryException - */ - protected function setupDevice() - { - /** @var Authentication $oAuthService */ - $oAuthService = Factory::service('Authentication', Constants::MODULE_SLUG); - /** @var Input $oInput */ - $oInput = Factory::service('Input'); - - if ($oInput->post()) { - - /** @var FormValidation $oFormValidation */ - $oFormValidation = Factory::service('FormValidation'); - - try { - - $oFormValidation - ->buildValidator([ - 'mfa_secret' => [FormValidation::RULE_REQUIRED], - 'mfa_code' => [FormValidation::RULE_REQUIRED], - ]) - ->run(); - - $sSecret = $oInput->post('mfa_secret'); - $sMfaCode = $oInput->post('mfa_code'); - - // Verify the inout - if ($oAuthService->mfaDeviceSecretValidate($this->mfaUser->id, $sSecret, $sMfaCode)) { - - // Codes have been validated and saved to the DB, sign the user in and move on - $this->oUserFeedback->success( - 'Multi Factor Authentication Enabled!
You successfully ' . - 'associated an MFA device with your account. You will be required to use it ' . - 'the next time you log in.' - ); - - $this->loginUser(); - - } else { - $this->oUserFeedback->error('Sorry, that code failed to validate. Please try again.'); - } - - } catch (ValidationException $e) { - $this->oUserFeedback->error($e->getMessage()); - } - } - - // Generate the secret - $this->data['secret'] = $oAuthService->mfaDeviceSecretGenerate( - $this->mfaUser->id, - $oInput->post('mfa_secret', true) - ); - - if (!$this->data['secret']) { - $this->oUserFeedback->error('Sorry, it has not been possible to get an MFA device set up for this user. ' . $oAuthService->lastError()); - redirect(loginUrl($this->returnTo ?: false)); - } - - // -------------------------------------------------------------------------- - - $this->oMetaData->setTitles(['Set up a new MFA device']); - $this->loadStyles(NAILS_APP_PATH . 'application/modules/auth/views/mfa/device/setup.php'); - Factory::service('View') - ->load([ - 'structure/header/blank', - 'auth/mfa/device/setup', - 'structure/footer/blank', - ]); - } - - // -------------------------------------------------------------------------- - - /** - * Requests a code from the user - * - * @throws FactoryException - */ - protected function requestCode() - { - /** @var Input $oInput */ - $oInput = Factory::service('Input'); - if ($oInput->post()) { - - /** @var FormValidation $oFormValidation */ - $oFormValidation = Factory::service('FormValidation'); - - try { - - $oFormValidation - ->buildValidator([ - 'mfa_code' => [FormValidation::RULE_REQUIRED], - ]) - ->run(); - - /** @var Authentication $oAuthService */ - $oAuthService = Factory::service('Authentication', Constants::MODULE_SLUG); - $sMfaCode = $oInput->post('mfa_code'); - - // Verify the inout - if ($oAuthService->mfaDeviceCodeValidate($this->mfaUser->id, $sMfaCode)) { - $this->loginUser(); - } else { - $this->oUserFeedback->error(sprintf( - 'Sorry, that code failed to validate. Please try again. %s', - $oAuthService->lastError() - )); - } - - } catch (ValidationException $e) { - $this->oUserFeedback->error($e->getMessage()); - } - } - - // -------------------------------------------------------------------------- - - $this->oMetaData->setTitles(['Enter your code']); - $this->loadStyles(NAILS_APP_PATH . 'application/modules/auth/views/mfa/device/ask.php'); - Factory::service('View') - ->load([ - 'structure/header/blank', - 'auth/mfa/device/ask', - 'structure/footer/blank', - ]); - } -} diff --git a/auth/controllers/MfaQuestion.php b/auth/controllers/MfaQuestion.php deleted file mode 100644 index d827e66d..00000000 --- a/auth/controllers/MfaQuestion.php +++ /dev/null @@ -1,260 +0,0 @@ -authMfaMode == 'QUESTION') { - $this->index(); - } else { - show404(); - } - } - - // -------------------------------------------------------------------------- - - /** - * Sets up, or asks an MFA Question - * - * @throws FactoryException - */ - public function index() - { - // Validates the request token and generates a new one for the next request - $this->validateToken(); - - // -------------------------------------------------------------------------- - - /** @var Input $oInput */ - $oInput = Factory::service('Input'); - /** @var Authentication $oAuthService */ - $oAuthService = Factory::service('Authentication', Constants::MODULE_SLUG); - - if ($oInput->post('answer')) { - - /** - * Validate the answer, if correct then log user in and forward, if - * not then generate a new token and show errors - */ - - $this->data['question'] = $oAuthService->mfaQuestionGet($this->mfaUser->id); - $bIsValid = $oAuthService->mfaQuestionValidate( - $this->data['question']->id, - $this->mfaUser->id, - $oInput->post('answer') - ); - - if ($bIsValid) { - $this->loginUser(); - } else { - $this->oUserFeedback->error(lang('auth_twofactor_answer_incorrect')); - $this->askQuestion(); - } - - } else { - - // Determine whether the user has any security questions set - $this->data['question'] = $oAuthService->mfaQuestionGet($this->mfaUser->id); - - if ($this->data['question']) { - - // Ask away cap'n! - $this->askQuestion(); - - } else { - - // Fetch the security questions - $this->data['questions'] = $this->authMfaConfig['questions']; - - /** - * Determine how many questions a user must have, if the number of questions - * is smaller than the number of questions available, use the smaller. - */ - if (count($this->data['questions']) < $this->authMfaConfig['numQuestions']) { - $this->data['num_questions'] = count($this->data['questions']); - } else { - $this->data['num_questions'] = $this->authMfaConfig['numQuestions']; - } - - // The number of user generated questions a user must have - $this->data['num_custom_questions'] = $this->authMfaConfig['numUserQuestions']; - - if ($this->data['num_questions'] + $this->data['num_custom_questions'] <= 0) { - throw new NailsException('Two-factor auth is enabled, but no questions available'); - } - - if ($oInput->post()) { - - /** @var FormValidation $oFormValidation */ - $oFormValidation = Factory::service('FormValidation'); - - $aRules = []; - - for ($i = 0; $i < $this->data['num_questions']; $i++) { - $aRules['question[' . $i . '][question]'] = [ - FormValidation::RULE_REQUIRED, - FormValidation::RULE_IS_NATURAL_NO_ZERO, - ]; - $aRules['question[' . $i . '][answer]'] = ['trim', FormValidation::RULE_REQUIRED]; - } - - for ($i = 0; $i < $this->data['num_custom_questions']; $i++) { - $aRules['custom_question[' . $i . '][question]'] = ['trim', FormValidation::RULE_REQUIRED]; - $aRules['custom_question[' . $i . '][answer]'] = ['trim', FormValidation::RULE_REQUIRED]; - } - - try { - - $oValidator = $oFormValidation->buildValidator( - $aRules, - [FormValidation::RULE_IS_NATURAL_NO_ZERO => lang('fv_required')] - ); - $oValidator->run(); - - // The validated data carries the trimmed values - $aPost = $oValidator->getValidatedData(); - - // Make sure that we have different questions - $aQuestionIndex = []; - $aQuestion = array_filter((array) ($aPost['question'] ?? [])); - $bError = false; - - foreach ($aQuestion as $q) { - - if (!in_array($q['question'], $aQuestionIndex)) { - $aQuestionIndex[] = $q['question']; - } else { - $bError = true; - break; - } - } - - $aQuestionIndex = []; - $aQuestion = array_filter((array) ($aPost['custom_question'] ?? [])); - - foreach ($aQuestion as $q) { - if (array_search($q['question'], $aQuestionIndex) === false) { - $aQuestionIndex[] = $q['question']; - } else { - $bError = true; - break; - } - } - - if (!$bError) { - - // Good arrows. Save questions - $aData = []; - - if (!empty($aPost['question'])) { - - foreach ($aPost['question'] as $q) { - - $oTemp = new stdClass(); - - if (isset($this->data['questions'][$q['question'] - 1])) { - $oTemp->question = $this->data['questions'][$q['question'] - 1]; - } else { - $oTemp->question = null; - } - $oTemp->answer = $q['answer']; - - $aData[] = $oTemp; - } - } - - if (!empty($aPost['custom_question'])) { - foreach ((array) $aPost['custom_question'] as $aQuestion) { - $aData[] = (object) [ - 'question' => trim($aQuestion['question']), - 'answer' => $aQuestion['answer'], - ]; - } - } - - if ($oAuthService->mfaQuestionSet($this->mfaUser->id, $aData)) { - - $this->oUserFeedback->success( - 'Multi Factor Authentication Enabled!
You successfully ' . - 'set your security questions. You will be asked to answer one of them every time ' . - 'you log in.' - ); - - $this->loginUser(); - - } else { - $oUserModel = Factory::model('User', Constants::MODULE_SLUG); - $this->oUserFeedback->error(sprintf( - '%s %s', - lang('auth_twofactor_question_set_fail'), - $oUserModel->lastError() - )); - } - - } else { - $this->oUserFeedback->error(lang('auth_twofactor_question_unique')); - } - - } catch (ValidationException $e) { - $this->oUserFeedback->error($e->getMessage()); - } - } - - // No questions, request they set them - $this->oMetaData->setTitles([lang('auth_twofactor_question_set_title')]); - $this->loadStyles(NAILS_APP_PATH . 'application/modules/auth/views/mfa/question/set.php'); - Factory::service('View') - ->load([ - 'structure/header/blank', - 'auth/mfa/question/set', - 'structure/footer/blank', - ]); - } - } - } - - // -------------------------------------------------------------------------- - - /** - * Asks one of the user's questions - * - * @throws FactoryException - */ - protected function askQuestion() - { - // Ask away cap'n! - $this->oMetaData->setTitles([lang('auth_twofactor_answer_title')]); - $this->loadStyles(NAILS_APP_PATH . 'application/modules/auth/views/mfa/question/ask.php'); - Factory::service('View') - ->load([ - 'structure/header/blank', - 'auth/mfa/question/ask', - 'structure/footer/blank', - ]); - } -} diff --git a/auth/controllers/PasswordForgotten.php b/auth/controllers/PasswordForgotten.php index e41c79a2..8705ad7a 100644 --- a/auth/controllers/PasswordForgotten.php +++ b/auth/controllers/PasswordForgotten.php @@ -8,7 +8,6 @@ * @category Controller * @author Nails Dev Team * @link - * @todo Refactor this class so that not so much code is being duplicated, especially re: MFA */ use Nails\Auth\Constants; @@ -16,10 +15,7 @@ use Nails\Auth\Factory\Email\ForgottenPassword; use Nails\Auth\Model\User; use Nails\Auth\Model\User\Password; -use Nails\Auth\Service\Authentication; use Nails\Auth\Validator\User\Identifier; -use Nails\Common\Exception\Encrypt\DecodeException; -use Nails\Common\Exception\EnvironmentException; use Nails\Common\Exception\FactoryException; use Nails\Common\Exception\NailsException; use Nails\Common\Service\Config; @@ -195,264 +191,50 @@ public function index() * @param string $sCode The code to validate * * @throws FactoryException - * @throws DecodeException - * @throws EnvironmentException */ public function _validate($sCode) { - /** @var Input $oInput */ - $oInput = Factory::service('Input'); - /** @var Config $oConfig */ - $oConfig = Factory::service('Config'); - /** @var Authentication $oAuthService */ - $oAuthService = Factory::service('Authentication', Constants::MODULE_SLUG); /** @var Password $oUserPasswordModel */ $oUserPasswordModel = Factory::model('UserPassword', Constants::MODULE_SLUG); - /** - * Attempt to verify code, if two factor auth is enabled then don't generate a - * new password, we'll need the user to jump through some hoops first. - */ - $bGenerateNewPw = !$oConfig->item('authTwoFactorMode'); - $mNewPassword = $oUserPasswordModel->validateToken($sCode, $bGenerateNewPw); + $mNewPassword = $oUserPasswordModel->validateToken($sCode, true); // -------------------------------------------------------------------------- - // Determine outcome of validation if ($mNewPassword === 'EXPIRED') { - // Code has expired $this->oUserFeedback->error(lang('auth_forgot_expired_code')); } elseif ($mNewPassword === false) { - // Code was invalid $this->oUserFeedback->error(lang('auth_forgot_invalid_code')); } else { - if ($oConfig->item('authTwoFactorMode') == 'QUESTION') { - - // Show them a security question - $this->data['question'] = $oAuthService->mfaQuestionGet($mNewPassword['user_id']); - - if ($this->data['question']) { - - if ($oInput->post()) { - - $bIsValid = $oAuthService->mfaQuestionValidate( - $this->data['question']->id, - $mNewPassword['user_id'], - $oInput->post('answer') - ); - - if ($bIsValid) { - - // Correct answer, reset password and render views - $mNewPassword = $oUserPasswordModel->validateToken($sCode, true); - - // @todo (Pablo - 2019-07-17) - Do failures need handled here? - - // -------------------------------------------------------------------------- - - // Set some flashdata for the login page when they go to it; just a little reminder - $this->oUserFeedback->warning(lang('auth_forgot_reminder', htmlentities($mNewPassword['password']))); - - // -------------------------------------------------------------------------- - - // Load the views - $this->loadStyles( - \Nails\Config::get('NAILS_APP_PATH') . 'application/modules/auth/views/password/forgotten_reset.php' - ); - - Factory::service('View') - ->setData([ - 'new_password' => $mNewPassword['password'], - 'user' => (object) [ - 'id' => $mNewPassword['user_id'], - 'identity' => $mNewPassword['user_identity'], - ], - ]) - ->load([ - 'structure/header/blank', - 'auth/password/forgotten_reset', - 'structure/footer/blank', - ]); - return; - - } else { - $this->oUserFeedback->error(lang('auth_twofactor_answer_incorrect')); - } - } - - $this->oMetaData->setTitles([lang('auth_title_forgotten_password_security_question')]); - - $this->loadStyles(\Nails\Config::get('NAILS_APP_PATH') . 'application/modules/auth/views/mfa/question/ask.php'); - - Factory::service('View') - ->load([ - 'structure/header/blank', - 'auth/mfa/question/ask', - 'structure/footer/blank', - ]); - - } else { - - // No questions, reset and load views - $mNewPassword = $oUserPasswordModel->validateToken($sCode, true); - - // @todo (Pablo - 2019-07-17) - Do failures need handled here? - - // -------------------------------------------------------------------------- - - // Set some flashdata for the login page when they go to it; just a little reminder - $this->oUserFeedback->warning(lang('auth_forgot_reminder', htmlentities($mNewPassword['password']))); - - // -------------------------------------------------------------------------- - - // Load the views - $this->loadStyles( - \Nails\Config::get('NAILS_APP_PATH') . 'application/modules/auth/views/password/forgotten_reset.php' - ); - - Factory::service('View') - ->setData([ - 'new_password' => $mNewPassword['password'], - 'user' => (object) [ - 'id' => $mNewPassword['user_id'], - 'identity' => $mNewPassword['user_identity'], - ], - ]) - ->load([ - 'structure/header/blank', - 'auth/password/forgotten_reset', - 'structure/footer/blank', - ]); - } - - } elseif ($oConfig->item('authTwoFactorMode') == 'DEVICE') { - - $mSecret = $oAuthService->mfaDeviceSecretGet($mNewPassword['user_id']); - - if ($mSecret) { + $this->oUserFeedback->warning(lang('auth_forgot_reminder', htmlentities($mNewPassword['password']))); - if ($oInput->post()) { - - $sMfaCode = $oInput->post('mfaCode'); - - // Verify the inout - if ($oAuthService->mfaDeviceCodeValidate($mNewPassword['user_id'], $sMfaCode)) { - - // Correct answer, reset password and render views - $mNewPassword = $oUserPasswordModel->validateToken($sCode, true); - - // @todo (Pablo - 2019-07-17) - Do failures need handled here? - - // -------------------------------------------------------------------------- - - // Set some flashdata for the login page when they go to it; just a little reminder - $this->oUserFeedback->warning(lang('auth_forgot_reminder', htmlentities($mNewPassword['password']))); - - // -------------------------------------------------------------------------- - - // Load the views - $this->loadStyles( - \Nails\Config::get('NAILS_APP_PATH') . 'application/modules/auth/views/password/forgotten_reset.php' - ); - - Factory::service('View') - ->setData([ - 'new_password' => $mNewPassword['password'], - 'user' => (object) [ - 'id' => $mNewPassword['user_id'], - 'identity' => $mNewPassword['user_identity'], - ], - ]) - ->load([ - 'structure/header/blank', - 'auth/password/forgotten_reset', - 'structure/footer/blank', - ]); - return; - - } else { - $this->oUserFeedback->error('Sorry, that code failed to validate. Please try again. ' . $oAuthService->lastError()); - } - } - - $this->oMetaData->setTitles(['Please enter the code from your device']); - - $this->loadStyles(\Nails\Config::get('NAILS_APP_PATH') . 'application/modules/auth/views/mfa/device/ask.php'); - - Factory::service('View') - ->load([ - 'structure/header/blank', - 'auth/mfa/device/ask', - 'structure/footer/blank', - ]); - - } else { - - // No devices, reset and load views - $mNewPassword = $oUserPasswordModel->validateToken($sCode, true); - - // @todo (Pablo - 2019-07-17) - Do failures need handled here? - - // -------------------------------------------------------------------------- - - // Set some flashdata for the login page when they go to it; just a little reminder - $this->oUserFeedback->warning(lang('auth_forgot_reminder', htmlentities($mNewPassword['password']))); - - // -------------------------------------------------------------------------- - - // Load the views - $this->loadStyles(\Nails\Config::get('NAILS_APP_PATH') . 'application/modules/auth/views/password/forgotten_reset.php'); - Factory::service('View') - ->setData([ - 'new_password' => $mNewPassword['password'], - 'user' => (object) [ - 'id' => $mNewPassword['user_id'], - 'identity' => $mNewPassword['user_identity'], - ], - ]) - ->load([ - 'structure/header/blank', - 'auth/password/forgotten_reset', - 'structure/footer/blank', - ]); - } - - } else { - - // Everything worked! - // Set some flashdata for the login page when they go to it; just a little reminder - $this->oUserFeedback->warning(lang('auth_forgot_reminder', htmlentities($mNewPassword['password']))); - - // -------------------------------------------------------------------------- - - // Load the views - $this->loadStyles(\Nails\Config::get('NAILS_APP_PATH') . 'application/modules/auth/views/password/forgotten_reset.php'); - Factory::service('View') - ->setData([ - 'new_password' => $mNewPassword['password'], - 'user' => (object) [ - 'id' => $mNewPassword['user_id'], - 'identity' => $mNewPassword['user_identity'], - ], - ]) - ->load([ - 'structure/header/blank', - 'auth/password/forgotten_reset', - 'structure/footer/blank', - ]); - } + $this->loadStyles( + \Nails\Config::get('NAILS_APP_PATH') . 'application/modules/auth/views/password/forgotten_reset.php' + ); + Factory::service('View') + ->setData([ + 'new_password' => $mNewPassword['password'], + 'user' => (object) [ + 'id' => $mNewPassword['user_id'], + 'identity' => $mNewPassword['user_identity'], + ], + ]) + ->load([ + 'structure/header/blank', + 'auth/password/forgotten_reset', + 'structure/footer/blank', + ]); return; } // -------------------------------------------------------------------------- - // Load the views $this->loadStyles(\Nails\Config::get('NAILS_APP_PATH') . 'application/modules/auth/views/password/forgotten.php'); Factory::service('View') ->load([ @@ -469,8 +251,6 @@ public function _validate($sCode) * * @param string $sMethod The method being called * - * @throws DecodeException - * @throws EnvironmentException * @throws FactoryException */ public function _remap($sMethod) diff --git a/auth/controllers/PasswordReset.php b/auth/controllers/PasswordReset.php index 877cae58..48b7c5d8 100644 --- a/auth/controllers/PasswordReset.php +++ b/auth/controllers/PasswordReset.php @@ -70,96 +70,7 @@ protected function validate($iUserId, $sHash) if ($oUser && isset($oUser->salt) && $sHash == $oUserPasswordModel::resetHash($oUser)) { - // Valid combination, is there MFA on the account? - if ($oConfig->item('authTwoFactorMode')) { - - /** - * This variable will stop the password resetting until we're confident - * that MFA has been passed - */ - - $bMfaValid = false; - - /** - * Check the user's account to see if they have MFA enabled, if so - * require that they pass that before allowing the password to be reset - */ - - switch ($oConfig->item('authTwoFactorMode')) { - - case 'QUESTION': - $this->data['mfaQuestion'] = $oAuthService->mfaQuestionGet($oUser->id); - - if ($this->data['mfaQuestion']) { - - if ($oInput->post()) { - - // Validate answer - $isValid = $oAuthService->mfaQuestionValidate( - $this->data['mfaQuestion']->id, - $oUser->id, - $oInput->post('mfaAnswer') - ); - - if ($isValid) { - - $bMfaValid = true; - - } else { - $this->oUserFeedback->error('Sorry, the answer to your security question was incorrect.'); - } - } - - } else { - - // No questions set up, allow for now - $bMfaValid = true; - } - - break; - - case 'DEVICE': - $this->data['mfaDevice'] = $oAuthService->mfaDeviceSecretGet($oUser->id); - - if ($this->data['mfaDevice']) { - - if ($oInput->post()) { - - // Validate answer - $isValid = $oAuthService->mfaDeviceCodeValidate( - $oUser->id, - $oInput->post('mfaCode') - ); - - if ($isValid) { - $bMfaValid = true; - - } else { - $this->oUserFeedback->error(sprintf( - 'Sorry, that code could not be validated. %s', - $oAuthService->lastError() - )); - } - } - - } else { - - // No devices set up, allow for now - $bMfaValid = true; - } - break; - } - - } else { - - // No MFA so just set this to true - $bMfaValid = true; - } - - // -------------------------------------------------------------------------- - - // Only run if MFA has been passed and there's POST data - if ($bMfaValid && $oInput->post()) { + if ($oInput->post()) { try { @@ -199,8 +110,7 @@ protected function validate($iUserId, $sHash) $oLoginUser = $oAuthService->loginWithCredentials( $oUser, $oInput->post('new_password'), - $bRemember, - false + $bRemember ); if ($oLoginUser) { @@ -244,11 +154,6 @@ protected function validate($iUserId, $sHash) )); } - // If MFA is setup then we'll need to set the user's session data - if ($oConfig->item('authTwoFactorMode')) { - $oUserModel->setLoginData($oUser->id); - } - // Log user in and forward to wherever they need to go if ($oInput->get('return_to')) { redirect($oInput->get('return_to')); diff --git a/auth/controllers/Register.php b/auth/controllers/Register.php index 8fbce023..870d95d8 100644 --- a/auth/controllers/Register.php +++ b/auth/controllers/Register.php @@ -143,7 +143,6 @@ public function index() // Redirect to the group homepage // @todo (Pablo - 2017-07-11) - Setting for forced email activation - // @todo (Pablo - 2017-07-11) - Handle setting MFA questions and/or devices $oGroup = $oUserGroupModel->getById($oUser->group_id); diff --git a/auth/language/english/auth_lang.php b/auth/language/english/auth_lang.php index 3f29229e..50fc629c 100644 --- a/auth/language/english/auth_lang.php +++ b/auth/language/english/auth_lang.php @@ -10,9 +10,8 @@ // Page Titles $lang['auth_title_login'] = 'Please log in'; $lang['auth_title_register'] = 'Register'; -$lang['auth_title_forgotten_password'] = 'Forgotten your password?'; -$lang['auth_title_forgotten_password_security_question'] = 'Please answer this security question'; -$lang['auth_title_reset'] = 'Reset your password'; +$lang['auth_title_forgotten_password'] = 'Forgotten your password?'; +$lang['auth_title_reset'] = 'Reset your password'; // -------------------------------------------------------------------------- @@ -63,26 +62,6 @@ $lang['auth_login_fail_blocked'] = 'This account has been temporarily blocked due to repeated failed logins. Please wait %s minutes before trying again (each failed login resets the block). '; $lang['auth_login_fail_no_password'] = 'This account does not have a password. Click here to set a password using the Forgotten Password tool.'; -// Two-factor auth strings -$lang['auth_twofactor_token_could_not_generate'] = 'Unable to generate two factor auth token.'; -$lang['auth_twofactor_token_invalid'] = 'Invalid token.'; -$lang['auth_twofactor_token_expired'] = 'Token has expired.'; -$lang['auth_twofactor_token_bad_ip'] = 'Invalid IP address.'; -$lang['auth_twofactor_token_unverified'] = 'Sorry, there was a problem verifying your login session. As a precaution we have logged you out.'; - -$lang['auth_twofactor_question_set_title'] = 'Set Your Security Questions'; -$lang['auth_twofactor_question_set_body'] = 'This website offers enhanced security for your account, please specify a few security questions which we\'ll use to verify your identity when you log in to the system.'; -$lang['auth_twofactor_question_set_system_body'] = 'The following questions are generated by the system, please choose your preferred question and provide an answer.'; -$lang['auth_twofactor_question_set_system_legend'] = 'System questions'; -$lang['auth_twofactor_question_set_custom_body'] = 'Specify your own security question and answer combination. Remember to make questions hard for an attacker to guess or research (avoid information which can easily be found on public mediums, such as social networks).'; -$lang['auth_twofactor_question_set_custom_legend'] = 'Custom questions'; -$lang['auth_twofactor_question_set_fail'] = 'Sorry, there was a problem saving your security questions.'; -$lang['auth_twofactor_question_unique'] = 'Sorry, questions must be unique. Please don\'t specify the same question more than once.'; - -$lang['auth_twofactor_answer_title'] = 'Security question'; -$lang['auth_twofactor_answer_body'] = 'Please answer the following security question.'; -$lang['auth_twofactor_answer_incorrect'] = 'Sorry, your answer was incorrect.'; - // -------------------------------------------------------------------------- // Logout lang strings diff --git a/auth/views/mfa/device/ask.php b/auth/views/mfa/device/ask.php deleted file mode 100644 index 65c30c2d..00000000 --- a/auth/views/mfa/device/ask.php +++ /dev/null @@ -1,55 +0,0 @@ - $return_to, - 'remember' => $remember, -]); - -$sQuery = !empty($aQuery) ? '?' . http_build_query($aQuery) : ''; -$sFormUrl = null; - -if (isset($user_id) && isset($token)) { - $sFormUrl = 'auth/mfa/device/' . $user_id . '/' . $token['salt'] . '/' . $token['token'] . $sQuery; - $sFormUrl = siteUrl($sFormUrl); -} - -?> -
-
-
-

- Two Factor Authentication -

-
-
- load('auth/_components/alerts'); - - $sFieldKey = 'mfa_code'; - $sFieldLabel = 'Please enter a code generated by your device:'; - $sFieldPlaceholder = 'Enter a code generated by your device'; - $sFieldAttr = 'id="input-' . $sFieldKey . '" autocomplete="off" placeholder="' . $sFieldPlaceholder . '" class="form__control"'; - - ?> -
- - - ', '

')?> -
-
- -
- -
-
-
diff --git a/auth/views/mfa/device/setup.php b/auth/views/mfa/device/setup.php deleted file mode 100644 index 85755f5c..00000000 --- a/auth/views/mfa/device/setup.php +++ /dev/null @@ -1,61 +0,0 @@ - $return_to, - 'remember' => $remember, -]); - -$sQuery = !empty($aQuery) ? '?' . http_build_query($aQuery) : ''; - -?> -
-
-
-

- Set up Two Factor Authentication -

-
-
- load('auth/_components/alerts'); - - ?> -

- This site requires that you use Two Factor Authentication when logging in. To set up, please scan the QR - code with your device, then enter a valid code. -

-

- $secret['url'], 'class' => 'img-responsive img-thumbnail'])?> -

- -
- - - ', '

')?> -
-
- -
- -
-
-
diff --git a/auth/views/mfa/question/ask.php b/auth/views/mfa/question/ask.php deleted file mode 100644 index f6d40f58..00000000 --- a/auth/views/mfa/question/ask.php +++ /dev/null @@ -1,62 +0,0 @@ - $return_to, - 'remember' => $remember, -]); - -$sQuery = !empty($aQuery) ? '?' . http_build_query($aQuery) : ''; -$sFormUrl = null; - -if (isset($login_method) && isset($user_id) && isset($token)) { - $login_method = $login_method && $login_method != 'native' ? '/' . $login_method : ''; - $sFormUrl = 'auth/mfa/question/' . $user_id . '/' . $token['salt'] . '/' . $token['token'] . $login_method . $sQuery; - $sFormUrl = siteUrl($sFormUrl); -} - -?> -
-
-
-

- Two Factor Authentication -

-
-
- load('auth/_components/alerts'); - - ?> -

- -

- question; - $sFieldPlaceholder = 'Type your answer here'; - $sFieldAttr = 'id="input-' . $sFieldKey . '" autocomplete="off" placeholder="' . $sFieldPlaceholder . '" class="form__control"'; - - ?> -
- - - ', '

')?> -
-
- -
- -
-
-
diff --git a/auth/views/mfa/question/set.php b/auth/views/mfa/question/set.php deleted file mode 100644 index 21985666..00000000 --- a/auth/views/mfa/question/set.php +++ /dev/null @@ -1,126 +0,0 @@ - $return_to, - 'remember' => $remember, -]); - -$sQuery = !empty($aQuery) ? '?' . http_build_query($aQuery) : ''; - -?> -
-
-
-

- Set up Two Factor Authentication -

-
-
- load('auth/_components/alerts'); - - if ($num_questions) { - ?> -

- -

- -

- -

- -
- - - ', '

')?> -
- -
- - - ', '

')?> -
- -

- -

- -

- -

- -
- - - ', '

')?> -
- -
- - - ', '

')?> -
- -
- -
- -
-
-
diff --git a/auth/views/password/change_temp.php b/auth/views/password/change_temp.php index e109946e..ce910051 100644 --- a/auth/views/password/change_temp.php +++ b/auth/views/password/change_temp.php @@ -36,52 +36,6 @@ echo form_open($resetUrl . $sQuery, 'class="form form-horizontal"'); $oView->load('auth/_components/alerts'); - if (!empty($mfaQuestion)) { - - $sFieldKey = 'mfaAnswer'; - $sFieldLabel = 'Security Question'; - $sFieldPlaceholder = 'Type your answer'; - $sFieldAttr = 'id="input-' . $sFieldKey . '" placeholder="' . $sFieldPlaceholder . '" class="form__control"'; - - ?> -
- -

- - question?> - -

- - ', '

')?> -
- -
- - - ', '

')?> -

- - Use your device to generate a single use code. - -

-
- -
-
-
-

- Security Question -

-
-
- load('auth/_components/alerts'); - - ?> -

- -

-
- - -
-
- -
- -
-
-
diff --git a/composer.json b/composer.json index c5b373a9..1e8de158 100644 --- a/composer.json +++ b/composer.json @@ -39,7 +39,6 @@ "nails/module-email": "dev-develop", "nails/module-form-builder": "dev-develop", "nails/module-housekeeping": "dev-develop", - "sonata-project/google-authenticator": "~2.3.0", "wikimedia/common-passwords": "^v0.5", "lbuchs/webauthn": "^2.2", "ext-json": "*", diff --git a/src/Controller/BaseMfa.php b/src/Controller/BaseMfa.php deleted file mode 100644 index a68bd1b4..00000000 --- a/src/Controller/BaseMfa.php +++ /dev/null @@ -1,197 +0,0 @@ -authMfaMode = $oConfig->item('authTwoFactorMode'); - $aConfig = $oConfig->item('authTwoFactor'); - $this->authMfaConfig = $aConfig[$this->authMfaMode]; - } - - // -------------------------------------------------------------------------- - - protected function validateToken() - { - /** @var User $oUserModel */ - $oUserModel = Factory::model('User', Constants::MODULE_SLUG); - /** @var Input $oInput */ - $oInput = Factory::service('Input'); - /** @var Uri $oUri */ - $oUri = Factory::service('Uri'); - - $this->returnTo = $oInput->get('return_to', true); - $this->remember = $oInput->get('remember', true); - $iUserId = (int) $oUri->segment(4); - $this->mfaUser = $oUserModel->getById($iUserId); - - if (!$this->mfaUser) { - $this->oUserFeedback->error(lang('auth_twofactor_token_unverified')); - redirect(loginUrl($this->returnTo)); - } - - $sSalt = $oUri->segment(5); - $sToken = $oUri->segment(6); - $sIpAddress = $oInput->ipAddress(); - $this->loginMethod = $oUri->segment(7) ? $oUri->segment(7) : 'native'; - - // Safety first - switch (strtolower($this->loginMethod)) { - - case 'facebook': - case 'twitter': - case 'linkedin': - case 'native': - // All good, homies. - break; - - default: - $this->loginMethod = 'native'; - break; - } - - /** @var Authentication $oAuthService */ - $oAuthService = Factory::service('Authentication', Constants::MODULE_SLUG); - if (!$oAuthService->mfaTokenValidate($this->mfaUser->id, $sSalt, $sToken, $sIpAddress)) { - - $this->oUserFeedback->error(lang('auth_twofactor_token_unverified')); - redirect(loginUrl($this->returnTo, ['remember' => $this->remember])); - - } else { - - // Token is valid, generate a new one for the next request - $this->data['token'] = $oAuthService->mfaTokenGenerate($this->mfaUser->id); - - // Set other data for the views - $this->data['user_id'] = $this->mfaUser->id; - $this->data['login_method'] = $this->loginMethod; - $this->data['return_to'] = $this->returnTo; - $this->data['remember'] = $this->remember; - } - } - - // -------------------------------------------------------------------------- - - /** - * Logs a user In - */ - protected function loginUser() - { - // Set login data for this user - /** @var User $oUserModel */ - $oUserModel = Factory::model('User', Constants::MODULE_SLUG); - $oUserModel->setLoginData($this->mfaUser->id); - - // If we're remembering this user set a cookie - if ($this->remember) { - $oUserModel->setRememberCookie( - $this->mfaUser->id, - $this->mfaUser->password, - $this->mfaUser->email - ); - } - - // Update their last login and increment their login count - $oUserModel->updateLastLogin($this->mfaUser->id); - - // -------------------------------------------------------------------------- - - // Generate an event for this log in - createUserEvent('did_log_in', ['method' => $this->loginMethod], null, $this->mfaUser->id); - - // -------------------------------------------------------------------------- - - // Say hello - if ($this->mfaUser->last_login) { - - /** @var Config $oConfig */ - $oConfig = Factory::service('Config'); - - $sLastLogin = $oConfig->item('authShowNicetimeOnLogin') - ? niceTime(strtotime($this->mfaUser->last_login)) - : toUserDatetime($this->mfaUser->last_login); - - if ($oConfig->item('authShowLastIpOnLogin')) { - $this->oUserFeedback->success(lang( - 'auth_login_ok_welcome_with_ip', - [ - $this->mfaUser->first_name, - $sLastLogin, - $this->mfaUser->last_ip, - ] - )); - - } else { - $this->oUserFeedback->success(lang( - 'auth_login_ok_welcome', - [ - $this->mfaUser->first_name, - $sLastLogin, - ] - )); - } - - } else { - $this->oUserFeedback->success(lang( - 'auth_login_ok_welcome_notime', - [ - $this->mfaUser->first_name, - ] - )); - } - - // -------------------------------------------------------------------------- - - // Delete the token we generated, it's no needed, eh! - /** @var Authentication $oAuthService */ - $oAuthService = Factory::service('Authentication', Constants::MODULE_SLUG); - $oAuthService->mfaTokenDelete($this->data['token']['id']); - - // -------------------------------------------------------------------------- - - $sRedirectUrl = $this->returnTo != siteUrl() ? $this->returnTo : $this->mfaUser->group_homepage; - redirect($sRedirectUrl); - } -} diff --git a/src/Exception/Login/RequiresMfaException.php b/src/Exception/Login/RequiresMfaException.php deleted file mode 100644 index 41071c46..00000000 --- a/src/Exception/Login/RequiresMfaException.php +++ /dev/null @@ -1,14 +0,0 @@ -format('Y-m-d H:i:s'); - $iBatchSize = 200; - $iProcessed = 0; - $iLastId = 0; - - $oContext - ->writeln(sprintf('Deleting from %s in batches of %d', $sTable, $iBatchSize)) - ->log(sprintf( - 'TABLE %s batch_size=%d dry_run=%s', - $sTable, - $iBatchSize, - $oContext->isDryRun() ? 'true' : 'false' - )); - - while (true) { - $aRows = $oDb - ->select('id, user_id, expires') - ->where('expires <', $sCutOff) - ->where('id >', $iLastId) - ->order_by('id', 'asc') - ->limit($iBatchSize) - ->get($sTable) - ->result(); - - if (empty($aRows)) { - break; - } - - $aIds = []; - foreach ($aRows as $oRow) { - $iId = (int) $oRow->id; - $iLastId = $iId; - $aIds[] = $iId; - $sAudit = sprintf( - 'id=%d user_id=%s expires=%s', - $iId, - $oRow->user_id === null ? 'null' : (string) $oRow->user_id, - (string) $oRow->expires - ); - $oContext - ->log('DELETE ' . $sAudit) - ->writeln(' ↳ ' . $sAudit); - } - - if (!$oContext->isDryRun()) { - $oDb - ->where_in('id', $aIds) - ->delete($sTable); - } - - $iProcessed += count($aIds); - } - - $oContext->writeln(sprintf( - '%s %s', - number_format($iProcessed), - $oContext->isDryRun() ? 'would be deleted' : 'deleted' - )); - - return Result::ok($iProcessed); - } -} diff --git a/src/Routes.php b/src/Routes.php index 63d3bf23..fca676e5 100644 --- a/src/Routes.php +++ b/src/Routes.php @@ -26,8 +26,6 @@ public static function generate(): array 'auth/override/login_as/(.+)/(.+)' => 'auth/sessionOverride/login_as', 'auth/password/forgotten(/(.+))?' => 'auth/PasswordForgotten/$2', 'auth/password/reset/(\d+)/(.+)' => 'auth/PasswordReset/$1/$2', - 'auth/mfa/device/(\d+)/(.+)/(.+)(/(.+))?' => 'auth/MfaDevice', - 'auth/mfa/question/(\d+)/(.+)/(.+)(/(.+))?' => 'auth/MfaQuestion', ]; } } diff --git a/src/Service/Authentication.php b/src/Service/Authentication.php index 584fb759..215342a0 100644 --- a/src/Service/Authentication.php +++ b/src/Service/Authentication.php @@ -12,37 +12,27 @@ namespace Nails\Auth\Service; -use DateInterval; use DateTime; use Nails\Auth\Constants; use Nails\Auth\Exception\Login\InvalidCredentialsException; use Nails\Auth\Exception\Login\IsLockedOutException; use Nails\Auth\Exception\Login\IsSuspendedException; use Nails\Auth\Exception\Login\NoUserException; -use Nails\Auth\Exception\Login\RequiresMfaException; use Nails\Auth\Exception\Login\RequiresPasswordResetExpiredException; use Nails\Auth\Exception\Login\RequiresPasswordResetTempException; use Nails\Auth\Exception\Login\NoPasswordException; use Nails\Auth\Exception\Passkey\PasskeyException; use Nails\Auth\Model\User\Password; use Nails\Auth\Resource; -use Nails\Common\Exception\Encrypt\DecodeException; -use Nails\Common\Exception\EnvironmentException; use Nails\Common\Exception\FactoryException; use Nails\Common\Exception\ModelException; use Nails\Common\Exception\NailsException; -use Nails\Common\Helper\Url; -use Nails\Common\Service\Config; use Nails\Common\Service\Database; -use Nails\Common\Service\Encrypt; -use Nails\Common\Service\Input; use Nails\Common\Service\Session; use Nails\Common\Traits\ErrorHandling; use Nails\Environment; use Nails\Factory; use ReflectionException; -use Sonata\GoogleAuthenticator\GoogleAuthenticator; -use Sonata\GoogleAuthenticator\GoogleQrUrl; use stdClass; /** @@ -56,10 +46,6 @@ class Authentication // -------------------------------------------------------------------------- - const TABLE_TWO_FACTOR_DEVICE_SECRET = NAILS_DB_PREFIX . 'user_auth_two_factor_device_secret'; - const TABLE_TWO_FACTOR_QUESTION = NAILS_DB_PREFIX . 'user_auth_two_factor_question'; - const TABLE_TWO_FACTOR_TOKEN = NAILS_DB_PREFIX . 'user_auth_two_factor_token'; - /** * The minimum length of time to wait between attempts, in microseconds * @@ -131,7 +117,6 @@ public function login($oUser): Resource\User * @param Resource\User|string|int $oUser The user's Resource, ID, or identifier * @param string $sPassword The user's password * @param bool $bRemember Whether to 'remember' the user or not - * @param bool $bMfaCheck * * @return Resource\User * @throws FactoryException @@ -142,7 +127,6 @@ public function login($oUser): Resource\User * @throws NailsException * @throws NoUserException * @throws ReflectionException - * @throws RequiresMfaException * @throws RequiresPasswordResetExpiredException * @throws RequiresPasswordResetTempException * @throws NoPasswordException @@ -150,8 +134,7 @@ public function login($oUser): Resource\User public function loginWithCredentials( $oUser, string $sPassword, - bool $bRemember = false, - bool $bMfaCheck = true + bool $bRemember = false ): Resource\User { // Delay execution for a moment (reduces brute force efficiently) @@ -232,16 +215,6 @@ public function loginWithCredentials( // Successful login means we can forget about failures $oUserModel->resetFailedLogin($oUser->id); - // Check if MFA is required - // @todo (Pablo - 2019-12-10) - This should consider trusted devices - // @todo (Pablo - 2019-12-10) - This should consider time since last checked (i.e a passed MFA is valid for a short valid and won't be asked for again) - - /** @var Config $oConfig */ - $oConfig = Factory::service('Config'); - if ($bMfaCheck && !empty($oConfig->item('authTwoFactorMode'))) { - throw new RequiresMfaException(); - } - // Check if password needs changed if ($oUserPasswordModel->isTemporary($oUser)) { throw new RequiresPasswordResetTempException(); @@ -250,7 +223,6 @@ public function loginWithCredentials( } // Set the remember me cookie - // @todo (Pablo - 2019-12-10) - Check this is respected as part of 2FA if ($bRemember) { $oUserModel->setRememberCookie($oUser->id, $oUser->password, $oUser->email); } @@ -575,485 +547,4 @@ public function logout() return true; } - - // -------------------------------------------------------------------------- - - /** - * Generate an MFA token - * - * @param int $iUserId The user ID to generate the token for - * - * @return array|false - * @throws FactoryException - */ - public function mfaTokenGenerate($iUserId) - { - /** @var Password $oPasswordModel */ - $oPasswordModel = Factory::model('UserPassword', Constants::MODULE_SLUG); - /** @var DateTime $oNow */ - $oNow = Factory::factory('DateTime'); - /** @var Input $oInput */ - $oInput = Factory::service('Input'); - /** @var Database $oDb */ - $oDb = Factory::service('Database'); - - $sSalt = $oPasswordModel->salt(); - $sIp = $oInput->ipAddress(); - $sCreated = $oNow->format('Y-m-d H:i:s'); - $sExpires = $oNow->add(new DateInterval('PT10M'))->format('Y-m-d H:i:s'); - $aToken = [ - 'token' => sha1(sha1(\Nails\Config::get('PRIVATE_KEY') . $iUserId . $sCreated . $sExpires . $sIp) . $sSalt), - 'salt' => md5($sSalt), - ]; - - // Add this to the DB - $oDb->set('user_id', $iUserId); - $oDb->set('token', $aToken['token']); - $oDb->set('salt', $aToken['salt']); - $oDb->set('created', $sCreated); - $oDb->set('expires', $sExpires); - $oDb->set('ip', $sIp); - - if ($oDb->insert(static::TABLE_TWO_FACTOR_TOKEN)) { - $aToken['id'] = $oDb->insert_id(); - return $aToken; - } else { - $error = lang('auth_twofactor_token_could_not_generate'); - $this->setError($error); - return false; - } - } - - // -------------------------------------------------------------------------- - - /** - * Validate a MFA token - * - * @param int $iUserId The ID of the user the token belongs to - * @param string $sSalt The token's salt - * @param string $sToken The token's hash - * @param string $sIp The user's IP address - * - * @return bool - * @throws FactoryException - */ - public function mfaTokenValidate($iUserId, $sSalt, $sToken, $sIp) - { - /** @var Database $oDb */ - $oDb = Factory::service('Database'); - $oDb->where('user_id', $iUserId); - $oDb->where('salt', $sSalt); - $oDb->where('token', $sToken); - - $oToken = $oDb->get(static::TABLE_TWO_FACTOR_TOKEN)->row(); - $bReturn = true; - - if (!$oToken) { - - $this->setError(lang('auth_twofactor_token_invalid')); - return false; - - } elseif (strtotime($oToken->expires) <= time()) { - - $this->setError(lang('auth_twofactor_token_expired')); - $bReturn = false; - - } elseif ($oToken->ip != $sIp) { - - $this->setError(lang('auth_twofactor_token_bad_ip')); - $bReturn = false; - } - - // Delete the token - $this->mfaTokenDelete($oToken->id); - - return $bReturn; - } - - // -------------------------------------------------------------------------- - - /** - * Delete an MFA token - * - * @param int $iTokenId The token's ID - * - * @return bool - * @throws FactoryException - */ - public function mfaTokenDelete($iTokenId) - { - /** @var Database $oDb */ - $oDb = Factory::service('Database'); - $oDb->where('id', $iTokenId); - $oDb->delete(static::TABLE_TWO_FACTOR_TOKEN); - return (bool) $oDb->affected_rows(); - } - - // -------------------------------------------------------------------------- - - /** - * Fetches a random MFA question for a user - * - * @param int $iUserId The user's ID - * - * @return bool|stdClass - * @throws DecodeException - * @throws EnvironmentException - * @throws FactoryException - */ - public function mfaQuestionGet($iUserId) - { - /** @var Database $oDb */ - $oDb = Factory::service('Database'); - /** @var Input $oInput */ - $oInput = Factory::service('Input'); - - $oDb->where('user_id', $iUserId); - $oDb->order_by('last_requested', 'DESC'); - $aQuestions = $oDb->get(static::TABLE_TWO_FACTOR_QUESTION)->result(); - - if (!$aQuestions) { - $this->setError('No security questions available for this user.'); - return false; - } - - // -------------------------------------------------------------------------- - - // Choose a question to return - if (count($aQuestions) == 1) { - - // No choice, just return the lonely question - $oOut = reset($aQuestions); - - } elseif (count($aQuestions) > 1) { - - /** - * Has the most recently asked question been asked in the last 10 minutes? - * If so, return that one again (to make harvesting all the user's questions - * a little more time consuming). If not randomly choose one. - */ - - $oOut = reset($aQuestions); - if (strtotime($oOut->last_requested) < strtotime('-10 MINS')) { - $oOut = $aQuestions[array_rand($aQuestions)]; - } - - } else { - $this->setError('Could not determine security question.'); - return false; - } - - // Decode the question - /** @var Encrypt $oEncrypt */ - $oEncrypt = Factory::service('Encrypt'); - $oOut->question = $oEncrypt->decode($oOut->question, \Nails\Config::get('PRIVATE_KEY') . $oOut->salt); - - // Update the last requested details - $oDb->set('last_requested', 'NOW()', false); - $oDb->set('last_requested_ip', $oInput->ipAddress()); - $oDb->where('id', $oOut->id); - $oDb->update(static::TABLE_TWO_FACTOR_QUESTION); - - return $oOut; - } - - // -------------------------------------------------------------------------- - - /** - * Validates the answer to an MFA Question - * - * @param int $iQuestionId The question's ID - * @param int $iUserId The user's ID - * @param string $answer The user's answer - * - * @return bool - * @throws FactoryException - */ - public function mfaQuestionValidate($iQuestionId, $iUserId, $answer) - { - /** @var Database $oDb */ - $oDb = Factory::service('Database'); - $oDb->select('answer, salt'); - $oDb->where('id', $iQuestionId); - $oDb->where('user_id', $iUserId); - $oQuestion = $oDb->get(static::TABLE_TWO_FACTOR_QUESTION)->row(); - - if (!$oQuestion) { - return false; - } - - $hash = sha1(sha1(strtolower($answer)) . \Nails\Config::get('PRIVATE_KEY') . $oQuestion->salt); - - return $hash === $oQuestion->answer; - } - - // -------------------------------------------------------------------------- - - /** - * Sets MFA questions for a user - * - * @param int $iUserId The user's ID - * @param array $aData An array of question and answers - * @param bool $bClearOld Whether or not to clear old questions - * - * @return bool - * @throws FactoryException - * @throws EnvironmentException - */ - public function mfaQuestionSet($iUserId, $aData, $bClearOld = true) - { - // Check input - foreach ($aData as $oDatum) { - if (empty($oDatum->question) || empty($oDatum->answer)) { - $this->setError('Malformed question/answer data.'); - return false; - } - } - - // Begin transaction - /** @var Database $oDb */ - $oDb = Factory::service('Database'); - $oDb->transaction()->start(); - - // Delete old questions? - if ($bClearOld) { - $oDb->where('user_id', $iUserId); - $oDb->delete(static::TABLE_TWO_FACTOR_QUESTION); - } - - /** @var Password $oPasswordModel */ - $oPasswordModel = Factory::model('UserPassword', Constants::MODULE_SLUG); - /** @var Encrypt $oEncrypt */ - $oEncrypt = Factory::service('Encrypt'); - - $aQuestionData = []; - $iCounter = 0; - $oNow = Factory::factory('DateTime'); - $sDateTime = $oNow->format('Y-m-d H:i:s'); - - foreach ($aData as $oDatum) { - $sSalt = $oPasswordModel->salt(); - $aQuestionData[$iCounter] = [ - 'user_id' => $iUserId, - 'salt' => $sSalt, - 'question' => $oEncrypt->encode($oDatum->question, \Nails\Config::get('PRIVATE_KEY') . $sSalt), - 'answer' => sha1(sha1(strtolower($oDatum->answer)) . \Nails\Config::get('PRIVATE_KEY') . $sSalt), - 'created' => $sDateTime, - 'last_requested' => null, - ]; - $iCounter++; - } - - if ($aQuestionData) { - - $oDb->insert_batch(static::TABLE_TWO_FACTOR_QUESTION, $aQuestionData); - - if ($oDb->transaction()->status() !== false) { - $oDb->transaction()->commit(); - return true; - } else { - $oDb->transaction()->rollback(); - return false; - } - - } else { - $oDb->transaction()->rollback(); - $this->setError('No data to save.'); - return false; - } - } - - // -------------------------------------------------------------------------- - - /** - * Gets the user's MFA device if there is one - * - * @param int $iUserId The user's ID - * - * @return bool|stdClass \stdClass on success, false on failure - * @throws EnvironmentException - * @throws FactoryException - * @throws DecodeException - */ - public function mfaDeviceSecretGet($iUserId) - { - /** @var Database $oDb */ - $oDb = Factory::service('Database'); - /** @var Encrypt $oEncrypt */ - $oEncrypt = Factory::service('Encrypt'); - - $oDb->where('user_id', $iUserId); - $oDb->limit(1); - $aResult = $oDb->get(static::TABLE_TWO_FACTOR_DEVICE_SECRET)->result(); - - if (empty($aResult)) { - return false; - } - - $oReturn = reset($aResult); - $oReturn->secret = $oEncrypt->decode($oReturn->secret, \Nails\Config::get('PRIVATE_KEY')); - - return $oReturn; - } - - // -------------------------------------------------------------------------- - - /** - * Generates a MFA Device Secret - * - * @param int $iUserId The user ID to generate for - * @param string $sExistingSecret The existing secret to use instead of generating a new one - * - * @return bool|array - * @throws FactoryException - * @throws ModelException - */ - public function mfaDeviceSecretGenerate($iUserId, $sExistingSecret = null) - { - // Get an identifier for the user - /** @var \Nails\Auth\Model\User $oUserModel */ - $oUserModel = Factory::model('User', Constants::MODULE_SLUG); - $oUser = $oUserModel->getById($iUserId); - - if (!$oUser) { - $this->setError('User does not exist.'); - return false; - } - - $oGoogleAuth = new GoogleAuthenticator(); - - // Generate the secret - if (empty($sExistingSecret)) { - $sSecret = $oGoogleAuth->generateSecret(); - } else { - $sSecret = $sExistingSecret; - } - - // Get the hostname - $sHostname = Url::extractRegistrableDomain(\Nails\Config::get('BASE_URL')); - - // User identifier - $sUsername = $oUser->username; - $sUsername = empty($sUsername) ? preg_replace('/[^a-z]/', '', strtolower($oUser->first_name . $oUser->last_name)) : $sUsername; - $sUsername = empty($sUsername) ? preg_replace('/[^a-z]/', '', strtolower($oUser->email)) : $sUsername; - - - return [ - 'secret' => $sSecret, - 'url' => GoogleQrUrl::generate($sUsername . '@' . $sHostname, $sSecret), - ]; - } - - // -------------------------------------------------------------------------- - - /** - * Validates a secret against two given codes, if valid adds as a device for - * the user - * - * @param int $iUserId The user's ID - * @param string $sSecret The secret being used - * @param int $iCode The first code to be generate - * - * @return bool - * @throws FactoryException - * @throws EnvironmentException - */ - public function mfaDeviceSecretValidate($iUserId, $sSecret, $iCode) - { - // Tidy up codes so that they only contain digits - $sCode = preg_replace('/[^\d]/', '', $iCode); - - // New instance of the authenticator - $oGoogleAuth = new GoogleAuthenticator(); - - if ($oGoogleAuth->checkCode($sSecret, $sCode)) { - - /** @var Database $oDb */ - $oDb = Factory::service('Database'); - /** @var Encrypt $oEncrypt */ - $oEncrypt = Factory::service('Encrypt'); - - $oDb->set('user_id', $iUserId); - $oDb->set('secret', $oEncrypt->encode($sSecret, \Nails\Config::get('PRIVATE_KEY'))); - $oDb->set('created', 'NOW()', false); - - if ($oDb->insert(static::TABLE_TWO_FACTOR_DEVICE_SECRET)) { - - $iSecretId = $oDb->insert_id(); - $oNow = Factory::factory('DateTime'); - - $oDb->set('secret_id', $iSecretId); - $oDb->set('code', $sCode); - $oDb->set('used', $oNow->format('Y-m-d H:i:s')); - $oDb->insert(\Nails\Config::get('NAILS_DB_PREFIX') . 'user_auth_two_factor_device_code'); - - return true; - - } else { - $this->setError('Could not save secret.'); - return false; - } - - } else { - $this->setError('Codes did not validate.'); - return false; - } - } - - // -------------------------------------------------------------------------- - - /** - * Validates an MFA Device code - * - * @param int $iUserId The user's ID - * @param string $sCode The code to validate - * - * @return bool - * @throws DecodeException - * @throws EnvironmentException - * @throws FactoryException - */ - public function mfaDeviceCodeValidate($iUserId, $sCode) - { - // Get the user's secret - $oSecret = $this->mfaDeviceSecretGet($iUserId); - - if (!$oSecret) { - $this->setError('Invalid User'); - return false; - } - - // Has the code been used before? - /** @var Database $oDb */ - $oDb = Factory::service('Database'); - $oDb->where('secret_id', $oSecret->id); - $oDb->where('code', $sCode); - - if ($oDb->count_all_results(\Nails\Config::get('NAILS_DB_PREFIX') . 'user_auth_two_factor_device_code')) { - $this->setError('Code has already been used.'); - return false; - } - - // Tidy up codes so that they only contain digits - $sCode = preg_replace('/[^\d]/', '', $sCode); - - // New instance of the authenticator - $oGoogleAuth = new GoogleAuthenticator(); - $checkCode = $oGoogleAuth->checkCode($oSecret->secret, $sCode); - - if ($checkCode) { - - // Log the code so it can't be used again - $oDb->set('secret_id', $oSecret->id); - $oDb->set('code', $sCode); - $oDb->set('used', 'NOW()', false); - - $oDb->insert(\Nails\Config::get('NAILS_DB_PREFIX') . 'user_auth_two_factor_device_code'); - - return true; - - } else { - return false; - } - } } diff --git a/tests/RoutesTest.php b/tests/RoutesTest.php index b10ad694..54dda060 100644 --- a/tests/RoutesTest.php +++ b/tests/RoutesTest.php @@ -19,11 +19,9 @@ public function test_the_generated_routes_are_unchanged(): void { self::assertSame( [ - 'auth/override/login_as/(.+)/(.+)' => 'auth/sessionOverride/login_as', - 'auth/password/forgotten(/(.+))?' => 'auth/PasswordForgotten/$2', - 'auth/password/reset/(\d+)/(.+)' => 'auth/PasswordReset/$1/$2', - 'auth/mfa/device/(\d+)/(.+)/(.+)(/(.+))?' => 'auth/MfaDevice', - 'auth/mfa/question/(\d+)/(.+)/(.+)(/(.+))?' => 'auth/MfaQuestion', + 'auth/override/login_as/(.+)/(.+)' => 'auth/sessionOverride/login_as', + 'auth/password/forgotten(/(.+))?' => 'auth/PasswordForgotten/$2', + 'auth/password/reset/(\d+)/(.+)' => 'auth/PasswordReset/$1/$2', ], Routes::generate() ); From 5e6c0f92642e770dae59226edc3b29b6602a383a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20de=20la=20Pen=CC=83a?= Date: Tue, 22 Sep 2026 21:04:21 +0100 Subject: [PATCH 3/4] feat: Drop the tables left behind by legacy MFA Apps that already migrated to module-multi-factor-auth should not keep the old question, device, and token rows. --- src/Database/Migration/Migration23.php | 40 ++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 src/Database/Migration/Migration23.php diff --git a/src/Database/Migration/Migration23.php b/src/Database/Migration/Migration23.php new file mode 100644 index 00000000..bfefc573 --- /dev/null +++ b/src/Database/Migration/Migration23.php @@ -0,0 +1,40 @@ +query('DROP TABLE IF EXISTS `{{NAILS_DB_PREFIX}}user_auth_two_factor_device_code`'); + $this->query('DROP TABLE IF EXISTS `{{NAILS_DB_PREFIX}}user_auth_two_factor_device_secret`'); + $this->query('DROP TABLE IF EXISTS `{{NAILS_DB_PREFIX}}user_auth_two_factor_question`'); + $this->query('DROP TABLE IF EXISTS `{{NAILS_DB_PREFIX}}user_auth_two_factor_token`'); + } +} From 954bffa5d69b19b5234803f204e80e936a7de785 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20de=20la=20Pen=CC=83a?= Date: Tue, 22 Sep 2026 21:07:25 +0100 Subject: [PATCH 4/4] fix: Run the legacy MFA table drop once Both branches will carry this as migration 23, so it does not need to run on every migrate. --- src/Database/Migration/Migration23.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Database/Migration/Migration23.php b/src/Database/Migration/Migration23.php index bfefc573..44326e8c 100644 --- a/src/Database/Migration/Migration23.php +++ b/src/Database/Migration/Migration23.php @@ -16,12 +16,7 @@ use Nails\Common\Interfaces; use Nails\Common\Traits; -/** - * Repeatable because `feature/pre-new-admin` has no equivalent migration, so an app - * arriving from that branch resumes above this number and would never run it. - * DROP TABLE IF EXISTS is safe to evaluate on every migrate. - */ -class Migration23 implements Interfaces\Database\Migration\Repeatable +class Migration23 implements Interfaces\Database\Migration { use Traits\Database\Migration;