1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-01-22 20:51:10 +01:00

Convert "Rename User" from session MFA to one-shot MFA

Summary:
Depends on D20035. Ref T13222.

  - Allow individual transactions to request one-shot MFA if available.
  - Make "change username" request MFA.

Test Plan:
  - Renamed a user, got prompted for MFA, provided it.
  - Saw that I no longer remain in high-security after performing the edit.
  - Grepped for other uses of `PhabricatorUserUsernameTransaction`, found none.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13222

Differential Revision: https://secure.phabricator.com/D20036
This commit is contained in:
epriestley 2019-01-25 10:16:49 -08:00
parent 29b4fad941
commit d24e66724d
4 changed files with 49 additions and 20 deletions

View file

@ -17,14 +17,9 @@ final class PhabricatorPeopleRenameController
$done_uri = $this->getApplicationURI("manage/{$id}/");
id(new PhabricatorAuthSessionEngine())->requireHighSecuritySession(
$viewer,
$request,
$done_uri);
$validation_exception = null;
$username = $user->getUsername();
if ($request->isFormPost()) {
if ($request->isFormOrHisecPost()) {
$username = $request->getStr('username');
$xactions = array();
@ -36,6 +31,7 @@ final class PhabricatorPeopleRenameController
$editor = id(new PhabricatorUserTransactionEditor())
->setActor($viewer)
->setContentSourceFromRequest($request)
->setCancelURI($done_uri)
->setContinueOnMissingFields(true);
try {

View file

@ -89,4 +89,11 @@ final class PhabricatorUserUsernameTransaction
return null;
}
public function shouldTryMFA(
$object,
PhabricatorApplicationTransaction $xaction) {
return true;
}
}

View file

@ -4906,20 +4906,47 @@ abstract class PhabricatorApplicationTransactionEditor
PhabricatorLiskDAO $object,
array $xactions) {
$is_mfa = ($object instanceof PhabricatorEditEngineMFAInterface);
if (!$is_mfa) {
$has_engine = ($object instanceof PhabricatorEditEngineMFAInterface);
if ($has_engine) {
$engine = PhabricatorEditEngineMFAEngine::newEngineForObject($object)
->setViewer($this->getActor());
$require_mfa = $engine->shouldRequireMFA();
$try_mfa = $engine->shouldTryMFA();
} else {
$require_mfa = false;
$try_mfa = false;
}
// If the user is mentioning an MFA object on another object or creating
// a relationship like "parent" or "child" to this object, we always
// allow the edit to move forward without requiring MFA.
if ($this->getIsInverseEdgeEditor()) {
return $xactions;
}
$engine = PhabricatorEditEngineMFAEngine::newEngineForObject($object)
->setViewer($this->getActor());
$require_mfa = $engine->shouldRequireMFA();
if (!$require_mfa) {
$try_mfa = $engine->shouldTryMFA();
// If the object hasn't already opted into MFA, see if any of the
// transactions want it.
if (!$try_mfa) {
foreach ($xactions as $xaction) {
$type = $xaction->getTransactionType();
$xtype = $this->getModularTransactionType($type);
if ($xtype) {
$xtype = clone $xtype;
$xtype->setStorage($xaction);
if ($xtype->shouldTryMFA($object, $xaction)) {
$try_mfa = true;
break;
}
}
}
}
if ($try_mfa) {
$this->setShouldRequireMFA(true);
}
return $xactions;
}
@ -4937,13 +4964,6 @@ abstract class PhabricatorApplicationTransactionEditor
return $xactions;
}
// If the user is mentioning an MFA object on another object or creating
// a relationship like "parent" or "child" to this object, we allow the
// edit to move forward without requiring MFA.
if ($this->getIsInverseEdgeEditor()) {
return $xactions;
}
$template = $object->getApplicationTransactionTemplate();
$mfa_xaction = id(clone $template)

View file

@ -425,4 +425,10 @@ abstract class PhabricatorModularTransactionType
return PhabricatorPolicyCapability::CAN_EDIT;
}
public function shouldTryMFA(
$object,
PhabricatorApplicationTransaction $xaction) {
return false;
}
}