feat(login): add remember me functionality

[remicastaing]

Notes:

• $sessionStorage is used when « remember me » is not checked
• $localStorage is used when « remember me » is checked
• with « remember me » checked, the token will expire after 7 days

Closes #259 (partly)

[DaftMonk]

One issue I have with local/session storage: when you log in and open a new tab or new window, you're logged out. I don't think thats acceptable behavior.

[remicastaing]

I don't think thats acceptable behavior.

I would say, it's a NO GO. ;)

[meeDamian]

I'm not sure what this feature does exactly. How is localStorage useful with

• remember me
  ?

Isn't extending token lifetime enough?

IMO:

1. We should make sure that browsers are able to remember contents of login and password fields correctly,
2. Checking remember me should inform server to generate token that lives longer (a year?), but I'm not sure if that's secure...

Here I extended token lifetime to 30 days by default, that should do in most cases (esp. if we combine it with 1.)

---

Alternatively, but that's more difficult and I'm not certain if worth doing, with remember me checked in we should generate refreshToken that would allow users to get new token once previous expires. That would also increase size of each request...

[remicastaing]

ngCookie store the token only in a session cookie. That means the token is lost every time the user close his browser. That's because I switch to ngStorage.

If you want to keep the token longer, you have to store it in a local cookie (not supported with ngCookie).

IMO:

• we shouldn't store login and password in the brower. It's too easy to grab.
• checking remember me should generate a longer living token (one week, month, year, what ever) and not checking it, should generate a short living token (a couple of hours).

I will work on it and will come back with a new PR.

[meeDamian]

Regarding 1. I'm not saying we should store it in browser. Browsers already do that, and that's what users are used to, we should only make sure we aren't blocking this default behaviour. I hate websites where it's blocked (except for banks).

Also browsers always prompt you whether or not you want those credentials stored.

[DaftMonk]

Maybe we should swap out ngCookie for a more robust cookie library so that we aren't restricted to a session cookie.

[remicastaing]

Sorry @chester1000, I didn't read your point 1 thoroughly enough. You're right.

@DaftMonk, I also found another npm: basil.js and I had in mind to angularize it in a service.

[remicastaing]

Good news: the token seems to be not shared between taps/windows only with $sessionStorage. With $localStoragethe token is available.
I could keep on $cookieStorewhen remember me is not checked and make a new PR.
Or we could find some other cookieStore which would handle both session and local storage. Perhaps:

• https://github.com/ivpusic/angular-cookie (I don't like the API);
• an angularized https://github.com/Wisembly/basil.js
• or writing our own token storage service/module;

[remicastaing]

I replaced $sessionStorage simply by $cookies if remember meis not check and it works fine.

[DaftMonk]

I'm still having this issue with localStorage

[remicastaing]

For on Chrome, Firefox and Safari, It can't reproduce the issue with localStorage.

What is your browser?

Here is a browser compatibility list.

[meeDamian]

Anyway, do we really need this? Seems like a-lot-of-services-don't-even-bother-with-this-feature-any-more.

[meeDamian]

Oh, I thought that this issue changes default behaviour to remove token upon exit when remember me isn't checked, but #381 showed me I was wrong :).

How about skipping remember me checkbox, and just remembering token for at least it's lifetime? After that it will be useless anyway and before that, even if we remove it from user's browser storage, someone who obtained it will still be able to login as a user. It just gives illusion of security.

IMO:
if remember me is to stay:

• it's lifetime should be regulated by generation process, by altering expiresInMinutes value.

if remember me is to be dropped:

• hardcoded token lifetime (30 days?).

In both cases:

• token should be stored in localStorage for as long as possible,
• we should consider adding revoke token functionality, but it's relatively more complex.

[remicastaing]

Anyway, do we really need this? Seems like a-lot-of-services-don't-even-bother-with-this-feature-any-more.

Totally right. If you develop an app, you should able to say if security is a critical point (like banking web portal). So you could choose between never remember and ever remember until logout. Skipping the checkbox would be the right option.

