diff --git a/resources/sql/autopatches/20140430.auth.1.partial.sql b/resources/sql/autopatches/20140430.auth.1.partial.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20140430.auth.1.partial.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_user.phabricator_session + ADD isPartial BOOL NOT NULL DEFAULT 0; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1213,6 +1213,7 @@ 'PhabricatorAuthFactorConfig' => 'applications/auth/storage/PhabricatorAuthFactorConfig.php', 'PhabricatorAuthFactorTOTP' => 'applications/auth/factor/PhabricatorAuthFactorTOTP.php', 'PhabricatorAuthFactorTOTPTestCase' => 'applications/auth/factor/__tests__/PhabricatorAuthFactorTOTPTestCase.php', + 'PhabricatorAuthFinishController' => 'applications/auth/controller/PhabricatorAuthFinishController.php', 'PhabricatorAuthHighSecurityRequiredException' => 'applications/auth/exception/PhabricatorAuthHighSecurityRequiredException.php', 'PhabricatorAuthHighSecurityToken' => 'applications/auth/data/PhabricatorAuthHighSecurityToken.php', 'PhabricatorAuthLinkController' => 'applications/auth/controller/PhabricatorAuthLinkController.php', @@ -3980,6 +3981,7 @@ 'PhabricatorAuthFactorConfig' => 'PhabricatorAuthDAO', 'PhabricatorAuthFactorTOTP' => 'PhabricatorAuthFactor', 'PhabricatorAuthFactorTOTPTestCase' => 'PhabricatorTestCase', + 'PhabricatorAuthFinishController' => 'PhabricatorAuthController', 'PhabricatorAuthHighSecurityRequiredException' => 'Exception', 'PhabricatorAuthLinkController' => 'PhabricatorAuthController', 'PhabricatorAuthListController' => 'PhabricatorAuthProviderConfigController', diff --git a/src/aphront/console/DarkConsoleController.php b/src/aphront/console/DarkConsoleController.php --- a/src/aphront/console/DarkConsoleController.php +++ b/src/aphront/console/DarkConsoleController.php @@ -16,6 +16,10 @@ return !PhabricatorEnv::getEnvConfig('darkconsole.always-on'); } + public function shouldAllowPartialSessions() { + return true; + } + public function processRequest() { $request = $this->getRequest(); $user = $request->getUser(); diff --git a/src/aphront/console/DarkConsoleDataController.php b/src/aphront/console/DarkConsoleDataController.php --- a/src/aphront/console/DarkConsoleDataController.php +++ b/src/aphront/console/DarkConsoleDataController.php @@ -15,6 +15,10 @@ return !PhabricatorEnv::getEnvConfig('darkconsole.always-on'); } + public function shouldAllowPartialSessions() { + return true; + } + public function willProcessRequest(array $data) { $this->key = $data['key']; } diff --git a/src/applications/auth/application/PhabricatorApplicationAuth.php b/src/applications/auth/application/PhabricatorApplicationAuth.php --- a/src/applications/auth/application/PhabricatorApplicationAuth.php +++ b/src/applications/auth/application/PhabricatorApplicationAuth.php @@ -83,6 +83,7 @@ 'register/(?:(?P[^/]+)/)?' => 'PhabricatorAuthRegisterController', 'start/' => 'PhabricatorAuthStartController', 'validate/' => 'PhabricatorAuthValidateController', + 'finish/' => 'PhabricatorAuthFinishController', 'unlink/(?P[^/]+)/' => 'PhabricatorAuthUnlinkController', '(?Plink|refresh)/(?P[^/]+)/' => 'PhabricatorAuthLinkController', diff --git a/src/applications/auth/controller/PhabricatorAuthController.php b/src/applications/auth/controller/PhabricatorAuthController.php --- a/src/applications/auth/controller/PhabricatorAuthController.php +++ b/src/applications/auth/controller/PhabricatorAuthController.php @@ -82,7 +82,7 @@ $should_login = $event->getValue('shouldLogin'); if ($should_login) { $session_key = id(new PhabricatorAuthSessionEngine()) - ->establishSession($session_type, $user->getPHID()); + ->establishSession($session_type, $user->getPHID(), $partial = true); // NOTE: We allow disabled users to login and roadblock them later, so // there's no check for users being disabled here. diff --git a/src/applications/auth/controller/PhabricatorAuthFinishController.php b/src/applications/auth/controller/PhabricatorAuthFinishController.php new file mode 100644 --- /dev/null +++ b/src/applications/auth/controller/PhabricatorAuthFinishController.php @@ -0,0 +1,71 @@ +getRequest(); + $viewer = $request->getUser(); + + // If the user already has a full session, just kick them out of here. + $has_partial_session = $viewer->hasSession() && + $viewer->getSession()->getIsPartial(); + if (!$has_partial_session) { + return id(new AphrontRedirectResponse())->setURI('/'); + } + + $engine = new PhabricatorAuthSessionEngine(); + + try { + $token = $engine->requireHighSecuritySession( + $viewer, + $request, + '/logout/'); + } catch (PhabricatorAuthHighSecurityRequiredException $ex) { + $form = id(new PhabricatorAuthSessionEngine())->renderHighSecurityForm( + $ex->getFactors(), + $ex->getFactorValidationResults(), + $viewer, + $request); + + return $this->newDialog() + ->setTitle(pht('Provide Multi-Factor Credentials')) + ->setShortTitle(pht('Multi-Factor Login')) + ->setWidth(AphrontDialogView::WIDTH_FORM) + ->addHiddenInput(AphrontRequest::TYPE_HISEC, true) + ->appendParagraph( + pht( + 'Welcome, %s. To complete the login process, provide your '. + 'multi-factor credentials.', + phutil_tag('strong', array(), $viewer->getUsername()))) + ->appendChild($form->buildLayoutView()) + ->setSubmitURI($request->getPath()) + ->addCancelButton($ex->getCancelURI()) + ->addSubmitButton(pht('Continue')); + } + + // Upgrade the partial session to a full session. + $engine->upgradePartialSession($viewer); + + // TODO: It might be nice to add options like "bind this session to my IP" + // here, even for accounts without multi-factor auth attached to them. + + $next = PhabricatorCookies::getNextURICookie($request); + $request->clearCookie(PhabricatorCookies::COOKIE_NEXTURI); + + if (!PhabricatorEnv::isValidLocalWebResource($next)) { + $next = '/'; + } + + return id(new AphrontRedirectResponse())->setURI($next); + } + +} diff --git a/src/applications/auth/controller/PhabricatorAuthValidateController.php b/src/applications/auth/controller/PhabricatorAuthValidateController.php --- a/src/applications/auth/controller/PhabricatorAuthValidateController.php +++ b/src/applications/auth/controller/PhabricatorAuthValidateController.php @@ -7,6 +7,10 @@ return false; } + public function shouldAllowPartialSessions() { + return true; + } + public function processRequest() { $request = $this->getRequest(); $viewer = $request->getUser(); @@ -54,14 +58,8 @@ return $this->renderErrors($failures); } - $next = PhabricatorCookies::getNextURICookie($request); - $request->clearCookie(PhabricatorCookies::COOKIE_NEXTURI); - - if (!PhabricatorEnv::isValidLocalWebResource($next)) { - $next = '/'; - } - - return id(new AphrontRedirectResponse())->setURI($next); + $finish_uri = $this->getApplicationURI('finish/'); + return id(new AphrontRedirectResponse())->setURI($finish_uri); } private function renderErrors(array $messages) { diff --git a/src/applications/auth/controller/PhabricatorLogoutController.php b/src/applications/auth/controller/PhabricatorLogoutController.php --- a/src/applications/auth/controller/PhabricatorLogoutController.php +++ b/src/applications/auth/controller/PhabricatorLogoutController.php @@ -17,6 +17,10 @@ return false; } + public function shouldAllowPartialSessions() { + return true; + } + public function processRequest() { $request = $this->getRequest(); $user = $request->getUser(); diff --git a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php --- a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php +++ b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php @@ -94,6 +94,7 @@ s.sessionExpires AS s_sessionExpires, s.sessionStart AS s_sessionStart, s.highSecurityUntil AS s_highSecurityUntil, + s.isPartial AS s_isPartial, u.* FROM %T u JOIN %T s ON u.phid = s.userPHID AND s.type = %s AND s.sessionKey = %s', @@ -159,9 +160,10 @@ * @{class:PhabricatorAuthSession}). * @param phid|null Identity to establish a session for, usually a user * PHID. With `null`, generates an anonymous session. + * @param bool True to issue a partial session. * @return string Newly generated session key. */ - public function establishSession($session_type, $identity_phid) { + public function establishSession($session_type, $identity_phid, $partial) { // Consume entropy to generate a new session key, forestalling the eventual // heat death of the universe. $session_key = Filesystem::readRandomCharacters(40); @@ -176,26 +178,32 @@ // This has a side effect of validating the session type. $session_ttl = PhabricatorAuthSession::getSessionTypeTTL($session_type); + $digest_key = PhabricatorHash::digest($session_key); + // Logging-in users don't have CSRF stuff yet, so we have to unguard this // write. $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); id(new PhabricatorAuthSession()) ->setUserPHID($identity_phid) ->setType($session_type) - ->setSessionKey(PhabricatorHash::digest($session_key)) + ->setSessionKey($digest_key) ->setSessionStart(time()) ->setSessionExpires(time() + $session_ttl) + ->setIsPartial($partial ? 1 : 0) ->save(); $log = PhabricatorUserLog::initializeNewLog( null, $identity_phid, - PhabricatorUserLog::ACTION_LOGIN); + ($partial + ? PhabricatorUserLog::ACTION_LOGIN_PARTIAL + : PhabricatorUserLog::ACTION_LOGIN)); + $log->setDetails( array( 'session_type' => $session_type, )); - $log->setSession($session_key); + $log->setSession($digest_key); $log->save(); unset($unguarded); @@ -287,6 +295,12 @@ new PhabricatorAuthTryFactorAction(), -1); + if ($session->getIsPartial()) { + // If we have a partial session, just issue a token without + // putting it in high security mode. + return $this->issueHighSecurityToken($session, true); + } + $until = time() + phutil_units('15 minutes in seconds'); $session->setHighSecurityUntil($until); @@ -303,9 +317,6 @@ PhabricatorUserLog::ACTION_ENTER_HISEC); $log->save(); } else { - - - $log = PhabricatorUserLog::initializeNewLog( $viewer, $viewer->getPHID(), @@ -331,11 +342,15 @@ * Issue a high security token for a session, if authorized. * * @param PhabricatorAuthSession Session to issue a token for. + * @param bool Force token issue. * @return PhabricatorAuthHighSecurityToken|null Token, if authorized. */ - private function issueHighSecurityToken(PhabricatorAuthSession $session) { + private function issueHighSecurityToken( + PhabricatorAuthSession $session, + $force = false) { + $until = $session->getHighSecurityUntil(); - if ($until > time()) { + if ($until > time() || $force) { return new PhabricatorAuthHighSecurityToken(); } return null; @@ -390,4 +405,42 @@ $log->save(); } + + /** + * Upgrade a partial session to a full session. + * + * @param PhabricatorAuthSession Session to upgrade. + * @return void + */ + public function upgradePartialSession(PhabricatorUser $viewer) { + if (!$viewer->hasSession()) { + throw new Exception( + pht('Upgrading partial session of user with no session!')); + } + + $session = $viewer->getSession(); + + if (!$session->getIsPartial()) { + throw new Exception(pht('Session is not partial!')); + } + + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + $session->setIsPartial(0); + + queryfx( + $session->establishConnection('w'), + 'UPDATE %T SET isPartial = %d WHERE id = %d', + $session->getTableName(), + 0, + $session->getID()); + + $log = PhabricatorUserLog::initializeNewLog( + $viewer, + $viewer->getPHID(), + PhabricatorUserLog::ACTION_LOGIN_FULL); + $log->save(); + unset($unguarded); + + } + } diff --git a/src/applications/auth/storage/PhabricatorAuthSession.php b/src/applications/auth/storage/PhabricatorAuthSession.php --- a/src/applications/auth/storage/PhabricatorAuthSession.php +++ b/src/applications/auth/storage/PhabricatorAuthSession.php @@ -12,6 +12,7 @@ protected $sessionStart; protected $sessionExpires; protected $highSecurityUntil; + protected $isPartial; private $identityObject = self::ATTACHABLE; diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php --- a/src/applications/base/controller/PhabricatorController.php +++ b/src/applications/base/controller/PhabricatorController.php @@ -20,6 +20,10 @@ return false; } + public function shouldAllowPartialSessions() { + return false; + } + public function shouldRequireEmailVerification() { return PhabricatorUserEmail::isEmailVerificationRequired(); } @@ -53,7 +57,8 @@ // session. This is used to provide CSRF protection to logged-out users. $phsid = $session_engine->establishSession( PhabricatorAuthSession::TYPE_WEB, - null); + null, + $partial = false); // This may be a resource request, in which case we just don't set // the cookie. @@ -133,14 +138,24 @@ return $this->delegateToController($checker_controller); } + $auth_class = 'PhabricatorApplicationAuth'; + $auth_application = PhabricatorApplication::getByClass($auth_class); + + // Require partial sessions to finish login before doing anything. + if (!$this->shouldAllowPartialSessions()) { + if ($user->hasSession() && + $user->getSession()->getIsPartial()) { + $login_controller = new PhabricatorAuthFinishController($request); + $this->setCurrentApplication($auth_application); + return $this->delegateToController($login_controller); + } + } + if ($this->shouldRequireLogin()) { // This actually means we need either: // - a valid user, or a public controller; and // - permission to see the application. - $auth_class = 'PhabricatorApplicationAuth'; - $auth_application = PhabricatorApplication::getByClass($auth_class); - $allow_public = $this->shouldAllowPublic() && PhabricatorEnv::getEnvConfig('policy.allow-public'); diff --git a/src/applications/conduit/method/ConduitAPI_conduit_connect_Method.php b/src/applications/conduit/method/ConduitAPI_conduit_connect_Method.php --- a/src/applications/conduit/method/ConduitAPI_conduit_connect_Method.php +++ b/src/applications/conduit/method/ConduitAPI_conduit_connect_Method.php @@ -145,10 +145,10 @@ if ($valid != $signature) { throw new ConduitException('ERR-INVALID-CERTIFICATE'); } - $session_key = id(new PhabricatorAuthSessionEngine()) - ->establishSession( - PhabricatorAuthSession::TYPE_CONDUIT, - $user->getPHID()); + $session_key = id(new PhabricatorAuthSessionEngine())->establishSession( + PhabricatorAuthSession::TYPE_CONDUIT, + $user->getPHID(), + $partial = false); } else { throw new ConduitException('ERR-NO-CERTIFICATE'); } diff --git a/src/applications/people/storage/PhabricatorUserLog.php b/src/applications/people/storage/PhabricatorUserLog.php --- a/src/applications/people/storage/PhabricatorUserLog.php +++ b/src/applications/people/storage/PhabricatorUserLog.php @@ -4,6 +4,8 @@ implements PhabricatorPolicyInterface { const ACTION_LOGIN = 'login'; + const ACTION_LOGIN_PARTIAL = 'login-partial'; + const ACTION_LOGIN_FULL = 'login-full'; const ACTION_LOGOUT = 'logout'; const ACTION_LOGIN_FAILURE = 'login-fail'; const ACTION_RESET_PASSWORD = 'reset-pass'; @@ -46,7 +48,9 @@ public static function getActionTypeMap() { return array( self::ACTION_LOGIN => pht('Login'), - self::ACTION_LOGIN_FAILURE => pht('Login Failure'), + self::ACTION_LOGIN_PARTIAL => pht('Login: Partial Login'), + self::ACTION_LOGIN_FULL => pht('Login: Upgrade to Full'), + self::ACTION_LOGIN_FAILURE => pht('Login: Failure'), self::ACTION_LOGOUT => pht('Logout'), self::ACTION_RESET_PASSWORD => pht('Reset Password'), self::ACTION_CREATE => pht('Create Account'), diff --git a/src/infrastructure/celerity/CelerityResourceController.php b/src/infrastructure/celerity/CelerityResourceController.php --- a/src/infrastructure/celerity/CelerityResourceController.php +++ b/src/infrastructure/celerity/CelerityResourceController.php @@ -14,6 +14,10 @@ return false; } + public function shouldAllowPartialSessions() { + return true; + } + abstract public function getCelerityResourceMap(); protected function serveResource($path, $package_hash = null) {