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 @@ -147,7 +147,7 @@ $viewer, $challenges); - if ($new_challenges instanceof PhabricatorAuthFactorResult) { + if ($this->isAuthResult($new_challenges)) { unset($unguarded); return $new_challenges; } @@ -200,7 +200,7 @@ return $result; } - if (!($result instanceof PhabricatorAuthFactorResult)) { + if (!$this->isAuthResult($result)) { throw new Exception( pht( 'Expected "newResultFromIssuedChallenges()" to return null or '. @@ -232,7 +232,7 @@ $request, $challenges); - if (!($result instanceof PhabricatorAuthFactorResult)) { + if (!$this->isAuthResult($result)) { throw new Exception( pht( 'Expected "newResultFromChallengeResponse()" to return an object '. @@ -408,6 +408,10 @@ $provider, $user); + if ($this->isAuthResult($properties)) { + return $properties; + } + foreach ($properties as $key => $value) { $sync_token->setTemporaryTokenProperty($key, $value); } @@ -555,4 +559,8 @@ ->execute(); } + final protected function isAuthResult($object) { + return ($object instanceof PhabricatorAuthFactorResult); + } + } 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 @@ -157,6 +157,10 @@ PhabricatorUser $user) { $token = $this->loadMFASyncToken($provider, $request, $form, $user); + if ($this->isAuthResult($token)) { + $form->appendChild($this->newAutomaticControl($token)); + return; + } $enroll = $token->getTemporaryTokenProperty('duo.enroll'); $duo_id = $token->getTemporaryTokenProperty('duo.user-id'); @@ -350,6 +354,7 @@ $external_uri = null; $result_code = $result['response']['result']; + $status_message = $result['response']['status_msg']; switch ($result_code) { case 'auth': case 'allow': @@ -376,7 +381,13 @@ return $this->newResult() ->setIsError(true) ->setErrorMessage( - pht('Your account is not permitted to access this system.')); + pht( + 'Your Duo account ("%s") is not permitted to access this '. + 'system. Contact your Duo administrator for help. '. + 'The Duo preauth API responded with status message ("%s"): %s', + $duo_user, + $result_code, + $status_message)); } // Duo's "/enroll" API isn't repeatable for the same username. If we're @@ -476,7 +487,10 @@ ->setIsError(true) ->setErrorMessage( pht( - 'Duo has denied you access. Duo status message ("%s"): %s', + 'Your Duo account ("%s") is not permitted to access this '. + 'system. Contact your Duo administrator for help. The Duo '. + 'preauth API responded with status message ("%s"): %s', + $duo_user, $next_step, $status_message)); }