Make Herald transcripts policy-aware

Summary:
Ref T603. Herald transcripts potentially leak a bunch of content (task text, revision/commit content). Don't let users see them if they can't see the actual objects.

This is a little messy but ends up mostly reasonable-ish.

Test Plan:
  - Verified that transcripts for objects I couldn't see no longer appear in the list, and reject access.
  - Verified that transcripts for objects in applications I can't see reject access, albeit less gracefully.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

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

src/__phutil_library_map__.php

@@ -661,6 +661,7 @@ phutil_register_library_map(array(
     'HeraldTranscript' => 'applications/herald/storage/transcript/HeraldTranscript.php',
     'HeraldTranscriptController' => 'applications/herald/controller/HeraldTranscriptController.php',
     'HeraldTranscriptListController' => 'applications/herald/controller/HeraldTranscriptListController.php',
+    'HeraldTranscriptQuery' => 'applications/herald/query/HeraldTranscriptQuery.php',
     'Javelin' => 'infrastructure/javelin/Javelin.php',
     'JavelinReactorExample' => 'applications/uiexample/examples/JavelinReactorExample.php',
     'JavelinUIExample' => 'applications/uiexample/examples/JavelinUIExample.php',
@@ -2753,6 +2754,7 @@ phutil_register_library_map(array(
     'HeraldTranscript' => 'HeraldDAO',
     'HeraldTranscriptController' => 'HeraldController',
     'HeraldTranscriptListController' => 'HeraldController',
+    'HeraldTranscriptQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'JavelinReactorExample' => 'PhabricatorUIExample',
     'JavelinUIExample' => 'PhabricatorUIExample',
     'JavelinViewExample' => 'PhabricatorUIExample',

src/applications/herald/adapter/HeraldAdapter.php

@@ -93,10 +93,6 @@ abstract class HeraldAdapter {
 
   abstract public function applyHeraldEffects(array $effects);
 
-  public function isEnabled() {
-    return true;
-  }
-
   public function isAvailableToUser(PhabricatorUser $viewer) {
     $applications = id(new PhabricatorApplicationQuery())
       ->setViewer($viewer)

src/applications/herald/controller/HeraldTranscriptController.php

@@ -25,10 +25,15 @@ final class HeraldTranscriptController extends HeraldController {
   }
 
   public function processRequest() {
+    $request = $this->getRequest();
+    $viewer = $request->getUser();
 
-    $xscript = id(new HeraldTranscript())->load($this->id);
+    $xscript = id(new HeraldTranscriptQuery())
+      ->setViewer($viewer)
+      ->withIDs(array($this->id))
+      ->executeOne();
     if (!$xscript) {
-      throw new Exception('Uknown transcript!');
+      return new Aphront404Response();
     }
 
     require_celerity_resource('herald-test-css');
@@ -46,9 +51,19 @@ final class HeraldTranscriptController extends HeraldController {
           pht('Details of this transcript have been garbage collected.')));
       $nav->appendChild($notice);
     } else {
+      $map = HeraldAdapter::getEnabledAdapterMap($viewer);
+      $object_type = $object_xscript->getType();
+      if (empty($map[$object_type])) {
+        // TODO: We should filter these out in the Query, but we have to load
+        // the objectTranscript right now, which is potentially enormous. We
+        // should denormalize the object type, or move the data into a separate
+        // table, and then filter this earlier (and thus raise a better error).
+        // For now, just block access so we don't violate policies.
+        throw new Exception(
+          pht("This transcript has an invalid or inaccessible adapter."));
+      }
 
-      $this->adapter = HeraldAdapter::getAdapterForContentType(
+      $this->adapter = HeraldAdapter::getAdapterForContentType($object_type);
-        $object_xscript->getType());
 
       $filter = $this->getFilterPHIDs();
       $this->filterTranscript($xscript, $filter);

src/applications/herald/controller/HeraldTranscriptListController.php

@@ -7,61 +7,33 @@ final class HeraldTranscriptListController extends HeraldController {
     $request = $this->getRequest();
     $user = $request->getUser();
 
-    // Get one page of data together with the pager.
+    $pager = new AphrontCursorPagerView();
-    // Pull these objects manually since the serialized fields are gigantic.
+    $pager->readFromRequest($request);
-    $transcript = new HeraldTranscript();
 
-    $conn_r = $transcript->establishConnection('r');
+    $transcripts = id(new HeraldTranscriptQuery())
-    $phid = $request->getStr('phid');
+      ->setViewer($user)
-    $where_clause = '';
+      ->needPartialRecords(true)
-    if ($phid) {
+      ->executeWithCursorPager($pager);
-      $where_clause = qsprintf(
-        $conn_r,
-        'WHERE objectPHID = %s',
-        $phid);
-    }
-
-    $pager = new AphrontPagerView();
-    $pager->setOffset($request->getInt('offset'));
-    $pager->setURI($request->getRequestURI(), 'offset');
-
-    $limit_clause = qsprintf(
-      $conn_r,
-      'LIMIT %d, %d',
-      $pager->getOffset(),
-      $pager->getPageSize() + 1);
-
-    $data = queryfx_all(
-      $conn_r,
-      'SELECT id, objectPHID, time, duration, dryRun FROM %T
-        %Q
-        ORDER BY id DESC
-        %Q',
-      $transcript->getTableName(),
-      $where_clause,
-      $limit_clause);
-
-    $data = $pager->sliceResults($data);
 
     // Render the table.
     $handles = array();
-    if ($data) {
+    if ($transcripts) {
-      $phids = ipull($data, 'objectPHID', 'objectPHID');
+      $phids = mpull($transcripts, 'getObjectPHID', 'getObjectPHID');
       $handles = $this->loadViewerHandles($phids);
     }
 
     $rows = array();
-    foreach ($data as $xscript) {
+    foreach ($transcripts as $xscript) {
       $rows[] = array(
-        phabricator_date($xscript['time'], $user),
+        phabricator_date($xscript->getTime(), $user),
-        phabricator_time($xscript['time'], $user),
+        phabricator_time($xscript->getTime(), $user),
-        $handles[$xscript['objectPHID']]->renderLink(),
+        $handles[$xscript->getObjectPHID()]->renderLink(),
-        $xscript['dryRun'] ? 'Yes' : '',
+        $xscript->getDryRun() ? pht('Yes') : '',
-        number_format((int)(1000 * $xscript['duration'])).' ms',
+        number_format((int)(1000 * $xscript->getDuration())).' ms',
         phutil_tag(
           'a',
           array(
-            'href' => '/herald/transcript/'.$xscript['id'].'/',
+            'href' => '/herald/transcript/'.$xscript->getID().'/',
             'class' => 'button small grey',
           ),
           pht('View Transcript')),

src/applications/herald/query/HeraldTranscriptQuery.php

@@ -0,0 +1,98 @@
+<?php
+
+final class HeraldTranscriptQuery
+  extends PhabricatorCursorPagedPolicyAwareQuery {
+
+  private $ids;
+  private $needPartialRecords;
+
+  public function withIDs(array $ids) {
+    $this->ids = $ids;
+    return $this;
+  }
+
+  public function needPartialRecords($need_partial) {
+    $this->needPartialRecords = $need_partial;
+    return $this;
+  }
+
+  public function loadPage() {
+    $transcript = new HeraldTranscript();
+    $conn_r = $transcript->establishConnection('r');
+
+    // NOTE: Transcripts include a potentially enormous amount of serialized
+    // data, so we're loading only some of the fields here if the caller asked
+    // for partial records.
+
+    if ($this->needPartialRecords) {
+      $fields = implode(
+        ', ',
+        array(
+          'id',
+          'phid',
+          'objectPHID',
+          'time',
+          'duration',
+          'dryRun',
+          'host',
+        ));
+    } else {
+      $fields = '*';
+    }
+
+    $rows = queryfx_all(
+      $conn_r,
+      'SELECT %Q FROM %T t %Q %Q %Q',
+      $fields,
+      $transcript->getTableName(),
+      $this->buildWhereClause($conn_r),
+      $this->buildOrderClause($conn_r),
+      $this->buildLimitClause($conn_r));
+
+    $transcripts = $transcript->loadAllFromArray($rows);
+
+    if ($this->needPartialRecords) {
+      // Make sure nothing tries to write these; they aren't complete.
+      foreach ($transcripts as $transcript) {
+        $transcript->makeEphemeral();
+      }
+    }
+
+    return $transcripts;
+  }
+
+  public function willFilterPage(array $transcripts) {
+    $phids = mpull($transcripts, 'getObjectPHID');
+
+    $objects = id(new PhabricatorObjectQuery())
+      ->setViewer($this->getViewer())
+      ->withPHIDs($phids)
+      ->execute();
+
+    foreach ($transcripts as $key => $transcript) {
+      if (empty($objects[$transcript->getObjectPHID()])) {
+        $this->didRejectResult($transcript);
+        unset($transcripts[$key]);
+      }
+    }
+
+    return $transcripts;
+  }
+
+  public function buildWhereClause(AphrontDatabaseConnection $conn_r) {
+    $where = array();
+
+    if ($this->ids) {
+      $where[] = qsprintf(
+        $conn_r,
+        'id IN (%Ld)',
+        $this->ids);
+    }
+
+    $where[] = $this->buildPagingClause($conn_r);
+
+    return $this->formatWhereClause($where);
+  }
+
+
+}

src/applications/herald/storage/transcript/HeraldTranscript.php

@@ -1,6 +1,7 @@
 <?php
 
-final class HeraldTranscript extends HeraldDAO {
+final class HeraldTranscript extends HeraldDAO
+  implements PhabricatorPolicyInterface {
 
   protected $id;
   protected $phid;
@@ -166,4 +167,30 @@ final class HeraldTranscript extends HeraldDAO {
     return PhabricatorPHID::generateNewPHID('HLXS');
   }
 
+/* -(  PhabricatorPolicyInterface  )----------------------------------------- */
+
+  public function getCapabilities() {
+    return array(
+      PhabricatorPolicyCapability::CAN_VIEW,
+    );
+  }
+
+  public function getPolicy($capability) {
+    switch ($capability) {
+      case PhabricatorPolicyCapability::CAN_VIEW:
+        return PhabricatorPolicies::POLICY_USER;
+    }
+  }
+
+  public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
+    return false;
+  }
+
+  public function describeAutomaticCapability($capability) {
+    return pht(
+      'To view a transcript, you must be able to view the object the '.
+      'transcript is about.');
+  }
+
+
 }