Page MenuHomePhabricator
Files F17829701

D8886.diff
No OneTemporary
Actions

D8886.diff
View Options

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);

File Metadata

Mime Type
text/plain
Expires
Sun, Jul 27, 10:17 AM (3 d, 59 m ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
8649556
Default Alt Text
D8886.diff (11 KB)

Event Timeline

Phabricator · Built by Phacility