Skip to content
This repository was archived by the owner on Dec 2, 2021. It is now read-only.

Commit 44ae7d1

Browse files
schebscheb
committed
Add option to allow only POST requests to check_path
1 parent 3ba8705 commit 44ae7d1

File tree

9 files changed

+85
-17
lines changed

9 files changed

+85
-17
lines changed

‎DependencyInjection/Factory/Security/TwoFactorFactory.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ class TwoFactorFactory implements SecurityFactoryInterface
1717
public const AUTHENTICATION_PROVIDER_KEY = 'two_factor';
1818

1919
public const DEFAULT_CHECK_PATH = '/2fa_check';
20+
public const DEFAULT_POST_ONLY = false;
2021
public const DEFAULT_AUTH_FORM_PATH = '/2fa';
2122
public const DEFAULT_ALWAYS_USE_DEFAULT_TARGET_PATH = false;
2223
public const DEFAULT_TARGET_PATH = '/';
@@ -64,6 +65,7 @@ public function addConfiguration(NodeDefinition $node)
6465
*/
6566
$builder
6667
->scalarNode('check_path')->defaultValue(self::DEFAULT_CHECK_PATH)->end()
68+
->booleanNode('post_only')->defaultValue(self::DEFAULT_POST_ONLY)->end()
6769
->scalarNode('auth_form_path')->defaultValue(self::DEFAULT_AUTH_FORM_PATH)->end()
6870
->booleanNode('always_use_default_target_path')->defaultValue(self::DEFAULT_ALWAYS_USE_DEFAULT_TARGET_PATH)->end()
6971
->scalarNode('default_target_path')->defaultValue(self::DEFAULT_TARGET_PATH)->end()

‎Resources/doc/configuration.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ security:
9393
two_factor:
9494
auth_form_path: /2fa # Path or route name of the two-factor form
9595
check_path: /2fa_check # Path or route name of the two-factor code check
96+
post_only: false # If the check_path should accept the code only as a POST request
9697
default_target_path: / # Where to redirect by default after successful authentication
9798
always_use_default_target_path: false # If it should always redirect to default_target_path
9899
auth_code_parameter_name: _auth_code # Name of the parameter for the two-factor authentication code

‎Security/Http/Authentication/DefaultAuthenticationRequiredHandler.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ class DefaultAuthenticationRequiredHandler implements AuthenticationRequiredHand
1818
private const DEFAULT_OPTIONS = [
1919
'auth_form_path' => TwoFactorFactory::DEFAULT_AUTH_FORM_PATH,
2020
'check_path' => TwoFactorFactory::DEFAULT_CHECK_PATH,
21+
'post_only' => TwoFactorFactory::DEFAULT_POST_ONLY,
2122
];
2223

2324
/**
@@ -31,7 +32,7 @@ class DefaultAuthenticationRequiredHandler implements AuthenticationRequiredHand
3132
private $firewallName;
3233

3334
/**
34-
* @var string[]
35+
* @var array
3536
*/
3637
private $options;
3738

@@ -58,6 +59,7 @@ public function onAuthenticationRequired(Request $request, TokenInterface $token
5859

5960
private function isCheckAuthCodeRequest(Request $request): bool
6061
{
61-
return $this->httpUtils->checkRequestPath($request, $this->options['check_path']);
62+
return ($this->options['post_only'] ? $request->isMethod('POST') : true)
63+
&& $this->httpUtils->checkRequestPath($request, $this->options['check_path']);
6264
}
6365
}

‎Security/Http/Firewall/TwoFactorListener.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ class TwoFactorListener
3636
private const DEFAULT_OPTIONS = [
3737
'auth_form_path' => TwoFactorFactory::DEFAULT_AUTH_FORM_PATH,
3838
'check_path' => TwoFactorFactory::DEFAULT_CHECK_PATH,
39+
'post_only' => TwoFactorFactory::DEFAULT_POST_ONLY,
3940
'auth_code_parameter_name' => TwoFactorFactory::DEFAULT_AUTH_CODE_PARAMETER_NAME,
4041
'trusted_parameter_name' => TwoFactorFactory::DEFAULT_TRUSTED_PARAMETER_NAME,
4142
];
@@ -81,7 +82,7 @@ class TwoFactorListener
8182
private $csrfTokenValidator;
8283

8384
/**
84-
* @var string[]
85+
* @var array
8586
*/
8687
private $options;
8788

@@ -184,7 +185,8 @@ public function __invoke($event)
184185

185186
private function isCheckAuthCodeRequest(Request $request): bool
186187
{
187-
return $this->httpUtils->checkRequestPath($request, $this->options['check_path']);
188+
return ($this->options['post_only'] ? $request->isMethod('POST') : true)
189+
&& $this->httpUtils->checkRequestPath($request, $this->options['check_path']);
188190
}
189191

190192
private function isAuthFormRequest(Request $request): bool

‎Security/TwoFactor/TwoFactorFirewallConfig.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,4 +57,9 @@ public function getCheckPath(): string
5757
{
5858
return $this->options['check_path'] ?? TwoFactorFactory::DEFAULT_CHECK_PATH;
5959
}
60+
61+
public function isPostOnly(): bool
62+
{
63+
return $this->options['post_only'] ?? TwoFactorFactory::DEFAULT_POST_ONLY;
64+
}
6065
}

‎Tests/DependencyInjection/Factory/Security/TwoFactorFactoryTest.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ private function getFullConfig(): array
5353
$yaml = <<<EOF
5454
two_factor:
5555
check_path: /check_path
56+
post_only: true
5657
auth_form_path: /auth_form_path
5758
always_use_default_target_path: true
5859
default_target_path: /default_target_path
@@ -105,6 +106,7 @@ public function addConfiguration_emptyConfig_setDefaultValues(): void
105106
$processedConfiguration = $this->processConfiguration($config);
106107

107108
$this->assertEquals(TwoFactorFactory::DEFAULT_CHECK_PATH, $processedConfiguration['check_path']);
109+
$this->assertEquals(TwoFactorFactory::DEFAULT_POST_ONLY, $processedConfiguration['post_only']);
108110
$this->assertEquals(TwoFactorFactory::DEFAULT_AUTH_FORM_PATH, $processedConfiguration['auth_form_path']);
109111
$this->assertEquals(TwoFactorFactory::DEFAULT_ALWAYS_USE_DEFAULT_TARGET_PATH, $processedConfiguration['always_use_default_target_path']);
110112
$this->assertEquals(TwoFactorFactory::DEFAULT_TARGET_PATH, $processedConfiguration['default_target_path']);
@@ -130,6 +132,7 @@ public function addConfiguration_fullConfig_setConfigValues(): void
130132
$processedConfiguration = $this->processConfiguration($config);
131133

132134
$this->assertEquals('/check_path', $processedConfiguration['check_path']);
135+
$this->assertTrue($processedConfiguration['post_only']);
133136
$this->assertEquals('/auth_form_path', $processedConfiguration['auth_form_path']);
134137
$this->assertTrue($processedConfiguration['always_use_default_target_path']);
135138
$this->assertEquals('/default_target_path', $processedConfiguration['default_target_path']);

