From d7b7b1933793cf49f5ccc0e20a3717d6d1535904 Mon Sep 17 00:00:00 2001 From: lkassianik Date: Fri, 2 May 2014 18:05:19 -0700 Subject: [PATCH] Add a "Lock Permanently" action to Passphrase Summary: Fixes T4931. Each new credential should come with the ability to lock the credential permanently, so that no one can ever edit again. Each existing credential must allow user to lock existing credential. Test Plan: Create new credential, verify that you can lock it before saving it. Open existing unlocked credential, verify that option to lock it exists. Once credential is locked, the option to reveal it should be disabled, and editing the credential won't allow username/password updates. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley, Korvin Maniphest Tasks: T4931 Differential Revision: https://secure.phabricator.com/D8947 --- .../20140501.passphraselockcredential.sql | 2 + src/__phutil_library_map__.php | 2 + .../PhabricatorApplicationPassphrase.php | 1 + .../PassphraseCredentialDestroyController.php | 8 +- .../PassphraseCredentialEditController.php | 62 ++++++++++++---- .../PassphraseCredentialLockController.php | 74 +++++++++++++++++++ .../PassphraseCredentialRevealController.php | 23 ++++-- .../PassphraseCredentialViewController.php | 22 +++++- .../PassphraseCredentialTransactionEditor.php | 13 +++- .../storage/PassphraseCredential.php | 1 + .../PassphraseCredentialTransaction.php | 7 ++ 11 files changed, 183 insertions(+), 32 deletions(-) create mode 100644 resources/sql/autopatches/20140501.passphraselockcredential.sql create mode 100644 src/applications/passphrase/controller/PassphraseCredentialLockController.php diff --git a/resources/sql/autopatches/20140501.passphraselockcredential.sql b/resources/sql/autopatches/20140501.passphraselockcredential.sql new file mode 100644 index 0000000000..c398afc1fe --- /dev/null +++ b/resources/sql/autopatches/20140501.passphraselockcredential.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_passphrase.passphrase_credential + ADD COLUMN isLocked BOOL NOT NULL; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index ea567da921..8ff99b40e6 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1046,6 +1046,7 @@ phutil_register_library_map(array( 'PassphraseCredentialDestroyController' => 'applications/passphrase/controller/PassphraseCredentialDestroyController.php', 'PassphraseCredentialEditController' => 'applications/passphrase/controller/PassphraseCredentialEditController.php', 'PassphraseCredentialListController' => 'applications/passphrase/controller/PassphraseCredentialListController.php', + 'PassphraseCredentialLockController' => 'applications/passphrase/controller/PassphraseCredentialLockController.php', 'PassphraseCredentialPublicController' => 'applications/passphrase/controller/PassphraseCredentialPublicController.php', 'PassphraseCredentialQuery' => 'applications/passphrase/query/PassphraseCredentialQuery.php', 'PassphraseCredentialRevealController' => 'applications/passphrase/controller/PassphraseCredentialRevealController.php', @@ -3806,6 +3807,7 @@ phutil_register_library_map(array( 0 => 'PassphraseController', 1 => 'PhabricatorApplicationSearchResultsControllerInterface', ), + 'PassphraseCredentialLockController' => 'PassphraseController', 'PassphraseCredentialPublicController' => 'PassphraseController', 'PassphraseCredentialQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PassphraseCredentialRevealController' => 'PassphraseController', diff --git a/src/applications/passphrase/application/PhabricatorApplicationPassphrase.php b/src/applications/passphrase/application/PhabricatorApplicationPassphrase.php index f62314b6a7..2632106044 100644 --- a/src/applications/passphrase/application/PhabricatorApplicationPassphrase.php +++ b/src/applications/passphrase/application/PhabricatorApplicationPassphrase.php @@ -41,6 +41,7 @@ final class PhabricatorApplicationPassphrase extends PhabricatorApplication { 'destroy/(?P\d+)/' => 'PassphraseCredentialDestroyController', 'reveal/(?P\d+)/' => 'PassphraseCredentialRevealController', 'public/(?P\d+)/' => 'PassphraseCredentialPublicController', + 'lock/(?P\d+)/' => 'PassphraseCredentialLockController', )); } diff --git a/src/applications/passphrase/controller/PassphraseCredentialDestroyController.php b/src/applications/passphrase/controller/PassphraseCredentialDestroyController.php index 30c744e96f..53f9e0650f 100644 --- a/src/applications/passphrase/controller/PassphraseCredentialDestroyController.php +++ b/src/applications/passphrase/controller/PassphraseCredentialDestroyController.php @@ -50,18 +50,16 @@ final class PassphraseCredentialDestroyController return id(new AphrontRedirectResponse())->setURI($view_uri); } - $dialog = id(new AphrontDialogView()) + return $this->newDialog() ->setUser($viewer) ->setTitle(pht('Really destroy credential?')) ->appendChild( pht( 'This credential will be deactivated and the secret will be '. - 'unrecoverably destroyed. Anything relying on this credential will '. - 'cease to function. This operation can not be undone.')) + 'unrecoverably destroyed. Anything relying on this credential '. + 'will cease to function. This operation can not be undone.')) ->addSubmitButton(pht('Destroy Credential')) ->addCancelButton($view_uri); - - return id(new AphrontDialogResponse())->setDialog($dialog); } } diff --git a/src/applications/passphrase/controller/PassphraseCredentialEditController.php b/src/applications/passphrase/controller/PassphraseCredentialEditController.php index 3f241b3d6a..92b62b8062 100644 --- a/src/applications/passphrase/controller/PassphraseCredentialEditController.php +++ b/src/applications/passphrase/controller/PassphraseCredentialEditController.php @@ -93,6 +93,7 @@ final class PassphraseCredentialEditController extends PassphraseController { $v_username = $request->getStr('username'); $v_view_policy = $request->getStr('viewPolicy'); $v_edit_policy = $request->getStr('editPolicy'); + $v_is_locked = $request->getStr('lock'); $v_secret = $request->getStr('secret'); $v_password = $request->getStr('password'); @@ -126,6 +127,7 @@ final class PassphraseCredentialEditController extends PassphraseController { $type_username = PassphraseCredentialTransaction::TYPE_USERNAME; $type_destroy = PassphraseCredentialTransaction::TYPE_DESTROY; $type_secret_id = PassphraseCredentialTransaction::TYPE_SECRET_ID; + $type_is_locked = PassphraseCredentialTransaction::TYPE_LOCK; $type_view_policy = PhabricatorTransactions::TYPE_VIEW_POLICY; $type_edit_policy = PhabricatorTransactions::TYPE_EDIT_POLICY; @@ -139,10 +141,6 @@ final class PassphraseCredentialEditController extends PassphraseController { ->setTransactionType($type_desc) ->setNewValue($v_desc); - $xactions[] = id(new PassphraseCredentialTransaction()) - ->setTransactionType($type_username) - ->setNewValue($v_username); - $xactions[] = id(new PassphraseCredentialTransaction()) ->setTransactionType($type_view_policy) ->setNewValue($v_view_policy); @@ -155,20 +153,30 @@ final class PassphraseCredentialEditController extends PassphraseController { // the amount of code which handles secret plaintexts. $credential->openTransaction(); - $min_secret = str_replace($bullet, '', trim($v_decrypt)); - if (strlen($min_secret)) { - // If the credential was previously destroyed, restore it when it is - // edited if a secret is provided. + if (!$credential->getIsLocked()) { $xactions[] = id(new PassphraseCredentialTransaction()) - ->setTransactionType($type_destroy) - ->setNewValue(0); + ->setTransactionType($type_username) + ->setNewValue($v_username); + + $min_secret = str_replace($bullet, '', trim($v_decrypt)); + if (strlen($min_secret)) { + // If the credential was previously destroyed, restore it when it is + // edited if a secret is provided. + $xactions[] = id(new PassphraseCredentialTransaction()) + ->setTransactionType($type_destroy) + ->setNewValue(0); + + $new_secret = id(new PassphraseSecret()) + ->setSecretData($v_decrypt) + ->save(); + $xactions[] = id(new PassphraseCredentialTransaction()) + ->setTransactionType($type_secret_id) + ->setNewValue($new_secret->getID()); + } - $new_secret = id(new PassphraseSecret()) - ->setSecretData($v_decrypt) - ->save(); $xactions[] = id(new PassphraseCredentialTransaction()) - ->setTransactionType($type_secret_id) - ->setNewValue($new_secret->getID()); + ->setTransactionType($type_is_locked) + ->setNewValue($v_is_locked); } try { @@ -210,6 +218,7 @@ final class PassphraseCredentialEditController extends PassphraseController { ->execute(); $secret_control = $type->newSecretControl(); + $credential_is_locked = $credential->getIsLocked(); $form = id(new AphrontFormView()) ->setUser($viewer) @@ -245,17 +254,26 @@ final class PassphraseCredentialEditController extends PassphraseController { ->setCapability(PhabricatorPolicyCapability::CAN_EDIT) ->setPolicies($policies)) ->appendChild( - id(new AphrontFormDividerControl())) + id(new AphrontFormDividerControl())); + + if ($credential_is_locked) { + $form->appendRemarkupInstructions( + pht('This credential is permanently locked and can not be edited.')); + } + + $form ->appendChild( id(new AphrontFormTextControl()) ->setName('username') ->setLabel(pht('Login/Username')) ->setValue($v_username) + ->setDisabled($credential_is_locked) ->setError($e_username)) ->appendChild( $secret_control ->setName('secret') ->setLabel($type->getSecretLabel()) + ->setDisabled($credential_is_locked) ->setValue($v_secret)); if ($type->shouldShowPasswordField()) { @@ -263,9 +281,21 @@ final class PassphraseCredentialEditController extends PassphraseController { id(new AphrontFormPasswordControl()) ->setName('password') ->setLabel($type->getPasswordLabel()) + ->setDisabled($credential_is_locked) ->setError($e_password)); } + if ($is_new) { + $form->appendChild( + id(new AphrontFormCheckboxControl()) + ->addCheckbox( + 'lock', + 1, + pht('Lock Permanently'), + $v_is_locked) + ->setDisabled($credential_is_locked)); + } + $crumbs = $this->buildApplicationCrumbs(); if ($is_new) { diff --git a/src/applications/passphrase/controller/PassphraseCredentialLockController.php b/src/applications/passphrase/controller/PassphraseCredentialLockController.php new file mode 100644 index 0000000000..3abe4c8820 --- /dev/null +++ b/src/applications/passphrase/controller/PassphraseCredentialLockController.php @@ -0,0 +1,74 @@ +id = $data['id']; + } + + public function processRequest() { + $request = $this->getRequest(); + $viewer = $request->getUser(); + + $credential = id(new PassphraseCredentialQuery()) + ->setViewer($viewer) + ->withIDs(array($this->id)) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->executeOne(); + if (!$credential) { + return new Aphront404Response(); + } + + $type = PassphraseCredentialType::getTypeByConstant( + $credential->getCredentialType()); + if (!$type) { + throw new Exception(pht('Credential has invalid type "%s"!', $type)); + } + + $view_uri = '/K'.$credential->getID(); + + if ($credential->getIsLocked()) { + return $this->newDialog() + ->setTitle(pht('Credential Already Locked')) + ->appendChild( + pht( + 'This credential has been locked and the secret is '. + 'hidden forever. Anything relying on this credential will '. + 'still function. This operation can not be undone.')) + ->addCancelButton($view_uri, pht('Close')); + } + + if ($request->isFormPost()) { + $xactions = array(); + $xactions[] = id(new PassphraseCredentialTransaction()) + ->setTransactionType(PassphraseCredentialTransaction::TYPE_LOCK) + ->setNewValue(1); + + $editor = id(new PassphraseCredentialTransactionEditor()) + ->setActor($viewer) + ->setContinueOnMissingFields(true) + ->setContentSourceFromRequest($request) + ->applyTransactions($credential, $xactions); + + return id(new AphrontRedirectResponse())->setURI($view_uri); + } + + return $this->newDialog() + ->setTitle(pht('Really lock credential?')) + ->appendChild( + pht( + 'This credential will be locked and the secret will be '. + 'hidden forever. Anything relying on this credential will '. + 'still function. This operation can not be undone.')) + ->addSubmitButton(pht('Lock Credential')) + ->addCancelButton($view_uri); + } + +} diff --git a/src/applications/passphrase/controller/PassphraseCredentialRevealController.php b/src/applications/passphrase/controller/PassphraseCredentialRevealController.php index d1357c44c2..55489481c7 100644 --- a/src/applications/passphrase/controller/PassphraseCredentialRevealController.php +++ b/src/applications/passphrase/controller/PassphraseCredentialRevealController.php @@ -33,6 +33,17 @@ final class PassphraseCredentialRevealController $viewer, $request, $view_uri); + $is_locked = $credential->getIsLocked(); + + if ($is_locked) { + return $this->newDialog() + ->setUser($viewer) + ->setTitle(pht('Credential is locked')) + ->appendChild( + pht( + 'This credential can not be shown, because it is locked.')) + ->addCancelButton($view_uri); + } if ($request->isFormPost()) { if ($credential->getSecret()) { @@ -73,6 +84,7 @@ final class PassphraseCredentialRevealController } $is_serious = PhabricatorEnv::getEnvConfig('phabricator.serious-business'); + if ($is_serious) { $body = pht( 'The secret associated with this credential will be shown in plain '. @@ -80,19 +92,16 @@ final class PassphraseCredentialRevealController } else { $body = pht( 'The secret associated with this credential will be shown in plain '. - 'text on your screen. Before continuing, wrap your arms around your '. - 'monitor to create a human shield, keeping it safe from prying eyes. '. - 'Protect company secrets!'); + 'text on your screen. Before continuing, wrap your arms around '. + 'your monitor to create a human shield, keeping it safe from '. + 'prying eyes. Protect company secrets!'); } - - $dialog = id(new AphrontDialogView()) + return $this->newDialog() ->setUser($viewer) ->setTitle(pht('Really show secret?')) ->appendChild($body) ->addSubmitButton(pht('Show Secret')) ->addCancelButton($view_uri); - - return id(new AphrontDialogResponse())->setDialog($dialog); } } diff --git a/src/applications/passphrase/controller/PassphraseCredentialViewController.php b/src/applications/passphrase/controller/PassphraseCredentialViewController.php index fcd87c83df..91bcb847a6 100644 --- a/src/applications/passphrase/controller/PassphraseCredentialViewController.php +++ b/src/applications/passphrase/controller/PassphraseCredentialViewController.php @@ -85,6 +85,15 @@ final class PassphraseCredentialViewController extends PassphraseController { $id = $credential->getID(); + $is_locked = $credential->getIsLocked(); + if ($is_locked) { + $credential_lock_text = pht('Locked Permanently'); + $credential_lock_icon = 'lock'; + } else { + $credential_lock_text = pht('Lock Permanently'); + $credential_lock_icon = 'unlock'; + } + $actions = id(new PhabricatorActionListView()) ->setObjectURI('/K'.$id) ->setUser($viewer); @@ -116,7 +125,7 @@ final class PassphraseCredentialViewController extends PassphraseController { ->setName(pht('Show Secret')) ->setIcon('preview') ->setHref($this->getApplicationURI("reveal/{$id}/")) - ->setDisabled(!$can_edit) + ->setDisabled(!$can_edit || $is_locked) ->setWorkflow(true)); if ($type->hasPublicKey()) { @@ -125,8 +134,17 @@ final class PassphraseCredentialViewController extends PassphraseController { ->setName(pht('Show Public Key')) ->setIcon('download-alt') ->setHref($this->getApplicationURI("public/{$id}/")) - ->setWorkflow(true)); + ->setWorkflow(true) + ->setDisabled($is_locked)); } + + $actions->addAction( + id(new PhabricatorActionView()) + ->setName($credential_lock_text) + ->setIcon($credential_lock_icon) + ->setHref($this->getApplicationURI("lock/{$id}/")) + ->setDisabled($is_locked) + ->setWorkflow(true)); } diff --git a/src/applications/passphrase/editor/PassphraseCredentialTransactionEditor.php b/src/applications/passphrase/editor/PassphraseCredentialTransactionEditor.php index 77fae20c5e..d188183a51 100644 --- a/src/applications/passphrase/editor/PassphraseCredentialTransactionEditor.php +++ b/src/applications/passphrase/editor/PassphraseCredentialTransactionEditor.php @@ -15,6 +15,7 @@ final class PassphraseCredentialTransactionEditor $types[] = PassphraseCredentialTransaction::TYPE_SECRET_ID; $types[] = PassphraseCredentialTransaction::TYPE_DESTROY; $types[] = PassphraseCredentialTransaction::TYPE_LOOKEDATSECRET; + $types[] = PassphraseCredentialTransaction::TYPE_LOCK; return $types; } @@ -35,7 +36,9 @@ final class PassphraseCredentialTransactionEditor case PassphraseCredentialTransaction::TYPE_SECRET_ID: return $object->getSecretID(); case PassphraseCredentialTransaction::TYPE_DESTROY: - return $object->getIsDestroyed(); + return (int)$object->getIsDestroyed(); + case PassphraseCredentialTransaction::TYPE_LOCK: + return (int)$object->getIsLocked(); case PassphraseCredentialTransaction::TYPE_LOOKEDATSECRET: return null; } @@ -51,9 +54,11 @@ final class PassphraseCredentialTransactionEditor case PassphraseCredentialTransaction::TYPE_DESCRIPTION: case PassphraseCredentialTransaction::TYPE_USERNAME: case PassphraseCredentialTransaction::TYPE_SECRET_ID: - case PassphraseCredentialTransaction::TYPE_DESTROY: case PassphraseCredentialTransaction::TYPE_LOOKEDATSECRET: return $xaction->getNewValue(); + case PassphraseCredentialTransaction::TYPE_DESTROY: + case PassphraseCredentialTransaction::TYPE_LOCK: + return (int)$xaction->getNewValue(); } return parent::getCustomTransactionNewValue($object, $xaction); } @@ -98,6 +103,9 @@ final class PassphraseCredentialTransactionEditor return; case PassphraseCredentialTransaction::TYPE_LOOKEDATSECRET: return; + case PassphraseCredentialTransaction::TYPE_LOCK: + $object->setIsLocked((int)$xaction->getNewValue()); + return; } return parent::applyCustomInternalTransaction($object, $xaction); @@ -114,6 +122,7 @@ final class PassphraseCredentialTransactionEditor case PassphraseCredentialTransaction::TYPE_SECRET_ID: case PassphraseCredentialTransaction::TYPE_DESTROY: case PassphraseCredentialTransaction::TYPE_LOOKEDATSECRET: + case PassphraseCredentialTransaction::TYPE_LOCK: case PhabricatorTransactions::TYPE_VIEW_POLICY: case PhabricatorTransactions::TYPE_EDIT_POLICY: return; diff --git a/src/applications/passphrase/storage/PassphraseCredential.php b/src/applications/passphrase/storage/PassphraseCredential.php index a4a9de5a9b..574fb8364e 100644 --- a/src/applications/passphrase/storage/PassphraseCredential.php +++ b/src/applications/passphrase/storage/PassphraseCredential.php @@ -12,6 +12,7 @@ final class PassphraseCredential extends PassphraseDAO protected $username; protected $secretID; protected $isDestroyed; + protected $isLocked = 0; private $secret = self::ATTACHABLE; diff --git a/src/applications/passphrase/storage/PassphraseCredentialTransaction.php b/src/applications/passphrase/storage/PassphraseCredentialTransaction.php index 6caed5af3c..fb1aced068 100644 --- a/src/applications/passphrase/storage/PassphraseCredentialTransaction.php +++ b/src/applications/passphrase/storage/PassphraseCredentialTransaction.php @@ -9,6 +9,7 @@ final class PassphraseCredentialTransaction const TYPE_SECRET_ID = 'passphrase:secretID'; const TYPE_DESTROY = 'passphrase:destroy'; const TYPE_LOOKEDATSECRET = 'passphrase:lookedAtSecret'; + const TYPE_LOCK = 'passphrase:lock'; public function getApplicationName() { return 'passphrase'; @@ -27,6 +28,8 @@ final class PassphraseCredentialTransaction switch ($this->getTransactionType()) { case self::TYPE_DESCRIPTION: return ($old === null); + case self::TYPE_LOCK: + return ($old === null); case self::TYPE_USERNAME: return !strlen($old); case self::TYPE_LOOKEDATSECRET: @@ -84,6 +87,10 @@ final class PassphraseCredentialTransaction return pht( '%s examined the secret plaintext for this credential.', $this->renderHandleLink($author_phid)); + case self::TYPE_LOCK: + return pht( + '%s locked this credential.', + $this->renderHandleLink($author_phid)); } return parent::getTitle();