From a751db5eba09d0158f26014e0425005a17cc10b7 Mon Sep 17 00:00:00 2001 From: Bradley M. Froehle Date: Thu, 7 Apr 2011 10:25:02 -0700 Subject: [PATCH] Issue #1082602: Remove unnecessary admin page clutter. --- cas.admin.inc | 20 +++----------------- cas.install | 37 +++++++++++++++++++++++++++++++++++-- cas.module | 21 ++++++++------------- cas.test | 28 +++++++++++++++++----------- 4 files changed, 63 insertions(+), 43 deletions(-) diff --git a/cas.admin.inc b/cas.admin.inc index 7de6849..59072e3 100644 --- a/cas.admin.inc +++ b/cas.admin.inc @@ -244,20 +244,13 @@ function cas_admin_settings() { // Settings for redirection upon first login - $form['pages']['cas_first_login'] = array( - '#type' => 'checkbox', - '#title' => t('Force redirection on initial login'), - '#default_value' => variable_get('cas_first_login', 0), - '#description' => t("Activate this option if you want a user to be directed to the following page after their first CAS login."), - ); - $form['pages']['cas_first_login_destination'] = array( '#type' => 'textfield', - '#title' => t('Initial login landing page'), + '#title' => t('Initial login destination'), '#default_value' => variable_get('cas_first_login_destination', ''), '#size' => 30, '#maxlength' => 255, - '#description' => t("Drupal path or URL. An example path is 'blog' for the blog page. '<front>' is the front page. An example URL is 'http://www.example.com'."), + '#description' => t("Drupal path or URL. Enter a destination if you want the user to be redirected to this page on their first CAS login. An example path is blog for the blog page, <front> for the front page, or user for the user's page."), ); // Setting for message displayed to user upon successfull login @@ -269,20 +262,13 @@ function cas_admin_settings() { ); // Setting for page to return to after a CAS logout - $form['pages']['cas_logout_redirect'] = array( - '#type' => 'checkbox', - '#title' => t('Redirect user on logout'), - '#default_value' => variable_get('cas_logout_redirect', 0), - '#description' => t("Activate this option if you want a user to be directed to the following page after logging out of CAS. The logout destination must be specified below."), - ); - $form['pages']['cas_logout_destination'] = array( '#type' => 'textfield', '#title' => t('Logout destination'), '#default_value' => variable_get('cas_logout_destination', ''), '#size' => 30, '#maxlength' => 255, - '#description' => t("URL. An example URL is 'http://www.example.com'. You can also use '<front>' for the front page."), + '#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."), ); diff --git a/cas.install b/cas.install index efa9d40..d3c46eb 100644 --- a/cas.install +++ b/cas.install @@ -85,7 +85,6 @@ function cas_uninstall() { variable_del('cas_debugfile'); variable_del('cas_domain'); variable_del('cas_exclude'); - variable_del('cas_first_login'); variable_del('cas_first_login_destination'); variable_del('cas_hide_email'); variable_del('cas_hide_password'); @@ -96,7 +95,6 @@ function cas_uninstall() { variable_del('cas_login_message'); variable_del('cas_login_redir_message'); variable_del('cas_logout_destination'); - variable_del('cas_logout_redirect'); variable_del('cas_pages'); variable_del('cas_pgtformat'); variable_del('cas_pgtpath'); @@ -110,8 +108,10 @@ function cas_uninstall() { variable_del('cas_version'); // And old (un-used) variables. + variable_del('cas_first_login'); variable_del('cas_hijack_user'); variable_del('cas_ldap_email_attribute'); + variable_del('cas_logout_redirect'); variable_del('cas_useldap'); variable_del('cas_useldap_groups'); variable_del('cas_verify'); @@ -277,3 +277,36 @@ function cas_update_6302() { variable_del('cas_hijack_user'); return $message; } + +/** + * Remove unnecessary CAS settings. + */ +function cas_update_6303() { + // We have removed the cas_first_login option, and instead verify that + // cas_first_login_destination is non-empty. To preserve functionality, + // we need to update the destination to '' if previously the option + // was selected but the destination was empty. + if (variable_get('cas_first_login', FALSE)) { + if (variable_get('cas_first_login_destination', '') == '') { + variable_set('cas_first_login_destination', ''); + } + } + else { + variable_set('cas_first_login_destination', ''); + } + variable_del('cas_first_login'); + + // Similarly for the cas_logout_redirect and cas_logout_destination + // variables. + if (variable_get('cas_logout_redirect', FALSE)) { + if (variable_get('cas_logout_destination', '') == '') { + variable_set('cas_logout_destination', ''); + } + } + else { + variable_set('cas_first_login_destination', ''); + } + variable_del('cas_logout_redirect'); + + return array(); +} diff --git a/cas.module b/cas.module index f368fc8..298ed92 100644 --- a/cas.module +++ b/cas.module @@ -578,7 +578,7 @@ function cas_login_page($cas_first_login = FALSE) { global $user; $destination = ''; // 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)) { + if ($cas_first_login && variable_get('cas_first_login_destination', '')) { $destination = variable_get('cas_first_login_destination', ''); } elseif (!empty($_REQUEST['destination'])) { @@ -611,19 +611,14 @@ function cas_logout() { $logout_url = phpCAS::getServerLogoutURL(); $options = array(); - // Add destination override so that a destination can be specified on the logout link - // e.g. caslogout?desination=http://foo.bar.com/foobar. - $destination = drupal_get_destination(); - $destination = $destination['destination']; - - // Do not accept 'caslogout' as a valid destination. - if ($destination == 'caslogout') { - $destination = ''; + if (isset($_GET['destination']) && !url_is_external($_GET['destination'])) { + // Add destination override so that a destination can be specified on the + // logout link, e.g., caslogout?desination=http://foo.bar.com/foobar. We do + // not allow absolute URLs to be passed via $_GET, as this can be an attack + // vector. + $destination = $_GET['destination']; } - - // 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)) { + else { $destination = variable_get('cas_logout_destination', ''); } diff --git a/cas.test b/cas.test index 7780c37..5150280 100644 --- a/cas.test +++ b/cas.test @@ -383,17 +383,24 @@ class CasLogoutRedirectionTestCase extends CasTestHelper { $this->assertText('Logged out. No redirection provided.'); $this->assertLoggedOut(); - // Check that caslogout?destination= redirects. - $destinations = array('http://example.com/', 'http://example.com/?query=yes#fragment'); - foreach ($destinations as $destination) { - $this->casLogin($account); - $this->drupalGet('caslogout', array('query' => array('destination' => $destination))); - $this->assertText(t('Logged out. Continue to @url.', array('@url' => url($destination, array('absolute' => TRUE))))); - $this->assertLoggedOut(); - } + // Verify the destination parameter may be passed on logout, i.e., + // caslogout?destination=node + $destinations = array('node', ); + $destination = 'node'; + $this->casLogin($account); + $this->drupalGet('caslogout', array('query' => array('destination' => $destination))); + $this->assertText(t('Logged out. Continue to @url.', array('@url' => url($destination, array('absolute' => TRUE))))); + $this->assertLoggedOut(); + + // Verify that remote destination parameters are not allowed. + $destination = 'http://example.com/?query=yes#fragment'; + $this->casLogin($account); + $this->drupalGet('caslogout', array('query' => array('destination' => $destination))); + $this->assertText(t('Logged out. No redirection provided.')); + $this->assertLoggedOut(); - // Verify 'cas_logout_destination' works for a variety of destinations. - variable_set('cas_logout_redirect', TRUE); + // Verify 'cas_logout_destination' works for a variety of destinations, + // including remote destinations. $destinations = array('', 'http://example.com/?query=yes#fragment', 'node/1'); foreach ($destinations as $destination) { variable_set('cas_logout_destination', $destination); @@ -519,7 +526,6 @@ class CasLoginRedirectionTestCase extends CasTestHelper { // Initial login with redirection goes to specified destination. $node = $this->drupalCreateNode(); - variable_set('cas_first_login', TRUE); variable_set('cas_first_login_destination', "node/$node->nid"); $cas_name = $this->randomName(); $account = $this->casLogin($cas_name); -- 1.7.4.1