Page MenuHomePhabricator
Files F13995913

D18791.diff
No OneTemporary
Actions

D18791.diff
View Options

diff --git a/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php b/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php
--- a/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php
+++ b/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php
@@ -9,9 +9,21 @@
return false;
}
+ public function shouldRequireEnabledUser() {
+ // Users who haven't been approved yet are allowed to enroll in MFA. We'll
+ // kick disabled users out later.
+ return false;
+ }
+
public function handleRequest(AphrontRequest $request) {
$viewer = $this->getViewer();
+ if ($viewer->getIsDisabled()) {
+ // We allowed unapproved and disabled users to hit this controller, but
+ // want to kick out disabled users now.
+ return new Aphront400Response();
+ }
+
$panel = id(new PhabricatorMultiFactorSettingsPanel())
->setUser($viewer)
->setViewer($viewer)
diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php
--- a/src/applications/base/controller/PhabricatorController.php
+++ b/src/applications/base/controller/PhabricatorController.php
@@ -137,10 +137,6 @@
}
if ($this->shouldRequireEnabledUser()) {
- if ($user->isLoggedIn() && !$user->getIsApproved()) {
- $controller = new PhabricatorAuthNeedsApprovalController();
- return $this->delegateToController($controller);
- }
if ($user->getIsDisabled()) {
$controller = new PhabricatorDisabledUserController();
return $this->delegateToController($controller);
@@ -233,6 +229,17 @@
->withPHIDs(array($application->getPHID()))
->executeOne();
}
+
+ // If users need approval, require they wait here. We do this near the
+ // end so they can take other actions (like verifying email, signing
+ // documents, and enrolling in MFA) while waiting for an admin to take a
+ // look at things. See T13024 for more discussion.
+ if ($this->shouldRequireEnabledUser()) {
+ if ($user->isLoggedIn() && !$user->getIsApproved()) {
+ $controller = new PhabricatorAuthNeedsApprovalController();
+ return $this->delegateToController($controller);
+ }
+ }
}
// NOTE: We do this last so that users get a login page instead of a 403
diff --git a/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php b/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php
--- a/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php
+++ b/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php
@@ -159,10 +159,10 @@
$u_unverified,
$u_admin,
$u_public,
+ $u_notapproved,
),
array(
$u_disabled,
- $u_notapproved,
));
@@ -224,7 +224,7 @@
));
$this->checkAccess(
- pht('Application Controller'),
+ pht('Application Controller, No Login Required'),
id(clone $app_controller)->setConfig('login', false),
$request,
array(
@@ -232,10 +232,10 @@
$u_unverified,
$u_admin,
$u_public,
+ $u_notapproved,
),
array(
$u_disabled,
- $u_notapproved,
));
}

File Metadata

Mime Type
text/plain
Expires
Thu, Oct 24, 4:40 PM (3 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6731103
Default Alt Text
D18791.diff (3 KB)

Event Timeline

Phabricator · Built by Phacility