From 2022a70e163d0952b658fab93b61800d3fdb7922 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 1 May 2014 18:23:31 -0700 Subject: [PATCH] Implement bin/remove, for structured destruction of objects Summary: Ref T4749. Ref T3265. Ref T4909. Several goals here: - Move user destruction to the CLI to limit the power of rogue admins. - Start consolidating all "destroy named object" scripts into a single UI, to make it easier to know how to destroy things. - Structure object destruction so we can do a better and more automatic job of cleaning up transactions, edges, search indexes, etc. - Log when we destroy objects so there's a record if data goes missing. Test Plan: Used `bin/remove destroy` to destroy several users. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T3265, T4749, T4909 Differential Revision: https://secure.phabricator.com/D8940 --- bin/remove | 1 + .../autopatches/20140501.remove.1.dlog.sql | 9 ++ scripts/setup/manage_remove.php | 21 ++++ src/__phutil_library_map__.php | 14 +++ .../PhabricatorPeopleDeleteController.php | 60 ++-------- .../people/editor/PhabricatorUserEditor.php | 63 ---------- .../people/storage/PhabricatorUser.php | 70 ++++++++++- .../engine/PhabricatorDestructionEngine.php | 60 ++++++++++ ...catorSystemDestructionGarbageCollector.php | 21 ++++ .../PhabricatorDestructableInterface.php | 24 ++++ ...PhabricatorSystemRemoveDestroyWorkflow.php | 110 ++++++++++++++++++ .../PhabricatorSystemRemoveLogWorkflow.php | 30 +++++ .../PhabricatorSystemRemoveWorkflow.php | 6 + .../PhabricatorSystemDestructionLog.php | 17 +++ 14 files changed, 391 insertions(+), 115 deletions(-) create mode 120000 bin/remove create mode 100644 resources/sql/autopatches/20140501.remove.1.dlog.sql create mode 100755 scripts/setup/manage_remove.php create mode 100644 src/applications/system/engine/PhabricatorDestructionEngine.php create mode 100644 src/applications/system/garbagecollector/PhabricatorSystemDestructionGarbageCollector.php create mode 100644 src/applications/system/interface/PhabricatorDestructableInterface.php create mode 100644 src/applications/system/management/PhabricatorSystemRemoveDestroyWorkflow.php create mode 100644 src/applications/system/management/PhabricatorSystemRemoveLogWorkflow.php create mode 100644 src/applications/system/management/PhabricatorSystemRemoveWorkflow.php create mode 100644 src/applications/system/storage/PhabricatorSystemDestructionLog.php diff --git a/bin/remove b/bin/remove new file mode 120000 index 0000000000..a073e3ebef --- /dev/null +++ b/bin/remove @@ -0,0 +1 @@ +../scripts/setup/manage_remove.php \ No newline at end of file diff --git a/resources/sql/autopatches/20140501.remove.1.dlog.sql b/resources/sql/autopatches/20140501.remove.1.dlog.sql new file mode 100644 index 0000000000..526bcd87da --- /dev/null +++ b/resources/sql/autopatches/20140501.remove.1.dlog.sql @@ -0,0 +1,9 @@ +CREATE TABLE {$NAMESPACE}_system.system_destructionlog ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + objectClass VARCHAR(128) NOT NULL COLLATE utf8_bin, + rootLogID INT UNSIGNED, + objectPHID VARCHAR(64) COLLATE utf8_bin, + objectMonogram VARCHAR(64) COLLATE utf8_bin, + epoch INT UNSIGNED NOT NULL, + KEY `key_epoch` (epoch) +) ENGINE=InnoDB, COLLATE utf8_general_ci; diff --git a/scripts/setup/manage_remove.php b/scripts/setup/manage_remove.php new file mode 100755 index 0000000000..db7dd19269 --- /dev/null +++ b/scripts/setup/manage_remove.php @@ -0,0 +1,21 @@ +#!/usr/bin/env php +setTagline('remove objects'); +$args->setSynopsis(<<parseStandardArguments(); + +$workflows = id(new PhutilSymbolLoader()) + ->setAncestorClass('PhabricatorSystemRemoveWorkflow') + ->loadObjects(); +$workflows[] = new PhutilHelpArgumentWorkflow(); +$args->parseWorkflows($workflows); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 69d55a8570..1768e557ba 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1471,6 +1471,8 @@ phutil_register_library_map(array( 'PhabricatorDebugController' => 'applications/system/controller/PhabricatorDebugController.php', 'PhabricatorDefaultFileStorageEngineSelector' => 'applications/files/engineselector/PhabricatorDefaultFileStorageEngineSelector.php', 'PhabricatorDefaultSearchEngineSelector' => 'applications/search/selector/PhabricatorDefaultSearchEngineSelector.php', + 'PhabricatorDestructableInterface' => 'applications/system/interface/PhabricatorDestructableInterface.php', + 'PhabricatorDestructionEngine' => 'applications/system/engine/PhabricatorDestructionEngine.php', 'PhabricatorDeveloperConfigOptions' => 'applications/config/option/PhabricatorDeveloperConfigOptions.php', 'PhabricatorDifferenceEngine' => 'infrastructure/diff/PhabricatorDifferenceEngine.php', 'PhabricatorDifferentialConfigOptions' => 'applications/differential/config/PhabricatorDifferentialConfigOptions.php', @@ -2179,6 +2181,11 @@ phutil_register_library_map(array( 'PhabricatorSystemActionLog' => 'applications/system/storage/PhabricatorSystemActionLog.php', 'PhabricatorSystemActionRateLimitException' => 'applications/system/exception/PhabricatorSystemActionRateLimitException.php', 'PhabricatorSystemDAO' => 'applications/system/storage/PhabricatorSystemDAO.php', + 'PhabricatorSystemDestructionGarbageCollector' => 'applications/system/garbagecollector/PhabricatorSystemDestructionGarbageCollector.php', + 'PhabricatorSystemDestructionLog' => 'applications/system/storage/PhabricatorSystemDestructionLog.php', + 'PhabricatorSystemRemoveDestroyWorkflow' => 'applications/system/management/PhabricatorSystemRemoveDestroyWorkflow.php', + 'PhabricatorSystemRemoveLogWorkflow' => 'applications/system/management/PhabricatorSystemRemoveLogWorkflow.php', + 'PhabricatorSystemRemoveWorkflow' => 'applications/system/management/PhabricatorSystemRemoveWorkflow.php', 'PhabricatorTaskmasterDaemon' => 'infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php', 'PhabricatorTestCase' => 'infrastructure/testing/PhabricatorTestCase.php', 'PhabricatorTestController' => 'applications/base/controller/__tests__/PhabricatorTestController.php', @@ -4297,6 +4304,7 @@ phutil_register_library_map(array( 'PhabricatorDebugController' => 'PhabricatorController', 'PhabricatorDefaultFileStorageEngineSelector' => 'PhabricatorFileStorageEngineSelector', 'PhabricatorDefaultSearchEngineSelector' => 'PhabricatorSearchEngineSelector', + 'PhabricatorDestructionEngine' => 'Phobject', 'PhabricatorDeveloperConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorDifferentialConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorDifferentialRevisionTestDataGenerator' => 'PhabricatorTestDataGenerator', @@ -5103,6 +5111,11 @@ phutil_register_library_map(array( 'PhabricatorSystemActionLog' => 'PhabricatorSystemDAO', 'PhabricatorSystemActionRateLimitException' => 'Exception', 'PhabricatorSystemDAO' => 'PhabricatorLiskDAO', + 'PhabricatorSystemDestructionGarbageCollector' => 'PhabricatorGarbageCollector', + 'PhabricatorSystemDestructionLog' => 'PhabricatorSystemDAO', + 'PhabricatorSystemRemoveDestroyWorkflow' => 'PhabricatorSystemRemoveWorkflow', + 'PhabricatorSystemRemoveLogWorkflow' => 'PhabricatorSystemRemoveWorkflow', + 'PhabricatorSystemRemoveWorkflow' => 'PhabricatorManagementWorkflow', 'PhabricatorTaskmasterDaemon' => 'PhabricatorDaemon', 'PhabricatorTestCase' => 'ArcanistPhutilTestCase', 'PhabricatorTestController' => 'PhabricatorController', @@ -5159,6 +5172,7 @@ phutil_register_library_map(array( 1 => 'PhutilPerson', 2 => 'PhabricatorPolicyInterface', 3 => 'PhabricatorCustomFieldInterface', + 4 => 'PhabricatorDestructableInterface', ), 'PhabricatorUserBlurbField' => 'PhabricatorUserCustomField', 'PhabricatorUserConfigOptions' => 'PhabricatorApplicationConfigOptions', diff --git a/src/applications/people/controller/PhabricatorPeopleDeleteController.php b/src/applications/people/controller/PhabricatorPeopleDeleteController.php index 25ba5c751f..fcd819e06b 100644 --- a/src/applications/people/controller/PhabricatorPeopleDeleteController.php +++ b/src/applications/people/controller/PhabricatorPeopleDeleteController.php @@ -27,36 +27,6 @@ final class PhabricatorPeopleDeleteController return $this->buildDeleteSelfResponse($profile_uri); } - $errors = array(); - - $v_username = ''; - $e_username = true; - if ($request->isFormPost()) { - $v_username = $request->getStr('username'); - - if (!strlen($v_username)) { - $errors[] = pht( - 'You must type the username to confirm that you want to delete '. - 'this user account.'); - $e_username = pht('Required'); - } else if ($v_username != $user->getUsername()) { - $errors[] = pht( - 'You must type the username correctly to confirm that you want '. - 'to delete this user account.'); - $e_username = pht('Incorrect'); - } - - if (!$errors) { - id(new PhabricatorUserEditor()) - ->setActor($admin) - ->deleteUser($user); - - $done_uri = $this->getApplicationURI(); - - return id(new AphrontRedirectResponse())->setURI($done_uri); - } - } - $str1 = pht( 'Be careful when deleting users! This will permanently and '. 'irreversibly destroy this user account.'); @@ -66,7 +36,7 @@ final class PhabricatorPeopleDeleteController 'disable them, not delete them. If you delete them, it will no longer '. 'be possible to (for example) search for objects they created, and you '. 'will lose other information about their history. Disabling them '. - 'instead will prevent them from logging in but not destroy any of '. + 'instead will prevent them from logging in, but will not destroy any of '. 'their data.'); $str3 = pht( @@ -74,38 +44,26 @@ final class PhabricatorPeopleDeleteController 'so on), but less safe to delete established users. If possible, '. 'disable them instead.'); + $str4 = pht( + 'To permanently destroy this user, run this command:'); + $form = id(new AphrontFormView()) ->setUser($admin) ->appendRemarkupInstructions( pht( - 'To confirm that you want to permanently and irrevocably destroy '. - 'this user account, type their username:')) - ->appendChild( - id(new AphrontFormStaticControl()) - ->setLabel(pht('Username')) - ->setValue($user->getUsername())) - ->appendChild( - id(new AphrontFormTextControl()) - ->setLabel(pht('Confirm')) - ->setValue($v_username) - ->setName('username') - ->setError($e_username)); - - if ($errors) { - $errors = id(new AphrontErrorView())->setErrors($errors); - } + " phabricator/ $ ./bin/remove destroy %s\n", + csprintf('%R', '@'.$user->getUsername()))); return $this->newDialog() ->setWidth(AphrontDialogView::WIDTH_FORM) - ->setTitle(pht('Really Delete User?')) + ->setTitle(pht('Permanently Delete User')) ->setShortTitle(pht('Delete User')) - ->appendChild($errors) ->appendParagraph($str1) ->appendParagraph($str2) ->appendParagraph($str3) + ->appendParagraph($str4) ->appendChild($form->buildLayoutView()) - ->addSubmitButton(pht('Delete User')) - ->addCancelButton($profile_uri); + ->addCancelButton($profile_uri, pht('Close')); } private function buildDeleteSelfResponse($profile_uri) { diff --git a/src/applications/people/editor/PhabricatorUserEditor.php b/src/applications/people/editor/PhabricatorUserEditor.php index c368c5c834..85cb7eabea 100644 --- a/src/applications/people/editor/PhabricatorUserEditor.php +++ b/src/applications/people/editor/PhabricatorUserEditor.php @@ -332,69 +332,6 @@ final class PhabricatorUserEditor extends PhabricatorEditor { return $this; } - /** - * @task role - */ - public function deleteUser(PhabricatorUser $user, $disable) { - $actor = $this->requireActor(); - - if (!$user->getID()) { - throw new Exception("User has not been created yet!"); - } - - if ($actor->getPHID() == $user->getPHID()) { - throw new Exception("You can not delete yourself!"); - } - - $user->openTransaction(); - $externals = id(new PhabricatorExternalAccount())->loadAllWhere( - 'userPHID = %s', - $user->getPHID()); - foreach ($externals as $external) { - $external->delete(); - } - - $prefs = id(new PhabricatorUserPreferences())->loadAllWhere( - 'userPHID = %s', - $user->getPHID()); - foreach ($prefs as $pref) { - $pref->delete(); - } - - $profiles = id(new PhabricatorUserProfile())->loadAllWhere( - 'userPHID = %s', - $user->getPHID()); - foreach ($profiles as $profile) { - $profile->delete(); - } - - $keys = id(new PhabricatorUserSSHKey())->loadAllWhere( - 'userPHID = %s', - $user->getPHID()); - foreach ($keys as $key) { - $key->delete(); - } - - $emails = id(new PhabricatorUserEmail())->loadAllWhere( - 'userPHID = %s', - $user->getPHID()); - foreach ($emails as $email) { - $email->delete(); - } - - $log = PhabricatorUserLog::initializeNewLog( - $actor, - $user->getPHID(), - PhabricatorUserLog::ACTION_DELETE); - $log->save(); - - $user->delete(); - - $user->saveTransaction(); - - return $this; - } - /* -( Adding, Removing and Changing Email )-------------------------------- */ diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index a62be09fe7..051a77f2b8 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -5,7 +5,8 @@ final class PhabricatorUser implements PhutilPerson, PhabricatorPolicyInterface, - PhabricatorCustomFieldInterface { + PhabricatorCustomFieldInterface, + PhabricatorDestructableInterface { const SESSION_TABLE = 'phabricator_session'; const NAMETOKEN_TABLE = 'user_nametoken'; @@ -139,6 +140,10 @@ final class PhabricatorUser return $this->sex; } + public function getMonogram() { + return '@'.$this->getUsername(); + } + public function getTranslation() { try { if ($this->translation && @@ -814,4 +819,67 @@ EOBODY; return $this; } + +/* -( PhabricatorDestructableInterface )----------------------------------- */ + + + public function destroyObjectPermanently( + PhabricatorDestructionEngine $engine) { + + $this->openTransaction(); + $this->delete(); + + $externals = id(new PhabricatorExternalAccount())->loadAllWhere( + 'userPHID = %s', + $this->getPHID()); + foreach ($externals as $external) { + $external->delete(); + } + + $prefs = id(new PhabricatorUserPreferences())->loadAllWhere( + 'userPHID = %s', + $this->getPHID()); + foreach ($prefs as $pref) { + $pref->delete(); + } + + $profiles = id(new PhabricatorUserProfile())->loadAllWhere( + 'userPHID = %s', + $this->getPHID()); + foreach ($profiles as $profile) { + $profile->delete(); + } + + $keys = id(new PhabricatorUserSSHKey())->loadAllWhere( + 'userPHID = %s', + $this->getPHID()); + foreach ($keys as $key) { + $key->delete(); + } + + $emails = id(new PhabricatorUserEmail())->loadAllWhere( + 'userPHID = %s', + $this->getPHID()); + foreach ($emails as $email) { + $email->delete(); + } + + $sessions = id(new PhabricatorAuthSession())->loadAllWhere( + 'userPHID = %s', + $this->getPHID()); + foreach ($sessions as $session) { + $session->delete(); + } + + $factors = id(new PhabricatorAuthFactorConfig())->loadAllWhere( + 'userPHID = %s', + $this->getPHID()); + foreach ($factors as $factor) { + $factor->delete(); + } + + $this->saveTransaction(); + } + + } diff --git a/src/applications/system/engine/PhabricatorDestructionEngine.php b/src/applications/system/engine/PhabricatorDestructionEngine.php new file mode 100644 index 0000000000..99ea47b2be --- /dev/null +++ b/src/applications/system/engine/PhabricatorDestructionEngine.php @@ -0,0 +1,60 @@ +setEpoch(time()) + ->setObjectClass(get_class($object)); + + if ($this->rootLogID) { + $log->setRootLogID($this->rootLogID); + } + + $object_phid = null; + if (method_exists($object, 'getPHID')) { + try { + $object_phid = $object->getPHID(); + $log->setObjectPHID($object_phid); + } catch (Exception $ex) { + // Ignore. + } + } + + if (method_exists($object, 'getMonogram')) { + try { + $log->setObjectMonogram($object->getMonogram()); + } catch (Exception $ex) { + // Ignore. + } + } + + $log->save(); + + if (!$this->rootLogID) { + $this->rootLogID = $log->getID(); + } + + $object->destroyObjectPermanently($this); + + if ($object_phid) { + $this->destroyEdges($object_phid); + } + } + + private function destroyEdges($src_phid) { + $edges = id(new PhabricatorEdgeQuery()) + ->withSourcePHIDs(array($src_phid)) + ->execute(); + + $editor = id(new PhabricatorEdgeEditor()) + ->setSuppressEvents(true); + foreach ($edges as $edge) { + $editor->removeEdge($edge['src'], $edge['type'], $edge['dst']); + } + $editor->save(); + } + +} diff --git a/src/applications/system/garbagecollector/PhabricatorSystemDestructionGarbageCollector.php b/src/applications/system/garbagecollector/PhabricatorSystemDestructionGarbageCollector.php new file mode 100644 index 0000000000..ad068cd5c9 --- /dev/null +++ b/src/applications/system/garbagecollector/PhabricatorSystemDestructionGarbageCollector.php @@ -0,0 +1,21 @@ +establishConnection('w'); + + queryfx( + $conn_w, + 'DELETE FROM %T WHERE epoch < %d LIMIT 100', + $table->getTableName(), + time() - $ttl); + + return ($conn_w->getAffectedRows() == 100); + } + +} diff --git a/src/applications/system/interface/PhabricatorDestructableInterface.php b/src/applications/system/interface/PhabricatorDestructableInterface.php new file mode 100644 index 0000000000..6130789f3a --- /dev/null +++ b/src/applications/system/interface/PhabricatorDestructableInterface.php @@ -0,0 +1,24 @@ +nuke();>>> + + } + +*/ diff --git a/src/applications/system/management/PhabricatorSystemRemoveDestroyWorkflow.php b/src/applications/system/management/PhabricatorSystemRemoveDestroyWorkflow.php new file mode 100644 index 0000000000..74b16ced52 --- /dev/null +++ b/src/applications/system/management/PhabricatorSystemRemoveDestroyWorkflow.php @@ -0,0 +1,110 @@ +setName('destroy') + ->setSynopsis(pht('Permanently destroy objects.')) + ->setExamples('**destroy** [__options__] __object__ ...') + ->setArguments( + array( + array( + 'name' => 'force', + 'help' => pht('Destroy objects without prompting.'), + ), + array( + 'name' => 'objects', + 'wildcard' => true, + ), + )); + } + + public function execute(PhutilArgumentParser $args) { + $console = PhutilConsole::getConsole(); + + $object_names = $args->getArg('objects'); + if (!$object_names) { + throw new PhutilArgumentUsageException( + pht('Specify one or more objects to destroy.')); + } + + $object_query = id(new PhabricatorObjectQuery()) + ->setViewer($this->getViewer()) + ->withNames($object_names); + + $object_query->execute(); + + $named_objects = $object_query->getNamedResults(); + foreach ($object_names as $object_name) { + if (empty($named_objects[$object_name])) { + throw new PhutilArgumentUsageException( + pht('No such object "%s" exists!', $object_name)); + } + } + + foreach ($named_objects as $object_name => $object) { + if (!($object instanceof PhabricatorDestructableInterface)) { + throw new PhutilArgumentUsageException( + pht( + 'Object "%s" can not be destroyed (it does not implement %s).', + $object_name, + 'PhabricatorDestructableInterface')); + } + } + + $console->writeOut( + "**%s**\n\n", + pht(' IMPORTANT: OBJECTS WILL BE PERMANENTLY DESTROYED! ')); + + $console->writeOut( + pht( + "There is no way to undo this operation or ever retrieve this data.". + "\n\n". + "These %s object(s) will be **completely destroyed forever**:". + "\n\n", + new PhutilNumber(count($named_objects)))); + + foreach ($named_objects as $object_name => $object) { + $console->writeOut( + " - %s (%s)\n", + $object_name, + get_class($object)); + } + + $force = $args->getArg('force'); + if (!$force) { + $ok = $console->confirm( + pht( + 'Are you absolutely certain you want to destroy these %s object(s)?', + new PhutilNumber(count($named_objects)))); + if (!$ok) { + throw new PhutilArgumentUsageException( + pht('Aborted, your objects are safe.')); + } + } + + $console->writeOut("%s\n", pht('Destroying objects...')); + + foreach ($named_objects as $object_name => $object) { + $console->writeOut( + pht( + "Destroying %s **%s**...\n", + get_class($object), + $object_name)); + + id(new PhabricatorDestructionEngine()) + ->destroyObject($object); + } + + $console->writeOut( + "%s\n", + pht( + 'Permanently destroyed %s object(s).', + new PhutilNumber(count($named_objects)))); + + return 0; + } + +} diff --git a/src/applications/system/management/PhabricatorSystemRemoveLogWorkflow.php b/src/applications/system/management/PhabricatorSystemRemoveLogWorkflow.php new file mode 100644 index 0000000000..768ff30735 --- /dev/null +++ b/src/applications/system/management/PhabricatorSystemRemoveLogWorkflow.php @@ -0,0 +1,30 @@ +setName('log') + ->setSynopsis(pht('Show a log of permanently destroyed objects.')) + ->setExamples('**log**') + ->setArguments(array()); + } + + public function execute(PhutilArgumentParser $args) { + $console = PhutilConsole::getConsole(); + + $table = new PhabricatorSystemDestructionLog(); + foreach (new LiskMigrationIterator($table) as $row) { + $console->writeOut( + "[%s]\t%s\t%s\t%s\n", + phabricator_datetime($row->getEpoch(), $this->getViewer()), + $row->getObjectClass(), + $row->getObjectPHID(), + $row->getObjectMonogram()); + } + + return 0; + } + +} diff --git a/src/applications/system/management/PhabricatorSystemRemoveWorkflow.php b/src/applications/system/management/PhabricatorSystemRemoveWorkflow.php new file mode 100644 index 0000000000..250bdf2f19 --- /dev/null +++ b/src/applications/system/management/PhabricatorSystemRemoveWorkflow.php @@ -0,0 +1,6 @@ + false, + ) + parent::getConfiguration(); + } + +}