Currently, when the CAS client module logs in a CAS user, it stores the cas_session_id and the user ID into the cas_login_data table. Then when it receives a valid logoutRequest from the CAS server, it looks up the uid, and logs out ALL sessions for that user. So if you have two separate sessions on two separate computers, and you log out on one of them, you get logged out on the other as well. This doesn't strike me as intended behaviour. I'm planning to write a patch to store the session ID in cas_login_data instead of the user ID, and only log out the appropriate session. (This is sort of related to CAS server issue #1569766: Old tickets are never purged, where I'm hoping to only send logoutRequests for tickets related to the appropriate session on the server.) Does anyone see a problem with or have objections to this approach?
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | interdiff-1-14.txt | 440 bytes | mvc |
| #14 | cas-logoutRequest_should_work_per_session_not_per_user-1908878-14.patch | 3.65 KB | mvc |
Comments
Comment #1
olarin commentedOK, here's a patch which does what I said. In conjunction with the cas_server patch in the issue referenced above, I was able to get my all-Drupal CAS client and server test setup to properly perform single sign outs on a per-session basis, rather than per-user.
I should note that the update hook here empties the sessions table and cas_login_data table (effectively logging everybody out when it's run) because I was paranoid about things getting out of sync (and unlike with the aforementioned server patch I didn't feel taking an educated guess at which session went with which ticket was "good enough" in this case).
Let's see if I can get a patch right on first try today...
Comment #2
olarin commentedComment #3
bkosborneSo I'm wondering why you don't think it's appropriate to log out the user completely - for all their sessions? I actually think it's more unexpected to log out just that browser session. If someone logs out of a single sign on service, they would likely think they were logging out of all sites that used it as well. What's the use case for just removing the session tied to that browser?
Comment #4
metzlerd commentedI think I agree with this one. The cas server would maintain two different sessions, one for each browser. If you revisited the cas server, via another web page on the second browser, I think you'd find yourself still logged in and it would just be the drupal session that would be logged out. We could test this to be sure, but I'm betting this case would prove the inconsistent behavior.
Comment #5
emsearcy commentedIt was also pointed out (by metzlerd) in #1569766: Old tickets are never purged that JASIG CAS operates on browser sessions as well, not user, fwiw.
Comment #6
yalet commentedI agree with #4 and #5 here. I would expect that if I logged out of CAS in one browser, existing CAS sessions in another browser would still be valid.
Also my interpretation of the logout section of the protocol (http://www.jasig.org/cas/protocol)
is that sessions are expired, not users. It refers to the cookie being destroyed, which obviously only affects the browser session.
Comment #7
yalet commentedComment #8
yalet commentedI think I misread the discussion here. Re-opening.
Comment #9
bkosborneHmm yeah ok that seems to be the general consensus then. This patch just needs to be reviewed and possibly re-rolled.
Comment #10
coolkind commentedIt's appropriate to log out the session rather than the user as it can avoid some unnecessary force authentication for those not logged out sessions. Normally, it only takes extra server calls. In some scenarios, it will affect AJAX calls.
Here is my case:
Session A and Session B of same user are logged in. Session B needs to do another authentication to CAS server once Session A logged out. As my CAS server is set to user HTTPS protocol only, all session B's HTTP ajax calls before a page refresh will be redirected to CAS authentication and not receive any response because of cross domain since Session A logged out.
Although this can be solved by CORS or XDR proxy, the root cause is CAS module empty the correct session B.
Comment #11
mvcI'm not sure why this calls db_drop_field() in cas_update_7103() as this field shouldn't already exist. However, that seems harmless (I checked, it simply returns FALSE if the field doesn't exist, as opposed to throwing an error). Other than that this looks good to me.
Re-rolling & setting version to 7.x-1.x-dev.
Comment #14
mvcOops, forgot to update the function name when I rerolled for latest dev.
Comment #15
mvc