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 expiring email codes #216

Closed
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
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
"egulias/email-validator": "^4.0",
"phpunit/phpunit": "^10.1",
"psr/container": ">=1.1",
"psr/clock": "^1.0",
"squizlabs/php_codesniffer": "^3.5",
"symfony/mailer": "^6.4 || ^7.0",
"symfony/yaml": "^6.4 || ^7.0",
Expand Down
2 changes: 2 additions & 0 deletions doc/providers/email.rst
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ Configuration Reference
sender_name: John Doe # Sender name
digits: 4 # Number of digits in authentication code
template: security/2fa_form.html.twig # Template used to render the authentication form
expires_after: PT5M # Date interval after which the code is considered invalid. NULL disables the expiration.
resend_expired: true # Whether to regenerate an expired code when user submits form

Custom Mailer
-------------
Expand Down
19 changes: 19 additions & 0 deletions src/bundle/DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Scheb\TwoFactorBundle\DependencyInjection;

use DateInterval;
use Scheb\TwoFactorBundle\Model\BackupCodeInterface;
use Scheb\TwoFactorBundle\Model\Email\TwoFactorInterface as EMailTwoFactorInterface;
use Scheb\TwoFactorBundle\Model\Google\TwoFactorInterface as GoogleTwoFactorInterface;
Expand All @@ -12,8 +13,10 @@
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
use Symfony\Component\Config\Definition\ConfigurationInterface;
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
use Symfony\Component\Security\Http\Authenticator\Token\PostAuthenticationToken;
use Throwable;
use function interface_exists;

