Login during hook_init interferes with other modules (like Rules)

A few comments:

0) Rules integration is a fantastic idea, and I'd definitely like to include this.

1) Why doesn't the "User has logged in" rule work? I assume it attaches to hook_user_login which should get called via user_login_finalize.

2) I dislike the 'authentification_via_cas' name. It should be 'cas_login' or similar.

3) I'd rather see us instead provide a condition of the type "user is CAS authenticated" or similar (much like the "user has role" condition already in Rules).

izus: Great work so far.

The proper (American) english spelling of "authentificated" is actually "authenticated."

In terms of phrasing, I'd prefer the following changes:
- "User is authentificated via CAS" -> "User has logged in using CAS"
- "User is CAS authentificated" -> "User has CAS credentials" (??? I'm not sure here...)

Can you try again to explain to me how "User is authenticated via CAS" (i.e. at login) is any different than "User has logged in" + "User is CAS authentificated"? Naively I'd expect that the second approach would still work.

Lastly, we should be able to remove rules_condition_user_is_cas and instead just refer to (the poorly named) cas_is_external_user. That is, something like:

/**
 * Implements hook_rules_condition_info().
 */
function cas_rules_condition_info() {
  $items = array(
    'cas_is_external_user' => array(
      'label' => t('User is CAS authentificated'), // @todo fix this label.
      'parameter' => array(
        'account' => array('type' => 'user', 'label' => t('User')),
      ),
      'group' => t('User'),
    ),
  );
  return $items;
}

Hi,

Relatively to 1).

I am also trying to integrate the Rules module with CAS, but none of the rules' user related events seem to be triggered (login, insert, update, etc).

Oddly it works when I trigger the rules' events in my custom module (my_module.module). I simply do:

function my_module_user_login(&$edit, $account){
  if (module_exists('rules')) {
    rules_invoke_event('user_login', $account);
  }
}

I don't even need to create a custom event for the Rules module to trigger the event 'user_login'. I think it might be a problem with the weight defined for CAS or the Rules module, since the user_login hook isn't being called in rules/modules/events.inc:

/**
 * Implements hook_user_login().
 */
function rules_user_login(&$edit, $account) {
  rules_invoke_event('user_login', $account);
}

If we need to change the CAS weight, I'd suggest that we set it to a fixed constant. Otherwise it'll differ between installs depending on whether CAS was around at install time. Additionally we'll need to put the change in hook_install (I think...?) so that the weight is set for new installs as well as upgrades.

izus: I very much appreciate the detective work. Can you help me understand why it is that a regular user login does call rules_user_login, but CAS login does not?

It seems that changing the weight of the CAS module shouldn't change the behavior... In each case we call user_login_finalize which does the appropriate work to log in the user and fire the hook_user_login.

I suspect the true issue is that we are starting the CAS login process in the cas_init function.

Hi,

After a little research and more tests, it seems that the problem is in the rules' hook_init:

/**
 * Implements hook_init().
 */
function rules_init() {
  module_load_include('inc', 'rules', 'modules/events');
  rules_invoke_event('init');
}

As you can see on the code above, the hooks related to the user_* (login, update, etc.) are only declared once the rules_init is called and since the Rules module has a weight of 20, the hooks' functions defined in events.inc are included too late on run time.

To verify this, try defining the following hook in the rules.module file. You will see that the hook is correctly called when the user logs in from CAS.

/**
 * Implements hook_user_login().
 */
function rules_user_login(&$edit, $account) {
  rules_invoke_event('user_login', $account);
}

It seems to me that, from the rules module side, this could be fixed by including the events.inc file sooner than any hook, or defining all the hook_* functions on the rules.module file.

Otherwise, from the CAS project side, as @bfroehle said:

I suspect the true issue is that we are starting the CAS login process in the cas_init function.

This could be fixed by triggering the login in another hook level.

So I see two ways to deal with this:

1. Set the CAS weight a lot higher (so that we can be assured it is nearly the last module to load). This might have unintended side effects.
2. Refactor the CAS login mechanism to only do logins on the path cas. Other pages could conceivably do gateway checks or the like by redirecting to cas?destination=...&gateway=true. I tried to do such a refactoring in the past, but it was rather difficult to ensure all of the legacy login methods still worked since we didn't have automated tests at that point. There might be some other creative ways to handle the gateway issue ... for example we could check if the user is logged in, and if so, redirect to cas?destination=... to actually process the login.

Thoughts?

