Make event-triggered actions more aware of application access

Summary:
Fixes T3675.

  - Maniphest had a couple of old non-event listeners; move them to events.
  - Make most of the similar listeners a little more similar.
  - Add checks for access to the application.

Test Plan:
  - Viewed profile, project, task, revision.
  - Clicked all the actions.
  - Blocked access to various applications and verified the actions vanished.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3675

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

src/__phutil_library_map__.php

@@ -297,7 +297,7 @@ phutil_register_library_map(array(
     'DefaultDatabaseConfigurationProvider' => 'infrastructure/storage/configuration/DefaultDatabaseConfigurationProvider.php',
     'DifferentialAction' => 'applications/differential/constants/DifferentialAction.php',
     'DifferentialActionHasNoEffectException' => 'applications/differential/exception/DifferentialActionHasNoEffectException.php',
-    'DifferentialActionMenuEventListener' => 'applications/differential/events/DifferentialActionMenuEventListener.php',
+    'DifferentialActionMenuEventListener' => 'applications/differential/event/DifferentialActionMenuEventListener.php',
     'DifferentialAddCommentView' => 'applications/differential/view/DifferentialAddCommentView.php',
     'DifferentialAffectedPath' => 'applications/differential/storage/DifferentialAffectedPath.php',
     'DifferentialApplyPatchFieldSpecification' => 'applications/differential/field/specification/DifferentialApplyPatchFieldSpecification.php',
@@ -368,7 +368,7 @@ phutil_register_library_map(array(
     'DifferentialFreeformFieldTestCase' => 'applications/differential/field/specification/__tests__/DifferentialFreeformFieldTestCase.php',
     'DifferentialGitSVNIDFieldSpecification' => 'applications/differential/field/specification/DifferentialGitSVNIDFieldSpecification.php',
     'DifferentialHostFieldSpecification' => 'applications/differential/field/specification/DifferentialHostFieldSpecification.php',
-    'DifferentialHovercardEventListener' => 'applications/differential/events/DifferentialHovercardEventListener.php',
+    'DifferentialHovercardEventListener' => 'applications/differential/event/DifferentialHovercardEventListener.php',
     'DifferentialHunk' => 'applications/differential/storage/DifferentialHunk.php',
     'DifferentialHunkParser' => 'applications/differential/parser/DifferentialHunkParser.php',
     'DifferentialHunkParserTestCase' => 'applications/differential/parser/__tests__/DifferentialHunkParserTestCase.php',
@@ -1876,6 +1876,7 @@ phutil_register_library_map(array(
     'PhluxVariableEditor' => 'applications/phlux/editor/PhluxVariableEditor.php',
     'PhluxVariableQuery' => 'applications/phlux/query/PhluxVariableQuery.php',
     'PhluxViewController' => 'applications/phlux/controller/PhluxViewController.php',
+    'PholioActionMenuEventListener' => 'applications/pholio/event/PholioActionMenuEventListener.php',
     'PholioConstants' => 'applications/pholio/constants/PholioConstants.php',
     'PholioController' => 'applications/pholio/controller/PholioController.php',
     'PholioDAO' => 'applications/pholio/storage/PholioDAO.php',
@@ -4122,6 +4123,7 @@ phutil_register_library_map(array(
     'PhluxVariableEditor' => 'PhabricatorApplicationTransactionEditor',
     'PhluxVariableQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhluxViewController' => 'PhluxController',
+    'PholioActionMenuEventListener' => 'PhabricatorEventListener',
     'PholioController' => 'PhabricatorController',
     'PholioDAO' => 'PhabricatorLiskDAO',
     'PholioImage' =>

src/applications/audit/events/AuditActionMenuEventListener.php

@@ -14,24 +14,32 @@ final class AuditActionMenuEventListener extends PhabricatorEventListener {
     }
   }
 
-  private function handleActionsEvent($event) {
+  private function handleActionsEvent(PhutilEvent $event) {
-    $person = $event->getValue('object');
+    $object = $event->getValue('object');
-    if (!($person instanceof PhabricatorUser)) {
+
-      return;
+    $actions = null;
+    if ($object instanceof PhabricatorUser) {
+      $actions = $this->renderUserItems($event);
     }
 
-    $actions = $event->getValue('actions');
+    $this->addActionMenuItems($event, $actions);
+  }
 
-    $username = phutil_escape_uri($person->getUsername());
+  private function renderUserItems(PhutilEvent $event) {
-    $href = '/audit/view/author/'.$username.'/';
+    if (!$this->canUseApplication($event->getUser())) {
+      return null;
+    }
 
-    $actions[] = id(new PhabricatorActionView())
+    $user = $event->getValue('object');
+
+    $username = phutil_escape_uri($user->getUsername());
+    $view_uri = '/audit/view/author/'.$username.'/';
+
+    return id(new PhabricatorActionView())
       ->setIcon('audit-dark')
       ->setIconSheet(PHUIIconView::SPRITE_APPS)
       ->setName(pht('View Commits'))
-      ->setHref($href);
+      ->setHref($view_uri);
-
-    $event->setValue('actions', $actions);
   }
 
 }

src/applications/conpherence/events/ConpherenceActionMenuEventListener.php

@@ -15,23 +15,30 @@ final class ConpherenceActionMenuEventListener
     }
   }
 
-  private function handleActionsEvent($event) {
+  private function handleActionsEvent(PhutilEvent $event) {
-    $person = $event->getValue('object');
+    $object = $event->getValue('object');
-    if (!($person instanceof PhabricatorUser)) {
+
-      return;
+    $actions = null;
+    if ($object instanceof PhabricatorUser) {
+      $actions = $this->renderUserItems($event);
     }
 
-    $href = '/conpherence/new/?participant='.$person->getPHID();
+    $this->addActionMenuItems($event, $actions);
+  }
 
-    $actions = $event->getValue('actions');
+  private function renderUserItems(PhutilEvent $event) {
+    if (!$this->canUseApplication($event->getUser())) {
+      return null;
+    }
 
-    $actions[] = id(new PhabricatorActionView())
+    $user = $event->getValue('object');
+    $href = '/conpherence/new/?participant='.$user->getPHID();
+
+    return id(new PhabricatorActionView())
       ->setIcon('message')
       ->setName(pht('Send Message'))
       ->setWorkflow(true)
       ->setHref($href);
-
-    $event->setValue('actions', $actions);
   }
 
 }

src/applications/differential/event/DifferentialActionMenuEventListener.php

@@ -0,0 +1,70 @@
+<?php
+
+final class DifferentialActionMenuEventListener
+  extends PhabricatorEventListener {
+
+  public function register() {
+    $this->listen(PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS);
+  }
+
+  public function handleEvent(PhutilEvent $event) {
+    switch ($event->getType()) {
+      case PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS:
+        $this->handleActionsEvent($event);
+      break;
+    }
+  }
+
+  private function handleActionsEvent(PhutilEvent $event) {
+    $object = $event->getValue('object');
+
+    $actions = null;
+    if ($object instanceof PhabricatorUser) {
+      $actions = $this->renderUserItems($event);
+    } else if ($object instanceof ManiphestTask) {
+      $actions = $this->renderTaskItems($event);
+    }
+
+    $this->addActionMenuItems($event, $actions);
+  }
+
+  private function renderUserItems(PhutilEvent $event) {
+    if (!$this->canUseApplication($event->getUser())) {
+      return null;
+    }
+
+    $person = $event->getValue('object');
+    $href = '/differential/?authorPHIDs[]='.$person->getPHID();
+
+    return id(new PhabricatorActionView())
+      ->setRenderAsForm(true)
+      ->setIcon('differential-dark')
+      ->setIconSheet(PHUIIconView::SPRITE_APPS)
+      ->setName(pht('View Revisions'))
+      ->setHref($href);
+  }
+
+  private function renderTaskItems(PhutilEvent $event) {
+    if (!$this->canUseApplication($event->getUser())) {
+      return null;
+    }
+
+    $task = $event->getValue('object');
+    $phid = $task->getPHID();
+
+    $can_edit = PhabricatorPolicyFilter::hasCapability(
+      $event->getUser(),
+      $task,
+      PhabricatorPolicyCapability::CAN_EDIT);
+
+    return id(new PhabricatorActionView())
+      ->setName(pht('Edit Differential Revisions'))
+      ->setHref("/search/attach/{$phid}/DREV/")
+      ->setWorkflow(true)
+      ->setIcon('attach')
+      ->setDisabled(!$can_edit)
+      ->setWorkflow(true);
+  }
+
+}
+

src/applications/differential/events/DifferentialActionMenuEventListener.php

@@ -1,39 +0,0 @@
-<?php
-
-final class DifferentialActionMenuEventListener
-  extends PhabricatorEventListener {
-
-  public function register() {
-    $this->listen(PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS);
-  }
-
-  public function handleEvent(PhutilEvent $event) {
-    switch ($event->getType()) {
-      case PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS:
-        $this->handleActionsEvent($event);
-      break;
-    }
-  }
-
-  private function handleActionsEvent($event) {
-    $person = $event->getValue('object');
-    if (!($person instanceof PhabricatorUser)) {
-      return;
-    }
-
-    $href = '/differential/?authorPHIDs[]='.$person->getPHID();
-
-    $actions = $event->getValue('actions');
-
-    $actions[] = id(new PhabricatorActionView())
-      ->setRenderAsForm(true)
-      ->setIcon('differential-dark')
-      ->setIconSheet(PHUIIconView::SPRITE_APPS)
-      ->setName(pht('View Revisions'))
-      ->setHref($href);
-
-    $event->setValue('actions', $actions);
-  }
-
-}
-

src/applications/flag/events/PhabricatorFlagsUIEventListener.php

@@ -31,6 +31,10 @@ final class PhabricatorFlagsUIEventListener extends PhabricatorEventListener {
       return;
     }
 
+    if (!$this->canUseApplication($event->getUser())) {
+      return null;
+    }
+
     $flag = PhabricatorFlagQuery::loadUserFlag($user, $object->getPHID());
 
     if ($flag) {

src/applications/maniphest/controller/ManiphestTaskDetailController.php

@@ -500,28 +500,6 @@ final class ManiphestTaskDetailController extends ManiphestController {
         ->setDisabled(!$can_edit)
         ->setWorkflow(true));
 
-    $view->addAction(
-      id(new PhabricatorActionView())
-        ->setName(pht('Edit Differential Revisions'))
-        ->setHref("/search/attach/{$phid}/DREV/")
-        ->setWorkflow(true)
-        ->setIcon('attach')
-        ->setDisabled(!$can_edit)
-        ->setWorkflow(true));
-
-    $pholio_app =
-      PhabricatorApplication::getByClass('PhabricatorApplicationPholio');
-    if ($pholio_app->isInstalled()) {
-      $view->addAction(
-        id(new PhabricatorActionView())
-        ->setName(pht('Edit Pholio Mocks'))
-        ->setHref("/search/attach/{$phid}/MOCK/edge/")
-        ->setWorkflow(true)
-        ->setIcon('attach')
-        ->setDisabled(!$can_edit)
-        ->setWorkflow(true));
-    }
-
     return $view;
   }
 

src/applications/maniphest/event/ManiphestActionMenuEventListener.php

@@ -14,29 +14,58 @@ final class ManiphestActionMenuEventListener extends PhabricatorEventListener {
     }
   }
 
-  private function handleActionsEvent($event) {
+  private function handleActionsEvent(PhutilEvent $event) {
-    $actions = $event->getValue('actions');
-
-    $action = id(new PhabricatorActionView())
-      ->setIcon('maniphest-dark')
-      ->setIconSheet(PHUIIconView::SPRITE_APPS)
-      ->setName(pht('View Tasks'));
-
     $object = $event->getValue('object');
-    if ($object instanceof PhabricatorUser) {
-      $href = '/maniphest/?statuses[]=0&assigned='.$object->getPHID().'#R';
-      $actions[] = $action->setHref($href);
-    } else if ($object instanceof PhabricatorProject) {
-      $href = '/maniphest/?statuses[]=0&allProjects[]='.$object->getPHID().'#R';
-      $actions[] = $action->setHref($href);
 
-      $actions[] = id(new PhabricatorActionView())
+    $actions = null;
-        ->setName(pht("Add Task"))
+    if ($object instanceof PhabricatorUser) {
-        ->setIcon('create')
+      $actions = $this->renderUserItems($event);
-        ->setHref('/maniphest/task/create/?projects=' . $object->getPHID());
+    } else if ($object instanceof PhabricatorProject) {
+      $actions = $this->renderProjectItems($event);
     }
 
-    $event->setValue('actions', $actions);
+    $this->addActionMenuItems($event, $actions);
   }
 
+  private function renderUserItems(PhutilEvent $event) {
+    if (!$this->canUseApplication($event->getUser())) {
+      return null;
+    }
+
+    $user = $event->getValue('object');
+    $phid = $user->getPHID();
+    $view_uri = '/maniphest/?statuses[]=0&assigned='.$phid.'#R';
+
+    return id(new PhabricatorActionView())
+      ->setIcon('maniphest-dark')
+      ->setIconSheet(PHUIIconView::SPRITE_APPS)
+      ->setName(pht('View Tasks'))
+      ->setHref($view_uri);
+  }
+
+  private function renderProjectItems(PhutilEvent $event) {
+    if (!$this->canUseApplication($event->getUser())) {
+      return null;
+    }
+
+    $project = $event->getValue('object');
+
+    $phid = $project->getPHID();
+    $view_uri = '/maniphest/?statuses[]=0&allProjects[]='.$phid.'#R';
+    $create_uri = '/maniphest/task/create/?projects='.$phid;
+
+    return array(
+      id(new PhabricatorActionView())
+        ->setIcon('maniphest-dark')
+        ->setIconSheet(PHUIIconView::SPRITE_APPS)
+        ->setName(pht('View Tasks'))
+        ->setHref($view_uri),
+      id(new PhabricatorActionView())
+        ->setName(pht("Add Task"))
+        ->setIcon('create')
+        ->setHref($create_uri),
+    );
+  }
+
+
 }

src/applications/pholio/application/PhabricatorApplicationPholio.php

@@ -34,6 +34,12 @@ final class PhabricatorApplicationPholio extends PhabricatorApplication {
     return true;
   }
 
+  public function getEventListeners() {
+    return array(
+      new PholioActionMenuEventListener(),
+    );
+  }
+
   public function getRemarkupRules() {
     return array(
       new PholioRemarkupRule(),

src/applications/pholio/event/PholioActionMenuEventListener.php

@@ -0,0 +1,52 @@
+<?php
+
+final class PholioActionMenuEventListener
+  extends PhabricatorEventListener {
+
+  public function register() {
+    $this->listen(PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS);
+  }
+
+  public function handleEvent(PhutilEvent $event) {
+    switch ($event->getType()) {
+      case PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS:
+        $this->handleActionsEvent($event);
+      break;
+    }
+  }
+
+  private function handleActionsEvent(PhutilEvent $event) {
+    $object = $event->getValue('object');
+
+    $actions = null;
+    if ($object instanceof ManiphestTask) {
+      $actions = $this->renderTaskItems($event);
+    }
+
+    $this->addActionMenuItems($event, $actions);
+  }
+
+  private function renderTaskItems(PhutilEvent $event) {
+    if (!$this->canUseApplication($event->getUser())) {
+      return;
+    }
+
+    $task = $event->getValue('object');
+    $phid = $task->getPHID();
+
+    $can_edit = PhabricatorPolicyFilter::hasCapability(
+      $event->getUser(),
+      $task,
+      PhabricatorPolicyCapability::CAN_EDIT);
+
+    return  id(new PhabricatorActionView())
+      ->setName(pht('Edit Pholio Mocks'))
+      ->setHref("/search/attach/{$phid}/MOCK/edge/")
+      ->setWorkflow(true)
+      ->setIcon('attach')
+      ->setDisabled(!$can_edit)
+      ->setWorkflow(true);
+  }
+
+}
+

src/applications/phrequent/event/PhrequentUIEventListener.php

@@ -33,6 +33,10 @@ final class PhrequentUIEventListener
       return;
     }
 
+    if (!$this->canUseApplication($event->getUser())) {
+      return;
+    }
+
     $tracking = PhrequentUserTimeQuery::isUserTrackingObject(
       $user,
       $object->getPHID());
@@ -56,9 +60,7 @@ final class PhrequentUIEventListener
       $track_action->setDisabled(true);
     }
 
-    $actions = $event->getValue('actions');
+    $this->addActionMenuItems($event, $track_action);
-    $actions[] = $track_action;
-    $event->setValue('actions', $actions);
   }
 
   private function handlePropertyEvent($ui_event) {
@@ -75,6 +77,10 @@ final class PhrequentUIEventListener
       return;
     }
 
+    if (!$this->canUseApplication($ui_event->getUser())) {
+      return;
+    }
+
     $events = id(new PhrequentUserTimeQuery())
       ->setViewer($user)
       ->withObjectPHIDs(array($object->getPHID()))

src/applications/phriction/event/PhrictionActionMenuEventListener.php

@@ -14,22 +14,31 @@ final class PhrictionActionMenuEventListener extends PhabricatorEventListener {
     }
   }
 
-  private function handleActionsEvent($event) {
+  private function handleActionsEvent(PhutilEvent $event) {
-    $actions = $event->getValue('actions');
-
-    $action = id(new PhabricatorActionView())
-      ->setIcon('phriction-dark')
-      ->setIconSheet(PHUIIconView::SPRITE_APPS)
-      ->setName(pht('View Wiki'));
-
     $object = $event->getValue('object');
+
+    $actions = null;
     if ($object instanceof PhabricatorProject) {
-      $slug = PhabricatorSlug::normalize($object->getPhrictionSlug());
+      $actions = $this->buildProjectActions($event);
-      $href = '/w/projects/'.$slug;
-      $actions[] = $action->setHref($href);
     }
 
-    $event->setValue('actions', $actions);
+    $this->addActionMenuItems($event, $actions);
+  }
+
+  private function buildProjectActions(PhutilEvent $event) {
+    if (!$this->canUseApplication($event->getUser())) {
+      return null;
+    }
+
+    $project = $event->getValue('object');
+    $slug = PhabricatorSlug::normalize($project->getPhrictionSlug());
+    $href = '/w/projects/'.$slug;
+
+    return id(new PhabricatorActionView())
+      ->setIcon('phriction-dark')
+      ->setIconSheet(PHUIIconView::SPRITE_APPS)
+      ->setName(pht('View Wiki'))
+      ->setHref($href);
   }
 
 }

src/applications/tokens/event/PhabricatorTokenUIEventListener.php

@@ -33,6 +33,10 @@ final class PhabricatorTokenUIEventListener
       return;
     }
 
+    if (!$this->canUseApplication($event->getUser())) {
+      return null;
+    }
+
     $current = id(new PhabricatorTokenGivenQuery())
       ->setViewer($user)
       ->withAuthorPHIDs(array($user->getPHID()))
@@ -75,6 +79,10 @@ final class PhabricatorTokenUIEventListener
       return;
     }
 
+    if (!$this->canUseApplication($event->getUser())) {
+      return null;
+    }
+
     $limit = 1;
 
     $tokens_given = id(new PhabricatorTokenGivenQuery())

src/infrastructure/events/PhabricatorEventListener.php

@@ -28,6 +28,26 @@ abstract class PhabricatorEventListener extends PhutilEventListener {
       PhabricatorPolicyCapability::CAN_VIEW);
   }
 
+  protected function addActionMenuItems(PhutilEvent $event, $items) {
+    if ($event->getType() !== PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS) {
+      throw new Exception("Not an action menu event!");
+    }
+
+    if (!$items) {
+      return;
+    }
+
+    if (!is_array($items)) {
+      $items = array($items);
+    }
+
+    $event_actions = $event->getValue('actions');
+    foreach ($items as $item) {
+      $event_actions[] = $item;
+    }
+    $event->setValue('actions', $event_actions);
+  }
+
 }
 
 

src/view/layout/PhabricatorActionListView.php

@@ -42,7 +42,6 @@ final class PhabricatorActionListView extends AphrontView {
     PhutilEventEngine::dispatchEvent($event);
 
     $actions = $event->getValue('actions');
-
     if (!$actions) {
       return null;
     }