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
This commit is contained in:
parent
ece9d792b2
commit
42fba24523
|
@ -31,10 +31,17 @@ class PhabricatorLoginController extends PhabricatorAuthController {
|
||||||
}
|
}
|
||||||
|
|
||||||
$next_uri = $this->getRequest()->getPath();
|
$next_uri = $this->getRequest()->getPath();
|
||||||
if ($next_uri == '/login/') {
|
$request->setCookie('next_uri', $next_uri);
|
||||||
$next_uri = null;
|
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');
|
$password_auth = PhabricatorEnv::getEnvConfig('auth.password-auth-enabled');
|
||||||
|
|
||||||
$forms = array();
|
$forms = array();
|
||||||
|
@ -66,7 +73,7 @@ class PhabricatorLoginController extends PhabricatorAuthController {
|
||||||
$request->setCookie('phsid', $session_key);
|
$request->setCookie('phsid', $session_key);
|
||||||
|
|
||||||
return id(new AphrontRedirectResponse())
|
return id(new AphrontRedirectResponse())
|
||||||
->setURI('/');
|
->setURI($request->getCookie('next_uri', '/'));
|
||||||
} else {
|
} else {
|
||||||
$log = PhabricatorUserLog::newLog(
|
$log = PhabricatorUserLog::newLog(
|
||||||
null,
|
null,
|
||||||
|
@ -93,7 +100,6 @@ class PhabricatorLoginController extends PhabricatorAuthController {
|
||||||
$form
|
$form
|
||||||
->setUser($request->getUser())
|
->setUser($request->getUser())
|
||||||
->setAction('/login/')
|
->setAction('/login/')
|
||||||
->addHiddenInput('next', $next_uri)
|
|
||||||
->appendChild(
|
->appendChild(
|
||||||
id(new AphrontFormTextControl())
|
id(new AphrontFormTextControl())
|
||||||
->setLabel('Username/Email')
|
->setLabel('Username/Email')
|
||||||
|
@ -115,8 +121,6 @@ class PhabricatorLoginController extends PhabricatorAuthController {
|
||||||
$forms['Phabricator Login'] = $form;
|
$forms['Phabricator Login'] = $form;
|
||||||
}
|
}
|
||||||
|
|
||||||
$oauth_state = $next_uri;
|
|
||||||
|
|
||||||
$providers = array(
|
$providers = array(
|
||||||
PhabricatorOAuthProvider::PROVIDER_FACEBOOK,
|
PhabricatorOAuthProvider::PROVIDER_FACEBOOK,
|
||||||
PhabricatorOAuthProvider::PROVIDER_GITHUB,
|
PhabricatorOAuthProvider::PROVIDER_GITHUB,
|
||||||
|
@ -160,7 +164,6 @@ class PhabricatorLoginController extends PhabricatorAuthController {
|
||||||
->addHiddenInput('client_id', $client_id)
|
->addHiddenInput('client_id', $client_id)
|
||||||
->addHiddenInput('redirect_uri', $redirect_uri)
|
->addHiddenInput('redirect_uri', $redirect_uri)
|
||||||
->addHiddenInput('scope', $minimum_scope)
|
->addHiddenInput('scope', $minimum_scope)
|
||||||
->addHiddenInput('state', $oauth_state)
|
|
||||||
->setUser($request->getUser())
|
->setUser($request->getUser())
|
||||||
->setMethod('GET')
|
->setMethod('GET')
|
||||||
->appendChild(
|
->appendChild(
|
||||||
|
|
|
@ -135,12 +135,7 @@ class PhabricatorOAuthLoginController extends PhabricatorAuthController {
|
||||||
->setURI('/settings/page/'.$provider_key.'/');
|
->setURI('/settings/page/'.$provider_key.'/');
|
||||||
}
|
}
|
||||||
|
|
||||||
$next_uri = '/';
|
$next_uri = $request->getCookie('next_uri', '/');
|
||||||
if ($this->oauthState) {
|
|
||||||
// Make sure a blind redirect to evil.com is impossible.
|
|
||||||
$uri = new PhutilURI($this->oauthState);
|
|
||||||
$next_uri = $uri->getPath();
|
|
||||||
}
|
|
||||||
|
|
||||||
// Login with known auth.
|
// Login with known auth.
|
||||||
|
|
||||||
|
@ -158,6 +153,7 @@ class PhabricatorOAuthLoginController extends PhabricatorAuthController {
|
||||||
|
|
||||||
$request->setCookie('phusr', $known_user->getUsername());
|
$request->setCookie('phusr', $known_user->getUsername());
|
||||||
$request->setCookie('phsid', $session_key);
|
$request->setCookie('phsid', $session_key);
|
||||||
|
$request->clearCookie('next_uri');
|
||||||
return id(new AphrontRedirectResponse())
|
return id(new AphrontRedirectResponse())
|
||||||
->setURI($next_uri);
|
->setURI($next_uri);
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue