# Two Factor local login proposal
**URL:** https://meta.discourse.org/t/two-factor-local-login-proposal/76689
**Category:** Feature
**Created:** [24.Декабрь.2017 07:17:59 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689 "2017-12-24T07:17:59Z")
**Posts on this page:** 20
**Page:** 2
### Author:  [@jomaxro](https://meta.discourse.org/u/jomaxro)
#### Post date: [23.Январь.2018 01:54:39 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/21 "2018-01-23T01:54:39Z")
> [@awole20](#):
>
> Admin panel to turn off other users’ 2-fa:
Is this logged in staff logs? I don’t think user enabling/disabling 2factor themselves, but for admin disabling a user’s 2factor we might want to consider logging it.
Awesome work, by the way. Excited to try this out!
---
### Author:  [@featheredtoast](https://meta.discourse.org/u/featheredtoast)
#### Post date: [23.Январь.2018 02:06:51 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/22 "2018-01-23T02:06:51Z")
> [@jomaxro](#):
>
> Is this logged in staff logs?
Not just yet, though I agree that this should. I’ll add it to my to-do
---
### Author:  [@riking](https://meta.discourse.org/u/riking)
#### Post date: [23.Январь.2018 02:58:07 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/23 "2018-01-23T02:58:07Z")
> [@awole20](#):
>
> Login prompt:
Remove the red, this is not an error condition. It’s a normal part of the login process.
> [@awole20](#):
>
> OK, I’ve added a simple rate limiter, hard coded with a rate of 6/minute/ip for now.
Codes are generated at a rate of 1 every 30 seconds. Add a 50% buffer, and 3 per minute should be just fine.
---
### Author:  [@featheredtoast](https://meta.discourse.org/u/featheredtoast)
#### Post date: [23.Январь.2018 18:57:46 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/24 "2018-01-23T18:57:46Z")
Fair enough - Without a banner color, the text looks suspiciously floaty, so I’ll see what I can do about that.
I’d prefer to be more generous with the rate limit, but I don’t mind either way.
Looks like the build is failing due to formatting issues – I’ll be able to implement those changes and fix up the formatting tonight.
---
### Author:  [@fefrei](https://meta.discourse.org/u/fefrei)
#### Post date: [23.Январь.2018 19:11:00 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/25 "2018-01-23T19:11:00Z")
Are you blocking double login attempts the previously used verification code?
Doing so is a bit of a trade-off because it means that users can only authenticate once every 30 seconds (because they need to wait for a new code), but it prevents an attacker from (immediately) using a token he for some reason saw to open a second session.
My guess is that most websites do that, but I didn’t check. I _do_ know that this check is optional in the `google-authenticator` Linux PAM module that prompts for these codes when connecting via SSH 🙂
---
### Author:  [@featheredtoast](https://meta.discourse.org/u/featheredtoast)
#### Post date: [23.Январь.2018 19:17:15 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/26 "2018-01-23T19:17:15Z")
Not currently - but it should be a relatively simple change if we wanted to turn that on. The ROTP library supports it natively, so I’d just need to additionally store the last time a specific token was used.
---
### Author:  [@featheredtoast](https://meta.discourse.org/u/featheredtoast)
#### Post date: [25.Январь.2018 01:23:36 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/27 "2018-01-25T01:23:36Z")
Staff actions now log ✅
Changed the login label to be an h3, rather than an error banner ✅
Second factor rate limiting dropped down to 3/minute ✅
Fixed formatting issues and failing tests ✅
I haven’t yet gotten around to tackling the token re-use yet – that will be next on the chopping block.
---
### Author:  [@sam](https://meta.discourse.org/u/sam)
#### Post date: [25.Январь.2018 05:45:16 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/28 "2018-01-25T05:45:16Z")
Not a huge fan of having the title “Google Authenticator”, cause I just feel like this is advertising Google on our user page.
Instead perhaps?
Two-factor authentication
[Enable two factor authentication]
what is two factor authentication?
^^^ when you click that it pops a modal:
Basic spiel about 2fa with links to Google authenticator and FreeOTP
* * *
Great work, will be happy to try it out.
---
### Author:  [@featheredtoast](https://meta.discourse.org/u/featheredtoast)
#### Post date: [25.Январь.2018 17:03:40 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/29 "2018-01-25T17:03:40Z")
Thanks Sam - I agree with that assessment. I’ll change it to be less advertise-y.
To do:
- Google authenticator - change verbiage to 2 factor with description text.
From review:
- Add js tests
- Add controller tests
- Missed a few translatable texts, move the remaining
- Refactor and reorganize controllers
- add ember components to de-duplicate
- Fix up code formatting
---
### Author:  [@featheredtoast](https://meta.discourse.org/u/featheredtoast)
#### Post date: [31.Январь.2018 02:04:52 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/30 "2018-01-31T02:04:52Z")
Worked this week at getting up to speed with qunit, and moving forward on most of the review items.
Done:
- Blocking previous codes - valid 2-factor auth tokens can only be authenticated once/30 seconds. ✅
- I played with updating the “last used” any time the token was _attempted_ but that seemed to be overkill, and frustrating as to why a token would fail.
- Translatable texts. ✅
- Move second factor logic to a helper class. ✅
- Move second factor specific controller endpoints to its own controller. ✅
- Move serialization logic for 2-factor details in admin user views. ✅
- Add a login ember component for de-duplication ✅
- Fix up code formatting ✅
- Change verbiage of google authenticator ✅
Todo:
- js tests
- controller tests
- Add modal for explaining what 2fa with links to Google Authenticator/FreeOTP
Questions:
For reviewers, would it be easier to squash my changes in a new commit, or have an incremental “since last review” commit? Regardless, I will finish writing the tests and other todo items, and push it back up soon.
For running qunit tests, is there a way of quickly running a single test? As I’m building out a suite, It would be great to be able to build out expectations without having to re-run all tests each time.
---
### Author:  [@gerhard](https://meta.discourse.org/u/gerhard)
#### Post date: [31.Январь.2018 20:50:25 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/31 "2018-01-31T20:50:25Z")
> [@awole20](#):
>
> For reviewers, would it be easier to squash my changes in a new commit, or have an incremental “since last review” commit?
It doesn’t matter – whatever suits you best. GitHub has an option to show only changes since last review and we can always squash your commits when we merge the PR.
> [@awole20](#):
>
> For running qunit tests, is there a way of quickly running a single test?
> [@Developing Discourse Plugins - Part 6 - Add acceptance tests](https://meta.discourse.org/t/beginner-s-guide-to-creating-discourse-plugins-part-6-acceptance-tests/32619/6):
>
> If you click “rerun” beside a test, you’ll get a URL that is just for that particular test that you can refresh over and over. You can also run by module by clicking in the “All Module” box and checking off just the modules you want to run.
---
### Author:  [@featheredtoast](https://meta.discourse.org/u/featheredtoast)
#### Post date: [03.Февраль.2018 23:19:08 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/32 "2018-02-03T23:19:08Z")
I’ve finished wiring up the tests this week, so I think I’m ready for the next batch of reviews. Travis (specifically yarnpkg) seems to be having some issue today with setting up some of the tests, so having some difficulty getting that sweet green checkmark sadly.
@sam re: naming conventions, I’d ideally be evolving the user\_second\_factors table to hold all second factor information, so you can easily parse all second factors via the type column. For this pass, this is a 1-1 relationship to a user with TOTP only, but as we add more this would become a 1-many, as I was following @riking’s suggestion for table structure above. That would allow for uf2, and back up codes all in a single table.
Done:
- Rebased on latest master ✅
- add controller tests ✅
- second factor controller tests
- change email tests
- change password tests
- admin login tests
- add qunit tests ✅
- password reset
- preferences
- Add modal for explaining what 2fa with links to Google Authenticator, Authy, and FreeOTP ✅
Fixes:
- check for 2factor on change email controller ✅
- email controller - only show second factor errors on attempt ✅
- properly check against ‘true’ to enable second factor. ✅
---
### Author:  [@8BIT](https://meta.discourse.org/u/8BIT)
#### Post date: [12.Февраль.2018 19:45:15 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/33 "2018-02-12T19:45:15Z")
i can’t wait for this! great work!
---
### Author:  [@featheredtoast](https://meta.discourse.org/u/featheredtoast)
#### Post date: [14.Февраль.2018 15:34:41 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/34 "2018-02-14T15:34:41Z")
Hm, looks like I’ve got another route to take care of now that sign in via email link is here. Adding that to the to-do.
> [@Passwordless signin via email link](https://meta.discourse.org/t/passwordless-signin-via-email-link/57569/23):
>
> Thank you @fantasticfears for lying the ground work here! I made some changes as to how error messages are displayed and made the rate limits more aggressive for the new routes. As per @sam’s request, the feature is also off by default and can be enabled via enable\_local\_logins\_via\_email site setting. [https://github.com/discourse/discourse/commit/03b3e57a44da228bca7296bd752e26447956e1d6](https://github.com/discourse/discourse/commit/03b3e57a44da228bca7296bd752e26447956e1d6)
---
### Author:  [@featheredtoast](https://meta.discourse.org/u/featheredtoast)
#### Post date: [15.Февраль.2018 22:08:09 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/35 "2018-02-15T22:08:09Z")
Alright, I’ve rebased on the latest, and implemented extra second factor check when logging in via email - this is back in working order with the latest version of Discourse.
I also check for the second factor param to determine how to rate limit, rather than check via a session/per IP, which slipped my mind before somehow.
I’ve decided to squash just so further conflicts won’t happen on earlier versions before I split out some of the logic to a separate second factor controller.
All in all, it looks all good and complete from here - Hoping the team finds the work acceptable soon! 🙂
---
### Author:  [@sam](https://meta.discourse.org/u/sam)
#### Post date: [15.Февраль.2018 22:28:01 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/36 "2018-02-15T22:28:01Z")
Thanks heaps, the plan is for me, @eviltrout and @tgxworld to review this on Monday, aiming to have it merged in next week.
---
### Author:  [@tgxworld](https://meta.discourse.org/u/tgxworld)
#### Post date: [21.Февраль.2018 07:14:54 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/37 "2018-02-21T07:14:54Z")
Merged in
[https://github.com/discourse/discourse/pull/5612](https://github.com/discourse/discourse/pull/5612)
Many thanks to @awole20 for pushing this through.
---
### Author:  [@8BIT](https://meta.discourse.org/u/8BIT)
#### Post date: [25.Февраль.2018 18:27:09 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/40 "2018-02-25T18:27:09Z")
a much-needed addition and very grateful for it! our community gives you thanks!
---
### Author:  [@terraboss](https://meta.discourse.org/u/terraboss)
#### Post date: [26.Февраль.2018 07:02:26 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/41 "2018-02-26T07:02:26Z")
Nice feature! Thank you guys.
I‘m just missing the possibility to show the QR Code again, to setup another device (iPad) as backup.
---
### Author:  [@jomaxro](https://meta.discourse.org/u/jomaxro)
#### Post date: [26.Февраль.2018 07:15:37 UTC](https://meta.discourse.org/t/two-factor-local-login-proposal/76689/42 "2018-02-26T07:15:37Z")
> [@terraboss](#):
>
> I‘m just missing the possibility to show the QR Code again, to setup another device (iPad) as backup.
This seems unusual. All sites I’ve ever used TOTP on require all devices to be configured at once (Google, Dropbox, Microsoft, Facebook, LastPass, Amazon…), and do not display the QR code ever again. Do you use TOTP on another system that displays the QR code multiple times?
[Previous page](https://meta.discourse.org/t/two-factor-local-login-proposal/76689.md?page=1)
[Next page](https://meta.discourse.org/t/two-factor-local-login-proposal/76689.md?page=3)