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

@@ -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-crumbs-view-css' => 'af52829a',
-    'aphront-dialog-view-css' => '86c4a3b2',
+    'aphront-dialog-view-css' => 'af52829a',
-    'aphront-error-view-css' => '86c4a3b2',
+    'aphront-error-view-css' => 'af52829a',
-    'aphront-form-view-css' => '86c4a3b2',
+    'aphront-form-view-css' => 'af52829a',
     'aphront-headsup-action-list-view-css' => 'ec01d039',
-    'aphront-headsup-view-css' => '86c4a3b2',
+    'aphront-headsup-view-css' => 'af52829a',
-    'aphront-list-filter-view-css' => '86c4a3b2',
+    'aphront-list-filter-view-css' => 'af52829a',
-    'aphront-pager-view-css' => '86c4a3b2',
+    'aphront-pager-view-css' => 'af52829a',
-    'aphront-panel-view-css' => '86c4a3b2',
+    'aphront-panel-view-css' => 'af52829a',
-    'aphront-table-view-css' => '86c4a3b2',
+    'aphront-table-view-css' => 'af52829a',
-    'aphront-tokenizer-control-css' => '86c4a3b2',
+    'aphront-tokenizer-control-css' => 'af52829a',
-    'aphront-tooltip-css' => '86c4a3b2',
+    'aphront-tooltip-css' => 'af52829a',
-    'aphront-typeahead-control-css' => '86c4a3b2',
+    '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-buttons-css' => 'af52829a',
-    'phabricator-core-css' => '86c4a3b2',
+    'phabricator-core-css' => 'af52829a',
-    'phabricator-crumbs-view-css' => '86c4a3b2',
+    'phabricator-crumbs-view-css' => 'af52829a',
-    'phabricator-directory-css' => '86c4a3b2',
+    '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-filetree-view-css' => 'af52829a',
-    'phabricator-flag-css' => '86c4a3b2',
+    'phabricator-flag-css' => 'af52829a',
-    'phabricator-form-view-css' => '86c4a3b2',
+    'phabricator-form-view-css' => 'af52829a',
-    'phabricator-header-view-css' => '86c4a3b2',
+    'phabricator-header-view-css' => 'af52829a',
-    'phabricator-jump-nav' => '86c4a3b2',
+    '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-css' => 'af52829a',
-    'phabricator-notification-menu-css' => '86c4a3b2',
+    'phabricator-notification-menu-css' => 'af52829a',
-    'phabricator-object-item-list-view-css' => '86c4a3b2',
+    '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-side-menu-view-css' => 'af52829a',
-    'phabricator-standard-page-view' => '86c4a3b2',
+    'phabricator-standard-page-view' => 'af52829a',
     'phabricator-textareautils' => 'c90b892e',
     'phabricator-tooltip' => 'c90b892e',
-    'phabricator-transaction-view-css' => '86c4a3b2',
+    'phabricator-transaction-view-css' => 'af52829a',
-    'phabricator-zindex-css' => '86c4a3b2',
+    'phabricator-zindex-css' => 'af52829a',
-    'sprite-apps-large-css' => '86c4a3b2',
+    'sprite-apps-large-css' => 'af52829a',
-    'sprite-gradient-css' => '86c4a3b2',
+    'sprite-gradient-css' => 'af52829a',
-    'sprite-icon-css' => '86c4a3b2',
+    'sprite-icon-css' => 'af52829a',
-    'sprite-menu-css' => '86c4a3b2',
+    'sprite-menu-css' => 'af52829a',
-    'syntax-highlighting-css' => '86c4a3b2',
+    'syntax-highlighting-css' => 'af52829a',
   ),
 ));

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',

src/aphront/AphrontRequest.php

@@ -267,9 +267,12 @@ 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');
+    if ($base_uri) {
       $base_uri = new PhutilURI($base_uri);
 
       $base_domain = $base_uri->getDomain();
@@ -284,12 +287,16 @@ final class AphrontRequest {
           "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,

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()) {
 

src/applications/config/check/PhabricatorSetupCheckBaseURI.php

@@ -0,0 +1,47 @@
+<?php
+
+final class PhabricatorSetupCheckBaseURI extends PhabricatorSetupCheck {
+
+  protected function executeChecks() {
+    $base_uri = PhabricatorEnv::getEnvConfig('phabricator.base-uri');
+    if ($base_uri) {
+      return;
+    }
+
+    $base_uri_guess = PhabricatorEnv::getRequestBaseURI();
+
+    $summary = pht(
+      'The base URI for this install is not configured. Configuring it will '.
+      'improve security and enable features.');
+
+    $message = pht(
+      'The base URI for this install is not configured. Configuring it will '.
+      'improve security and allow background processes (like daemons and '.
+      'scripts) to generate links.'.
+      "\n\n".
+      'You should set the base URI to the URI you will use to access '.
+      'Phabricator, like "http://phabricator.example.com/".'.
+      "\n\n".
+      'Include the protocol (http or https), domain name, and port number if '.
+      'you are using a port other than 80 (http) or 443 (https).'.
+      "\n\n".
+      'Based on this request, it appears that the correct setting is:'.
+      "\n\n".
+      '%s'.
+      "\n\n".
+      'To configure the base URI, run the command shown below.',
+      $base_uri_guess);
+
+    $this
+      ->newIssue('config.phabricator.base-uri')
+      ->setShortName(pht('No Base URI'))
+      ->setName(pht("Base URI Not Configured"))
+      ->setSummary($summary)
+      ->setMessage($message)
+      ->addCommand(
+        csprintf(
+          '<tt>phabricator/ $</tt> '.
+          './bin/config set phabricator.base-uri %s',
+          $base_uri_guess));
+  }
+}

src/applications/config/option/PhabricatorConfigOption.php

@@ -2,7 +2,7 @@
 
 final class PhabricatorConfigOption
   extends Phobject
-  implements PhabricatorMarkupInterface{
+  implements PhabricatorMarkupInterface {
 
   private $key;
   private $default;

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(

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[] = '<li>'.$link.'</li>';
       }
+      if ($update) {
         $update = '<ul>'.implode("\n", $update).'</ul>';
+        $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(

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");

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  )-------------------------------------------------- */
 

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/'.");
-  }
-}
-

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 {