From e90cff6203e4c4fee59f2958263c911cc2d37d8b 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.admin.inc | 2 +- cas.install | 11 ++++++++ cas.module | 80 +++++++++++++++++++++++++++++++------------------------- cas.test | 34 ++---------------------- 4 files changed, 59 insertions(+), 68 deletions(-) diff --git a/cas.admin.inc b/cas.admin.inc index 59072e3..51df021 100644 --- a/cas.admin.inc +++ b/cas.admin.inc @@ -268,7 +268,7 @@ function cas_admin_settings() { '#default_value' => variable_get('cas_logout_destination', ''), '#size' => 30, '#maxlength' => 255, - '#description' => t("Drupal path or URL. Enter a destination if you want a user to be redirected to this page after logging out of CAS. An example path is blog for the blog page or <front> for the front page."), + '#description' => t("Drupal path or URL. Enter a destination if you want a user to be directed to this page after logging out of CAS, or leave blank to direct users back to the previous page. An example path is blog for the blog page or <front> for the front page."), ); diff --git a/cas.install b/cas.install index 0b22cf3..b1ac6c0 100644 --- a/cas.install +++ b/cas.install @@ -310,3 +310,14 @@ function cas_update_6303() { return array(); } + +/** + * Add destination parameter to CAS Login / CAS Logout menu links. + */ +function cas_update_6304() { + foreach (array('cas', 'caslogout') as $link_path) { + $mlid = db_query("SELECT mlid FROM {menu_links} WHERE link_path = :link_path AND module = :module", array(':link_path' => $link_path, ':module' => 'system'))->fetchField(); + $link = menu_link_load($mlid); + menu_link_save($link); + } +} diff --git a/cas.module b/cas.module index 298ed92..6876f56 100644 --- a/cas.module +++ b/cas.module @@ -51,7 +51,6 @@ function cas_login_check() { // Start a drupal session drupal_session_start(); - cas_save_page(); _cas_single_sign_out_save_ticket(); // We use this later for CAS 3 logoutRequests // Initialize phpCAS. @@ -255,6 +254,9 @@ function cas_phpcas_init() { break; } + ; + phpCAS::setFixedServiceURL(url(isset($_GET['q']) ? $_GET['q'] : '', 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'); @@ -348,6 +350,45 @@ function cas_menu() { } /** + * 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) { + 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) { + if ($item['href'] == 'cas') { + $item['localized_options']['query'] = drupal_get_destination(); + } + elseif ($item['href'] == 'caslogout' && !variable_get('cas_logout_destination', '')) { + $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(); + if ($destination['destination'] == 'cas') { + $destination['destination'] = ''; + } + return $destination; +} + +/** * Implements hook_user_operations(). */ function cas_user_operations($form = array(), $form_state = array()) { @@ -547,27 +588,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 (arg(0) != '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 @@ -580,22 +600,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_destination', '')) { $destination = variable_get('cas_first_login_destination', ''); + unset($_GET['destination']); } - elseif (!empty($_REQUEST['destination'])) { - $destination = $_REQUEST['destination']; - } - elseif (!empty($_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($_GET['destination']); + // Respect the query string, if transmitted. drupal_goto($destination); } diff --git a/cas.test b/cas.test index 4aae4a7..929e102 100644 --- a/cas.test +++ b/cas.test @@ -132,17 +132,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 { @@ -451,7 +440,6 @@ class CasLoginRedirectionTestCase extends CasTestHelper { $node1 = $this->drupalCreateNode(); $node2 = $this->drupalCreateNode(); $node3 = $this->drupalCreateNode(); - $node4 = $this->drupalCreateNode(); // Create a CAS user. $account = $this->casCreateUser(); @@ -490,25 +478,9 @@ class CasLoginRedirectionTestCase extends CasTestHelper { $this->assertLoggedInWithUrl($account, $destination); $this->drupalLogout(); - // Click on a 'cas' link on node 4. - $destination = "node/$node4->nid"; - $this->url = url($destination, array('absolute' => TRUE)); - $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