TOTP implementation for 2FA #1031

colinodell · 2019-10-02T13:48:41Z

Implements #1020 using TOTP codes

Overview

A new section has been added to the user area:

Clicking the button link takes you to this new page:

Using a compatible app, the user must scan the QR code and enter in a valid TOTP code to prove they've got it working. Once they submit a valid code 2FA will be active on their account:

The user is also given a single-use backup code - more on that later.

When a user with a 2FA-enabled account logs in, we redirect them to this new page:

They must enter a valid TOTP code from their authenticator in order to continue and be fully logged-in. They can also request that the current browser be remembered for 30 days so they aren't prompted to constantly re-enter TOTP codes from the current computer.

If the user loses access to their authenticator, they can instead use that backup code to gain access. Using this code will cause 2FA to become disabled until they set it up again:

Users can also disable 2FA manually if they wish. From the main 2FA status in their account area, they simply click that giant red button which takes them to a confirmation page:

Clicking the red button will disable 2FA:

Notes

A few notes about this implementation:

This 2FA functionality works with OAuth accounts too!
I've added very basic rate-limiting with Redis - you basically get locked out for 15 minutes after entering 5 incorrect codes
Users are sent an email notification whenever 2FA is enabled or disabled
I added new mobile icon to represent 2FA in the menu
The bundle I'm using does offer the ability to use email-based codes, but I have not implemented that

I'll add some additional notes inline with the code

Happy #Hacktoberfest!

…nts in the future

…enticator

colinodell · 2019-10-02T13:51:44Z

app/config/config.yml

+        enabled: true
+        server_name: '%packagist_host%'
+        issuer: Packagist
+        window: 1


A new code is generated every 30 seconds. A window of 1 will allow three codes to be accepted:

The current code

The code generated 30 seconds before it

The next code that will be generated

This allows some flexibility in case the server clock and mobile clock are slightly out-of-sync, or the user needs a few extra seconds to enter in their code.

colinodell · 2019-10-02T13:52:46Z

src/Packagist/WebBundle/Controller/UserController.php

+            throw new AccessDeniedException('You cannot change this user\'s two-factor authentication settings');
+        }
+
+        if ($backupCode = $this->get('session')->get('backup_code')) {


I briefly considered using a flash message for this, but I wanted special formatting and therefore opted to just stash the code in normal session storage and manually remove it once we read it.

src/Packagist/WebBundle/Controller/UserController.php

colinodell · 2019-10-02T13:54:35Z

src/Packagist/WebBundle/Entity/User.php

+        return $this->backupCode !== null && hash_equals($this->backupCode, $code);
+    }
+
+    public function invalidateBackupCode(string $code): void


The interface assumes the user may have multiple codes, but we're only using a single code here.

colinodell · 2019-10-02T13:55:29Z

src/Packagist/WebBundle/Resources/public/font/config.json

