LOCAL: Make 'All Users' space extremely magic

Phriction's view policy is ancestral: in order to access /w/foo/bar/baz,
you must be able to access /w/foo and /w/bar in addition to
/w/foo/bar/baz itself.

This is fine and makes life easy: by setting restrictive policies on
top-level pages, we can lessen the risk of someone exposing information
they shouldn't, by accidentally making
/w/cold-fusion/secret-research/funding-meeting/2018-09-14 public, when
the rest of the hierarchy is super locked down.

Phriction also recently gained Spaces support, which is nice: rather
than trying to lock down with groups and harmonise permissions, we can
just move top-level wiki pages to a particular Space, and then we don't
need to worry about groups.

Our clients don't know Spaces even exist, which is great since it avoids
us having to explain the two-tier permission model to them. The reason
they don't know it exists is because if you can only see a single Space,
then Phabricator hides the entire Spaces UI away from you. Great!

Unfortunately one detail ruins everything: /w/ is a top-level page
itself, it counts for permission checks, and it _must be in a Space_.
So, there is no way to have wiki documents in mutually-invisible Spaces
unless you also have a common Space, at which point the whole Spaces UI
suddenly becomes very visible everywhere.

In order to try to keep our wiki partitioned, but to not confuse our
clients (and give them the chance to potentially expose confidential
information!), we:
  - have a magic 'Visible to Everyone' space
  - actually hide that space from everyone with policies
  - hack policy filters to make this space visible to everyone _only
    for the purpose of checking policies on wiki objects_
  - only allow admins to change view/edit policies on the root wiki
    page (see comment for reason why)

This actual patch can obviously never go anywhere near upstream, but on
the other hand we should probably make them aware of the problem and see
if they're interested in discussing a solution, which is probably just
to bless the root page with magic semantics.
Signed-off-by: Daniel Stone <daniels@collabora.com>

src/applications/phriction/controller/PhrictionEditController.php

@@ -137,18 +137,24 @@ final class PhrictionEditController
       $xactions[] = id(new PhrictionTransaction())
         ->setTransactionType($edit_type)
         ->setNewValue($content_text);
-      $xactions[] = id(new PhrictionTransaction())
-        ->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY)
-        ->setNewValue($v_view)
-        ->setIsDefaultTransaction($is_new && ($v_view === $default_view));
-      $xactions[] = id(new PhrictionTransaction())
-        ->setTransactionType(PhabricatorTransactions::TYPE_EDIT_POLICY)
-        ->setNewValue($v_edit)
-        ->setIsDefaultTransaction($is_new && ($v_edit === $default_edit));
-      $xactions[] = id(new PhrictionTransaction())
-        ->setTransactionType(PhabricatorTransactions::TYPE_SPACE)
-        ->setNewValue($v_space)
-        ->setIsDefaultTransaction($is_new && ($v_space === $default_space));
+      if ($v_view) {
+        $xactions[] = id(new PhrictionTransaction())
+          ->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY)
+          ->setNewValue($v_view)
+          ->setIsDefaultTransaction($is_new && ($v_view === $default_view));
+      }
+      if ($v_edit) {
+        $xactions[] = id(new PhrictionTransaction())
+          ->setTransactionType(PhabricatorTransactions::TYPE_EDIT_POLICY)
+          ->setNewValue($v_edit)
+          ->setIsDefaultTransaction($is_new && ($v_edit === $default_edit));
+      }
+      if ($v_space) {
+        $xactions[] = id(new PhrictionTransaction())
+          ->setTransactionType(PhabricatorTransactions::TYPE_SPACE)
+          ->setNewValue($v_space)
+          ->setIsDefaultTransaction($is_new && ($v_space === $default_space));
+      }
       $xactions[] = id(new PhrictionTransaction())
         ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
         ->setNewValue(array('=' => $v_cc));
@@ -265,20 +271,6 @@ final class PhrictionEditController
           ->setValue($v_cc)
           ->setUser($viewer)
           ->setDatasource(new PhabricatorMetaMTAMailableDatasource()))
-      ->appendChild(
-        id(new AphrontFormPolicyControl())
-          ->setViewer($viewer)
-          ->setName('viewPolicy')
-          ->setSpacePHID($v_space)
-          ->setPolicyObject($document)
-          ->setCapability($view_capability)
-          ->setPolicies($policies))
-      ->appendChild(
-        id(new AphrontFormPolicyControl())
-          ->setName('editPolicy')
-          ->setPolicyObject($document)
-          ->setCapability($edit_capability)
-          ->setPolicies($policies))
       ->appendChild(
         id(new AphrontFormTextControl())
           ->setLabel(pht('Edit Notes'))
@@ -286,6 +278,30 @@ final class PhrictionEditController
           ->setError(null)
           ->setName('description'));
 
