Skip to content
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

Add backup code functionality for 2FA #719

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 33 additions & 19 deletions core-bundle/src/Controller/FrontendModule/TwoFactorController.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@

namespace Contao\CoreBundle\Controller\FrontendModule;

use Contao\CoreBundle\Exception\InsufficientAuthenticationException;
use Contao\CoreBundle\Framework\ContaoFramework;
use Contao\CoreBundle\Routing\ScopeMatcher;
use Contao\CoreBundle\Security\TwoFactor\Authenticator;
use Contao\CoreBundle\Security\TwoFactor\BackupCode\BackupCodeManager;
use Contao\FrontendUser;
use Contao\ModuleModel;
use Contao\PageModel;
Expand All @@ -24,8 +26,7 @@
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Security;
use Symfony\Component\Security\Http\Authentication\AuthenticationUtils;
use Symfony\Contracts\Translation\TranslatorInterface;

Expand All @@ -41,6 +42,10 @@ class TwoFactorController extends AbstractFrontendModuleController

public function __invoke(Request $request, ModuleModel $model, string $section, array $classes = null, PageModel $page = null): Response
{
if (!$this->get('security.helper')->isGranted('IS_AUTHENTICATED_FULLY')) {
throw new InsufficientAuthenticationException('User is not fully authenticated');
}
Comment on lines +45 to +47
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this mean the module generates an exception if you open the page with a REMEMBERME cookie?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately yes. And since we have no "re-enter your password" screen yet, the user will be redirected to the login module, which will happily tell them that they are already logged in. 😢


$this->page = $page;

if (
Expand All @@ -64,21 +69,16 @@ public static function getSubscribedServices(): array
$services['contao.routing.scope_matcher'] = ScopeMatcher::class;
$services['contao.security.two_factor.authenticator'] = Authenticator::class;
$services['security.authentication_utils'] = AuthenticationUtils::class;
$services['security.token_storage'] = TokenStorageInterface::class;
$services['translator'] = TranslatorInterface::class;
$services['contao.security.two_factor.backup_code_manager'] = BackupCodeManager::class;
$services['security.helper'] = Security::class;

return $services;
}

protected function getResponse(Template $template, ModuleModel $model, Request $request): Response
{
$token = $this->get('security.token_storage')->getToken();

if (!$token instanceof TokenInterface) {
return new Response('', Response::HTTP_NO_CONTENT);
}

$user = $token->getUser();
$user = $this->get('security.helper')->getUser();

if (!$user instanceof FrontendUser) {
return new Response('', Response::HTTP_NO_CONTENT);
Expand Down Expand Up @@ -117,13 +117,23 @@ protected function getResponse(Template $template, ModuleModel $model, Request $
}
}

if ('tl_two_factor_show_backup_codes' === $request->request->get('FORM_SUBMIT')) {
if (!$user->backupCodes || !\count(json_decode($user->backupCodes, true))) {
$this->generateBackupCodes($user);
}

$template->showBackupCodes = true;
}

if ('tl_two_factor_generate_backup_codes' === $request->request->get('FORM_SUBMIT')) {
$this->generateBackupCodes($user);

$template->showBackupCodes = true;
}

$template->isEnabled = (bool) $user->useTwoFactor;
$template->href = $this->page->getAbsoluteUrl().'?2fa=enable';
$template->twoFactor = $translator->trans('MSC.twoFactorAuthentication', [], 'contao_default');
$template->explain = $translator->trans('MSC.twoFactorExplain', [], 'contao_default');
$template->active = $translator->trans('MSC.twoFactorActive', [], 'contao_default');
$template->enableButton = $translator->trans('MSC.enable', [], 'contao_default');
$template->disableButton = $translator->trans('MSC.disable', [], 'contao_default');
$template->backupCodes = json_decode((string) $user->backupCodes, true) ?? [];

return new Response($template->parse());
}
Expand Down Expand Up @@ -164,11 +174,7 @@ private function enableTwoFactor(Template $template, Request $request, FrontendU

$template->enable = true;
$template->secret = Base32::encodeUpperUnpadded($user->secret);
$template->textCode = $translator->trans('MSC.twoFactorTextCode', [], 'contao_default');
$template->qrCode = base64_encode($authenticator->getQrCode($user, $request));
$template->scan = $translator->trans('MSC.twoFactorScan', [], 'contao_default');
$template->verify = $translator->trans('MSC.twoFactorVerification', [], 'contao_default');
$template->verifyHelp = $translator->trans('MSC.twoFactorVerificationHelp', [], 'contao_default');

return null;
}
Expand All @@ -182,8 +188,16 @@ private function disableTwoFactor(FrontendUser $user): ?Response

$user->secret = null;
$user->useTwoFactor = '';
$user->backupCodes = null;
$user->save();

return new RedirectResponse($this->page->getAbsoluteUrl());
}

