Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F13962257
D20028.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
20 KB
Referenced Files
None
Subscribers
None
D20028.diff
View Options
diff --git a/resources/celerity/map.php b/resources/celerity/map.php
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -9,7 +9,7 @@
'names' => array(
'conpherence.pkg.css' => '3c8a0668',
'conpherence.pkg.js' => '020aebcf',
- 'core.pkg.css' => 'a66ea2e7',
+ 'core.pkg.css' => 'e0cb8094',
'core.pkg.js' => '5c737607',
'differential.pkg.css' => 'b8df73d4',
'differential.pkg.js' => '67c9ea4c',
@@ -151,7 +151,7 @@
'rsrc/css/phui/phui-document.css' => '52b748a5',
'rsrc/css/phui/phui-feed-story.css' => 'a0c05029',
'rsrc/css/phui/phui-fontkit.css' => '9b714a5e',
- 'rsrc/css/phui/phui-form-view.css' => '9508671e',
+ 'rsrc/css/phui/phui-form-view.css' => '0807e7ac',
'rsrc/css/phui/phui-form.css' => '159e2d9c',
'rsrc/css/phui/phui-head-thing.css' => 'd7f293df',
'rsrc/css/phui/phui-header-view.css' => '93cea4ec',
@@ -159,7 +159,7 @@
'rsrc/css/phui/phui-icon-set-selector.css' => '7aa5f3ec',
'rsrc/css/phui/phui-icon.css' => '281f964d',
'rsrc/css/phui/phui-image-mask.css' => '62c7f4d2',
- 'rsrc/css/phui/phui-info-view.css' => 'f9464caf',
+ 'rsrc/css/phui/phui-info-view.css' => '37b8d9ce',
'rsrc/css/phui/phui-invisible-character-view.css' => 'c694c4a4',
'rsrc/css/phui/phui-left-right.css' => '68513c34',
'rsrc/css/phui/phui-lightbox.css' => '4ebf22da',
@@ -817,7 +817,7 @@
'phui-font-icon-base-css' => 'd7994e06',
'phui-fontkit-css' => '9b714a5e',
'phui-form-css' => '159e2d9c',
- 'phui-form-view-css' => '9508671e',
+ 'phui-form-view-css' => '0807e7ac',
'phui-head-thing-view-css' => 'd7f293df',
'phui-header-view-css' => '93cea4ec',
'phui-hovercard' => '074f0783',
@@ -825,7 +825,7 @@
'phui-icon-set-selector-css' => '7aa5f3ec',
'phui-icon-view-css' => '281f964d',
'phui-image-mask-css' => '62c7f4d2',
- 'phui-info-view-css' => 'f9464caf',
+ 'phui-info-view-css' => '37b8d9ce',
'phui-inline-comment-view-css' => '48acce5b',
'phui-invisible-character-view-css' => 'c694c4a4',
'phui-left-right-css' => '68513c34',
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
@@ -2239,6 +2239,7 @@
'PhabricatorAuthFactorProviderTransactionType' => 'applications/auth/xaction/PhabricatorAuthFactorProviderTransactionType.php',
'PhabricatorAuthFactorProviderViewController' => 'applications/auth/controller/mfa/PhabricatorAuthFactorProviderViewController.php',
'PhabricatorAuthFactorResult' => 'applications/auth/factor/PhabricatorAuthFactorResult.php',
+ 'PhabricatorAuthFactorResultException' => 'applications/auth/exception/PhabricatorAuthFactorResultException.php',
'PhabricatorAuthFactorTestCase' => 'applications/auth/factor/__tests__/PhabricatorAuthFactorTestCase.php',
'PhabricatorAuthFinishController' => 'applications/auth/controller/PhabricatorAuthFinishController.php',
'PhabricatorAuthHMACKey' => 'applications/auth/storage/PhabricatorAuthHMACKey.php',
@@ -7965,6 +7966,7 @@
'PhabricatorAuthFactorProviderTransactionType' => 'PhabricatorModularTransactionType',
'PhabricatorAuthFactorProviderViewController' => 'PhabricatorAuthFactorProviderController',
'PhabricatorAuthFactorResult' => 'Phobject',
+ 'PhabricatorAuthFactorResultException' => 'Exception',
'PhabricatorAuthFactorTestCase' => 'PhabricatorTestCase',
'PhabricatorAuthFinishController' => 'PhabricatorAuthController',
'PhabricatorAuthHMACKey' => 'PhabricatorAuthDAO',
diff --git a/src/aphront/handler/PhabricatorHighSecurityRequestExceptionHandler.php b/src/aphront/handler/PhabricatorHighSecurityRequestExceptionHandler.php
--- a/src/aphront/handler/PhabricatorHighSecurityRequestExceptionHandler.php
+++ b/src/aphront/handler/PhabricatorHighSecurityRequestExceptionHandler.php
@@ -38,10 +38,14 @@
$request);
$is_wait = false;
+ $is_continue = false;
foreach ($results as $result) {
if ($result->getIsWait()) {
$is_wait = true;
- break;
+ }
+
+ if ($result->getIsContinue()) {
+ $is_continue = true;
}
}
@@ -55,7 +59,7 @@
if ($is_wait) {
$submit = pht('Wait Patiently');
- } else if ($is_upgrade) {
+ } else if ($is_upgrade && !$is_continue) {
$submit = pht('Enter High Security');
} else {
$submit = pht('Continue');
@@ -74,19 +78,21 @@
$form_layout = $form->buildLayoutView();
if ($is_upgrade) {
+ $messages = array(
+ pht(
+ 'You are taking an action which requires you to enter '.
+ 'high security.'),
+ );
+
+ $info_view = id(new PHUIInfoView())
+ ->setSeverity(PHUIInfoView::SEVERITY_MFA)
+ ->setErrors($messages);
+
$dialog
- ->setErrors(
- array(
- pht(
- 'You are taking an action which requires you to enter '.
- 'high security.'),
- ))
+ ->appendChild($info_view)
->appendParagraph(
pht(
- 'High security mode helps protect your account from security '.
- 'threats, like session theft or someone messing with your stuff '.
- 'while you\'re grabbing a coffee. To enter high security mode, '.
- 'confirm your credentials.'))
+ 'To enter high security mode, confirm your credentials:'))
->appendChild($form_layout)
->appendParagraph(
pht(
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
@@ -47,6 +47,7 @@
private $workflowKey;
+ private $request;
public function setWorkflowKey($workflow_key) {
$this->workflowKey = $workflow_key;
@@ -65,6 +66,10 @@
return $this->workflowKey;
}
+ public function getRequest() {
+ return $this->request;
+ }
+
/**
* Get the session kind (e.g., anonymous, user, external account) from a
@@ -480,6 +485,7 @@
return $this->issueHighSecurityToken($session, true);
}
+ $this->request = $request;
foreach ($factors as $factor) {
$factor->setSessionEngine($this);
}
@@ -523,10 +529,17 @@
$provider = $factor->getFactorProvider();
$impl = $provider->getFactor();
- $new_challenges = $impl->getNewIssuedChallenges(
- $factor,
- $viewer,
- $issued_challenges);
+ try {
+ $new_challenges = $impl->getNewIssuedChallenges(
+ $factor,
+ $viewer,
+ $issued_challenges);
+ } catch (PhabricatorAuthFactorResultException $ex) {
+ $ok = false;
+ $validation_results[$factor_phid] = $ex->getResult();
+ $challenge_map[$factor_phid] = $issued_challenges;
+ continue;
+ }
foreach ($new_challenges as $new_challenge) {
$issued_challenges[] = $new_challenge;
@@ -546,7 +559,10 @@
continue;
}
- $ok = false;
+ if (!$result->getIsValid()) {
+ $ok = false;
+ }
+
$validation_results[$factor_phid] = $result;
}
diff --git a/src/applications/auth/exception/PhabricatorAuthFactorResultException.php b/src/applications/auth/exception/PhabricatorAuthFactorResultException.php
new file mode 100644
--- /dev/null
+++ b/src/applications/auth/exception/PhabricatorAuthFactorResultException.php
@@ -0,0 +1,17 @@
+<?php
+
+final class PhabricatorAuthFactorResultException
+ extends Exception {
+
+ private $result;
+
+ public function __construct(PhabricatorAuthFactorResult $result) {
+ $this->result = $result;
+ parent::__construct();
+ }
+
+ public function getResult() {
+ return $this->result;
+ }
+
+}
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
@@ -232,6 +232,16 @@
final protected function newAutomaticControl(
PhabricatorAuthFactorResult $result) {
+ $is_error = $result->getIsError();
+ if ($is_error) {
+ return $this->newErrorControl($result);
+ }
+
+ $is_continue = $result->getIsContinue();
+ if ($is_continue) {
+ return $this->newContinueControl($result);
+ }
+
$is_answered = (bool)$result->getAnsweredChallenge();
if ($is_answered) {
return $this->newAnsweredControl($result);
@@ -271,6 +281,34 @@
pht('You responded to this challenge correctly.'));
}
+ private function newErrorControl(
+ PhabricatorAuthFactorResult $result) {
+
+ $error = $result->getErrorMessage();
+
+ $icon = id(new PHUIIconView())
+ ->setIcon('fa-times', 'red');
+
+ return id(new PHUIFormTimerControl())
+ ->setIcon($icon)
+ ->appendChild($error)
+ ->setError(pht('Error'));
+ }
+
+ private function newContinueControl(
+ PhabricatorAuthFactorResult $result) {
+
+ $error = $result->getErrorMessage();
+
+ $icon = id(new PHUIIconView())
+ ->setIcon('fa-commenting', 'green');
+
+ return id(new PHUIFormTimerControl())
+ ->setIcon($icon)
+ ->appendChild($error);
+ }
+
+
/* -( Synchronizing New Factors )------------------------------------------ */
@@ -400,4 +438,86 @@
return null;
}
+
+ /**
+ * @phutil-external-symbol class QRcode
+ */
+ final protected function newQRCode($uri) {
+ $root = dirname(phutil_get_library_root('phabricator'));
+ require_once $root.'/externals/phpqrcode/phpqrcode.php';
+
+ $lines = QRcode::text($uri);
+
+ $total_width = 240;
+ $cell_size = floor($total_width / count($lines));
+
+ $rows = array();
+ foreach ($lines as $line) {
+ $cells = array();
+ for ($ii = 0; $ii < strlen($line); $ii++) {
+ if ($line[$ii] == '1') {
+ $color = '#000';
+ } else {
+ $color = '#fff';
+ }
+
+ $cells[] = phutil_tag(
+ 'td',
+ array(
+ 'width' => $cell_size,
+ 'height' => $cell_size,
+ 'style' => 'background: '.$color,
+ ),
+ '');
+ }
+ $rows[] = phutil_tag('tr', array(), $cells);
+ }
+
+ return phutil_tag(
+ 'table',
+ array(
+ 'style' => 'margin: 24px auto;',
+ ),
+ $rows);
+ }
+
+ final protected function throwResult(PhabricatorAuthFactorResult $result) {
+ throw new PhabricatorAuthFactorResultException($result);
+ }
+
+ final protected function getInstallDisplayName() {
+ $uri = PhabricatorEnv::getURI('/');
+ $uri = new PhutilURI($uri);
+ return $uri->getDomain();
+ }
+
+ final protected function getChallengeResponseParameterName(
+ PhabricatorAuthFactorConfig $config) {
+ return $this->getParameterName($config, 'mfa.response');
+ }
+
+ final protected function getChallengeResponseFromRequest(
+ PhabricatorAuthFactorConfig $config,
+ AphrontRequest $request) {
+
+ $name = $this->getChallengeResponseParameterName($config);
+
+ $value = $request->getStr($name);
+ $value = (string)$value;
+ $value = trim($value);
+
+ return $value;
+ }
+
+ final protected function hasCSRF(PhabricatorAuthFactorConfig $config) {
+ $engine = $config->getSessionEngine();
+ $request = $engine->getRequest();
+
+ if (!$request->isHTTPPost()) {
+ return false;
+ }
+
+ return $request->validateCSRF();
+ }
+
}
diff --git a/src/applications/auth/factor/PhabricatorAuthFactorResult.php b/src/applications/auth/factor/PhabricatorAuthFactorResult.php
--- a/src/applications/auth/factor/PhabricatorAuthFactorResult.php
+++ b/src/applications/auth/factor/PhabricatorAuthFactorResult.php
@@ -5,6 +5,8 @@
private $answeredChallenge;
private $isWait = false;
+ private $isError = false;
+ private $isContinue = false;
private $errorMessage;
private $value;
private $issuedChallenges = array();
@@ -44,6 +46,24 @@
return $this->isWait;
}
+ public function setIsError($is_error) {
+ $this->isError = $is_error;
+ return $this;
+ }
+
+ public function getIsError() {
+ return $this->isError;
+ }
+
+ public function setIsContinue($is_continue) {
+ $this->isContinue = $is_continue;
+ return $this;
+ }
+
+ public function getIsContinue() {
+ return $this->isContinue;
+ }
+
public function setErrorMessage($error_message) {
$this->errorMessage = $error_message;
return $this;
diff --git a/src/applications/auth/factor/PhabricatorSMSAuthFactor.php b/src/applications/auth/factor/PhabricatorSMSAuthFactor.php
--- a/src/applications/auth/factor/PhabricatorSMSAuthFactor.php
+++ b/src/applications/auth/factor/PhabricatorSMSAuthFactor.php
@@ -171,6 +171,38 @@
return array();
}
+ if (!$this->loadUserContactNumber($viewer)) {
+ $result = $this->newResult()
+ ->setIsError(true)
+ ->setErrorMessage(
+ pht(
+ 'Your account has no primary contact number.'));
+
+ $this->throwResult($result);
+ }
+
+ if (!$this->isSMSMailerConfigured()) {
+ $result = $this->newResult()
+ ->setIsError(true)
+ ->setErrorMessage(
+ pht(
+ 'No outbound mailer which can deliver SMS messages is '.
+ 'configured.'));
+
+ $this->throwResult($result);
+ }
+
+ if (!$this->hasCSRF($config)) {
+ $result = $this->newResult()
+ ->setIsContinue(true)
+ ->setErrorMessage(
+ pht(
+ 'A text message with an authorization code will be sent to your '.
+ 'primary contact number.'));
+
+ $this->throwResult($result);
+ }
+
// Otherwise, issue a new challenge.
$challenge_code = $this->newSMSChallengeCode();
@@ -329,10 +361,6 @@
private function sendSMSCodeToUser(
PhutilOpaqueEnvelope $envelope,
PhabricatorUser $user) {
-
- $uri = PhabricatorEnv::getURI('/');
- $uri = new PhutilURI($uri);
-
return id(new PhabricatorMetaMTAMail())
->setMessageType(PhabricatorMailSMSMessage::MESSAGETYPE)
->addTos(array($user->getPHID()))
@@ -341,7 +369,7 @@
->setBody(
pht(
'Phabricator (%s) MFA Code: %s',
- $uri->getDomain(),
+ $this->getInstallDisplayName(),
$envelope->openEnvelope()))
->save();
}
@@ -350,22 +378,4 @@
return trim($code);
}
- private function getChallengeResponseParameterName(
- PhabricatorAuthFactorConfig $config) {
- return $this->getParameterName($config, 'sms.code');
- }
-
- private function getChallengeResponseFromRequest(
- PhabricatorAuthFactorConfig $config,
- AphrontRequest $request) {
-
- $name = $this->getChallengeResponseParameterName($config);
-
- $value = $request->getStr($name);
- $value = (string)$value;
- $value = trim($value);
-
- return $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
@@ -90,7 +90,7 @@
$secret,
$issuer);
- $qrcode = $this->renderQRCode($uri);
+ $qrcode = $this->newQRCode($uri);
$form->appendChild($qrcode);
$form->appendChild(
@@ -390,49 +390,6 @@
return $code;
}
-
- /**
- * @phutil-external-symbol class QRcode
- */
- private function renderQRCode($uri) {
- $root = dirname(phutil_get_library_root('phabricator'));
- require_once $root.'/externals/phpqrcode/phpqrcode.php';
-
- $lines = QRcode::text($uri);
-
- $total_width = 240;
- $cell_size = floor($total_width / count($lines));
-
- $rows = array();
- foreach ($lines as $line) {
- $cells = array();
- for ($ii = 0; $ii < strlen($line); $ii++) {
- if ($line[$ii] == '1') {
- $color = '#000';
- } else {
- $color = '#fff';
- }
-
- $cells[] = phutil_tag(
- 'td',
- array(
- 'width' => $cell_size,
- 'height' => $cell_size,
- 'style' => 'background: '.$color,
- ),
- '');
- }
- $rows[] = phutil_tag('tr', array(), $cells);
- }
-
- return phutil_tag(
- 'table',
- array(
- 'style' => 'margin: 24px auto;',
- ),
- $rows);
- }
-
private function getTimestepDuration() {
return 30;
}
@@ -470,24 +427,6 @@
return null;
}
- private function getChallengeResponseParameterName(
- PhabricatorAuthFactorConfig $config) {
- return $this->getParameterName($config, 'totpcode');
- }
-
- private function getChallengeResponseFromRequest(
- PhabricatorAuthFactorConfig $config,
- AphrontRequest $request) {
-
- $name = $this->getChallengeResponseParameterName($config);
-
- $value = $request->getStr($name);
- $value = (string)$value;
- $value = trim($value);
-
- return $value;
- }
-
protected function newMFASyncTokenProperties(PhabricatorUser $user) {
return array(
'secret' => self::generateNewTOTPKey(),
diff --git a/src/applications/auth/future/PhabricatorDuoFuture.php b/src/applications/auth/future/PhabricatorDuoFuture.php
--- a/src/applications/auth/future/PhabricatorDuoFuture.php
+++ b/src/applications/auth/future/PhabricatorDuoFuture.php
@@ -112,6 +112,11 @@
$this->secretKey->openEnvelope());
$signature = new PhutilOpaqueEnvelope($signature);
+ if ($http_method === 'GET') {
+ $uri->setQueryParams($data);
+ $data = array();
+ }
+
$future = id(new HTTPSFuture($uri, $data))
->setHTTPBasicAuthCredentials($this->integrationKey, $signature)
->setMethod($http_method)
diff --git a/src/applications/auth/storage/PhabricatorAuthChallenge.php b/src/applications/auth/storage/PhabricatorAuthChallenge.php
--- a/src/applications/auth/storage/PhabricatorAuthChallenge.php
+++ b/src/applications/auth/storage/PhabricatorAuthChallenge.php
@@ -163,10 +163,16 @@
$token = Filesystem::readRandomCharacters(32);
$token = new PhutilOpaqueEnvelope($token);
- return $this
+ $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
+
+ $this
->setResponseToken($token)
->setResponseTTL($ttl)
->save();
+
+ unset($unguarded);
+
+ return $this;
}
public function markChallengeAsCompleted() {
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
@@ -71,6 +71,15 @@
return $this->mfaSyncToken;
}
+ public function getAuthFactorConfigProperty($key, $default = null) {
+ return idx($this->properties, $key, $default);
+ }
+
+ public function setAuthFactorConfigProperty($key, $value) {
+ $this->properties[$key] = $value;
+ return $this;
+ }
+
/* -( PhabricatorPolicyInterface )----------------------------------------- */
diff --git a/src/view/phui/PHUIInfoView.php b/src/view/phui/PHUIInfoView.php
--- a/src/view/phui/PHUIInfoView.php
+++ b/src/view/phui/PHUIInfoView.php
@@ -8,6 +8,7 @@
const SEVERITY_NODATA = 'nodata';
const SEVERITY_SUCCESS = 'success';
const SEVERITY_PLAIN = 'plain';
+ const SEVERITY_MFA = 'mfa';
private $title;
private $errors = array();
@@ -73,20 +74,22 @@
switch ($this->getSeverity()) {
case self::SEVERITY_ERROR:
$icon = 'fa-exclamation-circle';
- break;
+ break;
case self::SEVERITY_WARNING:
$icon = 'fa-exclamation-triangle';
- break;
+ break;
case self::SEVERITY_NOTICE:
$icon = 'fa-info-circle';
- break;
+ break;
case self::SEVERITY_PLAIN:
case self::SEVERITY_NODATA:
return null;
- break;
case self::SEVERITY_SUCCESS:
$icon = 'fa-check-circle';
- break;
+ break;
+ case self::SEVERITY_MFA:
+ $icon = 'fa-lock';
+ break;
}
$icon = id(new PHUIIconView())
diff --git a/webroot/rsrc/css/phui/phui-form-view.css b/webroot/rsrc/css/phui/phui-form-view.css
--- a/webroot/rsrc/css/phui/phui-form-view.css
+++ b/webroot/rsrc/css/phui/phui-form-view.css
@@ -574,3 +574,7 @@
color: {$darkgreytext};
vertical-align: middle;
}
+
+.mfa-form-enroll-button {
+ text-align: center;
+}
diff --git a/webroot/rsrc/css/phui/phui-info-view.css b/webroot/rsrc/css/phui/phui-info-view.css
--- a/webroot/rsrc/css/phui/phui-info-view.css
+++ b/webroot/rsrc/css/phui/phui-info-view.css
@@ -93,6 +93,15 @@
color: {$red};
}
+.phui-info-severity-mfa {
+ border-color: {$blue};
+ border-left-width: 6px;
+}
+
+.phui-info-severity-mfa .phui-info-icon {
+ color: {$blue};
+}
+
.phui-info-severity-warning {
border-color: {$yellow};
border-left-width: 6px;
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Wed, Oct 16, 9:03 AM (1 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6716876
Default Alt Text
D20028.diff (20 KB)
Attached To
Mode
D20028: Add CSRF to SMS challenges, and pave the way for more MFA types (including Duo)
Attached
Detach File
Event Timeline
Log In to Comment