From ded641ae3270056f0710fca81b1f70c80cf0d6f7 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 14 Apr 2012 10:13:29 -0700 Subject: [PATCH] Add basic per-object privacy policies Summary: Provides a basic start for access policies. Objects expose various capabilities, like CAN_VIEW, CAN_EDIT, etc., and set a policy for each capability. We currently implement three policies, PUBLIC (anyone, including logged-out), USERS (any logged-in) and NOONE (nobody). There's also a way to provide automatic capability grants (e.g., the owner of an object can always see it, even if some capability is set to "NOONE"), but I'm not sure how great the implementation feels and it might change. Most of the code here is providing a primitive for efficient policy-aware list queries. The problem with doing queries naively is that you have to do crazy amounts of filtering, e.g. to show the user page 6, you need to filter at least 600 objects (and likely more) before you can figure out which ones are 500-600 for them. You can't just do "LIMIT 500, 100" because that might have only 50 results, or no results. Instead, the query looks like "WHERE id > last_visible_id", and then we fetch additional pages as necessary to satisfy the request. The general idea is that we move all data access to Query classes and have them do object filtering. The ID paging primitive allows efficient paging in most cases, and the executeOne() method provides a concise way to do policy checks for edit/view screens. We'll probably end up with mostly broader policy UIs or configuration-based policies, but there are at least a few cases for per-object privacy (e.g., marking tasks as "Security", and restricting things to the members of projects) so I figured we'd start with a flexible primitive and the simplify it in the UI where we can. Test Plan: Unit tests, played around in the UI with various policy settings. Reviewers: btrahan, vrana, jungejason Reviewed By: btrahan CC: aran Maniphest Tasks: T603 Differential Revision: https://secure.phabricator.com/D2210 --- src/__phutil_library_map__.php | 31 +++ ...AphrontDefaultApplicationConfiguration.php | 39 +++- .../list/PhabricatorPasteListController.php | 202 +++++++----------- .../paste/controller/list/__init__.php | 3 +- .../view/PhabricatorPasteViewController.php | 6 +- .../paste/controller/view/__init__.php | 1 + .../query/paste/PhabricatorPasteQuery.php | 73 +++++++ .../paste/query/paste/__init__.php | 15 ++ .../paste/storage/paste/PhabricatorPaste.php | 17 +- .../paste/storage/paste/__init__.php | 3 + .../__tests__/PhabricatorPolicyTestCase.php | 138 ++++++++++++ .../__tests__/PhabricatorPolicyTestObject.php | 57 +++++ .../__tests__/PhabricatorPolicyTestQuery.php | 40 ++++ .../policy/__tests__/__init__.php | 21 ++ .../base/PhabricatorPolicyConstants.php | 21 ++ .../policy/constants/base/__init__.php | 10 + .../PhabricatorPolicyCapability.php | 23 ++ .../policy/constants/capability/__init__.php | 12 ++ .../constants/policy/PhabricatorPolicies.php | 25 +++ .../policy/constants/policy/__init__.php | 12 ++ .../base/PhabricatorPolicyException.php | 21 ++ .../policy/exception/base/__init__.php | 10 + .../filter/policy/PhabricatorPolicyFilter.php | 116 ++++++++++ .../policy/filter/policy/__init__.php | 15 ++ .../policy/PhabricatorPolicyInterface.php | 25 +++ .../policy/interface/policy/__init__.php | 10 + .../query/base/PhabricatorQuery.php | 3 + .../policy/base/PhabricatorPolicyQuery.php | 96 +++++++++ .../query/policy/base/__init__.php | 16 ++ .../idpaged/PhabricatorIDPagedPolicyQuery.php | 129 +++++++++++ .../query/policy/idpaged/__init__.php | 15 ++ .../control/idpager/AphrontIDPagerView.php | 142 ++++++++++++ src/view/control/idpager/__init__.php | 14 ++ .../policy/AphrontFormPolicyControl.php | 52 +++++ src/view/form/control/policy/__init__.php | 14 ++ webroot/rsrc/css/aphront/dialog-view.css | 4 + 36 files changed, 1297 insertions(+), 134 deletions(-) create mode 100644 src/applications/paste/query/paste/PhabricatorPasteQuery.php create mode 100644 src/applications/paste/query/paste/__init__.php create mode 100644 src/applications/policy/__tests__/PhabricatorPolicyTestCase.php create mode 100644 src/applications/policy/__tests__/PhabricatorPolicyTestObject.php create mode 100644 src/applications/policy/__tests__/PhabricatorPolicyTestQuery.php create mode 100644 src/applications/policy/__tests__/__init__.php create mode 100644 src/applications/policy/constants/base/PhabricatorPolicyConstants.php create mode 100644 src/applications/policy/constants/base/__init__.php create mode 100644 src/applications/policy/constants/capability/PhabricatorPolicyCapability.php create mode 100644 src/applications/policy/constants/capability/__init__.php create mode 100644 src/applications/policy/constants/policy/PhabricatorPolicies.php create mode 100644 src/applications/policy/constants/policy/__init__.php create mode 100644 src/applications/policy/exception/base/PhabricatorPolicyException.php create mode 100644 src/applications/policy/exception/base/__init__.php create mode 100644 src/applications/policy/filter/policy/PhabricatorPolicyFilter.php create mode 100644 src/applications/policy/filter/policy/__init__.php create mode 100644 src/applications/policy/interface/policy/PhabricatorPolicyInterface.php create mode 100644 src/applications/policy/interface/policy/__init__.php create mode 100644 src/infrastructure/query/policy/base/PhabricatorPolicyQuery.php create mode 100644 src/infrastructure/query/policy/base/__init__.php create mode 100644 src/infrastructure/query/policy/idpaged/PhabricatorIDPagedPolicyQuery.php create mode 100644 src/infrastructure/query/policy/idpaged/__init__.php create mode 100644 src/view/control/idpager/AphrontIDPagerView.php create mode 100644 src/view/control/idpager/__init__.php create mode 100644 src/view/form/control/policy/AphrontFormPolicyControl.php create mode 100644 src/view/form/control/policy/__init__.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 8934e7f9a1..dfbaedd6bb 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -39,6 +39,7 @@ phutil_register_library_map(array( 'AphrontFormLayoutView' => 'view/form/layout', 'AphrontFormMarkupControl' => 'view/form/control/markup', 'AphrontFormPasswordControl' => 'view/form/control/password', + 'AphrontFormPolicyControl' => 'view/form/control/policy', 'AphrontFormRadioButtonControl' => 'view/form/control/radio', 'AphrontFormRecaptchaControl' => 'view/form/control/recaptcha', 'AphrontFormSelectControl' => 'view/form/control/select', @@ -54,6 +55,7 @@ phutil_register_library_map(array( 'AphrontHeadsupActionListView' => 'view/layout/headsup/actionlist', 'AphrontHeadsupActionView' => 'view/layout/headsup/action', 'AphrontHeadsupView' => 'view/layout/headsup/panel', + 'AphrontIDPagerView' => 'view/control/idpager', 'AphrontIsolatedDatabaseConnection' => 'storage/connection/isolated', 'AphrontIsolatedDatabaseConnectionTestCase' => 'storage/connection/isolated/__tests__', 'AphrontIsolatedHTTPSink' => 'aphront/sink/test', @@ -633,6 +635,7 @@ phutil_register_library_map(array( 'PhabricatorHash' => 'infrastructure/util/hash', 'PhabricatorHelpController' => 'applications/help/controller/base', 'PhabricatorHelpKeyboardShortcutController' => 'applications/help/controller/keyboardshortcut', + 'PhabricatorIDPagedPolicyQuery' => 'infrastructure/query/policy/idpaged', 'PhabricatorIRCBot' => 'infrastructure/daemon/irc/bot', 'PhabricatorIRCDifferentialNotificationHandler' => 'infrastructure/daemon/irc/handler/differentialnotification', 'PhabricatorIRCHandler' => 'infrastructure/daemon/irc/handler/base', @@ -745,12 +748,23 @@ phutil_register_library_map(array( 'PhabricatorPasteController' => 'applications/paste/controller/base', 'PhabricatorPasteDAO' => 'applications/paste/storage/base', 'PhabricatorPasteListController' => 'applications/paste/controller/list', + 'PhabricatorPasteQuery' => 'applications/paste/query/paste', 'PhabricatorPasteViewController' => 'applications/paste/controller/view', 'PhabricatorPeopleController' => 'applications/people/controller/base', 'PhabricatorPeopleEditController' => 'applications/people/controller/edit', 'PhabricatorPeopleListController' => 'applications/people/controller/list', 'PhabricatorPeopleLogsController' => 'applications/people/controller/logs', 'PhabricatorPeopleProfileController' => 'applications/people/controller/profile', + 'PhabricatorPolicies' => 'applications/policy/constants/policy', + 'PhabricatorPolicyCapability' => 'applications/policy/constants/capability', + 'PhabricatorPolicyConstants' => 'applications/policy/constants/base', + 'PhabricatorPolicyException' => 'applications/policy/exception/base', + 'PhabricatorPolicyFilter' => 'applications/policy/filter/policy', + 'PhabricatorPolicyInterface' => 'applications/policy/interface/policy', + 'PhabricatorPolicyQuery' => 'infrastructure/query/policy/base', + 'PhabricatorPolicyTestCase' => 'applications/policy/__tests__', + 'PhabricatorPolicyTestObject' => 'applications/policy/__tests__', + 'PhabricatorPolicyTestQuery' => 'applications/policy/__tests__', 'PhabricatorProfileHeaderView' => 'view/layout/profileheader', 'PhabricatorProject' => 'applications/project/storage/project', 'PhabricatorProjectAffiliation' => 'applications/project/storage/affiliation', @@ -1023,6 +1037,7 @@ phutil_register_library_map(array( 'AphrontFormLayoutView' => 'AphrontView', 'AphrontFormMarkupControl' => 'AphrontFormControl', 'AphrontFormPasswordControl' => 'AphrontFormControl', + 'AphrontFormPolicyControl' => 'AphrontFormControl', 'AphrontFormRadioButtonControl' => 'AphrontFormControl', 'AphrontFormRecaptchaControl' => 'AphrontFormControl', 'AphrontFormSelectControl' => 'AphrontFormControl', @@ -1037,6 +1052,7 @@ phutil_register_library_map(array( 'AphrontHeadsupActionListView' => 'AphrontView', 'AphrontHeadsupActionView' => 'AphrontView', 'AphrontHeadsupView' => 'AphrontView', + 'AphrontIDPagerView' => 'AphrontView', 'AphrontIsolatedDatabaseConnection' => 'AphrontDatabaseConnection', 'AphrontIsolatedDatabaseConnectionTestCase' => 'PhabricatorTestCase', 'AphrontIsolatedHTTPSink' => 'AphrontHTTPSink', @@ -1502,6 +1518,7 @@ phutil_register_library_map(array( 'PhabricatorGoodForNothingWorker' => 'PhabricatorWorker', 'PhabricatorHelpController' => 'PhabricatorController', 'PhabricatorHelpKeyboardShortcutController' => 'PhabricatorHelpController', + 'PhabricatorIDPagedPolicyQuery' => 'PhabricatorPolicyQuery', 'PhabricatorIRCBot' => 'PhabricatorDaemon', 'PhabricatorIRCDifferentialNotificationHandler' => 'PhabricatorIRCHandler', 'PhabricatorIRCLogHandler' => 'PhabricatorIRCHandler', @@ -1594,12 +1611,18 @@ phutil_register_library_map(array( 'PhabricatorPasteController' => 'PhabricatorController', 'PhabricatorPasteDAO' => 'PhabricatorLiskDAO', 'PhabricatorPasteListController' => 'PhabricatorPasteController', + 'PhabricatorPasteQuery' => 'PhabricatorIDPagedPolicyQuery', 'PhabricatorPasteViewController' => 'PhabricatorPasteController', 'PhabricatorPeopleController' => 'PhabricatorController', 'PhabricatorPeopleEditController' => 'PhabricatorPeopleController', 'PhabricatorPeopleListController' => 'PhabricatorPeopleController', 'PhabricatorPeopleLogsController' => 'PhabricatorPeopleController', 'PhabricatorPeopleProfileController' => 'PhabricatorPeopleController', + 'PhabricatorPolicies' => 'PhabricatorPolicyConstants', + 'PhabricatorPolicyCapability' => 'PhabricatorPolicyConstants', + 'PhabricatorPolicyQuery' => 'PhabricatorQuery', + 'PhabricatorPolicyTestCase' => 'PhabricatorTestCase', + 'PhabricatorPolicyTestQuery' => 'PhabricatorPolicyQuery', 'PhabricatorProfileHeaderView' => 'AphrontView', 'PhabricatorProject' => 'PhabricatorProjectDAO', 'PhabricatorProjectAffiliation' => 'PhabricatorProjectDAO', @@ -1804,5 +1827,13 @@ phutil_register_library_map(array( array( 0 => 'PhabricatorInlineCommentInterface', ), + 'PhabricatorPaste' => + array( + 0 => 'PhabricatorPolicyInterface', + ), + 'PhabricatorPolicyTestObject' => + array( + 0 => 'PhabricatorPolicyInterface', + ), ), )); diff --git a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php index e74a089f8a..8940263ccd 100644 --- a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php @@ -468,11 +468,7 @@ class AphrontDefaultApplicationConfiguration public function handleException(Exception $ex) { - // Always log the unhandled exception. - phlog($ex); - - $class = phutil_escape_html(get_class($ex)); - $message = phutil_escape_html($ex->getMessage()); + $is_serious = PhabricatorEnv::getEnvConfig('phabricator.serious-business'); $user = $this->getRequest()->getUser(); if (!$user) { @@ -480,6 +476,39 @@ class AphrontDefaultApplicationConfiguration $user = new PhabricatorUser(); } + if ($ex instanceof PhabricatorPolicyException) { + $content = + '
'. + phutil_escape_html($ex->getMessage()). + '
'; + + $dialog = new AphrontDialogView(); + $dialog + ->setTitle( + $is_serious + ? 'Access Denied' + : "You Shall Not Pass") + ->setClass('aphront-access-dialog') + ->setUser($user) + ->appendChild($content); + + if ($this->getRequest()->isAjax()) { + $dialog->addCancelButton('/', 'Close'); + } else { + $dialog->addCancelButton('/', $is_serious ? 'OK' : 'Away With Thee'); + } + + $response = new AphrontDialogResponse(); + $response->setDialog($dialog); + return $response; + } + + // Always log the unhandled exception. + phlog($ex); + + $class = phutil_escape_html(get_class($ex)); + $message = phutil_escape_html($ex->getMessage()); + if (PhabricatorEnv::getEnvConfig('phabricator.show-stack-traces')) { $trace = $this->renderStackTrace($ex->getTrace(), $user); } else { diff --git a/src/applications/paste/controller/list/PhabricatorPasteListController.php b/src/applications/paste/controller/list/PhabricatorPasteListController.php index 5ef111e61b..f4a2ff1ea5 100644 --- a/src/applications/paste/controller/list/PhabricatorPasteListController.php +++ b/src/applications/paste/controller/list/PhabricatorPasteListController.php @@ -24,10 +24,6 @@ final class PhabricatorPasteListController extends PhabricatorPasteController { private $paste; private $pasteText; - private $offset; - private $pageSize; - private $author; - private function setFilter($filter) { $this->filter = $filter; return $this; @@ -40,6 +36,7 @@ final class PhabricatorPasteListController extends PhabricatorPasteController { $this->errorView = $error_view; return $this; } + private function getErrorView() { return $this->errorView; } @@ -68,40 +65,19 @@ final class PhabricatorPasteListController extends PhabricatorPasteController { return $this->pasteText; } - private function setOffset($offset) { - $this->offset = $offset; - return $this; - } - private function getOffset() { - return $this->offset; - } - - private function setPageSize($page_size) { - $this->pageSize = $page_size; - return $this; - } - private function getPageSize() { - return $this->pageSize; - } - - private function setAuthor($author) { - $this->author = $author; - return $this; - } - private function getAuthor() { - return $this->author; - } - public function willProcessRequest(array $data) { $this->setFilter(idx($data, 'filter', 'create')); } public function processRequest() { - $request = $this->getRequest(); $user = $request->getUser(); - $paste_list = array(); - $pager = null; + + $pager = new AphrontIDPagerView(); + $pager->readFromRequest($request); + + $query = new PhabricatorPasteQuery(); + $query->setViewer($user); switch ($this->getFilter()) { case 'create': @@ -111,22 +87,18 @@ final class PhabricatorPasteListController extends PhabricatorPasteController { if ($created_paste_redirect) { return $created_paste_redirect; } - // if we didn't succeed or we weren't trying, load just a few - // recent pastes with NO pagination - $this->setOffset(0); - $this->setPageSize(10); - list($paste_list, $pager) = $this->loadPasteList(); - break; + $query->setLimit(10); + $paste_list = $query->execute(); + + $pager = null; + break; case 'my': - $this->setAuthor($user->getPHID()); - $this->setOffset($request->getInt('page', 0)); - list($paste_list, $pager) = $this->loadPasteList(); + $query->withAuthorPHIDs(array($user->getPHID())); + $paste_list = $query->executeWithPager($pager); break; - case 'all': - $this->setOffset($request->getInt('page', 0)); - list($paste_list, $pager) = $this->loadPasteList(); + $paste_list = $query->executeWithPager($pager); break; } @@ -171,24 +143,19 @@ final class PhabricatorPasteListController extends PhabricatorPasteController { ), 'See all Pastes'); $header = "Recent Pastes · {$see_all}"; - $side_nav->appendChild($this->renderPasteList($paste_list, - $header, - $pager = null)); break; case 'my': $header = 'Your Pastes'; - $side_nav->appendChild($this->renderPasteList($paste_list, - $header, - $pager)); break; case 'all': $header = 'All Pastes'; - $side_nav->appendChild($this->renderPasteList($paste_list, - $header, - $pager)); break; } + $side_nav->appendChild( + $this->renderPasteList($paste_list, $header, $pager)); + + return $this->buildStandardPageResponse( $side_nav, array( @@ -282,30 +249,66 @@ final class PhabricatorPasteListController extends PhabricatorPasteController { $this->setPaste($new_paste); } - private function loadPasteList() { + private function renderCreatePaste() { $request = $this->getRequest(); + $user = $request->getUser(); - $pager = new AphrontPagerView(); - $pager->setOffset($this->getOffset()); - if ($this->getPageSize()) { - $pager->setPageSize($this->getPageSize()); - } + $new_paste = $this->getPaste(); - if ($this->getAuthor()) { - $pastes = id(new PhabricatorPaste())->loadAllWhere( - 'authorPHID = %s ORDER BY id DESC LIMIT %d, %d', - $this->getAuthor(), - $pager->getOffset(), - $pager->getPageSize() + 1); - } else { - $pastes = id(new PhabricatorPaste())->loadAllWhere( - '1 = 1 ORDER BY id DESC LIMIT %d, %d', - $pager->getOffset(), - $pager->getPageSize() + 1); - } + $form = new AphrontFormView(); - $pastes = $pager->sliceResults($pastes); - $pager->setURI($request->getRequestURI(), 'page'); + $available_languages = PhabricatorEnv::getEnvConfig( + 'pygments.dropdown-choices'); + asort($available_languages); + $language_select = id(new AphrontFormSelectControl()) + ->setLabel('Language') + ->setName('language') + ->setValue($new_paste->getLanguage()) + ->setOptions($available_languages); + + $form + ->setUser($user) + ->setAction($request->getRequestURI()->getPath()) + ->addHiddenInput('parent', $new_paste->getParentPHID()) + ->appendChild( + id(new AphrontFormTextControl()) + ->setLabel('Title') + ->setValue($new_paste->getTitle()) + ->setName('title')) + ->appendChild($language_select) + ->appendChild( + id(new AphrontFormTextAreaControl()) + ->setLabel('Text') + ->setError($this->getErrorText()) + ->setValue($this->getPasteText()) + ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_TALL) + ->setName('text')) + + /* TODO: Doesn't have any useful options yet. + ->appendChild( + id(new AphrontFormPolicyControl()) + ->setLabel('Visible To') + ->setUser($user) + ->setValue( + $new_paste->getPolicy(PhabricatorPolicyCapability::CAN_VIEW)) + ->setName('policy')) + */ + + ->appendChild( + id(new AphrontFormSubmitControl()) + ->addCancelButton('/paste/') + ->setValue('Create Paste')); + + $create_panel = new AphrontPanelView(); + $create_panel->setWidth(AphrontPanelView::WIDTH_FULL); + $create_panel->setHeader('Create a Paste'); + $create_panel->appendChild($form); + + return $create_panel; + } + + private function renderPasteList(array $pastes, $header, $pager) { + assert_instances_of($pastes, 'PhabricatorPaste'); $phids = mpull($pastes, 'getAuthorPHID'); $handles = array(); @@ -318,8 +321,7 @@ final class PhabricatorPasteListController extends PhabricatorPasteController { if ($phids) { $files = id(new PhabricatorFile())->loadAllWhere( 'phid in (%Ls)', - $phids - ); + $phids); if ($files) { $file_uris = mpull($files, 'getBestURI', 'getPHID'); } @@ -363,59 +365,7 @@ final class PhabricatorPasteListController extends PhabricatorPasteController { ); } - return array($paste_list_rows, $pager); - } - private function renderCreatePaste() { - $request = $this->getRequest(); - $user = $request->getUser(); - - $new_paste = $this->getPaste(); - - $form = new AphrontFormView(); - - $available_languages = PhabricatorEnv::getEnvConfig( - 'pygments.dropdown-choices'); - asort($available_languages); - $language_select = id(new AphrontFormSelectControl()) - ->setLabel('Language') - ->setName('language') - ->setValue($new_paste->getLanguage()) - ->setOptions($available_languages); - - $form - ->setUser($user) - ->setAction($request->getRequestURI()->getPath()) - ->addHiddenInput('parent', $new_paste->getParentPHID()) - ->appendChild( - id(new AphrontFormTextControl()) - ->setLabel('Title') - ->setValue($new_paste->getTitle()) - ->setName('title')) - ->appendChild($language_select) - ->appendChild( - id(new AphrontFormTextAreaControl()) - ->setLabel('Text') - ->setError($this->getErrorText()) - ->setValue($this->getPasteText()) - ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_TALL) - ->setName('text')) - ->appendChild( - id(new AphrontFormSubmitControl()) - ->addCancelButton('/paste/') - ->setValue('Create Paste')); - - $create_panel = new AphrontPanelView(); - $create_panel->setWidth(AphrontPanelView::WIDTH_FULL); - $create_panel->setHeader('Create a Paste'); - $create_panel->appendChild($form); - - return $create_panel; - } - - private function renderPasteList($paste_list_rows, - $header, - $pager = null) { $table = new AphrontTableView($paste_list_rows); $table->setHeaders( array( diff --git a/src/applications/paste/controller/list/__init__.php b/src/applications/paste/controller/list/__init__.php index 40f73b234f..7c46b6da12 100644 --- a/src/applications/paste/controller/list/__init__.php +++ b/src/applications/paste/controller/list/__init__.php @@ -9,10 +9,11 @@ phutil_require_module('phabricator', 'aphront/response/redirect'); phutil_require_module('phabricator', 'applications/files/storage/file'); phutil_require_module('phabricator', 'applications/paste/controller/base'); +phutil_require_module('phabricator', 'applications/paste/query/paste'); phutil_require_module('phabricator', 'applications/paste/storage/paste'); phutil_require_module('phabricator', 'applications/phid/handle/data'); phutil_require_module('phabricator', 'infrastructure/env'); -phutil_require_module('phabricator', 'view/control/pager'); +phutil_require_module('phabricator', 'view/control/idpager'); phutil_require_module('phabricator', 'view/control/table'); phutil_require_module('phabricator', 'view/form/base'); phutil_require_module('phabricator', 'view/form/control/select'); diff --git a/src/applications/paste/controller/view/PhabricatorPasteViewController.php b/src/applications/paste/controller/view/PhabricatorPasteViewController.php index 26ccb1d7b9..9b3cdc44e2 100644 --- a/src/applications/paste/controller/view/PhabricatorPasteViewController.php +++ b/src/applications/paste/controller/view/PhabricatorPasteViewController.php @@ -29,7 +29,11 @@ final class PhabricatorPasteViewController extends PhabricatorPasteController { $request = $this->getRequest(); $user = $request->getUser(); - $paste = id(new PhabricatorPaste())->load($this->id); + $paste = id(new PhabricatorPasteQuery()) + ->setViewer($user) + ->withPasteIDs(array($this->id)) + ->executeOne(); + if (!$paste) { return new Aphront404Response(); } diff --git a/src/applications/paste/controller/view/__init__.php b/src/applications/paste/controller/view/__init__.php index 57ad86603b..635b80b2ca 100644 --- a/src/applications/paste/controller/view/__init__.php +++ b/src/applications/paste/controller/view/__init__.php @@ -11,6 +11,7 @@ phutil_require_module('phabricator', 'aphront/response/404'); phutil_require_module('phabricator', 'applications/files/storage/file'); phutil_require_module('phabricator', 'applications/markup/syntax'); phutil_require_module('phabricator', 'applications/paste/controller/base'); +phutil_require_module('phabricator', 'applications/paste/query/paste'); phutil_require_module('phabricator', 'applications/paste/storage/paste'); phutil_require_module('phabricator', 'infrastructure/celerity/api'); phutil_require_module('phabricator', 'infrastructure/javelin/api'); diff --git a/src/applications/paste/query/paste/PhabricatorPasteQuery.php b/src/applications/paste/query/paste/PhabricatorPasteQuery.php new file mode 100644 index 0000000000..4a8d12a87e --- /dev/null +++ b/src/applications/paste/query/paste/PhabricatorPasteQuery.php @@ -0,0 +1,73 @@ +pasteIDs = $ids; + return $this; + } + + public function withAuthorPHIDs(array $phids) { + $this->authorPHIDs = $phids; + return $this; + } + + public function loadPage() { + $table = new PhabricatorPaste(); + $conn_r = $table->establishConnection('r'); + + $data = queryfx_all( + $conn_r, + 'SELECT paste.* FROM %T paste %Q %Q %Q', + $table->getTableName(), + $this->buildWhereClause($conn_r), + $this->buildOrderClause($conn_r), + $this->buildLimitClause($conn_r)); + + $results = $table->loadAllFromArray($data); + + return $this->processResults($results); + } + + protected function buildWhereClause($conn_r) { + $where = array(); + + $where[] = $this->buildPagingClause($conn_r); + + if ($this->pasteIDs) { + $where[] = qsprintf( + $conn_r, + 'id IN (%Ls)', + $this->pasteIDs); + } + + if ($this->authorPHIDs) { + $where[] = qsprintf( + $conn_r, + 'authorPHID IN (%Ls)', + $this->authorPHIDs); + } + + return $this->formatWhereClause($where); + } + +} diff --git a/src/applications/paste/query/paste/__init__.php b/src/applications/paste/query/paste/__init__.php new file mode 100644 index 0000000000..56f2397fde --- /dev/null +++ b/src/applications/paste/query/paste/__init__.php @@ -0,0 +1,15 @@ +getPHID() == $this->getAuthorPHID()); + } + } diff --git a/src/applications/paste/storage/paste/__init__.php b/src/applications/paste/storage/paste/__init__.php index ef2ac38a2f..815bfa13a2 100644 --- a/src/applications/paste/storage/paste/__init__.php +++ b/src/applications/paste/storage/paste/__init__.php @@ -9,6 +9,9 @@ phutil_require_module('phabricator', 'applications/paste/storage/base'); phutil_require_module('phabricator', 'applications/phid/constants'); phutil_require_module('phabricator', 'applications/phid/storage/phid'); +phutil_require_module('phabricator', 'applications/policy/constants/capability'); +phutil_require_module('phabricator', 'applications/policy/constants/policy'); +phutil_require_module('phabricator', 'applications/policy/interface/policy'); phutil_require_source('PhabricatorPaste.php'); diff --git a/src/applications/policy/__tests__/PhabricatorPolicyTestCase.php b/src/applications/policy/__tests__/PhabricatorPolicyTestCase.php new file mode 100644 index 0000000000..7f8b3c69de --- /dev/null +++ b/src/applications/policy/__tests__/PhabricatorPolicyTestCase.php @@ -0,0 +1,138 @@ +setCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + )); + $object->setPolicies( + array( + PhabricatorPolicyCapability::CAN_VIEW => + PhabricatorPolicies::POLICY_PUBLIC, + )); + + $query = new PhabricatorPolicyTestQuery(); + $query->setResults(array($object)); + $query->setViewer($viewer); + $result = $query->executeOne(); + + $this->assertEqual($object, $result, 'Policy: Public'); + } + + + /** + * Verify that any logged-in user can view an object with POLICY_USER, but + * logged-out users can not. + */ + public function testUsersPolicy() { + $viewer = new PhabricatorUser(); + + $object = new PhabricatorPolicyTestObject(); + $object->setCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + )); + $object->setPolicies( + array( + PhabricatorPolicyCapability::CAN_VIEW => + PhabricatorPolicies::POLICY_USER, + )); + + $query = new PhabricatorPolicyTestQuery(); + $query->setResults(array($object)); + $query->setViewer($viewer); + + $caught = null; + try { + $query->executeOne(); + } catch (PhabricatorPolicyException $ex) { + $caught = $ex; + } + + $this->assertEqual( + true, + ($caught instanceof PhabricatorPolicyException), + 'Policy: Users rejects logged out users.'); + + $viewer->setPHID(1); + $result = $query->executeOne(); + $this->assertEqual( + $object, + $result, + 'Policy: Users'); + } + + + /** + * Verify that no one can view an object with POLICY_NOONE. + */ + public function testNoOnePolicy() { + $viewer = new PhabricatorUser(); + + $object = new PhabricatorPolicyTestObject(); + $object->setCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + )); + $object->setPolicies( + array( + PhabricatorPolicyCapability::CAN_VIEW => + PhabricatorPolicies::POLICY_NOONE, + )); + + $query = new PhabricatorPolicyTestQuery(); + $query->setResults(array($object)); + $query->setViewer($viewer); + + $caught = null; + try { + $query->executeOne(); + } catch (PhabricatorPolicyException $ex) { + $caught = $ex; + } + + $this->assertEqual( + true, + ($caught instanceof PhabricatorPolicyException), + 'Policy: No One rejects logged out users.'); + + $viewer->setPHID(1); + + $caught = null; + try { + $query->executeOne(); + } catch (PhabricatorPolicyException $ex) { + $caught = $ex; + } + + $this->assertEqual( + true, + ($caught instanceof PhabricatorPolicyException), + 'Policy: No One rejects logged-in users.'); + } + +} diff --git a/src/applications/policy/__tests__/PhabricatorPolicyTestObject.php b/src/applications/policy/__tests__/PhabricatorPolicyTestObject.php new file mode 100644 index 0000000000..d9f310ff0d --- /dev/null +++ b/src/applications/policy/__tests__/PhabricatorPolicyTestObject.php @@ -0,0 +1,57 @@ +capabilities; + } + + public function getPolicy($capability) { + return idx($this->policies, $capability); + } + + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + $auto = idx($this->automaticCapabilities, $capability, array()); + return idx($auto, $viewer->getPHID()); + } + + public function setCapabilities(array $capabilities) { + $this->capabilities = $capabilities; + return $this; + } + + public function setPolicies(array $policy_map) { + $this->policies = $policy_map; + return $this; + } + + public function setAutomaticCapabilities(array $auto_map) { + $this->automaticCapabilities = $auto_map; + return $this; + } + +} diff --git a/src/applications/policy/__tests__/PhabricatorPolicyTestQuery.php b/src/applications/policy/__tests__/PhabricatorPolicyTestQuery.php new file mode 100644 index 0000000000..e1389782c2 --- /dev/null +++ b/src/applications/policy/__tests__/PhabricatorPolicyTestQuery.php @@ -0,0 +1,40 @@ +results = $results; + return $this; + } + + public function loadPage() { + return $this->results; + } + + public function nextPage(array $page) { + return null; + } + +} diff --git a/src/applications/policy/__tests__/__init__.php b/src/applications/policy/__tests__/__init__.php new file mode 100644 index 0000000000..48d2be7ea0 --- /dev/null +++ b/src/applications/policy/__tests__/__init__.php @@ -0,0 +1,21 @@ +viewer = $user; + return $this; + } + + public function setCapability($capability) { + $this->capability = $capability; + return $this; + } + + public function raisePolicyExceptions($raise) { + $this->raisePolicyExceptions = $raise; + return $this; + } + + public function apply(array $objects) { + assert_instances_of($objects, 'PhabricatorPolicyInterface'); + + $viewer = $this->viewer; + $capability = $this->capability; + + if (!$viewer || !$capability) { + throw new Exception( + 'Call setViewer() and setCapability() before apply()!'); + } + + $filtered = array(); + + foreach ($objects as $key => $object) { + $object_capabilities = $object->getCapabilities(); + + if (!in_array($capability, $object_capabilities)) { + throw new Exception( + "Testing for capability '{$capability}' on an object which does not ". + "have that capability!"); + } + + if ($object->hasAutomaticCapability($capability, $this->viewer)) { + $filtered[$key] = $object; + continue; + } + + $policy = $object->getPolicy($capability); + + switch ($policy) { + case PhabricatorPolicies::POLICY_PUBLIC: + $filtered[$key] = $object; + break; + case PhabricatorPolicies::POLICY_USER: + if ($viewer->getPHID()) { + $filtered[$key] = $object; + } else { + $this->rejectObject($object, $policy); + } + break; + case PhabricatorPolicies::POLICY_NOONE: + $this->rejectObject($object, $policy); + break; + default: + throw new Exception("Object has unknown policy '{$policy}'!"); + } + } + + return $filtered; + } + + private function rejectObject($object, $policy) { + if (!$this->raisePolicyExceptions) { + return; + } + + $message = "You do not have permission to view this object."; + + switch ($policy) { + case PhabricatorPolicies::POLICY_PUBLIC: + $who = "This is curious, since anyone can view the object."; + break; + case PhabricatorPolicies::POLICY_USER: + $who = "To view this object, you must be logged in."; + break; + case PhabricatorPolicies::POLICY_NOONE: + $who = "No one can view this object."; + break; + default: + $who = "It is unclear who can view this object."; + break; + } + + throw new PhabricatorPolicyException("{$message} {$who}"); + } +} diff --git a/src/applications/policy/filter/policy/__init__.php b/src/applications/policy/filter/policy/__init__.php new file mode 100644 index 0000000000..53e078af9d --- /dev/null +++ b/src/applications/policy/filter/policy/__init__.php @@ -0,0 +1,15 @@ +viewer = $viewer; + return $this; + } + + final public function getViewer() { + return $this->viewer; + } + + final public function setLimit($limit) { + $this->limit = $limit; + return $this; + } + + final public function getLimit() { + return $this->limit; + } + + final public function executeOne() { + + $this->raisePolicyExceptions = true; + try { + $results = $this->execute(); + } catch (Exception $ex) { + $this->raisePolicyExceptions = false; + throw $ex; + } + + if (count($results) > 1) { + throw new Exception("Expected a single result!"); + } + return head($results); + } + + final public function execute() { + if (!$this->viewer) { + throw new Exception("Call setViewer() before execute()!"); + } + + $results = array(); + + $filter = new PhabricatorPolicyFilter(); + $filter->setViewer($this->viewer); + $filter->setCapability(PhabricatorPolicyCapability::CAN_VIEW); + + $filter->raisePolicyExceptions($this->raisePolicyExceptions); + + do { + $page = $this->loadPage(); + + $visible = $filter->apply($page); + foreach ($visible as $key => $result) { + $results[$key] = $result; + if ($this->getLimit() && count($results) >= $this->getLimit()) { + break 2; + } + } + + if (!$this->getLimit() || (count($page) < $this->getLimit())) { + break; + } + + $this->nextPage($page); + } while (true); + + return $results; + } + + abstract protected function loadPage(); + abstract protected function nextPage(array $page); + +} diff --git a/src/infrastructure/query/policy/base/__init__.php b/src/infrastructure/query/policy/base/__init__.php new file mode 100644 index 0000000000..a9ec3c47f2 --- /dev/null +++ b/src/infrastructure/query/policy/base/__init__.php @@ -0,0 +1,16 @@ +getID(); + } + + protected function nextPage(array $page) { + if ($this->beforeID) { + $this->beforeID = $this->getPagingValue(head($page)); + } else { + $this->afterID = $this->getPagingValue(last($page)); + } + } + + final public function setAfterID($object_id) { + $this->afterID = $object_id; + return $this; + } + + final public function setBeforeID($object_id) { + $this->beforeID = $object_id; + return $this; + } + + final protected function buildLimitClause(AphrontDatabaseConnection $conn_r) { + if ($this->getLimit()) { + return qsprintf($conn_r, 'LIMIT %d', $this->getLimit()); + } else { + return ''; + } + } + + final protected function buildPagingClause( + AphrontDatabaseConnection $conn_r) { + + if ($this->beforeID) { + return qsprintf( + $conn_r, + '%C > %s', + $this->getPagingColumn(), + $this->beforeID); + } else if ($this->afterID) { + return qsprintf( + $conn_r, + '%C < %s', + $this->getPagingColumn(), + $this->afterID); + } + + return null; + } + + final protected function buildOrderClause(AphrontDatabaseConnection $conn_r) { + if ($this->beforeID) { + return qsprintf( + $conn_r, + 'ORDER BY %C ASC', + $this->getPagingColumn()); + } else { + return qsprintf( + $conn_r, + 'ORDER BY %C DESC', + $this->getPagingColumn()); + } + } + + final protected function processResults(array $results) { + if ($this->beforeID) { + $results = array_reverse($results, $preserve_keys = true); + } + return $results; + } + + + final public function executeWithPager(AphrontIDPagerView $pager) { + $this->setLimit($pager->getPageSize() + 1); + + if ($pager->getAfterID()) { + $this->setAfterID($pager->getAfterID()); + } else if ($pager->getBeforeID()) { + $this->setBeforeID($pager->getBeforeID()); + } + + $results = $this->execute(); + + $sliced_results = $pager->sliceResults($results); + + if ($this->beforeID || (count($results) > $pager->getPageSize())) { + $pager->setNextPageID($this->getPagingValue(last($sliced_results))); + } + + if ($this->afterID || + ($this->beforeID && (count($results) > $pager->getPageSize()))) { + $pager->setPrevPageID($this->getPagingValue(head($sliced_results))); + } + + return $sliced_results; + } + +} diff --git a/src/infrastructure/query/policy/idpaged/__init__.php b/src/infrastructure/query/policy/idpaged/__init__.php new file mode 100644 index 0000000000..9852f089a6 --- /dev/null +++ b/src/infrastructure/query/policy/idpaged/__init__.php @@ -0,0 +1,15 @@ +pageSize = max(1, $page_size); + return $this; + } + + final public function getPageSize() { + return $this->pageSize; + } + + final public function setURI(PhutilURI $uri) { + $this->uri = $uri; + return $this; + } + + final public function readFromRequest(AphrontRequest $request) { + $this->uri = $request->getRequestURI(); + $this->afterID = $request->getStr('after'); + $this->beforeID = $request->getStr('before'); + return $this; + } + + final public function setAfterID($after_id) { + $this->afterID = $after_id; + return $this; + } + + final public function getAfterID() { + return $this->afterID; + } + + final public function setBeforeID($before_id) { + $this->beforeID = $before_id; + return $this; + } + + final public function getBeforeID() { + return $this->beforeID; + } + + final public function setNextPageID($next_page_id) { + $this->nextPageID = $next_page_id; + return $this; + } + + final public function getNextPageID() { + return $this->nextPageID; + } + + final public function setPrevPageID($prev_page_id) { + $this->prevPageID = $prev_page_id; + return $this; + } + + final public function getPrevPageID() { + return $this->prevPageID; + } + + final public function sliceResults(array $results) { + if (count($results) > $this->getPageSize()) { + $results = array_slice($results, 0, $this->getPageSize(), true); + } + return $results; + } + + public function render() { + if (!$this->uri) { + throw new Exception( + "You must call setURI() before you can call render()."); + } + + $links = array(); + + if ($this->beforeID || $this->afterID) { + $links[] = phutil_render_tag( + 'a', + array( + 'href' => $this->uri + ->alter('before', null) + ->alter('after', null), + ), + "\xC2\xAB First"); + } + + if ($this->prevPageID) { + $links[] = phutil_render_tag( + 'a', + array( + 'href' => $this->uri + ->alter('after', null) + ->alter('before', $this->prevPageID), + ), + "\xE2\x80\xB9 Prev"); + } + + if ($this->nextPageID) { + $links[] = phutil_render_tag( + 'a', + array( + 'href' => $this->uri + ->alter('after', $this->nextPageID) + ->alter('before', null), + ), + "Next \xE2\x80\xBA"); + } + + return + '
'. + implode('', $links). + '
'; + } + +} diff --git a/src/view/control/idpager/__init__.php b/src/view/control/idpager/__init__.php new file mode 100644 index 0000000000..e332da4a43 --- /dev/null +++ b/src/view/control/idpager/__init__.php @@ -0,0 +1,14 @@ +user = $user; + return $this; + } + + public function getUser() { + return $this->user; + } + + protected function getCustomControlClass() { + return 'aphront-form-control-policy'; + } + + private function getOptions() { + return array( + PhabricatorPolicies::POLICY_USER => 'All Users', + ); + } + + protected function renderInput() { + return AphrontFormSelectControl::renderSelectTag( + $this->getValue(), + $this->getOptions(), + array( + 'name' => $this->getName(), + 'disabled' => $this->getDisabled() ? 'disabled' : null, + 'id' => $this->getID(), + )); + } + + +} diff --git a/src/view/form/control/policy/__init__.php b/src/view/form/control/policy/__init__.php new file mode 100644 index 0000000000..bb17d845ba --- /dev/null +++ b/src/view/form/control/policy/__init__.php @@ -0,0 +1,14 @@ +