From a6c4117ec434e33b0eccf276a3b8a993b6d29861 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 3 Oct 2013 19:05:47 -0700 Subject: [PATCH] Fix controller-level access rules Summary: Ref T603. I had to partially revert this earlier because it accidentally blocked access to Conduit and File data for installs without "policy.allow-public", since the applications are available to "all users" but some endpoints actually need to be available even when not logged in. This readjusts the gating in the controller to properly apply application visibility restrictions, and then adds a giant pile of unit test coverage to make sure it sticks and all the weird cases are covered. Test Plan: - Added and executed unit tests. - Executed most of the tests manually, by using logged in / admin / public / disabled users. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T603 Differential Revision: https://secure.phabricator.com/D7211 --- src/__phutil_library_map__.php | 6 + src/aphront/AphrontRequest.php | 25 +- src/aphront/console/DarkConsoleController.php | 4 + .../console/DarkConsoleDataController.php | 4 + .../base/PhabricatorApplication.php | 5 +- .../base/controller/PhabricatorController.php | 149 +++++----- .../PhabricatorAccessControlTestCase.php | 268 ++++++++++++++++++ .../__tests__/PhabricatorApplicationTest.php | 38 +++ .../__tests__/PhabricatorTestController.php | 40 +++ .../PhabricatorApplicationsListController.php | 34 ++- src/infrastructure/env/PhabricatorEnv.php | 2 + .../testing/PhabricatorTestCase.php | 22 ++ 12 files changed, 502 insertions(+), 95 deletions(-) create mode 100644 src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php create mode 100644 src/applications/base/controller/__tests__/PhabricatorApplicationTest.php create mode 100644 src/applications/base/controller/__tests__/PhabricatorTestController.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index e76fed12a0..f6254a8c3b 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -798,6 +798,7 @@ phutil_register_library_map(array( 'PasteReplyHandler' => 'applications/paste/mail/PasteReplyHandler.php', 'Phabricator404Controller' => 'applications/base/controller/Phabricator404Controller.php', 'PhabricatorAWSConfigOptions' => 'applications/config/option/PhabricatorAWSConfigOptions.php', + 'PhabricatorAccessControlTestCase' => 'applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php', 'PhabricatorAccessLog' => 'infrastructure/PhabricatorAccessLog.php', 'PhabricatorAccessLogConfigOptions' => 'applications/config/option/PhabricatorAccessLogConfigOptions.php', 'PhabricatorActionHeaderExample' => 'applications/uiexample/examples/PhabricatorActionHeaderExample.php', @@ -866,6 +867,7 @@ phutil_register_library_map(array( 'PhabricatorApplicationSlowvote' => 'applications/slowvote/application/PhabricatorApplicationSlowvote.php', 'PhabricatorApplicationStatusView' => 'applications/meta/view/PhabricatorApplicationStatusView.php', 'PhabricatorApplicationSubscriptions' => 'applications/subscriptions/application/PhabricatorApplicationSubscriptions.php', + 'PhabricatorApplicationTest' => 'applications/base/controller/__tests__/PhabricatorApplicationTest.php', 'PhabricatorApplicationTokens' => 'applications/tokens/application/PhabricatorApplicationTokens.php', 'PhabricatorApplicationTransaction' => 'applications/transactions/storage/PhabricatorApplicationTransaction.php', 'PhabricatorApplicationTransactionComment' => 'applications/transactions/storage/PhabricatorApplicationTransactionComment.php', @@ -1697,6 +1699,7 @@ phutil_register_library_map(array( 'PhabricatorTagView' => 'view/layout/PhabricatorTagView.php', 'PhabricatorTaskmasterDaemon' => 'infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php', 'PhabricatorTestCase' => 'infrastructure/testing/PhabricatorTestCase.php', + 'PhabricatorTestController' => 'applications/base/controller/__tests__/PhabricatorTestController.php', 'PhabricatorTestDataGenerator' => 'applications/lipsum/generator/PhabricatorTestDataGenerator.php', 'PhabricatorTestStorageEngine' => 'applications/files/engine/PhabricatorTestStorageEngine.php', 'PhabricatorTestWorker' => 'infrastructure/daemon/workers/__tests__/PhabricatorTestWorker.php', @@ -2909,6 +2912,7 @@ phutil_register_library_map(array( 'PasteReplyHandler' => 'PhabricatorMailReplyHandler', 'Phabricator404Controller' => 'PhabricatorController', 'PhabricatorAWSConfigOptions' => 'PhabricatorApplicationConfigOptions', + 'PhabricatorAccessControlTestCase' => 'PhabricatorTestCase', 'PhabricatorAccessLogConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorActionHeaderExample' => 'PhabricatorUIExample', 'PhabricatorActionHeaderView' => 'AphrontView', @@ -2974,6 +2978,7 @@ phutil_register_library_map(array( 'PhabricatorApplicationSlowvote' => 'PhabricatorApplication', 'PhabricatorApplicationStatusView' => 'AphrontView', 'PhabricatorApplicationSubscriptions' => 'PhabricatorApplication', + 'PhabricatorApplicationTest' => 'PhabricatorApplication', 'PhabricatorApplicationTokens' => 'PhabricatorApplication', 'PhabricatorApplicationTransaction' => array( @@ -3876,6 +3881,7 @@ phutil_register_library_map(array( 'PhabricatorTagView' => 'AphrontView', 'PhabricatorTaskmasterDaemon' => 'PhabricatorDaemon', 'PhabricatorTestCase' => 'ArcanistPhutilTestCase', + 'PhabricatorTestController' => 'PhabricatorController', 'PhabricatorTestStorageEngine' => 'PhabricatorFileStorageEngine', 'PhabricatorTestWorker' => 'PhabricatorWorker', 'PhabricatorTimeTestCase' => 'PhabricatorTestCase', diff --git a/src/aphront/AphrontRequest.php b/src/aphront/AphrontRequest.php index df58762baf..d4d055f8ec 100644 --- a/src/aphront/AphrontRequest.php +++ b/src/aphront/AphrontRequest.php @@ -342,14 +342,23 @@ final class AphrontRequest { $expire = time() + (60 * 60 * 24 * 365 * 5); } - setcookie( - $name, - $value, - $expire, - $path = '/', - $base_domain, - $is_secure, - $http_only = true); + + if (php_sapi_name() == 'cli') { + // Do nothing, to avoid triggering "Cannot modify header information" + // warnings. + + // TODO: This is effectively a test for whether we're running in a unit + // test or not. Move this actual call to HTTPSink? + } else { + setcookie( + $name, + $value, + $expire, + $path = '/', + $base_domain, + $is_secure, + $http_only = true); + } $_COOKIE[$name] = $value; diff --git a/src/aphront/console/DarkConsoleController.php b/src/aphront/console/DarkConsoleController.php index cb11db7b5c..9135e1c77c 100644 --- a/src/aphront/console/DarkConsoleController.php +++ b/src/aphront/console/DarkConsoleController.php @@ -12,6 +12,10 @@ final class DarkConsoleController extends PhabricatorController { return !PhabricatorEnv::getEnvConfig('darkconsole.always-on'); } + public function shouldRequireEnabledUser() { + return !PhabricatorEnv::getEnvConfig('darkconsole.always-on'); + } + public function processRequest() { $request = $this->getRequest(); $user = $request->getUser(); diff --git a/src/aphront/console/DarkConsoleDataController.php b/src/aphront/console/DarkConsoleDataController.php index f761308ada..b860b310d7 100644 --- a/src/aphront/console/DarkConsoleDataController.php +++ b/src/aphront/console/DarkConsoleDataController.php @@ -11,6 +11,10 @@ final class DarkConsoleDataController extends PhabricatorController { return !PhabricatorEnv::getEnvConfig('darkconsole.always-on'); } + public function shouldRequireEnabledUser() { + return !PhabricatorEnv::getEnvConfig('darkconsole.always-on'); + } + public function willProcessRequest(array $data) { $this->key = $data['key']; } diff --git a/src/applications/base/PhabricatorApplication.php b/src/applications/base/PhabricatorApplication.php index 4574a7bb10..938073273b 100644 --- a/src/applications/base/PhabricatorApplication.php +++ b/src/applications/base/PhabricatorApplication.php @@ -82,6 +82,10 @@ abstract class PhabricatorApplication return false; } + public function isUnlisted() { + return false; + } + /** * Returns true if an application is first-party (developed by Phacility) * and false otherwise. @@ -298,7 +302,6 @@ abstract class PhabricatorApplication $applications = $apps; } - return $applications; } diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php index 8456b625e5..62c882d131 100644 --- a/src/applications/base/controller/PhabricatorController.php +++ b/src/applications/base/controller/PhabricatorController.php @@ -5,14 +5,6 @@ abstract class PhabricatorController extends AphrontController { private $handles; public function shouldRequireLogin() { - - // If this install is configured to allow public resources and the - // controller works in public mode, allow the request through. - $is_public_allowed = PhabricatorEnv::getEnvConfig('policy.allow-public'); - if ($is_public_allowed && $this->shouldAllowPublic()) { - return false; - } - return true; } @@ -29,33 +21,38 @@ abstract class PhabricatorController extends AphrontController { } public function shouldRequireEmailVerification() { - $need_verify = PhabricatorUserEmail::isEmailVerificationRequired(); - $need_login = $this->shouldRequireLogin(); - - return ($need_login && $need_verify); + return PhabricatorUserEmail::isEmailVerificationRequired(); } final public function willBeginExecution() { $request = $this->getRequest(); + if ($request->getUser()) { + // NOTE: Unit tests can set a user explicitly. Normal requests are not + // permitted to do this. + PhabricatorTestCase::assertExecutingUnitTests(); + $user = $request->getUser(); + } else { + $user = new PhabricatorUser(); - $user = new PhabricatorUser(); + $phusr = $request->getCookie('phusr'); + $phsid = $request->getCookie('phsid'); - $phusr = $request->getCookie('phusr'); - $phsid = $request->getCookie('phsid'); - - if (strlen($phusr) && $phsid) { - $info = queryfx_one( - $user->establishConnection('r'), - 'SELECT u.* FROM %T u JOIN %T s ON u.phid = s.userPHID - AND s.type LIKE %> AND s.sessionKey = %s', - $user->getTableName(), - 'phabricator_session', - 'web-', - PhabricatorHash::digest($phsid)); - if ($info) { - $user->loadFromArray($info); + if (strlen($phusr) && $phsid) { + $info = queryfx_one( + $user->establishConnection('r'), + 'SELECT u.* FROM %T u JOIN %T s ON u.phid = s.userPHID + AND s.type LIKE %> AND s.sessionKey = %s', + $user->getTableName(), + 'phabricator_session', + 'web-', + PhabricatorHash::digest($phsid)); + if ($info) { + $user->loadFromArray($info); + } } + + $request->setUser($user); } $translation = $user->getTranslation(); @@ -67,7 +64,15 @@ abstract class PhabricatorController extends AphrontController { ->addTranslations($translation->getTranslations()); } - $request->setUser($user); + $preferences = $user->loadPreferences(); + if (PhabricatorEnv::getEnvConfig('darkconsole.enabled')) { + $dark_console = PhabricatorUserPreferences::PREFERENCE_DARK_CONSOLE; + if ($preferences->getPreference($dark_console) || + PhabricatorEnv::getEnvConfig('darkconsole.always-on')) { + $console = new DarkConsoleCore(); + $request->getApplicationConfiguration()->setConsole($console); + } + } if ($user->getIsDisabled() && $this->shouldRequireEnabledUser()) { $disabled_user_controller = new PhabricatorDisabledUserController( @@ -88,57 +93,59 @@ abstract class PhabricatorController extends AphrontController { return $this->delegateToController($checker_controller); } - $preferences = $user->loadPreferences(); + if ($this->shouldRequireLogin()) { + // This actually means we need either: + // - a valid user, or a public controller; and + // - permission to see the application. - if (PhabricatorEnv::getEnvConfig('darkconsole.enabled')) { - $dark_console = PhabricatorUserPreferences::PREFERENCE_DARK_CONSOLE; - if ($preferences->getPreference($dark_console) || - PhabricatorEnv::getEnvConfig('darkconsole.always-on')) { - $console = new DarkConsoleCore(); - $request->getApplicationConfiguration()->setConsole($console); - } - } - - if ($this->shouldRequireLogin() && !$user->getPHID()) { - $login_controller = new PhabricatorAuthStartController($request); - $this->setCurrentApplication( - PhabricatorApplication::getByClass('PhabricatorApplicationAuth')); - return $this->delegateToController($login_controller); - } - - if ($this->shouldRequireEmailVerification()) { - $email = $user->loadPrimaryEmail(); - if (!$email) { - throw new Exception( - "No primary email address associated with this account!"); - } - if (!$email->getIsVerified()) { - $verify_controller = new PhabricatorMustVerifyEmailController($request); - return $this->delegateToController($verify_controller); + $auth_class = 'PhabricatorApplicationAuth'; + $auth_application = PhabricatorApplication::getByClass($auth_class); + + $allow_public = $this->shouldAllowPublic() && + PhabricatorEnv::getEnvConfig('policy.allow-public'); + + // If this controller isn't public, and the user isn't logged in, require + // login. + if (!$allow_public && !$user->isLoggedIn()) { + $login_controller = new PhabricatorAuthStartController($request); + $this->setCurrentApplication($auth_application); + return $this->delegateToController($login_controller); + } + + if ($user->isLoggedIn()) { + if ($this->shouldRequireEmailVerification()) { + $email = $user->loadPrimaryEmail(); + if (!$email) { + throw new Exception( + "No primary email address associated with this account!"); + } + if (!$email->getIsVerified()) { + $controller = new PhabricatorMustVerifyEmailController($request); + $this->setCurrentApplication($auth_application); + return $this->delegateToController($controller); + } + } + } + + // If the user doesn't have access to the application, don't let them use + // any of its controllers. We query the application in order to generate + // a policy exception if the viewer doesn't have permission. + + $application = $this->getCurrentApplication(); + if ($application) { + id(new PhabricatorApplicationQuery()) + ->setViewer($user) + ->withPHIDs(array($application->getPHID())) + ->executeOne(); } } + // NOTE: We do this last so that users get a login page instead of a 403 + // if they need to login. if ($this->shouldRequireAdmin() && !$user->getIsAdmin()) { return new Aphront403Response(); } - // If the user doesn't have access to the application, don't let them use - // any of its controllers. We query the application in order to generate - // a policy exception if the viewer doesn't have permission. - $application = $this->getCurrentApplication(); - if ($application) { -/* - - TODO: Reenable this, but it's breaking some applications which need public - access in all cases, like Files and Conduit. - - id(new PhabricatorApplicationQuery()) - ->setViewer($user) - ->withPHIDs(array($application->getPHID())) - ->executeOne(); -*/ - } - } public function buildStandardPageView() { diff --git a/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php b/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php new file mode 100644 index 0000000000..1d6be0e793 --- /dev/null +++ b/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php @@ -0,0 +1,268 @@ + true, + ); + } + + public function testControllerAccessControls() { + $root = dirname(phutil_get_library_root('phabricator')); + require_once $root.'/support/PhabricatorStartup.php'; + + $application_configuration = new AphrontDefaultApplicationConfiguration(); + + $host = 'meow.example.com'; + + $_SERVER['REQUEST_METHOD'] = 'GET'; + + $request = id(new AphrontRequest($host, '/')) + ->setApplicationConfiguration($application_configuration) + ->setRequestData(array()); + + $controller = new PhabricatorTestController($request); + + $u_public = id(new PhabricatorUser()) + ->setUsername('public'); + + $u_unverified = $this->generateNewTestUser() + ->setUsername('unverified') + ->save(); + $u_unverified->loadPrimaryEmail()->setIsVerified(0)->save(); + + $u_normal = $this->generateNewTestUser() + ->setUsername('normal') + ->save(); + + $u_disabled = $this->generateNewTestUser() + ->setIsDisabled(true) + ->setUsername('disabled') + ->save(); + + $u_admin = $this->generateNewTestUser() + ->setIsAdmin(true) + ->setUsername('admin') + ->save(); + + $env = PhabricatorEnv::beginScopedEnv(); + $env->overrideEnvConfig('phabricator.base-uri', 'http://'.$host); + $env->overrideEnvConfig('policy.allow-public', false); + $env->overrideEnvConfig('auth.require-email-verification', false); + $env->overrideEnvConfig('auth.email-domains', array()); + + + // Test standard defaults. + + $this->checkAccess( + "Default", + id(clone $controller), + $request, + array( + $u_normal, + $u_admin, + $u_unverified, + ), + array( + $u_public, + $u_disabled, + )); + + + // Test email verification. + + $env->overrideEnvConfig('auth.require-email-verification', true); + $this->checkAccess( + "Email Verification Required", + id(clone $controller), + $request, + array( + $u_normal, + $u_admin, + ), + array( + $u_unverified, + $u_public, + $u_disabled, + )); + + $this->checkAccess( + "Email Verification Required, With Exception", + id(clone $controller)->setConfig('email', false), + $request, + array( + $u_normal, + $u_admin, + $u_unverified, + ), + array( + $u_public, + $u_disabled, + )); + $env->overrideEnvConfig('auth.require-email-verification', false); + + + // Test admin access. + + $this->checkAccess( + "Admin Required", + id(clone $controller)->setConfig('admin', true), + $request, + array( + $u_admin, + ), + array( + $u_normal, + $u_unverified, + $u_public, + $u_disabled, + )); + + + // Test disabled access. + + $this->checkAccess( + "Allow Disabled", + id(clone $controller)->setConfig('enabled', false), + $request, + array( + $u_normal, + $u_unverified, + $u_admin, + $u_disabled, + ), + array( + $u_public, + )); + + + // Test no login required. + + $this->checkAccess( + "No Login Required", + id(clone $controller)->setConfig('login', false), + $request, + array( + $u_normal, + $u_unverified, + $u_admin, + $u_public, + ), + array( + $u_disabled, + )); + + + // Test public access. + + $this->checkAccess( + "No Login Required", + id(clone $controller)->setConfig('public', true), + $request, + array( + $u_normal, + $u_unverified, + $u_admin, + ), + array( + $u_disabled, + $u_public, + )); + + $env->overrideEnvConfig('policy.allow-public', true); + $this->checkAccess( + "Public + configured", + id(clone $controller)->setConfig('public', true), + $request, + array( + $u_normal, + $u_unverified, + $u_admin, + $u_public, + ), + array( + $u_disabled, + )); + $env->overrideEnvConfig('policy.allow-public', false); + + + $app = PhabricatorApplication::getByClass('PhabricatorApplicationTest'); + $app->reset(); + $app->setPolicy( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicies::POLICY_NOONE); + + $app_controller = id(clone $controller)->setCurrentApplication($app); + + $this->checkAccess( + "Application Controller", + $app_controller, + $request, + array( + ), + array( + $u_normal, + $u_unverified, + $u_admin, + $u_public, + $u_disabled, + )); + + $this->checkAccess( + "Application Controller", + id(clone $app_controller)->setConfig('login', false), + $request, + array( + $u_normal, + $u_unverified, + $u_admin, + $u_public, + ), + array( + $u_disabled, + )); + } + + private function checkAccess( + $label, + $controller, + $request, + array $yes, + array $no) { + + foreach ($yes as $user) { + $request->setUser($user); + $uname = $user->getUsername(); + + try { + $result = id(clone $controller)->willBeginExecution(); + } catch (Exception $ex) { + $result = $ex; + } + + $this->assertEqual( + true, + ($result === null), + "Expect user '{$uname}' to be allowed access to '{$label}'."); + } + + foreach ($no as $user) { + $request->setUser($user); + $uname = $user->getUsername(); + + try { + $result = id(clone $controller)->willBeginExecution(); + } catch (Exception $ex) { + $result = $ex; + } + + $this->assertEqual( + false, + ($result === null), + "Expect user '{$uname}' to be denied access to '{$label}'."); + } + } + +} diff --git a/src/applications/base/controller/__tests__/PhabricatorApplicationTest.php b/src/applications/base/controller/__tests__/PhabricatorApplicationTest.php new file mode 100644 index 0000000000..b20faaf553 --- /dev/null +++ b/src/applications/base/controller/__tests__/PhabricatorApplicationTest.php @@ -0,0 +1,38 @@ +policies = array(); + } + + public function setPolicy($capability, $value) { + $this->policies[$capability] = $value; + return $this; + } + + public function getPolicy($capability) { + return idx($this->policies, $capability, parent::getPolicy($capability)); + } + + public function shouldAppearInLaunchView() { + return false; + } + + public function canUninstall() { + return false; + } + + public function getRoutes() { + return array( + ); + } + +} + diff --git a/src/applications/base/controller/__tests__/PhabricatorTestController.php b/src/applications/base/controller/__tests__/PhabricatorTestController.php new file mode 100644 index 0000000000..cba2bc0b13 --- /dev/null +++ b/src/applications/base/controller/__tests__/PhabricatorTestController.php @@ -0,0 +1,40 @@ +config[$key] = $value; + return $this; + } + + public function getConfig($key, $default) { + return idx($this->config, $key, $default); + } + + public function shouldRequireLogin() { + return $this->getConfig('login', parent::shouldRequireLogin()); + } + + public function shouldRequireAdmin() { + return $this->getConfig('admin', parent::shouldRequireAdmin()); + } + + public function shouldAllowPublic() { + return $this->getConfig('public', parent::shouldAllowPublic()); + } + + public function shouldRequireEmailVerification() { + return $this->getConfig('email', parent::shouldRequireEmailVerification()); + } + + public function shouldRequireEnabledUser() { + return $this->getConfig('enabled', parent::shouldRequireEnabledUser()); + } + + public function processRequest() { + + } + +} diff --git a/src/applications/meta/controller/PhabricatorApplicationsListController.php b/src/applications/meta/controller/PhabricatorApplicationsListController.php index a458e866a9..5a05aee538 100644 --- a/src/applications/meta/controller/PhabricatorApplicationsListController.php +++ b/src/applications/meta/controller/PhabricatorApplicationsListController.php @@ -30,22 +30,26 @@ final class PhabricatorApplicationsListController $applications = msort($applications, 'getName'); foreach ($applications as $application) { - $item = id(new PHUIObjectItemView()) - ->setHeader($application->getName()) - ->setHref('/applications/view/'.get_class($application).'/') - ->addAttribute($application->getShortDescription()); - - if (!$application->isInstalled()) { - $item->addIcon('delete', pht('Uninstalled')); - } - - if ($application->isBeta()) { - $item->addIcon('lint-warning', pht('Beta')); - } - - $list->addItem($item); - + if ($application->isUnlisted()) { + continue; } + + $item = id(new PHUIObjectItemView()) + ->setHeader($application->getName()) + ->setHref('/applications/view/'.get_class($application).'/') + ->addAttribute($application->getShortDescription()); + + if (!$application->isInstalled()) { + $item->addIcon('delete', pht('Uninstalled')); + } + + if ($application->isBeta()) { + $item->addIcon('lint-warning', pht('Beta')); + } + + $list->addItem($item); + } + return $list; } diff --git a/src/infrastructure/env/PhabricatorEnv.php b/src/infrastructure/env/PhabricatorEnv.php index d6044e046d..44a43e62ec 100644 --- a/src/infrastructure/env/PhabricatorEnv.php +++ b/src/infrastructure/env/PhabricatorEnv.php @@ -528,6 +528,8 @@ final class PhabricatorEnv { foreach ($tmp as $source) { self::$sourceStack->pushSource($source); } + + self::dropConfigCache(); } private static function dropConfigCache() { diff --git a/src/infrastructure/testing/PhabricatorTestCase.php b/src/infrastructure/testing/PhabricatorTestCase.php index df85f87add..18c0ed7dcf 100644 --- a/src/infrastructure/testing/PhabricatorTestCase.php +++ b/src/infrastructure/testing/PhabricatorTestCase.php @@ -37,6 +37,7 @@ abstract class PhabricatorTestCase extends ArcanistPhutilTestCase { private static $storageFixtureReferences = 0; private static $storageFixture; private static $storageFixtureObjectSeed = 0; + private static $testsAreRunning = 0; protected function getPhabricatorTestCaseConfiguration() { return array(); @@ -68,6 +69,8 @@ abstract class PhabricatorTestCase extends ArcanistPhutilTestCase { self::$storageFixture = $this->newStorageFixture(); } } + + ++self::$testsAreRunning; } public function didRunTestCases(array $test_cases) { @@ -77,6 +80,8 @@ abstract class PhabricatorTestCase extends ArcanistPhutilTestCase { self::$storageFixture = null; } } + + --self::$testsAreRunning; } protected function willRunTests() { @@ -184,4 +189,21 @@ abstract class PhabricatorTestCase extends ArcanistPhutilTestCase { return $user; } + + /** + * Throws unless tests are currently executing. This method can be used to + * guard code which is specific to unit tests and should not normally be + * reachable. + * + * If tests aren't currently being executed, throws an exception. + */ + public static function assertExecutingUnitTests() { + if (!self::$testsAreRunning) { + throw new Exception( + "Executing test code outside of test execution! This code path can ". + "only be run during unit tests."); + } + } + + }