WebDevStudios / aad-sso-wordpress

Azure Active Directory Single Sign-on for WordPress
MIT License
11 stars 6 forks source link

Remove sessions dependency #3

Open jtsternberg opened 9 years ago

jtsternberg commented 9 years ago

At first glance, I don't see that there's anything requiring sessions, and the php error log is filled with "PHP Fatal error: Maximum execution time of 300 seconds exceeded in D:\home\site\wwwroot\wp-content\plugins\aad-sso-wordpress\aad-sso-wordpress.php on line 110" on IIS. Based on call with Microsoft team, the plan is to remove that dependency.

psignoret commented 9 years ago

$_SESSION is really only used for debugging (there are better ways if doing this) and for the nonce and state parameters of the authorization request. If you use a different method (e.g. WP_Session to persist the antiforgery ID (used for both nonce and state) between the authorization request and the authorization response), then you don't need PHP sessions.

jtsternberg commented 9 years ago

Thanks @psignoret, that's good to hear. We'll be making this a priority to remove in the next few weeks, I'm guessing.

psignoret commented 9 years ago

Would probably be worthwhile to understand why you're getting the error, though. I can't repro.

jtsternberg commented 9 years ago

@psignoret I think it has more to do with the environment/wincache/etc than the actual plugin itself, but I see it as an opportunity for improvement in the plugin itself. SESSIONS are a mixed bag of issues, so avoiding them, if possible, is ideal IMO.

psignoret commented 9 years ago

@jtsternberg Agreed, they were meant to be a stop-gap method anyway.

jtsternberg commented 9 years ago

Ah yah, definitely understand @psignoret!