Prevent locked credentials from being made accessible via conduit

Summary:
Via HackerOne. Currently, you can use "Lock Permanently" to lock a credential permanently, but you can still enable Conduit API access to it. This directly contradicts both intent of the setting and its description as presented to the user.

Instead:

  - When a credential is locked, revoke Conduit API access.
  - Prevent API access from being enabled for locked credentials.
  - Prevent API access to locked credentials, period.

Test Plan:
  - Created a credential.
  - Enabled API access.
  - Locked credential.
  - Saw API access become disabled.
  - Tried to enable API access; was rebuffed.
  - Queried credential via API, wasn't granted access.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D15944

src/applications/passphrase/conduit/PassphraseQueryConduitAPIMethod.php

@@ -63,9 +63,12 @@ final class PassphraseQueryConduitAPIMethod
 
       $material = array();
 
+      $is_locked = $credential->getIsLocked();
+      $allow_api = ($credential->getAllowConduit() && !$is_locked);
+
       $secret = null;
       if ($request->getValue('needSecrets')) {
-        if ($credential->getAllowConduit()) {
+        if ($allow_api) {
           $secret = $credential->getSecret();
           if ($secret) {
             $secret = $secret->openEnvelope();
@@ -102,7 +105,7 @@ final class PassphraseQueryConduitAPIMethod
           break;
       }
 
-      if (!$credential->getAllowConduit()) {
+      if (!$allow_api) {
         $material['noAPIAccess'] = pht(
           'This private material for this credential is not accessible via '.
           'API calls.');

src/applications/passphrase/controller/PassphraseCredentialConduitController.php

@@ -33,8 +33,22 @@ final class PassphraseCredentialConduitController
       throw new Exception(pht('Credential has invalid type "%s"!', $type));
     }
 
+    $is_locked = $credential->getIsLocked();
+
+    if ($is_locked) {
+      return $this->newDialog()
+        ->setUser($viewer)
+        ->setTitle(pht('Credential Locked'))
+        ->appendChild(
+          pht(
+            'This credential can not be made available via Conduit because '.
+            'it is locked.'))
+        ->addCancelButton($view_uri);
+    }
+
     if ($request->isFormPost()) {
       $xactions = array();
+
       $xactions[] = id(new PassphraseCredentialTransaction())
         ->setTransactionType(PassphraseCredentialTransaction::TYPE_CONDUIT)
         ->setNewValue(!$credential->getAllowConduit());

src/applications/passphrase/controller/PassphraseCredentialEditController.php

@@ -270,8 +270,7 @@ final class PassphraseCredentialEditController extends PassphraseController {
     }
 
     if ($type->shouldRequireUsername()) {
-      $form
+      $form->appendChild(
-      ->appendChild(
         id(new AphrontFormTextControl())
           ->setName('username')
           ->setLabel(pht('Login/Username'))
@@ -279,8 +278,8 @@ final class PassphraseCredentialEditController extends PassphraseController {
           ->setDisabled($credential_is_locked)
           ->setError($e_username));
     }
-       $form
+
-       ->appendChild(
+    $form->appendChild(
       $secret_control
         ->setName('secret')
         ->setLabel($type->getSecretLabel())

src/applications/passphrase/controller/PassphraseCredentialLockController.php

@@ -32,15 +32,17 @@ final class PassphraseCredentialLockController
       return $this->newDialog()
         ->setTitle(pht('Credential Already Locked'))
         ->appendChild(
-          pht(
+          pht('This credential is already locked.'))
-            '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_CONDUIT)
+        ->setNewValue(0);
+
       $xactions[] = id(new PassphraseCredentialTransaction())
         ->setTransactionType(PassphraseCredentialTransaction::TYPE_LOCK)
         ->setNewValue(1);
@@ -48,6 +50,7 @@ final class PassphraseCredentialLockController
       $editor = id(new PassphraseCredentialTransactionEditor())
         ->setActor($viewer)
         ->setContinueOnMissingFields(true)
+        ->setContinueOnNoEffect(true)
         ->setContentSourceFromRequest($request)
         ->applyTransactions($credential, $xactions);
 
@@ -55,12 +58,13 @@ final class PassphraseCredentialLockController
     }
 
     return $this->newDialog()
-      ->setTitle(pht('Really lock credential?'))
+      ->setTitle(pht('Lock Credential'))
       ->appendChild(
         pht(
-          'This credential will be locked and the secret will be '.
+          'This credential will be locked and the secret will be hidden '.
-          'hidden forever. Anything relying on this credential will '.
+          'forever. If Conduit access is enabled, it will be revoked. '.
-          'still function. This operation can not be undone.'))
+          'Anything relying on this credential will still function. This '.
+          'operation can not be undone.'))
       ->addSubmitButton(pht('Lock Credential'))
       ->addCancelButton($view_uri);
   }

src/applications/passphrase/controller/PassphraseCredentialViewController.php

@@ -119,6 +119,8 @@ final class PassphraseCredentialViewController extends PassphraseController {
       $credential,
       PhabricatorPolicyCapability::CAN_EDIT);
 
+    $can_conduit = ($can_edit && !$is_locked);
+
     $curtain = $this->newCurtainView($credential);
 
     $curtain->addAction(
@@ -161,7 +163,7 @@ final class PassphraseCredentialViewController extends PassphraseController {
           ->setName($credential_conduit_text)
           ->setIcon($credential_conduit_icon)
           ->setHref($this->getApplicationURI("conduit/{$id}/"))
-          ->setDisabled(!$can_edit)
+          ->setDisabled(!$can_conduit)
           ->setWorkflow(true));
 
       $curtain->addAction(