Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single-Sign-Out not working (for me) #47

Closed
nicolai86 opened this issue Apr 20, 2012 · 12 comments
Closed

Single-Sign-Out not working (for me) #47

nicolai86 opened this issue Apr 20, 2012 · 12 comments
Assignees

Comments

@nicolai86
Copy link
Contributor

Hey!

I'm using multiple Rails 3.2.3 apps which authenticate against a single rubycas-server using devise + devise_cas_authenticatable.

First, the authentication works great, thanks for all the hard work and affort you've put into the gem.

But when I enable the single-sign-out functionality in both the rubycas-server and devise_cas_authenticatable single-sign-out does not work. According to the rubycas-server logfile the sign-out notifications are posted to the correct services. I can validate that my applications are receiving the posted notifications, thus invoking Devise::CasSessionsController#single_sign_out.

However, the user sessions are only destroyed in the application which initiated the original logout action.
In the other applications the log reads like this:

Intercepted single-sign-out request for CAS session ST-<some-session>
Found session id  for index ST-<some-session>
... rendering stuff, done

Using the logs I've traced the route the code takes through the gem and it turns out that the session-id as returned by find_session_id_by_index is nil. I have no idea why this is happening.

Any thoughts on how I can debug this?

Kind regards & keep up the good work,
Raphael

@nbudin
Copy link
Owner

nbudin commented Apr 20, 2012

Thanks for the bug report, Raphael!

Is the session ID the same on all the applications' logs, or is it different for each?

@nicolai86
Copy link
Contributor Author

The ID's are different, as far as I can see.

I might have noticed the source of my problem tho: I missed a log-flush which only happened after another request hit the app.

A single sign out request was received for ticket ST-1334927739r55804EF465ED310EEF but the Rails session_store is not a type supported for single-sign-out by devise_cas_authenticatable.

I'm using ActiveRecord::SessionStore. Shouldn't it be supported?

@nbudin
Copy link
Owner

nbudin commented Apr 20, 2012

Ah, interesting. ActiveRecord::SessionStore should indeed be supported.

Looking at that code path, I see there are two ways it can produce that warning:

  1. The global Rails object doesn't respond to application. (This is unlikely to be the case unless you're using Rails earlier than 3.0.)
  2. Rails.application.config.session_store doesn't respond to destroy_session. Could you check that this is true in a Rails console? (If it's true, something strange is going on that's preventing this action from seeing it.)

@nicolai86
Copy link
Contributor Author

The SessionStore indeed does not respond to destroy_session

1.9.3p125 :002 > Rails.application.config.session_store.destroy_session
NoMethodError: undefined method `destroy_session' for ActiveRecord::SessionStore:Class

Adding this code in active_record.rb fixes the missing method error but leads to a redirect loop upon sign-in.

def destroy_session(env, session_id, options)
  if session = ActiveRecord::SessionStore::Session.find_by_session_id(session_id)
    Rails.logger.debug "destroying session with id #{session_id}"
    session.destroy
  end
end

I've search for places where destroy_session is called, but it looks like it's called outside of devise_cas_authenticatable.

@nicolai86
Copy link
Contributor Author

Turns out destroy_session is called inside rack 1.4.1:
Here's the call stack for destroy_session:

"gems/rack-1.4.1/lib/rack/session/abstract/id.rb:312:in `commit_session'",
"gems/rack-1.4.1/lib/rack/session/abstract/id.rb:206:in `context'",
"gems/rack-1.4.1/lib/rack/session/abstract/id.rb:200:in `call'",
"gems/actionpack-3.2.2/lib/action_dispatch/middleware/cookies.rb:338:in `call'",
"gems/activerecord-3.2.2/lib/active_record/query_cache.rb:64:in `call'",
"gems/activerecord-3.2.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:443:in `call'",
"gems/actionpack-3.2.2/lib/action_dispatch/middleware/callbacks.rb:28:in `block in call'",
"gems/activesupport-3.2.2/lib/active_support/callbacks.rb:405:in `_run__1078363315929402895__call__3185202986236687197__callbacks'",
"gems/activesupport-3.2.2/lib/active_support/callbacks.rb:405:in `__run_callback'",
"gems/activesupport-3.2.2/lib/active_support/callbacks.rb:385:in `_run_call_callbacks'",
"gems/activesupport-3.2.2/lib/active_support/callbacks.rb:81:in `run_callbacks'",
"gems/actionpack-3.2.2/lib/action_dispatch/middleware/callbacks.rb:27:in `call'",
"gems/actionpack-3.2.2/lib/action_dispatch/middleware/reloader.rb:65:in `call'",
"gems/actionpack-3.2.2/lib/action_dispatch/middleware/remote_ip.rb:31:in `call'",
"gems/actionpack-3.2.2/lib/action_dispatch/middleware/debug_exceptions.rb:16:in `call'",
"gems/actionpack-3.2.2/lib/action_dispatch/middleware/show_exceptions.rb:56:in `call'",
"gems/railties-3.2.2/lib/rails/rack/logger.rb:26:in `call_app'",
"gems/railties-3.2.2/lib/rails/rack/logger.rb:16:in `call'",
"gems/actionpack-3.2.2/lib/action_dispatch/middleware/request_id.rb:22:in `call'",
"gems/rack-1.4.1/lib/rack/methodoverride.rb:21:in `call'",
"gems/rack-1.4.1/lib/rack/runtime.rb:17:in `call'",
"gems/activesupport-3.2.2/lib/active_support/cache/strategy/local_cache.rb:72:in `call'",
"gems/rack-1.4.1/lib/rack/lock.rb:15:in `call'",
"gems/actionpack-3.2.2/lib/action_dispatch/middleware/static.rb:61:in `call'",
"gems/railties-3.2.2/lib/rails/engine.rb:479:in `call'",
"gems/railties-3.2.2/lib/rails/application.rb:220:in `call'",
"gems/rack-1.4.1/lib/rack/content_length.rb:14:in `call'",
"gems/railties-3.2.2/lib/rails/rack/log_tailer.rb:14:in `call'",
"gems/puma-1.2.1/lib/puma/server.rb:415:in `handle_request'",
"gems/puma-1.2.1/lib/puma/server.rb:288:in `process_client'",
"gems/puma-1.2.1/lib/puma/server.rb:197:in `block in run'",
"gems/puma-1.2.1/lib/puma/thread_pool.rb:92:in `call'",
"gems/puma-1.2.1/lib/puma/thread_pool.rb:92:in `block in spawn_thread'"]

