8000 feature #28446 [SecurityBundle] make remember-me cookies auto-secure … · symfony/symfony@10df10c · GitHub
[go: up one dir, main page]
Skip to content

Commit 10df10c

Browse files
fabpotfabpot
committed
feature #28446 [SecurityBundle] make remember-me cookies auto-secure + inherit their default config from framework.session.cookie_* (nicolas-grekas)
This PR was merged into the 4.2-dev branch. Discussion ---------- [SecurityBundle] make remember-me cookies auto-secure + inherit their default config from framework.session.cookie_* | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #28338 | License | MIT | Doc PR | - Let's make it easier to have a good default security level, now for the remember-me cookie. Commits ------- 6ec223b [SecurityBundle] make remember-me cookies auto-secure + inherit their default config from framework.session.cookie_*
2 parents b36172c + 6ec223b commit 10df10c

File tree

17 files changed

+91
-17
lines changed

17 files changed

+91
-17
lines changed

‎src/Symfony/Bridge/Monolog/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ CHANGELOG
77
* added `ProcessorInterface`: an optional interface to allow autoconfiguration of Monolog processors
88
* The methods `DebugProcessor::getLogs()`, `DebugProcessor::countErrors()`, `Logger::getLogs()`
99
and `Logger::countErrors()` will have a new `$request` argument in version 5.0, not defining
10-
it is deprecated since Symfony 4.2.
10+
it is deprecated
1111

1212
4.1.0
1313
-----

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@ CHANGELOG
66

77
* Using the `security.authentication.trust_resolver.anonymous_class` and
88
`security.authentication.trust_resolver.rememberme_class` parameters to define
9-
the token classes is deprecated. To use
10-
custom tokens extend the existing `Symfony\Component\Security\Core\Authentication\Token\AnonymousToken`
9+
the token classes is deprecated. To use custom tokens extend the existing
10+
`Symfony\Component\Security\Core\Authentication\Token\AnonymousToken`.
1111
or `Symfony\Component\Security\Core\Authentication\Token\RememberMeToken`.
1212
* Added `Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddExpressionLanguageProvidersPass`
1313
* Added `json_login_ldap` authentication provider to use LDAP authentication with a REST API.
14+
* Made remember-me cookies inherit their default config from `framework.session.cookie_*`
15+
and added an "auto" mode to their "secure" config option to make them secure on HTTPS automatically.
1416

1517
4.1.0
1618
-----

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Component\DependencyInjection\ChildDefinition;
1616
use Symfony\Component\DependencyInjection\ContainerBuilder;
1717
use Symfony\Component\DependencyInjection\Reference;
18+
use Symfony\Component\HttpFoundation\Cookie;
1819

1920
class RememberMeFactory implements SecurityFactoryInterface
2021
{
@@ -140,7 +141,11 @@ public function addConfiguration(NodeDefinition $node)
140141
;
141142

142143
foreach ($this->options as $name => $value) {
143-
if (\is_bool($value)) {
144+
if ('secure' === $name) {
145+
$builder->enumNode($name)->values(array(true, false, 'auto'))->defaultValue('auto' === $value ? null : $value);
146+
} elseif ('samesite' === $name) {
147+
$builder->enumNode($name)->values(array(null, Cookie::SAMESITE_LAX, Cookie::SAMESITE_STRICT))->defaultValue($value);
148+
} elseif (\is_bool($value)) {
144149
$builder->booleanNode($name)->defaultValue($value);
145150
} else {
146151
$builder->scalarNode($name)->defaultValue($value);

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Bundle\SecurityBundle\DependencyInjection;
1313

14+
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\RememberMeFactory;
1415
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\SecurityFactoryInterface;
1516
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\UserProvider\UserProviderFactoryInterface;
1617
use Symfony\Bundle\SecurityBundle\SecurityUserValueResolver;
@@ -22,6 +23,7 @@
2223
use Symfony\Component\DependencyInjection\ChildDefinition;
2324
use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass;
2425
use Symfony\Component\DependencyInjection\ContainerBuilder;
26+
use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface;
2527
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
2628
use Symfony\Component\DependencyInjection\Parameter;
2729
use Symfony\Component\DependencyInjection\Reference;
@@ -37,7 +39,7 @@
3739
* @author Fabien Potencier <fabien@symfony.com>
3840
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
3941
*/
40-
class SecurityExtension extends Extension
42+
class SecurityExtension extends Extension implements PrependExtensionInterface
4143
{
4244
private $requestMatchers = array();
4345
private $expressions = array();
@@ -54,6 +56,32 @@ public function __construct()
5456
}
5557
}
5658

59+
public function prepend(ContainerBuilder $container)
60+
{
61+
$rememberMeSecureDefault = false;
62+
$rememberMeSameSiteDefault = null;
63+
64+
if (!isset($container->getExtensions()['framework'])) {
65+
return;
66+
}
67+
foreach ($container->getExtensionConfig('framework') as $config) {
68+
if (isset($config['session'])) {
69+
$rememberMeSecureDefault = $config['session']['cookie_secure'] ?? $rememberMeSecureDefault;
70+
$rememberMeSameSiteDefault = array_key_exists('cookie_samesite', $config['session']) ? $config['session']['cookie_samesite'] : $rememberMeSameSiteDefault;
71+
}
72+
}
73+
foreach ($this->listenerPositions as $position) {
74+
foreach ($this->factories[$position] as $factory) {
75+
if ($factory instanceof RememberMeFactory) {
76+
\Closure::bind(function () use ($rememberMeSecureDefault, $rememberMeSameSiteDefault) {
77+
$this->options['secure'] = $rememberMeSecureDefault;
78+
$this->options['samesite'] = $rememberMeSameSiteDefault;
79+
}, $factory, $factory)();
80+
}
81+
}
82+
}
83+
}
84+
5785
public function load(array $configs, ContainerBuilder $container)
5886
{
5987
if (!array_filter($configs)) {

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/LogoutTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public function testSessionLessRememberMeLogout()
2626
$cookieJar->expire(session_name());
2727

2828
$this->assertNotNull($cookieJar->get('REMEMBERME'));
29+
$this->assertSame('lax', $cookieJar->get('REMEMBERME')->getSameSite());
2930

3031
$client->request('GET', '/logout');
3132

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/RememberMeLogout/config.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
imports:
22
- { resource: ./../config/framework.yml }
33

4+
framework:
5+
session:
6+
cookie_secure: auto
7+
cookie_samesite: lax
8+
49
security:
510
encoders:
611
Symfony\Component\Security\Core\User\User: plaintext

‎src/Symfony/Bundle/SecurityBundle/composer.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
},
2929
"require-dev": {
3030
"symfony/asset": "~3.4|~4.0",
31-
"symfony/browser-kit": "~3.4|~4.0",
31+
"symfony/browser-kit": "~4.2",
3232
"symfony/console": "~3.4|~4.0",
3333
"symfony/css-selector": "~3.4|~4.0",
3434
"symfony/dom-crawler": "~3.4|~4.0",
@@ -48,6 +48,7 @@
4848
"twig/twig": "~1.34|~2.4"
4949
},
5050
"conflict": {
51+
"symfony/browser-kit": "<4.2",
5152
"symfony/var-dumper": "<3.4",
5253
"symfony/event-dispatcher": "<3.4",
5354
"symfony/framework-bundle": "<4.2",

‎src/Symfony/Component/BrowserKit/CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ CHANGELOG
55
-----
66

77
* The method `Client::submit()` will have a new `$serverParameters` argument
8-
in version 5.0, not defining it is deprecated since version 4.2
8+
in version 5.0, not defining it is deprecated
9+
* Added ability to read the "samesite" attribute of cookies using `Cookie::getSameSite()`
910

1011
3.4.0
1112
-----

‎src/Symfony/Component/BrowserKit/Cookie.php

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ class Cookie
4040
protected $secure;
4141
protected $httponly;
4242
protected $rawValue;
43+
private $samesite;
4344

4445
/**
4546
* Sets a cookie.
@@ -52,8 +53,9 @@ class Cookie
5253
* @param bool $secure Indicates that the cookie should only be transmitted over a secure HTTPS connection from the client
5354
* @param bool $httponly The cookie httponly flag
5455
* @param bool $encodedValue Whether the value is encoded or not
56+
* @param string|null $samesite The cookie samesite attribute
5557
*/
56-
public function __construct(string $name, ?string $value, string $expires = null, string $path = null, string $domain = '', bool $secure = false, bool $httponly = true, bool $encodedValue = false)
58+
public function __construct(string $name, ?string $value, string $expires = null, string $path = null, string $domain = '', bool $secure = false, bool $httponly = true, bool $encodedValue = false, string $samesite = null)
5759
{
5860
if ($encodedValue) {
5961
$this->value = urldecode($value);
@@ -67,6 +69,7 @@ public function __construct(string $name, ?string $value, string $expires = null
6769
$this->domain = $domain;
6870
$this->secure = $secure;
6971
$this->httponly = $httponly;
72+
$this->samesite = $samesite;
7073

7174
if (null !== $expires) {
7275
$timestampAsDateTime = \DateTime::createFromFormat('U', $expires);
@@ -106,6 +109,10 @@ public function __toString()
106109
$cookie .= '; httponly';
107110
}
108111

112+
if (null !== $this->samesite) {
113+
$str .= '; samesite='.$this->samesite;
114+
}
115+
109116
return $cookie;
110117
}
111118

@@ -138,6 +145,7 @@ public static function fromString($cookie, $url = null)
138145
'secure' => false,
139146
'httponly' => false,
140147
'passedRawValue' => true,
148+
'samesite' => null,
141149
);
142150

143151
if (null !== $url) {
@@ -186,7 +194,8 @@ public static function fromString($cookie, $url = null)
186194
$values['domain'],
187195
$values['secure'],
188196
$values['httponly'],
189-
$values['passedRawValue']
197+
$values['passedRawValue'],
198+
$values['samesite']
190199
);
191200
}
192201

@@ -298,4 +307,14 @@ public function isExpired()
298307
{
299308
return null !== $this->expires && 0 != $this->expires && $this->expires < time();
300309
}
310+
311+
/**
312+
* Gets the samesite attribute of the cookie.
313+
*
314+
* @return string|null The cookie samesite attribute
315+
*/
316+
public function getSameSite(): ?string
317+
{
318+
return $this->samesite;
319+
}
301320
}

‎src/Symfony/Component/BrowserKit/Tests/CookieTest.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,4 +202,13 @@ public function testConstructException()
202202
{
203203
$cookie = new Cookie('foo', 'bar', 'string');
204204
}
205+
206+
public function testSameSite()
207+
{
208+
$cookie = new Cookie('foo', 'bar');
209+
$this->assertNull($cookie->getSameSite());
210+
211+
$cookie = new Cookie('foo', 'bar', 0, '/', 'foo.com', false, true, false, 'lax');
212+
$this->assertSame('lax', $cookie->getSameSite());
213+
}
205214
}

‎src/Symfony/Component/DomCrawler/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ CHANGELOG
77
* The `$currentUri` constructor argument of the `AbstractUriElement`, `Link` and
88
`Image` classes is now optional.
99
* The `Crawler::children()` method will have a new `$selector` argument in version 5.0,
10-
not defining it is deprecated since version 4.2.
10+
not defining it is deprecated.
1111

1212
3.1.0
1313
-----

‎src/Symfony/Component/Finder/CHANGELOG.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ CHANGELOG
55
-----
66

77
* added $useNaturalSort option to Finder::sortByName() method
8-
* The `Finder::sortByName()` method will have a new `$useNaturalSort`
9-
argument in version 5.0, not defining it is deprecated since version 4.2.
8+
* the `Finder::sortByName()` method will have a new `$useNaturalSort`
9+
argument in version 5.0, not defining it is deprecated
1010

1111
4.0.0
1212
-----

‎src/Symfony/Component/HttpKernel/DataCollector/RequestDataCollector.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,8 @@ public function collect(Request $request, Response $response, \Exception $except
165165
'controller' => $this->parseController($request->attributes->get('_controller')),
166166
'status_code' => $statusCode,
167167
'status_text' => Response::$statusTexts[(int) $statusCode],
168-
))
168+
)),
169+
0, '/', null, $request->isSecure(), true, false, 'lax'
169170
));
170171
}
171172

