Skip to content

Commit 2022a70

Browse files
author
epriestley
committedMay 2, 2014
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
1 parent 1876bef commit 2022a70

14 files changed

+391
-115
lines changed
 

‎bin/remove

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../scripts/setup/manage_remove.php

‎resources/sql/autopatches/20140501.remove.1.dlog.sql

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
CREATE TABLE {$NAMESPACE}_system.system_destructionlog (
2+
id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
3+
objectClass VARCHAR(128) NOT NULL COLLATE utf8_bin,
4+
rootLogID INT UNSIGNED,
5+
objectPHID VARCHAR(64) COLLATE utf8_bin,
6+
objectMonogram VARCHAR(64) COLLATE utf8_bin,
7+
epoch INT UNSIGNED NOT NULL,
8+
KEY `key_epoch` (epoch)
9+
) ENGINE=InnoDB, COLLATE utf8_general_ci;

‎scripts/setup/manage_remove.php

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#!/usr/bin/env php
2+
<?php
3+
4+
$root = dirname(dirname(dirname(__FILE__)));
5+
require_once $root.'/scripts/__init_script__.php';
6+
7+
$args = new PhutilArgumentParser($argv);
8+
$args->setTagline('remove objects');
9+
$args->setSynopsis(<<<EOSYNOPSIS
10+
**remove** __command__ [__options__]
11+
Administrative tool for destroying objects permanently.
12+
13+
EOSYNOPSIS
14+
);
15+
$args->parseStandardArguments();
16+
17+
$workflows = id(new PhutilSymbolLoader())
18+
->setAncestorClass('PhabricatorSystemRemoveWorkflow')
19+
->loadObjects();
20+
$workflows[] = new PhutilHelpArgumentWorkflow();
21+
$args->parseWorkflows($workflows);

‎src/__phutil_library_map__.php

+14
Original file line numberDiff line numberDiff line change
@@ -1471,6 +1471,8 @@
14711471
'PhabricatorDebugController' => 'applications/system/controller/PhabricatorDebugController.php',
14721472
'PhabricatorDefaultFileStorageEngineSelector' => 'applications/files/engineselector/PhabricatorDefaultFileStorageEngineSelector.php',
14731473
'PhabricatorDefaultSearchEngineSelector' => 'applications/search/selector/PhabricatorDefaultSearchEngineSelector.php',
1474+
'PhabricatorDestructableInterface' => 'applications/system/interface/PhabricatorDestructableInterface.php',
1475+
'PhabricatorDestructionEngine' => 'applications/system/engine/PhabricatorDestructionEngine.php',
14741476
'PhabricatorDeveloperConfigOptions' => 'applications/config/option/PhabricatorDeveloperConfigOptions.php',
14751477
'PhabricatorDifferenceEngine' => 'infrastructure/diff/PhabricatorDifferenceEngine.php',
14761478
'PhabricatorDifferentialConfigOptions' => 'applications/differential/config/PhabricatorDifferentialConfigOptions.php',
@@ -2179,6 +2181,11 @@
21792181
'PhabricatorSystemActionLog' => 'applications/system/storage/PhabricatorSystemActionLog.php',
21802182
'PhabricatorSystemActionRateLimitException' => 'applications/system/exception/PhabricatorSystemActionRateLimitException.php',
21812183
'PhabricatorSystemDAO' => 'applications/system/storage/PhabricatorSystemDAO.php',
2184+
'PhabricatorSystemDestructionGarbageCollector' => 'applications/system/garbagecollector/PhabricatorSystemDestructionGarbageCollector.php',
2185+
'PhabricatorSystemDestructionLog' => 'applications/system/storage/PhabricatorSystemDestructionLog.php',
2186+
'PhabricatorSystemRemoveDestroyWorkflow' => 'applications/system/management/PhabricatorSystemRemoveDestroyWorkflow.php',
2187+
'PhabricatorSystemRemoveLogWorkflow' => 'applications/system/management/PhabricatorSystemRemoveLogWorkflow.php',
2188+
'PhabricatorSystemRemoveWorkflow' => 'applications/system/management/PhabricatorSystemRemoveWorkflow.php',
21822189
'PhabricatorTaskmasterDaemon' => 'infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php',
21832190
'PhabricatorTestCase' => 'infrastructure/testing/PhabricatorTestCase.php',
21842191
'PhabricatorTestController' => 'applications/base/controller/__tests__/PhabricatorTestController.php',
@@ -4297,6 +4304,7 @@
42974304
'PhabricatorDebugController' => 'PhabricatorController',
42984305
'PhabricatorDefaultFileStorageEngineSelector' => 'PhabricatorFileStorageEngineSelector',
42994306
'PhabricatorDefaultSearchEngineSelector' => 'PhabricatorSearchEngineSelector',
4307+
'PhabricatorDestructionEngine' => 'Phobject',
43004308
'PhabricatorDeveloperConfigOptions' => 'PhabricatorApplicationConfigOptions',
43014309
'PhabricatorDifferentialConfigOptions' => 'PhabricatorApplicationConfigOptions',
43024310
'PhabricatorDifferentialRevisionTestDataGenerator' => 'PhabricatorTestDataGenerator',
@@ -5103,6 +5111,11 @@
51035111
'PhabricatorSystemActionLog' => 'PhabricatorSystemDAO',
51045112
'PhabricatorSystemActionRateLimitException' => 'Exception',
51055113
'PhabricatorSystemDAO' => 'PhabricatorLiskDAO',
5114+
'PhabricatorSystemDestructionGarbageCollector' => 'PhabricatorGarbageCollector',
5115+
'PhabricatorSystemDestructionLog' => 'PhabricatorSystemDAO',
5116+
'PhabricatorSystemRemoveDestroyWorkflow' => 'PhabricatorSystemRemoveWorkflow',
5117+
'PhabricatorSystemRemoveLogWorkflow' => 'PhabricatorSystemRemoveWorkflow',
5118+
'PhabricatorSystemRemoveWorkflow' => 'PhabricatorManagementWorkflow',
51065119
'PhabricatorTaskmasterDaemon' => 'PhabricatorDaemon',
51075120
'PhabricatorTestCase' => 'ArcanistPhutilTestCase',
51085121
'PhabricatorTestController' => 'PhabricatorController',
@@ -5159,6 +5172,7 @@
51595172
1 => 'PhutilPerson',
51605173
2 => 'PhabricatorPolicyInterface',
51615174
3 => 'PhabricatorCustomFieldInterface',
5175+
4 => 'PhabricatorDestructableInterface',
51625176
),
51635177
'PhabricatorUserBlurbField' => 'PhabricatorUserCustomField',
51645178
'PhabricatorUserConfigOptions' => 'PhabricatorApplicationConfigOptions',

