search

ohmage / gwt-front-end

The javascript/GWT code behind the front end.
6 stars 2 forks source link

Log users out when they close the web browser #265

Closed ctrance closed 11 years ago

ctrance commented 11 years ago

When I closed then re-opened the browser I'm still logged in. Please log the user out automatically after closing the browser.

stevenolen commented 11 years ago

Can you include what browser/OS this was tested on? Chrome, Firefox and possibly other browsers store cookies across browser termination. I believe, based on that, this is working as intended.

ctrance commented 11 years ago

Oops sorry about that, this issue was encountered on all browsers IE 9, FF 18.0.1, Chrome 21, Safari 5.1.7/Windows 7.

ctrance commented 11 years ago

@hongsudt - Tested this on all browsers and this issue has been fixed.

jeroen commented 11 years ago

It is generally a bad idea to hook actions to the page close event. I understand you want to help students secure their privacy, but this method is hacky and has a lot of nasty side effects. E.g. a user can have multiple tabs open with different dashboard or front-end pages. When one is closed, it shouldn't logout all of them. Other services like gmail and hotmail, which students also use and also contain privacy sensitive data, don't do this either.

The way to do is, is to let the cookie automatically expire when the browser is closed. According to Wikipedia, when no expiration date is set, a cookie is cleared when the user closes the browser:

The cookie setter can specify a deletion date, in which case the cookie will be removed on that date. If the cookie setter does not specify a date, the cookie is removed once the user quits his or her browser.

The cookie setter can specify a deletion date, in which case the cookie will be removed on that date. If the cookie setter does not specify a date, the cookie is removed once the user quits his or her browser.

Another way to help students is to configure browsers in schools to clear all history/cache/userdata when the browser is closed (they should do this anyway).

If you insist on deploying this method in the schools anyway, please make it configurable in the preference table, so that this hack doesn't affect other deployments.

jshslsky commented 11 years ago

So the default cookie timeout is 30 minutes. We could set it to something smaller for Mobilize. @hongsudt what do you think? I think @jeroenooms is correct here and we shouldn't be manually removing the cookie on browser close.

jeroen commented 11 years ago

Just set it to blank. That is the only way to make the cookie automatically remove itself when the browser closes. See: session cookie.

If you want to set a hard timeout, just expire the token on the server.

hongsudt commented 11 years ago

I don't want to set the timeout shorter than 30 mins.. If it is too short, it could be too annoying to use the front-end.. @joshuaselsky @jojenki what do you think about Jeroen's suggestion re tracking cookie timeout on the server side?

@stevenolen For 2.14 release, I think it might be ok to ask students to always logout from the app. What do you think?

jshslsky commented 11 years ago

On logout, the server sets the Max-Age property of the cookie to zero. If the user doesn't logout, the cookie will expire in the browser after 30 minutes. So, we handle both cases correctly. I am going to remove the onWindowClose handler for now because it is causing too many other problems.

jeroen commented 11 years ago

If you don't set a 'max-age' or 'expiration date' on the cookie, it will automatically be removed when the browser closes, which is in most cases desired behavior.

On Thu, Feb 14, 2013 at 10:55 AM, Joshua Selsky notifications@github.comwrote:

On logout, the server sets the Max-Age property of the cookie to zero. If the user doesn't logout, the cookie will expire in the browser after 30 minutes. So, we handle both cases correctly. I am going to remove the onWindowClose handler for now because it is causing too many other problems.

— Reply to this email directly or view it on GitHubhttps://github.com/cens/ohmageFrontEnd/issues/265#issuecomment-13571893.

jojenki commented 11 years ago

Asking people to logout seems extremely reasonable. The cookie timeout is just a thin safety net.

The cookie's expiration is updated every time a successful call is made. So, the user will only be automatically logged out if they do not interact with the website for 30 minutes. This seems to be unnecessarily long. I like Jeroen's suggestion of knocking it down to 5 or 10 minutes of inactivity.

jeroen commented 11 years ago

Haha that is not my suggestion. I don't care when you expire the token on the server. I just suggest to use a session cookie instead of a persistent cookie. Somewhat counter intuitively , but the safest value for timeout is just not setting it at all.

On Thu, Feb 14, 2013 at 11:08 AM, John Jenkins notifications@github.comwrote:

Asking people to logout seems extremely reasonable. The cookie timeout is just a thin safety net.

The cookie's expiration is updated every time a successful call is made. So, the user will only be automatically logged out if they do not interact with the website for 30 minutes. This seems to be unnecessarily long. I like Jeroen's suggestion of knocking it down to 5 or 10 minutes of inactivity.

— Reply to this email directly or view it on GitHubhttps://github.com/cens/ohmageFrontEnd/issues/265#issuecomment-13572547.

jshslsky commented 11 years ago

Let's just not set the timeout then.