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 )----------------------------------------- */