At the moment there's no way to have a request to user.logout trigger a CAS logout. Obviously this is not essential for all applications, but it would a nice configurable option.

CommentFileSizeAuthor
#11 cas_2536128_11.patch11.46 KByalet
#8 cas_2536128_8.patch5.39 KBcaseyfw
#6 cas_2536128_6.patch4.16 KBcaseyfw
#5 2536128_5.patch6.32 KByalet
#3 cas_2536128_3.patch7.16 KBpingers
#2 cas_2536128_2.patch6.96 KBcaseyfw
#1 cas_2536128_1.patch3.44 KBcaseyfw

Comments

caseyfw’s picture

caseyfw commented
Status: Active » Needs review
StatusFileSize
new cas_2536128_1.patch3.44 KB

Adds the "Drupal Logout Triggers CAS Logout" configuration item and adds a logout request handler to the Kernel Event subscriber.

The mechanism used to trigger a CAS logout is to simply fire a redirect to the cas.logout route. This isn't the most ideal solution because it means a request to /user/logout results in two successive 302 redirects. A better solution would be an internal forward to LogoutController::logout(), but I have no idea how to do that. I tried creating a request and adding it to the request stack, but no dice.

caseyfw’s picture

caseyfw commented
StatusFileSize
new cas_2536128_2.patch6.96 KB

Forgot to include logic to test if user session was authenticated through CAS. This fixes that issue.

During login, an "auth_provider" attribute is set to "cas" on the session. This is then used as the determinant when the user does a /user/logout to determine if a redirection to /caslogout should occur.

If anyone can think of a better way to do this, I'm all ears, but we kind of need a way to determine "was this session authenticated with CAS?"

pingers’s picture

pingers commented
StatusFileSize
new cas_2536128_3.patch7.16 KB

Re-rolled the patch against latest build...

yalet’s picture

yalet commented

I think we want a Route enhancer here to make the same logical check and switch the controller to the cas logout controller. I have to dig into this some more. We also store the session ID hashes of cas-authenticated sessions in the database for single sign out purposes (which is not fully implemented yet anyway), so we could look up sessions that way.

yalet’s picture

yalet commented
StatusFileSize
new 2536128_5.patch6.32 KB

Here's a patch that implements what I was talking about in my comment: a route enhancer and lookup from the database.

caseyfw’s picture

caseyfw commented
StatusFileSize
new cas_2536128_6.patch4.16 KB

Re-rolled patch for current head. Removed trailing whitespace and maintenance_access config from routing.yml.

yalet’s picture

yalet commented
Status: Needs review » Needs work

I think your patch is missing the class file that actually declares the route enhancer (probably didn't add the file to git before patch generate?).

caseyfw’s picture

caseyfw commented
Status: Needs work » Needs review
StatusFileSize
new cas_2536128_8.patch5.39 KB

Awww, dammit. Yeah, sorry - new to the patch workflow.

yalet’s picture

yalet commented

This is going to need to be reworked a little, based on what we decided about SLO and storing cas-authenticated session identifiers. This doesn't affect the mechanics of the logout redirect, only the method of checking whether we have a cas session or not.

yalet’s picture

yalet commented

Actually, no. We can just add a column for non-hashed session id when we implement SLO and leave this alone.

yalet’s picture

yalet commented
StatusFileSize
new cas_2536128_11.patch11.46 KB

Added some tests.

  • yalet committed 011d292 on 8.x-1.x 
    Issue #2536128 by caseyfw, yalet, pingers: Setting to trigger CAS logout...
yalet’s picture

yalet commented
Status: Needs review » Fixed

Committed, thanks everyone.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.