Skip to content

Commit

Permalink
Add "leeway" option, deprecate "window" option
Browse files Browse the repository at this point in the history
Addresses issue #201
  • Loading branch information
scheb committed Nov 23, 2023
1 parent 7a0c545 commit 1fe03e4
Show file tree
Hide file tree
Showing 14 changed files with 209 additions and 32 deletions.
2 changes: 2 additions & 0 deletions app/config/packages/scheb_2fa.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@ scheb_two_factor:
server_name: Server Name # Server name used in QR code
issuer: Issuer Name # Issuer name used in QR code
window: 1 # How many codes before/after the current one would be accepted as valid
# leeway: 30
template: security/2fa.html.twig # Template used to render the authentication form

totp:
enabled: true # If TOTP authentication should be enabled, default false
server_name: Server Name # Server name used in QR code
issuer: Issuer Name # Issuer name used in QR code
window: 1 # How many codes before/after the current one would be accepted as valid
# leeway: 30
parameters: # Additional parameters added in the QR code
image: 'https://my-service/img/logo.png'
template: security/2fa.html.twig # Template used to render the authentication form
Expand Down
20 changes: 14 additions & 6 deletions doc/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,13 @@ Bundle Configuration
server_name: Server Name # Server name used in QR code
issuer: Issuer Name # Issuer name used in QR code
digits: 6 # Number of digits in authentication code
window: 1 # Depends on the version of Spomky-Labs/otphp used:
# Until v10: How many codes before/after the current one would be accepted
# From v11: Acceptable time drift in seconds
window: 1 # [DEPRECATED since v6.11, will be removed in v7] Use "leeway", if possible
# Behavior depends on the version of Spomky-Labs/otphp used:
# - Until v10: How many codes before/after the current one would be accepted
# - From v11: Acceptable time drift in seconds
leeway: 0 # Acceptable time drift in seconds, requires Spomky-Labs/otphp v11 to be used
# Must be less or equal than 30 seconds
# If configured, takes precedence over the "window" option
template: security/2fa_form.html.twig # Template used to render the authentication form
form_renderer: acme.custom_form_renderer # Use a custom form renderer service
Expand All @@ -58,9 +62,13 @@ Bundle Configuration
enabled: true # If TOTP authentication should be enabled, default false
server_name: Server Name # Server name used in QR code
issuer: Issuer Name # Issuer name used in QR code
window: 1 # Depends on the version of Spomky-Labs/otphp used:
# Until v10: How many codes before/after the current one would be accepted
# From v11: Acceptable time drift in seconds
window: 1 # [DEPRECATED since v6.11, will be removed in v7] Use "leeway", if possible
# Behavior depends on the version of Spomky-Labs/otphp used:
# - Until v10: How many codes before/after the current one would be accepted
# - From v11: Acceptable time drift in seconds
leeway: 0 # Acceptable time drift in seconds, requires Spomky-Labs/otphp v11 to be used
# Must be less or equal than the TOTP code's period
# If configured, takes precedence over the "window" option
parameters: # Additional parameters added in the QR code
image: 'https://my-service/img/logo.png'
template: security/2fa_form.html.twig # Template used to render the authentication form
Expand Down
10 changes: 7 additions & 3 deletions doc/providers/google.rst
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,13 @@ Configuration Reference
server_name: Server Name # Server name used in QR code
issuer: Issuer Name # Issuer name used in QR code
digits: 6 # Number of digits in authentication code
window: 1 # Depends on the version of Spomky-Labs/otphp used:
# Until v10: How many codes before/after the current one would be accepted
# From v11: Acceptable time drift in seconds
window: 1 # [DEPRECATED since v6.11, will be removed in v7] Use "leeway", if possible
# Behavior depends on the version of Spomky-Labs/otphp used:
# - Until v10: How many codes before/after the current one would be accepted
# - From v11: Acceptable time drift in seconds
leeway: 0 # Acceptable time drift in seconds, requires Spomky-Labs/otphp v11 to be used
# Must be less or equal than 30 seconds
# If configured, takes precedence over the "window" option
template: security/2fa_form.html.twig # Template used to render the authentication form
Custom Authentication Form Template
Expand Down
10 changes: 7 additions & 3 deletions doc/providers/totp.rst
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,13 @@ Configuration Options
enabled: true # If TOTP authentication should be enabled, default false
server_name: Server Name # Server name used in QR code
issuer: Issuer Name # Issuer name used in QR code
window: 1 # Depends on the version of Spomky-Labs/otphp used:
# Until v10: How many codes before/after the current one would be accepted
# From v11: Acceptable time drift in seconds
window: 1 # [DEPRECATED since v6.11, will be removed in v7] Use "leeway", if possible
# Behavior depends on the version of Spomky-Labs/otphp used:
# - Until v10: How many codes before/after the current one would be accepted
# - From v11: Acceptable time drift in seconds
leeway: 0 # Acceptable time drift in seconds, requires Spomky-Labs/otphp v11 to be used
# Must be less or equal than the TOTP code's period
# If configured, takes precedence over the "window" option
parameters: # Additional parameters added in the QR code
image: 'https://my-service/img/logo.png'
template: security/2fa_form.html.twig # Template used to render the authentication form
Expand Down
24 changes: 16 additions & 8 deletions doc/troubleshooting.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ it depends on your configuration). The bigger the time difference between server
window, the higher the chance that the codes generated on server and from the app don't match up. When the time
difference becomes larger than the time window, it becomes impossible to provide the right code.

