diff --git a/conf/default.conf.php b/conf/default.conf.php index c4e1977102..742d6a2513 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -69,6 +69,18 @@ return array( 'phabricator.custom.logo' => null, +// -- Access Policies ------------------------------------------------------- // + + // Phabricator allows you to set the visibility of objects (like repositories + // and source code) to "Public", which means anyone on the internet can see + // them, even without being logged in. This is great for open source, but + // some installs may never want to make anything public, so this policy is + // disabled by default. You can enable it here, which will let you set the + // policy for objects to "Public". With this option disabled, the most open + // policy is "All Users", which means users must be logged in to view things. + 'policy.allow-public' => false, + + // -- DarkConsole ----------------------------------------------------------- // // DarkConsole is a administrative debugging/profiling tool built into diff --git a/src/applications/policy/__tests__/PhabricatorPolicyTestCase.php b/src/applications/policy/__tests__/PhabricatorPolicyTestCase.php index 7f8b3c69de..93fc56ed2d 100644 --- a/src/applications/policy/__tests__/PhabricatorPolicyTestCase.php +++ b/src/applications/policy/__tests__/PhabricatorPolicyTestCase.php @@ -21,26 +21,37 @@ final class PhabricatorPolicyTestCase extends PhabricatorTestCase { /** * Verify that any user can view an object with POLICY_PUBLIC. */ - public function testPublicPolicy() { - $viewer = new PhabricatorUser(); + public function testPublicPolicyEnabled() { + $env = PhabricatorEnv::beginScopedEnv(); + $env->overrideEnvConfig('policy.allow-public', true); - $object = new PhabricatorPolicyTestObject(); - $object->setCapabilities( + $this->expectVisibility( + $this->buildObject(PhabricatorPolicies::POLICY_PUBLIC), array( - PhabricatorPolicyCapability::CAN_VIEW, - )); - $object->setPolicies( + 'public' => true, + 'user' => true, + 'admin' => true, + ), + 'Public Policy (Enabled in Config)'); + } + + + /** + * Verify that POLICY_PUBLIC is interpreted as POLICY_USER when public + * policies are disallowed. + */ + public function testPublicPolicyDisabled() { + $env = PhabricatorEnv::beginScopedEnv(); + $env->overrideEnvConfig('policy.allow-public', false); + + $this->expectVisibility( + $this->buildObject(PhabricatorPolicies::POLICY_PUBLIC), 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'); + 'public' => false, + 'user' => true, + 'admin' => true, + ), + 'Public Policy (Disabled in Config)'); } @@ -49,41 +60,29 @@ final class PhabricatorPolicyTestCase extends PhabricatorTestCase { * logged-out users can not. */ public function testUsersPolicy() { - $viewer = new PhabricatorUser(); - - $object = new PhabricatorPolicyTestObject(); - $object->setCapabilities( + $this->expectVisibility( + $this->buildObject(PhabricatorPolicies::POLICY_USER), array( - PhabricatorPolicyCapability::CAN_VIEW, - )); - $object->setPolicies( + 'public' => false, + 'user' => true, + 'admin' => true, + ), + 'User Policy'); + } + + + /** + * Verify that only administrators can view an object with POLICY_ADMIN. + */ + public function testAdminPolicy() { + $this->expectVisibility( + $this->buildObject(PhabricatorPolicies::POLICY_ADMIN), 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'); + 'public' => false, + 'user' => false, + 'admin' => true, + ), + 'Admin Policy'); } @@ -91,8 +90,59 @@ final class PhabricatorPolicyTestCase extends PhabricatorTestCase { * Verify that no one can view an object with POLICY_NOONE. */ public function testNoOnePolicy() { - $viewer = new PhabricatorUser(); + $this->expectVisibility( + $this->buildObject(PhabricatorPolicies::POLICY_NOONE), + array( + 'public' => false, + 'user' => false, + 'admin' => false, + ), + 'No One Policy'); + } + + + /** + * Test an object for visibility across multiple user specifications. + */ + private function expectVisibility( + PhabricatorPolicyTestObject $object, + array $map, + $description) { + + foreach ($map as $spec => $expect) { + $viewer = $this->buildUser($spec); + + $query = new PhabricatorPolicyTestQuery(); + $query->setResults(array($object)); + $query->setViewer($viewer); + + $caught = null; + try { + $result = $query->executeOne(); + } catch (PhabricatorPolicyException $ex) { + $caught = $ex; + } + + if ($expect) { + $this->assertEqual( + $object, + $result, + "{$description} with user {$spec} should succeed."); + } else { + $this->assertEqual( + true, + $caught instanceof PhabricatorPolicyException, + "{$description} with user {$spec} should fail."); + } + } + } + + + /** + * Build a test object to spec. + */ + private function buildObject($policy) { $object = new PhabricatorPolicyTestObject(); $object->setCapabilities( array( @@ -100,39 +150,34 @@ final class PhabricatorPolicyTestCase extends PhabricatorTestCase { )); $object->setPolicies( array( - PhabricatorPolicyCapability::CAN_VIEW => - PhabricatorPolicies::POLICY_NOONE, + PhabricatorPolicyCapability::CAN_VIEW => $policy, )); - $query = new PhabricatorPolicyTestQuery(); - $query->setResults(array($object)); - $query->setViewer($viewer); + return $object; + } - $caught = null; - try { - $query->executeOne(); - } catch (PhabricatorPolicyException $ex) { - $caught = $ex; + + /** + * Build a test user to spec. + */ + private function buildUser($spec) { + $user = new PhabricatorUser(); + + switch ($spec) { + case 'public': + break; + case 'user': + $user->setPHID(1); + break; + case 'admin': + $user->setPHID(1); + $user->setIsAdmin(true); + break; + default: + throw new Exception("Unknown user spec '{$spec}'."); } - $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.'); + return $user; } } diff --git a/src/applications/policy/__tests__/__init__.php b/src/applications/policy/__tests__/__init__.php index 48d2be7ea0..fdd538706f 100644 --- a/src/applications/policy/__tests__/__init__.php +++ b/src/applications/policy/__tests__/__init__.php @@ -10,6 +10,7 @@ phutil_require_module('phabricator', 'applications/people/storage/user'); 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_module('phabricator', 'infrastructure/env'); phutil_require_module('phabricator', 'infrastructure/query/policy/base'); phutil_require_module('phabricator', 'infrastructure/testing/testcase'); diff --git a/src/applications/policy/constants/policy/PhabricatorPolicies.php b/src/applications/policy/constants/policy/PhabricatorPolicies.php index 5666bf97e1..3fc3f818c5 100644 --- a/src/applications/policy/constants/policy/PhabricatorPolicies.php +++ b/src/applications/policy/constants/policy/PhabricatorPolicies.php @@ -20,6 +20,7 @@ final class PhabricatorPolicies extends PhabricatorPolicyConstants { const POLICY_PUBLIC = 'public'; const POLICY_USER = 'users'; + const POLICY_ADMIN = 'admin'; const POLICY_NOONE = 'no-one'; } diff --git a/src/applications/policy/filter/policy/PhabricatorPolicyFilter.php b/src/applications/policy/filter/policy/PhabricatorPolicyFilter.php index 3612b00de5..b0d261b2b0 100644 --- a/src/applications/policy/filter/policy/PhabricatorPolicyFilter.php +++ b/src/applications/policy/filter/policy/PhabricatorPolicyFilter.php @@ -67,6 +67,14 @@ final class PhabricatorPolicyFilter { $policy = $object->getPolicy($capability); + // If the object is set to "public" but that policy is disabled for this + // install, restrict the policy to "user". + if ($policy == PhabricatorPolicies::POLICY_PUBLIC) { + if (!PhabricatorEnv::getEnvConfig('policy.allow-public')) { + $policy = PhabricatorPolicies::POLICY_USER; + } + } + switch ($policy) { case PhabricatorPolicies::POLICY_PUBLIC: $filtered[$key] = $object; @@ -78,6 +86,13 @@ final class PhabricatorPolicyFilter { $this->rejectObject($object, $policy); } break; + case PhabricatorPolicies::POLICY_ADMIN: + if ($viewer->getIsAdmin()) { + $filtered[$key] = $object; + } else { + $this->rejectObject($object, $policy); + } + break; case PhabricatorPolicies::POLICY_NOONE: $this->rejectObject($object, $policy); break; @@ -103,6 +118,9 @@ final class PhabricatorPolicyFilter { case PhabricatorPolicies::POLICY_USER: $who = "To view this object, you must be logged in."; break; + case PhabricatorPolicies::POLICY_ADMIN: + $who = "To view this object, you must be an administrator."; + break; case PhabricatorPolicies::POLICY_NOONE: $who = "No one can view this object."; break; diff --git a/src/applications/policy/filter/policy/__init__.php b/src/applications/policy/filter/policy/__init__.php index 53e078af9d..2e62b35a2f 100644 --- a/src/applications/policy/filter/policy/__init__.php +++ b/src/applications/policy/filter/policy/__init__.php @@ -8,6 +8,7 @@ phutil_require_module('phabricator', 'applications/policy/constants/policy'); phutil_require_module('phabricator', 'applications/policy/exception/base'); +phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phutil', 'utils'); diff --git a/src/view/form/control/policy/AphrontFormPolicyControl.php b/src/view/form/control/policy/AphrontFormPolicyControl.php index a2ff3d88e4..472bf29ff7 100644 --- a/src/view/form/control/policy/AphrontFormPolicyControl.php +++ b/src/view/form/control/policy/AphrontFormPolicyControl.php @@ -32,9 +32,30 @@ final class AphrontFormPolicyControl extends AphrontFormControl { } private function getOptions() { - return array( - PhabricatorPolicies::POLICY_USER => 'All Users', - ); + $show_public = PhabricatorEnv::getEnvConfig('policy.allow-public'); + + if ($this->getValue() == PhabricatorPolicies::POLICY_PUBLIC) { + // If the object already has a "public" policy, show the option in + // the dropdown even if it will be enforced as "users", so we don't + // change the policy just because the config is changing. + $show_public = true; + } + + $options = array(); + + if ($show_public) { + $options[PhabricatorPolicies::POLICY_PUBLIC] = 'Public'; + } + + $options[PhabricatorPolicies::POLICY_USER] = 'All Users'; + + if ($this->user->getIsAdmin()) { + $options[PhabricatorPolicies::POLICY_ADMIN] = 'Administrators'; + } + + $options[PhabricatorPolicies::POLICY_NOONE] = 'No One'; + + return $options; } protected function renderInput() { diff --git a/src/view/form/control/policy/__init__.php b/src/view/form/control/policy/__init__.php index bb17d845ba..0c13330664 100644 --- a/src/view/form/control/policy/__init__.php +++ b/src/view/form/control/policy/__init__.php @@ -7,6 +7,7 @@ phutil_require_module('phabricator', 'applications/policy/constants/policy'); +phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phabricator', 'view/form/control/base'); phutil_require_module('phabricator', 'view/form/control/select');