fnakstad / angular-client-side-auth

One way to implement authentication/authorization in Angular applications
http://angular-client-side-auth.herokuapp.com/
MIT License
1.63k stars 346 forks source link

Register already registerd user allowed. #3

Closed Kahramon closed 11 years ago

Kahramon commented 11 years ago

Hello Frederik,

a) I noted that if we want register already registered user, for example "admin" as a normal user, it behaves like we registered -> may be it can be fixed something like we get alert "you cannot register already registered user".

b) I would one suggestion to your app here: you can implement also "Remember me" functionality to do your app more usable. However, this is not so important and your app is still simple clever and thank you!

Kahramon

fnakstad commented 11 years ago

You are absolutely right! Since I intended this project only to work as a basic example with focus on the client-side handling of things, some of the server-side code is quite rushed :p Now that it's starting to gain popularity I think it would be a good idea to revisit and fix up though.

I'll try to set aside some time to look at it, but if you're up for it feel free to send me a pull request.

fnakstad commented 11 years ago

a) 452448f5fa12984b901d9d284c776a26258e6151 b) ae6f08390447dbce2b6cb6054f86444db602cd6e A session-type cookie is now set by default when logging in. Checking "Remember me" will store a persistent cookie. However, since sessions are stored in memory server-side they will expire as soon as the server is restarted.

Kahramon commented 11 years ago

Wow, thank you!

Kahramon commented 11 years ago

Hi,

it seems here is a bug: setting the MaxAge to 30 days than 7 days does not work? if(req.body.rememberme) req.session.cookie.maxAge = 1000 * 60 * 60 * 24 * 30;

fnakstad commented 11 years ago

I just tried changing it, and it seems to work for me... Could you be a little more specific about what problem you are encountering?

Kahramon commented 11 years ago

1) Change the code as above; 2) Login; 3) Restart the browser, 4) here I need to again login....-> not ok

Details: I cloned your last whole project to local and changed the above code in the routes.js and it works for refreshing the pages (F5), but if we restart the bowser again we need to login, simply it does not work. I tried this with different browsers like FireFox and Chrome. But, surprisingly it works with your originaly code: if(req.body.rememberme) req.session.cookie.maxAge = 1000 * 60 * 60 * 24 * 7;

fnakstad commented 11 years ago

Hmmm, I see something is fishy though for me it isn't related to the timeout of the cookie. I am also redirected to the login page if I close the browser window and reopen it, however I am still logged in so I can still visit pages like / and /private if I specify them in the location bar. Could you try repeating the steps you just posted, and then specifying / or /private in the location bar to see if that works? I'm looking into why it's behaving like this right now.

Kahramon commented 11 years ago

I am completely logged out after closing the browser, so "/private", "/admin" always redirects me to login.

fnakstad commented 11 years ago

Ah, yea. I'm getting the same behavior in Firefox now. Looks like it doesn't change the expiration date of the session cookie when logging in. I'll see if I can fix it right away.

Kahramon commented 11 years ago

By the way, if we set the cookie maxAge via app.use, it always works, however setting the maxAge in this way loses the meaning the "Remeber me" checkbox.

fnakstad commented 11 years ago

Yep, it looks like a problem with Express/Connect. Even if you change the maxAge property of an existing session, it won't send out an updated cookie to the browser...

fnakstad commented 11 years ago

Okay. I was finally able to fix this problem by using cookieSessions instead of normal Express sessions. This will store all the session data into the cookie and has the added benefit of any server restarts not causing any loss of existing session data. I pushed the fix out just now, so try it out and let me know if it works for you too :)

There's still an unrelated but similar-looking problem when restoring closed tabs with e.g. Chrome. If you close Chrome, and then open it again it will restore your previous tabs from a local cache. This means that the server won't be contacted, and thus the client has no information about whether the current user is authenticated or not. A simple refresh will fix this, but it's still annoying and I'm not quite sure how to tackle it yet...

Kahramon commented 11 years ago

It seems it is working now.