Index: src/applications/auth/controller/PhabricatorAuthStartController.php =================================================================== --- src/applications/auth/controller/PhabricatorAuthStartController.php +++ src/applications/auth/controller/PhabricatorAuthStartController.php @@ -24,18 +24,33 @@ return $this->processConduitRequest(); } - if (strlen($request->getCookie(PhabricatorCookies::COOKIE_SESSION))) { - // The session cookie is invalid, so clear it. - $request->clearCookie(PhabricatorCookies::COOKIE_USERNAME); - $request->clearCookie(PhabricatorCookies::COOKIE_SESSION); - - return $this->renderError( - pht( - "Your login session is invalid. Try reloading the page and logging ". - "in again. If that does not work, clear your browser cookies.")); + // If the user gets this far, they aren't logged in, so if they have a + // user session token we can conclude that it's invalid: if it was valid, + // they'd have been logged in above and never made it here. Try to clear + // it and warn the user they may need to nuke their cookies. + + $session_token = $request->getCookie(PhabricatorCookies::COOKIE_SESSION); + if (strlen($session_token)) { + $kind = PhabricatorAuthSessionEngine::getSessionKindFromToken( + $session_token); + switch ($kind) { + case PhabricatorAuthSessionEngine::KIND_ANONYMOUS: + // If this is an anonymous session. It's expected that they won't + // be logged in, so we can just continue. + break; + default: + // The session cookie is invalid, so clear it. + $request->clearCookie(PhabricatorCookies::COOKIE_USERNAME); + $request->clearCookie(PhabricatorCookies::COOKIE_SESSION); + + return $this->renderError( + pht( + "Your login session is invalid. Try reloading the page and ". + "logging in again. If that does not work, clear your browser ". + "cookies.")); + } } - $providers = PhabricatorAuthProvider::getAllEnabledProviders(); foreach ($providers as $key => $provider) { if (!$provider->shouldAllowLogin()) { Index: src/applications/auth/engine/PhabricatorAuthSessionEngine.php =================================================================== --- src/applications/auth/engine/PhabricatorAuthSessionEngine.php +++ src/applications/auth/engine/PhabricatorAuthSessionEngine.php @@ -2,7 +2,79 @@ final class PhabricatorAuthSessionEngine extends Phobject { - public function loadUserForSession($session_type, $session_key) { + /** + * Session issued to normal users after they login through a standard channel. + * Associates the client with a standard user identity. + */ + const KIND_USER = 'U'; + + + /** + * Session issued to users who login with some sort of credentials but do not + * have full accounts. These are sometimes called "grey users". + * + * TODO: We do not currently issue these sessions, see T4310. + */ + const KIND_EXTERNAL = 'X'; + + + /** + * Session issued to logged-out users which has no real identity information. + * Its purpose is to protect logged-out users from CSRF. + */ + const KIND_ANONYMOUS = 'A'; + + + /** + * Session kind isn't known. + */ + const KIND_UNKNOWN = '?'; + + + /** + * Get the session kind (e.g., anonymous, user, external account) from a + * session token. Returns a `KIND_` constant. + * + * @param string Session token. + * @return const Session kind constant. + */ + public static function getSessionKindFromToken($session_token) { + if (strpos($session_token, '/') === false) { + // Old-style session, these are all user sessions. + return self::KIND_USER; + } + + list($kind, $key) = explode('/', $session_token, 2); + + switch ($kind) { + case self::KIND_ANONYMOUS: + case self::KIND_USER: + case self::KIND_EXTERNAL: + return $kind; + default: + return self::KIND_UNKNOWN; + } + } + + + public function loadUserForSession($session_type, $session_token) { + $session_kind = self::getSessionKindFromToken($session_token); + switch ($session_kind) { + case self::KIND_ANONYMOUS: + // Don't bother trying to load a user for an anonymous session, since + // neither the session nor the user exist. + return null; + case self::KIND_UNKNOWN: + // If we don't know what kind of session this is, don't go looking for + // it. + return null; + case self::KIND_USER: + break; + case self::KIND_EXTERNAL: + // TODO: Implement these (T4310). + return null; + } + $session_table = new PhabricatorAuthSession(); $user_table = new PhabricatorUser(); $conn_r = $session_table->establishConnection('r'); @@ -18,7 +90,7 @@ $user_table->getTableName(), $session_table->getTableName(), $session_type, - PhabricatorHash::digest($session_key)); + PhabricatorHash::digest($session_token)); if (!$info) { return null; @@ -63,19 +135,24 @@ * You can configure the maximum number of concurrent sessions for various * session types in the Phabricator configuration. * - * @param const Session type constant (see - * @{class:PhabricatorAuthSession}). - * @param phid Identity to establish a session for, usually a user PHID. - * @return string Newly generated session key. + * @param const Session type constant (see + * @{class:PhabricatorAuthSession}). + * @param phid|null Identity to establish a session for, usually a user + * PHID. With `null`, generates an anonymous session. + * @return string Newly generated session key. */ public function establishSession($session_type, $identity_phid) { - $session_table = new PhabricatorAuthSession(); - $conn_w = $session_table->establishConnection('w'); - // Consume entropy to generate a new session key, forestalling the eventual // heat death of the universe. $session_key = Filesystem::readRandomCharacters(40); + if ($identity_phid === null) { + return self::KIND_ANONYMOUS.'/'.$session_key; + } + + $session_table = new PhabricatorAuthSession(); + $conn_w = $session_table->establishConnection('w'); + // This has a side effect of validating the session type. $session_ttl = PhabricatorAuthSession::getSessionTypeTTL($session_type); Index: src/applications/auth/storage/PhabricatorAuthSession.php =================================================================== --- src/applications/auth/storage/PhabricatorAuthSession.php +++ src/applications/auth/storage/PhabricatorAuthSession.php @@ -50,7 +50,6 @@ } } - /* -( PhabricatorPolicyInterface )----------------------------------------- */ Index: src/applications/base/controller/PhabricatorController.php =================================================================== --- src/applications/base/controller/PhabricatorController.php +++ src/applications/base/controller/PhabricatorController.php @@ -36,7 +36,7 @@ $user = new PhabricatorUser(); $phsid = $request->getCookie(PhabricatorCookies::COOKIE_SESSION); - if ($phsid) { + if (strlen($phsid)) { $session_user = id(new PhabricatorAuthSessionEngine()) ->loadUserForSession(PhabricatorAuthSession::TYPE_WEB, $phsid); if ($session_user) { Index: src/applications/people/storage/PhabricatorUserLog.php =================================================================== --- src/applications/people/storage/PhabricatorUserLog.php +++ src/applications/people/storage/PhabricatorUserLog.php @@ -68,6 +68,7 @@ if (!$this->session) { // TODO: This is not correct if there's a cookie prefix. This object // should take an AphrontRequest. + // TODO: Maybe record session kind, or drop this for anonymous sessions? $this->setSession(idx($_COOKIE, PhabricatorCookies::COOKIE_SESSION)); } $this->details['host'] = php_uname('n'); Index: src/view/page/PhabricatorStandardPageView.php =================================================================== --- src/view/page/PhabricatorStandardPageView.php +++ src/view/page/PhabricatorStandardPageView.php @@ -163,6 +163,17 @@ Javelin::initBehavior('history-install'); Javelin::initBehavior('phabricator-gesture'); + // If the client doesn't have a session token, generate an anonymous + // session. This is used to provide CSRF protection to logged-out users. + $session_token = $request->getCookie(PhabricatorCookies::COOKIE_SESSION); + if (!strlen($session_token)) { + $anonymous_session = id(new PhabricatorAuthSessionEngine()) + ->establishSession('web', null); + $request->setCookie( + PhabricatorCookies::COOKIE_SESSION, + $anonymous_session); + } + $current_token = null; if ($user) { $current_token = $user->getCSRFToken();