From dbcf2e44e890ae9da7edd78da367d2cdadd15c90 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 15 Oct 2012 14:49:52 -0700 Subject: [PATCH] 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 --- resources/sql/patches/phamepolicy.sql | 18 +++ src/__phutil_library_map__.php | 8 +- .../AphrontApplicationConfiguration.php | 14 ++- .../blog/PhameBlogDeleteController.php | 58 +++------ .../blog/PhameBlogEditController.php | 115 ++++++++---------- .../blog/PhameBlogViewController.php | 7 +- .../blog/list/PhameAllBlogListController.php | 6 +- .../blog/list/PhameUserBlogListController.php | 6 +- .../post/PhamePostEditController.php | 1 + .../phame/query/PhameBlogQuery.php | 15 +-- src/applications/phame/storage/PhameBlog.php | 57 ++++++++- .../phame/view/PhameBlogListView.php | 7 +- .../patch/PhabricatorBuiltinPatchList.php | 4 + 13 files changed, 184 insertions(+), 132 deletions(-) create mode 100644 resources/sql/patches/phamepolicy.sql diff --git a/resources/sql/patches/phamepolicy.sql b/resources/sql/patches/phamepolicy.sql new file mode 100644 index 0000000000..d48bdf2223 --- /dev/null +++ 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; + diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index b2a7c296a8..2a4a80544d 100644 --- 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', diff --git a/src/aphront/configuration/AphrontApplicationConfiguration.php b/src/aphront/configuration/AphrontApplicationConfiguration.php index f3bbeaeaf4..a78555189f 100644 --- 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; diff --git a/src/applications/phame/controller/blog/PhameBlogDeleteController.php b/src/applications/phame/controller/blog/PhameBlogDeleteController.php index 2743cc4d06..1e1d22a99c 100644 --- 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)) { + $request = $this->getRequest(); + $user = $request->getUser(); + + $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); } diff --git a/src/applications/phame/controller/blog/PhameBlogEditController.php b/src/applications/phame/controller/blog/PhameBlogEditController.php index 4c0216887c..db5a4592ab 100644 --- a/src/applications/phame/controller/blog/PhameBlogEditController.php +++ b/src/applications/phame/controller/blog/PhameBlogEditController.php @@ -74,30 +74,26 @@ final class PhameBlogEditController } public function processRequest() { - $request = $this->getRequest(); - $user = $request->getUser(); - $e_name = null; - $e_bloggers = null; + $request = $this->getRequest(); + $user = $request->getUser(); + + $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) - ->setUser($user) - ->setDatasource('/typeahead/common/users/') - ->setError($e_bloggers) - ) + id(new AphrontFormPolicyControl()) + ->setUser($user) + ->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; diff --git a/src/applications/phame/controller/blog/PhameBlogViewController.php b/src/applications/phame/controller/blog/PhameBlogViewController.php index 9ad099ec5f..dcf89570b5 100644 --- 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(); } diff --git a/src/applications/phame/controller/blog/list/PhameAllBlogListController.php b/src/applications/phame/controller/blog/list/PhameAllBlogListController.php index 61bec22836..04010f4aa5 100644 --- 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'; diff --git a/src/applications/phame/controller/blog/list/PhameUserBlogListController.php b/src/applications/phame/controller/blog/list/PhameUserBlogListController.php index 7e97050b89..2059423607 100644 --- 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); diff --git a/src/applications/phame/controller/post/PhamePostEditController.php b/src/applications/phame/controller/post/PhamePostEditController.php index 013632bf9a..547e2fadf9 100644 --- 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'); diff --git a/src/applications/phame/query/PhameBlogQuery.php b/src/applications/phame/query/PhameBlogQuery.php index 31159d5ca6..ae58c28c1b 100644 --- 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'; - } } diff --git a/src/applications/phame/storage/PhameBlog.php b/src/applications/phame/storage/PhameBlog.php index cb516ae31e..83ac1f074b 100644 --- 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; + } + } diff --git a/src/applications/phame/view/PhameBlogListView.php b/src/applications/phame/view/PhameBlogListView.php index 6aacc1070e..0e345ccf9f 100644 --- 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', diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index 225af8be4d..d34b45535c 100644 --- 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'), + ), ); }