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

Account verifying seems flawed #15

Open
sparik opened this issue Dec 29, 2016 · 7 comments
Open

Account verifying seems flawed #15

sparik opened this issue Dec 29, 2016 · 7 comments
Labels

Comments

@sparik
Copy link
Contributor

sparik commented Dec 29, 2016

I think this is supposed to be a setInterval instead of setTimeout. Otherwise it will clear the object only once, then the object will keep growing.
Even with that fix, an account verification link may be valid not for 1 hour, but for up to 1 hour. It may be valid for 1 second if a user registers 1 second before the register_email_users cleanup. This may be fixed by keeping a queue of registered users, pushing when a user registers and the link is sent, and poping all expired links every 1 hour, for example. Also this will make the implementation of 're-send the link' easier.

@fxfactorial
Copy link
Owner

Oops! yea, was supposed to be setInterval.

Yea I thought about the up to 1 hour issue, keeping a queue is a fancier version of what I imagined as the fix which would have been just to iterate the whole thing every hour.

Can you make a PR fixing the setInterval mistake?

@sparik
Copy link
Contributor Author

sparik commented Dec 29, 2016

I'm new to this. I'll try now

@sparik
Copy link
Contributor Author

sparik commented Dec 29, 2016

@fxfactorial done

@aantron
Copy link

aantron commented Dec 29, 2016

Was fixing all of this; but shouldn't we store this in the database. so the links set will survive a server restart?

@sparik
Copy link
Contributor Author

sparik commented Dec 29, 2016

Yes, I think the link along with its creation date (for checking if it's expired) should be stored in the database.

@fxfactorial
Copy link
Owner

fxfactorial commented Dec 29, 2016 via email

@aantron
Copy link

aantron commented Dec 29, 2016

Ok, already have it locally :) will PR later.

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

No branches or pull requests

3 participants