diff --git a/src/Entity/UserSystem/User.php b/src/Entity/UserSystem/User.php index 9f60e15f..e933b583 100644 --- a/src/Entity/UserSystem/User.php +++ b/src/Entity/UserSystem/User.php @@ -205,7 +205,7 @@ class User extends AttachmentContainingDBElement implements UserInterface, HasPe protected $trustedDeviceCookieVersion = 0; /** @var Collection - * @ORM\OneToMany(targetEntity="App\Entity\UserSystem\U2FKey", mappedBy="user") + * @ORM\OneToMany(targetEntity="App\Entity\UserSystem\U2FKey", mappedBy="user", cascade={"REMOVE"}, orphanRemoval=true) */ protected $u2fKeys; diff --git a/src/EventSubscriber/PasswordChangeNeededSubscriber.php b/src/EventSubscriber/PasswordChangeNeededSubscriber.php new file mode 100644 index 00000000..1fe8471a --- /dev/null +++ b/src/EventSubscriber/PasswordChangeNeededSubscriber.php @@ -0,0 +1,127 @@ +security = $security; + $this->flashBag = $flashBag; + $this->httpUtils = $httpUtils; + } + + /** + * This function is called when the kernel encounters a request. + * It checks if the user must change its password or add an 2FA mehtod and redirect it to the user settings page, + * if needed. + * @param RequestEvent $event + */ + public function redirectToSettingsIfNeeded(RequestEvent $event) : void + { + $user = $this->security->getUser(); + $request = $event->getRequest(); + + if(!$event->isMasterRequest()) { + return; + } + if(!$user instanceof User) { + return; + } + + //Abort if we dont need to redirect the user. + if (!$user->isNeedPwChange() && !static::TFARedirectNeeded($user)) { + return; + } + + //Check for a whitelisted URL + foreach (static::ALLOWED_ROUTES as $route) { + //Dont do anything if we encounter an allowed route + if ($this->httpUtils->checkRequestPath($request, $route)) { + return; + } + } + + /* Dont redirect tree endpoints, as this would cause trouble and creates multiple flash + warnigs for one page reload */ + if(strpos($request->getUri(), '/tree/') !== false) { + return; + } + + //Show appropriate message to user about the reason he was redirected + if($user->isNeedPwChange()) { + $this->flashBag->add('warning', 'user.pw_change_needed.flash'); + } + + if(static::TFARedirectNeeded($user)) { + $this->flashBag->add('warning', 'user.2fa_needed.flash'); + } + + $event->setResponse($this->httpUtils->createRedirectResponse($request, static::REDIRECT_TARGET)); + + } + + /** + * Check if a redirect because of a missing 2FA method is needed. + * That is the case if the group of the user enforces 2FA, but the user has neither Google Authenticator nor an + * U2F key setup. + * @param User $user The user for which should be checked if it needs to be redirected. + * @return bool True if the user needs to be redirected. + */ + public static function TFARedirectNeeded(User $user) : bool + { + $tfa_enabled = $user->isU2FAuthEnabled() || $user->isGoogleAuthenticatorEnabled(); + + if ($user->getGroup() !== null && $user->getGroup()->isEnforce2FA() && !$tfa_enabled) { + return true; + } + + return false; + } + + /** + * @inheritDoc + */ + public static function getSubscribedEvents() + { + return [ + KernelEvents::REQUEST => 'redirectToSettingsIfNeeded', + ]; + } +} \ No newline at end of file diff --git a/src/Form/AdminPages/GroupAdminForm.php b/src/Form/AdminPages/GroupAdminForm.php index 095c2383..a3b070e8 100644 --- a/src/Form/AdminPages/GroupAdminForm.php +++ b/src/Form/AdminPages/GroupAdminForm.php @@ -23,12 +23,22 @@ namespace App\Form\AdminPages; use App\Entity\Base\NamedDBElement; use App\Form\Permissions\PermissionsType; +use Symfony\Component\Form\Extension\Core\Type\CheckboxType; use Symfony\Component\Form\FormBuilderInterface; class GroupAdminForm extends BaseEntityAdminForm { protected function additionalFormElements(FormBuilderInterface $builder, array $options, NamedDBElement $entity) { + $is_new = null === $entity->getID(); + + $builder->add('enforce2FA', CheckboxType::class, ['required' => false, + 'label' => 'group.edit.enforce_2fa', + 'help' => 'entity.edit.enforce_2fa.help', + 'label_attr' => ['class' => 'checkbox-custom'], + 'disabled' => !$this->security->isGranted($is_new ? 'create' : 'edit', $entity) + ]); + $builder->add('permissions', PermissionsType::class, [ 'mapped' => false, 'data' => $builder->getData(), diff --git a/templates/AdminPages/GroupAdmin.html.twig b/templates/AdminPages/GroupAdmin.html.twig index 7b06632a..305ae156 100644 --- a/templates/AdminPages/GroupAdmin.html.twig +++ b/templates/AdminPages/GroupAdmin.html.twig @@ -12,6 +12,10 @@ {% block additional_panes %}
- {{ form_row(form.permissions) }} + {{ form_row(form.permissions) }}
+{% endblock %} + +{% block additional_controls %} + {{ form_row(form.enforce2FA) }} {% endblock %} \ No newline at end of file diff --git a/tests/EventSubscriber/PasswordChangeNeededSubscriberTest.php b/tests/EventSubscriber/PasswordChangeNeededSubscriberTest.php new file mode 100644 index 00000000..caccca4d --- /dev/null +++ b/tests/EventSubscriber/PasswordChangeNeededSubscriberTest.php @@ -0,0 +1,41 @@ +setGroup(null); + $this->assertFalse(PasswordChangeNeededSubscriber::TFARedirectNeeded($user)); + + //When the group does not enforce the redirect the user must not be redirected + $user->setGroup($group); + $this->assertFalse(PasswordChangeNeededSubscriber::TFARedirectNeeded($user)); + + //The user must be redirected if the group enforces 2FA and it does not have a method + $group->setEnforce2FA(true); + $this->assertTrue(PasswordChangeNeededSubscriber::TFARedirectNeeded($user)); + + //User must not be redirect if google authenticator is setup + $user->setGoogleAuthenticatorSecret('abcd'); + $this->assertFalse(PasswordChangeNeededSubscriber::TFARedirectNeeded($user)); + + //User must not be redirect if 2FA is setup + $user->setGoogleAuthenticatorSecret(null); + $user->addU2FKey(new U2FKey()); + $this->assertFalse(PasswordChangeNeededSubscriber::TFARedirectNeeded($user)); + + } +}