Skip to content

Commit

Permalink
feature #30111 [SecurityBundle] Deprecate the normalization of the co…
Browse files Browse the repository at this point in the history
…okie names (javiereguiluz)

This PR was squashed before being merged into the 4.3-dev branch (closes #30111).

Discussion
----------

[SecurityBundle] Deprecate the normalization of the cookie names

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

This is an alternative solution to #24018 providing a BC layer until Symfony 5.0.

Commits
-------

36c5df4 [SecurityBundle] Deprecate the normalization of the cookie names
  • Loading branch information
fabpot committed Feb 12, 2019
2 parents 33145da + 36c5df4 commit 865127b
Show file tree
Hide file tree
Showing 7 changed files with 107 additions and 5 deletions.
9 changes: 7 additions & 2 deletions UPGRADE-5.0.md
Expand Up @@ -192,7 +192,7 @@ HttpKernel
* The `Kernel::getRootDir()` and the `kernel.root_dir` parameter have been removed
* The `KernelInterface::getName()` and the `kernel.name` parameter have been removed
* Removed the first and second constructor argument of `ConfigDataCollector`
* Removed `ConfigDataCollector::getApplicationName()`
* Removed `ConfigDataCollector::getApplicationName()`
* Removed `ConfigDataCollector::getApplicationVersion()`

Monolog
Expand Down Expand Up @@ -278,6 +278,11 @@ SecurityBundle
use Guard instead.
* The `SimpleFormFactory` and `SimplePreAuthenticationFactory` classes have been removed,
use Guard instead.
* The names of the cookies configured in the `logout.delete_cookies` option are
no longer normalized. If any of your cookie names has dashes they won't be
changed to underscores.
Before: `my-cookie` deleted the `my_cookie` cookie (with an underscore).
After: `my-cookie` deletes the `my-cookie` cookie (with a dash).

Serializer
----------
Expand Down Expand Up @@ -326,5 +331,5 @@ Workflow
Yaml
----

* The parser is now stricter and will throw a `ParseException` when a
* The parser is now stricter and will throw a `ParseException` when a
mapping is found inside a multi-line string.
14 changes: 11 additions & 3 deletions src/Symfony/Bundle/SecurityBundle/CHANGELOG.md
@@ -1,10 +1,18 @@
CHANGELOG
=========

4.3.0
-----

* The normalization of the cookie names configured in the `logout.delete_cookies`
option is deprecated and will be disabled in Symfony 5.0. This affects to cookies
with dashes in their names. For example, starting from Symfony 5.0, the `my-cookie`
name will delete `my-cookie` (with a dash) instead of `my_cookie` (with an underscore).

4.2.0
-----

* Using the `security.authentication.trust_resolver.anonymous_class` and
* Using the `security.authentication.trust_resolver.anonymous_class` and
`security.authentication.trust_resolver.rememberme_class` parameters to define
the token classes is deprecated. To use custom tokens extend the existing
`Symfony\Component\Security\Core\Authentication\Token\AnonymousToken`.
Expand All @@ -17,7 +25,7 @@ CHANGELOG
* Deprecated the `SimpleFormFactory` and `SimplePreAuthenticationFactory` classes, use Guard instead.
* Added `port` in access_control
* Added individual voter decisions to the profiler

4.1.0
-----

Expand Down Expand Up @@ -50,7 +58,7 @@ CHANGELOG
* Tagging voters with the `security.voter` tag without implementing the
`VoterInterface` on the class is now deprecated and will be removed in 4.0.
* [BC BREAK] `FirewallContext::getListeners()` now returns `\Traversable|array`
* added info about called security listeners in profiler
* added info about called security listeners in profiler
* Added `logout_on_user_change` to the firewall options. This config item will
trigger a logout when the user has changed. Should be set to true to avoid
deprecations in the configuration.
Expand Down
Expand Up @@ -218,10 +218,27 @@ private function addFirewallsSection(ArrayNodeDefinition $rootNode, array $facto
->fixXmlConfig('delete_cookie')
->children()
->arrayNode('delete_cookies')
->normalizeKeys(false)
->beforeNormalization()
->ifTrue(function ($v) { return \is_array($v) && \is_int(key($v)); })
->then(function ($v) { return array_map(function ($v) { return ['name' => $v]; }, $v); })
->end()
->beforeNormalization()
->ifArray()->then(function ($v) {
foreach ($v as $originalName => $cookieConfig) {
if (false !== strpos($originalName, '-')) {
$normalizedName = str_replace('-', '_', $originalName);
@trigger_error(sprintf('Normalization of cookie names is deprecated since Symfony 4.3. Starting from Symfony 5.0, the "%s" cookie configured in "logout.delete_cookies" will delete the "%s" cookie instead of the "%s" cookie.', $originalName, $originalName, $normalizedName), E_USER_DEPRECATED);

// normalize cookie names manually for BC reasons. Remove it in Symfony 5.0.
$v[$normalizedName] = $cookieConfig;
unset($v[$originalName]);
}
}

return $v;
})
->end()
->useAttributeAsKey('name')
->prototype('array')
->children()
Expand Down
Expand Up @@ -506,6 +506,20 @@ public function testSimpleAuth()
]], $listeners);
}

/**
* @group legacy
* @expectedDeprecation Normalization of cookie names is deprecated since Symfony 4.3. Starting from Symfony 5.0, the "cookie1-name" cookie configured in "logout.delete_cookies" will delete the "cookie1-name" cookie instead of the "cookie1_name" cookie.
* @expectedDeprecation Normalization of cookie names is deprecated since Symfony 4.3. Starting from Symfony 5.0, the "cookie3-long_name" cookie configured in "logout.delete_cookies" will delete the "cookie3-long_name" cookie instead of the "cookie3_long_name" cookie.
*/
public function testLogoutDeleteCookieNamesNormalization()
{
$container = $this->getContainer('logout_delete_cookies');
$cookiesToDelete = $container->getDefinition('security.logout.handler.cookie_clearing.main')->getArgument(0);
$expectedCookieNames = ['cookie2_name', 'cookie1_name', 'cookie3_long_name'];

$this->assertSame($expectedCookieNames, array_keys($cookiesToDelete));
}

protected function getContainer($file)
{
$file .= '.'.$this->getFileExtension();
Expand Down
@@ -0,0 +1,21 @@
<?php

$container->loadFromExtension('security', [
'providers' => [
'default' => ['id' => 'foo'],
],

'firewalls' => [
'main' => [
'provider' => 'default',
'form_login' => true,
'logout' => [
'delete_cookies' => [
'cookie1-name' => true,
'cookie2_name' => true,
'cookie3-long_name' => ['path' => '/'],
],
],
],
],
]);
@@ -0,0 +1,22 @@
<?xml version="1.0" encoding="UTF-8"?>

<srv:container xmlns="http://symfony.com/schema/dic/security"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:srv="http://symfony.com/schema/dic/services"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">

<config>
<provider name="default" id="foo" />

<firewall name="main" provider="default">
<form-login />
<logout>
<delete-cookies>
<cookie1-name/>
<cookie2_name/>
<cookie3-long_name path="/" />
</delete-cookies>
</logout>
</firewall>
</config>
</srv:container>
@@ -0,0 +1,15 @@
security:
providers:
default:
id: foo

firewalls:
main:
provider: default
form_login: true
logout:
delete_cookies:
cookie1-name: ~
cookie2_name: ~
cookie3-long_name:
path: '/'

0 comments on commit 865127b

Please sign in to comment.