Skip to content

Commit 4f7b41a

Browse files
nicolas-grekasnicolas-grekas
committed
[FrameworkBundle] Added new "auto" mode for framework.session.cookie_secure to turn it on when https is used
1 parent c099d86 commit 4f7b41a

File tree

12 files changed

+110
-9
lines changed

12 files changed

+110
-9
lines changed

‎src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ CHANGELOG
99
* Deprecated auto-injection of the container in AbstractController instances, register them as service subscribers instead
1010
* Deprecated processing of services tagged `security.expression_language_provider` in favor of a new `AddExpressionLanguageProvidersPass` in SecurityBundle.
1111
* Enabled autoconfiguration for `Psr\Log\LoggerAwareInterface`
12+
* Added new "auto" mode for `framework.session.cookie_secure` to turn it on when HTTPS is used
1213

1314
4.1.0
1415
-----

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ private function addSessionSection(ArrayNodeDefinition $rootNode)
482482
->scalarNode('cookie_lifetime')->end()
483483
->scalarNode('cookie_path')->end()
484484
->scalarNode('cookie_domain')->end()
485-
->booleanNode('cookie_secure')->end()
485+
->enumNode('cookie_secure')->values(array(true, false, 'auto'))->end()
486486
->booleanNode('cookie_httponly')->defaultTrue()->end()
487487
->booleanNode('use_cookies')->end()
488488
->scalarNode('gc_divisor')->end()

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -765,6 +765,14 @@ private function registerSessionConfiguration(array $config, ContainerBuilder $c
765765
}
766766
}
767767

768+
if ('auto' === ($options['cookie_secure'] ?? null)) {
769+
$locator = $container->getDefinition('session_listener')->getArgument(0);
770+
$locator->setValues($locator->getValues() + array(
771+
'session_storage' => new Reference('session.storage', ContainerInterface::IGNORE_ON_INVALID_REFERENCE),
772+
'request_stack' => new Reference('request_stack'),
773+
));
774+
}
775+
768776
$container->setParameter('session.storage.options', $options);
769777

770778
// session handler (the internal callback registered with PHP session management)

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@
112112
<xsd:attribute name="cookie-lifetime" type="xsd:string" />
113113
<xsd:attribute name="cookie-path" type="xsd:string" />
114114
<xsd:attribute name="cookie-domain" type="xsd:string" />
115-
<xsd:attribute name="cookie-secure" type="xsd:boolean" />
115+
<xsd:attribute name="cookie-secure" type="cookie_secure" />
116116
<xsd:attribute name="cookie-httponly" type="xsd:boolean" />
117117
<xsd:attribute name="use-cookies" type="xsd:boolean" />
118118
<xsd:attribute name="cache-limiter" type="xsd:string" />
@@ -329,6 +329,16 @@
329329
</xsd:sequence>
330330
</xsd:complexType>
331331

332+
<xsd:simpleType name="cookie_secure">
333+
<xsd:restriction base="xsd:string">
334+
<xsd:enumeration value="true" />
335+
<xsd:enumeration value="false" />
336+
<xsd:enumeration value="1" />
337+
<xsd:enumeration value="0" />
338+
<xsd:enumeration value="auto" />
339+
</xsd:restriction>
340+
</xsd:simpleType>
341+
332342
<xsd:simpleType name="workflow_type">
333343
<xsd:restriction base="xsd:string">
334344
<xsd:enumeration value="state_machine" />

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/session_cookie_secure_auto.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
$container->loadFromExtension('framework', array(
4+
'session' => array(
5+
'handler_id' => null,
6+
'cookie_secure' => 'auto',
7+
),
8+
));

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/session_cookie_secure_auto.xml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?xml version="1.0" ?>
2+
3+
<container xmlns="http://symfony.com/schema/dic/services"
4+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
5+
xmlns:framework="http://symfony.com/schema/dic/symfony"
6+
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd
7+
http://symfony.com/schema/dic/symfony http://symfony.com/schema/dic/symfony/symfony-1.0.xsd">
8+
9+
<framework:config>
10+
<framework:session handler-id="null" cookie-secure="auto" />
11+
</framework:config>
12+
</container>

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/session_cookie_secure_auto.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
framework:
2+
session:
3+
handler_id: ~
4+
cookie_secure: auto

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,9 @@ public function testNullSessionHandler()
423423
$this->assertTrue($container->hasDefinition('session'), '->registerSessionConfiguration() loads session.xml');
424424
$this->assertNull($container->getDefinition('session.storage.native')->getArgument(1));
425425
$this->assertNull($container->getDefinition('session.storage.php_bridge')->getArgument(0));
426+
427+
$expected = array('session', 'initialized_session');
428+
$this->assertEquals($expected, array_keys($container->getDefinition('session_listener')->getArgument(0)->getValues()));
426429
}
427430

