NelmioSecurityBundle clickjacking paths settings could not be overwritten

[koertho]

Description

Today I wanted to set some routes to x-frame-option: allow to allow iframe inclusion. I tried to set the corresponding settings in my project config, but it always give me x-frame-options: sameorigin. After a lot of searching and debugging I found the reason: the settings from this bundle are alway the first in the nelmio_security.clickjacking.paths parameter and my project settings are afterwards. But in NelmioSecurityBundle it uses alway the first match, so it always gives sameorigin. It is not possible to change the settings or unset them from the config. I needed to write a compiler pass to bypass this issue. While this is a good base to start a new extension, it would be great to have the option to simply set this from the config files.

Steps to reproduce:

1. Create a new contao managed edition project.
2. Create a page you want to have x-frame-origin: allow and give it a route starting with /extern
3. add this to your config.yml (maybe you need to import it to your config_prod.yml/config_dev.yml):

   nelmio_security:
   clickjacking:
   paths:
     '^/extern/.*': ALLOW
     '^/.*': DENY

   Bottom line

   Maybe this is an issue due the handling of the treebuilder of symfony, that keeps the order of the config error, even if overwritten in a later config file (like in my reprodiction example). Just wanted to let you know, maybe you have an idea to bypass this issue.

[aschempp]

A compiler pass is not necessary a bad option. What you also can do is implement the ExtensionPluginInterface in your manager plugin. The plugin will then receive each extension config and be able to modify it.

[leofeyer]

That's strange, because it should work using a config.yml file. I know at least one client who adjusted his installation this way.

[aschempp]

It‘s an array merge, so the paths are added. And if the first matches (because its a catch-all)…

[Toflar]

The problem is that the Managed Edition provides certain settings by default and for some configurations in Symfony, the order matters. In the Symfony "core" this applies to security.firewalls. The first one that matches always wins. It also applies here, the first nelmio_security.clickjacking.paths entry that matches, wins. So merging doesn't work here, because it would lead to arbitrary ordering of entries which would be especially dangerous for security firewalls. So it wouldn't work in a plain old Symfony edition either. But there you are in charge of updating the config.yml yourself, so you don't have a managed edition.

You have two choices:

• Either don't use the Contao Managed Edition because you have specific requirements and you might feel more comfortable updating the configuration, AppKernel etc. yourself.
• Or use the Managed Edition which means you need to understand how the manager-bundle works and how you can use plugins to adapt the stuff. Here's the manager plugin you need to achieve what you want:

<?php

use Contao\ManagerPlugin\Config\ContainerBuilder;
use Contao\ManagerPlugin\Config\ExtensionPluginInterface;
use Contao\ManagerPlugin\Routing\RoutingPluginInterface;
use Symfony\Component\Config\Loader\LoaderResolverInterface;
use Symfony\Component\HttpKernel\KernelInterface;

class ContaoManagerPlugin implements RoutingPluginInterface, ExtensionPluginInterface
{

    /**
     * Returns a collection of routes for this bundle.
     *
     * @param LoaderResolverInterface $resolver
     * @param KernelInterface         $kernel
     *
     * @return null|RouteCollection
     */
    public function getRouteCollection(LoaderResolverInterface $resolver, KernelInterface $kernel)
    {
        return $resolver
            ->resolve(__DIR__ . '/routing.yml')
            ->load(__DIR__ . '/routing.yml')
        ;
    }

    /**
     * Allows a plugin to override extension configuration.
     *
     * @param string           $extensionName
     * @param array            $extensionConfigs
     * @param ContainerBuilder $container
     *
     * @return array
     */
    public function getExtensionConfig($extensionName, array $extensionConfigs, ContainerBuilder $container)
    {
        if ('nelmio_security' === $extensionName) {

            $customCors = [
                '^/extern.*' => 'ALLOW'
            ];

            foreach ($extensionConfigs as &$extensionConfig) {

                if (isset($extensionConfig['clickjacking'])
                    && is_array($extensionConfig['clickjacking']['paths'])
                ) {
                    $extensionConfig['clickjacking']['paths'] = $customCors + $extensionConfig['clickjacking']['paths'];
                }
            }
        }

        return $extensionConfigs;
    }
}

[leofeyer]

We should probably set the paths dynamically depending on a route attribute?

[koertho]

@Toflar The ExtensionPluginInterface is completly undocumented, so I didn't know that. I used a CompilerPass instead and did basically the same. I just think this this should be configurable over config.yml or the backend (I prefer config.yml, because it's a configuration thing that shouldn't change that often and it's symfony practice).

[Toflar]

@Toflar The ExtensionPluginInterface is completly undocumented,

You could've done that :) I did in the meantime: https://github.com/contao/docs/pull/468

[koertho]

@Toflar not so easy, if you don't know it even exists ;) but thank for changing this :+1: