Skip to content

Commit e9cf8e6

Browse files
committed
bug #40330 [SecurityBundle] Empty line starting with dash under "access_control" causes all rules to be skipped (monteiro)
This PR was squashed before being merged into the 4.4 branch. Discussion ---------- [SecurityBundle] Empty line starting with dash under "access_control" causes all rules to be skipped | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | Fix #40235 ... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead --> | License | MIT When the IDE by mistake puts an empty line in `access_control` in security.yaml there is no warning that we have an empty row, making the rest of routes defined, to be ignored and possible to be accessed by anyone that can authenticate no matter the role. # How to reproduce the issue - git clone [email protected]:monteiro/symfony-issue-40235.git - composer install - symfony server:start - open 127.0.0.1:8000/admin with username: "john_user" and password "123456" - Since that user has only ROLE_USER should not be able to access the route... but because there is an empty line in "access_control" in `security.yaml`, "by mistake" it is possible to access the protected `ROLE_ADMIN` route. Commits ------- ee26ce5987 [SecurityBundle] Empty line starting with dash under "access_control" causes all rules to be skipped
2 parents 4f0f283 + d2fbe24 commit e9cf8e6

File tree

2 files changed

+56
-0
lines changed

2 files changed

+56
-0
lines changed

DependencyInjection/SecurityExtension.php

+6
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,12 @@ private function createAuthorization(array $config, ContainerBuilder $container)
191191
$attributes[] = $this->createExpression($container, $access['allow_if']);
192192
}
193193

194+
$emptyAccess = 0 === \count(array_filter($access));
195+
196+
if ($emptyAccess) {
197+
throw new InvalidConfigurationException('One or more access control items are empty. Did you accidentally add lines only containing a "-" under "security.access_control"?');
198+
}
199+
194200
$container->getDefinition('security.access_map')
195201
->addMethodCall('add', [$matcher, $attributes, $access['requires_channel']]);
196202
}

Tests/DependencyInjection/SecurityExtensionTest.php

+50
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,56 @@ public function testSwitchUserWithSeveralDefinedProvidersButNoFirewallRootProvid
415415
$this->assertEquals(new Reference('security.user.provider.concrete.second'), $container->getDefinition('security.authentication.switchuser_listener.foobar')->getArgument(1));
416416
}
417417

418+
public function testInvalidAccessControlWithEmptyRow()
419+
{
420+
$container = $this->getRawContainer();
421+
422+
$container->loadFromExtension('security', [
423+
'providers' => [
424+
'default' => ['id' => 'foo'],
425+
],
426+
'firewalls' => [
427+
'some_firewall' => [
428+
'pattern' => '/.*',
429+
'http_basic' => [],
430+
],
431+
],
432+
'access_control' => [
433+
[],
434+
['path' => '/admin', 'roles' => 'ROLE_ADMIN'],
435+
],
436+
]);
437+
438+
$this->expectException(InvalidConfigurationException::class);
439+
$this->expectExceptionMessage('One or more access control items are empty. Did you accidentally add lines only containing a "-" under "security.access_control"?');
440+
$container->compile();
441+
}
442+
443+
public function testValidAccessControlWithEmptyRow()
444+
{
445+
$container = $this->getRawContainer();
446+
447+
$container->loadFromExtension('security', [
448+
'providers' => [
449+
'default' => ['id' => 'foo'],
450+
],
451+
'firewalls' => [
452+
'some_firewall' => [
453+
'pattern' => '/.*',
454+
'http_basic' => [],
455+
],
456+
],
457+
'access_control' => [
458+
['path' => '^/login'],
459+
['path' => '^/', 'roles' => 'ROLE_USER'],
460+
],
461+
]);
462+
463+
$container->compile();
464+
465+
$this->assertTrue(true, 'extension throws an InvalidConfigurationException if there is one more more empty access control items');
466+
}
467+
418468
protected function getRawContainer()
419469
{
420470
$container = new ContainerBuilder();

0 commit comments

Comments
 (0)