[RFC] two-factor authentication

bytehead commented 6 years ago

This is a working draft to add a two-factor authentication mechanism 2FA with TOTP for backend users. It uses scheb/two-factor-bundle to extend the symfony security authentication mechanism, spomky-labs/otphp for generating and validating one-time passwords and bacon/bacon-qr-code to render QR codes as SVGs.

Please review and mention all missed topics.

Changes to security.yml:

security:
    firewalls:
        contao_backend:
            two_factor:
                auth_form_path: contao_backend_two_factor
                check_path: contao_backend_two_factor_check
                auth_code_parameter_name: verify

    access_control:
        - { path: ^/contao/two-factor, roles: [IS_AUTHENTICATED_2FA_IN_PROGRESS, ROLE_USER] }

Changes to AppKernel.php:

/**
     * {@inheritdoc}
     */
    public function registerBundles()
    {
        $bundles = [
            // ...
            new Scheb\TwoFactorBundle\SchebTwoFactorBundle(),
        ];
        // ...
    }

How to enforce 2FA for backend users via config.yml:

contao:
    authentication:
        two_factor:
            enforce_backend: true    #default: false

ToDos:

[x] Move QR code image to user profile
[x] Unit tests
[x] Option to enforce 2FA for all backend users
[x] Icon in the backend list view of tl_user indicating which users have 2FA enabled

Possible new features (future PRs):

2FA also for frontend users

richardhj commented 6 years ago

🙌

I'm really looking forward to see this feature.

You aren't using the Google Authenticator already part of scheb/two-factor-bundle because it is not that generic than the implementation of spomky-labs/otphp?
We might want an icon in the backend list view of tl_user indicating which users have 2FA enabled.
We might want to force users to use 2FA.
2FA must only get activated after the user successfully confirmed the setup with a token generated on his 2FA device.

And currently im getting the Unrecognized option "two_factor" under "security.firewalls.contao_backend" error. Do I need to alter the configuration in the DI extension?

bytehead commented 6 years ago

You aren't using the Google Authenticator already part of scheb/two-factor-bundle because it is not that generic than the implementation of spomky-labs/otphp?

Yes I don't use it. It didn't work with all of my apps I've tried upfront. That's why I decided to choose a library that supports actually the RFCs. Even easier, if we decide to support HOTP in future.

We might want an icon in the backend list view of tl_user indicating which users have 2FA enabled.

I like the idea, @contao/developers what do you think about?

We might want to force users to use 2FA.

IMHO the users should enable 2FA by themselves.

2FA must only get activated after the user successfully confirmed the setup with a token generated on his 2FA device.

I will implement this, already discussed 👍

And currently im getting the Unrecognized option "two_factor" under "security.firewalls.contao_backend" error. Do I need to alter the configuration in the DI extension?

Did you miss to enable the SchebTwoFactorBundle?

leofeyer commented 6 years ago

We might want an icon in the backend list view of tl_user indicating which users have 2FA enabled.

I like the idea, @contao/developers what do you think about?

Go for it.

We might want to force users to use 2FA.

IMHO the users should enable 2FA by themselves.

I also think we should not force them.

fritzmg commented 6 years ago

We might want to force users to use 2FA.

IMHO the users should enable 2FA by themselves.

I also think we should not force them.

I think @richardhj meant that you should optionally be able to enforce 2FA for one Contao Installation for everyone. Not that it should be enforced in general. This makes sense imho.

richardhj commented 6 years ago

I think @richardhj meant that you should optionally be able to enforce 2FA for one Contao Installation for everyone. Not that it should be enforced in general. This makes sense imho.

Optionally for some users or user groups, in order to comply with some TOMs :-) Fore sure it is not necessary.

bytehead commented 6 years ago

I think @richardhj meant that you should optionally be able to enforce 2FA for one Contao Installation for everyone. Not that it should be enforced in general. This makes sense imho.

I don't get this one.

If somebody else enforces 2FA in general the question will be how the users will initially setup the app in this case?

aschempp commented 6 years ago

We might want an icon in the backend list view of tl_user indicating which users have 2FA enabled.

I like the idea, @contao/developers what do you think about?

I like it as well. Maybe a badge on the user/admin icon like the page publishing options?

If somebody else enforces 2FA in general the question will be how the users will initially setup the app in this case?

Something like forcing the setup after login attempt?

leofeyer commented 6 years ago

Please don't do this. None of the apps I am using 2FA with does it – and I am using a lot of them.

patrickjDE commented 6 years ago

As an option to enforce 2FA upon users would be, well.. optional, I don't see a problem there. It'd work analogous to "Password change required". Upon first/next login the user is required to change his password/setup 2FA, whatever the admin's reasons for that decision might be.

If 2FA becomes a core feature, so should the option to enforce its usage upon all or specific users.

fritzmg commented 6 years ago

I agree with @patrickjDE . A company should ideally be able to decide whether to force all editors of their website two use 2FA in order to increase security.

@leofeyer you are probably talking about a front end context there, right? But this is about the back end functionality. If your website has 2 administrators for the back end and only one uses 2FA, the other account is still (maybe) more vulnerable. Hence you might want to decide that every back end user (or specific back end user groups - or just administrators) should use 2FA.

bytehead commented 6 years ago

We might want an icon in the backend list view of tl_user indicating which users have 2FA enabled.

@frontendschlampe do you want to create an icon for it? 😎

frontendschlampe commented 6 years ago

@frontendschlampe do you want to create an icon for it?

yes ... :-) Today is icon day :-D

frontendschlampe commented 6 years ago

What do you think?

2fa

richardhj commented 6 years ago

Love it. Should be on the left side since it is no action button though.

bytehead commented 6 years ago

@frontendschlampe Yeah, I like! I'll place it on the left as @richardhj already stated.