@nbudin
Copy link
Owner

nbudin commented Apr 20, 2012

Oh man, super weird...

I'm going to message jeremyhaile, who wrote the single sign-out support, and see if he can come over and have a look at this. We're a bit beyond my depth here. Sorry!

@ghost ghost assigned jeremyhaile Apr 20, 2012
@nbudin
Copy link
Owner

nbudin commented Apr 20, 2012

Hey Jeremy - sorry about the assign, but Github seems to have removed the "Message" feature from user pages, so this was the best way I could figure out to notify you. If you have any idea what might be going on with this one, could you comment here?

Thanks!

@nicolai86
Copy link
Contributor Author

Btw: using Redis as SessionStore doesnt work with Single-Sign-Out either:

1.9.3p125 :001 >  Rails.application.config.session_store
 => ActionDispatch::Session::RedisStore 
1.9.3p125 :003 >  Rails.application.config.session_store.respond_to? :destroy_session
 => false 

@nicolai86
Copy link
Contributor Author

I got Single Sign Out to work with Rails 3.2.2 & 3.2.3 using active_record_store in my fork.
Funny thing is set_session seems to do it's job ( tho I might just remove that and see if it still works ) but destroy_session doesn't.

I did some digging in the rails source to see how they work with ActiveRecord::SessionStore, and I made some small adjustments for session removal. Works great for me. Maybe we should think about restructuring the SingleSignOut part a bit, as well as at tests for it?

@nbudin
Copy link
Owner

nbudin commented Apr 20, 2012

IMO, the changes you're doing make a lot of sense. I'd like to see Redis support back in there, but in general, I agree with the direction your fork has taken.

Also agree that tests would be good.

@jeremyhaile
Copy link
Collaborator

No problem - unfortunately my original single_sign_out implementation worked in Rails 3.0, but not Rails 3.1 or 3.2 due to changes in the internals of the session management code. Unfortunately Rails doesn't provide a clean way to access the actual session store that I've been able to find. In my original version, I was tying into a prepare method that all session stores implemented in 3.0, but it was removed in 3.1. nbudin tried to update my code to make it compatible with Rails 3.1, but completely broke it. He assumed that the :session_store stored in the application config was the actual session store, but in reality it is just a symbol that tells Rails which session store to use.

In my fork of devise_cas-authenticatable, I updated it again to work in Rails 3.1, but hadn't tested it thoroughly enough to send a pull request although I am using it in a production project and it's working fine. I've been unable to find a way to cleanly get at the SessionStore in Rails 3.1, so I gave up. Instead I just create a filter that needs to be added to ApplicationController. For example: "include DeviseCasAuthenticatable::SingleSignOut::StoreSessionIdFilter" This seems to be a much more reliable way to get at the session ID, although it's Rails specific.

I rebased and retested the fork this morning to ensure it works with the latest changes. I went ahead and created a pull request to discuss this more effectively:
#48

Feel free to try out my branch and see if that works. Then we can clean it up and integrate if appropriate.

@nicolai86
Copy link
Contributor Author

Thanks for your work, seems to work for me. Personally I don't like the approach with a before_filter, which is why I'd opt-in to staying with the current approach, and changing it a little into a more OO pattern without Monkey Patches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants