Skip to content

Commit

Permalink
Remove deprecated "window" option
Browse files Browse the repository at this point in the history
  • Loading branch information
scheb committed Nov 23, 2023
1 parent c5386f0 commit eb1f906
Show file tree
Hide file tree
Showing 14 changed files with 26 additions and 212 deletions.
6 changes: 2 additions & 4 deletions app/config/packages/scheb_2fa.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,14 @@ scheb_two_factor:
enabled: true # If Google Authenticator 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
leeway: 15 # Acceptable time drift in seconds
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
leeway: 15 # Acceptable time drift in seconds
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
16 changes: 2 additions & 14 deletions doc/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,7 @@ 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 # [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
leeway: 0 # Acceptable time drift in seconds, must be less or equal than 30 seconds
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 @@ -62,13 +56,7 @@ 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 # [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
leeway: 0 # Acceptable time drift in seconds, must be less or equal than the TOTP period
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
8 changes: 1 addition & 7 deletions doc/providers/google.rst
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,7 @@ 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 # [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
leeway: 0 # Acceptable time drift in seconds, must be less or equal than 30 seconds
template: security/2fa_form.html.twig # Template used to render the authentication form
Custom Authentication Form Template
Expand Down
8 changes: 1 addition & 7 deletions doc/providers/totp.rst
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,7 @@ 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 # [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
leeway: 0 # Acceptable time drift in seconds, must be less or equal than the TOTP period
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
22 changes: 6 additions & 16 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 ``leeway`` or ``window`` (deprecated) setting,
then more codes around the current time window will be accepted:
To counteract the issue of time differences you could increase the ``leeway`` setting, then more codes around the
current time window will be accepted:

.. code-block:: yaml
Expand All @@ -30,23 +30,13 @@ then more codes around the current time window will be accepted:
# For TOTP
totp:
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
leeway: 0 # Acceptable time drift in seconds, must be less or equal than the TOTP period
# For Google Authenticator
google:
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
leeway: 0 # Acceptable time drift in seconds, must be less or equal than 30 seconds
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: 2 additions & 10 deletions src/bundle/DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,7 @@ 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)
->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()
->integerNode('leeway')->defaultValue(0)->min(0)->end()
->arrayNode('parameters')
->scalarPrototype()->end()
->end()
Expand Down Expand Up @@ -210,11 +206,7 @@ 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)
->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()
->integerNode('leeway')->defaultValue(0)->min(0)->end()
->end()
->end()
->end();
Expand Down
29 changes: 0 additions & 29 deletions src/bundle/DependencyInjection/SchebTwoFactorExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,13 @@

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 @@ -193,19 +188,13 @@ 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');

$container->setParameter('scheb_two_factor.google.server_name', $config['google']['server_name']);
$container->setParameter('scheb_two_factor.google.issuer', $config['google']['issuer']);
$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']) {
Expand All @@ -220,17 +209,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');

$container->setParameter('scheb_two_factor.totp.issuer', $config['totp']['issuer']);
$container->setParameter('scheb_two_factor.totp.server_name', $config['totp']['server_name']);
$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']);
Expand All @@ -242,18 +225,6 @@ 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: 0 additions & 1 deletion src/bundle/Resources/config/two_factor_provider_google.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
->public()
->args([
service('scheb_two_factor.security.google_totp_factory'),
'%scheb_two_factor.google.window%',
'%scheb_two_factor.google.leeway%',
])

Expand Down
1 change: 0 additions & 1 deletion src/bundle/Resources/config/two_factor_provider_totp.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
->public()
->args([
service('scheb_two_factor.security.totp_factory'),
'%scheb_two_factor.totp.window%',
'%scheb_two_factor.totp.leeway%',
])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ class GoogleAuthenticator implements GoogleAuthenticatorInterface
public function __construct(
private readonly GoogleTotpFactory $totpFactory,
/** @var 0|positive-int */
private readonly int $window,
/** @var 0|positive-int|null */
private readonly null|int $leeway,
private readonly int $leeway,
) {
}

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

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

public function getQRContent(TwoFactorInterface $user): string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ class TotpAuthenticator implements TotpAuthenticatorInterface
public function __construct(
private readonly TotpFactory $totpFactory,
/** @var 0|positive-int */
private readonly int $window,
/** @var 0|positive-int|null */
private readonly null|int $leeway,
private readonly int $leeway,
) {
}

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

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

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

0 comments on commit eb1f906

Please sign in to comment.