From f686a0b8275eb0dd63ff3717b835a32d5e6ed0b7 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 26 May 2020 07:02:31 -0700 Subject: [PATCH] In Phortune accounts, prevent self-removal more narrowly Summary: Currently, Phortune attempts to prevent users from removing themselves as account managers. It does this by checking that the new list includes them. Usually this is sufficient, because you can't normally edit an account unless you're already a manager. However, we get the wrong result (incorrect rejection of the edit) if the actor is omnipotent and the acting user was not already a member. It's okay to edit an account into a state which doesn't include you if you have permission to edit the account and aren't already a manager. Specifically, this supports more formal tooling around staff modifications to billing accounts, where the actor has staff-omnipotence and the acting user is a staff member and only used for purposes of leaving a useful audit trail. Test Plan: Elsewhere, ran staff tooling to modify accounts and was able to act as "alice" to add "bailey", even though "alice" was not herself a manager. Differential Revision: https://secure.phabricator.com/D21288 --- src/applications/phortune/editor/PhortuneAccountEditor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/applications/phortune/editor/PhortuneAccountEditor.php b/src/applications/phortune/editor/PhortuneAccountEditor.php index 50a71c476f..8344bc2b6e 100644 --- a/src/applications/phortune/editor/PhortuneAccountEditor.php +++ b/src/applications/phortune/editor/PhortuneAccountEditor.php @@ -63,7 +63,7 @@ final class PhortuneAccountEditor } $actor_phid = $this->getActingAsPHID(); - if (!isset($new[$actor_phid])) { + if (isset($old[$actor_phid]) && !isset($new[$actor_phid])) { $error = new PhabricatorApplicationTransactionValidationError( $type, pht('Invalid'),