search

catalyst / moodle-auth_saml2

SAML done 100% in Moodle, fast, simple, secure
https://moodle.org/plugins/auth_saml2
71 stars 134 forks source link

Rule out session age bug with cron cleanup #42

Open brendanheywood opened 8 years ago

brendanheywood commented 8 years ago

From plugin page comments:

We are using the setting Allow any auth types and importing our users into imap and ldap (for several reason) We also had these auth types disabled just to be sure all users must come in through saml

Discovered that the Moodle task Cleanup old sessions would then remove all sessions and we would get new sessions every cron which will cause you to get invalidsesskey if it runs while you are in the middle of forum post or another process

Its debatable if this is a bug as it should only affect saml2 under specific settings

brendanheywood commented 7 years ago

I have a vague feeling this may be due to the logged in time, and last login, timemodified timestamps not being set correctly. It should be easy to reproduce by setting the $CFG->sessiontimeout to something super small like 1 minute and then running the cleanup task:

php admin/tool/task/cli/schedule_task.php --execute='\core\task\session_cleanup_task'

This may also be an issue with auth_basic and others, so if you can reproduce this and fix it lets side port it to the other auth plugins as needed.

nhoobin commented 7 years ago

Keeping track of a few details that we discussed regarding the plugin page comment.

The session cleanup task will remove the sessions of plugins that have been disabled, which may be why the user was reporting that sessions were being regenerated and the sesskey was invalid. https://github.com/moodle/moodle/blob/master/lib/classes/session/manager.php#L744

During debugging we also found that it may be possible to implement an auth plugin hook ignore_timeout_hook to prevent the sessions from being removed based on how we code it. A basic implementation is on this commit https://github.com/nhoobin/moodle-auth_saml2/commit/4c2e6b3

Yet with the discovery of that hook we identified that core Moodle does not properly respect the return value for the function ignore_timeout_hook and we created a tracker ticket to resolve this. https://tracker.moodle.org/browse/MDL-56417