Rethink cas_goto session variable and cas_save_page()

I'd like to hammer this out as well. If we can establish the desired behavior, I can implement this using some test driven development (via #1011848: Add Testing Routines).

Here's what I think we'd need to test for:

• Logging in via cas:
  1. Clicking on a link to 'cas' should return you to the page you were previously on.
  2. Visiting 'cas?destination=foo' should return you to 'foo'
• Logging in via user or user/login:
  1. Should bring you to your user page, unless a destination parameter was passed.
• Logging in via the user login block:
  1. Should return you to the page you were visiting on that block.

In general, I guess the code flow should be this:

1. Use a routine like user_login_destination() to find the desired login destination.
2. Save the login destination to a variable like cas_goto.
3. After authentication, redirect to that location.

This is pretty similar to what already exists...

What sort of change did you have in mind?

Currently in cas_save_page() we never overwrite $_SESSION['cas_goto']. This means that if we are on page A, click on a link to authenticate, change our minds and go back, navigate to page B, click on a link to authenticate, and log in, we'll be redirected back to A.

A more robust check might be to set $_SESSION['cas_goto'] only if $_GET['ticket'] is not present.

Yes, this all sounds good to me.

As noted in #1011848-5: Add Testing Routines, several login redirection tests are failing.

Most of these failures relate to the handling of '' vs variable_get('site_frontpage', 'node') as the default front page. CAS currently uses the latter option which redirects to 'node', but I think for the sake of pretty URLs it'd be better to redirect to '' which would truly give you the front page. Both have the same page content.

Another failures relate to logging in using the user login block which redirects you to the front page instead of keeping you at the page you started from.

Here's the 7.x-1.x code for cas_save_page():

/**
 * Saves the page that the user was on when login was requested or required
 */
function cas_save_page() {
  if (!empty($_SESSION['cas_goto'])) {
    if (arg(0) == 'cas') {
      //we're were destined to the CAS login page, so set
      //the page to the referrer.
      $_SESSION['cas_goto'] = $_SERVER['HTTP_REFERER'];
    }
    else {
      //We're destined for another page, so save the group.
      $_SESSION['cas_goto'] = $_GET['q'];
    }
  }
}

If I'm reading this right, 'cas_goto' will never be saved as it is by default empty so the first if statement will never evaluate to TRUE....

Feeling a little confident with the CAS test routines, I have attached a patch which rips out the cas_goto code completely.

The login mechanism works like this:
- A user visits a CAS protected page or clicks on the 'CAS login' link.
- We look to see if $_REQUEST['destination'] is already set. If so, we keep it. If not, we generate the destination by looking for the HTTP Referrer (if provided) or by defaulting on the site frontpage.
- We set the phpCAS service URL (that is, the URL the CAS server returns to after login, to http://example.com/cas?destination=....
- We use drupal_goto to redirect to the destination provided in the URL.

Weaknesses:
- drupal_goto in D6 (but not D7, I think) will not allow redirecting to external URLs using the destination parameter. So in cas_login_destination(), we have to convert the HTTP Referrer URI to its Drupal path. This is non-trivial, and the attached code, likely does not account for a few edge cases (like SSL sites, for example).

Triggering the test bot.

And it failed.

I've rethought out the login destination logic, and present the following patch for 7.x-1.x

(1): We use hook_menu_link_alter() and hook_translated_menu_link_alter() to set the destination query string on all CAS login/logout links. This means that the links turn into things like "/cas?destination=node/5"
(2): The cas_phpcas_init() routine sets the "service URL" -- that is, the URL to return to after the user logs in with CAS to 'http://.../cas?destination=node/5'. After logging in, we use drupal_goto() to automatically redirect to the specified destination.
(3): This allows us to completely remove all $SESSION['cas_goto'] logic and the entire cas_save_page() function.

This is reasonably well tested for basic configurations, but I haven't tested the interaction with CAS gateway ('check to see if the user is already logged in') or with cas_pages ('Require CAS login for specific pages').

Here's the 6.x-3.x port of the patch in #10.

One issue is that the necessary changes to the menu items ("CAS Login" and "CAS Logout") won't be picked up unless the menu item is deliberately resaved. (i.e., resetting the cache is insufficient).

That is, for testing you will need to visit Admin > Site Building > Menus > ... > CAS Login (edit) and choose "Save." Ditto for "CAS Logout."

Eventually we'll need to provide a cas_update_6304() that does:

foreach (array('cas', 'caslogout') as $link_path) {
  $mlid = db_result(db_query("SELECT mlid FROM {menu_links} WHERE link_path = '%s' AND module = '%s'", $link_path, 'system'));
  $link = menu_link_load($mlid);
  menu_link_save($link);
}

Added the cas_update_6305() function in 7.x-1.x. Working on writing tests for CAS gateway and cas_pages before I commit this.

Rerolled the patch in #13 to account for additional tests (single sign out, and gateway).

I've done some pretty thorough testing and have committed this to 6.x-3.x and 7.x-1.x. There might be a few remaining bugs, but overall I think it is pretty solid.