#144538 by Damien Tournoud: harden the logout link. From: damz --- menu.inc | 17 +++++++++++++++++ menu/menu.test | 2 +- simpletest/drupal_web_test_case.php | 34 +++++++++++++++++++++++++++++++++- system/system.install | 4 ++-- user/user.module | 2 +- user/user.test | 33 +++++++++++++++++++++++++++++++++ 6 files changed, 87 insertions(+), 5 deletions(-) diff --git includes/menu.inc includes/menu.inc index 6ea540b..47d63dd 100644 --- includes/menu.inc +++ includes/menu.inc @@ -669,11 +669,28 @@ function _menu_link_map_translate(&$map, $to_arg_functions) { } } +/** + * Menu to arg function: convert %menu_tail by the tail used in the current page. + */ function menu_tail_to_arg($arg, $map, $index) { return implode('/', array_slice($map, $index)); } /** + * Loader function: check that a token is valid. + */ +function drupal_token_load($token) { + return drupal_valid_token($token) ? $token : FALSE; +} + +/** + * Menu to arg function: convert %token into a user specific token. + */ +function drupal_token_to_arg() { + return drupal_get_token(); +} + +/** * This function is similar to _menu_translate() but does link-specific * preparation such as always calling to_arg functions * diff --git modules/menu/menu.test modules/menu/menu.test index c9197b4..fa3b964 100644 --- modules/menu/menu.test +++ modules/menu/menu.test @@ -417,7 +417,7 @@ class MenuTestCase extends DrupalWebTestCase { */ private function getStandardMenuLink() { // Retrieve menu link id of the Log out menu link, which will always be on the front page. - $mlid = db_query("SELECT mlid FROM {menu_links} WHERE module = 'system' AND router_path = 'user/logout'")->fetchField(); + $mlid = db_query("SELECT mlid FROM {menu_links} WHERE module = 'system' AND router_path = 'user/logout/%'")->fetchField(); $this->assertTrue($mlid > 0, 'Standard menu link id was found'); // Load menu link. // Use api function so that link is translated for rendering. diff --git modules/simpletest/drupal_web_test_case.php modules/simpletest/drupal_web_test_case.php index 3665426..bb104ca 100644 --- modules/simpletest/drupal_web_test_case.php +++ modules/simpletest/drupal_web_test_case.php @@ -592,6 +592,16 @@ class DrupalWebTestCase extends DrupalTestCase { protected $httpauth_credentials = NULL; /** + * The current session name, if available. + */ + protected $session_name = NULL; + + /** + * The current session ID, if available. + */ + protected $session_id = NULL; + + /** * Constructor for DrupalWebTestCase. */ function __construct($test_id = NULL) { @@ -913,12 +923,20 @@ class DrupalWebTestCase extends DrupalTestCase { } } + /** + * Generate a token for the currently logged in user. + */ + protected function drupalGetToken($value = '') { + $private_key = drupal_get_private_key(); + return md5($this->session_id . $value . $private_key); + } + /* * Logs a user out of the internal browser, then check the login page to confirm logout. */ protected function drupalLogout() { // Make a request to the logout page. - $this->drupalGet('user/logout'); + $this->drupalGet('user/logout/' . $this->drupalGetToken()); // Load the user page, the idea being if you were properly logged out you should be seeing a login screen. $this->drupalGet('user'); @@ -1169,6 +1187,7 @@ class DrupalWebTestCase extends DrupalTestCase { */ protected function curlHeaderCallback($curlHandler, $header) { $this->headers[] = $header; + // Errors are being sent via X-Drupal-Assertion-* headers, // generated by _drupal_log_error() in the exact form required // by DrupalWebTestCase::error(). @@ -1176,6 +1195,19 @@ class DrupalWebTestCase extends DrupalTestCase { // Call DrupalWebTestCase::error() with the parameters from the header. call_user_func_array(array(&$this, 'error'), unserialize(urldecode($matches[1]))); } + + // Save the session cookie, if set. + if (preg_match('/^Set-Cookie: (SESS[a-z0-9]+)=([a-z90-9]+)/', $header, $matches)) { + if ($matches[2] != 'deleted') { + $this->session_name = $matches[1]; + $this->session_id = $matches[2]; + } + else { + $this->session_name = NULL; + $this->session_id = NULL; + } + } + // This is required by cURL. return strlen($header); } diff --git modules/system/system.install modules/system/system.install index f0505aa..61fa728 100644 --- modules/system/system.install +++ modules/system/system.install @@ -3251,8 +3251,8 @@ function system_update_7014() { */ function system_update_7015() { $ret = array(); - $ret[] = update_sql("UPDATE {menu_links} SET link_path = 'user/logout' WHERE link_path = 'logout'"); - $ret[] = update_sql("UPDATE {menu_links} SET router_path = 'user/logout' WHERE router_path = 'logout'"); + $ret[] = update_sql("UPDATE {menu_links} SET link_path = 'user/logout/%' WHERE link_path = 'logout'"); + $ret[] = update_sql("UPDATE {menu_links} SET router_path = 'user/logout/%' WHERE router_path = 'logout'"); return $ret; } diff --git modules/user/user.module modules/user/user.module index ba3b19f..fda73c3 100644 --- modules/user/user.module +++ modules/user/user.module @@ -1321,7 +1321,7 @@ function user_menu() { 'type' => MENU_CALLBACK, ); - $items['user/logout'] = array( + $items['user/logout/%drupal_token'] = array( 'title' => 'Log out', 'access callback' => 'user_is_logged_in', 'page callback' => 'user_logout', diff --git modules/user/user.test modules/user/user.test index 30ecf09..fc2441c 100644 --- modules/user/user.test +++ modules/user/user.test @@ -717,6 +717,39 @@ class UserPictureTestCase extends DrupalWebTestCase { } } +/** + * Test basic user login and logout. Verify that the logout link is protected. + */ +class UserLogoutTestCase extends DrupalWebTestCase { + public static function getInfo() { + return array( + 'name' => t('User login / Logout'), + 'description' => t('Test user login and logout.'), + 'group' => t('User') + ); + } + + function setUp() { + parent::setUp(); + + $this->user = $this->drupalCreateUser(); + } + + /** + * Change user permissions and check user_access(). + */ + function testUserLoginLogout() { + // No assertion needed here: assertions are already verified in drupalLogin(). + $this->drupalLogin($this->user); + + $this->drupalGet('user/logout'); + $this->assertText(t('Page not found'), t('Visiting user/logout results in 404 page.')); + + // No assertion needed here: assertions are already verified in drupalLogin(). + $this->drupalLogout(); + } + +} class UserPermissionsTestCase extends DrupalWebTestCase { protected $admin_user;