From 104d3221d9b8b663b8f3e59c585dc7b1a60c1d2a Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 16 Jun 2013 10:15:33 -0700 Subject: [PATCH] Implement new auth login flow and login validation controller Summary: Ref T1536. None of this code is reachable. Implements new-auth login (so you can actually login) and login validation (which checks that cookies were set correctly). Test Plan: Manually enabled FB auth, went through the auth flow to login/logout. Manually hit most of the validation errors. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T1536 Differential Revision: https://secure.phabricator.com/D6162 --- src/__phutil_library_map__.php | 2 + .../PhabricatorApplicationAuth.php | 1 + .../PhabricatorAuthLoginController.php | 19 ++++- .../PhabricatorAuthStartController.php | 17 +---- .../PhabricatorAuthValidateController.php | 73 +++++++++++++++++++ 5 files changed, 95 insertions(+), 17 deletions(-) create mode 100644 src/applications/auth/controller/PhabricatorAuthValidateController.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 6d984f7488..4ac1d86c56 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -821,6 +821,7 @@ phutil_register_library_map(array( 'PhabricatorAuthProviderOAuthFacebook' => 'applications/auth/provider/PhabricatorAuthProviderOAuthFacebook.php', 'PhabricatorAuthRegisterController' => 'applications/auth/controller/PhabricatorAuthRegisterController.php', 'PhabricatorAuthStartController' => 'applications/auth/controller/PhabricatorAuthStartController.php', + 'PhabricatorAuthValidateController' => 'applications/auth/controller/PhabricatorAuthValidateController.php', 'PhabricatorAuthenticationConfigOptions' => 'applications/config/option/PhabricatorAuthenticationConfigOptions.php', 'PhabricatorBarePageExample' => 'applications/uiexample/examples/PhabricatorBarePageExample.php', 'PhabricatorBarePageView' => 'view/page/PhabricatorBarePageView.php', @@ -2679,6 +2680,7 @@ phutil_register_library_map(array( 'PhabricatorAuthProviderOAuthFacebook' => 'PhabricatorAuthProviderOAuth', 'PhabricatorAuthRegisterController' => 'PhabricatorAuthController', 'PhabricatorAuthStartController' => 'PhabricatorAuthController', + 'PhabricatorAuthValidateController' => 'PhabricatorAuthController', 'PhabricatorAuthenticationConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorBarePageExample' => 'PhabricatorUIExample', 'PhabricatorBarePageView' => 'AphrontPageView', diff --git a/src/applications/auth/application/PhabricatorApplicationAuth.php b/src/applications/auth/application/PhabricatorApplicationAuth.php index 6b5618151a..38d92b9b49 100644 --- a/src/applications/auth/application/PhabricatorApplicationAuth.php +++ b/src/applications/auth/application/PhabricatorApplicationAuth.php @@ -39,6 +39,7 @@ final class PhabricatorApplicationAuth extends PhabricatorApplication { 'login/(?P[^/]+)/' => 'PhabricatorAuthLoginController', 'register/(?P[^/]+)/' => 'PhabricatorAuthRegisterController', 'start/' => 'PhabricatorAuthStartController', + 'validate/' => 'PhabricatorAuthValidateController', ), ); } diff --git a/src/applications/auth/controller/PhabricatorAuthLoginController.php b/src/applications/auth/controller/PhabricatorAuthLoginController.php index 44eceec8a8..bf6cc3605a 100644 --- a/src/applications/auth/controller/PhabricatorAuthLoginController.php +++ b/src/applications/auth/controller/PhabricatorAuthLoginController.php @@ -80,7 +80,8 @@ final class PhabricatorAuthLoginController pht( 'The external account ("%s") you just authenticated with is '. 'not configured to allow account linking on this Phabricator '. - 'install. An administrator may have recently disabled it.')); + 'install. An administrator may have recently disabled it.', + $provider->getProviderName())); } } } @@ -90,8 +91,20 @@ final class PhabricatorAuthLoginController } private function processLoginUser(PhabricatorExternalAccount $account) { - // TODO: Implement. - return new Aphront404Response(); + $user = id(new PhabricatorUser())->loadOneWhere( + 'phid = %s', + $account->getUserPHID()); + + if (!$user) { + return $this->renderError( + pht( + 'The external account you just logged in with is not associated '. + 'with a valid Phabricator user.')); + } + + $this->establishWebSession($user); + + return $this->buildLoginValidateResponse($user); } private function processRegisterUser(PhabricatorExternalAccount $account) { diff --git a/src/applications/auth/controller/PhabricatorAuthStartController.php b/src/applications/auth/controller/PhabricatorAuthStartController.php index 5796ad1807..ee60c3b8a1 100644 --- a/src/applications/auth/controller/PhabricatorAuthStartController.php +++ b/src/applications/auth/controller/PhabricatorAuthStartController.php @@ -138,20 +138,9 @@ final class PhabricatorAuthStartController } private function renderError($message) { - $title = pht('Authentication Failure'); - - $view = new AphrontErrorView(); - $view->setTitle($title); - $view->appendChild($message); - - return $this->buildApplicationPage( - $view, - array( - 'title' => $title, - 'device' => true, - 'dust' => true, - )); + return $this->renderErrorPage( + pht('Authentication Failure'), + array($message)); } - } diff --git a/src/applications/auth/controller/PhabricatorAuthValidateController.php b/src/applications/auth/controller/PhabricatorAuthValidateController.php new file mode 100644 index 0000000000..9332d479d2 --- /dev/null +++ b/src/applications/auth/controller/PhabricatorAuthValidateController.php @@ -0,0 +1,73 @@ +getRequest(); + $viewer = $request->getUser(); + + $failures = array(); + + if (!strlen($request->getStr('phusr'))) { + return $this->renderErrors( + array( + pht( + 'Login validation is missing expected parameter ("%s").', + 'phusr'))); + } + + $expect_phusr = $request->getStr('phusr'); + $actual_phusr = $request->getCookie('phusr'); + if ($actual_phusr != $expect_phusr) { + if ($actual_phusr) { + $failures[] = pht( + "Attempted to set '%s' cookie to '%s', but your browser sent back ". + "a cookie with the value '%s'. Clear your browser's cookies and ". + "try again.", + 'phusr', + $expect_phusr, + $actual_phusr); + } else { + $failures[] = pht( + "Attempted to set '%s' cookie to '%s', but your browser did not ". + "accept the cookie. Check that cookies are enabled, clear them, ". + "and try again.", + 'phusr', + $expect_phusr); + } + } + + if (!$failures) { + if (!$viewer->getPHID()) { + $failures[] = pht( + "Login cookie was set correctly, but your login session is not ". + "valid. Try clearing cookies and logging in again."); + } + } + + if ($failures) { + return $this->renderErrors($failures); + } + + $next = $request->getCookie('next_uri'); + $request->clearCookie('next_uri'); + + if (!PhabricatorEnv::isValidLocalWebResource($next)) { + $next = '/'; + } + + return id(new AphrontRedirectResponse())->setURI($next); + } + + private function renderErrors(array $messages) { + return $this->renderErrorPage( + pht('Login Failure'), + $messages); + } + +}