@@ -0,0 +1,178 @@
+{


I had to manually look up each icon that was used to create the custom font, which took a while, so I figured why not commit this so it's easier to make future changes?

src/Packagist/WebBundle/Security/TwoFactorAuthRateLimiter.php

src/Packagist/WebBundle/Resources/views/user/enable_two_factor_auth.html.twig

src/Packagist/WebBundle/Controller/UserController.php

Fixes #1031 (comment)

Fixes: - #1031 (comment) - #1031 (comment)

colinodell · 2019-10-09T00:23:58Z

Thanks for the review, @stof! I have made the suggested edits. For the sake of tracking them, I have not squashed these into my earlier commits yet but can certainly do so to clean up the history before merging.

afk11

concept & code review ACK - PR looks great, and the two-factor-bundle & otphp deps are tested and actively maintained

stof

Looks good to me

colinodell · 2019-10-10T12:13:01Z

I have added commit 00ca834 which exposes the secret key to the user during setup. This will allow people without QR code scanners to set up 2FA:

Seldaek · 2019-10-10T12:23:25Z

Just a quick flow review based on the screenshots (thanks for that, really helpful), it looks great overall, I'd just think having a proper intermediate page to show the confirmation that it's been enabled and show the backup code there prominently would be good. Having it only in the flash message confirmation can be very easily glanced over IMO.

colinodell · 2019-10-10T12:27:38Z

Great idea! I'll work on implementing that over the next few days.

Implements #1031 (comment) Also includes a new checkbox icon used for the confirmation button

Additional testing revealed an issue with Github-based accounts. When such an account was logged in for a while, they eventually lost their 'fully-authenticated' status and would be forced to re-connect with Github, which was an unexpected behavior. Furthermore, even if you went through that process, you wouldn't become fully-authenticated afterwards. Instead of spending time to investigate this behavior, I've decided to revert to the convention used by the 'change password' functionality, which doesn't require this additional permission.

colinodell · 2019-10-10T22:35:13Z

I found time this evening to implement it 😃

When the user is setting up 2FA and they submit the confirmation code from their authenticator, they will be taken to this new intermediate confirmation screen:

2FA is fully enabled at this point (as per the green success message). The green button is simply a link to the main page showing 2FA status, which again confirms that 2FA is enabled:

(You don't actually have to click the big green button for 2FA to become enabled)

Hopefully this makes it more obvious that the user should save their backup code in a safe place.

colinodell · 2019-10-10T22:40:06Z

I don't like how that giant "disable two-factor auth" button detracts from the green success message, so I've turned that into a link instead:

(That link still takes the user to the confirmation page which does still have a giant red button on it.)

Seldaek · 2019-10-25T06:57:36Z

Thanks, looking all good! It's live.

Slamdunk · 2019-10-25T08:31:21Z

Awesome 👍

Minor GUI flaw: in the login the checkbox for "Trust this computer" is unaligned and overlaps the input field

Agent: Firefox 70 on Xubuntu 19.10

colinodell added 9 commits September 29, 2019 09:42

Commit the fontello configuration so we can modify/regenerate icon fo…

da3eede

…nts in the future

Add mobile icon

49e58ab

Add TOTP-based two-factor authentication

3e7ef23

Allow admins to disable 2fa

083d9e0

Generate a one-time backup code in case the user can't use their auth…

fd7d699

…enticator

Implement rate limiting to prevent brute-force attempts

45cf57a

Enable trust option so user isn't prompted for 2FA within 30 days

e317e8f

Allow 2FA to work with OAuth-based logins

56d0e11

Require user to be fully authenticated before changing 2FA settings

607f663

colinodell commented Oct 2, 2019

View reviewed changes

stof reviewed Oct 8, 2019

View reviewed changes

colinodell added 3 commits October 8, 2019 20:13

Only show backup codes when looking at the page for yourself

0b42720

Fixes #1031 (comment)

Remove stray filter

a67d311

Fixes #1031 (comment)

Minor changes to the 2FA template headers

df0d4f5

Fixes: - #1031 (comment) - #1031 (comment)

colinodell mentioned this pull request Oct 9, 2019

Template tweaks #1033

Merged

afk11 approved these changes Oct 9, 2019

View reviewed changes

stof approved these changes Oct 10, 2019

View reviewed changes

Show the secret key for users using PC-based authenticators

00ca834

colinodell mentioned this pull request Oct 10, 2019

[RFC] Two-Factor Authentication #1020

Closed

colinodell added 2 commits October 10, 2019 18:22

Show backup code on intermediate confirmation page

1ab1f28

Implements #1031 (comment) Also includes a new checkbox icon used for the confirmation button

Tone down the link for disabling 2FA

19eb8c1

Seldaek merged commit 19eb8c1 into composer:master Oct 25, 2019

Seldaek mentioned this pull request Oct 25, 2019

Spelling is hard #1035

Merged

colinodell deleted the feature/2fa branch October 25, 2019 12:00

colinodell mentioned this pull request Oct 25, 2019

Add missing form-group wrapper #1036

Merged

localheinz mentioned this pull request Oct 26, 2019

Update packagist 2FA support 2factorauth/twofactorauth#4194

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TOTP implementation for 2FA #1031

TOTP implementation for 2FA #1031

colinodell commented Oct 2, 2019

colinodell Oct 2, 2019

colinodell Oct 2, 2019

colinodell Oct 2, 2019

colinodell Oct 2, 2019

colinodell commented Oct 9, 2019

afk11 left a comment

stof left a comment

colinodell commented Oct 10, 2019

Seldaek commented Oct 10, 2019

colinodell commented Oct 10, 2019

colinodell commented Oct 10, 2019

colinodell commented Oct 10, 2019

Seldaek commented Oct 25, 2019

Slamdunk commented Oct 25, 2019

TOTP implementation for 2FA #1031

TOTP implementation for 2FA #1031

Conversation

colinodell commented Oct 2, 2019

Overview

Notes

colinodell Oct 2, 2019

Choose a reason for hiding this comment

colinodell Oct 2, 2019

Choose a reason for hiding this comment

colinodell Oct 2, 2019

Choose a reason for hiding this comment

colinodell Oct 2, 2019

Choose a reason for hiding this comment

colinodell commented Oct 9, 2019

afk11 left a comment

Choose a reason for hiding this comment

stof left a comment

Choose a reason for hiding this comment

colinodell commented Oct 10, 2019

Seldaek commented Oct 10, 2019

colinodell commented Oct 10, 2019

colinodell commented Oct 10, 2019

colinodell commented Oct 10, 2019

Seldaek commented Oct 25, 2019

Slamdunk commented Oct 25, 2019