From 9252d2a579e338bb821fbc5c0152a1652e493efb Mon Sep 17 00:00:00 2001
From: Bob Trahan <btrahan@phacility.com>
Date: Tue, 11 Nov 2014 13:18:37 -0800
Subject: [PATCH] Phriction - stop leaking document titles you can't see

Summary: Fixes T6495. convert ad hoc query to a PhrictionDocumentQuery, thus enforcing view permissions

Test Plan: noted my test user a had a great wiki while test user b couldn't see most things.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6495

Differential Revision: https://secure.phabricator.com/D10822
---
 .../PhrictionDocumentController.php           | 59 ++++++-----
 .../query/PhrictionDocumentQuery.php          | 97 +++++++++++++++++--
 2 files changed, 116 insertions(+), 40 deletions(-)

diff --git a/src/applications/phriction/controller/PhrictionDocumentController.php b/src/applications/phriction/controller/PhrictionDocumentController.php
index 2f1cc2f6e5..5c04eefaf2 100644
--- a/src/applications/phriction/controller/PhrictionDocumentController.php
+++ b/src/applications/phriction/controller/PhrictionDocumentController.php
@@ -358,33 +358,24 @@ final class PhrictionDocumentController
   }
 
   private function renderDocumentChildren($slug) {
-    $document_dao = new PhrictionDocument();
-    $content_dao = new PhrictionContent();
-    $conn = $document_dao->establishConnection('r');
 
-    $limit = 250;
     $d_child = PhabricatorSlug::getDepth($slug) + 1;
     $d_grandchild = PhabricatorSlug::getDepth($slug) + 2;
+    $limit = 250;
 
-    // Select children and grandchildren.
-    $children = queryfx_all(
-      $conn,
-      'SELECT d.slug, d.depth, c.title FROM %T d JOIN %T c
-        ON d.contentID = c.id
-        WHERE d.slug LIKE %> AND d.depth IN (%d, %d)
-          AND d.status IN (%Ld)
-        ORDER BY d.depth, c.title LIMIT %d',
-      $document_dao->getTableName(),
-      $content_dao->getTableName(),
-      ($slug == '/' ? '' : $slug),
-      $d_child,
-      $d_grandchild,
-      array(
+    $query = id(new PhrictionDocumentQuery())
+      ->setViewer($this->getRequest()->getUser())
+      ->withDepths(array($d_child, $d_grandchild))
+      ->withSlugPrefix($slug == '/' ? '' : $slug)
+      ->withStatuses(array(
         PhrictionDocumentStatus::STATUS_EXISTS,
         PhrictionDocumentStatus::STATUS_STUB,
-      ),
-      $limit);
+      ))
+      ->setLimit($limit)
+      ->setOrder(PhrictionDocumentQuery::ORDER_HIERARCHY)
+      ->needContent(true);
 
+    $children = $query->execute();
     if (!$children) {
       return;
     }
@@ -405,7 +396,7 @@ final class PhrictionDocumentController
     if (count($children) == $limit) {
       $more_children = true;
       foreach ($children as $child) {
-        if ($child['depth'] == $d_grandchild) {
+        if ($child->getDepth() == $d_grandchild) {
           $more_children = false;
         }
       }
@@ -415,24 +406,30 @@ final class PhrictionDocumentController
       $more_children = false;
     }
 
-    $grandchildren = array();
+    $children_dicts = array();
+    $grandchildren_dicts = array();
     foreach ($children as $key => $child) {
-      if ($child['depth'] == $d_child) {
+      $child_dict = array(
+        'slug' => $child->getSlug(),
+        'depth' => $child->getDepth(),
+        'title' => $child->getContent()->getTitle(),);
+      if ($child->getDepth() == $d_child) {
+        $children_dicts[] = $child_dict;
         continue;
       } else {
         unset($children[$key]);
         if ($show_grandchildren) {
-          $ancestors = PhabricatorSlug::getAncestry($child['slug']);
-          $grandchildren[end($ancestors)][] = $child;
+          $ancestors = PhabricatorSlug::getAncestry($child->getSlug());
+          $grandchildren_dicts[end($ancestors)][] = $child_dict;
         }
       }
     }
 
     // Fill in any missing children.
-    $known_slugs = ipull($children, null, 'slug');
-    foreach ($grandchildren as $slug => $ignored) {
+    $known_slugs = mpull($children, null, 'getSlug');
+    foreach ($grandchildren_dicts as $slug => $ignored) {
       if (empty($known_slugs[$slug])) {
-        $children[] = array(
+        $children_dicts[] = array(
           'slug'    => $slug,
           'depth'   => $d_child,
           'title'   => PhabricatorSlug::getDefaultTitle($slug),
@@ -441,13 +438,13 @@ final class PhrictionDocumentController
       }
     }
 
-    $children = isort($children, 'title');
+    $children_dicts = isort($children_dicts, 'title');
 
     $list = array();
-    foreach ($children as $child) {
+    foreach ($children_dicts as $child) {
       $list[] = hsprintf('<li>');
       $list[] = $this->renderChildDocumentLink($child);
-      $grand = idx($grandchildren, $child['slug'], array());
+      $grand = idx($grandchildren_dicts, $child['slug'], array());
       if ($grand) {
         $list[] = hsprintf('<ul>');
         foreach ($grand as $grandchild) {
diff --git a/src/applications/phriction/query/PhrictionDocumentQuery.php b/src/applications/phriction/query/PhrictionDocumentQuery.php
index 35b4ee1675..f47745d191 100644
--- a/src/applications/phriction/query/PhrictionDocumentQuery.php
+++ b/src/applications/phriction/query/PhrictionDocumentQuery.php
@@ -6,6 +6,10 @@ final class PhrictionDocumentQuery
   private $ids;
   private $phids;
   private $slugs;
+  private $depths;
+  private $slugPrefix;
+  private $statuses;
+
 
   private $needContent;
 
@@ -17,6 +21,7 @@ final class PhrictionDocumentQuery
   private $order        = 'order-created';
   const ORDER_CREATED   = 'order-created';
   const ORDER_UPDATED   = 'order-updated';
+  const ORDER_HIERARCHY = 'order-hierarchy';
 
   public function withIDs(array $ids) {
     $this->ids = $ids;
@@ -33,6 +38,21 @@ final class PhrictionDocumentQuery
     return $this;
   }
 
+  public function withDepths(array $depths) {
+    $this->depths = $depths;
+    return $this;
+  }
+
+  public function  withSlugPrefix($slug_prefix) {
+    $this->slugPrefix = $slug_prefix;
+    return $this;
+  }
+
+  public function withStatuses(array $statuses) {
+    $this->statuses = $statuses;
+    return $this;
+  }
+
   public function withStatus($status) {
     $this->status = $status;
     return $this;
@@ -52,12 +72,19 @@ final class PhrictionDocumentQuery
     $table = new PhrictionDocument();
     $conn_r = $table->establishConnection('r');
 
+    if ($this->order = self::ORDER_HIERARCHY) {
+      $order_clause = $this->buildHierarchicalOrderClause($conn_r);
+    } else {
+      $order_clause = $this->buildOrderClause($conn_r);
+    }
+
     $rows = queryfx_all(
       $conn_r,
-      'SELECT * FROM %T %Q %Q %Q',
+      'SELECT * FROM %T d %Q %Q %Q %Q',
       $table->getTableName(),
+      $this->buildJoinClause($conn_r),
       $this->buildWhereClause($conn_r),
-      $this->buildOrderClause($conn_r),
+      $order_clause,
       $this->buildLimitClause($conn_r));
 
     $documents = $table->loadAllFromArray($rows);
@@ -158,35 +185,67 @@ final class PhrictionDocumentQuery
     return $documents;
   }
 
+  private function buildJoinClause(AphrontDatabaseConnection $conn) {
+    $join = null;
+    if ($this->order = self::ORDER_HIERARCHY) {
+      $content_dao = new PhrictionContent();
+      $join = qsprintf(
+        $conn,
+        'JOIN %T c ON d.contentID = c.id',
+        $content_dao->getTableName());
+    }
+    return $join;
+  }
   protected function buildWhereClause(AphrontDatabaseConnection $conn) {
     $where = array();
 
     if ($this->ids) {
       $where[] = qsprintf(
         $conn,
-        'id IN (%Ld)',
+        'd.id IN (%Ld)',
         $this->ids);
     }
 
     if ($this->phids) {
       $where[] = qsprintf(
         $conn,
-        'phid IN (%Ls)',
+        'd.phid IN (%Ls)',
         $this->phids);
     }
 
     if ($this->slugs) {
       $where[] = qsprintf(
         $conn,
-        'slug IN (%Ls)',
+        'd.slug IN (%Ls)',
         $this->slugs);
     }
 
+    if ($this->statuses) {
+      $where[] = qsprintf(
+        $conn,
+        'd.status IN (%Ld)',
+        $this->statuses);
+    }
+
+    if ($this->slugPrefix) {
+      $where[] = qsprintf(
+        $conn,
+        'd.slug LIKE %>',
+        $this->slugPrefix);
+    }
+
+    if ($this->depths) {
+      $where[] = qsprintf(
+        $conn,
+        'd.depth IN (%Ld)',
+        $this->depths);
+    }
+
     switch ($this->status) {
       case self::STATUS_OPEN:
         $where[] = qsprintf(
           $conn,
-          'status NOT IN (%Ld)',
+          'd.status NOT IN (%Ld)',
           array(
             PhrictionDocumentStatus::STATUS_DELETED,
             PhrictionDocumentStatus::STATUS_MOVED,
@@ -196,7 +255,7 @@ final class PhrictionDocumentQuery
       case self::STATUS_NONSTUB:
         $where[] = qsprintf(
           $conn,
-          'status NOT IN (%Ld)',
+          'd.status NOT IN (%Ld)',
           array(
             PhrictionDocumentStatus::STATUS_MOVED,
             PhrictionDocumentStatus::STATUS_STUB,
@@ -213,12 +272,31 @@ final class PhrictionDocumentQuery
     return $this->formatWhereClause($where);
   }
 
+  private function buildHierarchicalOrderClause(
+    AphrontDatabaseConnection $conn_r) {
+
+    if ($this->getBeforeID()) {
+      return qsprintf(
+        $conn_r,
+        'ORDER BY d.depth, c.title, %Q %Q',
+        $this->getPagingColumn(),
+        $this->getReversePaging() ? 'DESC' : 'ASC');
+    } else {
+      return qsprintf(
+        $conn_r,
+        'ORDER BY d.depth, c.title, %Q %Q',
+        $this->getPagingColumn(),
+        $this->getReversePaging() ? 'ASC' : 'DESC');
+    }
+  }
+
   protected function getPagingColumn() {
     switch ($this->order) {
       case self::ORDER_CREATED:
-        return 'id';
+      case self::ORDER_HIERARCHY:
+        return 'd.id';
       case self::ORDER_UPDATED:
-        return 'contentID';
+        return 'd.contentID';
       default:
         throw new Exception("Unknown order '{$this->order}'!");
     }
@@ -227,6 +305,7 @@ final class PhrictionDocumentQuery
   protected function getPagingValue($result) {
     switch ($this->order) {
       case self::ORDER_CREATED:
+      case self::ORDER_HIERARCHY:
         return $result->getID();
       case self::ORDER_UPDATED:
         return $result->getContentID();