From 42fba24523c0440d85ff22e95cacb7475491ac22 Mon Sep 17 00:00:00 2001
From: Ricky Elrod <ricky@elrod.me>
Date: Wed, 6 Jul 2011 23:05:35 -0400
Subject: [PATCH] Fix github and local login redirects.

Summary:
Send the user where they were intending to go after github and localized logins.
Before, because Github didn't send oauthState, we would force / upon them.

Test Plan:
Tried all three methods of login successfully.

Reviewers:
epriestley

CC:

Differential Revision: 602
---
 .../login/PhabricatorLoginController.php        | 17 ++++++++++-------
 .../oauth/PhabricatorOAuthLoginController.php   |  8 ++------
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/src/applications/auth/controller/login/PhabricatorLoginController.php b/src/applications/auth/controller/login/PhabricatorLoginController.php
index 6736eb01f2..6980cac839 100644
--- a/src/applications/auth/controller/login/PhabricatorLoginController.php
+++ b/src/applications/auth/controller/login/PhabricatorLoginController.php
@@ -31,10 +31,17 @@ class PhabricatorLoginController extends PhabricatorAuthController {
     }
 
     $next_uri = $this->getRequest()->getPath();
-    if ($next_uri == '/login/') {
-      $next_uri = null;
+    $request->setCookie('next_uri', $next_uri);
+    if ($next_uri == '/login/' && !$request->isFormPost()) {
+      // The user went straight to /login/, so presumably they want to go
+      // to the dashboard upon logging in. Because, you know, that's logical.
+      // And people are logical. Sometimes... Fine, no they're not.
+      // We check for POST here because getPath() would get reset to /login/.
+       $request->setCookie('next_uri', '/');
     }
 
+    // Always use $request->getCookie('next_uri', '/') after the above.
+
     $password_auth = PhabricatorEnv::getEnvConfig('auth.password-auth-enabled');
 
     $forms = array();
@@ -66,7 +73,7 @@ class PhabricatorLoginController extends PhabricatorAuthController {
             $request->setCookie('phsid', $session_key);
 
             return id(new AphrontRedirectResponse())
-              ->setURI('/');
+              ->setURI($request->getCookie('next_uri', '/'));
           } else {
             $log = PhabricatorUserLog::newLog(
               null,
@@ -93,7 +100,6 @@ class PhabricatorLoginController extends PhabricatorAuthController {
       $form
         ->setUser($request->getUser())
         ->setAction('/login/')
-        ->addHiddenInput('next', $next_uri)
         ->appendChild(
           id(new AphrontFormTextControl())
             ->setLabel('Username/Email')
@@ -115,8 +121,6 @@ class PhabricatorLoginController extends PhabricatorAuthController {
       $forms['Phabricator Login'] = $form;
     }
 
-    $oauth_state = $next_uri;
-
     $providers = array(
       PhabricatorOAuthProvider::PROVIDER_FACEBOOK,
       PhabricatorOAuthProvider::PROVIDER_GITHUB,
@@ -160,7 +164,6 @@ class PhabricatorLoginController extends PhabricatorAuthController {
         ->addHiddenInput('client_id', $client_id)
         ->addHiddenInput('redirect_uri', $redirect_uri)
         ->addHiddenInput('scope', $minimum_scope)
-        ->addHiddenInput('state', $oauth_state)
         ->setUser($request->getUser())
         ->setMethod('GET')
         ->appendChild(
diff --git a/src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php b/src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php
index 6baf7fd6f4..bbcdad1538 100644
--- a/src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php
+++ b/src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php
@@ -135,12 +135,7 @@ class PhabricatorOAuthLoginController extends PhabricatorAuthController {
         ->setURI('/settings/page/'.$provider_key.'/');
     }
 
-    $next_uri = '/';
-    if ($this->oauthState) {
-      // Make sure a blind redirect to evil.com is impossible.
-      $uri = new PhutilURI($this->oauthState);
-      $next_uri = $uri->getPath();
-    }
+    $next_uri = $request->getCookie('next_uri', '/');
 
     // Login with known auth.
 
@@ -158,6 +153,7 @@ class PhabricatorOAuthLoginController extends PhabricatorAuthController {
 
       $request->setCookie('phusr', $known_user->getUsername());
       $request->setCookie('phsid', $session_key);
+      $request->clearCookie('next_uri');
       return id(new AphrontRedirectResponse())
         ->setURI($next_uri);
     }