2fa login by bjorvack · Pull Request #3580 · forkcms/forkcms

bjorvack · 2023-11-24T09:23:19Z

Type

Enhancement
Feature

Resolves the following issues

Adds 2FA login

src/Core/Domain/Form/TogglePasswordInputType.php

carakas · 2023-11-24T22:54:41Z

src/Modules/Internationalisation/Domain/Translator/ForkTranslator.php

            return match ($mainRequest->get('_route')) {
                'backend_action',
                'backend_login' => ActionSlug::fromRequest($mainRequest)->getTranslationDomain(),
+                'backend_2fa_login' => ActionSlug::fromRequest($mainRequest)->getTranslationDomain(),


please use the fallback, so just add them to the comma list with backend_action and backend_login

Could you clarify what you mean

The code is exactly the same as for backend_action and backend_login, so you can just include them in that comma separated list

fixed 1f2271c

src/Modules/Backend/templates/Backend/Actions/UserEdit.html.twig

src/Modules/Backend/Domain/User/Command/DisableTwoFactorAuthentication.php

src/Modules/Backend/Domain/User/Command/DisableTwoFactorAuthenticationHandler.php

src/Modules/Backend/Domain/User/Command/ChangeUserHandler.php

src/Modules/Backend/Backend/Actions/ModuleSettings.php

src/Modules/Backend/Backend/Ajax/AjaxActionGetTwoFactorAuthorizationCode.php

carakas · 2023-11-24T23:21:52Z

src/Modules/Backend/Controller/TwoFactorAuthenticationController.php

+        return parent::form($request);
+    }
+
+    protected function getTemplateVars(Request $request, TwoFactorTokenInterface $token): array


Note to self: update the code here in my own branch with changes (I've made this more generic and easy to add extra controllers)

@bjorvack this code has already landed, you only need to set the things specific for your controller all those others should already be there

fixed d3e8501

src/Modules/Backend/Domain/ModuleSettings/ModuleSettingsType.php

carakas · 2023-11-24T23:32:54Z

You'll also need to fix the tests and add tests for 2FA

carakas

The tests are still failing
A little bit of extra feedback
Can you rebase your code?

carakas · 2023-11-30T00:59:53Z

src/Modules/Extensions/Domain/Module/Command/ChangeModuleSettingsHandler.php

    {
        $this->moduleRepository->save($changeModuleSettings->module);
-        $this->moduleSettings->invalidateCache();
+        $this->commandBus->dispatch(new ClearContainerCache());


can you use $this->eventDispatcher->dispatch(new ClearCacheEvent()); instead?

fixed 7ef40cf

carakas · 2023-11-30T01:01:10Z

src/Modules/Internationalisation/Domain/Translator/ForkTranslator.php

            return match ($mainRequest->get('_route')) {
                'backend_action',
                'backend_login' => ActionSlug::fromRequest($mainRequest)->getTranslationDomain(),
+                'backend_2fa_login' => ActionSlug::fromRequest($mainRequest)->getTranslationDomain(),


The code is exactly the same as for backend_action and backend_login, so you can just include them in that comma separated list

carakas · 2023-11-30T01:03:39Z

src/Modules/Backend/Backend/Actions/ModuleSettings.php

+            validCallback: function (FormInterface $form): Response {
+                $this->commandBus->dispatch($form->getData());
+
+                if (!$this->moduleSettings->get(ModuleName::fromString('Backend'), '2fa_enabled', false)) {


Doesn't this also belong in the command? Otherwise if the command is used somewhere else this code won't be executed

fixed cd1ac62

carakas · 2023-11-30T01:06:38Z

src/Modules/Backend/Controller/TwoFactorAuthenticationController.php

+        );
+    }
+
+    #[Route('/2fa', name: 'backend_2fa_login')]


shouldn't this route be /private/2fa?

fixed 8ad6b65

carakas · 2023-11-30T01:07:02Z

src/Modules/Backend/Controller/TwoFactorAuthenticationController.php

+        return parent::form($request);
+    }
+
+    protected function getTemplateVars(Request $request, TwoFactorTokenInterface $token): array


@bjorvack this code has already landed, you only need to set the things specific for your controller all those others should already be there

bjorvack requested a review from a team as a code owner November 24, 2023 09:23

bjorvack requested review from carakas and tijsverkoyen November 24, 2023 09:24

carakas requested changes Nov 24, 2023

View reviewed changes

Bjorn Van Acker added 25 commits November 29, 2023 08:28

Initial setup for 2fa

300641b

Add trusted devices to 2fa login

ef93932

Load settigns from database

476ee9f

Clear cache after changes in 2fa

3cd6bb2

Add backupcodes

98d9726

Show backup codes on 2fa enable

01833b9

Set server name and issuer

15bbe9d

Redirect a user to his edit page when 2fa is required but not set

1ea2aa5

Fix code styles

f01f72e

Fix phpstan issues

3a96ce8

Use SvgWriter to create QR code

e850b23

Use the /private/2fa routes

82c075a

Add missing empty line

f70cf25

Remove 2fa on create user form

bfa4dc4

Add default value to setBackupCodes for readability

cc5baa3

Fix QR Code generation

1beb9d4

Refactor AbstractActionController

de9fa9b

Configure labels and help text correctly

16916a7

Use translation for alt text

bae2283

Add copy to clipboard

7677490

Add form validation to authorization key

7fb2d64

Use correct redirect url generation

30e1591

Refactor 2fa screen

5b28317

Add toggle to authorization key

79fa194

Remove leading and trailing spaces on copied lines

2180258

Bjorn Van Acker added 9 commits November 29, 2023 08:28

Use different translations and use fetch instead of $.ajax

72ac03a

Fix missed translation

650196e

Don't disable variables-dark

6d25ac0

Fix feedback @carakas

90c5f83

Use handleSettingsForm to handle setting form changes

39b52b0

Fix phpstan issues

b30e646

Fix standard js issues

837bd64

Fic php code sniffer issues

334a50f

Add missing translation

c6310d1

bjorvack force-pushed the 2fa-login branch from e67d5aa to c6310d1 Compare November 29, 2023 07:29

Bjorn Van Acker added 6 commits November 29, 2023 08:31

Use the fork6 version of the toggle password

0508c82

Use snake case

ec97af9

Use public readonly to get user from disable 2 fa handler

d533e75

Remove unused use statement

3ff9363

Clear the cache in the change module settings handler

f0c5e68

Refactor ModuleSettingsType to use the correct form types

f92c386

carakas requested changes Nov 30, 2023

View reviewed changes

Bjorn Van Acker added 12 commits December 22, 2023 13:04

Use fallback translation domain for backend_2fa

1f2271c

Remove unused use statement

26235e6

Merge branch 'fork6' into 2fa-login

d3e8501

Use clear cache event on module settings change

7ef40cf

Move 2fa enabled changes to settings handler

cd1ac62

use /private/2fa route

8ad6b65

Autoload EventDispatcherInterface

f9fd125

Fix phpstan issues

bfccf9c

Remove leading slash from use statement

8755369

Fix unit tests

d84fed8

Fix unit tests

cf32237

Test when the 2FA buttons should be visible

8b7240e

bjorvack mentioned this pull request Mar 19, 2024

forkcms 6 phpunit broken for php8.2 #3602

Open

Uh oh!

Conversation

bjorvack commented Nov 24, 2023

Type

Resolves the following issues

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

carakas commented Nov 24, 2023

Uh oh!

carakas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants