Merge pull request #109 from romainneutron/cached-ua-parser

Decouple UA analyzer and parser to allow caching
nelmio · Aug 24, 2016 · 2eda4c5 · 2eda4c5
2 parents b84e1d1 + 57e586b
commit 2eda4c5
Show file tree

Hide file tree

Showing 16 changed files with 284 additions and 20 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,7 @@
 ### 2.0.2 (2016-xx-xx)
   * Fix typo in the ALLOW-FROM implementation
+  * Update browser_adaptive configuration. Allow custom adapters
+  * Add Doctrine Cache and Psr Cache adapters for caching UA family parser
 
 ### 2.0.1 (2016-06-04)
   * Fix CookieSessionHandler::open that should return true unless there's an error

diff --git a/DependencyInjection/Compiler/UAParserCompilerPass.php b/DependencyInjection/Compiler/UAParserCompilerPass.php
@@ -0,0 +1,20 @@
+<?php
+
+namespace Nelmio\SecurityBundle\DependencyInjection\Compiler;
+
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+
+class UAParserCompilerPass implements CompilerPassInterface
+{
+    public function process(ContainerBuilder $container)
+    {
+        if (!$container->hasParameter('nelmio_browser_adaptive_parser')) {
+            return;
+        }
+
+        $container
+            ->getDefinition('nelmio_security.ua_parser')
+            ->setArguments(array($container->getDefinition($container->getParameter('nelmio_browser_adaptive_parser'))));
+    }
+}
diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -210,9 +210,29 @@ private function addReportOrEnforceNode($reportOrEnforce)
             ->end();
 
         $children
-            ->booleanNode('browser_adaptive')
+            ->arrayNode('browser_adaptive')
+                ->canBeEnabled()
                 ->info('Do not send directives that browser do not support')
