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

Beta: sign-in links do not expire after 15 minutes and are reusable #16912

Closed
ghost opened this issue Mar 18, 2018 · 9 comments
Closed

Beta: sign-in links do not expire after 15 minutes and are reusable #16912

ghost opened this issue Mar 18, 2018 · 9 comments
Assignees

Comments

@ghost
Copy link

ghost commented Mar 18, 2018

Issue Description

I am able to sign in using the links well after 15 minutes (and in one case, 2 hours). And I can do so repeatedly using the same link in different browsers (private mode) and on different devices. Reproducible on my local copy of freeCodeCamp and on beta.freecodecamp.org.

Browser Information

  • Browser Name, Version: Chrome, Firefox, Safari
  • Operating System: macOS
  • Mobile, Desktop, or Tablet: mobile and desktop
@ghost
Copy link
Author

ghost commented Mar 18, 2018

Wait… just checked my MongoDB records. The auth tokens' TTLs are all set to 900000. Is that intentional?

@ojongerius
Copy link
Contributor

ojongerius commented Mar 18, 2018

@leonfeng it is: they use miliseconds (15 * 60 * 1000) see

return this.createAuthToken({ ttl: 15 * 60 * 1000 });

Ok, I can confirm that:

  • authentication links are not deleted after use
  • authentication links are valid for longer than the ttl

My guess is that they will never expire, as I do not see the code for it, but I'm pretty new to the code base.

/cc @BerkeleyTrue ccing you as you've implemented standalone auth tokens in 07f3042

@raisedadead raisedadead self-assigned this Mar 19, 2018
@raisedadead
Copy link
Member

Hi @leonfeng

Nice find.

I guess, the last discussions we had about an year ago that MongoDB should be doing this for us. One of the causes this could be the TTL expiration thread needs to be configured explicitly?

But, I guess we would have to have this in to a message queue, and schedule it.

I will investigate further.

@raisedadead
Copy link
Member

Or, maybe we simply missed the invalidation in a recent refactoring.

@ojongerius
Copy link
Contributor

I'll just drop here what I cam across trying to unravel this little puzzle, hope it's helpful:

Intention to validate:

return authToken.validate$()

Intention to destroy:

return authToken.destroy$();

I noticed that validation of length and existence of the token does happen, I'm not familiar with the code base and frameworks but that seems to happen in passwordlessGetValidators:

const passwordlessGetValidators = [

I guess a shortcut is to add TTL validation in passwordlessGetValidators, but as you still need to solve the removal of used tokens, I'd suggest fixing it properly.

@scissorsneedfoodtoo
Copy link
Contributor

Hi all. I saw that @raisedadead submitted a PR to migrate authentication to Auth0, but I thought I'd comment here.

Did some searching through the Mongo docs for getting TTL to work. Seems like Mongo will automatically remove documents, but an index needs to be created on the collection first. To get the email authentication links to expire correctly, I had to run mongo, use freecodecamp, then run db.AuthToken.createIndex( { "created": 1 }, { expireAfterSeconds: 900 } ). After that, the links would be removed and would no longer work after 15 minutes, give or take a minute -- the background task that removes documents runs every 60 seconds, apparently.

One thing I noticed is that the authentication token is not always destroyed when the user clicks on the link and logs in. I'm not sure what is causing the issue, but it seems to be destroyed about half the time. So a user could click on an email link, log in, sign out, then log in multiple times until the token is destroyed or ~15 minutes passes.

Another potential issue I saw is that the db.AccessToken collection seems to grow every time a user logs in with a new authentication token. Not sure if that's supposed to happen, but I thought it worth mentioning. Also, the ttl field in the documents in db.AccessToken reads NaN. Again, not sure, but maybe it's supposed to be set by this line?:

User.settings.ttl = 900 * 24 * 60 * 60 * 1000;

@raisedadead
Copy link
Member

raisedadead commented May 4, 2018

@scissorsneedfoodtoo thanks for the excellent summary of what is happening. And Yes, you are correct that the indexing is not done automatically.

Well sadly that is because we originally thought LoopBack does that for us. Turns out it doesn't. In short their docs aren't as good as they claim to be (so much for being an enterprise grade SDK!!)

Anyways, what we have thought off is completely moving away from our custom auth solution. I know its sad, for the work it took to implement in the first place. But its worth every effort in handling it separate from our core.

This is further to our goals of breaking the huge monolith, and making life easier for everyone.

@scissorsneedfoodtoo
Copy link
Contributor

@raisedadead, that's alright! True that it's a shame to move away from the custom authentication after all the work that was put into it, but it will give you and the other core team members more time to better the platform in other ways.

Looking forward to the future Auth0 implementation!

@QuincyLarson
Copy link
Contributor

We've implemented Auth0 and heeded the advice "never roll your own authentication." Thanks for all the help QA'ing the old passwordless authentication, and to @raisedadead for building our new Auth0 implementation.

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

No branches or pull requests

4 participants