From 142ece29d0ef28fccc8bcd37c7aae2f9f52acb36 Mon Sep 17 00:00:00 2001 From: Bradley M. Froehle Date: Tue, 5 Apr 2011 09:52:21 -0700 Subject: [PATCH] Issue #1051694 - Remove cas_goto session variable in favor of destination query strings. The $_SESSION['cas_goto'] variable proved quite fragile. Instead, we implement destination query strings, much like the default Drupal user module. --- cas.module | 79 ++++++++++++++++++++++++++++++----------------------------- cas.test | 40 ++++------------------------- 2 files changed, 46 insertions(+), 73 deletions(-) diff --git a/cas.module b/cas.module index 508cb38..11a7c51 100644 --- a/cas.module +++ b/cas.module @@ -56,7 +56,6 @@ function cas_login_check() { // a valid session to complete the authentication process. drupal_session_start(); } - cas_save_page(); _cas_single_sign_out_save_ticket(); // We use this later for CAS 3 logoutRequests // Initialize phpCAS. @@ -265,6 +264,8 @@ function cas_phpcas_init($force = FALSE) { break; } + phpCAS::setFixedServiceURL(url('cas', array('query' => cas_login_destination(), 'absolute' => TRUE))); + // Allow other modules to call phpCAS routines. We do not call // drupal_alter() since there are no parameters to pass. module_invoke_all('cas_phpcas_alter'); @@ -353,7 +354,40 @@ function cas_menu() { } /** - * Implementation of hook_user_operations(). + * Implements hook_menu_link_alter(). + * + * Flag this link as needing alter at display time. + * @see cas_translated_menu_link_alter(). + **/ +function cas_menu_link_alter(&$item, $menu) { + if ($item['link_path'] == 'cas' || $item['link_path'] == 'caslogout') { + $item['options']['alter'] = TRUE; + } +} + +/** + * Implements hook_translated_menu_item_alter(). + * + * Append dynamic query 'destination' to several menu items. + **/ +function cas_translated_menu_link_alter(&$item, $map) { + if ($item['href'] == 'cas' || $item['href'] == 'caslogout') { + $item['localized_options']['query'] = drupal_get_destination(); + } +} + +/** + * Helper function to rewrite the destination to avoid redirecting to login page after login. + * + * Instead of the login page, we redirect to the front page. + */ +function cas_login_destination() { + $destination = user_login_destination(); + return $destination == 'destination=cas' ? 'destination=' : $destination; +} + +/** + * Implements hook_user_operations(). */ function cas_user_operations($form = array(), $form_state = array()) { $operations['cas_create'] = array( @@ -499,27 +533,6 @@ function cas_user_load_by_name($cas_name, $alter = FALSE) { } /** - * Saves the page that the user was on when login was requested or required - */ -function cas_save_page() { - if (!isset($_SESSION['cas_goto'])) { - if (arg(0) == 'cas' && isset($_SERVER['HTTP_REFERER'])) { - //we're were destined to the CAS login page, so set - //the page to the referrer. - $_SESSION['cas_goto'] = $_SERVER['HTTP_REFERER']; - } - elseif (isset($_GET['q']) && $_GET['q'] != 'cas') { - //We're destined for another page, so save the group. - $_SESSION['cas_goto'] = $_GET['q']; - } - else { - // No idea where we are coming from or going to, so pick the front page. - $_SESSION['cas_goto'] = ''; - } - } -} - -/** * Redirects to appropriate page based on user settings. * * @param $cas_first_login @@ -532,22 +545,10 @@ function cas_login_page($cas_first_login = FALSE) { // If it is the user's first CAS login and initial login redirection is enabled, go to the set page if ($cas_first_login && variable_get('cas_first_login', FALSE)) { $destination = variable_get('cas_first_login_destination', ''); + unset($_REQUEST['destination']); } - elseif (isset($_REQUEST['destination'])) { - $destination = $_REQUEST['destination']; - } - elseif (isset($_SESSION['cas_goto'])) { - $destination = $_SESSION['cas_goto']; - } - unset($_SESSION['cas_goto']); - // Change the user/login and user/register urls to the users current page. - if ($destination == 'user/login' || $destination == 'user/register') { - $destination = 'user/' . $user->uid; - } - - // Force redirection. - unset($_REQUEST['destination']); + // Respect the query string, if transmitted. drupal_goto($destination); } @@ -564,7 +565,7 @@ function cas_logout() { $query = NULL; // Add destination override so that a destination can be specified on the logout link - // e.g. caslogout?desination=http://foo.bar.com/foobar. + // e.g. caslogout?desination=node/4. $destination = drupal_get_destination(); $destination = preg_replace('/^destination=/', '', $destination); $destination = urldecode($destination); @@ -576,7 +577,7 @@ function cas_logout() { // If there was no override and admin has set and enabled a logout // destination, look for it - if (empty($destination) && variable_get('cas_logout_redirect', 0)) { + if (variable_get('cas_logout_redirect', FALSE)) { $destination = variable_get('cas_logout_destination', ''); } diff --git a/cas.test b/cas.test index ebaa989..406df3c 100644 --- a/cas.test +++ b/cas.test @@ -140,17 +140,6 @@ class CasTestHelper extends DrupalWebTestCase { $this->loggedInUser = cas_user_load_by_name($cas_name, TRUE); } } - - /** - * Retrieves a Drupal path or an absolute path, as if a link was clicked. - * - * Sets the HTTP Referer header for the request to the previous URL. - */ - function drupalGetWithHttpReferrer($path, array $options = array(), array $headers = array()) { - $this->additionalCurlOptions[CURLOPT_REFERER] = $this->url; - $this->drupalGet($path, $options, $headers); - unset($this->additionalCurlOptions[CURLOPT_REFERER]); - } } class CasUserAdminTestCase extends CasTestHelper { @@ -406,10 +395,10 @@ class CasLogoutRedirectionTestCase extends CasTestHelper { // Verify 'cas_logout_destination' can be overwritten by passing the // destination query string. - variable_set('cas_logout_destination', 'http://example.com/'); - $destination = 'node/1'; + $destination = 'node/2'; + variable_set('cas_logout_destination', $destination); $this->casLogin($account); - $this->drupalGet('caslogout', array('query' => array('destination' => $destination))); + $this->drupalGet('caslogout', array('query' => array('destination' => 'http://example.com/'))); $this->assertText(t('Logged out. Continue to @url.', array('@url' => url($destination, array('absolute' => TRUE))))); $this->assertLoggedOut(); } @@ -446,7 +435,6 @@ class CasLoginRedirectionTestCase extends CasTestHelper { $node1 = $this->drupalCreateNode(); $node2 = $this->drupalCreateNode(); $node3 = $this->drupalCreateNode(); - $node4 = $this->drupalCreateNode(); // Create a CAS user. $account = $this->casCreateUser(); @@ -485,25 +473,9 @@ class CasLoginRedirectionTestCase extends CasTestHelper { $this->assertLoggedInWithUrl($account, $destination); $this->drupalLogout(); - // Click on a 'cas' link on node 4. - $destination = "node/$node4->nid"; - $this->drupalGet($destination); - $this->drupalGetWithHttpReferrer('cas'); - $this->assertLoggedInWithUrl($account, $destination); - $this->drupalLogout(); - - // Clicking on a 'cas' link from an external host does not redirect back - // to that external host. - // $destination = ''; - // $this->url = 'http://example.com/node/3'; - // $this->drupalGetWithHttpReferrer('cas'); - // $this->assertLoggedInWithUrl($account, $destination); - // $this->drupalLogout(); - - // The destination parameter overrides the HTTP Referer header. - $destination = "node/$node2->nid"; - $this->url = url("node/$node3->nid", array('absolute' => TRUE)); - $this->drupalGetWithHttpReferrer('cas', array('query' => array('destination' => $destination))); + // External destinations are not allowed. + $destination = ''; + $this->drupalGet('cas', array('query' => array('destination' => 'http://example.com/node/3'))); $this->assertLoggedInWithUrl($account, $destination); $this->drupalLogout(); } -- 1.7.4.1