428431
public function testRequest()
@@ -1243,6 +1246,14 @@ public function testLoggerAwareRegistration()
12431246
$this->assertSame('logger', (string) $calls[0][1][0], 'Argument should be a reference to "logger"');
12441247
}
12451248

1249+
public function testSessionCookieSecureAuto()
1250+
{
1251+
$container = $this->createContainerFromFile('session_cookie_secure_auto');
1252+
1253+
$expected = array('session', 'initialized_session', 'session_storage', 'request_stack');
1254+
$this->assertEquals($expected, array_keys($container->getDefinition('session_listener')->getArgument(0)->getValues()));
1255+
}
1256+
12461257
protected function createContainer(array $data = array())
12471258
{
12481259
return new ContainerBuilder(new ParameterBag(array_merge(array(

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSessionDomainConstraintPass.php‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,17 @@ public function process(ContainerBuilder $container)
3232

3333
$sessionOptions = $container->getParameter('session.storage.options');
3434
$domainRegexp = empty($sessionOptions['cookie_domain']) ? '%s' : sprintf('(?:%%s|(?:.+\.)?%s)', preg_quote(trim($sessionOptions['cookie_domain'], '.')));
35-
$domainRegexp = (empty($sessionOptions['cookie_secure']) ? 'https?://' : 'https://').$domainRegexp;
3635

37-
$container->findDefinition('security.http_utils')->addArgument(sprintf('{^%s$}i', $domainRegexp));
36+
if ('auto' === ($sessionOptions['cookie_secure'] ?? null)) {
37+
$secureDomainRegexp = sprintf('{^https://%s$}i', $domainRegexp);
38+
$domainRegexp = 'https?://'.$domainRegexp;
39+
} else {
40+
$secureDomainRegexp = null;
41+
$domainRegexp = (empty($sessionOptions['cookie_secure']) ? 'https?://' : 'https://').$domainRegexp;
42+
}
43+
44+
$container->findDefinition('security.http_utils')
45+
->addArgument(sprintf('{^%s$}i', $domainRegexp))
46+
->addArgument($secureDomainRegexp);
3847
}
3948
}

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSessionDomainConstraintPassTest.php‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,26 @@ public function testNoSession()
9696
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://pirate.com/foo'));
9797
}
9898

99+
public function testSessionAutoSecure()
100+
{
101+
$container = $this->createContainer(array('cookie_domain' => '.symfony.com.', 'cookie_secure' => 'auto'));
102+
103+
$utils = $container->get('security.http_utils');
104+
$request = Request::create('/', 'get');
105+
106+
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('https://symfony.com/blog'));
107+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('https://www.symfony.com/blog'));
108+
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://symfony.com/blog'));
109+
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/'));
110+
111+
$container->get('router.request_context')->setScheme('https');
112+
113+
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('https://symfony.com/blog'));
114+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('https://www.symfony.com/blog'));
115+
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://localhost/'));
116+
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/'));
117+
}
118+
99119
private function createContainer($sessionStorageOptions)
100120
{
101121
$container = new ContainerBuilder();
@@ -121,7 +141,7 @@ private function createContainer($sessionStorageOptions)
121141
);
122142

123143
$ext = new FrameworkExtension();
124-
$ext->load(array('framework' => array('csrf_protection' => false)), $container);
144+
$ext->load(array('framework' => array('csrf_protection' => false, 'router' => array('resource' => 'dummy'))), $container);
125145

126146
$ext = new SecurityExtension();
127147
$ext->load($config, $container);

0 commit comments

Comments
 (0)