-                ->defaultValue(false)
+                ->addDefaultsIfNotSet()
+                ->children()
+                    ->scalarNode('parser')
+                        ->defaultValue('nelmio_security.ua_parser.ua_php')
+                    ->end()
+                ->end()
+                ->beforeNormalization()
+                    ->always(function ($v) {
+                        if (!is_array($v)) {
+                            @trigger_error("browser_adaptive configuration is now an array. Using boolean is deprecated and will not be supported anymore in version 3", E_USER_DEPRECATED);
+
+                            return array(
+                                'enabled' => $v,
+                                'parser' => 'nelmio_security.ua_parser.ua_php',
+                            );
+                        }
+
+                        return $v;
+                    })
+                ->end()
             ->end();
 
         foreach (DirectiveSet::getNames() as $name => $type) {

diff --git a/DependencyInjection/NelmioSecurityExtension.php b/DependencyInjection/NelmioSecurityExtension.php
@@ -157,14 +157,19 @@ private function buildDirectiveSetDefinition(ContainerBuilder $container, $confi
 
         $pmDefinition = $container->getDefinition('nelmio_security.policy_manager');
 
-        if (isset($config[$type]) && $config[$type]['browser_adaptive']) {
-            $service = $container->getParameter('nelmio_security.ua_parser.service');
+        if (isset($config[$type]) && $config[$type]['browser_adaptive']['enabled']) {
+            $service = $config[$type]['browser_adaptive']['parser'];
 
             if ($service === 'nelmio_security.ua_parser.ua_php' && !class_exists('UAParser\Parser')) {
                 throw new \RuntimeException('You must require "ua-parser/uap-php" as a dependency to use the browser_adaptive feature or configure your own "nelmio_security.ua_parser.service"');
             }
 
-            $pmDefinition->setArguments(array($container->getDefinition($service)));
+            $container->setParameter('nelmio_browser_adaptive_parser', $service);
+
+            $uaParser = $container->getDefinition('nelmio_security.ua_parser');
+            $uaParser->setArguments(array($container->getDefinition('nelmio_security.ua_parser.ua_php')));
+
+            $pmDefinition->setArguments(array($uaParser));
         }
 
         $directiveDefinition->setArguments(array($pmDefinition, $config, $type));

diff --git a/NelmioSecurityBundle.php b/NelmioSecurityBundle.php
@@ -11,6 +11,7 @@
 
 namespace Nelmio\SecurityBundle;
 
+use Nelmio\SecurityBundle\DependencyInjection\Compiler\UAParserCompilerPass;
 use Symfony\Component\HttpKernel\Bundle\Bundle;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Nelmio\SecurityBundle\DependencyInjection\Compiler\CSPTwigCompilerPass;
@@ -22,5 +23,6 @@ public function build(ContainerBuilder $container)
         parent::build($container);
 
         $container->addCompilerPass(new CSPTwigCompilerPass());
+        $container->addCompilerPass(new UAParserCompilerPass());
     }
 }
diff --git a/README.md b/README.md
@@ -93,7 +93,8 @@ nelmio_security:
         content_types: []
         enforce:
             level1_fallback: false
-            browser_adaptive: false
+            browser_adaptive:
+                enabled: false
             report-uri: %router.request_context.base_url%/nelmio/csp/report
             default-src:
                 - 'none'
@@ -168,7 +169,8 @@ nelmio_security:
             level1_fallback: true
             # Only send directives supported by the browser, defaults to false
             # This is a port of https://github.com/twitter/secureheaders/blob/83a564a235c8be1a8a3901373dbc769da32f6ed7/lib/secure_headers/headers/policy_management.rb#L97
-            browser_adaptive: false
+            browser_adaptive:
+                enabled: false
             report-uri: %router.request_context.base_url%/nelmio/csp/report
             default-src: [ 'self' ]
             frame-src: [ 'https://www.youtube.com' ]
@@ -186,7 +188,8 @@ nelmio_security:
             level1_fallback: true
             # Only send directives supported by the browser, defaults to false
             # This is a port of https://github.com/twitter/secureheaders/blob/83a564a235c8be1a8a3901373dbc769da32f6ed7/lib/secure_headers/headers/policy_management.rb#L97
-            browser_adaptive: true
+            browser_adaptive:
+                enabled: true
             report-uri: %router.request_context.base_url%/nelmio/csp/report
             script-src:
                 - 'self'
@@ -231,6 +234,46 @@ nelmio_security:
         compat_headers: false
 ```
 
+#### Using browser adaptive directives
+
+Nelmio can only send directives that can be understood by the browser. This reduces noise provided via the report URI.
+This is a direct port of what has been done in [Twitter SecureHeaders library](https://github.com/twitter/secureheaders).
+
+Use the `enabled` key to enable it.
+
+```yaml
+nelmio_security:
+    csp:
+        enforce:
+            browser_adaptive:
+                enabled: true
+```
+
+**WARNING** This will parse the user agent and can consume some CPU usage. You can specify a cached parser to
+avoid consumong to much CPU usage:
+
+```yaml
+nelmio_security:
+    csp:
+        enforce:
+            browser_adaptive:
+                enabled: true
+                parser: my_own_parser
+```
+
+And declare service `my_ow_parser` based on one of the cached parser NelmioSecurityBundle provides or your own one.
+For instance, using the `DoctrineCacheUAFamilyParser`:
+
+```xml
+    <service id="my_own_parser" class="Nelmio\SecurityBundle\UserAgent\UAFamilyParser\DoctrineCacheUAFamilyParser">
+      <argument type="service" id="doctrine_cache.providers.redis_cache"/>
+      <argument type="service" id="nelmio_security.ua_parser.ua_php"/>
+      <argument>604800</argument>
+    </service>
+```xml
+
+Have a look in the `Nelmio\SecurityBundle\UserAgent\UAFamilyParser` for these parsers.
+
 #### Message digest for inline script handling
 
 If you want to disable `'unsafe-inline'` on `script-src` or `style-src` (recommended), Nelmio Security Bundle

diff --git a/Resources/config/csp.yml b/Resources/config/csp.yml
@@ -1,11 +1,14 @@
 parameters:
     nelmio_security.nonce_generator.number_of_bytes: 16
-    nelmio_security.ua_parser.service: nelmio_security.ua_parser.ua_php
 
 services:
-    nelmio_security.ua_parser.ua_php:
-        class: Nelmio\SecurityBundle\UserAgent\UAParserUserAgentParser
+    nelmio_security.ua_parser:
+        class: Nelmio\SecurityBundle\UserAgent\UserAgentParser
         public: false
+
+    nelmio_security.ua_parser.ua_php:
+        class: Nelmio\SecurityBundle\UserAgent\UAFamilyParser\UAFamilyParser
+        public: true
         arguments: ['@nelmio_security.ua_parser.ua_php.provider']
 
     nelmio_security.ua_parser.ua_php.provider:

diff --git a/Resources/config/csp_legacy.yml b/Resources/config/csp_legacy.yml
@@ -1,11 +1,14 @@
 parameters:
     nelmio_security.nonce_generator.number_of_bytes: 16
-    nelmio_security.ua_parser.service: nelmio_security.ua_parser.ua_php
 
 services:
-    nelmio_security.ua_parser.ua_php:
-        class: Nelmio\SecurityBundle\UserAgent\UAParserUserAgentParser
+    nelmio_security.ua_parser:
+        class: Nelmio\SecurityBundle\UserAgent\UserAgentParser
         public: false
+
+    nelmio_security.ua_parser.ua_php:
+        class: Nelmio\SecurityBundle\UserAgent\UAFamilyParser\UAFamilyParser
+        public: true
         arguments: ['@nelmio_security.ua_parser.ua_php.provider']
 
     nelmio_security.ua_parser.ua_php.provider:

diff --git a/Tests/ContentSecurityPolicy/DirectiveSetTest.php b/Tests/ContentSecurityPolicy/DirectiveSetTest.php
@@ -4,7 +4,8 @@
 
 use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSet;
 use Nelmio\SecurityBundle\ContentSecurityPolicy\PolicyManager;
-use Nelmio\SecurityBundle\UserAgent\UAParserUserAgentParser;
+use Nelmio\SecurityBundle\UserAgent\UAFamilyParser\UAFamilyParser;
+use Nelmio\SecurityBundle\UserAgent\UserAgentParser;
 use Symfony\Component\HttpFoundation\Request;
 use UAParser\Parser;
 
@@ -30,7 +31,7 @@ public function testFromConfig($expected, $ua, array $directives)
 
     private function createPolicyManager()
     {
-        return new PolicyManager(new UAParserUserAgentParser(Parser::create()));
+        return new PolicyManager(new UserAgentParser(new UAFamilyParser(Parser::create())));
     }
 
     public function provideVariousConfig()

diff --git a/Tests/DependencyInjection/ConfigurationTest.php b/Tests/DependencyInjection/ConfigurationTest.php
@@ -63,6 +63,30 @@ public function testCspWithLevel2()
         );
     }
 
+    public function testBrowserAdaptiveBoolean()
+    {
+        $this->processYamlConfiguration(
+            "csp:\n".
+            "  report:\n".
+            "    script-src:\n".
+            "      - 'self'\n".
+            "    browser_adaptive: true\n"
+        );
+    }
+
+    public function testBrowserAdaptiveArray()
+    {
+        $this->processYamlConfiguration(
+            "csp:\n".
+            "  report:\n".
+            "    script-src:\n".
+            "      - 'self'\n".
+            "    browser_adaptive:\n".
+            "      enabled: true\n".
+            "      parser: service_name"
+        );
+    }
+
     private function processYamlConfiguration($config)
     {
         $parser = new Parser();

diff --git a/UserAgent/UAFamilyParser/DoctrineCacheUAFamilyParser.php b/UserAgent/UAFamilyParser/DoctrineCacheUAFamilyParser.php
@@ -0,0 +1,45 @@
+<?php
+
+/*
+ * This file is part of the Nelmio SecurityBundle.
+ *
+ * (c) Nelmio <hello@nelm.io>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Nelmio\SecurityBundle\UserAgent\UAFamilyParser;
+
+use Doctrine\Common\Cache\Cache;
+
+class DoctrineCacheUAFamilyParser implements UAFamilyParserInterface
+{
+    private $cache;
+    private $parser;
+    private $lifetime;
+    private $prefix;
+
+    public function __construct(Cache $cache, UAFamilyParser $parser, $lifetime = 0, $prefix = 'nelmio-ua-parser-')
+    {
+        $this->parser = $parser;
+        $this->cache = $cache;
+        $this->lifetime = $lifetime;
+        $this->prefix = $prefix;
+    }
+
+    public function getUaFamily($userAgent)
+    {
+        $id = $this->prefix.md5($userAgent);
+
+        if (false !== $name = $this->cache->fetch($id)) {
+            return $name;
+        }
+
+        $name = $this->parser->getUaFamily($userAgent);
+
+        $this->cache->save($id, $name, $this->lifetime);
+
+        return $name;
+    }
+}
diff --git a/UserAgent/UAFamilyParser/PsrCacheUAFamilyParser.php b/UserAgent/UAFamilyParser/PsrCacheUAFamilyParser.php
@@ -0,0 +1,48 @@
+<?php
+
+/*
+ * This file is part of the Nelmio SecurityBundle.
+ *
+ * (c) Nelmio <hello@nelm.io>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Nelmio\SecurityBundle\UserAgent\UAFamilyParser;
+
+use Psr\Cache\CacheItemPoolInterface;
+use Psr\Cache\CacheException;
+
+class PsrCacheUAFamilyParser implements UAFamilyParserInterface
+{
+    private $cache;
+    private $parser;
+    private $lifetime;
+    private $prefix;
+
+    public function __construct(CacheItemPoolInterface $cache, UAFamilyParser $parser, $lifetime = 0, $prefix = 'nelmio-ua-parser-')
+    {
+        $this->parser = $parser;
+        $this->cache = $cache;
+        $this->lifetime = $lifetime;
+        $this->prefix = $prefix;
+    }
+
+    public function getUaFamily($userAgent)
+    {
+        $id = $this->prefix.md5($userAgent);
+
+        $item = $this->cache->getItem($id);
+
+        if ($item->isHit()) {
+            return $item->get();
+        }
+
+        $name = $this->parser->getUaFamily($userAgent);
+
+        $this->cache->save($item->set($name)->expiresAfter($this->lifetime));
+
+        return $name;
+    }
+}
diff --git a/UserAgent/UAFamilyParser/UAFamilyParser.php b/UserAgent/UAFamilyParser/UAFamilyParser.php
@@ -0,0 +1,29 @@
+<?php
+
+/*
+ * This file is part of the Nelmio SecurityBundle.
+ *
+ * (c) Nelmio <hello@nelm.io>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Nelmio\SecurityBundle\UserAgent\UAFamilyParser;
+
+use UAParser\Parser;
+
+class UAFamilyParser implements UAFamilyParserInterface
+{
+    private $parser;
+
+    public function __construct(Parser $parser)
+    {
+        $this->parser = $parser;
+    }
+
+    public function getUaFamily($userAgent)
+    {
+        return strtolower($this->parser->parse($userAgent)->ua->family);
+    }
+}