Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Register already registerd user allowed. #3

Closed
Kahramon opened this issue May 11, 2013 · 13 comments
Closed

Register already registerd user allowed. #3

Kahramon opened this issue May 11, 2013 · 13 comments

Comments

@Kahramon
Copy link

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
Copy link
Owner

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
Copy link
Owner

a) 452448f
b) ae6f083 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
Copy link
Author

Wow, thank you!

@Kahramon
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

  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
Copy link
Owner

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
Copy link
Author

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

@fnakstad
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

It seems it is working now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants