mirror of
https://we.phorge.it/source/phorge.git
synced 2025-01-10 14:51:06 +01:00
Allow "bin/auth recover" to generate a link which forces a full login session
Summary: Depends on D19902. Ref T13222. This is mostly a "while I'm in here..." change since MFA is getting touched so much anyway. Doing cluster support, I sometimes need to log into user accounts on instances that have MFA. I currently accomplish this by doing `bin/auth recover`, getting a parital session, and then forcing it into a full session in the database. This is inconvenient and somewhat dangerous. Instead, allow `bin/auth recover` to generate a link that skips the "partial session" stage: adding required MFA, providing MFA, and signing legalpad documents. Anyone who can run `bin/auth recover` can do this anyway, this just reduces the chance I accidentally bypass MFA on the wrong session when doing support stuff. Test Plan: - Logged in with `bin/auth recover`, was prompted for MFA. - Logged in with `bin/auth recover --force-full-session`, was not prompted for MFA. - Did a password reset, followed reset link, was prompted for MFA. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13222 Differential Revision: https://secure.phabricator.com/D19903
This commit is contained in:
parent
6a6db0ac8e
commit
ff49d1ef77
5 changed files with 44 additions and 7 deletions
|
@ -45,9 +45,12 @@ abstract class PhabricatorAuthController extends PhabricatorController {
|
|||
* event and do something else if they prefer.
|
||||
*
|
||||
* @param PhabricatorUser User to log the viewer in as.
|
||||
* @param bool True to issue a full session immediately, bypassing MFA.
|
||||
* @return AphrontResponse Response which continues the login process.
|
||||
*/
|
||||
protected function loginUser(PhabricatorUser $user) {
|
||||
protected function loginUser(
|
||||
PhabricatorUser $user,
|
||||
$force_full_session = false) {
|
||||
|
||||
$response = $this->buildLoginValidateResponse($user);
|
||||
$session_type = PhabricatorAuthSession::TYPE_WEB;
|
||||
|
@ -66,8 +69,14 @@ abstract class PhabricatorAuthController extends PhabricatorController {
|
|||
|
||||
$should_login = $event->getValue('shouldLogin');
|
||||
if ($should_login) {
|
||||
if ($force_full_session) {
|
||||
$partial_session = false;
|
||||
} else {
|
||||
$partial_session = true;
|
||||
}
|
||||
|
||||
$session_key = id(new PhabricatorAuthSessionEngine())
|
||||
->establishSession($session_type, $user->getPHID(), $partial = true);
|
||||
->establishSession($session_type, $user->getPHID(), $partial_session);
|
||||
|
||||
// NOTE: We allow disabled users to login and roadblock them later, so
|
||||
// there's no check for users being disabled here.
|
||||
|
|
|
@ -152,7 +152,12 @@ final class PhabricatorAuthOneTimeLoginController
|
|||
|
||||
PhabricatorCookies::setNextURICookie($request, $next, $force = true);
|
||||
|
||||
return $this->loginUser($target_user);
|
||||
$force_full_session = false;
|
||||
if ($link_type === PhabricatorAuthSessionEngine::ONETIME_RECOVER) {
|
||||
$force_full_session = $token->getShouldForceFullSession();
|
||||
}
|
||||
|
||||
return $this->loginUser($target_user, $force_full_session);
|
||||
}
|
||||
|
||||
// NOTE: We need to CSRF here so attackers can't generate an email link,
|
||||
|
|
|
@ -893,24 +893,28 @@ final class PhabricatorAuthSessionEngine extends Phobject {
|
|||
* link is used.
|
||||
* @param string Optional context string for the URI. This is purely cosmetic
|
||||
* and used only to customize workflow and error messages.
|
||||
* @param bool True to generate a URI which forces an immediate upgrade to
|
||||
* a full session, bypassing MFA and other login checks.
|
||||
* @return string Login URI.
|
||||
* @task onetime
|
||||
*/
|
||||
public function getOneTimeLoginURI(
|
||||
PhabricatorUser $user,
|
||||
PhabricatorUserEmail $email = null,
|
||||
$type = self::ONETIME_RESET) {
|
||||
$type = self::ONETIME_RESET,
|
||||
$force_full_session = false) {
|
||||
|
||||
$key = Filesystem::readRandomCharacters(32);
|
||||
$key_hash = $this->getOneTimeLoginKeyHash($user, $email, $key);
|
||||
$onetime_type = PhabricatorAuthOneTimeLoginTemporaryTokenType::TOKENTYPE;
|
||||
|
||||
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
|
||||
id(new PhabricatorAuthTemporaryToken())
|
||||
$token = id(new PhabricatorAuthTemporaryToken())
|
||||
->setTokenResource($user->getPHID())
|
||||
->setTokenType($onetime_type)
|
||||
->setTokenExpires(time() + phutil_units('1 day in seconds'))
|
||||
->setTokenCode($key_hash)
|
||||
->setShouldForceFullSession($force_full_session)
|
||||
->save();
|
||||
unset($unguarded);
|
||||
|
||||
|
|
|
@ -13,7 +13,13 @@ final class PhabricatorAuthManagementRecoverWorkflow
|
|||
'of Phabricator.'))
|
||||
->setArguments(
|
||||
array(
|
||||
'username' => array(
|
||||
array(
|
||||
'name' => 'force-full-session',
|
||||
'help' => pht(
|
||||
'Recover directly into a full session without requiring MFA '.
|
||||
'or other login checks.'),
|
||||
),
|
||||
array(
|
||||
'name' => 'username',
|
||||
'wildcard' => true,
|
||||
),
|
||||
|
@ -54,11 +60,14 @@ final class PhabricatorAuthManagementRecoverWorkflow
|
|||
$username));
|
||||
}
|
||||
|
||||
$force_full_session = $args->getArg('force-full-session');
|
||||
|
||||
$engine = new PhabricatorAuthSessionEngine();
|
||||
$onetime_uri = $engine->getOneTimeLoginURI(
|
||||
$user,
|
||||
null,
|
||||
PhabricatorAuthSessionEngine::ONETIME_RECOVER);
|
||||
PhabricatorAuthSessionEngine::ONETIME_RECOVER,
|
||||
$force_full_session);
|
||||
|
||||
$console = PhutilConsole::getConsole();
|
||||
$console->writeOut(
|
||||
|
|
|
@ -106,6 +106,16 @@ final class PhabricatorAuthTemporaryToken extends PhabricatorAuthDAO
|
|||
return $this;
|
||||
}
|
||||
|
||||
public function setShouldForceFullSession($force_full) {
|
||||
return $this->setTemporaryTokenProperty('force-full-session', $force_full);
|
||||
}
|
||||
|
||||
public function getShouldForceFullSession() {
|
||||
return $this->getTemporaryTokenProperty('force-full-session', false);
|
||||
}
|
||||
|
||||
|
||||
|
||||
/* -( PhabricatorPolicyInterface )----------------------------------------- */
|
||||
|
||||
|
||||
|
|
Loading…
Reference in a new issue