Page MenuHomePhabricator
Files F14007574

D8043.diff
No OneTemporary
Actions

D8043.diff
View Options

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();

File Metadata

Mime Type
text/plain
Expires
Wed, Oct 30, 8:13 AM (2 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6731401
Default Alt Text
D8043.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility