Page MenuHomePhabricator
Files F15387837

D7978.id18056.diff
No OneTemporary
Actions

D7978.id18056.diff
View Options

Index: conf/default.conf.php
===================================================================
--- conf/default.conf.php
+++ conf/default.conf.php
@@ -543,15 +543,6 @@
// -- Auth ------------------------------------------------------------------ //
- // Maximum number of simultaneous web sessions each user is permitted to have.
- // Setting this to "1" will prevent a user from logging in on more than one
- // browser at the same time.
- 'auth.sessions.web' => 5,
-
- // Maximum number of simultaneous Conduit sessions each user is permitted
- // to have.
- 'auth.sessions.conduit' => 5,
-
// If true, email addresses must be verified (by clicking a link in an
// email) before a user can login. By default, verification is optional
// unless 'auth.email-domains' is nonempty (see below).
Index: resources/sql/autopatches/20140115.auth.3.unlimit.php
===================================================================
--- /dev/null
+++ resources/sql/autopatches/20140115.auth.3.unlimit.php
@@ -0,0 +1,26 @@
+<?php
+
+// Prior to this patch, we issued sessions "web-1", "web-2", etc., up to some
+// limit. This collapses all the "web-X" sessions into "web" sessions.
+
+$session_table = new PhabricatorAuthSession();
+$conn_w = $session_table->establishConnection('w');
+
+foreach (new LiskMigrationIterator($session_table) as $session) {
+ $id = $session->getID();
+
+ echo "Migrating session {$id}...\n";
+ $old_type = $session->getType();
+ $new_type = preg_replace('/-.*$/', '', $old_type);
+
+ if ($old_type !== $new_type) {
+ queryfx(
+ $conn_w,
+ 'UPDATE %T SET type = %s WHERE id = %d',
+ $session_table->getTableName(),
+ $new_type,
+ $id);
+ }
+}
+
+echo "Done.\n";
Index: src/applications/auth/engine/PhabricatorAuthSessionEngine.php
===================================================================
--- src/applications/auth/engine/PhabricatorAuthSessionEngine.php
+++ src/applications/auth/engine/PhabricatorAuthSessionEngine.php
@@ -14,10 +14,10 @@
$conn_r,
'SELECT s.sessionExpires AS _sessionExpires, s.id AS _sessionID, u.*
FROM %T u JOIN %T s ON u.phid = s.userPHID
- AND s.type LIKE %> AND s.sessionKey = %s',
+ AND s.type = %s AND s.sessionKey = %s',
$user_table->getTableName(),
$session_table->getTableName(),
- $session_type.'-',
+ $session_type,
PhabricatorHash::digest($session_key));
if (!$info) {
@@ -72,142 +72,35 @@
$session_table = new PhabricatorAuthSession();
$conn_w = $session_table->establishConnection('w');
- if (strpos($session_type, '-') !== false) {
- throw new Exception("Session type must not contain hyphen ('-')!");
- }
-
- // We allow multiple sessions of the same type, so when a caller requests
- // a new session of type "web", we give them the first available session in
- // "web-1", "web-2", ..., "web-N", up to some configurable limit. If none
- // of these sessions is available, we overwrite the oldest session and
- // reissue a new one in its place.
-
- $session_limit = 1;
- switch ($session_type) {
- case PhabricatorAuthSession::TYPE_WEB:
- $session_limit = PhabricatorEnv::getEnvConfig('auth.sessions.web');
- break;
- case PhabricatorAuthSession::TYPE_CONDUIT:
- $session_limit = PhabricatorEnv::getEnvConfig('auth.sessions.conduit');
- break;
- default:
- throw new Exception("Unknown session type '{$session_type}'!");
- }
-
- $session_limit = (int)$session_limit;
- if ($session_limit <= 0) {
- throw new Exception(
- "Session limit for '{$session_type}' must be at least 1!");
- }
-
- // NOTE: Session establishment is sensitive to race conditions, as when
- // piping `arc` to `arc`:
- //
- // arc export ... | arc paste ...
- //
- // To avoid this, we overwrite an old session only if it hasn't been
- // re-established since we read it.
-
// Consume entropy to generate a new session key, forestalling the eventual
// heat death of the universe.
$session_key = Filesystem::readRandomCharacters(40);
- $session_ttl = PhabricatorAuthSession::getSessionTypeTTL($session_type);
-
- // Load all the currently active sessions.
- $sessions = queryfx_all(
- $conn_w,
- 'SELECT type, sessionKey, sessionStart FROM %T
- WHERE userPHID = %s AND type LIKE %>',
- $session_table->getTableName(),
- $identity_phid,
- $session_type.'-');
- $sessions = ipull($sessions, null, 'type');
- $sessions = isort($sessions, 'sessionStart');
- $existing_sessions = array_keys($sessions);
+ // This has a side effect of validating the session type.
+ $session_ttl = PhabricatorAuthSession::getSessionTypeTTL($session_type);
- // UNGUARDED WRITES: Logging-in users don't have CSRF stuff yet.
+ // Logging-in users don't have CSRF stuff yet, so we have to unguard this
+ // write.
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
-
- $retries = 0;
- while (true) {
-
- // Choose which 'type' we'll actually establish, i.e. what number we're
- // going to append to the basic session type. To do this, just check all
- // the numbers sequentially until we find an available session.
- $establish_type = null;
- for ($ii = 1; $ii <= $session_limit; $ii++) {
- $try_type = $session_type.'-'.$ii;
- if (!in_array($try_type, $existing_sessions)) {
- $establish_type = $try_type;
- $expect_key = PhabricatorHash::digest($session_key);
- $existing_sessions[] = $try_type;
-
- // Ensure the row exists so we can issue an update below. We don't
- // care if we race here or not.
- queryfx(
- $conn_w,
- 'INSERT IGNORE INTO %T
- (userPHID, type, sessionKey, sessionStart, sessionExpires)
- VALUES (%s, %s, %s, 0, UNIX_TIMESTAMP() + %d)',
- $session_table->getTableName(),
- $identity_phid,
- $establish_type,
- PhabricatorHash::digest($session_key),
- $session_ttl);
- break;
- }
- }
-
- // If we didn't find an available session, choose the oldest session and
- // overwrite it.
- if (!$establish_type) {
- $oldest = reset($sessions);
- $establish_type = $oldest['type'];
- $expect_key = $oldest['sessionKey'];
- }
-
- // This is so that we'll only overwrite the session if it hasn't been
- // refreshed since we read it. If it has, the session key will be
- // different and we know we're racing other processes. Whichever one
- // won gets the session, we go back and try again.
-
- queryfx(
- $conn_w,
- 'UPDATE %T SET sessionKey = %s, sessionStart = UNIX_TIMESTAMP(),
- sessionExpires = UNIX_TIMESTAMP() + %d
- WHERE userPHID = %s AND type = %s AND sessionKey = %s',
- $session_table->getTableName(),
- PhabricatorHash::digest($session_key),
- $session_ttl,
+ id(new PhabricatorAuthSession())
+ ->setUserPHID($identity_phid)
+ ->setType($session_type)
+ ->setSessionKey(PhabricatorHash::digest($session_key))
+ ->setSessionStart(time())
+ ->setSessionExpires(time() + $session_ttl)
+ ->save();
+
+ $log = PhabricatorUserLog::initializeNewLog(
+ null,
$identity_phid,
- $establish_type,
- $expect_key);
-
- if ($conn_w->getAffectedRows()) {
- // The update worked, so the session is valid.
- break;
- } else {
- // We know this just got grabbed, so don't try it again.
- unset($sessions[$establish_type]);
- }
-
- if (++$retries > $session_limit) {
- throw new Exception("Failed to establish a session!");
- }
- }
-
- $log = PhabricatorUserLog::initializeNewLog(
- null,
- $identity_phid,
- PhabricatorUserLog::ACTION_LOGIN);
- $log->setDetails(
- array(
- 'session_type' => $session_type,
- 'session_issued' => $establish_type,
- ));
- $log->setSession($session_key);
- $log->save();
+ PhabricatorUserLog::ACTION_LOGIN);
+ $log->setDetails(
+ array(
+ 'session_type' => $session_type,
+ ));
+ $log->setSession($session_key);
+ $log->save();
+ unset($unguarded);
return $session_key;
}
Index: src/applications/auth/query/PhabricatorAuthSessionQuery.php
===================================================================
--- src/applications/auth/query/PhabricatorAuthSessionQuery.php
+++ src/applications/auth/query/PhabricatorAuthSessionQuery.php
@@ -81,14 +81,10 @@
}
if ($this->sessionTypes) {
- $clauses = array();
- foreach ($this->sessionTypes as $session_type) {
- $clauses[] = qsprintf(
- $conn_r,
- 'type LIKE %>',
- $session_type.'-');
- }
- $where[] = '('.implode(') OR (', $clauses).')';
+ $where[] = qsprintf(
+ $conn_r,
+ 'type IN (%Ls)',
+ $this->sessionTypes);
}
$where[] = $this->buildPagingClause($conn_r);
@@ -96,10 +92,6 @@
return $this->formatWhereClause($where);
}
- public function getPagingColumn() {
- return 'sessionKey';
- }
-
public function getQueryApplicationClass() {
return 'PhabricatorApplicationAuth';
}
Index: src/applications/config/check/PhabricatorSetupCheckExtraConfig.php
===================================================================
--- src/applications/config/check/PhabricatorSetupCheckExtraConfig.php
+++ src/applications/config/check/PhabricatorSetupCheckExtraConfig.php
@@ -146,6 +146,10 @@
'PhabricatorRemarkupCustomInlineRule or '.
'PhabricatorRemarkupCustomBlockRule.');
+ $session_reason = pht(
+ 'Sessions now expire and are garbage collected rather than having an '.
+ 'arbitrary concurrency limit.');
+
$ancient_config += array(
'phid.external-loaders' =>
pht(
@@ -172,6 +176,8 @@
'multiple maps. See T4222.'),
'metamta.send-immediately' => pht(
'Mail is now always delivered by the daemons.'),
+ 'auth.sessions.conduit' => $session_reason,
+ 'auth.sessions.web' => $session_reason,
);
return $ancient_config;
Index: src/applications/config/option/PhabricatorAuthenticationConfigOptions.php
===================================================================
--- src/applications/config/option/PhabricatorAuthenticationConfigOptions.php
+++ src/applications/config/option/PhabricatorAuthenticationConfigOptions.php
@@ -13,22 +13,6 @@
public function getOptions() {
return array(
- $this->newOption('auth.sessions.web', 'int', 5)
- ->setSummary(
- pht("Number of web sessions a user can have simultaneously."))
- ->setDescription(
- pht(
- "Maximum number of simultaneous web sessions each user is ".
- "permitted to have. Setting this to '1' will prevent a user from ".
- "logging in on more than one browser at the same time.")),
- $this->newOption('auth.sessions.conduit', 'int', 5)
- ->setSummary(
- pht(
- "Number of simultaneous Conduit sessions each user is permitted."))
- ->setDescription(
- pht(
- "Maximum number of simultaneous Conduit sessions each user is ".
- "permitted to have.")),
$this->newOption('auth.require-email-verification', 'bool', false)
->setBoolOptions(
array(

File Metadata

Mime Type
text/plain
Expires
Sun, Mar 16, 2:59 AM (6 d, 17 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7222077
Default Alt Text
D7978.id18056.diff (11 KB)

Event Timeline

Phabricator · Built by Phacility