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

move ActiveRecord dependency into ActiveRecordTicketStore #61

Closed nicolai86 closed 4 years ago

nicolai86 commented 11 years ago

The AbstractTicketStore should not be coupled to any storage engine at all because it's supposed to be abstract. Thus, I've introduced the method destroy_session_with_session_id(session_id, st) which needs to be implemented per TicketStore subclass.

With this change it's quite easy to enable single sign out using redis-store and no shared tmp sessions directory.

The tests are not green yet for the LocalDirTicketStore but hopefully I can fix this tomorrow.

If you like I'd create a second pr after this one which contains a RedisTicketStore implementation, as it eased the single sign out integration for me.

thank you :)

nicolai86 commented 11 years ago

Some discussion about the changes involved would be great.

In order to get the tests to pass again I had to duplicate the session.delete and ActiveRecord::SessionStore.session_class.find … parts, which is not what I wanted.

I think the current naming is a bit misleading because it suggests that e.g. LocalHashTicketStore does not depend on an ActiveRecord Session class. If the naming is correct the tests would need a little adjustment and the ActiveRecord dependency (including tests) can be dropped all together - except for the ActiveRecordTicketStore

Any thoughts?

nicolai86 commented 11 years ago

I've made a small adjustment just to demonstrate what I'm after.

Instead of explicitly checking for an ActiveRecord Session I'm just asking the SessionStore instance if it can resolve a given service_ticket to a session. This should not be possible after sign out I guess.

All tests are passing, and ActiveRecord is only required if an ActiveRecordTicketStore is used.