bytehead commented 6 years ago

Here is the implemented version:

bildschirmfoto 2018-06-20 um 07 33 06

frontendschlampe commented 6 years ago

yeah ... no problem. I‘ve added the icon to my PR yesterday with the other icons. :-)

frontendschlampe commented 6 years ago

I check the screenshot again and I will make the icon in the same size like the user icon. Now it's a little bit to big. I will update my PR.

bytehead commented 6 years ago

Here is my new screenshot with the smaller icons:

bildschirmfoto 2018-06-20 um 11 32 59

bytehead commented 6 years ago

So I'll wait with the finalization until #1575 is merged.

leofeyer commented 6 years ago

I would actually prefer a lock overlay instead of a separate icon. 🙈

frontendschlampe commented 6 years ago

I would actually prefer a lock overlay instead of a separate icon.

What do you mean with "lock overlay"? The user icon with lock - like secure pages?

leofeyer commented 6 years ago

Yes.

frontendschlampe commented 6 years ago

2fa_2

bytehead commented 6 years ago

Nice, I like it more this way!

frontendschlampe commented 6 years ago

@leofeyer how should I name the files?

user_2fa.svg
user_2fa_.svg
admin_2fa.svg
admin_2fa_.svg

aschempp commented 6 years ago

I like the overlay much better as well! Have you tried a "2FA" icon overlay? Maybe it would be too small to read though…

bytehead commented 6 years ago

You would not see "2FA", even on its own (bigger) icon it's rarely readable.

frontendschlampe commented 6 years ago

Maybe it would be too small to read though…

yes ... I tried ... it was to small ... with the big lock it was hard to read and with the small one ;-)

leofeyer commented 6 years ago

@leofeyer how should I name the files?

@frontendschlampe Exactly like you did. 😄

frontendschlampe commented 6 years ago

ok ... update in progress :-)

frontendschlampe commented 6 years ago

I added the icons in my PR https://github.com/contao/core-bundle/pull/1575 - I've changed the position of the lock because of the info circle in inactive status of user/admin icon. Ok for you?

bytehead commented 6 years ago

Updated preview:

bildschirmfoto 2018-06-22 um 10 00 27

leofeyer commented 6 years ago

Icons have been merged in f0ff185eada20b9bf259420c865d4550e2be6b78.

bytehead commented 6 years ago

Icons integrated in 45ae9ef2b3aab4ca419555ac5639f6d8cc9782db.

leofeyer commented 6 years ago

Once you have set up 2FA, the QR code remains visible in the back end profile. None of the 2FA apps I am using (including GitHub) does this though.

I think we should not show the QR code once 2FA has been activated.

bytehead commented 6 years ago

I'm fine with that. I'll update the PR accordingly.

leofeyer commented 6 years ago

If I open the contao/2fa route manually, I should be redirected to contao/login. Currently get an Access Denied exception.

That is because you have logged in already and your role is IS_AUTHENTICATED_2FA_IN_PROGRESS. If you log out and try again, you will be redirected. 😄

aschempp commented 6 years ago

If I open the contao/2fa route manually, I should be redirected to contao/login. Currently get an Access Denied exception.

That is because you have logged in already and your role is IS_AUTHENTICATED_2FA_IN_PROGRESS. If you log out and try again, you will be redirected. 😄

I know that. But why am I redirected when accessing the Contao backend then?

bytehead commented 6 years ago

If I open the contao/login page manually while in 2fa process, the login screen appears but the debug bar say's I'm logged it (using TwoFactorToken). I should be redirected to 2fa

Fixed in 4e36d0003.

bytehead commented 6 years ago

Make DCA/DB fields consistent. tl_user.2faEnabled, tl_user.2faCode, tl_user.2faSecret, tl_user.2faConfirmed. (Not sure field starting with number are allowed, but you get the point)

I've renamed it according to the bundle (fields starting with numbers has some issues) in 59ec21f76.

bytehead commented 6 years ago

If I disable 2FA and enable it again, do I understand correctly that there is no new secret generated? I think it should?

I'm not sure about this? What do @contao/developers think?

bytehead commented 6 years ago

If I open the contao/2fa route manually, I should be redirected to contao/login. Currently get an Access Denied exception.

That is because you have logged in already and your role is IS_AUTHENTICATED_2FA_IN_PROGRESS. If you log out and try again, you will be redirected. 😄

I know that. But why am I redirected when accessing the Contao backend then?

We have to adjust the access_control:

access_control:
        - { path: ^/contao/two-factor, roles: [IS_AUTHENTICATED_2FA_IN_PROGRESS, ROLE_USER] }

bytehead commented 6 years ago

If 2fa is enforced, I am required to setup 2fa on the first login (🎉). However, the checkbox in my user/profile stays disabled.

Fixed in bd26db610.

bytehead commented 6 years ago

Only show the 2fa checkbox on the profile page, but not in the regular palettes. Or show them for all users, so I can potentially help someone create an access code for their backend. And I could disable 2FA for users that lost their authentication.

You can do exactly that if you're an admin (only disable, not enable, enable only globally via configuration).

aschempp commented 6 years ago

With the change in https://github.com/contao/core-bundle/commit/4e36d0003a4f90dbd64adb8837d1c283328be970, so we actually still need a different route for the 2FA or can we just apply 2FA on the login route?

bytehead commented 6 years ago

@aschempp see here: https://github.com/contao/core-bundle/pull/1545#discussion_r201745468

leofeyer commented 6 years ago

Thank you big time @bytehead!

BoozieBadd commented 2 years ago

https://github.com/contao/core-bundle/pull/1545#

contao / core-bundle

[RFC] two-factor authentication #1545

ToDos:

Possible new features (future PRs):