-
Notifications
You must be signed in to change notification settings - Fork 992
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
2FA: "trust this device for 30 days" option #5867
Comments
Thanks for writing this up @brainwane! 💯 🎉 |
@brainwane This is a great idea, I don't see any issue from a security point of view, many companies do it (including Amazon), so the major security issues will really be on the server-side, on how we handle sessions in Warehouse, but that's it! |
Some additional discussion is in #8033 |
There are a few ways to ensure persistence of session information, and they mostly depend on whether we care about the longevity of these sessions enough that we need to store them outside Redis. If we don't care too much about the longevity of these sessions, all we need to do is extend the lifetime of the session cookie if the user logs in and selects the "Keep me signed in" checkbox (which can just be added to the login.html template). Currently session information is stored in Redis for max 12 hour sessions (see warehouse/sessions.py). If we want to ensure data persistence no matter what, it seems we'd have to use Postgres, since I don't think we can (or even want to) configure Redis to have data persistence. One way would be to just store relevant session information in a Postgres database in addition to Redis, and if Redis doesn't have the session information (whether Redis lost the data or if the session actually expired), we can double check in Postgres. However, this seems redundant and I imagine there's a better way to correlate session information with Redis with information in Postgres. @ewdurbin wondering if you have an opinion on how to best go about this? |
I think I'd like to work on this. Here's the approach I would take. Feedback is welcome. I would say that it would not be worth doing device fingerprinting. If an attacker can steal files off your machine, they can likely simply submit malicious requests from your device. Fingerprinting would protect against an attacker accessing a backup of your machine, and it would also prevent an attacker from stealing the session cookie and selling it to another party. It appears that GitHub does not fingerprint a device in order to allow a session cookie. I took the cookies from a Linux Chrome browser, entered them into a Windows Edge browser on a different physical machine, and GitHub logged me in. It's possible that GitHub is looking at the IP address to determine whether to honor the session cookie, but that seems unlikely since IP addresses can change for many reasons. Maybe down the road, it would be worth adding device fingerprinting to PyPI, but I don't think the security benefit outweighs the effort at this time. In terms of implementation details, I would store saved devices in Postgres. When the user enters the 2nd authentication factor and selects "Remember this device for 30 days", a row will be entered in a new table (I'll call it When a user logs in, after they enter their password, if the request has a device id cookie, the A user would still have to login every 12 hours (or if they explicitly logged out), but they would only have to enter their password, and not the 2nd factor. We could create a scheduled task to remove expired rows from A potential future enhancement could be to also store the browser's user agent string and IP address and display in the web UI the current saved devices. The user would have a button to revoke devices. |
What's the problem this feature will solve?
The multi-factor auth makes logging in a little less convenient, and the fact of a previous login to a user's account from the same device makes it more likely that the user auth is legit.
Describe the solution you'd like
Much as other 2FA implementations do, we could add a "trust this device for 30 days" (or some similar period) checkbox during the 2FA check (during login).
Additional context
Hat-tip to @ericholscher for asking for this ("don't bother me on this computer for 30 days") during the PyCon NA 2019 sprints.
The text was updated successfully, but these errors were encountered: