From fa1ecb7f66121d198e33c5d8d17f01ffd36b9389 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 20 Jan 2018 09:08:30 -0800 Subject: [PATCH] Add a `bin/auth revoke` revoker for SSH keys Summary: Ref T13043. Adds CLI support for revoking SSH keys. Also retargets UI language from "Deactivate" to "Revoke" to make it more clear that this is a one-way operation. This operation is already correctly implemented as a "Revoke" operation. Test Plan: Used `bin/auth revoke --type ssh` to revoke keys, verified they became revoked (with proper transactions) in the UI. Revoked keys from the web UI flow. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13043 Differential Revision: https://secure.phabricator.com/D18893 --- src/__phutil_library_map__.php | 6 ++- .../PhabricatorAuthApplication.php | 4 +- ...PhabricatorAuthSSHKeyRevokeController.php} | 8 +-- .../PhabricatorAuthSSHKeyViewController.php | 8 +-- .../auth/revoker/PhabricatorAuthRevoker.php | 2 +- .../revoker/PhabricatorAuthSSHRevoker.php | 52 +++++++++++++++++++ .../auth/storage/PhabricatorAuthSSHKey.php | 2 +- .../PhabricatorAuthSSHKeyTransaction.php | 4 +- 8 files changed, 70 insertions(+), 16 deletions(-) rename src/applications/auth/controller/{PhabricatorAuthSSHKeyDeactivateController.php => PhabricatorAuthSSHKeyRevokeController.php} (86%) create mode 100644 src/applications/auth/revoker/PhabricatorAuthSSHRevoker.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 3a7785d2e1..7ad2f2673c 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2105,7 +2105,6 @@ phutil_register_library_map(array( 'PhabricatorAuthRevoker' => 'applications/auth/revoker/PhabricatorAuthRevoker.php', 'PhabricatorAuthSSHKey' => 'applications/auth/storage/PhabricatorAuthSSHKey.php', 'PhabricatorAuthSSHKeyController' => 'applications/auth/controller/PhabricatorAuthSSHKeyController.php', - 'PhabricatorAuthSSHKeyDeactivateController' => 'applications/auth/controller/PhabricatorAuthSSHKeyDeactivateController.php', 'PhabricatorAuthSSHKeyEditController' => 'applications/auth/controller/PhabricatorAuthSSHKeyEditController.php', 'PhabricatorAuthSSHKeyEditor' => 'applications/auth/editor/PhabricatorAuthSSHKeyEditor.php', 'PhabricatorAuthSSHKeyGenerateController' => 'applications/auth/controller/PhabricatorAuthSSHKeyGenerateController.php', @@ -2113,12 +2112,14 @@ phutil_register_library_map(array( 'PhabricatorAuthSSHKeyPHIDType' => 'applications/auth/phid/PhabricatorAuthSSHKeyPHIDType.php', 'PhabricatorAuthSSHKeyQuery' => 'applications/auth/query/PhabricatorAuthSSHKeyQuery.php', 'PhabricatorAuthSSHKeyReplyHandler' => 'applications/auth/mail/PhabricatorAuthSSHKeyReplyHandler.php', + 'PhabricatorAuthSSHKeyRevokeController' => 'applications/auth/controller/PhabricatorAuthSSHKeyRevokeController.php', 'PhabricatorAuthSSHKeySearchEngine' => 'applications/auth/query/PhabricatorAuthSSHKeySearchEngine.php', 'PhabricatorAuthSSHKeyTableView' => 'applications/auth/view/PhabricatorAuthSSHKeyTableView.php', 'PhabricatorAuthSSHKeyTransaction' => 'applications/auth/storage/PhabricatorAuthSSHKeyTransaction.php', 'PhabricatorAuthSSHKeyTransactionQuery' => 'applications/auth/query/PhabricatorAuthSSHKeyTransactionQuery.php', 'PhabricatorAuthSSHKeyViewController' => 'applications/auth/controller/PhabricatorAuthSSHKeyViewController.php', 'PhabricatorAuthSSHPublicKey' => 'applications/auth/sshkey/PhabricatorAuthSSHPublicKey.php', + 'PhabricatorAuthSSHRevoker' => 'applications/auth/revoker/PhabricatorAuthSSHRevoker.php', 'PhabricatorAuthSession' => 'applications/auth/storage/PhabricatorAuthSession.php', 'PhabricatorAuthSessionEngine' => 'applications/auth/engine/PhabricatorAuthSessionEngine.php', 'PhabricatorAuthSessionEngineExtension' => 'applications/auth/engine/PhabricatorAuthSessionEngineExtension.php', @@ -7390,7 +7391,6 @@ phutil_register_library_map(array( 'PhabricatorApplicationTransactionInterface', ), 'PhabricatorAuthSSHKeyController' => 'PhabricatorAuthController', - 'PhabricatorAuthSSHKeyDeactivateController' => 'PhabricatorAuthSSHKeyController', 'PhabricatorAuthSSHKeyEditController' => 'PhabricatorAuthSSHKeyController', 'PhabricatorAuthSSHKeyEditor' => 'PhabricatorApplicationTransactionEditor', 'PhabricatorAuthSSHKeyGenerateController' => 'PhabricatorAuthSSHKeyController', @@ -7398,12 +7398,14 @@ phutil_register_library_map(array( 'PhabricatorAuthSSHKeyPHIDType' => 'PhabricatorPHIDType', 'PhabricatorAuthSSHKeyQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorAuthSSHKeyReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler', + 'PhabricatorAuthSSHKeyRevokeController' => 'PhabricatorAuthSSHKeyController', 'PhabricatorAuthSSHKeySearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorAuthSSHKeyTableView' => 'AphrontView', 'PhabricatorAuthSSHKeyTransaction' => 'PhabricatorApplicationTransaction', 'PhabricatorAuthSSHKeyTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'PhabricatorAuthSSHKeyViewController' => 'PhabricatorAuthSSHKeyController', 'PhabricatorAuthSSHPublicKey' => 'Phobject', + 'PhabricatorAuthSSHRevoker' => 'PhabricatorAuthRevoker', 'PhabricatorAuthSession' => array( 'PhabricatorAuthDAO', 'PhabricatorPolicyInterface', diff --git a/src/applications/auth/application/PhabricatorAuthApplication.php b/src/applications/auth/application/PhabricatorAuthApplication.php index a6127394fd..ff4ed1f136 100644 --- a/src/applications/auth/application/PhabricatorAuthApplication.php +++ b/src/applications/auth/application/PhabricatorAuthApplication.php @@ -80,8 +80,8 @@ final class PhabricatorAuthApplication extends PhabricatorApplication { 'generate/' => 'PhabricatorAuthSSHKeyGenerateController', 'upload/' => 'PhabricatorAuthSSHKeyEditController', 'edit/(?P\d+)/' => 'PhabricatorAuthSSHKeyEditController', - 'deactivate/(?P\d+)/' - => 'PhabricatorAuthSSHKeyDeactivateController', + 'revoke/(?P\d+)/' + => 'PhabricatorAuthSSHKeyRevokeController', 'view/(?P\d+)/' => 'PhabricatorAuthSSHKeyViewController', ), 'password/' => 'PhabricatorAuthSetPasswordController', diff --git a/src/applications/auth/controller/PhabricatorAuthSSHKeyDeactivateController.php b/src/applications/auth/controller/PhabricatorAuthSSHKeyRevokeController.php similarity index 86% rename from src/applications/auth/controller/PhabricatorAuthSSHKeyDeactivateController.php rename to src/applications/auth/controller/PhabricatorAuthSSHKeyRevokeController.php index 8eca02340d..0547c2175d 100644 --- a/src/applications/auth/controller/PhabricatorAuthSSHKeyDeactivateController.php +++ b/src/applications/auth/controller/PhabricatorAuthSSHKeyRevokeController.php @@ -1,6 +1,6 @@ getName()); return $this->newDialog() - ->setTitle(pht('Deactivate SSH Public Key')) + ->setTitle(pht('Revoke SSH Public Key')) ->appendParagraph( pht( - 'The key "%s" will be permanently deactivated, and you will no '. + 'The key "%s" will be permanently revoked, and you will no '. 'longer be able to use the corresponding private key to '. 'authenticate.', $name)) - ->addSubmitButton(pht('Deactivate Public Key')) + ->addSubmitButton(pht('Revoke Public Key')) ->addCancelButton($cancel_uri); } diff --git a/src/applications/auth/controller/PhabricatorAuthSSHKeyViewController.php b/src/applications/auth/controller/PhabricatorAuthSSHKeyViewController.php index 66c3a5f09b..54c6e3861b 100644 --- a/src/applications/auth/controller/PhabricatorAuthSSHKeyViewController.php +++ b/src/applications/auth/controller/PhabricatorAuthSSHKeyViewController.php @@ -35,7 +35,7 @@ final class PhabricatorAuthSSHKeyViewController if ($ssh_key->getIsActive()) { $header->setStatus('fa-check', 'bluegrey', pht('Active')); } else { - $header->setStatus('fa-ban', 'dark', pht('Deactivated')); + $header->setStatus('fa-ban', 'dark', pht('Revoked')); } $header->addActionLink( @@ -80,7 +80,7 @@ final class PhabricatorAuthSSHKeyViewController $id = $ssh_key->getID(); $edit_uri = $this->getApplicationURI("sshkey/edit/{$id}/"); - $deactivate_uri = $this->getApplicationURI("sshkey/deactivate/{$id}/"); + $revoke_uri = $this->getApplicationURI("sshkey/revoke/{$id}/"); $curtain = $this->newCurtainView($ssh_key); @@ -95,8 +95,8 @@ final class PhabricatorAuthSSHKeyViewController $curtain->addAction( id(new PhabricatorActionView()) ->setIcon('fa-times') - ->setName(pht('Deactivate SSH Key')) - ->setHref($deactivate_uri) + ->setName(pht('Revoke SSH Key')) + ->setHref($revoke_uri) ->setWorkflow(true) ->setDisabled(!$can_edit)); diff --git a/src/applications/auth/revoker/PhabricatorAuthRevoker.php b/src/applications/auth/revoker/PhabricatorAuthRevoker.php index f4467036b9..3a807e3089 100644 --- a/src/applications/auth/revoker/PhabricatorAuthRevoker.php +++ b/src/applications/auth/revoker/PhabricatorAuthRevoker.php @@ -5,7 +5,7 @@ abstract class PhabricatorAuthRevoker private $viewer; - abstract public function revokeAlLCredentials(); + abstract public function revokeAllCredentials(); abstract public function revokeCredentialsFrom($object); public function setViewer(PhabricatorUser $viewer) { diff --git a/src/applications/auth/revoker/PhabricatorAuthSSHRevoker.php b/src/applications/auth/revoker/PhabricatorAuthSSHRevoker.php new file mode 100644 index 0000000000..16cc8eaea4 --- /dev/null +++ b/src/applications/auth/revoker/PhabricatorAuthSSHRevoker.php @@ -0,0 +1,52 @@ +revokeWithQuery($query); + } + + public function revokeCredentialsFrom($object) { + $query = id(new PhabricatorAuthSSHKeyQuery()) + ->withObjectPHIDs(array($object->getPHID())); + + return $this->revokeWithQuery($query); + } + + private function revokeWithQuery(PhabricatorAuthSSHKeyQuery $query) { + $viewer = $this->getViewer(); + + // We're only going to revoke keys which have not already been revoked. + + $ssh_keys = $query + ->setViewer($viewer) + ->withIsActive(true) + ->execute(); + + $content_source = PhabricatorContentSource::newForSource( + PhabricatorDaemonContentSource::SOURCECONST); + + $auth_phid = id(new PhabricatorAuthApplication())->getPHID(); + foreach ($ssh_keys as $ssh_key) { + $xactions = array(); + $xactions[] = $ssh_key->getApplicationTransactionTemplate() + ->setTransactionType(PhabricatorAuthSSHKeyTransaction::TYPE_DEACTIVATE) + ->setNewValue(1); + + $editor = id(new PhabricatorAuthSSHKeyEditor()) + ->setActor($viewer) + ->setActingAsPHID($auth_phid) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true) + ->setContentSource($content_source) + ->applyTransactions($ssh_key, $xactions); + } + + return count($ssh_keys); + } + +} diff --git a/src/applications/auth/storage/PhabricatorAuthSSHKey.php b/src/applications/auth/storage/PhabricatorAuthSSHKey.php index 7cd23d297e..2a9a6273bf 100644 --- a/src/applications/auth/storage/PhabricatorAuthSSHKey.php +++ b/src/applications/auth/storage/PhabricatorAuthSSHKey.php @@ -139,7 +139,7 @@ final class PhabricatorAuthSSHKey public function describeAutomaticCapability($capability) { if (!$this->getIsACtive()) { return pht( - 'Deactivated SSH keys can not be edited or reactivated.'); + 'Revoked SSH keys can not be edited or reinstated.'); } return pht( diff --git a/src/applications/auth/storage/PhabricatorAuthSSHKeyTransaction.php b/src/applications/auth/storage/PhabricatorAuthSSHKeyTransaction.php index 37edb7384d..bb08310cf3 100644 --- a/src/applications/auth/storage/PhabricatorAuthSSHKeyTransaction.php +++ b/src/applications/auth/storage/PhabricatorAuthSSHKeyTransaction.php @@ -43,11 +43,11 @@ final class PhabricatorAuthSSHKeyTransaction case self::TYPE_DEACTIVATE: if ($new) { return pht( - '%s deactivated this key.', + '%s revoked this key.', $this->renderHandleLink($author_phid)); } else { return pht( - '%s activated this key.', + '%s reinstated this key.', $this->renderHandleLink($author_phid)); }