Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[SecurityBundle] Add a FirewallConfigRegistry service as main entry point #20591

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Bundle\SecurityBundle\DataCollector;

use Symfony\Bundle\SecurityBundle\Security\FirewallConfigRegistry;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Role\RoleHierarchyInterface;
use Symfony\Component\HttpFoundation\Request;
Expand All @@ -21,8 +22,6 @@
use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
use Symfony\Component\Security\Core\Authorization\DebugAccessDecisionManager;
use Symfony\Component\VarDumper\Cloner\Data;
use Symfony\Component\Security\Http\FirewallMapInterface;
use Symfony\Bundle\SecurityBundle\Security\FirewallMap;

/**
* SecurityDataCollector.
Expand All @@ -35,7 +34,7 @@ class SecurityDataCollector extends DataCollector
private $roleHierarchy;
private $logoutUrlGenerator;
private $accessDecisionManager;
private $firewallMap;
private $firewallConfigRegistry;

/**
* Constructor.
Expand All @@ -44,15 +43,15 @@ class SecurityDataCollector extends DataCollector
* @param RoleHierarchyInterface|null $roleHierarchy
* @param LogoutUrlGenerator|null $logoutUrlGenerator
* @param AccessDecisionManagerInterface|null $accessDecisionManager
* @param FirewallMapInterface|null $firewallMap
* @param FirewallConfigRegistry|null $firewallConfigRegistry
*/
public function __construct(TokenStorageInterface $tokenStorage = null, RoleHierarchyInterface $roleHierarchy = null, LogoutUrlGenerator $logoutUrlGenerator = null, AccessDecisionManagerInterface $accessDecisionManager = null, FirewallMapInterface $firewallMap = null)
public function __construct(TokenStorageInterface $tokenStorage = null, RoleHierarchyInterface $roleHierarchy = null, LogoutUrlGenerator $logoutUrlGenerator = null, AccessDecisionManagerInterface $accessDecisionManager = null, FirewallConfigRegistry $firewallConfigRegistry = null)
{
$this->tokenStorage = $tokenStorage;
$this->roleHierarchy = $roleHierarchy;
$this->logoutUrlGenerator = $logoutUrlGenerator;
$this->accessDecisionManager = $accessDecisionManager;
$this->firewallMap = $firewallMap;
$this->firewallConfigRegistry = $firewallConfigRegistry;
}

