Replace old login validation controller with new one

Summary: Ref T1536. We can safely replace the old login validation controller with this new one, and reduce code dplication while we're at it.

Test Plan: Logged in with LDAP, logged in with OAuth, logged in with username/password, did a password reset.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1536

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

src/aphront/configuration/AphrontDefaultApplicationConfiguration.php

@@ -24,30 +24,6 @@ class AphrontDefaultApplicationConfiguration
           => 'PhabricatorTypeaheadCommonDatasourceController',
       ),
 
-      '/login/' => array(
-        '' => 'PhabricatorLoginController',
-        'email/' => 'PhabricatorEmailLoginController',
-        'etoken/(?P<token>\w+)/' => 'PhabricatorEmailTokenController',
-        'refresh/' => 'PhabricatorRefreshCSRFController',
-        'validate/' => 'PhabricatorLoginValidateController',
-        'mustverify/' => 'PhabricatorMustVerifyEmailController',
-      ),
-
-      '/logout/' => 'PhabricatorLogoutController',
-
-      '/oauth/' => array(
-        '(?P<provider>\w+)/' => array(
-          'login/'     => 'PhabricatorOAuthLoginController',
-          'diagnose/'  => 'PhabricatorOAuthDiagnosticsController',
-          'unlink/'    => 'PhabricatorOAuthUnlinkController',
-        ),
-      ),
-
-      '/ldap/' => array(
-        'login/' => 'PhabricatorLDAPLoginController',
-        'unlink/'    => 'PhabricatorLDAPUnlinkController',
-      ),
-
       '/oauthserver/' => array(
         'auth/'          => 'PhabricatorOAuthServerAuthController',
         'test/'          => 'PhabricatorOAuthServerTestController',

src/applications/auth/application/PhabricatorApplicationAuth.php

@@ -41,6 +41,29 @@ final class PhabricatorApplicationAuth extends PhabricatorApplication {
         'start/' => 'PhabricatorAuthStartController',
         'validate/' => 'PhabricatorAuthValidateController',
       ),
+
+      '/login/' => array(
+        '' => 'PhabricatorLoginController',
+        'email/' => 'PhabricatorEmailLoginController',
+        'etoken/(?P<token>\w+)/' => 'PhabricatorEmailTokenController',
+        'refresh/' => 'PhabricatorRefreshCSRFController',
+        'mustverify/' => 'PhabricatorMustVerifyEmailController',
+      ),
+
+      '/logout/' => 'PhabricatorLogoutController',
+
+      '/oauth/' => array(
+        '(?P<provider>\w+)/' => array(
+          'login/'     => 'PhabricatorOAuthLoginController',
+          'diagnose/'  => 'PhabricatorOAuthDiagnosticsController',
+          'unlink/'    => 'PhabricatorOAuthUnlinkController',
+        ),
+      ),
+
+      '/ldap/' => array(
+        'login/' => 'PhabricatorLDAPLoginController',
+        'unlink/'    => 'PhabricatorLDAPUnlinkController',
+      ),
     );
   }
 

src/applications/auth/controller/PhabricatorEmailTokenController.php

@@ -71,11 +71,9 @@ final class PhabricatorEmailTokenController
     $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
       $target_email->setIsVerified(1);
       $target_email->save();
-      $session_key = $target_user->establishSession('web');
     unset($unguarded);
 
-    $request->setCookie('phusr', $target_user->getUsername());
+    $this->establishWebSession($target_user);
-    $request->setCookie('phsid', $session_key);
 
     $next = '/';
     if (!PhabricatorEnv::getEnvConfig('auth.password-auth-enabled')) {
@@ -95,14 +93,8 @@ final class PhabricatorEmailTokenController
           ));
     }
 
-    $uri = new PhutilURI('/login/validate/');
+    $request->setCookie('next_uri', $next);
-    $uri->setQueryParams(
-      array(
-        'phusr' => $target_user->getUsername(),
-        'next'  => $next,
-      ));
 
-    return id(new AphrontRedirectResponse())
+    return $this->buildLoginValidateResponse($target_user);
-      ->setURI((string)$uri);
   }
 }

src/applications/auth/controller/PhabricatorLDAPLoginController.php

@@ -81,27 +81,18 @@ final class PhabricatorLDAPLoginController extends PhabricatorAuthController {
             ->setURI('/settings/panel/ldap/');
         }
 
