Turn off 2FA if the user only has Backup Codes enabled #75
Conversation
There was a problem hiding this comment.
This works in my tests 👍🏻
I don't think it fixes #47, since IMO we still need the UI warning, instead of allowing the backup codes to be generated in the first place.
This is a good server-side protection to enforce that, though, and fixes it retroactively for the folks who've already experienced it.
|
I think I prefer this method, and since we can potentially reduce users' caps should their account fall into what we consider needing it, then it's probably a good solution. Ideally we'd be able to guide users' easier through our preferred setup. |
|
I've merged it with I don't have an environment I can run the tests for this repo in, and the steps to get to that point are far too involved to be worth it, so I've just updated the tests with what I believe should pass. |
If you already have a Core environment locally setup, and the tests working there, then IIRC this is all that's needed:
I could be forgetting something, though. I'm open to changes that make it easier to setup. |
`get_enabled_providers_for_user()` already calls `array_keys()` on the `ENABLED_PROVIDERS_USER_META_KEY` value.
Explicitly setting `PROVIDER_USER_META_KEY` isn't necessary and distorts the test. The same thing effect should be achieved just by calling `enable_provider_for_user()`, and hardcoding it would hide any error in that function. This is really an integration test rather than a unit test.
This is partially related to #47 and might fix it, I'm not sure.
This simply disables 2FA if the user only has Backup Codes enabled, which appears to be happening for users when viewing the TOTP screen, but not setting it up (This is a bug in our UI, partially due to a less-than-ideal upstream flow).
The result from that is that they're locked out, as they never saw the backup codes, and are prompted for them next time they login. There are currently ~5 users affected by this.
This PR should resolve the login for those users, and might resolve #47.