‎src/applications/people/controller/PhabricatorPeopleDeleteController.php

+9-51
Original file line numberDiff line numberDiff line change
@@ -27,36 +27,6 @@ public function processRequest() {
2727
return $this->buildDeleteSelfResponse($profile_uri);
2828
}
2929

30-
$errors = array();
31-
32-
$v_username = '';
33-
$e_username = true;
34-
if ($request->isFormPost()) {
35-
$v_username = $request->getStr('username');
36-
37-
if (!strlen($v_username)) {
38-
$errors[] = pht(
39-
'You must type the username to confirm that you want to delete '.
40-
'this user account.');
41-
$e_username = pht('Required');
42-
} else if ($v_username != $user->getUsername()) {
43-
$errors[] = pht(
44-
'You must type the username correctly to confirm that you want '.
45-
'to delete this user account.');
46-
$e_username = pht('Incorrect');
47-
}
48-
49-
if (!$errors) {
50-
id(new PhabricatorUserEditor())
51-
->setActor($admin)
52-
->deleteUser($user);
53-
54-
$done_uri = $this->getApplicationURI();
55-
56-
return id(new AphrontRedirectResponse())->setURI($done_uri);
57-
}
58-
}
59-
6030
$str1 = pht(
6131
'Be careful when deleting users! This will permanently and '.
6232
'irreversibly destroy this user account.');
@@ -66,46 +36,34 @@ public function processRequest() {
6636
'disable them, not delete them. If you delete them, it will no longer '.
6737
'be possible to (for example) search for objects they created, and you '.
6838
'will lose other information about their history. Disabling them '.
69-
'instead will prevent them from logging in but not destroy any of '.
39+
'instead will prevent them from logging in, but will not destroy any of '.
7040
'their data.');
7141

7242
$str3 = pht(
7343
'It is generally safe to delete newly created users (and test users and '.
7444
'so on), but less safe to delete established users. If possible, '.
7545
'disable them instead.');
7646