Regarding option 2, I would think we'd need to branch a new major revision to tackle such an effort as it is likely to be a rough road.

Any such activity would need to support full URL redirection with query parameters preserved on any page and still honor the require login for all pages/specfic pages options etc. (I'ts not just about the gateway option). Brad do you know if our test cases are robust enough for this? If not, we might want to start by making sure we have test cases for all the login options.

I'm also concerned about the side effects of option 1.... I see no easy roads forward on this one.

Re-titling. I'll see if I can find some time this weekend to mock up my suggestion in #2. I think this refactoring might also allow us to reduce some of the complexity in the login functions.

Regarding the tests, I think they cover most basic use cases. However there are some gaps in coverage, particularly things like single-sign out which is only tested for a few basic login methodologies (and is likely the easiest thing to accidentally break in such a refactoring).

I started mocking up the required changes in https://github.com/bfroehle/drupal-cas/compare/refactor.

All looks pretty reasonable at first glance. Looks like you're not yet to the point of refactoring init or boot hook to do login detection. If you let me know when you get that far I'll check out a copy and do some testing.

You can alter rules_init hook to be first by :

function cas_module_implements_alter(&$implementations, $hook) {
  if ($hook == 'init') {
    $group = $implementations['rules'];
    unset($implementations['rules']);
    $implementations = array('rules' => $group) + $implementations; 
  }
}

I'm not sure this is better to use default 'User has logged in' event because, with this event, we are not sure that all user_login hooks are processed so condition may return a bad result.
I've a case where i got datas within user_login hook in a module with a weight greater than rules, so, my condition based on this datas does not work because my hook has not been triggered yet.
With a new event called by CAS after user_login, i'm sure that my datas are retrieved.

In addition, change the module weight may cause malfunctions

So to summarize:

1. We always log in users in hook_init
2. The rules event that fires for user_login relies on an additional file that is loaded in rules' implementation of hook_init
3. Rules weight is higher than CAS weight, so the CAS hook_init is fired prior to Rules hook_init.

So any Rules that fire on the user_login event won't get run when a user is logged in CAS. There doesn't seem to be a clean way to handle this. One suggestion was routing all logins through the /cas page and perform the login in the /cas page callback itself, but that gets ugly when you have to redirect users back to other people.

Don't really have a suggestion yet, just wanted to summarize the issue. I also don't think this is a major issue, since it doesn't break functionality with the module itself, but with integration with Rules.

Thought about this a bit more. Wondering why we can't just perform the actually logging in via the /cas page callback and not in hook_init? We can leave the actually gateway detection in hook_init (actually should be hook_boot, but that's another issue), so we wouldn't need to redirect a user first to /cas?gateway=true as suggested. But we would always set the service URL to /cas and (and append a destination param if necessary to bring user back to previous page). CAS Server would then always redirect users back to that /cas path. If a login was successful, it would include the ST and we could handle that in the /cas callback and log the user in.

Another benefit of doing that is it keeps the service URL always rooted at /cas, whereas now it can be any URL on the site if the user is logging in via gateway. It just seems cleaner to keep the service URL /cas and focus as much biz logic in the page callback as possible and out of hook_init / hook_boot.

I think that's the solution here. I will talk with @yalet about this next week and see what he thinks as well.

I disagree with #20. The reason this is done in hook_init is so that we can force visitors into the login process for private sites (e.g. portals) where we want all users to be authenticated. I don't see how doing this will solve the rules problem, since if you leave the "check to see if a user is logged in" case, you still will not see this. Having login rules that only sometimes fire does not seem like a sustainable path forward. Hook_boot has always been avoided because most modules that need to fire on login events need higher bootstrap levels than hook_boot provides. The cas login process fires the user_login events. If we move to hook_boot, I'm pretty sure that we'll have a lot of issues filed relating to other modules breaking that are trying to do things on the user_login hook.

You can see my comment in #13 for my first response to the requirements.

The cas login process fires the user_login events.

Right, but I'm suggesting that we don't actually log the user in via hook_init. Instead, let the request pass through and actually perform the log in via the callback function for /cas. Then rules would have loaded its needed file to handle the user login event. I don't see how this would break anything, but I could be missing something.

Here's my first go at a patch to bring all the Drupal login logic into the /cas page callback instead of hook_init. This is mostly to prove that it can be done, and it could use some refinement, particularly in the different paths that can happen in /cas and how they should be handled. Particularly, I want to prevent phpCAS from uselessly performing a page refresh just to remove the ST parameter. We do a redirect anyway, so we can remove it ourselves to save a page refresh.

