rubycas / rubycas-client

Ruby client for Yale's Central Authentication Service protocol -- an open source enterprise single sign on system for web applications.
http://code.google.com/p/rubycas-client/
Other
332 stars 217 forks source link

Use the 'service_ticket' column in ActiveRecord storage #48

Closed ghost closed 12 years ago

ghost commented 12 years ago

Values stored in ActiveRecord sessions are not mapped onto database columns. They're serialized and stored in the data column. But read_service_session_lookup seems to be expecting service tickets to be stored in the service_ticket column of the sessions table:

def read_service_session_lookup(st)
  raise CASException, "No service_ticket specified." unless st
  st = st.ticket if st.kind_of? ServiceTicket
  session = ActiveRecord::SessionStore::Session.find_by_service_ticket(st) # <--
  session ? session.session_id : nil
end

I'd expect it to be set in store_service_session_lookup, but it seems to be stored in the session itself, instead:

def store_service_session_lookup(st, controller)
  raise CASException, "No service_ticket specified." unless st
  raise CASException, "No controller specified." unless controller

  st = st.ticket if st.kind_of? ServiceTicket
  session = controller.session
  session[:service_ticket] = st # <--
end

As a result, the service_ticket column of my sessions table is all NULLs. After this patch is applied, it's full of service tickets.


This seems to have been the case for a long time, so perhaps I'm missing something, but I can't see how this was ever working. Perhaps I'm doing it wrong. Your feedback would be appreciated, either way.

travisbot commented 12 years ago

This pull request fails (merged 638c1cd1 into 23fd4db5).

soupmatt commented 12 years ago

You need to add the service_ticket column to your sessions table. The documentation for that is there, even though it isn't the clearest.

adammck commented 12 years ago

The service_ticket column is definitely added to my sessions table. The documentation was clear enough on that. The problem is that it is not being populated; the ticket was being stored in the session data.

It's possible that the implementation has changed between Rails versions, and we're seeing different results. I'm on 2.3.14, which as you can see here and here, will only ever set the session_id and data columns. Even when the session contains keys which have corresponding columns on the sessions table, it will serialize them into the data column. Hence, session[:service_ticket]=x will not work as expected.

soupmatt commented 12 years ago

I'll see if can reproduce the issue. I have apps on 2.3.11 and 2.3.14 that are using this, I'll double check that column value is getting set correctly.

Sent from my iPad

On Sep 5, 2012, at 1:31 PM, Adam Mckaig notifications@github.com wrote:

The service_ticket column is definitely added to my sessions table. The documentation was clear enough on that. The problem is that it is not being populated; the ticket was being stored in the session data.

It's possible that the implementation has changed between Rails versions, and we're seeing different results. I'm on 2.3.14, which as you can see herehttps://github.com/rails/rails/blob/v2.3.14/activerecord/lib/active_record/session_store.rb#L246and herehttps://github.com/rails/rails/blob/v2.3.14/activerecord/lib/active_record/session_store.rb#L121, will only ever set the session_id and data columns. Even when the session contains keys which have corresponding columns on the sessions table, it will serialize them into the data column. Hence, session[:service_ticket]=xwill not work as expected.

— Reply to this email directly or view it on GitHubhttps://github.com/rubycas/rubycas-client/pull/48#issuecomment-8308783.

adammck commented 12 years ago

Thanks very much for looking into this. I know that I sound like a crazy person.