/**
Expand Down Expand Up @@ -140,9 +139,8 @@ public function collect(Request $request, Response $response, \Exception $except

// collect firewall context information
$this->data['firewall'] = null;
if ($this->firewallMap instanceof FirewallMap) {
$firewallConfig = $this->firewallMap->getFirewallConfig($request);
if (null !== $firewallConfig) {
if (null !== $this->firewallConfigRegistry) {
if (null !== $firewallConfig = $this->firewallConfigRegistry->fromRequest($request)) {
$this->data['firewall'] = array(
'name' => $firewallConfig->getName(),
'allows_anonymous' => $firewallConfig->allowsAnonymous(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,10 @@ private function createFirewalls($config, ContainerBuilder $container)
// load firewall map
$mapDef = $container->getDefinition('security.firewall.map');
$map = $authenticationProviders = array();
$firewallConfigs = array();
$matchers = array();
foreach ($firewalls as $name => $firewall) {
$configId = 'security.firewall.map.config.'.$name;
$configId = 'security.firewall.config.'.$name;

list($matcher, $listeners, $exceptionListener) = $this->createFirewall($container, $name, $firewall, $authenticationProviders, $providerIds, $configId);

Expand All @@ -246,10 +248,11 @@ private function createFirewalls($config, ContainerBuilder $container)
$context
->replaceArgument(0, $listeners)
->replaceArgument(1, $exceptionListener)
->replaceArgument(2, new Reference($configId))
;

$map[$contextId] = $matcher;
$matchers[$name] = $matcher;
$firewallConfigs[] = new Reference($configId);
}
$mapDef->replaceArgument(1, $map);

Expand All @@ -261,6 +264,11 @@ private function createFirewalls($config, ContainerBuilder $container)
->getDefinition('security.authentication.manager')
->replaceArgument(0, $authenticationProviders)
;

$container->getDefinition('security.firewall.config_registry')
->replaceArgument(0, $firewallConfigs)
->replaceArgument(1, $matchers)
;
}

private function createFirewall(ContainerBuilder $container, $id, $firewall, &$authenticationProviders, $providerIds, $configId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<argument type="service" id="security.role_hierarchy" />
<argument type="service" id="security.logout_url_generator" />
<argument type="service" id="security.access.decision_manager" />
<argument type="service" id="security.firewall.map" />
<argument type="service" id="security.firewall.config_registry" />
</service>
</services>
</container>
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@
<argument type="service" id="event_dispatcher" />
</service>

<service id="security.firewall.config_registry" class="Symfony\Bundle\SecurityBundle\Security\FirewallConfigRegistry">
<argument type="collection" /> <!-- Collection of FirewallConfig -->
<argument type="collection" /> <!-- Collection of RequestMatcher indexed by firewall name -->
<argument type="service" id="request_stack" on-invalid="null" />
</service>

<service id="security.firewall.map" class="Symfony\Bundle\SecurityBundle\Security\FirewallMap" public="false">
<argument type="service" id="service_container" />
<argument type="collection" />
Expand All @@ -111,7 +117,6 @@
<service id="security.firewall.context" class="Symfony\Bundle\SecurityBundle\Security\FirewallContext" abstract="true">
<argument type="collection" />
<argument type="service" id="security.exception_listener" />
<argument /> <!-- FirewallConfig -->
</service>

<service id="security.firewall.config" class="Symfony\Bundle\SecurityBundle\Security\FirewallConfig" abstract="true" public="false">
Expand Down
119 changes: 119 additions & 0 deletions 119 src/Symfony/Bundle/SecurityBundle/Security/FirewallConfigRegistry.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\SecurityBundle\Security;

use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestMatcher;
use Symfony\Component\HttpFoundation\RequestStack;

/**
* Stores firewall config objects.
*
* @author Maxime Steinhausser <maxime.steinhausser@gmail.com>
*/
class FirewallConfigRegistry
{
private $firewallConfigs;
private $requestStack;
private $requestMatchers;

/**
* @param FirewallConfig[] $firewallConfigs
* @param RequestMatcher[] $requestMatchers Indexed by firewall name
* @param RequestStack|null $requestStack To get the current firewall config
*/
public function __construct(array $firewallConfigs = array(), array $requestMatchers = array(), RequestStack $requestStack = null)
{
$this->firewallConfigs = $firewallConfigs;
$this->requestStack = $requestStack;
$this->requestMatchers = $requestMatchers;
}

/**
* @param string $name The firewall name
*
* @return FirewallConfig|null
*/
public function get($name)
{
foreach ($this->firewallConfigs as $config) {
if ($config->getName() === $name) {
return $config;
}
}
}

/**
* @return FirewallConfig|null
*/
public function current()
{
if (!$this->requestStack) {
throw new \LogicException('Unable to get current firewall config without a RequestStack.');
}

return $this->fromRequest($this->requestStack->getCurrentRequest());
}

/**
* @param Request $request
*
* @return FirewallConfig|null
*/
public function fromRequest(Request $request)
{
foreach ($this->firewallConfigs as $config) {
$requestMatcher = $this->getRequestMatcher($config);
if (null === $requestMatcher || $requestMatcher->matches($request)) {
return $config;
}
}
}

/**
* @return FirewallConfig[]
*/
public function all()
{
return $this->firewallConfigs;
}

/**
* @return FirewallConfig[]
*/
public function inContext($context)
{
return array_filter($this->firewallConfigs, function (FirewallConfig $config) use ($context) {
return $context === $config->getContext();
});
}

/**
* @param FirewallConfig $config
*
* @return RequestMatcher|null
*/
private function getRequestMatcher(FirewallConfig $config)
{
if (empty($config->getRequestMatcher())) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this would be replaced by if (null === $config->getRequestMatcher()) { if #20589 is merged

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case of this one is considered before #20589, I think it should be if (!$config->getRequestMatcher())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer empty which is semantically better IMHO.

return;
}

$firewallName = $config->getName();

if (!isset($this->requestMatchers[$firewallName])) {
throw new \LogicException(sprintf('Request matcher not found for "%s" firewall.', $firewallName));
}

return $this->requestMatchers[$firewallName];
}
}
13 changes: 1 addition & 12 deletions 13 src/Symfony/Bundle/SecurityBundle/Security/FirewallContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,11 @@ class FirewallContext
{
private $listeners;
private $exceptionListener;
private $config;

public function __construct(array $listeners, ExceptionListener $exceptionListener = null, FirewallConfig $config = null)
public function __construct(array $listeners, ExceptionListener $exceptionListener = null)
{
if (null === $config) {
@trigger_error(sprintf('"%s()" expects an instance of "%s" as third argument since version 3.2 and will trigger an error in 4.0 if not provided.', __METHOD__, FirewallConfig::class), E_USER_DEPRECATED);
}

$this->listeners = $listeners;
$this->exceptionListener = $exceptionListener;
$this->config = $config;
}

public function getConfig()
{
return $this->config;
}

public function getContext()
Expand Down
14 changes: 0 additions & 14 deletions 14 src/Symfony/Bundle/SecurityBundle/Security/FirewallMap.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,6 @@ public function getListeners(Request $request)
return $context->getContext();
}

