summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDries Buytaert2010-06-14 12:31:46 (GMT)
committerDries Buytaert2010-06-14 12:31:46 (GMT)
commit1cd8bc5a0048afd8c21c411adc5001db9bfa1dce (patch)
treebb385e0be90421580f209ffa125c1423e38f39b1
parent5eefb7ab89fea2d33c59c089c349d70be73badcf (diff)
- Patch #813492 by chx, Damien Tournoud, justinrandell: HTTPS sessions use an invalid merge query.
-rw-r--r--includes/session.inc45
-rw-r--r--modules/simpletest/tests/session.test45
-rw-r--r--modules/system/system.install12
3 files changed, 83 insertions, 19 deletions
diff --git a/includes/session.inc b/includes/session.inc
index ce5524a..2d2c2b6 100644
--- a/includes/session.inc
+++ b/includes/session.inc
@@ -147,6 +147,7 @@ function _drupal_session_write($sid, $value) {
return;
}
+ // Either ssid or sid or both will be added from $key below.
$fields = array(
'uid' => $user->uid,
'cache' => isset($user->cache) ? $user->cache : 0,
@@ -154,17 +155,22 @@ function _drupal_session_write($sid, $value) {
'session' => $value,
'timestamp' => REQUEST_TIME,
);
- $key = array('sid' => $sid);
+
+ // The "secure pages" setting allows a site to simultaneously use both secure
+ // and insecure session cookies. If enabled and both cookies are presented
+ // then use both keys. If not enabled but on HTTPS then use the PHP session
+ // id as 'ssid'. If on HTTP then use the PHP session id as 'sid'.
if ($is_https) {
$key['ssid'] = $sid;
$insecure_session_name = substr(session_name(), 1);
- // The "secure pages" setting allows a site to simultaneously use both
- // secure and insecure session cookies. If enabled, use the insecure session
- // identifier as the sid.
if (variable_get('https', FALSE) && isset($_COOKIE[$insecure_session_name])) {
$key['sid'] = $_COOKIE[$insecure_session_name];
}
}
+ else {
+ $key['sid'] = $sid;
+ }
+
db_merge('sessions')
->key($key)
->fields($fields)
@@ -198,11 +204,11 @@ function _drupal_session_write($sid, $value) {
* Initialize the session handler, starting a session if needed.
*/
function drupal_session_initialize() {
- global $user;
+ global $user, $is_https;
session_set_save_handler('_drupal_session_open', '_drupal_session_close', '_drupal_session_read', '_drupal_session_write', '_drupal_session_destroy', '_drupal_session_garbage_collection');
- if (isset($_COOKIE[session_name()])) {
+ if (isset($_COOKIE[session_name()]) || ($is_https && variable_get('https', FALSE) && isset($_COOKIE[substr(session_name(), 1)]))) {
// If a session cookie exists, initialize the session. Otherwise the
// session is only started on demand in drupal_session_commit(), making
// anonymous users not use a session cookie unless something is stored in
@@ -298,6 +304,9 @@ function drupal_session_regenerate() {
global $user, $is_https;
if ($is_https && variable_get('https', FALSE)) {
$insecure_session_name = substr(session_name(), 1);
+ if (isset($_COOKIE[$insecure_session_name])) {
+ $old_insecure_session_id = $_COOKIE[$insecure_session_name];
+ }
$params = session_get_cookie_params();
$session_id = drupal_hash_base64(uniqid(mt_rand(), TRUE) . drupal_random_bytes(55));
setcookie($insecure_session_name, $session_id, REQUEST_TIME + $params['lifetime'], $params['path'], $params['domain'], FALSE, $params['httponly']);
@@ -318,11 +327,27 @@ function drupal_session_regenerate() {
}
if (isset($old_session_id)) {
+ $fields = array('sid' => session_id());
+ if ($is_https) {
+ $fields['ssid'] = session_id();
+ // If the "secure pages" setting is enabled, use the newly-created
+ // insecure session identifier as the regenerated sid.
+ if (variable_get('https', FALSE)) {
+ $fields['sid'] = $session_id;
+ }
+ }
+ db_update('sessions')
+ ->fields($fields)
+ ->condition($is_https ? 'ssid' : 'sid', $old_session_id)
+ ->execute();
+ }
+ elseif (isset($old_insecure_session_id)) {
+ // If logging in to the secure site, and there was no active session on the
+ // secure site but a session was active on the insecure site, update the
+ // insecure session with the new session identifiers.
db_update('sessions')
- ->fields(array(
- $is_https ? 'ssid' : 'sid' => session_id()
- ))
- ->condition('sid', $old_session_id)
+ ->fields(array('sid' => $session_id, 'ssid' => session_id()))
+ ->condition('sid', $old_insecure_session_id)
->execute();
}
date_default_timezone_set(drupal_get_user_timezone());
diff --git a/modules/simpletest/tests/session.test b/modules/simpletest/tests/session.test
index 1b74405..600843b 100644
--- a/modules/simpletest/tests/session.test
+++ b/modules/simpletest/tests/session.test
@@ -274,7 +274,7 @@ class SessionHttpsTestCase extends DrupalWebTestCase {
// Check insecure cookie is not set.
$this->assertFalse(isset($this->cookies[$insecure_session_name]));
$ssid = $this->cookies[$secure_session_name]['value'];
- $this->assertSessionIds($ssid, $ssid, 'Session has two secure SIDs');
+ $this->assertSessionIds('', $ssid, 'Session has NULL for SID and a correct secure SID.');
$cookie = $secure_session_name . '=' . $ssid;
// Verify that user is logged in on secure URL.
@@ -303,12 +303,18 @@ class SessionHttpsTestCase extends DrupalWebTestCase {
variable_set('https', TRUE);
$this->curlClose();
- $this->drupalGet('session-test/set/1');
+ // Start an anonymous session on the insecure site.
+ $session_data = $this->randomName();
+ $this->drupalGet('session-test/set/' . $session_data);
// Check secure cookie on insecure page.
$this->assertFalse(isset($this->cookies[$secure_session_name]), 'The secure cookie is not sent on insecure pages.');
// Check insecure cookie on insecure page.
$this->assertFalse($this->cookies[$insecure_session_name]['secure'], 'The insecure cookie does not have the secure attribute');
+ // Store the anonymous cookie so we can validate that its session is killed
+ // after login.
+ $anonymous_cookie = $insecure_session_name . '=' . $this->cookies[$insecure_session_name]['value'];
+
// Check that password request form action is not secure.
$this->drupalGet('user/password');
$form = $this->xpath('//form[@id="user-pass"]');
@@ -339,6 +345,11 @@ class SessionHttpsTestCase extends DrupalWebTestCase {
$secure_session_name . '=' . $ssid,
);
+ // Test that session data saved before login is still available on the
+ // authenticated session.
+ $this->drupalGet('session-test/get');
+ $this->assertText($session_data, 'Session correctly returned the stored data set by the anonymous session.');
+
foreach ($cookies as $cookie_key => $cookie) {
foreach (array('admin/config', $this->httpsUrl('admin/config')) as $url_key => $url) {
$this->curlClose();
@@ -354,6 +365,33 @@ class SessionHttpsTestCase extends DrupalWebTestCase {
}
}
}
+
+ // Test that session data saved before login is not available using the
+ // pre-login anonymous cookie.
+ $this->cookies = array();
+ $this->drupalGet('session-test/get', array('Cookie: ' . $anonymous_cookie));
+ $this->assertNoText($session_data, 'Initial anonymous session is inactive after login.');
+
+ // Clear browser cookie jar.
+ $this->cookies = array();
+
+ // Start an anonymous session on the secure site.
+ $this->drupalGet($this->httpsUrl('session-test/set/1'));
+
+ // Mock a login to the secure site using the secure session cookie.
+ $this->drupalGet('user');
+ $form = $this->xpath('//form[@id="user-login"]');
+ $form[0]['action'] = $this->httpsUrl('user');
+ $this->drupalPost(NULL, $edit, t('Log in'), array(), array('Cookie: ' . $secure_session_name . '=' . $this->cookies[$secure_session_name]['value']));
+
+ // Get the insecure session cookie set by the secure login POST request.
+ $headers = $this->drupalGetHeaders(TRUE);
+ strtok($headers[0]['set-cookie'], ';=');
+ $session_id = strtok(';=');
+
+ // Test that the user is also authenticated on the insecure site.
+ $this->drupalGet("user/{$user->uid}/edit", array(), array('Cookie: ' . $insecure_session_name . '=' . $session_id));
+ $this->assertResponse(200);
}
/**
@@ -375,7 +413,7 @@ class SessionHttpsTestCase extends DrupalWebTestCase {
':sid' => $sid,
':ssid' => $ssid,
);
- return $this->assertTrue(db_query('SELECT sid FROM {sessions} WHERE sid = :sid AND ssid = :ssid', $args)->fetchField(), $assertion_text);
+ return $this->assertTrue(db_query('SELECT timestamp FROM {sessions} WHERE sid = :sid AND ssid = :ssid', $args)->fetchField(), $assertion_text);
}
protected function httpsUrl($url) {
@@ -383,3 +421,4 @@ class SessionHttpsTestCase extends DrupalWebTestCase {
return $base_url . '/modules/simpletest/tests/https.php?q=' . $url;
}
}
+
diff --git a/modules/system/system.install b/modules/system/system.install
index 9a57f1b..c6d0470 100644
--- a/modules/system/system.install
+++ b/modules/system/system.install
@@ -1413,8 +1413,8 @@ function system_schema() {
'description' => "Unique key: Secure session ID. The value is generated by PHP's Session API.",
'type' => 'varchar',
'length' => 64,
- 'not null' => FALSE,
- 'default' => NULL,
+ 'not null' => TRUE,
+ 'default' => '',
),
'hostname' => array(
'description' => 'The IP address that last used this session ID (sid).',
@@ -1442,14 +1442,14 @@ function system_schema() {
'size' => 'big',
),
),
- 'primary key' => array('sid'),
+ 'primary key' => array(
+ 'sid',
+ 'ssid',
+ ),
'indexes' => array(
'timestamp' => array('timestamp'),
'uid' => array('uid'),
),
- 'unique keys' => array(
- 'ssid' => array('ssid'),
- ),
'foreign keys' => array(
'uid' => array('users' => 'uid'),
),