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

Make 2FA persistent by using the database #86

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

duckarcher
Copy link

Problem

There is a limitation in the current state of the library in which the 2FA stays alive whilst the current session is alive. That means that remember_me functionalities and cookie-based approaches do not have the choice to provide long-term 2FA authentication and the OTP will be asked as soon as the session is expired.

Fix

By using a database column, we can give the choice of a persistent 2FA without being dependent on the session's lifespan. The column is a timestamp and it will be populated with the current time as soon as the OTP is provided and there is a remember_me cookie set in the user's browser.

The new twoFactorAuthStillValid will check if the user enabled the remember functionality and will pass the check by comparing the timestamp taken on the login with the current_time plus the lifetime of the config.

Bugs and considerations

  1. Keep in mind that the check in line #229 in Google2FA.php just checks for the default remember_me cookie of Laravel and there is no validation that the cookie is indeed valid.

  2. This PR doesn't play well with lifetime=0 (as you can see in the twoFactorAuthStillValid method) and it would need a hacky way to make it for eternity by passing a large amount of time (like 100 years or something). I will leave this up to you.

Final Notes

These changes will allow to the lifetime of the 2FA to be greater than the session's lifetime.
This way, users can stay "2FA authenticated" for a greater amount of time and not be limited by the session's lifespan.

@duckarcher
Copy link
Author

On second thought, this will allow other devices to bypass 2FA (because they will pass the database check if the remember option is enabled).
Another option would be to use a Cookie instead of the database to achieve the same result.

On this note, you can reject the PR. I will just leave this up here in order to focus on the problem which is really something that should be dealt with. Cheers!

@duckarcher
Copy link
Author

FYI in case you are interested and I am busy later:
You can make it cross device by generating a token to be kept in database and in a cookie. Then compare value+timestamps and add check to middleware.
Wish you add that support at some point. Cheers!

@yvomenezes
Copy link
Contributor

I'm currently having this same problem. The user is been asked to re-enter the 2fa code even though the Laravel remeber me option is activated.

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

Successfully merging this pull request may close these issues.

2 participants