When logged-out users hit a "Login Required" dialog, try to choose a better "next" URI

Summary:
Ref T10004. After a user logs in, we send them to the "next" URI cookie if there is one, but currently don't always do a very good job of selecting a "next" URI, especially if they tried to do something with a dialog before being asked to log in.

In particular, if a logged-out user clicks an action like "Edit Blocking Tasks" on a Maniphest task, the default behavior is to send them to the standalone page for that dialog after they log in. This can be pretty confusing.

See T2691 and D6416 for earlier efforts here. At that time, we added a mechanism to //manually// override the default behavior, and fixed the most common links. This worked, but I'd like to fix the //default// beahvior so we don't need to remember to `setObjectURI()` correctly all over the place.

ApplicationEditor has also introduced new cases which are more difficult to get right. While we could get them right by using the override and being careful about things, this also motivates fixing the default behavior.

Finally, we have better tools for fixing the default behavior now than we did in 2013.

Instead of using manual overrides, have JS include an "X-Phabricator-Via" header in Ajax requests. This is basically like a referrer header, and will contain the page the user's browser is on.

In essentially every case, this should be a very good place (and often the best place) to send them after login. For all pages currently using `setObjectURI()`, it should produce the same behavior by default.

I'll remove the `setObjectURI()` mechanism in the next diff.

Test Plan: Clicked various workflow actions while logged out, saw "next" get set to a reasonable value, was redirected to a sensible, non-confusing page after login (the page with whatever button I clicked on it).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10004

Differential Revision: https://secure.phabricator.com/D14804

resources/celerity/map.php