There's really two main changes:

1. Gateway calls now set the service URL rooted to /cas, and just retain the current page & params that were set.
2. The Drupal login logic is done in the the /cas page callback.

For example, here's how a gateway auth would flow:

1. Initial page load to /node/4?my_param=1
2. hook_init triggers gateway auth, redirect to CAS Server, using service URL "cas?destination=/node/4?my_param=1&auth_type=gateway"
3. CAS server logs the user in, and sends back to service URL with validation ticket "cas?destination=/node/4?my_param=1&auth_type=gateway&ticket=ST-123abc"
4. hook_init has phpCAS library validate the ticket, refresh page removing the ticket parameter
5. /cas page callback gets run and logs the Drupal user in and redirects to /node/4?my_param=1

So essentially, when forced auth or gateway auth kicks in, the service URL will always be rooted to /cas, so all the Drupal login logic can live there. I needed to pass the "auth_type" param because we only want to set some login errors in /cas if it was a forced auth, so we need to pass that state between the redirects from Drupal to CAS.

I would appreciate a review from someone to validate that I'm not crazy and that we should be performing log ins like this. It would surely solve the problem outlined in this issue.

What is the purpose of the drupal_goto('') calls sprinkled throughout the code? These are error conditions, but changing the behavior to redirect to the home page if that happens. If these errors occured in the gateway cases, we'd be changing the users URL. Is that appropriate in this case? What's the rationale?

I would like a chance to test this feature out a bit more extensively before committing this change, to make sure the redirect rules work in several environments.

Thanks for stepping up to the plate to take this challenging issue.

Dave

Thanks for the quick review.

The purpose of the drupal_goto() is that since the user is within the /cas page callback, we have to redirect them somewhere else no matter what. There's no content to display in /cas, just biz logic.

If they were on a previous page, the destination parameter would have been set and drupal_goto('') will send them there instead of the homepage. I don't completely like the way it looks, and will probably iterate on it, but I didn't want to produce a lot of changes for this patch. Wanted it to be clear that the biggest change was just moving all the code into the page callback.

Forgot to mention that I was working from the patch in #869926: Use phpCAS::setCacheTimesForAuthRecheck() instead of a custom cookie., which cleans up the service URLs simplifies a few things. It seems close to being committed.

I think tests should pass now that #869926: Use phpCAS::setCacheTimesForAuthRecheck() instead of a custom cookie. is committed.

23: login_via_callback-1420170-23.patch queued for re-testing.

It's complaining about not being able to apply the patch. I just applied it locally to 7.x-1.x and it works fine, and all tests pass. Maybe have to wait for the latest commit to hit the dev release or something?

Heh, wasn't on the correct branch.

23: login_via_callback-1420170-23.patch queued for re-testing.

Hate to keep re-trying this, but I can't explain why the tests are failing, unless I introduced some flickering tests. I've run the test suite a few times now and it always comes green.

23: login_via_callback-1420170-23.patch queued for re-testing.

Okay, pretty sure this is because the test bot runs the Drupal site in a subdirectory. I put my local site in a subdir and all sorts of weird things happen with this patch. Sorry for all the activity on this issue. I will address tomorrow.

Ok, simple fix. The problem is I was using url() to generate the "saved" return path for gateway calls, which puts the Drupal subdirectory in the path.

Assuming the user was on on a subdirectory drupal site on node/4, the old code did this:

$return_path = url(current_path(), array('query' => drupal_get_query_parameters()));
$return_path = substr($return_url, 1);
// return path becomes "drupalsubdir/node/4" because url() prepends the base path

$service_url = url('cas', array('query' => array('destination' => $return_path, 'cas_auth_type' => 'gateway'), 'absolute' => TRUE));

New code does this:

$return_path = current_path();
$params = drupal_get_query_parameters();
if (!empty($params)) {
    $return_path .= '?' . drupal_http_build_query($params);
}
// return path becomes "node/4"

$service_url = url('cas', array('query' => array('destination' => $return_path, 'cas_auth_type' => 'gateway'), 'absolute' => TRUE));

@metzlerd think you'll have time to test this?

I will try and get to this tommorrow.

Dave

Did some pretty extensive testing... going through some of my favorite bugs from the past ;). It's very close. I was however able to create a redirect loop if I uncheck the "Automatically create Drupal accounts" setting and visit the site as a cas logged in user with the "check for logins" option enabled, or when you require cas authentication to get to the site.

