From 4425903480147916486d9f6dffd3ced26b8654c3 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 22 Jan 2013 13:57:02 -0800 Subject: [PATCH] Don't require phabricator.base-uri to be configured Summary: Fixes T2293. We currently hard-require this in setup. We do not need to; we don't actually need it until we start running daemons. Move it to post-install and provide more guidance. We could make this even easier in the future, but we'd need to special case it, since it's dangerous to let it be set to any value (if you set it to the wrong value, you can't log in). We could safely have a workflow which writes the current request URI into the database configuration, or a two-stage workflow where we set the URI and then verify it, but these both imply some special casing and complication. This should be a step forward from where we are today, regardless. Test Plan: Removed "phabricator.base-uri" from my configuration. Verified Phabricator still works. Without "phabricator.base-uri" configured, logged in from multiple host names (127.0.0.1:8080, local.aphront.com:8080). Configured "phabricator.base-uri". Verified my unblessed session no longer worked. Verified setup issue went away. Reviewers: btrahan, vrana Reviewed By: btrahan CC: aran Maniphest Tasks: T2293 Differential Revision: https://secure.phabricator.com/D4580 --- src/__celerity_resource_map__.php | 84 +++++++++---------- src/__phutil_library_map__.php | 2 + src/aphront/AphrontRequest.php | 31 ++++--- .../AphrontApplicationConfiguration.php | 6 +- .../check/PhabricatorSetupCheckBaseURI.php | 47 +++++++++++ .../config/option/PhabricatorConfigOption.php | 2 +- .../option/PhabricatorCoreConfigOptions.php | 1 + .../config/view/PhabricatorSetupIssueView.php | 20 +++-- src/infrastructure/PhabricatorSetup.php | 59 ------------- src/infrastructure/env/PhabricatorEnv.php | 29 ++++++- webroot/index.php | 35 ++------ .../css/application/config/setup-issue.css | 5 +- 12 files changed, 168 insertions(+), 153 deletions(-) create mode 100644 src/applications/config/check/PhabricatorSetupCheckBaseURI.php diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index c166ad98bd..b647431b8e 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -3147,7 +3147,7 @@ celerity_register_resource_map(array( ), 'setup-issue-css' => array( - 'uri' => '/res/d642d4e5/rsrc/css/application/config/setup-issue.css', + 'uri' => '/res/a7034810/rsrc/css/application/config/setup-issue.css', 'type' => 'css', 'requires' => array( @@ -3192,7 +3192,7 @@ celerity_register_resource_map(array( ), 'sprite-icon-css' => array( - 'uri' => '/res/e7d63fcf/rsrc/css/sprite-icon.css', + 'uri' => '/res/ff841245/rsrc/css/sprite-icon.css', 'type' => 'css', 'requires' => array( @@ -3238,7 +3238,7 @@ celerity_register_resource_map(array( ), array( 'packages' => array( - '86c4a3b2' => + 'af52829a' => array( 'name' => 'core.pkg.css', 'symbols' => @@ -3282,7 +3282,7 @@ celerity_register_resource_map(array( 36 => 'phabricator-object-item-list-view-css', 37 => 'global-drag-and-drop-css', ), - 'uri' => '/res/pkg/86c4a3b2/core.pkg.css', + 'uri' => '/res/pkg/af52829a/core.pkg.css', 'type' => 'css', ), 'c90b892e' => @@ -3472,19 +3472,19 @@ celerity_register_resource_map(array( 'reverse' => array( 'aphront-attached-file-view-css' => '83f07678', - 'aphront-crumbs-view-css' => '86c4a3b2', - 'aphront-dialog-view-css' => '86c4a3b2', - 'aphront-error-view-css' => '86c4a3b2', - 'aphront-form-view-css' => '86c4a3b2', + 'aphront-crumbs-view-css' => 'af52829a', + 'aphront-dialog-view-css' => 'af52829a', + 'aphront-error-view-css' => 'af52829a', + 'aphront-form-view-css' => 'af52829a', 'aphront-headsup-action-list-view-css' => 'ec01d039', - 'aphront-headsup-view-css' => '86c4a3b2', - 'aphront-list-filter-view-css' => '86c4a3b2', - 'aphront-pager-view-css' => '86c4a3b2', - 'aphront-panel-view-css' => '86c4a3b2', - 'aphront-table-view-css' => '86c4a3b2', - 'aphront-tokenizer-control-css' => '86c4a3b2', - 'aphront-tooltip-css' => '86c4a3b2', - 'aphront-typeahead-control-css' => '86c4a3b2', + 'aphront-headsup-view-css' => 'af52829a', + 'aphront-list-filter-view-css' => 'af52829a', + 'aphront-pager-view-css' => 'af52829a', + 'aphront-panel-view-css' => 'af52829a', + 'aphront-table-view-css' => 'af52829a', + 'aphront-tokenizer-control-css' => 'af52829a', + 'aphront-tooltip-css' => 'af52829a', + 'aphront-typeahead-control-css' => 'af52829a', 'differential-changeset-view-css' => 'ec01d039', 'differential-core-view-css' => 'ec01d039', 'differential-inline-comment-editor' => 'ac53d36a', @@ -3498,7 +3498,7 @@ celerity_register_resource_map(array( 'differential-table-of-contents-css' => 'ec01d039', 'diffusion-commit-view-css' => 'c8ce2d88', 'diffusion-icons-css' => 'c8ce2d88', - 'global-drag-and-drop-css' => '86c4a3b2', + 'global-drag-and-drop-css' => 'af52829a', 'inline-comment-summary-css' => 'ec01d039', 'javelin-aphlict' => 'c90b892e', 'javelin-behavior' => 'fbeded59', @@ -3568,48 +3568,48 @@ celerity_register_resource_map(array( 'javelin-util' => 'fbeded59', 'javelin-vector' => 'fbeded59', 'javelin-workflow' => 'fbeded59', - 'lightbox-attachment-css' => '86c4a3b2', + 'lightbox-attachment-css' => 'af52829a', 'maniphest-task-summary-css' => '83f07678', 'maniphest-transaction-detail-css' => '83f07678', 'phabricator-busy' => 'c90b892e', 'phabricator-content-source-view-css' => 'ec01d039', - 'phabricator-core-buttons-css' => '86c4a3b2', - 'phabricator-core-css' => '86c4a3b2', - 'phabricator-crumbs-view-css' => '86c4a3b2', - 'phabricator-directory-css' => '86c4a3b2', + 'phabricator-core-buttons-css' => 'af52829a', + 'phabricator-core-css' => 'af52829a', + 'phabricator-crumbs-view-css' => 'af52829a', + 'phabricator-directory-css' => 'af52829a', 'phabricator-drag-and-drop-file-upload' => 'ac53d36a', 'phabricator-dropdown-menu' => 'c90b892e', 'phabricator-file-upload' => 'c90b892e', - 'phabricator-filetree-view-css' => '86c4a3b2', - 'phabricator-flag-css' => '86c4a3b2', - 'phabricator-form-view-css' => '86c4a3b2', - 'phabricator-header-view-css' => '86c4a3b2', - 'phabricator-jump-nav' => '86c4a3b2', + 'phabricator-filetree-view-css' => 'af52829a', + 'phabricator-flag-css' => 'af52829a', + 'phabricator-form-view-css' => 'af52829a', + 'phabricator-header-view-css' => 'af52829a', + 'phabricator-jump-nav' => 'af52829a', 'phabricator-keyboard-shortcut' => 'c90b892e', 'phabricator-keyboard-shortcut-manager' => 'c90b892e', - 'phabricator-main-menu-view' => '86c4a3b2', + 'phabricator-main-menu-view' => 'af52829a', 'phabricator-menu-item' => 'c90b892e', - 'phabricator-nav-view-css' => '86c4a3b2', + 'phabricator-nav-view-css' => 'af52829a', 'phabricator-notification' => 'c90b892e', - 'phabricator-notification-css' => '86c4a3b2', - 'phabricator-notification-menu-css' => '86c4a3b2', - 'phabricator-object-item-list-view-css' => '86c4a3b2', + 'phabricator-notification-css' => 'af52829a', + 'phabricator-notification-menu-css' => 'af52829a', + 'phabricator-object-item-list-view-css' => 'af52829a', 'phabricator-object-selector-css' => 'ec01d039', 'phabricator-paste-file-upload' => 'c90b892e', 'phabricator-prefab' => 'c90b892e', 'phabricator-project-tag-css' => '83f07678', - 'phabricator-remarkup-css' => '86c4a3b2', + 'phabricator-remarkup-css' => 'af52829a', 'phabricator-shaped-request' => 'ac53d36a', - 'phabricator-side-menu-view-css' => '86c4a3b2', - 'phabricator-standard-page-view' => '86c4a3b2', + 'phabricator-side-menu-view-css' => 'af52829a', + 'phabricator-standard-page-view' => 'af52829a', 'phabricator-textareautils' => 'c90b892e', 'phabricator-tooltip' => 'c90b892e', - 'phabricator-transaction-view-css' => '86c4a3b2', - 'phabricator-zindex-css' => '86c4a3b2', - 'sprite-apps-large-css' => '86c4a3b2', - 'sprite-gradient-css' => '86c4a3b2', - 'sprite-icon-css' => '86c4a3b2', - 'sprite-menu-css' => '86c4a3b2', - 'syntax-highlighting-css' => '86c4a3b2', + 'phabricator-transaction-view-css' => 'af52829a', + 'phabricator-zindex-css' => 'af52829a', + 'sprite-apps-large-css' => 'af52829a', + 'sprite-gradient-css' => 'af52829a', + 'sprite-icon-css' => 'af52829a', + 'sprite-menu-css' => 'af52829a', + 'syntax-highlighting-css' => 'af52829a', ), )); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 02897851c8..e83006d867 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1212,6 +1212,7 @@ phutil_register_library_map(array( 'PhabricatorSetup' => 'infrastructure/PhabricatorSetup.php', 'PhabricatorSetupCheck' => 'applications/config/check/PhabricatorSetupCheck.php', 'PhabricatorSetupCheckAPC' => 'applications/config/check/PhabricatorSetupCheckAPC.php', + 'PhabricatorSetupCheckBaseURI' => 'applications/config/check/PhabricatorSetupCheckBaseURI.php', 'PhabricatorSetupCheckExtraConfig' => 'applications/config/check/PhabricatorSetupCheckExtraConfig.php', 'PhabricatorSetupCheckFacebook' => 'applications/config/check/PhabricatorSetupCheckFacebook.php', 'PhabricatorSetupCheckGD' => 'applications/config/check/PhabricatorSetupCheckGD.php', @@ -2573,6 +2574,7 @@ phutil_register_library_map(array( 'PhabricatorSettingsPanelSSHKeys' => 'PhabricatorSettingsPanel', 'PhabricatorSettingsPanelSearchPreferences' => 'PhabricatorSettingsPanel', 'PhabricatorSetupCheckAPC' => 'PhabricatorSetupCheck', + 'PhabricatorSetupCheckBaseURI' => 'PhabricatorSetupCheck', 'PhabricatorSetupCheckExtraConfig' => 'PhabricatorSetupCheck', 'PhabricatorSetupCheckFacebook' => 'PhabricatorSetupCheck', 'PhabricatorSetupCheckGD' => 'PhabricatorSetupCheck', diff --git a/src/aphront/AphrontRequest.php b/src/aphront/AphrontRequest.php index b98011305f..c7a659c305 100644 --- a/src/aphront/AphrontRequest.php +++ b/src/aphront/AphrontRequest.php @@ -267,29 +267,36 @@ final class AphrontRequest { final public function setCookie($name, $value, $expire = null) { - // Ensure cookies are only set on the configured domain. + $is_secure = false; + // If a base URI has been configured, ensure cookies are only set on that + // domain. Also, use the URI protocol to control SSL-only cookies. $base_uri = PhabricatorEnv::getEnvConfig('phabricator.base-uri'); - $base_uri = new PhutilURI($base_uri); + if ($base_uri) { + $base_uri = new PhutilURI($base_uri); - $base_domain = $base_uri->getDomain(); - $base_protocol = $base_uri->getProtocol(); + $base_domain = $base_uri->getDomain(); + $base_protocol = $base_uri->getProtocol(); - $host = $this->getHost(); + $host = $this->getHost(); - if ($base_domain != $host) { - throw new Exception( - "This install of Phabricator is configured as '{$base_domain}' but ". - "you are accessing it via '{$host}'. Access Phabricator via ". - "the primary configured domain."); + if ($base_domain != $host) { + throw new Exception( + "This install of Phabricator is configured as '{$base_domain}' but ". + "you are accessing it via '{$host}'. Access Phabricator via ". + "the primary configured domain."); + } + + $is_secure = ($base_protocol == 'https'); + } else { + $base_uri = new PhutilURI(PhabricatorEnv::getRequestBaseURI()); + $base_domain = $base_uri->getDomain(); } if ($expire === null) { $expire = time() + (60 * 60 * 24 * 365 * 5); } - $is_secure = ($base_protocol == 'https'); - setcookie( $name, $value, diff --git a/src/aphront/configuration/AphrontApplicationConfiguration.php b/src/aphront/configuration/AphrontApplicationConfiguration.php index bdce0af0df..c5eee79179 100644 --- a/src/aphront/configuration/AphrontApplicationConfiguration.php +++ b/src/aphront/configuration/AphrontApplicationConfiguration.php @@ -118,7 +118,11 @@ abstract class AphrontApplicationConfiguration { $base_uri = PhabricatorEnv::getEnvConfig('phabricator.base-uri'); $prod_uri = PhabricatorEnv::getEnvConfig('phabricator.production-uri'); $file_uri = PhabricatorEnv::getEnvConfig('security.alternate-file-domain'); - if ($host != id(new PhutilURI($base_uri))->getDomain() && + + // NOTE: If the base URI isn't defined yet, don't activate alternate + // domains. + if ($base_uri && + $host != id(new PhutilURI($base_uri))->getDomain() && $host != id(new PhutilURI($prod_uri))->getDomain() && $host != id(new PhutilURI($file_uri))->getDomain()) { diff --git a/src/applications/config/check/PhabricatorSetupCheckBaseURI.php b/src/applications/config/check/PhabricatorSetupCheckBaseURI.php new file mode 100644 index 0000000000..34bcfb9599 --- /dev/null +++ b/src/applications/config/check/PhabricatorSetupCheckBaseURI.php @@ -0,0 +1,47 @@ +newIssue('config.phabricator.base-uri') + ->setShortName(pht('No Base URI')) + ->setName(pht("Base URI Not Configured")) + ->setSummary($summary) + ->setMessage($message) + ->addCommand( + csprintf( + 'phabricator/ $ '. + './bin/config set phabricator.base-uri %s', + $base_uri_guess)); + } +} diff --git a/src/applications/config/option/PhabricatorConfigOption.php b/src/applications/config/option/PhabricatorConfigOption.php index 5d0b5a286f..15977dceb1 100644 --- a/src/applications/config/option/PhabricatorConfigOption.php +++ b/src/applications/config/option/PhabricatorConfigOption.php @@ -2,7 +2,7 @@ final class PhabricatorConfigOption extends Phobject - implements PhabricatorMarkupInterface{ + implements PhabricatorMarkupInterface { private $key; private $default; diff --git a/src/applications/config/option/PhabricatorCoreConfigOptions.php b/src/applications/config/option/PhabricatorCoreConfigOptions.php index c10de994ed..d1ef5dbc60 100644 --- a/src/applications/config/option/PhabricatorCoreConfigOptions.php +++ b/src/applications/config/option/PhabricatorCoreConfigOptions.php @@ -14,6 +14,7 @@ final class PhabricatorCoreConfigOptions public function getOptions() { return array( $this->newOption('phabricator.base-uri', 'string', null) + ->setLocked(true) ->setSummary(pht("URI where Phabricator is installed.")) ->setDescription( pht( diff --git a/src/applications/config/view/PhabricatorSetupIssueView.php b/src/applications/config/view/PhabricatorSetupIssueView.php index 3d33a7cd6a..79886b01d4 100644 --- a/src/applications/config/view/PhabricatorSetupIssueView.php +++ b/src/applications/config/view/PhabricatorSetupIssueView.php @@ -147,6 +147,8 @@ final class PhabricatorSetupIssueView extends AphrontView { ), implode("\n", $table)); + $options = PhabricatorApplicationConfigOptions::loadAllOptions(); + if ($this->getIssue()->getIsFatal()) { $update_info = phutil_render_tag( 'p', @@ -165,12 +167,11 @@ final class PhabricatorSetupIssueView extends AphrontView { } $update = phutil_render_tag('pre', array(), implode("\n", $update)); } else { - $update_info = phutil_render_tag( - 'p', - array(), - pht("You can update these %d value(s) here:", count($configs))); $update = array(); foreach ($configs as $config) { + if ($options[$config]->getLocked()) { + continue; + } $link = phutil_render_tag( 'a', array( @@ -179,7 +180,16 @@ final class PhabricatorSetupIssueView extends AphrontView { pht('Edit %s', phutil_escape_html($config))); $update[] = '
  • '.$link.'
    • '; } - $update = ''; + if ($update) { + $update = ''; + $update_info = phutil_render_tag( + 'p', + array(), + pht("You can update these %d value(s) here:", count($configs))); + } else { + $update = null; + $update_info = null; + } } return phutil_render_tag( diff --git a/src/infrastructure/PhabricatorSetup.php b/src/infrastructure/PhabricatorSetup.php index eb6cecdeb8..899243abed 100644 --- a/src/infrastructure/PhabricatorSetup.php +++ b/src/infrastructure/PhabricatorSetup.php @@ -167,65 +167,6 @@ final class PhabricatorSetup { "directly. See this document for instructions:\n"); self::writeDoc('article/Configuration_Guide.html'); return; - } else { - $host = PhabricatorEnv::getEnvConfig('phabricator.base-uri'); - $host_uri = new PhutilURI($host); - $protocol = $host_uri->getProtocol(); - $allowed_protocols = array( - 'http' => true, - 'https' => true, - ); - if (empty($allowed_protocols[$protocol])) { - self::writeFailure(); - self::write( - "You must specify the protocol over which your host works (e.g.: ". - "\"http:// or https://\")\nin your custom config file.\nRefer to ". - "'default.conf.php' for documentation on configuration options.\n"); - return; - } - if (preg_match('/.*\/$/', $host)) { - self::write(" okay phabricator.base-uri protocol\n"); - } else { - self::writeFailure(); - self::write( - "You must add a trailing slash at the end of the host\n(e.g.: ". - "\"http://phabricator.example.com/ instead of ". - "http://phabricator.example.com\")\nin your custom config file.". - "\nRefer to 'default.conf.php' for documentation on configuration ". - "options.\n"); - return; - } - - $host_domain = $host_uri->getDomain(); - if (strpos($host_domain, '.') !== false) { - self::write(" okay phabricator.base-uri domain\n"); - } else { - self::writeFailure(); - self::write( - "You must host Phabricator on a domain that contains a dot ('.'). ". - "The current domain, '{$host_domain}', does not have a dot, so some ". - "browsers will not set cookies on it. For instance, ". - "'http://example.com/ is OK, but 'http://example/' won't work. ". - "If you are using localhost, create an entry in the hosts file like ". - "'127.0.0.1 example.com', and access the localhost with ". - "'http://example.com/'."); - return; - } - - $host_path = $host_uri->getPath(); - if ($host_path == '/') { - self::write(" okay phabricator.base-uri path\n"); - } else { - self::writeFailure(); - self::write( - "Your 'phabricator.base-uri' setting includes a path, but should ". - "not (e.g., 'http://phabricator.example.com/' is OK, but ". - "'http://example.com/phabricator/' is not). Phabricator must be ". - "installed on an entire domain, it can not be installed on a ". - "path alongside other applications. Consult the documentation ". - "for more details."); - return; - } } self::write("[OKAY] Basic configuration OKAY\n"); diff --git a/src/infrastructure/env/PhabricatorEnv.php b/src/infrastructure/env/PhabricatorEnv.php index 3351675413..0246216fd1 100644 --- a/src/infrastructure/env/PhabricatorEnv.php +++ b/src/infrastructure/env/PhabricatorEnv.php @@ -52,6 +52,7 @@ final class PhabricatorEnv { private static $sourceStack; private static $repairSource; + private static $requestBaseURI; /** * @phutil-external-symbol class PhabricatorStartup @@ -247,7 +248,7 @@ final class PhabricatorEnv { * @task read */ public static function getURI($path) { - return rtrim(self::getEnvConfig('phabricator.base-uri'), '/').$path; + return rtrim(self::getAnyBaseURI(), '/').$path; } @@ -267,7 +268,7 @@ final class PhabricatorEnv { $production_domain = self::getEnvConfig('phabricator.production-uri'); if (!$production_domain) { - $production_domain = self::getEnvConfig('phabricator.base-uri'); + $production_domain = self::getAnyBaseURI(); } return rtrim($production_domain, '/').$path; } @@ -281,7 +282,7 @@ final class PhabricatorEnv { public static function getCDNURI($path) { $alt = self::getEnvConfig('security.alternate-file-domain'); if (!$alt) { - $alt = self::getEnvConfig('phabricator.base-uri'); + $alt = self::getAnyBaseURI(); } $uri = new PhutilURI($alt); $uri->setPath($path); @@ -309,6 +310,28 @@ final class PhabricatorEnv { return newv($class, $args); } + public static function getAnyBaseURI() { + $base_uri = self::getEnvConfig('phabricator.base-uri'); + + if (!$base_uri) { + $base_uri = self::getRequestBaseURI(); + } + + if (!$base_uri) { + throw new Exception( + "Define 'phabricator.base-uri' in your configuration to continue."); + } + + return $base_uri; + } + + public static function getRequestBaseURI() { + return self::$requestBaseURI; + } + + public static function setRequestBaseURI($uri) { + self::$requestBaseURI = $uri; + } /* -( Unit Test Support )-------------------------------------------------- */ diff --git a/webroot/index.php b/webroot/index.php index fc60165d93..0745798258 100644 --- a/webroot/index.php +++ b/webroot/index.php @@ -40,8 +40,6 @@ try { PhabricatorSetupCheck::willProcessRequest(); - phabricator_detect_bad_base_uri(); - $host = $_SERVER['HTTP_HOST']; $path = $_REQUEST['__path__']; @@ -57,6 +55,13 @@ try { $application->willBuildRequest(); $request = $application->buildRequest(); + // Until an administrator sets "phabricator.base-uri", assume it is the same + // as the request URI. This will work fine in most cases, it just breaks down + // when daemons need to do things. + $request_protocol = ($request->isHTTPS() ? 'https' : 'http'); + $request_base_uri = "{$request_protocol}://{$host}/"; + PhabricatorEnv::setRequestBaseURI($request_base_uri); + $write_guard = new AphrontWriteGuard(array($request, 'validateCSRF')); $application->setRequest($request); @@ -157,29 +162,3 @@ try { PhabricatorStartup::didFatal("[Exception] ".$ex->getMessage()); } -function phabricator_detect_bad_base_uri() { - $conf = PhabricatorEnv::getEnvConfig('phabricator.base-uri'); - $uri = new PhutilURI($conf); - switch ($uri->getProtocol()) { - case 'http': - case 'https': - break; - default: - PhabricatorStartup::didFatal( - "'phabricator.base-uri' is set to '{$conf}', which is invalid. ". - "The URI must start with 'http://' or 'https://'."); - return; - } - - if (strpos($uri->getDomain(), '.') === false) { - PhabricatorStartup::didFatal( - "'phabricator.base-uri' is set to '{$conf}', which is invalid. The URI ". - "must contain a dot ('.'), like 'http://example.com/', not just ". - "'http://example/'. Some web browsers will not set cookies on domains ". - "with no TLD, and Phabricator requires cookies for login. ". - "If you are using localhost, create an entry in the hosts file like ". - "'127.0.0.1 example.com', and access the localhost with ". - "'http://example.com/'."); - } -} - diff --git a/webroot/rsrc/css/application/config/setup-issue.css b/webroot/rsrc/css/application/config/setup-issue.css index 5a358df4c1..1d365511fe 100644 --- a/webroot/rsrc/css/application/config/setup-issue.css +++ b/webroot/rsrc/css/application/config/setup-issue.css @@ -41,11 +41,12 @@ } .setup-issue pre { - width: 84%; + width: 95%; margin: auto; border: 1px solid #dfdfdf; - padding: 10px 3%; + padding: 10px 2%; background: #efefef; + overflow-x: auto; } .setup-issue tt {