+    // We don't allow the view/edit policy to be changed on the root document;
+    // partly because doing so is bad, but mostly because it's in a space
+    // which is only visible to admins. As some people changing it would not
+    // be able to actually view the space that it was in, any edit would reset
+    // the space back to whatever sorted first in the list that the user had
+    // access to, and then no-one can edit the wiki.
+    if ($document->getSlug() != '/' || $viewer->getIsAdmin()) {
+      $form
+        ->appendChild(
+          id(new AphrontFormPolicyControl())
+            ->setViewer($viewer)
+            ->setName('viewPolicy')
+            ->setSpacePHID($v_space)
+            ->setPolicyObject($document)
+            ->setCapability($view_capability)
+            ->setPolicies($policies))
+        ->appendChild(
+          id(new AphrontFormPolicyControl())
+            ->setName('editPolicy')
+            ->setPolicyObject($document)
+            ->setCapability($edit_capability)
+            ->setPolicies($policies));
+    }
+
     if ($is_draft_mode) {
       $form->appendControl(
         id(new AphrontFormSubmitControl())

src/applications/policy/filter/PhabricatorPolicyFilter.php

@@ -938,6 +938,11 @@ final class PhabricatorPolicyFilter extends Phobject {
       return false;
     }
 
+    // See long comment in PhabricatorCursorPagedPolicyAwareQuery
+    if ($space->getPHID() == PhabricatorSpacesNamespaceQuery::getMagicAllUsersSpace()->getPHID()) {
+      return true;
+    }
+
     // This may be more involved later, but for now being able to see the
     // space is equivalent to being able to see everything in it.
     return self::hasCapability(

src/applications/spaces/query/PhabricatorSpacesNamespaceQuery.php

@@ -6,6 +6,7 @@ final class PhabricatorSpacesNamespaceQuery
   const KEY_ALL = 'spaces.all';
   const KEY_DEFAULT = 'spaces.default';
   const KEY_VIEWER = 'spaces.viewer';
+  const KEY_MAGIC_ALL_USER_SPACE = 'spaces.magicalluserspace';
 
   private $ids;
   private $phids;
@@ -100,6 +101,8 @@ final class PhabricatorSpacesNamespaceQuery
     // If the viewer has access to only one space, pretend spaces simply don't
     // exist.
     $spaces = self::getViewerSpaces($viewer);
+
+    unset($spaces[self::getMagicAllUsersSpace()->getPHID()]);
     return (count($spaces) > 1);
   }
 
@@ -119,6 +122,34 @@ final class PhabricatorSpacesNamespaceQuery
     return $spaces;
   }
 
+  // See the comment in PhabricatorCursorPagedPolicyAwareQuery's call of this
+  // function to understand what is going on here.
+  public static function getMagicAllUsersSpace() {
+    $cache = PhabricatorCaches::getRequestCache();
+    $cache_key = self::KEY_MAGIC_ALL_USER_SPACE;
+
+    $magic_space = $cache->getKey($cache_key, false);
+    if ($magic_space === false) {
+      $magic_space = null;
+
+      $spaces = self::getAllSpaces();
+      if (isset($spaces['PHID-SPCE-enba4g3iew4gdmka77kj'])) {
+	// PHID from phab.ccu
+        $magic_space = $spaces['PHID-SPCE-enba4g3iew4gdmka77kj'];
+      } else if (isset($spaces['PHID-SPCE-7r6by3wz32pwnzpw55yl'])) {
+	// PHID from test-phab.ccu
+        $magic_space = $spaces['PHID-SPCE-7r6by3wz32pwnzpw55yl'];
+      } else if (isset($spaces['PHID-SPCE-d4pabaw5r2yagpnsnuj7'])) {
+	// PHID from @daniels development environment
+        $magic_space = $spaces['PHID-SPCE-d4pabaw5r2yagpnsnuj7'];
+      }
+
+      $cache->setKey($cache_key, $magic_space);
+    }
+
+    return $magic_space;
+  }
+
   public static function getDefaultSpace() {
     $cache = PhabricatorCaches::getRequestCache();
     $cache_key = self::KEY_DEFAULT;

src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php

@@ -3169,6 +3169,39 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
           $include_null = true;
         }
       }
+
+      // So.
+      //
+      // We isolate clients in Phabricator by way of Spaces. Each client has
+      // their own Space. Since they can only see one Space (theirs), they
+      // don't have to ever interact with confusing Spaces UI, which is a
+      // plus: we isolate the pain to ourselves, and there's no way a client
+      // can ever accidentally expose their data to other clients.
+      //
+      // Unfortunately, clients also use wikis, and Phriction's access policy
+      // is that you must be able to view a document _and all its ancestors_
+      // by policy. Since every page is a descendent of the root page, we lose
+      // the ability to only have our clients see one space: the root page must
+      // be visible to every client, ergo in a space that they can see.
+      //
+      // We get around this with a blisteringly unpleasant hack. This function
+      // adds restrictive WHERE clauses to object lookup, ensuring that any
+      // objects loaded from storage are restricted to spaces the client can
+      // see.
+      //
+      // Here we special-case Phriction, allowing every user on the system to
+      // access objects in a magical hardcoded space. If this space is not
+      // visible to regular users via its normal policy, we will not show the
+      // the space in the UI anywhere. But we can use it as a get-out clause
+      // for opening up some objects of our choosing. Like Phriction pages.
+      //
+      // Modifying this code is not recommended.
+      $result_object_interface = $this->newResultObject();
+      if ($result_object_interface instanceof PhrictionContent ||
+          $result_object_interface instanceof PhrictionDocument) {
+        $all_user_space = PhabricatorSpacesNamespaceQuery::getMagicAllUsersSpace();
+	$space_phids[$all_user_space->getPHID()] = $all_user_space->getPHID();
+      }
     }
 
     // If we have additional explicit constraints, evaluate them now.