someone who obtained it will still be able to login as a user

I don't know if there is something to do about it. Is there a mean to link the token with the browser? I don't think so.

IMO expiresInMinutes could be in the configuration file.

I tried to replace $cookies by localStorage and using

windows.onbeforeunload = function(){
    delete $localStorage.token;
}

but I can't figure out why it isn't working. Must be to tired... Any thought?

[meeDamian]

There are ways to revoke tokens. We would have to notify server about it upon logout, though.

[remicastaing]

Why notifying the server? The token is only stored on the client side, isn't it?

[meeDamian]

Yes, but token revoking must happen on the server side.

[remicastaing]

I don't understand how you can revoke a token? The verification is only based on the secret and expiration date.

The only way I see to revoke token would be to store the revoked token in a database. What is the point to use token if you need to perform a request to a database to check if a token is not revoked? Perhaps, by restraining this request only for specifics restricted endpoints?

[meeDamian]

As I said:

• we should consider adding revoke token functionality, but it's relatively more complex.

The easiest to implement, but also a little silly solution would be to attach revoked_tokens: [] property to User object, and checking if currently used token exists in this array inside of auth.isAuthenticated().

[remicastaing]

You convinced me. But why do mean it is little silly?

And btw, any idea why

windows.onbeforeunload = function(){
    delete $localStorage.token;
}

isn't working? I also tried with $window.

[meeDamian]

I'm not sure if all browsers respect onbeforeunload.

AFAIK you MUST return from this function ex. return null;

If that won't work, try putting your code inside: window.addEventListener('beforeunload', function (e) {});

[DaftMonk]

I think the default functionality should be to keep the token for as long as its valid, and set its expiration to around a month.

But as @chester1000 pointed out, we might want a way to invalidate tokens from the server side. This is one way I was thinking of implementing server side logouts for tokens: http://stackoverflow.com/questions/21978658/invalidating-json-web-tokens/24235103#24235103

It wouldn't be stateless, which is one of the advantages of using jwt in the first place, but you really can't get around requiring some state to be stored on the server if you're going to do a server side logout.

[meeDamian]

@DaftMonk how about multi-device setup?

[DaftMonk]

@chester1000 It would log you out everywhere, which wouldn't be ideal in some cases.

That said, with a blacklist of tokens, it could get messy over time if you don't clean up expired tokens.

[remicastaing]

It wouldn't be stateless, which is one of the advantages of using jwt in the first place, but you really can't get around requiring some state to be stored on the server if you're going to do a server side logout.

As @chester1000 said revoked token could be stored in the userdata. They don't have to be checked every times, for all requests, but every times the user data are requested.

[DaftMonk]

@remicastaing Right, but that still wouldn't be stateless.

[meeDamian]

No revoking solution will be stateless per se.

The best thing that comes to my mind is to create new DB blacklisted_tokenscollection, but instead of calling it with each request, we can just keep it in memory.

For one-instance setup it would be enough to append in-memory list with each revoke, for multiple-instance setup we can sync it with db each ex. 5 minutes. With this, number of db requests is independent to server load.

We obviously store tokens there only until they expire.

[remicastaing]

No revoking solution will be stateless per se.

+1

We obviously store tokens there only until they expire.

But, we would have a revoked token for each logout and with paranoid users, it would be a lot.

To keep the amount of revoked token in db/memory constant, we could set the expiration time of 2 days and provide a new token every day. Then we would have one revoked token per user and all per logout revoked token for the last two days.

[remicastaing]

I think the default functionality should be to keep the token for as long as its valid, and set its expiration to around a month.

Should I start to strip the remember me out and submit a new PR only based of localStorage?

I would keep the token revocation for later.

[meeDamian]

Well, we can implement it with short living tokens, and then add refreshToken to the mix. So before we ask user to login again we can negotiate new token with server first. But I'm not sure if that will not over-complicate this boilerplate. Opinions?

[remicastaing]

I made a new PR: #444.