Page MenuHomePhabricator
Files F15345325

D19885.id47482.diff
No OneTemporary
Actions

D19885.id47482.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -2198,6 +2198,7 @@
'PhabricatorAuthEditController' => 'applications/auth/controller/config/PhabricatorAuthEditController.php',
'PhabricatorAuthFactor' => 'applications/auth/factor/PhabricatorAuthFactor.php',
'PhabricatorAuthFactorConfig' => 'applications/auth/storage/PhabricatorAuthFactorConfig.php',
+ 'PhabricatorAuthFactorResult' => 'applications/auth/factor/PhabricatorAuthFactorResult.php',
'PhabricatorAuthFactorTestCase' => 'applications/auth/factor/__tests__/PhabricatorAuthFactorTestCase.php',
'PhabricatorAuthFinishController' => 'applications/auth/controller/PhabricatorAuthFinishController.php',
'PhabricatorAuthHMACKey' => 'applications/auth/storage/PhabricatorAuthHMACKey.php',
@@ -7832,6 +7833,7 @@
'PhabricatorAuthEditController' => 'PhabricatorAuthProviderConfigController',
'PhabricatorAuthFactor' => 'Phobject',
'PhabricatorAuthFactorConfig' => 'PhabricatorAuthDAO',
+ 'PhabricatorAuthFactorResult' => 'Phobject',
'PhabricatorAuthFactorTestCase' => 'PhabricatorTestCase',
'PhabricatorAuthFinishController' => 'PhabricatorAuthController',
'PhabricatorAuthHMACKey' => 'PhabricatorAuthDAO',
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
@@ -496,14 +496,25 @@
$id = $factor->getID();
$impl = $factor->requireImplementation();
- $validation_results[$id] = $impl->processValidateFactorForm(
+ $validation_result = $impl->processValidateFactorForm(
$factor,
$viewer,
$request);
- if (!$impl->isFactorValid($factor, $validation_results[$id])) {
+ if (!($validation_result instanceof PhabricatorAuthFactorResult)) {
+ throw new Exception(
+ pht(
+ 'Expected "processValidateFactorForm()" to return an object '.
+ 'of class "%s"; got something else (from "%s").',
+ 'PhabricatorAuthFactorResult',
+ get_class($impl)));
+ }
+
+ if (!$validation_result->getIsValid()) {
$ok = false;
}
+
+ $validation_results[$id] = $validation_result;
}
if ($ok) {
@@ -595,17 +606,20 @@
array $validation_results,
PhabricatorUser $viewer,
AphrontRequest $request) {
+ assert_instances_of($validation_results, 'PhabricatorAuthFactorResult');
$form = id(new AphrontFormView())
->setUser($viewer)
->appendRemarkupInstructions('');
foreach ($factors as $factor) {
+ $result = idx($validation_results, $factor->getID());
+
$factor->requireImplementation()->renderValidateFactorForm(
$factor,
$form,
$viewer,
- idx($validation_results, $factor->getID()));
+ $result);
}
$form->appendRemarkupInstructions('');
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
@@ -7,6 +7,7 @@
private $factorValidationResults;
public function setFactorValidationResults(array $results) {
+ assert_instances_of($results, 'PhabricatorAuthFactorResult');
$this->factorValidationResults = $results;
return $this;
}
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
@@ -14,19 +14,13 @@
PhabricatorAuthFactorConfig $config,
AphrontFormView $form,
PhabricatorUser $viewer,
- $validation_result);
+ PhabricatorAuthFactorResult $validation_result = null);
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) {
diff --git a/src/applications/auth/factor/PhabricatorAuthFactorResult.php b/src/applications/auth/factor/PhabricatorAuthFactorResult.php
new file mode 100644
--- /dev/null
+++ b/src/applications/auth/factor/PhabricatorAuthFactorResult.php
@@ -0,0 +1,37 @@
+<?php
+
+final class PhabricatorAuthFactorResult
+ extends Phobject {
+
+ private $isValid = false;
+ private $hint;
+ private $value;
+
+ public function setIsValid($is_valid) {
+ $this->isValid = $is_valid;
+ return $this;
+ }
+
+ public function getIsValid() {
+ return $this->isValid;
+ }
+
+ public function setHint($hint) {
+ $this->hint = $hint;
+ return $this;
+ }
+
+ public function getHint() {
+ return $this->hint;
+ }
+
+ public function setValue($value) {
+ $this->value = $value;
+ return $this;
+ }
+
+ public function getValue() {
+ return $this->value;
+ }
+
+}
diff --git a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php
--- a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php
+++ b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php
@@ -154,10 +154,14 @@
PhabricatorAuthFactorConfig $config,
AphrontFormView $form,
PhabricatorUser $viewer,
- $validation_result) {
+ PhabricatorAuthFactorResult $validation_result = null) {
- if (!$validation_result) {
- $validation_result = array();
+ if ($validation_result) {
+ $value = $validation_result->getValue();
+ $hint = $validation_result->getHint();
+ } else {
+ $value = null;
+ $hint = true;
}
$form->appendChild(
@@ -166,8 +170,8 @@
->setLabel(pht('App Code'))
->setDisableAutocomplete(true)
->setCaption(pht('Factor Name: %s', $config->getFactorName()))
- ->setValue(idx($validation_result, 'value'))
- ->setError(idx($validation_result, 'error', true)));
+ ->setValue($value)
+ ->setError($hint));
}
public function processValidateFactorForm(
@@ -178,21 +182,22 @@
$code = $request->getStr($this->getParameterName($config, 'totpcode'));
$key = new PhutilOpaqueEnvelope($config->getFactorSecret());
+ $result = id(new PhabricatorAuthFactorResult())
+ ->setValue($code);
+
if (self::verifyTOTPCode($viewer, $key, $code)) {
- return array(
- 'error' => null,
- 'value' => $code,
- 'valid' => true,
- );
+ $result->setIsValid(true);
} else {
- return array(
- 'error' => strlen($code) ? pht('Invalid') : pht('Required'),
- 'value' => $code,
- 'valid' => false,
- );
+ if (strlen($code)) {
+ $hint = pht('Invalid');
+ } else {
+ $hint = pht('Required');
+ }
+ $result->setHint($hint);
}
- }
+ return $result;
+ }
public static function generateNewTOTPKey() {
return strtoupper(Filesystem::readRandomCharacters(32));

File Metadata

Mime Type
text/plain
Expires
Tue, Mar 11, 9:51 AM (2 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7517496
Default Alt Text
D19885.id47482.diff (7 KB)

Event Timeline

Phabricator · Built by Phacility