47+
$str4 = pht(
48+
'To permanently destroy this user, run this command:');
49+
7750
$form = id(new AphrontFormView())
7851
->setUser($admin)
7952
->appendRemarkupInstructions(
8053
pht(
81-
'To confirm that you want to permanently and irrevocably destroy '.
82-
'this user account, type their username:'))
83-
->appendChild(
84-
id(new AphrontFormStaticControl())
85-
->setLabel(pht('Username'))
86-
->setValue($user->getUsername()))
87-
->appendChild(
88-
id(new AphrontFormTextControl())
89-
->setLabel(pht('Confirm'))
90-
->setValue($v_username)
91-
->setName('username')
92-
->setError($e_username));
93-
94-
if ($errors) {
95-
$errors = id(new AphrontErrorView())->setErrors($errors);
96-
}
54+
" phabricator/ $ ./bin/remove destroy %s\n",
55+
csprintf('%R', '@'.$user->getUsername())));
9756

9857
return $this->newDialog()
9958
->setWidth(AphrontDialogView::WIDTH_FORM)
100-
->setTitle(pht('Really Delete User?'))
59+
->setTitle(pht('Permanently Delete User'))
10160
->setShortTitle(pht('Delete User'))
102-
->appendChild($errors)
10361
->appendParagraph($str1)
10462
->appendParagraph($str2)
10563
->appendParagraph($str3)
64+
->appendParagraph($str4)
10665
->appendChild($form->buildLayoutView())
107-
->addSubmitButton(pht('Delete User'))
108-
->addCancelButton($profile_uri);
66+
->addCancelButton($profile_uri, pht('Close'));
10967
}
11068

