mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-23 07:12:41 +01:00
Correct a possible fatal in the non-CSRF Duo MFA workflow
Summary: Ref T13259. If we miss the separate CSRF step in Duo and proceed directly to prompting, we may fail to build a response which turns into a real control and fatal on `null->setLabel()`. Instead, let MFA providers customize their "bare prompt dialog" response, then make Duo use the same "you have an outstanding request" response for the CSRF and no-CSRF workflows. Test Plan: Hit Duo auth on a non-CSRF workflow (e.g., edit an MFA provider with Duo enabled). Previously: `setLabel()` fatal. After patch: smooth sailing. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13259 Differential Revision: https://secure.phabricator.com/D20234
This commit is contained in:
parent
d192d04586
commit
920ab13cfb
3 changed files with 55 additions and 1 deletions
|
@ -714,7 +714,14 @@ final class PhabricatorAuthSessionEngine extends Phobject {
|
||||||
if (isset($validation_results[$factor_phid])) {
|
if (isset($validation_results[$factor_phid])) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
$validation_results[$factor_phid] = new PhabricatorAuthFactorResult();
|
|
||||||
|
$issued_challenges = idx($challenge_map, $factor_phid, array());
|
||||||
|
|
||||||
|
$validation_results[$factor_phid] = $impl->getResultForPrompt(
|
||||||
|
$factor,
|
||||||
|
$viewer,
|
||||||
|
$request,
|
||||||
|
$issued_challenges);
|
||||||
}
|
}
|
||||||
|
|
||||||
throw id(new PhabricatorAuthHighSecurityRequiredException())
|
throw id(new PhabricatorAuthHighSecurityRequiredException())
|
||||||
|
|
|
@ -221,6 +221,40 @@ abstract class PhabricatorAuthFactor extends Phobject {
|
||||||
return $result;
|
return $result;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
final public function getResultForPrompt(
|
||||||
|
PhabricatorAuthFactorConfig $config,
|
||||||
|
PhabricatorUser $viewer,
|
||||||
|
AphrontRequest $request,
|
||||||
|
array $challenges) {
|
||||||
|
assert_instances_of($challenges, 'PhabricatorAuthChallenge');
|
||||||
|
|
||||||
|
$result = $this->newResultForPrompt(
|
||||||
|
$config,
|
||||||
|
$viewer,
|
||||||
|
$request,
|
||||||
|
$challenges);
|
||||||
|
|
||||||
|
if (!$this->isAuthResult($result)) {
|
||||||
|
throw new Exception(
|
||||||
|
pht(
|
||||||
|
'Expected "newResultForPrompt()" to return an object of class "%s", '.
|
||||||
|
'but it returned something else ("%s"; in "%s").',
|
||||||
|
'PhabricatorAuthFactorResult',
|
||||||
|
phutil_describe_type($result),
|
||||||
|
get_class($this)));
|
||||||
|
}
|
||||||
|
|
||||||
|
return $result;
|
||||||
|
}
|
||||||
|
|
||||||
|
protected function newResultForPrompt(
|
||||||
|
PhabricatorAuthFactorConfig $config,
|
||||||
|
PhabricatorUser $viewer,
|
||||||
|
AphrontRequest $request,
|
||||||
|
array $challenges) {
|
||||||
|
return $this->newResult();
|
||||||
|
}
|
||||||
|
|
||||||
abstract protected function newResultFromIssuedChallenges(
|
abstract protected function newResultFromIssuedChallenges(
|
||||||
PhabricatorAuthFactorConfig $config,
|
PhabricatorAuthFactorConfig $config,
|
||||||
PhabricatorUser $viewer,
|
PhabricatorUser $viewer,
|
||||||
|
|
|
@ -681,6 +681,19 @@ final class PhabricatorDuoAuthFactor
|
||||||
AphrontRequest $request,
|
AphrontRequest $request,
|
||||||
array $challenges) {
|
array $challenges) {
|
||||||
|
|
||||||
|
return $this->getResultForPrompt(
|
||||||
|
$config,
|
||||||
|
$viewer,
|
||||||
|
$request,
|
||||||
|
$challenges);
|
||||||
|
}
|
||||||
|
|
||||||
|
protected function newResultForPrompt(
|
||||||
|
PhabricatorAuthFactorConfig $config,
|
||||||
|
PhabricatorUser $viewer,
|
||||||
|
AphrontRequest $request,
|
||||||
|
array $challenges) {
|
||||||
|
|
||||||
$result = $this->newResult()
|
$result = $this->newResult()
|
||||||
->setIsContinue(true)
|
->setIsContinue(true)
|
||||||
->setErrorMessage(
|
->setErrorMessage(
|
||||||
|
|
Loading…
Reference in a new issue