diff --git a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php --- a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php @@ -126,6 +126,8 @@ if ($ex instanceof PhabricatorAuthHighSecurityRequiredException) { $form = id(new PhabricatorAuthSessionEngine())->renderHighSecurityForm( + $ex->getFactors(), + $ex->getFactorValidationResults(), $user, $request); diff --git a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php --- a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php +++ b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php @@ -227,34 +227,69 @@ $session = $viewer->getSession(); + // Check if the session is already in high security mode. $token = $this->issueHighSecurityToken($session); if ($token) { return $token; } + // Load the multi-factor auth sources attached to this account. + $factors = id(new PhabricatorAuthFactorConfig())->loadAllWhere( + 'userPHID = %s', + $viewer->getPHID()); + + // If the account has no associated multi-factor auth, just issue a token + // without putting the session into high security mode. This is generally + // easier for users. A minor but desirable side effect is that when a user + // adds an auth factor, existing sessions won't get a free pass into hisec, + // since they never actually got marked as hisec. + if (!$factors) { + return $this->issueHighSecurityToken($session, true); + } + + $validation_results = array(); if ($request->isHTTPPost()) { $request->validateCSRF(); if ($request->getExists(AphrontRequest::TYPE_HISEC)) { - // TODO: Actually verify that the user provided some multi-factor - // auth credentials here. For now, we just let you enter high - // security. - - $until = time() + phutil_units('15 minutes in seconds'); - $session->setHighSecurityUntil($until); - - queryfx( - $session->establishConnection('w'), - 'UPDATE %T SET highSecurityUntil = %d WHERE id = %d', - $session->getTableName(), - $until, - $session->getID()); - - $log = PhabricatorUserLog::initializeNewLog( - $viewer, - $viewer->getPHID(), - PhabricatorUserLog::ACTION_ENTER_HISEC); - $log->save(); + $ok = true; + foreach ($factors as $factor) { + $id = $factor->getID(); + $impl = $factor->requireImplementation(); + + $validation_results[$id] = $impl->processValidateFactorForm( + $factor, + $viewer, + $request); + + if (!$impl->isFactorValid($factor, $validation_results[$id])) { + $ok = false; + } + } + + if ($ok) { + $until = time() + phutil_units('15 minutes in seconds'); + $session->setHighSecurityUntil($until); + + queryfx( + $session->establishConnection('w'), + 'UPDATE %T SET highSecurityUntil = %d WHERE id = %d', + $session->getTableName(), + $until, + $session->getID()); + + $log = PhabricatorUserLog::initializeNewLog( + $viewer, + $viewer->getPHID(), + PhabricatorUserLog::ACTION_ENTER_HISEC); + $log->save(); + } else { + $log = PhabricatorUserLog::initializeNewLog( + $viewer, + $viewer->getPHID(), + PhabricatorUserLog::ACTION_FAIL_HISEC); + $log->save(); + } } } @@ -264,7 +299,9 @@ } throw id(new PhabricatorAuthHighSecurityRequiredException()) - ->setCancelURI($cancel_uri); + ->setCancelURI($cancel_uri) + ->setFactors($factors) + ->setFactorValidationResults($validation_results); } @@ -291,19 +328,25 @@ * @return AphrontFormView Renderable form. */ public function renderHighSecurityForm( + array $factors, + array $validation_results, PhabricatorUser $viewer, AphrontRequest $request) { - // TODO: This is stubbed. - $form = id(new AphrontFormView()) ->setUser($viewer) - ->appendRemarkupInstructions('') - ->appendChild( - id(new AphrontFormTextControl()) - ->setLabel(pht('Secret Stuff'))) ->appendRemarkupInstructions(''); + foreach ($factors as $factor) { + $factor->requireImplementation()->renderValidateFactorForm( + $factor, + $form, + $viewer, + idx($validation_results, $factor->getID())); + } + + $form->appendRemarkupInstructions(''); + return $form; } diff --git a/src/applications/auth/exception/PhabricatorAuthHighSecurityRequiredException.php b/src/applications/auth/exception/PhabricatorAuthHighSecurityRequiredException.php --- a/src/applications/auth/exception/PhabricatorAuthHighSecurityRequiredException.php +++ b/src/applications/auth/exception/PhabricatorAuthHighSecurityRequiredException.php @@ -3,6 +3,27 @@ final class PhabricatorAuthHighSecurityRequiredException extends Exception { private $cancelURI; + private $factors; + private $factorValidationResults; + + public function setFactorValidationResults(array $results) { + $this->factorValidationResults = $results; + return $this; + } + + public function getFactorValidationResults() { + return $this->factorValidationResults; + } + + public function setFactors(array $factors) { + assert_instances_of($factors, 'PhabricatorAuthFactorConfig'); + $this->factors = $factors; + return $this; + } + + public function getFactors() { + return $this->factors; + } public function setCancelURI($cancel_uri) { $this->cancelURI = $cancel_uri; diff --git a/src/applications/auth/factor/PhabricatorAuthFactor.php b/src/applications/auth/factor/PhabricatorAuthFactor.php --- a/src/applications/auth/factor/PhabricatorAuthFactor.php +++ b/src/applications/auth/factor/PhabricatorAuthFactor.php @@ -10,6 +10,29 @@ AphrontRequest $request, PhabricatorUser $user); + abstract public function renderValidateFactorForm( + PhabricatorAuthFactorConfig $config, + AphrontFormView $form, + PhabricatorUser $viewer, + $validation_result); + + abstract public function processValidateFactorForm( + PhabricatorAuthFactorConfig $config, + PhabricatorUser $viewer, + AphrontRequest $request); + + public function isFactorValid( + PhabricatorAuthFactorConfig $config, + $validation_result) { + return (idx($validation_result, 'valid') === true); + } + + public function getParameterName( + PhabricatorAuthFactorConfig $config, + $name) { + return 'authfactor.'.$config->getID().'.'.$name; + } + public static function getAllFactors() { static $factors; diff --git a/src/applications/auth/factor/PhabricatorAuthFactorTOTP.php b/src/applications/auth/factor/PhabricatorAuthFactorTOTP.php --- a/src/applications/auth/factor/PhabricatorAuthFactorTOTP.php +++ b/src/applications/auth/factor/PhabricatorAuthFactorTOTP.php @@ -97,6 +97,49 @@ } + public function renderValidateFactorForm( + PhabricatorAuthFactorConfig $config, + AphrontFormView $form, + PhabricatorUser $viewer, + $validation_result) { + + if (!$validation_result) { + $validation_result = array(); + } + + $form->appendChild( + id(new AphrontFormTextControl()) + ->setName($this->getParameterName($config, 'totpcode')) + ->setLabel(pht('App Code')) + ->setCaption(pht('Factor Name: %s', $config->getFactorName())) + ->setValue(idx($validation_result, 'value')) + ->setError(idx($validation_result, 'error', true))); + } + + public function processValidateFactorForm( + PhabricatorAuthFactorConfig $config, + PhabricatorUser $viewer, + AphrontRequest $request) { + + $code = $request->getStr($this->getParameterName($config, 'totpcode')); + $key = new PhutilOpaqueEnvelope($config->getFactorSecret()); + + if (self::verifyTOTPCode($viewer, $key, $code)) { + return array( + 'error' => null, + 'value' => $code, + 'valid' => true, + ); + } else { + return array( + 'error' => strlen($code) ? pht('Invalid') : pht('Required'), + 'value' => $code, + 'valid' => false, + ); + } + } + + public static function generateNewTOTPKey() { return strtoupper(Filesystem::readRandomCharacters(16)); } diff --git a/src/applications/auth/storage/PhabricatorAuthFactorConfig.php b/src/applications/auth/storage/PhabricatorAuthFactorConfig.php --- a/src/applications/auth/storage/PhabricatorAuthFactorConfig.php +++ b/src/applications/auth/storage/PhabricatorAuthFactorConfig.php @@ -26,4 +26,17 @@ return idx(PhabricatorAuthFactor::getAllFactors(), $this->getFactorKey()); } + public function requireImplementation() { + $impl = $this->getImplementation(); + if (!$impl) { + throw new Exception( + pht( + 'Attempting to operate on multi-factor auth which has no '. + 'corresponding implementation (factor key is "%s").', + $this->getFactorKey())); + } + + return $impl; + } + } diff --git a/src/applications/people/storage/PhabricatorUserLog.php b/src/applications/people/storage/PhabricatorUserLog.php --- a/src/applications/people/storage/PhabricatorUserLog.php +++ b/src/applications/people/storage/PhabricatorUserLog.php @@ -29,6 +29,7 @@ const ACTION_ENTER_HISEC = 'hisec-enter'; const ACTION_EXIT_HISEC = 'hisec-exit'; + const ACTION_FAIL_HISEC = 'hisec-fail'; const ACTION_MULTI_ADD = 'multi-add'; const ACTION_MULTI_REMOVE = 'multi-remove'; @@ -66,6 +67,7 @@ self::ACTION_CHANGE_USERNAME => pht('Change Username'), self::ACTION_ENTER_HISEC => pht('Hisec: Enter'), self::ACTION_EXIT_HISEC => pht('Hisec: Exit'), + self::ACTION_FAIL_HISEC => pht('Hisec: Failed Attempt'), self::ACTION_MULTI_ADD => pht('Multi-Factor: Add Factor'), self::ACTION_MULTI_REMOVE => pht('Multi-Factor: Remove Factor'), ); diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelMultiFactor.php b/src/applications/settings/panel/PhabricatorSettingsPanelMultiFactor.php --- a/src/applications/settings/panel/PhabricatorSettingsPanelMultiFactor.php +++ b/src/applications/settings/panel/PhabricatorSettingsPanelMultiFactor.php @@ -15,11 +15,6 @@ return pht('Authentication'); } - public function isEnabled() { - // TODO: Enable this panel once more pieces work correctly. - return false; - } - public function processRequest(AphrontRequest $request) { if ($request->getExists('new')) { return $this->processNew($request); diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelSSHKeys.php b/src/applications/settings/panel/PhabricatorSettingsPanelSSHKeys.php --- a/src/applications/settings/panel/PhabricatorSettingsPanelSSHKeys.php +++ b/src/applications/settings/panel/PhabricatorSettingsPanelSSHKeys.php @@ -38,17 +38,10 @@ return $this->renderKeyListView($request); } - /* - - NOTE: Uncomment this to test hisec. - TOOD: Implement this fully once hisec does something useful. - $token = id(new PhabricatorAuthSessionEngine())->requireHighSecuritySession( $viewer, $request, - '/settings/panel/ssh/'); - - */ + $this->getPanelURI()); $id = nonempty($edit, $delete);