To counteract the issue of time differences you could increase the ``window`` setting, then more codes around the
current time window will be accepted:
To counteract the issue of time differences you could increase the ``leeway`` or ``window`` (deprecated) setting,
then more codes around the current time window will be accepted:

.. code-block:: yaml
Expand All @@ -30,15 +30,23 @@ current time window will be accepted:
# For TOTP
totp:
window: 1 # Depends on the version of Spomky-Labs/otphp used:
# Until v10: How many codes before/after the current one would be accepted
# From v11: Acceptable time drift in seconds
window: 1 # [DEPRECATED since v6.11, will be removed in v7] Use "leeway", if possible
# Behavior depends on the version of Spomky-Labs/otphp used:
# - Until v10: How many codes before/after the current one would be accepted
# - From v11: Acceptable time drift in seconds
leeway: 0 # Acceptable time drift in seconds, requires Spomky-Labs/otphp v11 to be used
# Must be less or equal than the TOTP code's period
# If configured, takes precedence over the "window" option
# For Google Authenticator
google:
window: 1 # Depends on the version of Spomky-Labs/otphp used:
# Until v10: How many codes before/after the current one would be accepted
# From v11: Acceptable time drift in seconds
window: 1 # [DEPRECATED since v6.11, will be removed in v7] Use "leeway", if possible
# Behavior depends on the version of Spomky-Labs/otphp used:
# - Until v10: How many codes before/after the current one would be accepted
# - From v11: Acceptable time drift in seconds
leeway: 0 # Acceptable time drift in seconds, requires Spomky-Labs/otphp v11 to be used
# Must be less or equal than 30 seconds
# If configured, takes precedence over the "window" option
You might want to configure a time synchronization service, such as ``ntpdate`` on your server to make sure your server
time is always in sync with UTC.
Expand Down
12 changes: 10 additions & 2 deletions src/bundle/DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,11 @@ private function addTotpConfiguration(ArrayNodeDefinition $rootNode): void
->scalarNode('form_renderer')->defaultNull()->end()
->scalarNode('issuer')->defaultNull()->end()
->scalarNode('server_name')->defaultNull()->end()
->integerNode('window')->defaultValue(1)->min(0)->end()
->integerNode('window')
->defaultValue(1)->min(0)
->setDeprecated('scheb/2fa-totp', '6.11', 'The "%path%.%node%" option is deprecated. Use "leeway" instead, which requires spomky-labs/otphp v11 to be used.')
->end()
->integerNode('leeway')->defaultNull()->min(0)->end()
->arrayNode('parameters')
->scalarPrototype()->end()
->end()
Expand Down Expand Up @@ -206,7 +210,11 @@ private function addGoogleAuthenticatorConfiguration(ArrayNodeDefinition $rootNo
->scalarNode('server_name')->defaultNull()->end()
->scalarNode('template')->defaultValue('@SchebTwoFactor/Authentication/form.html.twig')->end()
->integerNode('digits')->defaultValue(6)->min(1)->end()
->integerNode('window')->defaultValue(1)->min(0)->end()
->integerNode('window')
->defaultValue(1)->min(0)
->setDeprecated('scheb/2fa-google-authenticator', '6.11', 'The "%path%.%node%" option is deprecated. Use "leeway" instead, which requires spomky-labs/otphp v11 to be used.')
->end()
->integerNode('leeway')->defaultNull()->min(0)->end()
->end()
->end()
->end();
Expand Down
29 changes: 29 additions & 0 deletions src/bundle/DependencyInjection/SchebTwoFactorExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,18 @@

namespace Scheb\TwoFactorBundle\DependencyInjection;

use OTPHP\TOTP;
use ReflectionMethod;
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Loader;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\HttpKernel\DependencyInjection\Extension;
use function assert;
use function class_exists;
use function count;
use function is_bool;
use function is_string;
use function trim;
Expand Down Expand Up @@ -188,6 +193,11 @@ private function configureEmailAuthenticationProvider(ContainerBuilder $containe
*/
private function configureGoogleAuthenticationProvider(ContainerBuilder $container, array $config): void
{
// Migration path for the "leeway" option, to be fully migrated in bundle version 7
if (null !== $config['google']['leeway'] && !$this->isSpomkyOtphpVersion11Used()) {
throw new InvalidConfigurationException('The "leeway" option can only be set when spomky-labs/otphp v11 is used.');
}

$loader = new Loader\PhpFileLoader($container, new FileLocator(__DIR__.'/../Resources/config'));
$loader->load('two_factor_provider_google.php');

Expand All @@ -196,6 +206,7 @@ private function configureGoogleAuthenticationProvider(ContainerBuilder $contain
$container->setParameter('scheb_two_factor.google.template', $config['google']['template']);
$container->setParameter('scheb_two_factor.google.digits', $config['google']['digits']);
$container->setParameter('scheb_two_factor.google.window', $config['google']['window']);
$container->setParameter('scheb_two_factor.google.leeway', $config['google']['leeway']);

if (null === $config['google']['form_renderer']) {
return;
Expand All @@ -209,6 +220,11 @@ private function configureGoogleAuthenticationProvider(ContainerBuilder $contain
*/
private function configureTotpAuthenticationProvider(ContainerBuilder $container, array $config): void
{
// Migration path for the "leeway" option, to be fully migrated in bundle version 7
if (null !== $config['totp']['leeway'] && !$this->isSpomkyOtphpVersion11Used()) {
throw new InvalidConfigurationException('The "leeway" option can only be set when spomky-labs/otphp v11 is used.');
}

$loader = new Loader\PhpFileLoader($container, new FileLocator(__DIR__.'/../Resources/config'));
$loader->load('two_factor_provider_totp.php');

Expand All @@ -217,6 +233,7 @@ private function configureTotpAuthenticationProvider(ContainerBuilder $container
$container->setParameter('scheb_two_factor.totp.window', $config['totp']['window']);
$container->setParameter('scheb_two_factor.totp.parameters', $config['totp']['parameters']);
$container->setParameter('scheb_two_factor.totp.template', $config['totp']['template']);
$container->setParameter('scheb_two_factor.totp.leeway', $config['totp']['leeway']);

if (null === $config['totp']['form_renderer']) {
return;
Expand All @@ -225,6 +242,18 @@ private function configureTotpAuthenticationProvider(ContainerBuilder $container
$container->setAlias('scheb_two_factor.security.totp.form_renderer', $config['totp']['form_renderer']);
}

private function isSpomkyOtphpVersion11Used(): bool
{
if (!class_exists(TOTP::class)) {
return false;
}

$parameters = (new ReflectionMethod(TOTP::class, 'verify'))->getParameters();

// Third parameter must be named "leeway"
return count($parameters) >= 3 && 'leeway' === $parameters[2]->getName();
}

private function resolveFeatureFlag(ContainerBuilder $container, bool|string $value): bool
{
$retValue = $container->resolveEnvPlaceholders($value, true);
Expand Down
1 change: 1 addition & 0 deletions src/bundle/Resources/config/two_factor_provider_google.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
->args([
service('scheb_two_factor.security.google_totp_factory'),
'%scheb_two_factor.google.window%',
'%scheb_two_factor.google.leeway%',
])

->set('scheb_two_factor.security.google.default_form_renderer', DefaultTwoFactorFormRenderer::class)
Expand Down
1 change: 1 addition & 0 deletions src/bundle/Resources/config/two_factor_provider_totp.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
->args([
service('scheb_two_factor.security.totp_factory'),
'%scheb_two_factor.totp.window%',
'%scheb_two_factor.totp.leeway%',
])

->set('scheb_two_factor.security.totp.default_form_renderer', DefaultTwoFactorFormRenderer::class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ public function __construct(
private GoogleTotpFactory $totpFactory,
/** @var 0|positive-int */
private int $window,
/** @var 0|positive-int|null */
private null|int $leeway,
) {
}

Expand All @@ -31,7 +33,7 @@ public function checkCode(TwoFactorInterface $user, string $code): bool
}

/** @var non-empty-string $code */
return $this->totpFactory->createTotpForUser($user)->verify($code, null, $this->window);
return $this->totpFactory->createTotpForUser($user)->verify($code, null, $this->leeway ?? $this->window);
}

public function getQRContent(TwoFactorInterface $user): string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ public function __construct(
private TotpFactory $totpFactory,
/** @var 0|positive-int */
private int $window,
/** @var 0|positive-int|null */
private null|int $leeway,
) {
}

Expand All @@ -31,7 +33,7 @@ public function checkCode(TwoFactorInterface $user, string $code): bool
}

/** @var non-empty-string $code */
return $this->totpFactory->createTotpForUser($user)->verify($code, null, $this->window);
return $this->totpFactory->createTotpForUser($user)->verify($code, null, $this->leeway ?? $this->window);
}

public function getQRContent(TwoFactorInterface $user): string
Expand Down
Loading

0 comments on commit 1fe03e4

Please sign in to comment.