/**
* @return FirewallConfig|null
*/
public function getFirewallConfig(Request $request)
{
$context = $this->getFirewallContext($request);

if (null === $context) {
return;
}

return $context->getConfig();
}

private function getFirewallContext(Request $request)
{
if ($this->contexts->contains($request)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,11 @@

use Symfony\Bundle\SecurityBundle\DataCollector\SecurityDataCollector;
use Symfony\Bundle\SecurityBundle\Security\FirewallConfig;
use Symfony\Bundle\SecurityBundle\Security\FirewallMap;
use Symfony\Bundle\SecurityBundle\Security\FirewallConfigRegistry;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
use Symfony\Component\Security\Core\Role\Role;
use Symfony\Component\Security\Core\Role\RoleHierarchy;
use Symfony\Component\Security\Http\FirewallMapInterface;

class SecurityDataCollectorTest extends \PHPUnit_Framework_TestCase
{
Expand Down Expand Up @@ -81,17 +80,17 @@ public function testGetFirewall()
$firewallConfig = new FirewallConfig('dummy', 'security.request_matcher.dummy', 'security.user_checker.dummy');
$request = $this->getRequest();

$firewallMap = $this
->getMockBuilder(FirewallMap::class)
$registry = $this
->getMockBuilder(FirewallConfigRegistry::class)
->disableOriginalConstructor()
->getMock();
$firewallMap
$registry
->expects($this->once())
->method('getFirewallConfig')
->method('fromRequest')
->with($request)
->willReturn($firewallConfig);

$collector = new SecurityDataCollector(null, null, null, null, $firewallMap);
$collector = new SecurityDataCollector(null, null, null, null, $registry);
$collector->collect($request, $this->getResponse());
$collected = $collector->getFirewall();

Expand Down Expand Up @@ -119,23 +118,13 @@ public function testGetFirewallReturnsNull()
$collector->collect($request, $response);
$this->assertNull($collector->getFirewall());

// Inject an instance that is not context aware
$firewallMap = $this
->getMockBuilder(FirewallMapInterface::class)
->disableOriginalConstructor()
->getMock();

$collector = new SecurityDataCollector(null, null, null, null, $firewallMap);
$collector->collect($request, $response);
$this->assertNull($collector->getFirewall());

// Null config
$firewallMap = $this
->getMockBuilder(FirewallMap::class)
$registry = $this
->getMockBuilder(FirewallConfigRegistry::class)
->disableOriginalConstructor()
->getMock();

$collector = new SecurityDataCollector(null, null, null, null, $firewallMap);
$collector = new SecurityDataCollector(null, null, null, null, $registry);
$collector->collect($request, $response);
$this->assertNull($collector->getFirewall());
}
Expand Down
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.