-        if ($ldap_info->getID()) {
+        if ($ldap_info->getUserPHID()) {
           $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
 
           $known_user = id(new PhabricatorUser())->loadOneWhere(
             'phid = %s',
             $ldap_info->getUserPHID());
 
-          $session_key = $known_user->establishSession('web');
-
           $this->saveLDAPInfo($ldap_info);
 
-          $request->setCookie('phusr', $known_user->getUsername());
+          $this->establishWebSession($known_user);
-          $request->setCookie('phsid', $session_key);
 
-          $uri = new PhutilURI('/login/validate/');
+          return $this->buildLoginValidateResponse($known_user);
-          $uri->setQueryParams(
-            array(
-              'phusr' => $known_user->getUsername(),
-            ));
-
-          return id(new AphrontRedirectResponse())->setURI((string)$uri);
         }
 
         $controller = newv('PhabricatorLDAPRegistrationController',

src/applications/auth/controller/PhabricatorLoginController.php

@@ -138,18 +138,8 @@ final class PhabricatorLoginController
         }
 
         if (!$errors) {
-          $session_key = $user->establishSession('web');
+          $this->establishWebSession($user);
-
+          return $this->buildLoginValidateResponse($user);
-          $request->setCookie('phusr', $user->getUsername());
-          $request->setCookie('phsid', $session_key);
-
-          $uri = id(new PhutilURI('/login/validate/'))
-            ->setQueryParams(
-              array('phusr' => $user->getUsername()
-            ));
-
-          return id(new AphrontRedirectResponse())
-            ->setURI((string)$uri);
         } else {
           $log = PhabricatorUserLog::newLog(
             null,

src/applications/auth/controller/PhabricatorLoginValidateController.php

@@ -1,80 +0,0 @@
-<?php
-
-final class PhabricatorLoginValidateController
-  extends PhabricatorAuthController {
-
-  public function shouldRequireLogin() {
-    return false;
-  }
-
-  public function processRequest() {
-    $request = $this->getRequest();
-
-    $failures = array();
-
-    if (!strlen($request->getStr('phusr'))) {
-      throw new Exception(
-        "Login validation is missing expected parameters!");
-    }
-
-    $expect_phusr = $request->getStr('phusr');
-    $actual_phusr = $request->getCookie('phusr');
-    if ($actual_phusr != $expect_phusr) {
-
-      if ($actual_phusr) {
-        $cookie_info = "sent back a cookie with the value '{$actual_phusr}'.";
-      } else {
-        $cookie_info = "did not accept the cookie.";
-      }
-
-      $failures[] =
-        "Attempted to set 'phusr' cookie to '{$expect_phusr}', but your ".
-        "browser {$cookie_info}";
-    }
-
-    if (!$failures) {
-      if (!$request->getUser()->getPHID()) {
-        $failures[] = "Cookies were set correctly, but your session ".
-                      "isn't valid.";
-      }
-    }
-
-    if ($failures) {
-
-      $list = array();
-      foreach ($failures as $failure) {
-        $list[] = phutil_tag('li', array(), $failure);
-      }
-      $list = phutil_tag('ul', array(), $list);
-
-      $view = new AphrontRequestFailureView();
-      $view->setHeader(pht('Login Failed'));
-      $view->appendChild(hsprintf(
-        '<p>%s</p>%s<p>%s</p>',
-        pht('Login failed:'),
-        $list,
-        pht(
-          '<strong>Clear your cookies</strong> and try again.',
-          hsprintf(''))));
-      $view->appendChild(hsprintf(
-        '<div class="aphront-failure-continue">'.
-          '<a class="button" href="/login/">%s</a>'.
-        '</div>',
-        pht('Try Again')));
-      return $this->buildStandardPageResponse(
-        $view,
-        array(
-          'title' => pht('Login Failed'),
-        ));
-    }
-
-    $next = nonempty($request->getStr('next'), $request->getCookie('next_uri'));
-    $request->clearCookie('next_uri');
-    if (!PhabricatorEnv::isValidLocalWebResource($next)) {
-      $next = '/';
-    }
-
-    return id(new AphrontRedirectResponse())->setURI($next);
-  }
-
-}

src/applications/auth/controller/PhabricatorOAuthLoginController.php

@@ -146,20 +146,11 @@ final class PhabricatorOAuthLoginController
         $oauth_info,
         $provider);
 
-      $session_key = $known_user->establishSession('web');
-
       $this->saveOAuthInfo($oauth_info);
 
-      $request->setCookie('phusr', $known_user->getUsername());
+      $this->establishWebSession($known_user);
-      $request->setCookie('phsid', $session_key);
 
-      $uri = new PhutilURI('/login/validate/');
+      return $this->buildLoginValidateResponse($known_user);
-      $uri->setQueryParams(
-        array(
-          'phusr' => $known_user->getUsername(),
-        ));
-
-      return id(new AphrontRedirectResponse())->setURI((string)$uri);
     }
 
     $oauth_email = $provider->retrieveUserEmail();