apereo
Create abstraction over session management
The session management needs an abstraction with an interface to allow it to be better integrated in to existing applications that have their own session management.
I have an implementation of this, but not sure if its correct, see https://github.com/chilversc/phpCAS/tree/session_handler for the current idea.
For an example of how this is then used, see https://github.com/chilversc/gallery3_cas_auth/blob/master/libraries/CAS_Gallery_Session_Adapter.php which attempts to add CAS authentication to gallery3.
I really like the idea. The code looks fine but i haven't really tested it yet.
@adamfranco: Do you have an opinion about this feature? I think it would really help integration into other software products.
I haven't tried the code yet, but in general your branch looks pretty clean.
The big question I have is: What is gained by this approach over making use of PHP's built in session_set_save_handler() method? As of PHP 5.4 there is also now a new SessionHandler interface that can be used to accomplish the same thing as session_set_save_handler() without rewriting all of the default functionality.
While I'm a big fan making as many parts of phpCAS as we can pluggable, I haven't been able to come up with a scenario where using session_set_save_handler() wouldn't accomplish the same result, and which also has the benefit of working transparently with all other code that uses sessions. If there is something lacking about session_set_save_handler() or some other good use-case for this new interface, I'd be happy to see this change tested and integrated.
@adamfranco: In my view the error in your argument is the assumtion is that everyone uses the proper session handler functions in php. :wink: I fully support your view that we should not reinvent the wheel and progamm as close to the php library as possible but I have seen many apps that have their own session handling stuff. Especially products that use databases for persisten session storage (remember me functions) come to mind. The simply assume they are a standalone app and can do whatever they want in terms of session storage because they have direct access to some DB.
But maybe @chilversc can explain a bit more what prompted this branch.
For instance, gallery3 does set up the session_set_save_handler, but it does so when you call session::instance($session_id = null).
So I didn't want to start changing things about the session without going through Gallery3's session methods in case Gallery3 wasn't expecting to have its session re-named from under it. Likewise, not calling session::instance seems to run the risk of having Gallery3 try and start a second session or destroy the current one.
My second thought is this should be a different abstraction based around how phpCAS interacts with the session. Hence the addition of the session_rename method. For instance, the open() method doesn't have the same contract as php's session_open(), allowing to phpCAS to simply say, and at this point I need the session to be open, removing the need to always check if a session is open before then opening it.
The idea of this abstraction is simplifies the CAS_Client class by allowing the CAS_Client class to indicate its intent as to what the session should be doing. For instance, it reduces a lot of the session handling code in handleLogoutRequests.
It's this second idea that I'm not sure if I've got it correct, as I'm not sure on all the nuances of how phpCAS uses sessions, so I just abstracted what was there.
I still haven't had a chance to test this yet, but since it has utility it gets my vote. :-) One thing I would like to see is some documentation that outlines when to use this interface and when not to (with specific use cases). I'll follow up with some inline comments (hopefully tagging them with #42 will link them here) and try to test the code tomorrow.
Ok, so I don't think my comments in your fork linked here automatically. Here they are:
- remove phpCAS session values before destroying session - I'm unclear as to how this change provides the
sessionHandlerwith more control. Wouldn't putting theunset($_SESSION['phpCAS']);line in the defaultdestroy()method allow for the most control? _sanatizeId($id)should probably be protected so that child classes don't have to re-work filtering if they just want to inject some functionality intoopenSpecificSession().
I look forward to this feature :) I used chilversc forks to create mine. I just delete all $_SESSION instructions for method in order to create full abstraction.
No, we have a lack of active developers. This is why this is simply a wishlist item at the moment.