New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2fa login #3580
base: fork6
Are you sure you want to change the base?
Conversation
@@ -156,6 +156,8 @@ private function determineDefaultTranslationDomain(): TranslationDomain | |||
return match ($mainRequest->get('_route')) { | |||
'backend_action', | |||
'backend_login' => ActionSlug::fromRequest($mainRequest)->getTranslationDomain(), | |||
'backend_2fa_login' => ActionSlug::fromRequest($mainRequest)->getTranslationDomain(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use the fallback, so just add them to the comma list with backend_action and backend_login
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you clarify what you mean
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code is exactly the same as for backend_action and backend_login, so you can just include them in that comma separated list
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed 1f2271c
src/Modules/Backend/templates/Backend/Actions/UserEdit.html.twig
Outdated
Show resolved
Hide resolved
src/Modules/Backend/Domain/User/Command/DisableTwoFactorAuthentication.php
Outdated
Show resolved
Hide resolved
src/Modules/Backend/Domain/User/Command/DisableTwoFactorAuthenticationHandler.php
Show resolved
Hide resolved
src/Modules/Backend/Backend/Ajax/AjaxActionGetTwoFactorAuthorizationCode.php
Show resolved
Hide resolved
return parent::form($request); | ||
} | ||
|
||
protected function getTemplateVars(Request $request, TwoFactorTokenInterface $token): array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bjorvack this code has already landed, you only need to set the things specific for your controller all those others should already be there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed d3e8501
src/Modules/Backend/Domain/ModuleSettings/ModuleSettingsType.php
Outdated
Show resolved
Hide resolved
You'll also need to fix the tests and add tests for 2FA |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests are still failing
A little bit of extra feedback
Can you rebase your code?
) { | ||
} | ||
|
||
public function __invoke(ChangeModuleSettings $changeModuleSettings): void | ||
{ | ||
$this->moduleRepository->save($changeModuleSettings->module); | ||
$this->moduleSettings->invalidateCache(); | ||
$this->commandBus->dispatch(new ClearContainerCache()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you use $this->eventDispatcher->dispatch(new ClearCacheEvent());
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed 7ef40cf
@@ -156,6 +156,8 @@ private function determineDefaultTranslationDomain(): TranslationDomain | |||
return match ($mainRequest->get('_route')) { | |||
'backend_action', | |||
'backend_login' => ActionSlug::fromRequest($mainRequest)->getTranslationDomain(), | |||
'backend_2fa_login' => ActionSlug::fromRequest($mainRequest)->getTranslationDomain(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code is exactly the same as for backend_action and backend_login, so you can just include them in that comma separated list
validCallback: function (FormInterface $form): Response { | ||
$this->commandBus->dispatch($form->getData()); | ||
|
||
if (!$this->moduleSettings->get(ModuleName::fromString('Backend'), '2fa_enabled', false)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this also belong in the command? Otherwise if the command is used somewhere else this code won't be executed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed cd1ac62
); | ||
} | ||
|
||
#[Route('/2fa', name: 'backend_2fa_login')] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this route be /private/2fa?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed 8ad6b65
return parent::form($request); | ||
} | ||
|
||
protected function getTemplateVars(Request $request, TwoFactorTokenInterface $token): array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bjorvack this code has already landed, you only need to set the things specific for your controller all those others should already be there
Type
Resolves the following issues
Adds 2FA login