‎src/Symfony/Component/HttpKernel/Tests/DataCollector/RequestDataCollectorTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,8 @@ public function testItSetsARedirectCookieIfTheResponseIsARedirection()
225225
$cookie = $this->getCookieByName($response, 'sf_redirect');
226226

227227
$this->assertNotEmpty($cookie->getValue());
228+
$this->assertSame('lax', $cookie->getSameSite());
229+
$this->assertFalse($cookie->isSecure());
228230
}
229231

230232
public function testItCollectsTheRedirectionAndClearTheCookie()

‎src/Symfony/Component/Security/Http/RememberMe/AbstractRememberMeServices.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ protected function cancelCookie(Request $request)
271271
$this->logger->debug('Clearing remember-me cookie.', array('name' => $this->options['name']));
272272
}
273273

274-
$request->attributes->set(self::COOKIE_ATTR_NAME, new Cookie($this->options['name'], null, 1, $this->options['path'], $this->options['domain'], $this->options['secure'], $this->options['httponly'], false, $this->options['samesite'] ?? null));
274+
$request->attributes->set(self::COOKIE_ATTR_NAME, new Cookie($this->options['name'], null, 1, $this->options['path'], $this->options['domain'], $this->options['secure'] ?? $request->isSecure(), $this->options['httponly'], false, $this->options['samesite'] ?? null));
275275
}
276276

277277
/**

‎src/Symfony/Component/Security/Http/RememberMe/PersistentTokenBasedRememberMeServices.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ protected function processAutoLoginCookie(array $cookieParts, Request $request)
8383
time() + $this->options['lifetime'],
8484
$this->options['path'],
8585
$this->options['domain'],
86-
$this->options['secure'],
86+
$this->options['secure'] ?? $request->isSecure(),
8787
$this->options['httponly'],
8888
false,
8989
$this->options['samesite'] ?? null
@@ -118,7 +118,7 @@ protected function onLoginSuccess(Request $request, Response $response, TokenInt
118118
time() + $this->options['lifetime'],
119119
$this->options['path'],
120120
$this->options['domain'],
121-
$this->options['secure'],
121+
$this->options['secure'] ?? $request->isSecure(),
122122
$this->options['httponly'],
123123
false,
124124
$this->options['samesite'] ?? null

‎src/Symfony/Component/Security/Http/RememberMe/TokenBasedRememberMeServices.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ protected function onLoginSuccess(Request $request, Response $response, TokenInt
8080
$expires,
8181
$this->options['path'],
8282
$this->options['domain'],
83-
$this->options['secure'],
83+
$this->options['secure'] ?? $request->isSecure(),
8484
$this->options['httponly'],
8585
false,
8686
$this->options['samesite'] ?? null

0 commit comments

Comments
 (0)
0