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

Allow to overwrite security config #24461

Closed
ro0NL opened this Issue Oct 6, 2017 · 5 comments

Comments

Projects
None yet
4 participants
@ro0NL
Contributor

ro0NL commented Oct 6, 2017

Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? no
Symfony version 4.1

Recall for #22308 :)

# security.yaml
firewalls:
    # ....
    api:
        pattern: ^/api
        stateless: true
        anonymous: true
    # ...
# bundle extension
public function prepend(ContainerBuilder $container)
{
    $security = $container->getExtensionConfig('security');
    $security[0]['firewalls']['api']['anonymous'] = false;
    $security[0]['firewalls']['api']['fos_oauth'] = true;

    $container->loadFromExtension('security', $security[0]);
}

Result

Configuration path "security.access_control" cannot be overwritten. You have to define all options for this path, and any of its sub-paths in one configuration section.

I understand loadFromExtension adds a new config branch ($configs[] = $config). But im trying to fake 1 config branch, so i guess i need reflection to clear current branch first.

Can we create API for that? Or what about allowing overwrites on existing elements?

Thx!

@ro0NL

This comment has been minimized.

Show comment
Hide comment
@ro0NL

ro0NL Oct 6, 2017

Contributor

Here's the patch for future reference :)

// magic happens
// https://github.com/symfony/symfony/issues/24461
$extensionConfigsRefl = new \ReflectionProperty(ContainerBuilder::class, 'extensionConfigs');
$extensionConfigsRefl->setAccessible(true);
$extensionConfigs = $extensionConfigsRefl->getValue($container);
$extensionConfigs['security'][0]['firewalls']['api']['anonymous'] = false;
$extensionConfigs['security'][0]['firewalls']['api']['fos_oauth'] = true;
$extensionConfigsRefl->setValue($container, $extensionConfigs);
Contributor

ro0NL commented Oct 6, 2017

Here's the patch for future reference :)

// magic happens
// https://github.com/symfony/symfony/issues/24461
$extensionConfigsRefl = new \ReflectionProperty(ContainerBuilder::class, 'extensionConfigs');
$extensionConfigsRefl->setAccessible(true);
$extensionConfigs = $extensionConfigsRefl->getValue($container);
$extensionConfigs['security'][0]['firewalls']['api']['anonymous'] = false;
$extensionConfigs['security'][0]['firewalls']['api']['fos_oauth'] = true;
$extensionConfigsRefl->setValue($container, $extensionConfigs);
@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas

nicolas-grekas Nov 26, 2017

Member

Broader duplicate of #25021?

Member

nicolas-grekas commented Nov 26, 2017

Broader duplicate of #25021?

@ro0NL

This comment has been minimized.

Show comment
Hide comment
@ro0NL

ro0NL Nov 26, 2017

Contributor

Guess so

I believe 2 things are going on;

  • You are not allowed to define new elements for path "security.firewalls". (#22308, #25021?)
  • Configuration path "security.access_control" cannot be overwritten. (this PR)

i know this usecase really annoyed me, but me might need a PR addressing both.

Contributor

ro0NL commented Nov 26, 2017

Guess so

I believe 2 things are going on;

  • You are not allowed to define new elements for path "security.firewalls". (#22308, #25021?)
  • Configuration path "security.access_control" cannot be overwritten. (this PR)

i know this usecase really annoyed me, but me might need a PR addressing both.

@ro0NL

This comment has been minimized.

Show comment
Hide comment
@ro0NL

ro0NL Nov 30, 2017

Contributor

Ok, i made a mistake. Existing firewalls can be overwritten in fact :-)

New patch is

$container->loadFromExtension('security', [
    'firewalls' => [
        'api' => [
            'anonymous' => false,
            'fos_oauth' => true,
        ],
    ],
]);

The exception for security.access_control is not the one blocking here. But someone else might try it in the future :)

Solved for me. Lets focus on #25021 👍

Contributor

ro0NL commented Nov 30, 2017

Ok, i made a mistake. Existing firewalls can be overwritten in fact :-)

New patch is

$container->loadFromExtension('security', [
    'firewalls' => [
        'api' => [
            'anonymous' => false,
            'fos_oauth' => true,
        ],
    ],
]);

The exception for security.access_control is not the one blocking here. But someone else might try it in the future :)

Solved for me. Lets focus on #25021 👍

@ro0NL ro0NL closed this Nov 30, 2017

@xxorax

This comment has been minimized.

Show comment
Hide comment
@xxorax

xxorax Jan 7, 2018

loadFromExtension does not work for if there is already defined firewalls.

Also, because the firewalls rules order is important, you should take care to insert firewall rule at the good place:

$extensionConfigsRefl = new \ReflectionProperty(ContainerBuilder::class, 'extensionConfigs');
$extensionConfigsRefl->setAccessible(true);
$extensionConfigs = $extensionConfigsRefl->getValue($container);
$extensionConfigs['security'][0]['firewalls'] = array_merge(
    ['status' => [
        'pattern' => '^/status',
        'security' => false, 
    ]],
    $extensionConfigs['security'][0]['firewalls']
);

$extensionConfigsRefl->setValue($container, $extensionConfigs);

Still waiting for a better way to fix it...

xxorax commented Jan 7, 2018

loadFromExtension does not work for if there is already defined firewalls.

Also, because the firewalls rules order is important, you should take care to insert firewall rule at the good place:

$extensionConfigsRefl = new \ReflectionProperty(ContainerBuilder::class, 'extensionConfigs');
$extensionConfigsRefl->setAccessible(true);
$extensionConfigs = $extensionConfigsRefl->getValue($container);
$extensionConfigs['security'][0]['firewalls'] = array_merge(
    ['status' => [
        'pattern' => '^/status',
        'security' => false, 
    ]],
    $extensionConfigs['security'][0]['firewalls']
);

$extensionConfigsRefl->setValue($container, $extensionConfigs);

Still waiting for a better way to fix it...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment