Add option to allow only POST requests to check_path

DependencyInjection/Factory/Security/TwoFactorFactory.php

@@ -17,6 +17,7 @@ class TwoFactorFactory implements SecurityFactoryInterface
     public const AUTHENTICATION_PROVIDER_KEY = 'two_factor';
 
     public const DEFAULT_CHECK_PATH = '/2fa_check';
+    public const DEFAULT_POST_ONLY = false;
     public const DEFAULT_AUTH_FORM_PATH = '/2fa';
     public const DEFAULT_ALWAYS_USE_DEFAULT_TARGET_PATH = false;
     public const DEFAULT_TARGET_PATH = '/';
@@ -64,6 +65,7 @@ public function addConfiguration(NodeDefinition $node)
          */
         $builder
             ->scalarNode('check_path')->defaultValue(self::DEFAULT_CHECK_PATH)->end()
+            ->booleanNode('post_only')->defaultValue(self::DEFAULT_POST_ONLY)->end()
             ->scalarNode('auth_form_path')->defaultValue(self::DEFAULT_AUTH_FORM_PATH)->end()
             ->booleanNode('always_use_default_target_path')->defaultValue(self::DEFAULT_ALWAYS_USE_DEFAULT_TARGET_PATH)->end()
             ->scalarNode('default_target_path')->defaultValue(self::DEFAULT_TARGET_PATH)->end()

Resources/doc/configuration.md

@@ -93,6 +93,7 @@ security:
             two_factor:
                 auth_form_path: /2fa                  # Path or route name of the two-factor form
                 check_path: /2fa_check                # Path or route name of the two-factor code check
+                post_only: false                      # If the check_path should accept the code only as a POST request
                 default_target_path: /                # Where to redirect by default after successful authentication
                 always_use_default_target_path: false # If it should always redirect to default_target_path
                 auth_code_parameter_name: _auth_code  # Name of the parameter for the two-factor authentication code

Security/Http/Authentication/DefaultAuthenticationRequiredHandler.php

@@ -18,6 +18,7 @@ class DefaultAuthenticationRequiredHandler implements AuthenticationRequiredHand
     private const DEFAULT_OPTIONS = [
         'auth_form_path' => TwoFactorFactory::DEFAULT_AUTH_FORM_PATH,
         'check_path' => TwoFactorFactory::DEFAULT_CHECK_PATH,
+        'post_only' => TwoFactorFactory::DEFAULT_POST_ONLY,
     ];
 
     /**
@@ -31,7 +32,7 @@ class DefaultAuthenticationRequiredHandler implements AuthenticationRequiredHand
     private $firewallName;
 
     /**
-     * @var string[]
+     * @var array
      */
     private $options;
 
@@ -58,6 +59,7 @@ public function onAuthenticationRequired(Request $request, TokenInterface $token
 
     private function isCheckAuthCodeRequest(Request $request): bool
     {
-        return $this->httpUtils->checkRequestPath($request, $this->options['check_path']);
+        return ($this->options['post_only'] ? $request->isMethod('POST') : true)
+            && $this->httpUtils->checkRequestPath($request, $this->options['check_path']);
     }
 }

Security/Http/Firewall/TwoFactorListener.php

@@ -36,6 +36,7 @@ class TwoFactorListener
     private const DEFAULT_OPTIONS = [
         'auth_form_path' => TwoFactorFactory::DEFAULT_AUTH_FORM_PATH,
         'check_path' => TwoFactorFactory::DEFAULT_CHECK_PATH,
+        'post_only' => TwoFactorFactory::DEFAULT_POST_ONLY,
         'auth_code_parameter_name' => TwoFactorFactory::DEFAULT_AUTH_CODE_PARAMETER_NAME,
         'trusted_parameter_name' => TwoFactorFactory::DEFAULT_TRUSTED_PARAMETER_NAME,
     ];
@@ -81,7 +82,7 @@ class TwoFactorListener
     private $csrfTokenValidator;
 
     /**
-     * @var string[]
+     * @var array
      */
     private $options;
 
@@ -184,7 +185,8 @@ public function __invoke($event)
 
     private function isCheckAuthCodeRequest(Request $request): bool
     {
-        return $this->httpUtils->checkRequestPath($request, $this->options['check_path']);
+        return ($this->options['post_only'] ? $request->isMethod('POST') : true)
+            && $this->httpUtils->checkRequestPath($request, $this->options['check_path']);
     }
 
     private function isAuthFormRequest(Request $request): bool

Security/TwoFactor/TwoFactorFirewallConfig.php

@@ -57,4 +57,9 @@ public function getCheckPath(): string
     {
         return $this->options['check_path'] ?? TwoFactorFactory::DEFAULT_CHECK_PATH;
     }
+
+    public function isPostOnly(): bool
+    {
+        return $this->options['post_only'] ?? TwoFactorFactory::DEFAULT_POST_ONLY;
+    }
 }

Tests/DependencyInjection/Factory/Security/TwoFactorFactoryTest.php

@@ -53,6 +53,7 @@ private function getFullConfig(): array
         $yaml = <<<EOF
 two_factor:
     check_path: /check_path
+    post_only: true
     auth_form_path: /auth_form_path
     always_use_default_target_path: true
     default_target_path: /default_target_path
@@ -105,6 +106,7 @@ public function addConfiguration_emptyConfig_setDefaultValues(): void
         $processedConfiguration = $this->processConfiguration($config);
 
         $this->assertEquals(TwoFactorFactory::DEFAULT_CHECK_PATH, $processedConfiguration['check_path']);
+        $this->assertEquals(TwoFactorFactory::DEFAULT_POST_ONLY, $processedConfiguration['post_only']);
         $this->assertEquals(TwoFactorFactory::DEFAULT_AUTH_FORM_PATH, $processedConfiguration['auth_form_path']);
         $this->assertEquals(TwoFactorFactory::DEFAULT_ALWAYS_USE_DEFAULT_TARGET_PATH, $processedConfiguration['always_use_default_target_path']);
         $this->assertEquals(TwoFactorFactory::DEFAULT_TARGET_PATH, $processedConfiguration['default_target_path']);
@@ -130,6 +132,7 @@ public function addConfiguration_fullConfig_setConfigValues(): void
         $processedConfiguration = $this->processConfiguration($config);
 
         $this->assertEquals('/check_path', $processedConfiguration['check_path']);
+        $this->assertTrue($processedConfiguration['post_only']);
         $this->assertEquals('/auth_form_path', $processedConfiguration['auth_form_path']);
         $this->assertTrue($processedConfiguration['always_use_default_target_path']);
         $this->assertEquals('/default_target_path', $processedConfiguration['default_target_path']);

Tests/Security/Http/Authentication/DefaultAuthenticationRequiredHandlerTest.php

@@ -17,6 +17,7 @@ class DefaultAuthenticationRequiredHandlerTest extends TestCase
 {
     const FORM_PATH = '/form_path';
     const CHECK_PATH = '/check_path';
+    private const POST_ONLY = true;
     const FIREWALL_NAME = 'firewallName';
 
     /**
@@ -54,6 +55,7 @@ protected function setUp(): void
         $options = [
             'auth_form_path' => self::FORM_PATH,
             'check_path' => self::CHECK_PATH,
+            'post_only' => self::POST_ONLY,
         ];
 
         $this->handler = new DefaultAuthenticationRequiredHandler(
@@ -79,6 +81,21 @@ private function stubCurrentPath(string $currentPath): void
             });
     }
 
+    private function stubPostRequest(): void
+    {
+        $this->request
+            ->expects($this->any())
+            ->method('isMethod')
+            ->with('POST')
+            ->willReturn(true);
+    }
+
+    private function stubCurrentPathIsCheckPath(): void
+    {
+        $this->stubPostRequest();
+        $this->stubCurrentPath(self::CHECK_PATH);
+    }
+
     private function assertSaveTargetUrl(string $targetUrl): void
     {
         $session = $this->createMock(SessionInterface::class);
@@ -158,7 +175,7 @@ public function onAuthenticationRequired_isNotCheckPath_saveRedirectUrl(): void
     public function onAuthenticationRequired_isCheckPath_notSaveRedirectUrl(): void
     {
         $token = $this->createMock(TokenInterface::class);
-        $this->stubCurrentPath(self::CHECK_PATH);
+        $this->stubCurrentPathIsCheckPath();
 
         $this->assertNotSaveTargetUrl();
         $this->handler->onAuthenticationRequired($this->request, $token);

Tests/Security/Http/Firewall/TwoFactorListenerTest.php

@@ -37,6 +37,7 @@ class TwoFactorListenerTest extends TestCase
 {
     private const FORM_PATH = '/form_path';
     private const CHECK_PATH = '/check_path';
+    private const POST_ONLY = true;
     private const AUTH_CODE_PARAM = 'auth_code_param';
     private const TRUSTED_PARAM = 'trusted_param';
     private const FIREWALL_NAME = 'firewallName';
@@ -164,6 +165,7 @@ protected function setUp(): void
         $options = [
             'auth_form_path' => self::FORM_PATH,
             'check_path' => self::CHECK_PATH,
+            'post_only' => self::POST_ONLY,
             'auth_code_parameter_name' => self::AUTH_CODE_PARAM,
             'trusted_parameter_name' => self::TRUSTED_PARAM,
         ];
@@ -248,6 +250,21 @@ private function stubCurrentPath(string $currentPath): void
             });
     }
 
+    private function stubPostRequest(): void
+    {
+        $this->request
+            ->expects($this->any())
+            ->method('isMethod')
+            ->with('POST')
+            ->willReturn(true);
+    }
+
+    private function stubCurrentPathIsCheckPath(): void
+    {
+        $this->stubPostRequest();
+        $this->stubCurrentPath(self::CHECK_PATH);
+    }
+
     private function stubRequestHasParameter(string $parameterName, $value): void
     {
         $this->requestParams[$parameterName] = $value;
@@ -447,7 +464,7 @@ public function handle_isCheckPath_authenticateWithAuthenticationManager(): void
         $authenticatedToken = $this->createMock(TokenInterface::class);
         $twoFactorToken = $this->createTwoFactorToken(self::FIREWALL_NAME, $authenticatedToken);
         $this->stubTokenManagerHasToken($twoFactorToken);
-        $this->stubCurrentPath(self::CHECK_PATH);
+        $this->stubCurrentPathIsCheckPath();
         $this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
         $this->stubHandlersReturnResponse();
 
@@ -473,7 +490,7 @@ public function handle_isCheckPath_authenticateWithAuthenticationManager(): void
     public function handle_authenticationException_dispatchFailureEvent(): void
     {
         $this->stubTokenManagerHasToken($this->createTwoFactorToken());
-        $this->stubCurrentPath(self::CHECK_PATH);
+        $this->stubCurrentPathIsCheckPath();
         $this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
         $this->stubAuthenticationManagerThrowsAuthenticationException();
         $this->stubHandlersReturnResponse();
@@ -492,7 +509,7 @@ public function handle_authenticationException_dispatchFailureEvent(): void
     public function handle_authenticationException_setResponseFromFailureHandler(): void
     {
         $this->stubTokenManagerHasToken($this->createTwoFactorToken());
-        $this->stubCurrentPath(self::CHECK_PATH);
+        $this->stubCurrentPathIsCheckPath();
         $this->stubAuthenticationManagerThrowsAuthenticationException();
 
         $response = $this->createMock(Response::class);
@@ -515,7 +532,7 @@ public function handle_authenticationException_setResponseFromFailureHandler():
     public function handle_csrfTokenInvalid_dispatchFailureEvent(): void
     {
         $this->stubTokenManagerHasToken($this->createTwoFactorToken());
-        $this->stubCurrentPath(self::CHECK_PATH);
+        $this->stubCurrentPathIsCheckPath();
         $this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsFalse();
         $this->stubHandlersReturnResponse();
 
@@ -531,7 +548,7 @@ public function handle_authenticationStepSuccessful_dispatchSuccessEvent(): void
     {
         $twoFactorToken = $this->createTwoFactorToken();
         $this->stubTokenManagerHasToken($twoFactorToken);
-        $this->stubCurrentPath(self::CHECK_PATH);
+        $this->stubCurrentPathIsCheckPath();
         $this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
         $this->stubAuthenticationManagerReturnsToken($twoFactorToken); // Must be TwoFactorToken
 
@@ -551,7 +568,7 @@ public function handle_authenticationStepSuccessfulButNotCompleted_redirectToAut
     {
         $twoFactorToken = $this->createTwoFactorToken();
         $this->stubTokenManagerHasToken($twoFactorToken);
-        $this->stubCurrentPath(self::CHECK_PATH);
+        $this->stubCurrentPathIsCheckPath();
         $this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
         $this->stubAuthenticationManagerReturnsToken($twoFactorToken); // Must be TwoFactorToken
 
@@ -572,7 +589,7 @@ public function handle_authenticationStepSuccessfulButNotCompleted_dispatchRequi
     {
         $twoFactorToken = $this->createTwoFactorToken();
         $this->stubTokenManagerHasToken($twoFactorToken);
-        $this->stubCurrentPath(self::CHECK_PATH);
+        $this->stubCurrentPathIsCheckPath();
         $this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
         $this->stubAuthenticationManagerReturnsToken($twoFactorToken); // Must be TwoFactorToken
 
@@ -591,7 +608,7 @@ public function handle_authenticationStepSuccessfulButNotCompleted_dispatchRequi
     public function handle_twoFactorProcessComplete_returnResponseFromSuccessHandler(): void
     {
         $this->stubTokenManagerHasToken($this->createTwoFactorToken());
-        $this->stubCurrentPath(self::CHECK_PATH);
+        $this->stubCurrentPathIsCheckPath();
         $this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
         $this->stubAuthenticationManagerReturnsToken($this->createMock(TokenInterface::class)); // Not a TwoFactorToken
 
@@ -615,7 +632,7 @@ public function handle_twoFactorProcessComplete_returnResponseFromSuccessHandler
     public function handle_twoFactorProcessComplete_dispatchCompleteEvent(): void
     {
         $this->stubTokenManagerHasToken($this->createTwoFactorToken());
-        $this->stubCurrentPath(self::CHECK_PATH);
+        $this->stubCurrentPathIsCheckPath();
         $this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
         $this->stubAuthenticationManagerReturnsToken($this->createMock(TokenInterface::class)); // Not a TwoFactorToken
         $this->stubHandlersReturnResponse();
@@ -641,7 +658,7 @@ public function handle_twoFactorProcessCompleteWithTrustedEnabled_setTrustedDevi
             ->willReturn('user');
 
         $this->stubTokenManagerHasToken($this->createTwoFactorToken());
-        $this->stubCurrentPath(self::CHECK_PATH);
+        $this->stubCurrentPathIsCheckPath();
         $this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
         $this->stubRequestHasParameter(self::TRUSTED_PARAM, '1');
         $this->stubAuthenticationManagerReturnsToken($authenticatedToken); // Not a TwoFactorToken
@@ -661,7 +678,7 @@ public function handle_twoFactorProcessCompleteWithTrustedEnabled_setTrustedDevi
     public function handle_twoFactorProcessCompleteWithTrustedDisabled_notSetTrustedDevice(): void
     {
         $this->stubTokenManagerHasToken($this->createTwoFactorToken());
-        $this->stubCurrentPath(self::CHECK_PATH);
+        $this->stubCurrentPathIsCheckPath();
         $this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
         $this->stubRequestHasParameter(self::TRUSTED_PARAM, '0');
         $this->stubAuthenticationManagerReturnsToken($this->createMock(TokenInterface::class)); // Not a TwoFactorToken
@@ -690,7 +707,7 @@ public function handle_twoFactorProcessCompleteWithRememberMeEnabled_setRemember
         $twoFactorToken = $this->createTwoFactorToken(self::FIREWALL_NAME, null, $attributes);
 
         $this->stubTokenManagerHasToken($twoFactorToken);
-        $this->stubCurrentPath(self::CHECK_PATH);
+        $this->stubCurrentPathIsCheckPath();
         $this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
         $this->stubAuthenticationManagerReturnsToken($authenticatedToken); // Not a TwoFactorToken
         $response = $this->stubHandlersReturnResponse();

Tests/Security/TwoFactor/TwoFactorFirewallConfigTest.php

@@ -11,6 +11,7 @@ class TwoFactorFirewallConfigTest extends TestCase
 {
     private const FULL_OPTIONS = [
         'check_path' => 'check_path_route_name',
+        'post_only' => true,
         'auth_form_path' => 'auth_form_path_route_name',
         'multi_factor' => true,
         'auth_code_parameter_name' => 'auth_code_param',
@@ -123,4 +124,22 @@ public function getCheckPath_optionNotSet_returnDefault(): void
         $returnValue = $this->createConfig([])->getCheckPath();
         $this->assertEquals('/2fa_check', $returnValue);
     }
+
+    /**
+     * @test
+     */
+    public function isPostOnly_optionSet_returnThatValue(): void
+    {
+        $returnValue = $this->createConfig()->isPostOnly();
+        $this->assertTrue($returnValue);
+    }
+
+    /**
+     * @test
+     */
+    public function isPostOnly_optionNotSet_returnDefault(): void
+    {
+        $returnValue = $this->createConfig([])->isPostOnly();
+        $this->assertFalse($returnValue);
+    }
 }