private function generateBackupCodes(FrontendUser $user): void
{
/** @var BackupCodeManager $backupCodeManager */
$backupCodeManager = $this->get('contao.security.two_factor.backup_code_manager');
$backupCodeManager->generateBackupCodes($user);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class MakeServicesPublicPass implements CompilerPassInterface
'security.authentication.trust_resolver',
'security.firewall.map',
'security.logout_url_generator',
'security.helper',
];

private const ALIASES = [
Expand Down
6 changes: 6 additions & 0 deletions core-bundle/src/Resources/config/services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ services:
Contao\CoreBundle\Routing\ScopeMatcher: '@contao.routing.scope_matcher'
Contao\CoreBundle\Security\Authentication\Token\TokenChecker: '@contao.security.token_checker'
Contao\CoreBundle\Security\TwoFactor\Authenticator: '@contao.security.two_factor.authenticator'
Contao\CoreBundle\Security\TwoFactor\BackupCode\BackupCodeManager: '@contao.security.two_factor.backup_code_manager'
Contao\CoreBundle\Slug\Slug: '@contao.slug'


# Backwards compatibility
Contao\CoreBundle\Framework\ContaoFrameworkInterface: '@contao.framework'

Expand Down Expand Up @@ -640,6 +642,10 @@ services:
class: Contao\CoreBundle\Security\TwoFactor\Authenticator
public: true

contao.security.two_factor.backup_code_manager:
class: Contao\CoreBundle\Security\TwoFactor\BackupCode\BackupCodeManager
public: true

contao.security.two_factor.provider:
class: Contao\CoreBundle\Security\TwoFactor\Provider
arguments:
Expand Down
5 changes: 5 additions & 0 deletions core-bundle/src/Resources/contao/dca/tl_member.php
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,11 @@
(
'eval' => array('isBoolean'=>true, 'doNotCopy'=>true, 'tl_class'=>'w50 m12'),
'sql' => "char(1) NOT NULL default ''"
),
'backupCodes' => array
(
'eval' => array('doNotCopy'=>true),
'sql' => "text NULL"
bytehead marked this conversation as resolved.
Show resolved Hide resolved
)
)
);
Expand Down
5 changes: 5 additions & 0 deletions core-bundle/src/Resources/contao/dca/tl_user.php
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,11 @@
(
'eval' => array('rgxp'=>'datim', 'doNotCopy'=>true),
'sql' => "int(10) unsigned NOT NULL default 0"
),
'backupCodes' => array
(
'eval' => array('doNotCopy'=>true),
'sql' => "text NULL"
)
)
);
Expand Down
21 changes: 21 additions & 0 deletions core-bundle/src/Resources/contao/languages/en/default.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -1919,6 +1919,27 @@
<trans-unit id="MSC.twoFactorTextCode">
<source>If you cannot scan the QR code, enter this key instead:</source>
</trans-unit>
<trans-unit id="MSC.twoFactorBackupCodesLabel">
<source>Two-factor backup codes</source>
</trans-unit>
<trans-unit id="MSC.twoFactorBackupCodesShow">
<source>Show backup codes</source>
</trans-unit>
<trans-unit id="MSC.twoFactorBackupCodesExplain">
<source>Backup codes can be used to access your account if you have lost your authentication device and cannot generate two-factor codes anymore. Treat your backup codes with the same level of attention as your password! We recommend saving them with a password manager such as Lastpass, 1Password or Keeper.</source>
</trans-unit>
<trans-unit id="MSC.twoFactorBackupCodesInfo">
<source>Put these in a safe spot. If you lose your device and do not have backup codes, you cannot access your account anymore!</source>
</trans-unit>
<trans-unit id="MSC.twoFactorBackupCodesGenerate">
<source>Generate backup codes</source>
</trans-unit>
<trans-unit id="MSC.twoFactorBackupCodesRegenerate">
<source>Regenerate backup codes</source>
</trans-unit>
<trans-unit id="MSC.twoFactorBackupCodesRegenerateInfo">
<source>When you regenerate the backup codes, your old codes will not work anymore.</source>
</trans-unit>
<trans-unit id="MSC.userTemplateEditor">
<source>There is at least one regular user with access to the template editor. Since templates are PHP files, these users implicitly have full control over the system!</source>
</trans-unit>
Expand Down
2 changes: 2 additions & 0 deletions core-bundle/src/Resources/contao/library/Contao/Template.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@
* @property string $scan
* @property string $verify
* @property string $verifyHelp
* @property boolean $showBackupCodes
* @property array $backupCodes
*
* @author Leo Feyer <https://github.com/leofeyer>
*/
Expand Down
27 changes: 26 additions & 1 deletion core-bundle/src/Resources/contao/library/Contao/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
namespace Contao;

