Modernize policies in Paste and Macro

Summary:
Ref T603. Fixes T2823. This updates Paste and Macro.

  - **Paste**
    - Added default view policy.
    - I didn't add a "create" policy, since I can't come up with any realistic scenario where you'd give users access to pastes but not let them create them.
  - **Macro**
    - Added a "manage" policy, which covers creating and editing macros. This lets an install only allow "People With An Approved Sense of Humor" or whatever to create macros.
    - Removed the "edit" policy, since giving individual users access to specific macros doesn't make much sense to me.
    - Changed the view policy to the "most public" policy the install allows.
    - Added view policy information to the header.

Also fix a couple of minor things in Maniphest.

Test Plan:
  - Set Paste policy, created pastes via web and Conduit, saw they got the right default policies.
  - Set Macro policy, tried to create/edit macros with valid and unauthorized users.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2823, T603

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

src/__phutil_library_map__.php

@@ -803,6 +803,7 @@ phutil_register_library_map(array(
     'PackageDeleteMail' => 'applications/owners/mail/PackageDeleteMail.php',
     'PackageMail' => 'applications/owners/mail/PackageMail.php',
     'PackageModifyMail' => 'applications/owners/mail/PackageModifyMail.php',
+    'PasteCapabilityDefaultView' => 'applications/paste/capability/PasteCapabilityDefaultView.php',
     'PasteCreateMailReceiver' => 'applications/paste/mail/PasteCreateMailReceiver.php',
     'PasteEmbedView' => 'applications/paste/view/PasteEmbedView.php',
     'PasteMockMailReceiver' => 'applications/paste/mail/PasteMockMailReceiver.php',
@@ -1292,6 +1293,7 @@ phutil_register_library_map(array(
     'PhabricatorLocalTimeTestCase' => 'view/__tests__/PhabricatorLocalTimeTestCase.php',
     'PhabricatorLogoutController' => 'applications/auth/controller/PhabricatorLogoutController.php',
     'PhabricatorMacroAudioController' => 'applications/macro/controller/PhabricatorMacroAudioController.php',
+    'PhabricatorMacroCapabilityManage' => 'applications/macro/capability/PhabricatorMacroCapabilityManage.php',
     'PhabricatorMacroCommentController' => 'applications/macro/controller/PhabricatorMacroCommentController.php',
     'PhabricatorMacroConfigOptions' => 'applications/macro/config/PhabricatorMacroConfigOptions.php',
     'PhabricatorMacroController' => 'applications/macro/controller/PhabricatorMacroController.php',
@@ -2943,6 +2945,7 @@ phutil_register_library_map(array(
     'PackageDeleteMail' => 'PackageMail',
     'PackageMail' => 'PhabricatorMail',
     'PackageModifyMail' => 'PackageMail',
+    'PasteCapabilityDefaultView' => 'PhabricatorPolicyCapability',
     'PasteCreateMailReceiver' => 'PhabricatorMailReceiver',
     'PasteEmbedView' => 'AphrontView',
     'PasteMockMailReceiver' => 'PhabricatorObjectMailReceiver',
@@ -3482,6 +3485,7 @@ phutil_register_library_map(array(
     'PhabricatorLocalTimeTestCase' => 'PhabricatorTestCase',
     'PhabricatorLogoutController' => 'PhabricatorAuthController',
     'PhabricatorMacroAudioController' => 'PhabricatorMacroController',
+    'PhabricatorMacroCapabilityManage' => 'PhabricatorPolicyCapability',
     'PhabricatorMacroCommentController' => 'PhabricatorMacroController',
     'PhabricatorMacroConfigOptions' => 'PhabricatorApplicationConfigOptions',
     'PhabricatorMacroController' => 'PhabricatorController',

src/applications/base/PhabricatorApplication.php

@@ -349,11 +349,7 @@ abstract class PhabricatorApplication
 
     switch ($capability) {
       case PhabricatorPolicyCapability::CAN_VIEW:
-        if (PhabricatorEnv::getEnvConfig('policy.allow-public')) {
+        return PhabricatorPolicies::getMostOpenPolicy();
-          return PhabricatorPolicies::POLICY_PUBLIC;
-        } else {
-          return PhabricatorPolicies::POLICY_USER;
-        }
       case PhabricatorPolicyCapability::CAN_EDIT:
         return PhabricatorPolicies::POLICY_ADMIN;
       default:

src/applications/macro/application/PhabricatorApplicationMacro.php

@@ -42,4 +42,12 @@ final class PhabricatorApplicationMacro extends PhabricatorApplication {
     );
   }
 
+  protected function getCustomCapabilities() {
+    return array(
+      PhabricatorMacroCapabilityManage::CAPABILITY => array(
+        'caption' => pht('Allows creating and editing macros.')
+      ),
+    );
+  }
+
 }

src/applications/macro/capability/PhabricatorMacroCapabilityManage.php

@@ -0,0 +1,20 @@
+<?php
+
+final class PhabricatorMacroCapabilityManage
+  extends PhabricatorPolicyCapability {
+
+  const CAPABILITY = 'macro.manage';
+
+  public function getCapabilityKey() {
+    return self::CAPABILITY;
+  }
+
+  public function getCapabilityName() {
+    return pht('Can Manage Macros');
+  }
+
+  public function describeCapabilityRejection() {
+    return pht('You do not have permission to manage image macros.');
+  }
+
+}

src/applications/macro/controller/PhabricatorMacroAudioController.php

@@ -10,6 +10,10 @@ final class PhabricatorMacroAudioController
   }
 
   public function processRequest() {
+
+    $this->requireApplicationCapability(
+      PhabricatorMacroCapabilityManage::CAPABILITY);
+
     $request = $this->getRequest();
     $viewer = $request->getUser();
 

src/applications/macro/controller/PhabricatorMacroController.php

@@ -28,11 +28,16 @@ abstract class PhabricatorMacroController
   protected function buildApplicationCrumbs() {
     $crumbs = parent::buildApplicationCrumbs();
 
+    $can_manage = $this->hasApplicationCapability(
+      PhabricatorMacroCapabilityManage::CAPABILITY);
+
     $crumbs->addAction(
       id(new PHUIListItemView())
         ->setName(pht('Create Macro'))
         ->setHref($this->getApplicationURI('/create/'))
-        ->setIcon('create'));
+        ->setIcon('create')
+        ->setDisabled(!$can_manage)
+        ->setWorkflow(!$can_manage));
 
     return $crumbs;
   }

src/applications/macro/controller/PhabricatorMacroDisableController.php

@@ -10,6 +10,10 @@ final class PhabricatorMacroDisableController
   }
 
   public function processRequest() {
+
+    $this->requireApplicationCapability(
+      PhabricatorMacroCapabilityManage::CAPABILITY);
+
     $request = $this->getRequest();
     $user = $request->getUser();
 

src/applications/macro/controller/PhabricatorMacroEditController.php

@@ -11,17 +11,15 @@ final class PhabricatorMacroEditController
 
   public function processRequest() {
 
+    $this->requireApplicationCapability(
+      PhabricatorMacroCapabilityManage::CAPABILITY);
+
     $request = $this->getRequest();
     $user = $request->getUser();
 
     if ($this->id) {
       $macro = id(new PhabricatorMacroQuery())
         ->setViewer($user)
-        ->requireCapabilities(
-          array(
-            PhabricatorPolicyCapability::CAN_VIEW,
-            PhabricatorPolicyCapability::CAN_EDIT,
-          ))
         ->withIDs(array($this->id))
         ->executeOne();
       if (!$macro) {

src/applications/macro/controller/PhabricatorMacroViewController.php

@@ -70,6 +70,8 @@ final class PhabricatorMacroViewController
       ->setMarkupEngine($engine);
 
     $header = id(new PHUIHeaderView())
+      ->setUser($user)
+      ->setPolicyObject($macro)
       ->setHeader($title_long);
 
     if ($macro->getIsDisabled()) {
@@ -128,6 +130,10 @@ final class PhabricatorMacroViewController
   }
 
   private function buildActionView(PhabricatorFileImageMacro $macro) {
+
+    $can_manage = $this->hasApplicationCapability(
+      PhabricatorMacroCapabilityManage::CAPABILITY);
+
     $request = $this->getRequest();
     $view = id(new PhabricatorActionListView())
       ->setUser($request->getUser())
@@ -137,12 +143,16 @@ final class PhabricatorMacroViewController
         id(new PhabricatorActionView())
         ->setName(pht('Edit Macro'))
         ->setHref($this->getApplicationURI('/edit/'.$macro->getID().'/'))
+        ->setDisabled(!$can_manage)
+        ->setWorkflow(!$can_manage)
         ->setIcon('edit'));
 
     $view->addAction(
       id(new PhabricatorActionView())
         ->setName(pht('Edit Audio'))
         ->setHref($this->getApplicationURI('/audio/'.$macro->getID().'/'))
+        ->setDisabled(!$can_manage)
+        ->setWorkflow(!$can_manage)
         ->setIcon('herald'));
 
     if ($macro->getIsDisabled()) {
@@ -151,6 +161,7 @@ final class PhabricatorMacroViewController
           ->setName(pht('Restore Macro'))
           ->setHref($this->getApplicationURI('/disable/'.$macro->getID().'/'))
           ->setWorkflow(true)
+          ->setDisabled(!$can_manage)
           ->setIcon('undo'));
     } else {
       $view->addAction(
@@ -158,6 +169,7 @@ final class PhabricatorMacroViewController
           ->setName(pht('Disable Macro'))
           ->setHref($this->getApplicationURI('/disable/'.$macro->getID().'/'))
           ->setWorkflow(true)
+          ->setDisabled(!$can_manage)
           ->setIcon('delete'));
     }
 

src/applications/macro/storage/PhabricatorFileImageMacro.php

@@ -64,12 +64,11 @@ final class PhabricatorFileImageMacro extends PhabricatorFileDAO
   public function getCapabilities() {
     return array(
       PhabricatorPolicyCapability::CAN_VIEW,
-      PhabricatorPolicyCapability::CAN_EDIT,
     );
   }
 
   public function getPolicy($capability) {
-    return PhabricatorPolicies::POLICY_USER;
+    return PhabricatorPolicies::getMostOpenPolicy();
   }
 
   public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {

src/applications/maniphest/controller/ManiphestTaskDetailController.php

@@ -462,7 +462,7 @@ final class ManiphestTaskDetailController extends ManiphestController {
         ->setWorkflow(true)
         ->setIcon('merge')
         ->setDisabled(!$can_edit)
-        ->setWorkflow(!$can_edit));
+        ->setWorkflow(true));
 
     $view->addAction(
       id(new PhabricatorActionView())

src/applications/paste/application/PhabricatorApplicationPaste.php

@@ -1,8 +1,5 @@
 <?php
 
-/**
- * group paste
- */
 final class PhabricatorApplicationPaste extends PhabricatorApplication {
 
   public function getBaseURI() {
@@ -44,4 +41,13 @@ final class PhabricatorApplicationPaste extends PhabricatorApplication {
     );
   }
 
+  protected function getCustomCapabilities() {
+    return array(
+      PasteCapabilityDefaultView::CAPABILITY => array(
+        'caption' => pht(
+          'Default view policy for newly created pastes.')
+      ),
+    );
+  }
+
 }

src/applications/paste/capability/PasteCapabilityDefaultView.php

@@ -0,0 +1,20 @@
+<?php
+
+final class PasteCapabilityDefaultView
+  extends PhabricatorPolicyCapability {
+
+  const CAPABILITY = 'paste.default.view';
+
+  public function getCapabilityKey() {
+    return self::CAPABILITY;
+  }
+
+  public function getCapabilityName() {
+    return pht('Default View Policy');
+  }
+
+  public function shouldAllowPublicPolicySetting() {
+    return true;
+  }
+
+}

src/applications/paste/conduit/ConduitAPI_paste_create_Method.php

@@ -51,12 +51,10 @@ final class ConduitAPI_paste_create_Method extends ConduitAPI_paste_Method {
 
     // TODO: This should use PhabricatorPasteEditor.
 
-    $paste = new PhabricatorPaste();
+    $paste = PhabricatorPaste::initializeNewPaste($user);
     $paste->setTitle($title);
     $paste->setLanguage($language);
     $paste->setFilePHID($paste_file->getPHID());
-    $paste->setAuthorPHID($user->getPHID());
-    $paste->setViewPolicy(PhabricatorPolicies::POLICY_USER);
     $paste->save();
 
     $paste_file->attachToObject($user, $paste->getPHID());

src/applications/paste/controller/PhabricatorPasteEditController.php

@@ -21,7 +21,7 @@ final class PhabricatorPasteEditController extends PhabricatorPasteController {
     if (!$this->id) {
       $is_create = true;
 
-      $paste = new PhabricatorPaste();
+      $paste = PhabricatorPaste::initializeNewPaste($user);
 
       $parent_id = $request->getStr('parent');
       if ($parent_id) {

src/applications/paste/query/PhabricatorPasteQuery.php

@@ -175,7 +175,14 @@ final class PhabricatorPasteQuery
         unset($pastes[$key]);
         continue;
       }
-      $paste->attachRawContent($file->loadFileData());
+      try {
+        $paste->attachRawContent($file->loadFileData());
+      } catch (Exception $ex) {
+        // We can hit various sorts of file storage issues here. Just drop the
+        // paste if the file is dead.
+        unset($pastes[$key]);
+        continue;
+      }
     }
 
     return $pastes;

src/applications/paste/storage/PhabricatorPaste.php

@@ -1,8 +1,5 @@
 <?php
 
-/**
- * @group paste
- */
 final class PhabricatorPaste extends PhabricatorPasteDAO
   implements
     PhabricatorSubscribableInterface,
@@ -20,6 +17,20 @@ final class PhabricatorPaste extends PhabricatorPasteDAO
   private $content = self::ATTACHABLE;
   private $rawContent = self::ATTACHABLE;
 
+  public static function initializeNewPaste(PhabricatorUser $actor) {
+    $app = id(new PhabricatorApplicationQuery())
+      ->setViewer($actor)
+      ->withClasses(array('PhabricatorApplicationPaste'))
+      ->executeOne();
+
+    $view_policy = $app->getPolicy(PasteCapabilityDefaultView::CAPABILITY);
+
+    return id(new PhabricatorPaste())
+      ->setTitle('')
+      ->setAuthorPHID($actor->getPHID())
+      ->setViewPolicy($view_policy);
+  }
+
   public function getURI() {
     return '/P'.$this->getID();
   }
@@ -42,29 +53,6 @@ final class PhabricatorPaste extends PhabricatorPasteDAO
     return parent::save();
   }
 
-  public function getCapabilities() {
-    return array(
-      PhabricatorPolicyCapability::CAN_VIEW,
-      PhabricatorPolicyCapability::CAN_EDIT,
-    );
-  }
-
-  public function getPolicy($capability) {
-    if ($capability == PhabricatorPolicyCapability::CAN_VIEW) {
-      return $this->viewPolicy;
-    }
-    return PhabricatorPolicies::POLICY_NOONE;
-  }
-
-  public function hasAutomaticCapability($capability, PhabricatorUser $user) {
-    return ($user->getPHID() == $this->getAuthorPHID());
-  }
-
-  public function describeAutomaticCapability($capability) {
-    return pht(
-      'The author of a paste can always view and edit it.');
-  }
-
   public function getFullName() {
     $title = $this->getTitle();
     if (!$title) {
@@ -91,7 +79,7 @@ final class PhabricatorPaste extends PhabricatorPasteDAO
     return $this;
   }
 
-/* -(  PhabricatorSubscribableInterface Implementation  )-------------------- */
+/* -(  PhabricatorSubscribableInterface  )----------------------------------- */
 
 
   public function isAutomaticallySubscribed($phid) {
@@ -107,4 +95,31 @@ final class PhabricatorPaste extends PhabricatorPasteDAO
     );
   }
 
+
+/* -(  PhabricatorPolicyInterface  )----------------------------------------- */
+
+
+  public function getCapabilities() {
+    return array(
+      PhabricatorPolicyCapability::CAN_VIEW,
+      PhabricatorPolicyCapability::CAN_EDIT,
+    );
+  }
+
+  public function getPolicy($capability) {
+    if ($capability == PhabricatorPolicyCapability::CAN_VIEW) {
+      return $this->viewPolicy;
+    }
+    return PhabricatorPolicies::POLICY_NOONE;
+  }
+
+  public function hasAutomaticCapability($capability, PhabricatorUser $user) {
+    return ($user->getPHID() == $this->getAuthorPHID());
+  }
+
+  public function describeAutomaticCapability($capability) {
+    return pht('The author of a paste can always view and edit it.');
+  }
+
+
 }

src/applications/policy/constants/PhabricatorPolicies.php

@@ -7,4 +7,19 @@ final class PhabricatorPolicies extends PhabricatorPolicyConstants {
   const POLICY_ADMIN    = 'admin';
   const POLICY_NOONE    = 'no-one';
 
+  /**
+   * Returns the most public policy this install's configuration permits.
+   * This is either "public" (if available) or "all users" (if not).
+   *
+   * @return const Most open working policy constant.
+   */
+  public static function getMostOpenPolicy() {
+    if (PhabricatorEnv::getEnvConfig('policy.allow-public')) {
+      return PhabricatorPolicies::POLICY_PUBLIC;
+    } else {
+      return PhabricatorPolicies::POLICY_USER;
+    }
+  }
+
+
 }