To be clear the redirect loop only happens when you successfully authenticate via cas but not drupal account exists and we aren't going to create one.

Hmm yes there's some issues with the patch now that I look at it more. Thanks for helping test.

Since I'm using /cas as the service URL now, things get a bit tricky since /cas is also used as the forced auth URL. So I have to detect if the page request to /cas is truly meant to force auth a user or instead just a return from the CAS server.

This leads me to think that there should really be another menu route introduced, /cascheck (or similar), that is exclusively used as the service URL. Then /cas would just be used to initiate a forced auth (that's the purpose of that menu route anyway).

While I don't want to introduce a new menu route, I think that will truly simplify things even further and help us out here. Thoughts on that?

I think you might be on the right track here. If it cleans up the code, I don't think advertising another menu route should be avoided. I might recommend making the new url be caslogin or something that might make sense to cas server administrators that are creating the white lists, but thats just a matter of style preference. I'd happily leave that to the folks writing the code.

With one exception, I have this working well locally with a new menu callback that acts as the service callback.

The exception is that now that gateway requests are sent back to /cas_service_callback instead of the original page directly, an additional redirect is introduced to bring them back to their original page, which breaks the "always check" functionality introduced in #869926: Use phpCAS::setCacheTimesForAuthRecheck() instead of a custom cookie..

With a website set to check with CAS server on every page request, and the user is not logged into CAS:
1. User visits /node/4
2. Redirected to CAS server with service URL set to /cas_service_callback?destination=node/4&cas_auth_method=gateway
3. CAS server authenticates user and sends back to /cas_service_callback?destination=node/4&cas_auth_method=gateway&ticket=ST-123abc
4. phpCAS validates authenticity of the token, removes it from URL and reloads page to /cas_service_callback?destination=node/4&cas_auth_method=gateway
5. User gets logged in locally when page callback for /cas_service_callback is run, and they are finally redirected back to /node/4
6. Repeat step #1, redirect loop

Previously that last redirect wouldn't be necessary since all the login logic happened in hook_init, and the service URL was actually the page they were already on to begin with instead of the dedicated /cas_service_callback page.

Have to think some on how to address that problem... but in the meantime, encountering that issue makes me wonder what the real use case of "always check" really is anyway. Why would you ever want that extra overhead of redirecting the user away from the page and back again for EVERY page just to CHECK if they are logged in (not even force it). What are the chances the user would somehow become logged into CAS server during their browsing session? I would love to strip out that functionality, which somehow made it into #869926: Use phpCAS::setCacheTimesForAuthRecheck() instead of a custom cookie..

For what it's worth, the approach I've started on this issue of using a dedicated page callback to log a user in was also mentioned in that issue above (but it was suggested to use /cas to do all the logging in, which would cause the same issue I've described above). I think this approach is the right one, but it's not compatible with "always check" as it works now.

In places where you have a lot of cas authenticated sites, this is actually a fairly common occurrance. If you look at the issue you referenced, (somewhere around #6), you'll see some of this discussion. This is a complicated problem that we've tried many solutions to. One that I could think of would be to add url parameter to the final redirect that would make sure that cas didn't initiate the redirect. I don't have acces to the settings page right now where I'm writing this, but if we give the option to re-check authentication with a reasonably short frequency then it might still be possible to remove this settings. That being said, if the frequency were really short (< a minute) I could perhpas see this happening occasionally by accident on slow networks anyway, so maybe adding in the URL perameter to skip the recheck might still be a good idea?

I think the first time it was introduced was around #44 in that issue. I still don't understand why anyone would want that behavior. What is the situation where a user is browsing a site and then somehow is magically logged into CAS? I guess if they were also browsing another site that forced them to log into CAS, so now they are logged in, and the Drupal site would then auto log them in? Seems iffy, but I'll trust that you indicate it's a useful feature.

I'd sooner add a session variable or cookie before adding a URL parameter, since I think the URL parameter would remain on every single page request since it has to be there to tell the module not to try gateway auth again. Although I think that also could lead to some issues if the user has two windows (same session) open at once and are browsing both at the same time. The setting/unsetting of that session var could get mangled between the two browser windows. Perhaps not totally likely since the redirects happen so quickly...

We use the always-on feature here, and I'll explain the use case.

We have a bunch of Drupal sites, some of which have both authenticated and anonymous content, and some of which are authenticated-only. One of the authenticated-only sites is our portal. The portal links to many of these services, and single sign-on was a big part of the portal initiative. The decision was made that the behavior of the links from the portal needed to be consistent with respect to SSO, so that you were always logged in to an application if you got there via the portal. Thus, those Drupal sites that have anonymous views needed to check on every page load to see if you had picked up an SSO session somewhere along the line.

I haven't gotten a chance to delve too deeply into the technical aspects of this thread recently, but the always-on gateway shouldn't be sacrificed as a result.

Tim - so I understand:

1. User is browsing a "Site A" that allows anonymous traffic, and they are NOT logged into CAS or the site
2. User visits the portal, which requires the user log into CAS and authenticate.
3. From the portal, there are links to a bunch of sites, one of which is "Site A". User goes to "Site A" from the portal.
4. To maintain consistency with what the user would expect after logging into the portal, "Site A" needs to check if the user is now logged into CAS

If so, then that makes sense and seems like a valid use case. I just couldn't come up with one in my head :)

Yeah, that's exactly the use case. I imagine most use cases for "always-on" would be something similar to that use case.

Implemented using a session variable that will prevent the following page request after the service callback from performing another redirect.

So, to recap, this patch contains two significant changes:

#1 - Moves all the login logic to a page callback function instead of within hook_init. This because logging a user in via hook_init interferes with other modules that may act on a user being logged in (and their code is not yet completely loaded)

#2 - Introduces a new menu callback "cas_login_service" that is used exclusively as the service callback for all CAS requests. Forced auth and gateway auth now will always return to /cas_login_service which will either log the user in on success, or redirect the user back to where they were.

Missed a variable definition

I just attempted to apply this patch but it says it failed.

I am assuming its a lot of work has been done to the dev version that things may have changed.

@bkosborne could you so kindly re-roll ?

@metzlerd when it gets re-rolled do you think you could commit ? Really need this feature for rules.

Re-rolled.

I'd like to get a review on this, and then can commit it. I can upgrade my sites to using it for testing.

Patch applied cleanly and was able to use the "User Logged In" event to redirect a user to their profile edit page.

THANKS !!!! @bkosborne !!! We need a buy a beer feature in Drupal :)

This patch breaks the 'Initialize CAS as a proxy' feature. The proxy callback URL becomes 'cas_login_service', which then fails. I haven't completed a full debug of what's going on, but it is definitely broken.

Here's what my web server's access log looks like when attempting a proxy initialization (with the patch):

10.101.0.29 - - [15/Jul/2014:10:32:19 -0400] "GET /cas HTTP/1.0" 302 285 "-" "Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:29.0) Gecko/20100101 Firefox/29.0"
139.147.167.100 - - [15/Jul/2014:10:32:19 -0400] "GET /cas_login_service HTTP/1.0" 302 486 "-" "Java/1.7.0"
139.147.167.100 - - [15/Jul/2014:10:32:19 -0400] "GET /cas_login_service HTTP/1.0" 302 486 "-" "Java/1.7.0"
139.147.167.100 - - [15/Jul/2014:10:32:19 -0400] "GET /cas_login_service HTTP/1.0" 302 486 "-" "Java/1.7.0"
139.147.167.100 - - [15/Jul/2014:10:32:20 -0400] "GET /cas_login_service HTTP/1.0" 302 486 "-" "Java/1.7.0"
139.147.167.100 - - [15/Jul/2014:10:32:20 -0400] "GET /cas_login_service HTTP/1.0" 302 486 "-" "Java/1.7.0"
139.147.167.100 - - [15/Jul/2014:10:32:20 -0400] "GET /cas_login_service HTTP/1.0" 302 486 "-" "Java/1.7.0"
139.147.167.100 - - [15/Jul/2014:10:32:20 -0400] "GET /cas_login_service HTTP/1.0" 302 486 "-" "Java/1.7.0"
139.147.167.100 - - [15/Jul/2014:10:32:20 -0400] "GET /cas_login_service HTTP/1.0" 302 486 "-" "Java/1.7.0"
139.147.167.100 - - [15/Jul/2014:10:32:20 -0400] "GET /cas_login_service HTTP/1.0" 302 486 "-" "Java/1.7.0"
139.147.167.100 - - [15/Jul/2014:10:32:21 -0400] "GET /cas_login_service HTTP/1.0" 302 486 "-" "Java/1.7.0"
10.101.0.29 - - [15/Jul/2014:10:32:19 -0400] "GET /cas_login_service?cas_auth_type=forced&ticket=ST-581-ooodroCYpFFaESKbaMsx-cas0.dev.lafayette.edu HTTP/1.0" 500 4364 "-" "Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:29.0) Gecko/20100101 Firefox/29.0"

Here is the same thing without the patch:

[Client IP] - - [15/Jul/2014:10:42:37 -0400] "GET /cas HTTP/1.0" 302 264 "-" "Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:29.0) Gecko/20100101 Firefox/29.0"
[CAS Server IP] - - [15/Jul/2014:10:42:44 -0400] "GET /cas HTTP/1.0" 302 459 "-" "Java/1.7.0"
[CAS Server IP] - - [15/Jul/2014:10:42:44 -0400] "GET /cas?pgtIou=PGTIOU-[REDACTED]&pgtId=TGT-[REDACTED] HTTP/1.0" 200 404 "-" "Java/1.7.0"
[Client IP] - - [15/Jul/2014:10:42:44 -0400] "GET /cas?ticket=ST-584-AoUcraBStRCgkzmrtWJ4-cas0.dev.lafayette.edu HTTP/1.0" 302 26 "https://cas.dev.lafayette.edu/cas/login?service=https%3A%2F%2Fcalendar.dev.lafayette.edu%2Fcas" "Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:29.0) Gecko/20100101 Firefox/29.0"
[Client IP] - - [15/Jul/2014:10:42:44 -0400] "GET /cas HTTP/1.0" 302 20 "https://cas.dev.lafayette.edu/cas/login?service=https%3A%2F%2Fcalendar.dev.lafayette.edu%2Fcas" "Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:29.0) Gecko/20100101 Firefox/29.0"
[Client IP] - - [15/Jul/2014:10:42:45 -0400] "GET / HTTP/1.0" 200 9608 "https://cas.dev.lafayette.edu/cas/login?service=https%3A%2F%2Fcalendar.dev.lafayette.edu%2Fcas" "Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:29.0) Gecko/20100101 Firefox/29.0"

In the current scenario (the second set of logs), the CAS server is able to correctly validate the proxy callback URL (in this case, /cas) and then callback with the pgtIou and pgtId parameters to initialize the proxy. With the patch (the first set of logs), it gets stuck in a redirect loop attempting to validate the proxy callback URL. Eventually the CAS server will give up, which causes a nasty 500 error (and no authentication).

I believe the relevant part of the code is here:

function cas_service_callback() {
  global $user;

  $params = drupal_get_query_parameters();
  $auth_type = $params['cas_auth_type'];
  unset($_GET['cas_auth_type']);

  // If the user was not authenticated with CAS, we can't log them in. Just
  // redirect back to where they were or the homepage. In doing so, we also
  // need to set a session var indicating that the next page request should
  // NOT authenticate with CAS again. This is to prevent an redirect loop
  // the "always check" gateway method is used. If we don't do this, then the
  // next page request would just again check with CAS server.
  if (!phpCAS::isSessionAuthenticated()) {
    $_SESSION['cas_prevent_check'] = true;
    drupal_goto('');
  }

The CAS Server "isn't affected" by this session-level safeguarding against this redirect.

For those who are super-observant, you'll notice that even in the 'good' current case, the CAS server proxy validation step is returning a 302... redirecting to the CAS server! So we're not doing a good job implementing this proxy feature to begin with, because we're shooting the CAS server back to itself when its attempting to validate the client server's certificate. I think this problem would be solved by explicitly setting a pgtCallbackUrl which would be used for only proxy validation purposes. This should solve the problem with this patch and the underlying wonkiness of the current behavior.

@yalet,

Could you roll a patch that merges your code and @bkosborne 's code ?

To be clear, I don't have a patch yet for the problem I related above. The code pasted in the comment is where the problem is, not where a solution might be.

Patch #57 resolves my issue with hook_user_presave() not being invoked.

I'm not using proxies so can't offer any testing on the problem described in #60.

We installed this patch and see that the rules are firing as expected.
But we also had a problem.
The _cas_redirect_after_login function never sends the user to the correct page.

Does anyone else have this problem?
We only enable the cas login on certain pages, not every page.

I've looked through the code but am struggling to grasp how to control the redirect.
Any help would be appreciated.

peace,
michael

Any chance of this being rolled into the module? This rules integration would be great, but I'm unable to apply the patch against 7.x-1.7.