Page MenuHomePhabricator

D20088.diff
No OneTemporary

D20088.diff

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
@@ -123,6 +123,7 @@
->setUserPHID($viewer->getPHID())
->setSessionPHID($viewer->getSession()->getPHID())
->setFactorPHID($config->getPHID())
+ ->setIsNewChallenge(true)
->setWorkflowKey($engine->getWorkflowKey());
}
@@ -283,8 +284,11 @@
$error = $result->getErrorMessage();
- $icon = id(new PHUIIconView())
- ->setIcon('fa-clock-o', 'red');
+ $icon = $result->getIcon();
+ if (!$icon) {
+ $icon = id(new PHUIIconView())
+ ->setIcon('fa-clock-o', 'red');
+ }
return id(new PHUIFormTimerControl())
->setIcon($icon)
@@ -295,8 +299,11 @@
private function newAnsweredControl(
PhabricatorAuthFactorResult $result) {
- $icon = id(new PHUIIconView())
- ->setIcon('fa-check-circle-o', 'green');
+ $icon = $result->getIcon();
+ if (!$icon) {
+ $icon = id(new PHUIIconView())
+ ->setIcon('fa-check-circle-o', 'green');
+ }
return id(new PHUIFormTimerControl())
->setIcon($icon)
@@ -309,8 +316,11 @@
$error = $result->getErrorMessage();
- $icon = id(new PHUIIconView())
- ->setIcon('fa-times', 'red');
+ $icon = $result->getIcon();
+ if (!$icon) {
+ $icon = id(new PHUIIconView())
+ ->setIcon('fa-times', 'red');
+ }
return id(new PHUIFormTimerControl())
->setIcon($icon)
@@ -323,8 +333,11 @@
$error = $result->getErrorMessage();
- $icon = id(new PHUIIconView())
- ->setIcon('fa-commenting', 'green');
+ $icon = $result->getIcon();
+ if (!$icon) {
+ $icon = id(new PHUIIconView())
+ ->setIcon('fa-commenting', 'green');
+ }
return id(new PHUIFormTimerControl())
->setIcon($icon)
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
@@ -10,6 +10,7 @@
private $errorMessage;
private $value;
private $issuedChallenges = array();
+ private $icon;
public function setAnsweredChallenge(PhabricatorAuthChallenge $challenge) {
if (!$challenge->getIsAnsweredChallenge()) {
@@ -92,4 +93,13 @@
return $this->issuedChallenges;
}
+ public function setIcon(PHUIIconView $icon) {
+ $this->icon = $icon;
+ return $this;
+ }
+
+ public function getIcon() {
+ return $this->icon;
+ }
+
}
diff --git a/src/applications/auth/factor/PhabricatorDuoAuthFactor.php b/src/applications/auth/factor/PhabricatorDuoAuthFactor.php
--- a/src/applications/auth/factor/PhabricatorDuoAuthFactor.php
+++ b/src/applications/auth/factor/PhabricatorDuoAuthFactor.php
@@ -612,7 +612,22 @@
return $this->newResult()
->setAnsweredChallenge($challenge);
case 'waiting':
- // No result yet, we'll render a default state later on.
+ // If we didn't just issue this challenge, give the user a stronger
+ // hint that they need to follow the instructions.
+ if (!$challenge->getIsNewChallenge()) {
+ return $this->newResult()
+ ->setIsContinue(true)
+ ->setIcon(
+ id(new PHUIIconView())
+ ->setIcon('fa-exclamation-triangle', 'yellow'))
+ ->setErrorMessage(
+ pht(
+ 'You must approve the challenge which was sent to your '.
+ 'phone. Open the Duo application and confirm the challenge, '.
+ 'then continue.'));
+ }
+
+ // Otherwise, we'll construct a default message later on.
break;
default:
case 'deny':
@@ -666,8 +681,7 @@
public function getRequestHasChallengeResponse(
PhabricatorAuthFactorConfig $config,
AphrontRequest $request) {
- $value = $this->getChallengeResponseFromRequest($config, $request);
- return (bool)strlen($value);
+ return false;
}
protected function newResultFromChallengeResponse(
@@ -675,41 +689,7 @@
PhabricatorUser $viewer,
AphrontRequest $request,
array $challenges) {
-
- $challenge = $this->getChallengeForCurrentContext(
- $config,
- $viewer,
- $challenges);
-
- $code = $this->getChallengeResponseFromRequest(
- $config,
- $request);
-
- $result = $this->newResult()
- ->setValue($code);
-
- if ($challenge->getIsAnsweredChallenge()) {
- return $result->setAnsweredChallenge($challenge);
- }
-
- if (phutil_hashes_are_identical($code, $challenge->getChallengeKey())) {
- $ttl = PhabricatorTime::getNow() + phutil_units('15 minutes in seconds');
-
- $challenge
- ->markChallengeAsAnswered($ttl);
-
- return $result->setAnsweredChallenge($challenge);
- }
-
- if (strlen($code)) {
- $error_message = pht('Invalid');
- } else {
- $error_message = pht('Required');
- }
-
- $result->setErrorMessage($error_message);
-
- return $result;
+ return $this->newResult();
}
private function newDuoFuture(PhabricatorAuthFactorProvider $provider) {
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
@@ -16,6 +16,7 @@
protected $properties = array();
private $responseToken;
+ private $isNewChallenge;
const HTTPKEY = '__hisec.challenges__';
const TOKEN_DIGEST_KEY = 'auth.challenge.token';
@@ -241,6 +242,15 @@
return $this->properties[$key];
}
+ public function setIsNewChallenge($is_new_challenge) {
+ $this->isNewChallenge = $is_new_challenge;
+ return $this;
+ }
+
+ public function getIsNewChallenge() {
+ return $this->isNewChallenge;
+ }
+
/* -( PhabricatorPolicyInterface )----------------------------------------- */

File Metadata

Mime Type
text/plain
Expires
Sun, Mar 9, 4:25 PM (2 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7389765
Default Alt Text
D20088.diff (6 KB)

Event Timeline