use Contao\CoreBundle\Exception\RedirectResponseException;
use Scheb\TwoFactorBundle\Model\BackupCodeInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
use Symfony\Component\Security\Core\User\EquatableInterface;
Expand Down Expand Up @@ -95,10 +96,11 @@
* @property object $objLogout
* @property string $useTwoFactor
* @property string|null $secret
* @property string|null $backupCodes
*
* @author Leo Feyer <https://github.com/leofeyer>
*/
abstract class User extends System implements UserInterface, EquatableInterface, \Serializable
abstract class User extends System implements UserInterface, EquatableInterface, BackupCodeInterface, \Serializable
{
/**
* Object instance (Singleton)
Expand Down Expand Up @@ -630,6 +632,29 @@ public function isEqualTo(UserInterface $user)
return true;
}

/**
* {@inheritdoc}
*/
public function isBackupCode(string $code): bool
{
return \in_array($code, json_decode($this->backupCodes, true));
}

/**
* {@inheritdoc}
*/
public function invalidateBackupCode(string $code): void
{
$backupCodes = json_decode($this->backupCodes, true);
$key = array_search($code, $backupCodes);

if ($key !== false)
{
unset($backupCodes[$key]);
$this->backupCodes = json_encode($backupCodes);
}
}
Comment on lines +638 to +656
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like extending that functionality in the user class. Can't the logic be in the BackupCodeManager?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact that we use our own backup code manager, it's probably possible to use empty method stubs on the user class instead...


/**
* Trigger the importUser hook
*
Expand Down
62 changes: 47 additions & 15 deletions core-bundle/src/Resources/contao/modules/ModuleTwoFactor.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,13 @@

namespace Contao;

use Contao\CoreBundle\Exception\AccessDeniedException;
use Contao\CoreBundle\Exception\RedirectResponseException;
use Contao\CoreBundle\Security\TwoFactor\Authenticator;
use Contao\CoreBundle\Security\TwoFactor\BackupCode\BackupCodeManager;
use ParagonIE\ConstantTime\Base32;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Security\Core\Security;

/**
* Back end module "two factor".
Expand All @@ -33,11 +36,16 @@ class ModuleTwoFactor extends BackendModule
*/
protected function compile()
{
$this->import(BackendUser::class, 'User');

$container = System::getContainer();
$ref = $container->get('request_stack')->getCurrentRequest()->attributes->get('_contao_referer_id');
$return = $container->get('router')->generate('contao_backend', array('do'=>'security', 'ref'=>$ref));

/** @var Security $security */
$security = $container->get('security.helper');

if (!$security->isGranted('IS_AUTHENTICATED_FULLY'))
{
throw new AccessDeniedException('User is not fully authenticated');
}

$user = BackendUser::getInstance();

// Inform the user if 2FA is enforced
Expand All @@ -46,9 +54,10 @@ protected function compile()
Message::addInfo($GLOBALS['TL_LANG']['MSC']['twoFactorEnforced']);
}

$ref = $container->get('request_stack')->getCurrentRequest()->attributes->get('_contao_referer_id');
$return = $container->get('router')->generate('contao_backend', array('do'=>'security', 'ref'=>$ref));

$this->Template->href = $this->getReferer(true);
$this->Template->title = StringUtil::specialchars($GLOBALS['TL_LANG']['MSC']['backBTTitle']);
$this->Template->button = $GLOBALS['TL_LANG']['MSC']['backBT'];
$this->Template->ref = $ref;
$this->Template->action = Environment::get('indexFreeRequest');
$this->Template->messages = Message::generateUnwrapped();
Expand All @@ -63,12 +72,25 @@ protected function compile()
$this->disableTwoFactor($user, $return);
}

