From f9673a72a87d61edd69be864491518e83d4645f7 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 13 Aug 2018 15:32:24 -0700 Subject: [PATCH] Allow "user.edit" to enable or disable users Summary: Depends on D19577. Ref T13164. See PHI642. This adds modern transaction-oriented enable/disable support. Currently, this also doesn't let you disable normal users even when you're an administrator. I'll refine the policy model later in this change series, since that's also the goal here (let users set "Can Disable Users" to some more broad set of users than "Administrators"). This also leaves us with two different edit pathways: the old UserEditor one and the new UserTransactionEditor one. The next couple diffs will redefine the other pathways in terms of this pathway. Test Plan: - Enabled/disabled a bot. - Tried to disable another non-bot user. This isn't allowed yet, since even as an administrator you don't have CAN_EDIT on them and currently need it: right now, there's no way for a particular set of transactions to say they can move forward with reduced permissions. - Tried to enable/disable myself. This isn't allowed since you can't enable/disable yourself. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13164 Differential Revision: https://secure.phabricator.com/D19579 --- src/__phutil_library_map__.php | 2 + .../editor/PhabricatorUserEditEngine.php | 13 +++- .../PhabricatorUserDisableTransaction.php | 72 +++++++++++++++++++ .../PhabricatorUserTransactionType.php | 11 ++- 4 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 src/applications/people/xaction/PhabricatorUserDisableTransaction.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 628f4958b5..c8b84d19fe 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -4562,6 +4562,7 @@ phutil_register_library_map(array( 'PhabricatorUserCustomFieldNumericIndex' => 'applications/people/storage/PhabricatorUserCustomFieldNumericIndex.php', 'PhabricatorUserCustomFieldStringIndex' => 'applications/people/storage/PhabricatorUserCustomFieldStringIndex.php', 'PhabricatorUserDAO' => 'applications/people/storage/PhabricatorUserDAO.php', + 'PhabricatorUserDisableTransaction' => 'applications/people/xaction/PhabricatorUserDisableTransaction.php', 'PhabricatorUserEditEngine' => 'applications/people/editor/PhabricatorUserEditEngine.php', 'PhabricatorUserEditor' => 'applications/people/editor/PhabricatorUserEditor.php', 'PhabricatorUserEditorTestCase' => 'applications/people/editor/__tests__/PhabricatorUserEditorTestCase.php', @@ -10549,6 +10550,7 @@ phutil_register_library_map(array( 'PhabricatorUserCustomFieldNumericIndex' => 'PhabricatorCustomFieldNumericIndexStorage', 'PhabricatorUserCustomFieldStringIndex' => 'PhabricatorCustomFieldStringIndexStorage', 'PhabricatorUserDAO' => 'PhabricatorLiskDAO', + 'PhabricatorUserDisableTransaction' => 'PhabricatorUserTransactionType', 'PhabricatorUserEditEngine' => 'PhabricatorEditEngine', 'PhabricatorUserEditor' => 'PhabricatorEditor', 'PhabricatorUserEditorTestCase' => 'PhabricatorTestCase', diff --git a/src/applications/people/editor/PhabricatorUserEditEngine.php b/src/applications/people/editor/PhabricatorUserEditEngine.php index b978eb2618..c547426b12 100644 --- a/src/applications/people/editor/PhabricatorUserEditEngine.php +++ b/src/applications/people/editor/PhabricatorUserEditEngine.php @@ -64,7 +64,18 @@ final class PhabricatorUserEditEngine } protected function buildCustomEditFields($object) { - return array(); + return array( + id(new PhabricatorBoolEditField()) + ->setKey('disabled') + ->setOptions(pht('Active'), pht('Disabled')) + ->setLabel(pht('Disabled')) + ->setDescription(pht('Disable the user.')) + ->setTransactionType(PhabricatorUserDisableTransaction::TRANSACTIONTYPE) + ->setIsConduitOnly(true) + ->setConduitDescription(pht('Disable or enable the user.')) + ->setConduitTypeDescription(pht('True to disable the user.')) + ->setValue($object->getIsDisabled()), + ); } } diff --git a/src/applications/people/xaction/PhabricatorUserDisableTransaction.php b/src/applications/people/xaction/PhabricatorUserDisableTransaction.php new file mode 100644 index 0000000000..0ea6ef36c1 --- /dev/null +++ b/src/applications/people/xaction/PhabricatorUserDisableTransaction.php @@ -0,0 +1,72 @@ +getIsDisabled(); + } + + public function generateNewValue($object, $value) { + return (bool)$value; + } + + public function applyInternalEffects($object, $value) { + $object->setIsDisabled((int)$value); + + $this->newUserLog(PhabricatorUserLog::ACTION_DISABLE) + ->setOldValue((bool)$object->getIsDisabled()) + ->setNewValue((bool)$value) + ->save(); + } + + public function getTitle() { + $new = $this->getNewValue(); + if ($new) { + return pht( + '%s disabled this user.', + $this->renderAuthor()); + } else { + return pht( + '%s enabled this user.', + $this->renderAuthor()); + } + } + + public function getTitleForFeed() { + $new = $this->getNewValue(); + if ($new) { + return pht( + '%s disabled %s.', + $this->renderAuthor(), + $this->renderObject()); + } else { + return pht( + '%s enabled %s.', + $this->renderAuthor(), + $this->renderObject()); + } + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + foreach ($xactions as $xaction) { + $is_disabled = (bool)$object->getIsDisabled(); + + if ((bool)$xaction->getNewValue() === $is_disabled) { + continue; + } + + if ($this->getActingAsPHID() === $object->getPHID()) { + $errors[] = $this->newInvalidError( + pht('You can not enable or disable your own account.')); + } + } + + return $errors; + } + +} diff --git a/src/applications/people/xaction/PhabricatorUserTransactionType.php b/src/applications/people/xaction/PhabricatorUserTransactionType.php index 89392fd039..dcd45d480e 100644 --- a/src/applications/people/xaction/PhabricatorUserTransactionType.php +++ b/src/applications/people/xaction/PhabricatorUserTransactionType.php @@ -1,4 +1,13 @@ getActor(), + $this->getObject()->getPHID(), + $action); + } + +}