@@ -8,7 +8,7 @@
 return array(
   'names' => array(
     'core.pkg.css' => '8378907a',
-    'core.pkg.js' => 'c60f35d8',
+    'core.pkg.js' => '5058979d',
     'darkconsole.pkg.js' => 'e7393ebb',
     'differential.pkg.css' => '2de124c9',
     'differential.pkg.js' => '6223dd9d',
@@ -227,14 +227,14 @@ return array(
     'rsrc/externals/javelin/lib/JSON.js' => '69adf288',
     'rsrc/externals/javelin/lib/Leader.js' => '331b1611',
     'rsrc/externals/javelin/lib/Mask.js' => '8a41885b',
-    'rsrc/externals/javelin/lib/Quicksand.js' => '4cebc641',
+    'rsrc/externals/javelin/lib/Quicksand.js' => '6b8ef10b',
     'rsrc/externals/javelin/lib/Request.js' => '94b750d2',
     'rsrc/externals/javelin/lib/Resource.js' => '44959b73',
     'rsrc/externals/javelin/lib/Routable.js' => 'b3e7d692',
     'rsrc/externals/javelin/lib/Router.js' => '29274e2b',
     'rsrc/externals/javelin/lib/Scrollbar.js' => '087e919c',
     'rsrc/externals/javelin/lib/Sound.js' => '949c0fe5',
-    'rsrc/externals/javelin/lib/URI.js' => '6eff08aa',
+    'rsrc/externals/javelin/lib/URI.js' => 'c989ade3',
     'rsrc/externals/javelin/lib/Vector.js' => '2caa8fb8',
     'rsrc/externals/javelin/lib/WebSocket.js' => 'e292eaf4',
     'rsrc/externals/javelin/lib/Workflow.js' => '5b2e3e2b',
@@ -487,7 +487,7 @@ return array(
     'rsrc/js/core/behavior-oncopy.js' => '2926fff2',
     'rsrc/js/core/behavior-phabricator-nav.js' => '56a1ca03',
     'rsrc/js/core/behavior-phabricator-remarkup-assist.js' => 'eeaa9e5a',
-    'rsrc/js/core/behavior-refresh-csrf.js' => '7814b593',
+    'rsrc/js/core/behavior-refresh-csrf.js' => 'ab2f381b',
     'rsrc/js/core/behavior-remarkup-preview.js' => 'f7379f45',
     'rsrc/js/core/behavior-reorder-applications.js' => '76b9fc3e',
     'rsrc/js/core/behavior-reveal-content.js' => '60821bc7',
@@ -657,7 +657,7 @@ return array(
     'javelin-behavior-project-create' => '065227cc',
     'javelin-behavior-quicksand-blacklist' => '7927a7d3',
     'javelin-behavior-recurring-edit' => '5f1c4d5f',
-    'javelin-behavior-refresh-csrf' => '7814b593',
+    'javelin-behavior-refresh-csrf' => 'ab2f381b',
     'javelin-behavior-releeph-preview-branch' => 'b2b4fbaf',
     'javelin-behavior-releeph-request-state-change' => 'a0b57eb8',
     'javelin-behavior-releeph-request-typeahead' => 'de2e896f',
@@ -690,7 +690,7 @@ return array(
     'javelin-leader' => '331b1611',
     'javelin-magical-init' => '3010e992',
     'javelin-mask' => '8a41885b',
-    'javelin-quicksand' => '4cebc641',
+    'javelin-quicksand' => '6b8ef10b',
     'javelin-reactor' => '2b8de964',
     'javelin-reactor-dom' => 'c90a04fc',
     'javelin-reactor-node-calmer' => '76f4ebed',
@@ -710,7 +710,7 @@ return array(
     'javelin-typeahead-preloaded-source' => '54f314a0',
     'javelin-typeahead-source' => '2818f5ce',
     'javelin-typeahead-static-source' => '6c0e62fa',
-    'javelin-uri' => '6eff08aa',
+    'javelin-uri' => 'c989ade3',
     'javelin-util' => '93cc50d6',
     'javelin-vector' => '2caa8fb8',
     'javelin-view' => '0f764c35',
@@ -1130,9 +1130,6 @@ return array(
       'javelin-request',
       'javelin-util',
     ),
-    '4cebc641' => array(
-      'javelin-install',
-    ),
     '4e3e79a6' => array(
       'javelin-behavior',
       'javelin-stratcom',
@@ -1302,6 +1299,9 @@ return array(
     '69adf288' => array(
       'javelin-install',
     ),
+    '6b8ef10b' => array(
+      'javelin-install',
+    ),
     '6c0e62fa' => array(
       'javelin-install',
       'javelin-typeahead-source',
@@ -1329,11 +1329,6 @@ return array(
       'phabricator-drag-and-drop-file-upload',
       'phabricator-textareautils',
     ),
-    '6eff08aa' => array(
-      'javelin-install',
-      'javelin-util',
-      'javelin-stratcom',
-    ),
     '70baed2f' => array(
       'javelin-install',
       'javelin-dom',
@@ -1368,14 +1363,6 @@ return array(
       'javelin-reactor',
       'javelin-util',
     ),
-    '7814b593' => array(
-      'javelin-request',
-      'javelin-behavior',
-      'javelin-dom',
-      'javelin-router',
-      'javelin-util',
-      'phabricator-busy',
-    ),
     '782ab6e7' => array(
       'javelin-behavior',
       'javelin-dom',
@@ -1642,6 +1629,14 @@ return array(
       'javelin-util',
       'phabricator-prefab',
     ),
+    'ab2f381b' => array(
+      'javelin-request',
+      'javelin-behavior',
+      'javelin-dom',
+      'javelin-router',
+      'javelin-util',
+      'phabricator-busy',
+    ),
     'ad10aeac' => array(
       'javelin-install',
       'javelin-util',
@@ -1803,6 +1798,11 @@ return array(
       'javelin-install',
       'javelin-util',
     ),
+    'c989ade3' => array(
+      'javelin-install',
+      'javelin-util',
+      'javelin-stratcom',
+    ),
     'ca3f91eb' => array(
       'javelin-behavior',
       'javelin-dom',

src/aphront/AphrontRequest.php

@@ -236,6 +236,10 @@ final class AphrontRequest extends Phobject {
     return 'X-Phabricator-Csrf';
   }
 
+  public static function getViaHeaderName() {
+    return 'X-Phabricator-Via';
+  }
+
   public function validateCSRF() {
     $token_name = self::getCSRFTokenName();
     $token = $this->getStr($token_name);

src/applications/auth/controller/PhabricatorAuthStartController.php

@@ -214,14 +214,24 @@ final class PhabricatorAuthStartController
         $request->getRequestURI());
     }
 
-    $dialog = new AphrontDialogView();
-    $dialog->setUser($viewer);
-    $dialog->setTitle(pht('Login Required'));
-    $dialog->appendChild(pht('You must login to continue.'));
-    $dialog->addSubmitButton(pht('Login'));
-    $dialog->addCancelButton('/');
+    // Often, users end up here by clicking a disabled action link in the UI
+    // (for example, they might click "Edit Blocking Tasks" on a Maniphest
+    // task page). After they log in we want to send them back to that main
+    // object page if we can, since it's confusing to end up on a standalone
+    // page with only a dialog (particularly if that dialog is another error,
+    // like a policy exception).
 
-    return id(new AphrontDialogResponse())->setDialog($dialog);
+    $via_header = AphrontRequest::getViaHeaderName();
+    $via_uri = AphrontRequest::getHTTPHeader($via_header);
+    if (strlen($via_uri)) {
+      PhabricatorCookies::setNextURICookie($request, $via_uri, $force = true);
+    }
+
+    return $this->newDialog()
+      ->setTitle(pht('Login Required'))
+      ->appendParagraph(pht('You must login to continue.'))
+      ->addSubmitButton(pht('Login'))
+      ->addCancelButton('/');
   }
 
 

src/view/page/PhabricatorStandardPageView.php

@@ -261,7 +261,8 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView
       'refresh-csrf',
       array(
         'tokenName' => AphrontRequest::getCSRFTokenName(),
-        'header'    => AphrontRequest::getCSRFHeaderName(),
+        'header' => AphrontRequest::getCSRFHeaderName(),
+        'viaHeader' => AphrontRequest::getViaHeaderName(),
         'current'   => $current_token,
       ));
 

webroot/rsrc/externals/javelin/lib/Quicksand.js

@@ -50,7 +50,7 @@ JX.install('Quicksand', {
       JX.Stratcom.listen('history:change', null, self._onchange);
 
       self._started = true;
-      var path = self._getRelativeURI(window.location);
+      var path = JX.$U(window.location).getRelativeURI();
       self._id = window.history.state || 0;
       var id = self._id;
       self._onpage = id;
@@ -157,7 +157,7 @@ JX.install('Quicksand', {
       }
 
       // Set up the new state and fire a request to fetch the page data.
-      var path = self._getRelativeURI(uri);
+      var path = JX.$U(uri).getRelativeURI();
       var id = ++self._id;
 
       self._history.push({path: path, id: id});
@@ -296,18 +296,6 @@ JX.install('Quicksand', {
     },
 
 
-    /**
-     * Get just the relative part of a URI, for History operations.
-     */
-    _getRelativeURI: function(uri) {
-      return JX.$U(uri)
-        .setProtocol(null)
-        .setPort(null)
-        .setDomain(null)
-        .toString();
-    },
-
-
     /**
      * Set a list of regular expressions which blacklist URIs as not navigable
      * via Quicksand.

webroot/rsrc/externals/javelin/lib/URI.js

@@ -178,6 +178,14 @@ JX.install('URI', {
       return this._domain;
     },
 
+    getRelativeURI: function() {
+      return JX.$U(this.toString())
+        .setProtocol(null)
+        .setPort(null)
+        .setDomain(null)
+        .toString();
+    },
+
     toString : function() {
       if (__DEV__) {
         if (this.getPath() && this.getPath().charAt(0) != '/') {

webroot/rsrc/js/core/behavior-refresh-csrf.js

@@ -50,7 +50,12 @@ JX.behavior('refresh-csrf', function(config) {
 
   // Additionally, add the CSRF token as an HTTP header to every AJAX request.
   JX.Request.listen('open', function(r) {
-    r.getTransport().setRequestHeader(config.header, current_token);
+    var via = JX.$U(window.location).getRelativeURI();
+
+    var xport = r.getTransport();
+
+    xport.setRequestHeader(config.header, current_token);
+    xport.setRequestHeader(config.viaHeader, via);
   });
 
   // Does this type of routable show the "Busy" spinner?