11169
private function buildDeleteSelfResponse($profile_uri) {

‎src/applications/people/editor/PhabricatorUserEditor.php

-63
Original file line numberDiff line numberDiff line change
@@ -332,69 +332,6 @@ public function approveUser(PhabricatorUser $user, $approve) {
332332
return $this;
333333
}
334334

335-
/**
336-
* @task role
337-
*/
338-
public function deleteUser(PhabricatorUser $user, $disable) {
339-
$actor = $this->requireActor();
340-
341-
if (!$user->getID()) {
342-
throw new Exception("User has not been created yet!");
343-
}
344-
345-
if ($actor->getPHID() == $user->getPHID()) {
346-
throw new Exception("You can not delete yourself!");
347-
}
348-
349-
$user->openTransaction();
350-
$externals = id(new PhabricatorExternalAccount())->loadAllWhere(
351-
'userPHID = %s',
352-
$user->getPHID());
353-
foreach ($externals as $external) {
354-
$external->delete();
355-
}
356-
357-
$prefs = id(new PhabricatorUserPreferences())->loadAllWhere(
358-
'userPHID = %s',
359-
$user->getPHID());
360-
foreach ($prefs as $pref) {
361-
$pref->delete();
362-
}
363-
364-
$profiles = id(new PhabricatorUserProfile())->loadAllWhere(
365-
'userPHID = %s',
366-
$user->getPHID());
367-
foreach ($profiles as $profile) {
368-
$profile->delete();
369-
}
370-
371-
$keys = id(new PhabricatorUserSSHKey())->loadAllWhere(
372-
'userPHID = %s',
373-
$user->getPHID());
374-
foreach ($keys as $key) {
375-
$key->delete();
376-
}
377-
378-
$emails = id(new PhabricatorUserEmail())->loadAllWhere(
379-
'userPHID = %s',
380-
$user->getPHID());
381-
foreach ($emails as $email) {
382-
$email->delete();
383-
}
384-
385-
$log = PhabricatorUserLog::initializeNewLog(
386-
$actor,
387-
$user->getPHID(),
388-
PhabricatorUserLog::ACTION_DELETE);
389-
$log->save();
390-
391-
$user->delete();
392-
393-
$user->saveTransaction();
394-
395-
return $this;
396-
}
397-
398335

399336
/* -( Adding, Removing and Changing Email )-------------------------------- */
400337

‎src/applications/people/storage/PhabricatorUser.php

+69-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ final class PhabricatorUser
55
implements
66
PhutilPerson,
77
PhabricatorPolicyInterface,
8-
PhabricatorCustomFieldInterface {
8+
PhabricatorCustomFieldInterface,
9+
PhabricatorDestructableInterface {
910

1011
const SESSION_TABLE = 'phabricator_session';
1112
const NAMETOKEN_TABLE = 'user_nametoken';
@@ -139,6 +140,10 @@ public function getSex() {
139140
return $this->sex;
140141
}
141142

143+
public function getMonogram() {
144+
return '@'.$this->getUsername();
145+
}
146+
142147
public function getTranslation() {
143148
try {
144149
if ($this->translation &&
@@ -814,4 +819,67 @@ public function attachCustomFields(PhabricatorCustomFieldAttachment $fields) {
814819
return $this;
815820
}
816821

822+
823+
/* -( PhabricatorDestructableInterface )----------------------------------- */
824+
825+
826+
public function destroyObjectPermanently(
827+
PhabricatorDestructionEngine $engine) {
828+
829+
$this->openTransaction();
830+
$this->delete();
831+
832+
$externals = id(new PhabricatorExternalAccount())->loadAllWhere(
833+
'userPHID = %s',
834+
$this->getPHID());
835+
foreach ($externals as $external) {
836+
$external->delete();
837+
}
838+
839+
$prefs = id(new PhabricatorUserPreferences())->loadAllWhere(
840+
'userPHID = %s',
841+
$this->getPHID());
842+
foreach ($prefs as $pref) {
843+
$pref->delete();
844+
}
845+
846+
$profiles = id(new PhabricatorUserProfile())->loadAllWhere(
847+
'userPHID = %s',
848+
$this->getPHID());
849+
foreach ($profiles as $profile) {
850+
$profile->delete();
851+
}
852+
853+
$keys = id(new PhabricatorUserSSHKey())->loadAllWhere(
854+
'userPHID = %s',
855+
$this->getPHID());
856+
foreach ($keys as $key) {
857+
$key->delete();
858+
}
859+
860+
$emails = id(new PhabricatorUserEmail())->loadAllWhere(
861+
'userPHID = %s',
862+
$this->getPHID());
863+
foreach ($emails as $email) {
864+
$email->delete();
865+
}
866+
867+
$sessions = id(new PhabricatorAuthSession())->loadAllWhere(
868+
'userPHID = %s',
869+
$this->getPHID());
870+
foreach ($sessions as $session) {
871+
$session->delete();
872+
}
873+
874+
$factors = id(new PhabricatorAuthFactorConfig())->loadAllWhere(
875+
'userPHID = %s',
876+
$this->getPHID());
877+
foreach ($factors as $factor) {
878+
$factor->delete();
879+
}
880+
881+
$this->saveTransaction();
882+
}
883+
884+
817885
}

‎src/applications/system/engine/PhabricatorDestructionEngine.php

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
<?php
2+
3+
final class PhabricatorDestructionEngine extends Phobject {
4+
5+
private $rootLogID;
6+
7+
public function destroyObject(PhabricatorDestructableInterface $object) {
8+
$log = id(new PhabricatorSystemDestructionLog())
9+
->setEpoch(time())
10+
->setObjectClass(get_class($object));
11+
12+
if ($this->rootLogID) {
13+
$log->setRootLogID($this->rootLogID);
14+
}
15+
16+
$object_phid = null;
17+
if (method_exists($object, 'getPHID')) {
18+
try {
19+
$object_phid = $object->getPHID();
20+
$log->setObjectPHID($object_phid);
21+
} catch (Exception $ex) {
22+
// Ignore.
23+
}
24+
}
25+
26+
if (method_exists($object, 'getMonogram')) {
27+
try {
28+
$log->setObjectMonogram($object->getMonogram());
29+
} catch (Exception $ex) {
30+
// Ignore.
31+
}
32+
}
33+
34+
$log->save();
35+
36+
if (!$this->rootLogID) {
37+
$this->rootLogID = $log->getID();
38+
}
39+
40+
$object->destroyObjectPermanently($this);
41+
42+
if ($object_phid) {
43+
$this->destroyEdges($object_phid);
44+
}
45+
}
46+
47+
private function destroyEdges($src_phid) {
48+
$edges = id(new PhabricatorEdgeQuery())
49+
->withSourcePHIDs(array($src_phid))
50+
->execute();
51+
52+
$editor = id(new PhabricatorEdgeEditor())
53+
->setSuppressEvents(true);
54+
foreach ($edges as $edge) {
55+
$editor->removeEdge($edge['src'], $edge['type'], $edge['dst']);
56+
}
57+
$editor->save();
58+
}
59+
60+
}

0 commit comments

Comments
 (0)
Failed to load comments.