Make PhameBlogs respect policies

Summary: Adds "can view" and "can edit" policies to blogs. Replaces "bloggers" with "can join". This doesn't fully remove "bloggers" because I didn't want this to get too crazy/huge. Test Plan: Created, edited, deleted blogs. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T1373 Differential Revision: https://secure.phabricator.com/D3693
2024-11-26 08:42:41 +01:00 · 2012-10-15 14:49:52 -07:00 · 2012-10-15 14:49:52 -07:00 · dbcf2e44e8
commit dbcf2e44e8
parent 304599eab0
13 changed files with 184 additions and 132 deletions
--- a/resources/sql/patches/phamepolicy.sql
+++ b/resources/sql/patches/phamepolicy.sql
@ -0,0 +1,18 @@
+ALTER TABLE `{$NAMESPACE}_phame`.`phame_blog`
+  ADD `viewPolicy` varchar(64) COLLATE utf8_bin;
+
+UPDATE `{$NAMESPACE}_phame`.`phame_blog` SET viewPolicy = 'users'
+  WHERE viewPolicy IS NULL;
+
+ALTER TABLE `{$NAMESPACE}_phame`.`phame_blog`
+  ADD `editPolicy` varchar(64) COLLATE utf8_bin;
+
+UPDATE `{$NAMESPACE}_phame`.`phame_blog` SET editPolicy = 'users'
+  WHERE editPolicy IS NULL;
+
+ALTER TABLE `{$NAMESPACE}_phame`.`phame_blog`
+  ADD `joinPolicy` varchar(64) COLLATE utf8_bin;
+
+UPDATE `{$NAMESPACE}_phame`.`phame_blog` SET joinPolicy = 'users'
+  WHERE joinPolicy IS NULL;
+
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@ -2276,13 +2276,17 @@ phutil_register_library_map(array(
    'PhabricatorXHProfSampleListView' => 'AphrontView',
    'PhameAllBlogListController' => 'PhameBlogListBaseController',
    'PhameAllPostListController' => 'PhamePostListBaseController',
-    'PhameBlog' => 'PhameDAO',
+    'PhameBlog' =>
+    array(
+      0 => 'PhameDAO',
+      1 => 'PhabricatorPolicyInterface',
+    ),
    'PhameBlogDeleteController' => 'PhameController',
    'PhameBlogDetailView' => 'AphrontView',
    'PhameBlogEditController' => 'PhameController',
    'PhameBlogListBaseController' => 'PhameController',
    'PhameBlogListView' => 'AphrontView',
-    'PhameBlogQuery' => 'PhabricatorOffsetPagedQuery',
+    'PhameBlogQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
    'PhameBlogSkin' => 'AphrontView',
    'PhameBlogViewController' => 'PhameController',
    'PhameBloggerPostListController' => 'PhamePostListBaseController',
--- a/src/aphront/configuration/AphrontApplicationConfiguration.php
+++ b/src/aphront/configuration/AphrontApplicationConfiguration.php
@ -137,8 +137,18 @@ abstract class AphrontApplicationConfiguration {
    if ($host != id(new PhutilURI($base_uri))->getDomain() &&
        $host != id(new PhutilURI($prod_uri))->getDomain() &&
        $host != id(new PhutilURI($file_uri))->getDomain()) {
-      $blogs = id(new PhameBlogQuery())->withDomain($host)->execute();
-      $blog = reset($blogs);
+
+      try {
+        $blog = id(new PhameBlogQuery())
+          ->setViewer($request->getUser())
+          ->withDomain($host)
+          ->executeOne();
+      } catch (PhabricatorPolicyException $ex) {
+        throw new Exception(
+          "This blog is not visible to logged out users, so it can not be ".
+          "visited from a custom domain.");
+      }
+
      if (!$blog) {
        if ($prod_uri && $prod_uri != $base_uri) {
          $prod_str = ' or '.$prod_uri;
--- a/src/applications/phame/controller/blog/PhameBlogDeleteController.php
+++ b/src/applications/phame/controller/blog/PhameBlogDeleteController.php
@ -51,53 +51,23 @@ extends PhameController {
  }

  public function processRequest() {
-    $blogger_edge_type = PhabricatorEdgeConfig::TYPE_BLOG_HAS_BLOGGER;
-    $post_edge_type    = PhabricatorEdgeConfig::TYPE_BLOG_HAS_POST;
    $request = $this->getRequest();
    $user = $request->getUser();
-    $blog_phid         = $this->getBlogPHID();
-    $blogs             = id(new PhameBlogQuery())
-                           ->withPHIDs(array($blog_phid))
-                           ->execute();
-    $blog              = reset($blogs);
-    if (empty($blog)) {
+
+    $blog = id(new PhameBlogQuery())
+      ->setViewer($user)
+      ->withPHIDs(array($this->getBlogPHID()))
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->executeOne();
+
+    if (!$blog) {
      return new Aphront404Response();
    }

-    $phids      = array($blog_phid);
-    $edge_types = array(
-      PhabricatorEdgeConfig::TYPE_BLOG_HAS_BLOGGER,
-      PhabricatorEdgeConfig::TYPE_BLOG_HAS_POST,
-    );
-
-    $edges = id(new PhabricatorEdgeQuery())
-      ->withSourcePHIDs($phids)
-      ->withEdgeTypes($edge_types)
-      ->execute();
-
-    $blogger_edges = $edges[$blog_phid][$blogger_edge_type];
-    // TODO -- make this check use a policy
-    if (!isset($blogger_edges[$user->getPHID()]) &&
-        !$user->isAdmin()) {
-      return new Aphront403Response();
-    }
-
-    $edit_uri = $blog->getEditURI();
-
    if ($request->isFormPost()) {
-      $blogger_phids = array_keys($blogger_edges);
-      $post_edges    = $edges[$blog_phid][$post_edge_type];
-      $post_phids    = array_keys($post_edges);
-      $editor        = id(new PhabricatorEdgeEditor())
-        ->setActor($user);
-      foreach ($blogger_phids as $phid) {
-        $editor->removeEdge($blog_phid, $blogger_edge_type, $phid);
-      }
-      foreach ($post_phids as $phid) {
-        $editor->removeEdge($blog_phid, $post_edge_type, $phid);
-      }
-      $editor->save();
-
      $blog->delete();
      return id(new AphrontRedirectResponse())
        ->setURI('/phame/blog/?deleted');
@ -108,7 +78,7 @@ extends PhameController {
      ->setTitle('Delete blog?')
      ->appendChild('Really delete this blog? It will be gone forever.')
      ->addSubmitButton('Delete')
-      ->addCancelButton($edit_uri);
+      ->addCancelButton($blog->getEditURI());

    return id(new AphrontDialogResponse())->setDialog($dialog);
  }
--- a/src/applications/phame/controller/blog/PhameBlogEditController.php
+++ b/src/applications/phame/controller/blog/PhameBlogEditController.php
@ -76,28 +76,24 @@ final class PhameBlogEditController
  public function processRequest() {
    $request = $this->getRequest();
    $user = $request->getUser();
-    $e_name          = null;
-    $e_bloggers      = null;
+
+    $e_name          = true;
    $e_custom_domain = null;
    $errors          = array();

    if ($this->isBlogEdit()) {
-      $blogs = id(new PhameBlogQuery())
+      $blog = id(new PhameBlogQuery())
+        ->setViewer($user)
        ->withPHIDs(array($this->getBlogPHID()))
-        ->execute();
-      $blog = reset($blogs);
-      if (empty($blog)) {
+        ->requireCapabilities(
+          array(
+            PhabricatorPolicyCapability::CAN_EDIT
+          ))
+        ->executeOne();
+      if (!$blog) {
        return new Aphront404Response();
      }

-      $bloggers = $blog->loadBloggers()->getBloggers();
-
-      // TODO -- make this check use a policy
-      if (!isset($bloggers[$user->getPHID()]) &&
-          !$user->isAdmin()) {
-        return new Aphront403Response();
-      }
-      $blogger_tokens = mpull($bloggers, 'getFullName', 'getPHID');
      $submit_button  = 'Save Changes';
      $delete_button  = javelin_render_tag(
        'a',
@ -118,32 +114,13 @@ final class PhameBlogEditController
    }

    if ($request->isFormPost()) {
-      $saved         = true;
      $name          = $request->getStr('name');
      $description   = $request->getStr('description');
-      $blogger_arr   = $request->getArr('bloggers');
      $custom_domain = $request->getStr('custom_domain');
      $skin          = $request->getStr('skin');

-      if (empty($blogger_arr)) {
-        $error = 'Bloggers must be nonempty.';
-        if ($this->isBlogEdit()) {
-          $error .= ' To delete the blog, use the delete button.';
-        } else {
-          $error .= ' A blog cannot exist without bloggers.';
-        }
-        $e_bloggers = 'Required';
-        $errors[] = $error;
-      }
-      $new_bloggers = array_values($blogger_arr);
-      if ($this->isBlogEdit()) {
-        $old_bloggers = array_keys($blogger_tokens);
-      } else {
-        $old_bloggers = array();
-      }
-
      if (empty($name)) {
-        $errors[] = 'Name must be nonempty.';
+        $errors[] = 'You must give the blog a name.';
        $e_name   = 'Required';
      }
      $blog->setName($name);
@ -158,27 +135,19 @@ final class PhameBlogEditController
      }
      $blog->setSkin($skin);

-      if (empty($errors)) {
+      $blog->setViewPolicy($request->getStr('can_view'));
+      $blog->setEditPolicy($request->getStr('can_edit'));
+      $blog->setJoinPolicy($request->getStr('can_join'));
+
+      // Don't let users remove their ability to edit blogs.
+      PhabricatorPolicyFilter::mustRetainCapability(
+        $user,
+        $blog,
+        PhabricatorPolicyCapability::CAN_EDIT);
+
+      if (!$errors) {
        $blog->save();

-        $add_phids = $new_bloggers;
-        $rem_phids = array_diff($old_bloggers, $new_bloggers);
-        $editor    = new PhabricatorEdgeEditor();
-        $edge_type = PhabricatorEdgeConfig::TYPE_BLOG_HAS_BLOGGER;
-        $editor->setActor($user);
-        foreach ($add_phids as $phid) {
-          $editor->addEdge($blog->getPHID(), $edge_type, $phid);
-        }
-        foreach ($rem_phids as $phid) {
-          $editor->removeEdge($blog->getPHID(), $edge_type, $phid);
-        }
-        $editor->save();
-
-      } else {
-        $saved = false;
-      }
-
-      if ($saved) {
        $uri = new PhutilURI($blog->getViewURI());
        if ($this->isBlogEdit()) {
          $uri->setQueryParam('edit', true);
@ -197,6 +166,11 @@ final class PhameBlogEditController
      $panel->addButton($delete_button);
    }

+    $policies = id(new PhabricatorPolicyQuery())
+      ->setViewer($user)
+      ->setObject($blog)
+      ->execute();
+
    $form = id(new AphrontFormView())
      ->setUser($user)
      ->appendChild(
@ -212,18 +186,29 @@ final class PhameBlogEditController
        ->setLabel('Description')
        ->setName('description')
        ->setValue($blog->getDescription())
-        ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_TALL)
        ->setID('blog-description')
      )
      ->appendChild(
-        id(new AphrontFormTokenizerControl())
-        ->setLabel('Bloggers')
-        ->setName('bloggers')
-        ->setValue($blogger_tokens)
+        id(new AphrontFormPolicyControl())
          ->setUser($user)
-        ->setDatasource('/typeahead/common/users/')
-        ->setError($e_bloggers)
-      )
+          ->setCapability(PhabricatorPolicyCapability::CAN_VIEW)
+          ->setPolicyObject($blog)
+          ->setPolicies($policies)
+          ->setName('can_view'))
+      ->appendChild(
+        id(new AphrontFormPolicyControl())
+          ->setUser($user)
+          ->setCapability(PhabricatorPolicyCapability::CAN_EDIT)
+          ->setPolicyObject($blog)
+          ->setPolicies($policies)
+          ->setName('can_edit'))
+      ->appendChild(
+        id(new AphrontFormPolicyControl())
+          ->setUser($user)
+          ->setCapability(PhabricatorPolicyCapability::CAN_JOIN)
+          ->setPolicyObject($blog)
+          ->setPolicies($policies)
+          ->setName('can_join'))
      ->appendChild(
        id(new AphrontFormTextControl())
        ->setLabel('Custom Domain')
@ -250,7 +235,7 @@ final class PhameBlogEditController

    if ($errors) {
      $error_view = id(new AphrontErrorView())
-        ->setTitle('Errors saving blog.')
+        ->setTitle('Form Errors')
        ->setErrors($errors);
    } else {
      $error_view = null;
--- a/src/applications/phame/controller/blog/PhameBlogViewController.php
+++ b/src/applications/phame/controller/blog/PhameBlogViewController.php
@ -71,11 +71,10 @@ final class PhameBlogViewController
    $user      = $request->getUser();
    $blog_phid = $this->getBlogPHID();

-    $blogs = id(new PhameBlogQuery())
+    $blog = id(new PhameBlogQuery())
+      ->setViewer($user)
      ->withPHIDs(array($blog_phid))
-      ->execute();
-    $blog = reset($blogs);
-
+      ->executeOne();
    if (!$blog) {
      return new Aphront404Response();
    }
--- a/src/applications/phame/controller/blog/list/PhameAllBlogListController.php
+++ b/src/applications/phame/controller/blog/list/PhameAllBlogListController.php
@ -29,9 +29,13 @@ final class PhameAllBlogListController
  public function processRequest() {
    $user = $this->getRequest()->getUser();

+    $pager = id(new AphrontCursorPagerView())
+      ->readFromRequest($this->getRequest());
+
    $blogs = id(new PhameBlogQuery())
+      ->setViewer($user)
      ->needBloggers(true)
-      ->executeWithOffsetPager($this->getPager());
+      ->executeWithCursorPager($pager);
    $this->setBlogs($blogs);

    $page_title = 'All Blogs';
--- a/src/applications/phame/controller/blog/list/PhameUserBlogListController.php
+++ b/src/applications/phame/controller/blog/list/PhameUserBlogListController.php
@ -48,10 +48,14 @@ final class PhameUserBlogListController
      PhabricatorEdgeConfig::TYPE_BLOGGER_HAS_BLOG
    );

+    $pager = id(new AphrontCursorPagerView())
+      ->readFromRequest($this->getRequest());
+
    $blogs = id(new PhameBlogQuery())
+      ->setViewer($user)
      ->withPHIDs($blog_phids)
      ->needBloggers(true)
-      ->executeWithOffsetPager($this->getPager());
+      ->executeWithCursorPager($pager);

    $this->setBlogs($blogs);

--- a/src/applications/phame/controller/post/PhamePostEditController.php
+++ b/src/applications/phame/controller/post/PhamePostEditController.php
@ -400,6 +400,7 @@ final class PhamePostEditController
    }

    $blogs = id(new PhameBlogQuery())
+      ->setViewer($this->getRequest()->getUser())
      ->withPHIDs(array_keys($all_blogs_assoc))
      ->execute();
    $blogs = mpull($blogs, null, 'getPHID');
--- a/src/applications/phame/query/PhameBlogQuery.php
+++ b/src/applications/phame/query/PhameBlogQuery.php
@ -19,7 +19,7 @@
 /**
 * @group phame
 */
-final class PhameBlogQuery extends PhabricatorOffsetPagedQuery {
+final class PhameBlogQuery extends PhabricatorCursorPagedPolicyAwareQuery {

  private $phids;
  private $domain;
@ -40,7 +40,7 @@ final class PhameBlogQuery extends PhabricatorOffsetPagedQuery {
    return $this;
  }

-  public function execute() {
+  public function loadPage() {
    $table  = new PhameBlog();
    $conn_r = $table->establishConnection('r');

@ -102,22 +102,19 @@ final class PhameBlogQuery extends PhabricatorOffsetPagedQuery {
      $where[] = qsprintf(
        $conn_r,
        'phid IN (%Ls)',
-        $this->phids
-      );
+        $this->phids);
    }

    if ($this->domain) {
      $where[] = qsprintf(
        $conn_r,
        'domain = %s',
-        $this->domain
-      );
+        $this->domain);
    }

+    $where[] = $this->buildPagingClause($conn_r);
+
    return $this->formatWhereClause($where);
  }

-  private function buildOrderClause($conn_r) {
-    return 'ORDER BY id DESC';
-  }
 }
--- a/src/applications/phame/storage/PhameBlog.php
+++ b/src/applications/phame/storage/PhameBlog.php
@ -19,7 +19,7 @@
 /**
 * @group phame
 */
-final class PhameBlog extends PhameDAO {
+final class PhameBlog extends PhameDAO implements PhabricatorPolicyInterface {

  const SKIN_DEFAULT = 'PhabricatorBlogSkin';

@ -30,8 +30,9 @@ final class PhameBlog extends PhameDAO {
  protected $domain;
  protected $configData;
  protected $creatorPHID;
-
-  private $skin;
+  protected $viewPolicy;
+  protected $editPolicy;
+  protected $joinPolicy;

  private $bloggerPHIDs;
  private $bloggers;
@ -212,4 +213,54 @@ final class PhameBlog extends PhameDAO {
  public static function getRequestBlog() {
    return self::$requestBlog;
  }
+
+
+/* -(  PhabricatorPolicyInterface Implementation  )-------------------------- */
+
+
+  public function getCapabilities() {
+    return array(
+      PhabricatorPolicyCapability::CAN_VIEW,
+      PhabricatorPolicyCapability::CAN_EDIT,
+      PhabricatorPolicyCapability::CAN_JOIN,
+    );
+  }
+
+
+  public function getPolicy($capability) {
+    switch ($capability) {
+      case PhabricatorPolicyCapability::CAN_VIEW:
+        return $this->getViewPolicy();
+      case PhabricatorPolicyCapability::CAN_EDIT:
+        return $this->getEditPolicy();
+      case PhabricatorPolicyCapability::CAN_JOIN:
+        return $this->getJoinPolicy();
+    }
+  }
+
+  public function hasAutomaticCapability($capability, PhabricatorUser $user) {
+    $can_edit = PhabricatorPolicyCapability::CAN_EDIT;
+    $can_join = PhabricatorPolicyCapability::CAN_JOIN;
+
+    switch ($capability) {
+      case PhabricatorPolicyCapability::CAN_VIEW:
+        // Users who can edit or post to a blog can always view it.
+        if (PhabricatorPolicyFilter::hasCapability($user, $this, $can_edit)) {
+          return true;
+        }
+        if (PhabricatorPolicyFilter::hasCapability($user, $this, $can_join)) {
+          return true;
+        }
+        break;
+      case PhabricatorPolicyCapability::CAN_JOIN:
+        // Users who can edit a blog can always post to it.
+        if (PhabricatorPolicyFilter::hasCapability($user, $this, $can_edit)) {
+          return true;
+        }
+        break;
+    }
+
+    return false;
+  }
+
 }
--- a/src/applications/phame/view/PhameBlogListView.php
+++ b/src/applications/phame/view/PhameBlogListView.php
@ -78,7 +78,12 @@ final class PhameBlogListView extends AphrontView {
          'Custom Domain',
          $blog->getDomain());

-      if (isset($bloggers[$user->getPHID()])) {
+      $can_edit = PhabricatorPolicyFilter::hasCapability(
+        $user,
+        $blog,
+        PhabricatorPolicyCapability::CAN_EDIT);
+
+      if ($can_edit) {
        $item->addAttribute(
          phutil_render_tag(
            'a',
--- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php
+++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php
@ -1004,6 +1004,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList {
        'type'    => 'php',
        'name'    => $this->getPatchPath('ponder-mailkey-populate.php'),
      ),
+      'phamepolicy.sql' => array(
+        'type'    => 'sql',
+        'name'    => $this->getPatchPath('phamepolicy.sql'),
+      ),
    );
  }