$this->Template->isEnabled = (bool) $this->User->useTwoFactor;
$this->Template->twoFactor = $GLOBALS['TL_LANG']['MSC']['twoFactorAuthentication'];
$this->Template->explain = $GLOBALS['TL_LANG']['MSC']['twoFactorExplain'];
$this->Template->active = $GLOBALS['TL_LANG']['MSC']['twoFactorActive'];
$this->Template->enableButton = $GLOBALS['TL_LANG']['MSC']['enable'];
$this->Template->disableButton = $GLOBALS['TL_LANG']['MSC']['disable'];
if (Input::post('FORM_SUBMIT') == 'tl_two_factor_show_backup_codes')
{
if (!$user->backupCodes || !\count(json_decode($user->backupCodes, true)))
{
$this->generateBackupCodes($user);
}

$this->Template->showBackupCodes = true;
}

if (Input::post('FORM_SUBMIT') == 'tl_two_factor_generate_backup_codes')
{
$this->generateBackupCodes($user);

$this->Template->showBackupCodes = true;
}

$this->Template->isEnabled = (bool) $user->useTwoFactor;
$this->Template->backupCodes = json_decode((string) $user->backupCodes, true) ?? array();
}

/**
Expand Down Expand Up @@ -119,10 +141,7 @@ protected function enableTwoFactor(BackendUser $user, $return)

$this->Template->enable = true;
$this->Template->secret = Base32::encodeUpperUnpadded($user->secret);
$this->Template->textCode = $GLOBALS['TL_LANG']['MSC']['twoFactorTextCode'];
$this->Template->qrCode = base64_encode($authenticator->getQrCode($user, $request));
$this->Template->scan = $GLOBALS['TL_LANG']['MSC']['twoFactorScan'];
$this->Template->verify = $GLOBALS['TL_LANG']['MSC']['twoFactorVerification'];
$this->Template->verifyHelp = $verifyHelp;
}

Expand All @@ -142,8 +161,21 @@ protected function disableTwoFactor(BackendUser $user, $return)

$user->secret = null;
$user->useTwoFactor = '';
$user->backupCodes = null;
$user->save();

throw new RedirectResponseException($return);
}

/**
* Generate backup codes for two-factor authentication
*
* @param BackendUser $user
*/
private function generateBackupCodes(BackendUser $user)
{
/** @var BackupCodeManager $backupCodeManager */
$backupCodeManager = System::getContainer()->get('contao.security.two_factor.backup_code_manager');
$backupCodeManager->generateBackupCodes($user);
}
}
Loading