‎Tests/Security/Http/Authentication/DefaultAuthenticationRequiredHandlerTest.php‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ class DefaultAuthenticationRequiredHandlerTest extends TestCase
1717
{
1818
const FORM_PATH = '/form_path';
1919
const CHECK_PATH = '/check_path';
20+
private const POST_ONLY = true;
2021
const FIREWALL_NAME = 'firewallName';
2122

2223
/**
@@ -54,6 +55,7 @@ protected function setUp(): void
5455
$options = [
5556
'auth_form_path' => self::FORM_PATH,
5657
'check_path' => self::CHECK_PATH,
58+
'post_only' => self::POST_ONLY,
5759
];
5860

5961
$this->handler = new DefaultAuthenticationRequiredHandler(
@@ -79,6 +81,21 @@ private function stubCurrentPath(string $currentPath): void
7981
});
8082
}
8183

84+
private function stubPostRequest(): void
85+
{
86+
$this->request
87+
->expects($this->any())
88+
->method('isMethod')
89+
->with('POST')
90+
->willReturn(true);
91+
}
92+
93+
private function stubCurrentPathIsCheckPath(): void
94+
{
95+
$this->stubPostRequest();
96+
$this->stubCurrentPath(self::CHECK_PATH);
97+
}
98+
8299
private function assertSaveTargetUrl(string $targetUrl): void
83100
{
84101
$session = $this->createMock(SessionInterface::class);
@@ -158,7 +175,7 @@ public function onAuthenticationRequired_isNotCheckPath_saveRedirectUrl(): void
158175
public function onAuthenticationRequired_isCheckPath_notSaveRedirectUrl(): void
159176
{
160177
$token = $this->createMock(TokenInterface::class);
161-
$this->stubCurrentPath(self::CHECK_PATH);
178+
$this->stubCurrentPathIsCheckPath();
162179

163180
$this->assertNotSaveTargetUrl();
164181
$this->handler->onAuthenticationRequired($this->request, $token);

‎Tests/Security/Http/Firewall/TwoFactorListenerTest.php‎

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ class TwoFactorListenerTest extends TestCase
3737
{
3838
private const FORM_PATH = '/form_path';
3939
private const CHECK_PATH = '/check_path';
40+
private const POST_ONLY = true;
4041
private const AUTH_CODE_PARAM = 'auth_code_param';
4142
private const TRUSTED_PARAM = 'trusted_param';
4243
private const FIREWALL_NAME = 'firewallName';
@@ -164,6 +165,7 @@ protected function setUp(): void
164165
$options = [
165166
'auth_form_path' => self::FORM_PATH,
166167
'check_path' => self::CHECK_PATH,
168+
'post_only' => self::POST_ONLY,
167169
'auth_code_parameter_name' => self::AUTH_CODE_PARAM,
168170
'trusted_parameter_name' => self::TRUSTED_PARAM,
169171
];
@@ -248,6 +250,21 @@ private function stubCurrentPath(string $currentPath): void
248250
});
249251
}
250252

253+
private function stubPostRequest(): void
254+
{
255+
$this->request
256+
->expects($this->any())
257+
->method('isMethod')
258+
->with('POST')
259+
->willReturn(true);
260+
}
261+
262+
private function stubCurrentPathIsCheckPath(): void
263+
{
264+
$this->stubPostRequest();
265+
$this->stubCurrentPath(self::CHECK_PATH);
266+
}
267+
251268
private function stubRequestHasParameter(string $parameterName, $value): void
252269
{
253270
$this->requestParams[$parameterName] = $value;
@@ -447,7 +464,7 @@ public function handle_isCheckPath_authenticateWithAuthenticationManager(): void
447464
$authenticatedToken = $this->createMock(TokenInterface::class);
448465
$twoFactorToken = $this->createTwoFactorToken(self::FIREWALL_NAME, $authenticatedToken);
449466
$this->stubTokenManagerHasToken($twoFactorToken);
450-
$this->stubCurrentPath(self::CHECK_PATH);
467+
$this->stubCurrentPathIsCheckPath();
451468
$this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
452469
$this->stubHandlersReturnResponse();
453470

@@ -473,7 +490,7 @@ public function handle_isCheckPath_authenticateWithAuthenticationManager(): void
473490
public function handle_authenticationException_dispatchFailureEvent(): void
474491
{
475492
$this->stubTokenManagerHasToken($this->createTwoFactorToken());
476-
$this->stubCurrentPath(self::CHECK_PATH);
493+
$this->stubCurrentPathIsCheckPath();
477494
$this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
478495
$this->stubAuthenticationManagerThrowsAuthenticationException();
479496
$this->stubHandlersReturnResponse();
@@ -492,7 +509,7 @@ public function handle_authenticationException_dispatchFailureEvent(): void
492509
public function handle_authenticationException_setResponseFromFailureHandler(): void
493510
{
494511
$this->stubTokenManagerHasToken($this->createTwoFactorToken());
495-
$this->stubCurrentPath(self::CHECK_PATH);
512+
$this->stubCurrentPathIsCheckPath();
496513
$this->stubAuthenticationManagerThrowsAuthenticationException();
497514

498515
$response = $this->createMock(Response::class);
@@ -515,7 +532,7 @@ public function handle_authenticationException_setResponseFromFailureHandler():
515532
public function handle_csrfTokenInvalid_dispatchFailureEvent(): void
516533
{
517534
$this->stubTokenManagerHasToken($this->createTwoFactorToken());
518-
$this->stubCurrentPath(self::CHECK_PATH);
535+
$this->stubCurrentPathIsCheckPath();
519536
$this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsFalse();
520537
$this->stubHandlersReturnResponse();
521538

@@ -531,7 +548,7 @@ public function handle_authenticationStepSuccessful_dispatchSuccessEvent(): void
531548
{
532549
$twoFactorToken = $this->createTwoFactorToken();
533550
$this->stubTokenManagerHasToken($twoFactorToken);
534-
$this->stubCurrentPath(self::CHECK_PATH);
551+
$this->stubCurrentPathIsCheckPath();
535552
$this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
536553
$this->stubAuthenticationManagerReturnsToken($twoFactorToken); // Must be TwoFactorToken
537554

@@ -551,7 +568,7 @@ public function handle_authenticationStepSuccessfulButNotCompleted_redirectToAut
551568
{
552569
$twoFactorToken = $this->createTwoFactorToken();
553570
$this->stubTokenManagerHasToken($twoFactorToken);
554-
$this->stubCurrentPath(self::CHECK_PATH);
571+
$this->stubCurrentPathIsCheckPath();
555572
$this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
556573
$this->stubAuthenticationManagerReturnsToken($twoFactorToken); // Must be TwoFactorToken
557574

@@ -572,7 +589,7 @@ public function handle_authenticationStepSuccessfulButNotCompleted_dispatchRequi
572589
{
573590
$twoFactorToken = $this->createTwoFactorToken();
574591
$this->stubTokenManagerHasToken($twoFactorToken);
575-
$this->stubCurrentPath(self::CHECK_PATH);
592+
$this->stubCurrentPathIsCheckPath();
576593
$this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
577594
$this->stubAuthenticationManagerReturnsToken($twoFactorToken); // Must be TwoFactorToken
578595

@@ -591,7 +608,7 @@ public function handle_authenticationStepSuccessfulButNotCompleted_dispatchRequi
591608
public function handle_twoFactorProcessComplete_returnResponseFromSuccessHandler(): void
592609
{
593610
$this->stubTokenManagerHasToken($this->createTwoFactorToken());
594-
$this->stubCurrentPath(self::CHECK_PATH);
611+
$this->stubCurrentPathIsCheckPath();
595612
$this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
596613
$this->stubAuthenticationManagerReturnsToken($this->createMock(TokenInterface::class)); // Not a TwoFactorToken
597614

@@ -615,7 +632,7 @@ public function handle_twoFactorProcessComplete_returnResponseFromSuccessHandler
615632
public function handle_twoFactorProcessComplete_dispatchCompleteEvent(): void
616633
{
617634
$this->stubTokenManagerHasToken($this->createTwoFactorToken());
618-
$this->stubCurrentPath(self::CHECK_PATH);
635+
$this->stubCurrentPathIsCheckPath();
619636
$this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
620637
$this->stubAuthenticationManagerReturnsToken($this->createMock(TokenInterface::class)); // Not a TwoFactorToken
621638
$this->stubHandlersReturnResponse();
@@ -641,7 +658,7 @@ public function handle_twoFactorProcessCompleteWithTrustedEnabled_setTrustedDevi
641658
->willReturn('user');
642659

643660
$this->stubTokenManagerHasToken($this->createTwoFactorToken());
644-
$this->stubCurrentPath(self::CHECK_PATH);
661+
$this->stubCurrentPathIsCheckPath();
645662
$this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
646663
$this->stubRequestHasParameter(self::TRUSTED_PARAM, '1');
647664
$this->stubAuthenticationManagerReturnsToken($authenticatedToken); // Not a TwoFactorToken
@@ -661,7 +678,7 @@ public function handle_twoFactorProcessCompleteWithTrustedEnabled_setTrustedDevi
661678
public function handle_twoFactorProcessCompleteWithTrustedDisabled_notSetTrustedDevice(): void
662679
{
663680
$this->stubTokenManagerHasToken($this->createTwoFactorToken());
664-
$this->stubCurrentPath(self::CHECK_PATH);
681+
$this->stubCurrentPathIsCheckPath();
665682
$this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
666683
$this->stubRequestHasParameter(self::TRUSTED_PARAM, '0');
667684
$this->stubAuthenticationManagerReturnsToken($this->createMock(TokenInterface::class)); // Not a TwoFactorToken
@@ -690,7 +707,7 @@ public function handle_twoFactorProcessCompleteWithRememberMeEnabled_setRemember
690707
$twoFactorToken = $this->createTwoFactorToken(self::FIREWALL_NAME, null, $attributes);
691708

692709
$this->stubTokenManagerHasToken($twoFactorToken);
693-
$this->stubCurrentPath(self::CHECK_PATH);
710+
$this->stubCurrentPathIsCheckPath();
694711
$this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue();
695712
$this->stubAuthenticationManagerReturnsToken($authenticatedToken); // Not a TwoFactorToken
696713
$response = $this->stubHandlersReturnResponse();

‎Tests/Security/TwoFactor/TwoFactorFirewallConfigTest.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ class TwoFactorFirewallConfigTest extends TestCase
1111
{
1212
private const FULL_OPTIONS = [
1313
'check_path' => 'check_path_route_name',
14+
'post_only' => true,
1415
'auth_form_path' => 'auth_form_path_route_name',
1516
'multi_factor' => true,
1617
'auth_code_parameter_name' => 'auth_code_param',
@@ -123,4 +124,22 @@ public function getCheckPath_optionNotSet_returnDefault(): void
123124
$returnValue = $this->createConfig([])->getCheckPath();
124125
$this->assertEquals('/2fa_check', $returnValue);
125126
}
127+
128+
/**
129+
* @test
130+
*/
131+
public function isPostOnly_optionSet_returnThatValue(): void
132+
{
133+
$returnValue = $this->createConfig()->isPostOnly();
134+
$this->assertTrue($returnValue);
135+
}
136+
137+
/**
138+
* @test
139+
*/
140+
public function isPostOnly_optionNotSet_returnDefault(): void
141+
{
142+
$returnValue = $this->createConfig([])->isPostOnly();
143+
$this->assertFalse($returnValue);
144+
}
126145
}

0 commit comments

Comments
 (0)