/**
Expand Down Expand Up @@ -150,6 +153,22 @@ private function addEmailConfiguration(ArrayNodeDefinition $rootNode): void
->scalarNode('sender_name')->defaultNull()->end()
->scalarNode('template')->defaultValue('@SchebTwoFactor/Authentication/form.html.twig')->end()
->integerNode('digits')->defaultValue(4)->min(1)->end()
->scalarNode('expires_after')
->defaultNull()
->validate()
->ifString()
->then(static function (string $value): string {
try {
new DateInterval($value);
} catch (Throwable) {
throw new InvalidConfigurationException('"scheb_two_factor.email.expires_after" is not a valid \DateInterval value.');
}

return $value;
})
->end()
->end()
->booleanNode('resend_expired')->defaultTrue()->end()
->end()
->end()
->end();
Expand Down
2 changes: 2 additions & 0 deletions src/bundle/DependencyInjection/SchebTwoFactorExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@ private function configureEmailAuthenticationProvider(ContainerBuilder $containe
$container->setParameter('scheb_two_factor.email.sender_name', $config['email']['sender_name']);
$container->setParameter('scheb_two_factor.email.template', $config['email']['template']);
$container->setParameter('scheb_two_factor.email.digits', $config['email']['digits']);
$container->setParameter('scheb_two_factor.email.expires_after', $config['email']['expires_after']);
$container->setParameter('scheb_two_factor.email.resend_expired', $config['email']['resend_expired']);
$container->setAlias('scheb_two_factor.security.email.code_generator', $config['email']['code_generator'])->setPublic(true);

if (null !== $config['email']['mailer']) {
Expand Down
4 changes: 4 additions & 0 deletions src/bundle/Resources/config/two_factor_provider_email.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

declare(strict_types=1);

use Psr\Clock\ClockInterface;
use Scheb\TwoFactorBundle\Mailer\SymfonyAuthCodeMailer;
use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\DefaultTwoFactorFormRenderer;
use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\Email\EmailTwoFactorProvider;
Expand All @@ -25,6 +26,8 @@
service('scheb_two_factor.persister'),
service('scheb_two_factor.security.email.auth_code_mailer'),
'%scheb_two_factor.email.digits%',
'%scheb_two_factor.email.expires_after%',
service(ClockInterface::class)->nullOnInvalid(),
])

->set('scheb_two_factor.security.email.default_form_renderer', DefaultTwoFactorFormRenderer::class)
Expand All @@ -39,6 +42,7 @@
->args([
service('scheb_two_factor.security.email.code_generator'),
service('scheb_two_factor.security.email.form_renderer'),
'%scheb_two_factor.email.resend_expired%',
])

->alias(CodeGeneratorInterface::class, 'scheb_two_factor.security.email.code_generator')
Expand Down
6 changes: 6 additions & 0 deletions src/email/Model/Email/TwoFactorInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@

namespace Scheb\TwoFactorBundle\Model\Email;

use DateTimeImmutable;

/**
* @method DateTimeImmutable|null getEmailAuthCodeCreatedAt()
* @method void setEmailAuthCodeCreatedAt(DateTimeImmutable|null $createdAt)
*/
interface TwoFactorInterface
{
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\Email\Generator\CodeGeneratorInterface;
use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorFormRendererInterface;
use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderInterface;
use function method_exists;
use function str_replace;

/**
Expand All @@ -19,6 +20,7 @@ class EmailTwoFactorProvider implements TwoFactorProviderInterface
public function __construct(
private readonly CodeGeneratorInterface $codeGenerator,
private readonly TwoFactorFormRendererInterface $formRenderer,
private readonly bool $resendExpired,
) {
}

Expand All @@ -45,6 +47,14 @@ public function validateAuthenticationCode(object $user, string $authenticationC
return false;
}

if (method_exists($this->codeGenerator, 'isCodeExpired') && $this->codeGenerator->isCodeExpired($user)) {
if ($this->resendExpired) {
$this->codeGenerator->generateAndSend($user);
}

return false;
}

// Strip any user added spaces
$authenticationCode = str_replace(' ', '', $authenticationCode);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,13 @@

namespace Scheb\TwoFactorBundle\Security\TwoFactor\Provider\Email\Generator;

use DateInterval;
use DateTimeImmutable;
use Psr\Clock\ClockInterface;
use Scheb\TwoFactorBundle\Mailer\AuthCodeMailerInterface;
use Scheb\TwoFactorBundle\Model\Email\TwoFactorInterface;
use Scheb\TwoFactorBundle\Model\PersisterInterface;
use function method_exists;
use function random_int;

/**
Expand All @@ -18,26 +22,62 @@ public function __construct(
private readonly PersisterInterface $persister,
private readonly AuthCodeMailerInterface $mailer,
private readonly int $digits,
private readonly string|null $expiresAfter = null,
private readonly ClockInterface|null $clock = null,
) {
}

public function generateAndSend(TwoFactorInterface $user): void
{
$min = 10 ** ($this->digits - 1);
$max = 10 ** $this->digits - 1;
$code = $this->generateCode($min, $max);
$user->setEmailAuthCode((string) $code);
$user->setEmailAuthCode((string) $this->doGenerateCode());

if (method_exists($user, 'setEmailAuthCodeCreatedAt')) {
$user->setEmailAuthCodeCreatedAt($this->now());
}

$this->persister->persist($user);
$this->mailer->sendAuthCode($user);
}

public function reSend(TwoFactorInterface $user): void
{
if ($this->isCodeExpired($user) && method_exists($user, 'getEmailAuthCodeCreatedAt') && method_exists($user, 'setEmailAuthCodeCreatedAt')) {
$user->setEmailAuthCode((string) $this->doGenerateCode());
$user->setEmailAuthCodeCreatedAt($this->now());

$this->persister->persist($user);
}

$this->mailer->sendAuthCode($user);
}

public function isCodeExpired(TwoFactorInterface $user): bool
{
if (null === $this->expiresAfter || !method_exists($user, 'getEmailAuthCodeCreatedAt') || !method_exists($user, 'setEmailAuthCodeCreatedAt')) {
return false;
}

$now = $this->now();
$expiresAt = $user->getEmailAuthCodeCreatedAt()?->add(new DateInterval($this->expiresAfter));

return null !== $expiresAt && $now->getTimestamp() >= $expiresAt->getTimestamp();
}

protected function generateCode(int $min, int $max): int
{
return random_int($min, $max);
}

private function doGenerateCode(): int
{
$min = 10 ** ($this->digits - 1);
$max = 10 ** $this->digits - 1;

return $this->generateCode($min, $max);
}

private function now(): DateTimeImmutable
{
return $this->clock?->now() ?? new DateTimeImmutable();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

use Scheb\TwoFactorBundle\Model\Email\TwoFactorInterface;

/**
* @method bool isCodeExpired(TwoFactorInterface $user)
*/
interface CodeGeneratorInterface
{
/**
Expand Down
6 changes: 6 additions & 0 deletions tests/DependencyInjection/SchebTwoFactorExtensionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ public function load_emptyConfig_setDefaultValues(): void
$this->assertHasNotParameter('scheb_two_factor.email.sender_name');
$this->assertHasNotParameter('scheb_two_factor.email.template');
$this->assertHasNotParameter('scheb_two_factor.email.digits');
$this->assertHasNotParameter('scheb_two_factor.email.expires_after');
$this->assertHasNotParameter('scheb_two_factor.email.resend_expired');
$this->assertHasNotParameter('scheb_two_factor.google.server_name');
$this->assertHasNotParameter('scheb_two_factor.google.issuer');
$this->assertHasNotParameter('scheb_two_factor.google.template');
Expand Down Expand Up @@ -80,6 +82,8 @@ public function load_fullConfig_setConfigValues(): void
$this->assertHasParameter('Sender Name', 'scheb_two_factor.email.sender_name');
$this->assertHasParameter('AcmeTestBundle:Authentication:emailForm.html.twig', 'scheb_two_factor.email.template');
$this->assertHasParameter(6, 'scheb_two_factor.email.digits');
$this->assertHasParameter('PT15M', 'scheb_two_factor.email.expires_after');
$this->assertHasParameter(false, 'scheb_two_factor.email.resend_expired');
$this->assertHasParameter('Server Name Google', 'scheb_two_factor.google.server_name');
$this->assertHasParameter('Issuer Google', 'scheb_two_factor.google.issuer');
$this->assertHasParameter('AcmeTestBundle:Authentication:googleForm.html.twig', 'scheb_two_factor.google.template');
Expand Down Expand Up @@ -674,6 +678,8 @@ private function getFullConfig(): array
template: AcmeTestBundle:Authentication:emailForm.html.twig
form_renderer: acme_test.email_form_renderer
digits: 6
expires_after: PT15M
resend_expired: false
google:
enabled: true
issuer: Issuer Google
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\Email\EmailTwoFactorProvider;
use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\Email\Generator\CodeGeneratorInterface;
use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorFormRendererInterface;
use Scheb\TwoFactorBundle\Tests\Security\TwoFactor\Provider\Email\Generator\TestableCodeGeneratorInterface;
use Scheb\TwoFactorBundle\Tests\TestCase;
use stdClass;
use Symfony\Component\Security\Core\User\UserInterface;
Expand All @@ -20,13 +21,14 @@ class EmailTwoFactorProviderTest extends TestCase
private const VALID_AUTH_CODE_WITH_SPACES = ' valid Code ';

private MockObject|CodeGeneratorInterface $generator;
private MockObject|TwoFactorFormRendererInterface $formRenderer;
private EmailTwoFactorProvider $provider;

protected function setUp(): void
{
$this->generator = $this->createMock(CodeGeneratorInterface::class);
$formRenderer = $this->createMock(TwoFactorFormRendererInterface::class);
$this->provider = new EmailTwoFactorProvider($this->generator, $formRenderer);
$this->generator = $this->createMock(TestableCodeGeneratorInterface::class);
$this->formRenderer = $this->createMock(TwoFactorFormRendererInterface::class);
$this->provider = new EmailTwoFactorProvider($this->generator, $this->formRenderer, true);
}

private function createUser(bool $emailAuthEnabled = true): MockObject|UserWithTwoFactorInterface
Expand Down Expand Up @@ -161,4 +163,85 @@ public function validateAuthenticationCode_validCodeGiven_returnFalse(): void
$returnValue = $this->provider->validateAuthenticationCode($user, self::INVALID_AUTH_CODE);
$this->assertFalse($returnValue);
}

/**
* @test
*/
public function validateAuthenticationCode_withNotExpiredCode_returnsTrue(): void
{
$user = $this->createUser();

$this->generator
->expects($this->once())
->method('isCodeExpired')
->with($user)
->willReturn(false);

$this->assertTrue($this->provider->validateAuthenticationCode($user, self::VALID_AUTH_CODE));
}

/**
* @test
*/
public function validateAuthenticationCode_withExpiredCode_regeneratesCode(): void
{
$user = $this->createUser();

$this->generator
->expects($this->once())
->method('isCodeExpired')
->with($user)
->willReturn(true);

$this->generator
->expects($this->once())
->method('generateAndSend')
->with($user);

$this->assertFalse($this->provider->validateAuthenticationCode($user, self::VALID_AUTH_CODE));
}

/**
* @test
*/
public function validateAuthenticationCode_withValidExpiredCode_doesNotRegenerateCode(): void
{
$provider = new EmailTwoFactorProvider($this->generator, $this->formRenderer, false);

$user = $this->createUser();

$this->generator
->expects($this->once())
->method('isCodeExpired')
->with($user)
->willReturn(true);

$this->generator
->expects($this->never())
->method('generateAndSend');

$this->assertFalse($provider->validateAuthenticationCode($user, self::VALID_AUTH_CODE));
}

/**
* @test
*/
public function validateAuthenticationCode_withInvalidExpiredCode_doesNotRegenerateCode(): void
{
$provider = new EmailTwoFactorProvider($this->generator, $this->formRenderer, false);

$user = $this->createUser();

$this->generator
->expects($this->once())
->method('isCodeExpired')
->with($user)
->willReturn(true);

$this->generator
->expects($this->never())
->method('generateAndSend');

$this->assertFalse($provider->validateAuthenticationCode($user, self::INVALID_AUTH_CODE));
}
}
Loading
Loading