openwebwork / webwork2

Course management front end for WeBWorK
http://webwork.maa.org/wiki/Main_Page
Other
145 stars 165 forks source link

Remove the $CookieLifeTime setting and use $sessionKeyTimeout instead. #2412

Closed drgrice1 closed 4 months ago

drgrice1 commented 5 months ago

The cookie lifetime should be the same as the session timeout.

It doesn't make sense to keep the cookie any longer than the session is valid for. That only results in stale cookies sitting unusable in the browser cache.

The other direction is even worse. If the cookie lifetime is less than the session key timeout, then the user will need to sign in again before the session key timeout actually occurs.

So remove the $CookieLifeTime setting entirely, and just use the $sessionKeyTimeout directly.

Also correct the $CookieSameSite documentation in localOverrides.conf. I think this was a copy and paste issue.

This is an alternate to #2408 and is probably the better way to go.

drgrice1 commented 5 months ago

I also want to point out that the $CookieLifeTime and $sessionKeyTimeout really needing to be the same is not new with the recent revisions to authentication. This has always been the case for all versions of WeBWorK for which the $CookieLifeTime setting existed.

somiaj commented 5 months ago

Overall this looks good, but I do notice a few things.

First, it appears you are removing the option to have a session life span, as $sessionKeyTimeout doesn't mention this option. Or would setting this to zero also make it so everything is session vs a set timeout? Or does the new auth settings allow for this in another way?

Second is in response to the default timeout of 30 mins. This is reasonable, but can be a bit short in some cases. I have students who spend 30-40 mins on a problem, and by the time they enter in an answer they are timed out and have to login again via the LMS (I have increased the timeout to deal with this which is fine).

But this does make me wonder what a good timeout would be. I am using 3 hours, but you mentioned you see anything over an hour as a security concern. Would this depend on how users are accessing WeBWorK? Lots of sites I use have longer than hour sessions, but I'm using a trusted computer only I have access to. Would what a reasonable amount be depend on if students are accessing webwork from a shared computer lab or a personal laptop? Because I could see a computer lab would wanting a shorter time out, but a trusted computer could use a longer timeout.

So last, could the newer auth option for students to remember them also tie into this, so maybe a longer time could be used for trusted machines, and a short for shared machines? I could also be overlooking some other security concern and shouldn't' be using a 3 hour timeout here.

drgrice1 commented 5 months ago

Yes, with this there no longer is a session life span option for cookies. The option does not entirely make sense with WeBWorK's authentication mechanism. Authentication is tied to the key that is stored in the database, and the database is not aware of the browser session.

A default of 30 minutes is reasonable. Anyone that wants to use a longer timeout can change the setting. Note that many sites that use longer timeouts also don't have the security concerns that WeBWorK has. Other sites that have security concerns (sites that expose confidential data) also have short timeouts. For example, most financial sites (banks, credit cards, and such) have much shorter timeouts.

There is no way to determine that a shared computer versus a personal computer is in use. Also, what newer auth option are you talking about? Are you referring to the "Remember Me" button that is only available if session_management_via = "key" is set? That is a very old option though.

pstaabp commented 4 months ago

I think this better than #2408 even though I just approved that.

drgrice1 commented 4 months ago

Several changes have now been added to this pull request.

pstaabp commented 4 months ago

Looking at the code, will this new feature of the server time handle timezone. I'm thinking if the client is in a different timezone from the server, will the time remaining be handled correctly?

drgrice1 commented 4 months ago

The time zone the user is in won't matter. This doesn't change the math that was used, and it will work regardless. The server time is always used. The code just finds the difference between the server time and the user's time.