Skip to content

[SecurityBundle] make remember-me cookies auto-secure + inherit their default config from framework.session.cookie_* #28446

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Symfony/Bridge/Monolog/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ CHANGELOG
* added `ProcessorInterface`: an optional interface to allow autoconfiguration of Monolog processors
* The methods `DebugProcessor::getLogs()`, `DebugProcessor::countErrors()`, `Logger::getLogs()`
and `Logger::countErrors()` will have a new `$request` argument in version 5.0, not defining
it is deprecated since Symfony 4.2.
it is deprecated

4.1.0
-----
Expand Down
6 changes: 4 additions & 2 deletions src/Symfony/Bundle/SecurityBundle/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ CHANGELOG

* 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`
the token classes is deprecated. To use custom tokens extend the existing
`Symfony\Component\Security\Core\Authentication\Token\AnonymousToken`.
or `Symfony\Component\Security\Core\Authentication\Token\RememberMeToken`.
* Added `Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddExpressionLanguageProvidersPass`
* Added `json_login_ldap` authentication provider to use LDAP authentication with a REST API.
* Made remember-me cookies inherit their default config from `framework.session.cookie_*`
and added an "auto" mode to their "secure" config option to make them secure on HTTPS automatically.

4.1.0
-----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\DependencyInjection\ChildDefinition;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\HttpFoundation\Cookie;

class RememberMeFactory implements SecurityFactoryInterface
{
Expand Down Expand Up @@ -140,7 +141,11 @@ public function addConfiguration(NodeDefinition $node)
;

foreach ($this->options as $name => $value) {
if (\is_bool($value)) {
if ('secure' === $name) {
$builder->enumNode($name)->values(array(true, false, 'auto'))->defaultValue('auto' === $value ? null : $value);
} elseif ('samesite' === $name) {
$builder->enumNode($name)->values(array(null, Cookie::SAMESITE_LAX, Cookie::SAMESITE_STRICT))->defaultValue($value);
} elseif (\is_bool($value)) {
$builder->booleanNode($name)->defaultValue($value);
} else {
$builder->scalarNode($name)->defaultValue($value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Bundle\SecurityBundle\DependencyInjection;

use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\RememberMeFactory;
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\SecurityFactoryInterface;
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\UserProvider\UserProviderFactoryInterface;
use Symfony\Bundle\SecurityBundle\SecurityUserValueResolver;
Expand All @@ -22,6 +23,7 @@
use Symfony\Component\DependencyInjection\ChildDefinition;
use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface;
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
use Symfony\Component\DependencyInjection\Parameter;
use Symfony\Component\DependencyInjection\Reference;
Expand All @@ -37,7 +39,7 @@
* @author Fabien Potencier <fabien@symfony.com>
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
*/
class SecurityExtension extends Extension
class SecurityExtension extends Extension implements PrependExtensionInterface
{
private $requestMatchers = array();
private $expressions = array();
Expand All @@ -54,6 +56,32 @@ public function __construct()
}
}

public function prepend(ContainerBuilder $container)
{
$rememberMeSecureDefault = false;
$rememberMeSameSiteDefault = null;

if (!isset($container->getExtensions()['framework'])) {
return;
}
foreach ($container->getExtensionConfig('framework') as $config) {
if (isset($config['session'])) {
$rememberMeSecureDefault = $config['session']['cookie_secure'] ?? $rememberMeSecureDefault;
$rememberMeSameSiteDefault = array_key_exists('cookie_samesite', $config['session']) ? $config['session']['cookie_samesite'] : $rememberMeSameSiteDefault;
}
}
foreach ($this->listenerPositions as $position) {
foreach ($this->factories[$position] as $factory) {
if ($factory instanceof RememberMeFactory) {
\Closure::bind(function () use ($rememberMeSecureDefault, $rememberMeSameSiteDefault) {
$this->options['secure'] = $rememberMeSecureDefault;
$this->options['samesite'] = $rememberMeSameSiteDefault;
}, $factory, $factory)();
}
}
}
}

public function load(array $configs, ContainerBuilder $container)
{
if (!array_filter($configs)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public function testSessionLessRememberMeLogout()
$cookieJar->expire(session_name());

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

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

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
imports:
- { resource: ./../config/framework.yml }

framework:
session:
cookie_secure: auto
cookie_samesite: lax

security:
encoders:
Symfony\Component\Security\Core\User\User: plaintext
Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Bundle/SecurityBundle/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
},
"require-dev": {
"symfony/asset": "~3.4|~4.0",
"symfony/browser-kit": "~3.4|~4.0",
"symfony/browser-kit": "~4.2",
"symfony/console": "~3.4|~4.0",
"symfony/css-selector": "~3.4|~4.0",
"symfony/dom-crawler": "~3.4|~4.0",
Expand All @@ -48,6 +48,7 @@
"twig/twig": "~1.34|~2.4"
},
"conflict": {
"symfony/browser-kit": "<4.2",
"symfony/var-dumper": "<3.4",
"symfony/event-dispatcher": "<3.4",
"symfony/framework-bundle": "<4.2",
Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Component/BrowserKit/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ CHANGELOG
-----

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

3.4.0
-----
Expand Down
23 changes: 21 additions & 2 deletions src/Symfony/Component/BrowserKit/Cookie.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class Cookie
protected $secure;
protected $httponly;
protected $rawValue;
private $samesite;

/**
* Sets a cookie.
Expand All @@ -52,8 +53,9 @@ class Cookie
* @param bool $secure Indicates that the cookie should only be transmitted over a secure HTTPS connection from the client
* @param bool $httponly The cookie httponly flag
* @param bool $encodedValue Whether the value is encoded or not
* @param string|null $samesite The cookie samesite attribute
*/
public function __construct(string $name, ?string $value, string $expires = null, string $path = null, string $domain = '', bool $secure = false, bool $httponly = true, bool $encodedValue = false)
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)
{
if ($encodedValue) {
$this->value = urldecode($value);
Expand All @@ -67,6 +69,7 @@ public function __construct(string $name, ?string $value, string $expires = null
$this->domain = $domain;
$this->secure = $secure;
$this->httponly = $httponly;
$this->samesite = $samesite;

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

if (null !== $this->samesite) {
$str .= '; samesite='.$this->samesite;
}

return $cookie;
}

Expand Down Expand Up @@ -138,6 +145,7 @@ public static function fromString($cookie, $url = null)
'secure' => false,
'httponly' => false,
'passedRawValue' => true,
'samesite' => null,
);

if (null !== $url) {
Expand Down Expand Up @@ -186,7 +194,8 @@ public static function fromString($cookie, $url = null)
$values['domain'],
$values['secure'],
$values['httponly'],
$values['passedRawValue']
$values['passedRawValue'],
$values['samesite']
);
}

Expand Down Expand Up @@ -298,4 +307,14 @@ public function isExpired()
{
return null !== $this->expires && 0 != $this->expires && $this->expires < time();
}

/**
* Gets the samesite attribute of the cookie.
*
* @return string|null The cookie samesite attribute
*/
public function getSameSite(): ?string
{
return $this->samesite;
}
}
9 changes: 9 additions & 0 deletions src/Symfony/Component/BrowserKit/Tests/CookieTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -202,4 +202,13 @@ public function testConstructException()
{
$cookie = new Cookie('foo', 'bar', 'string');
}

public function testSameSite()
{
$cookie = new Cookie('foo', 'bar');
$this->assertNull($cookie->getSameSite());

$cookie = new Cookie('foo', 'bar', 0, '/', 'foo.com', false, true, false, 'lax');
$this->assertSame('lax', $cookie->getSameSite());
}
}
2 changes: 1 addition & 1 deletion src/Symfony/Component/DomCrawler/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ CHANGELOG
* The `$currentUri` constructor argument of the `AbstractUriElement`, `Link` and
`Image` classes is now optional.
* The `Crawler::children()` method will have a new `$selector` argument in version 5.0,
not defining it is deprecated since version 4.2.
not defining it is deprecated.

3.1.0
-----
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Finder/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ CHANGELOG
-----

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

4.0.0
-----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ public function collect(Request $request, Response $response, \Exception $except
'controller' => $this->parseController($request->attributes->get('_controller')),
'status_code' => $statusCode,
'status_text' => Response::$statusTexts[(int) $statusCode],
))
)),
0, '/', null, $request->isSecure(), true, false, 'lax'
));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,8 @@ public function testItSetsARedirectCookieIfTheResponseIsARedirection()
$cookie = $this->getCookieByName($response, 'sf_redirect');

$this->assertNotEmpty($cookie->getValue());
$this->assertSame('lax', $cookie->getSameSite());
$this->assertFalse($cookie->isSecure());
}

public function testItCollectsTheRedirectionAndClearTheCookie()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ protected function cancelCookie(Request $request)
$this->logger->debug('Clearing remember-me cookie.', array('name' => $this->options['name']));
}

$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));
$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));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ protected function processAutoLoginCookie(array $cookieParts, Request $request)
time() + $this->options['lifetime'],
$this->options['path'],
$this->options['domain'],
$this->options['secure'],
$this->options['secure'] ?? $request->isSecure(),
$this->options['httponly'],
false,
$this->options['samesite'] ?? null
Expand Down Expand Up @@ -118,7 +118,7 @@ protected function onLoginSuccess(Request $request, Response $response, TokenInt
time() + $this->options['lifetime'],
$this->options['path'],
$this->options['domain'],
$this->options['secure'],
$this->options['secure'] ?? $request->isSecure(),
$this->options['httponly'],
false,
$this->options['samesite'] ?? null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ protected function onLoginSuccess(Request $request, Response $response, TokenInt
$expires,
$this->options['path'],
$this->options['domain'],
$this->options['secure'],
$this->options['secure'] ?? $request->isSecure(),
$this->options['httponly'],
false,
$this->options['samesite'] ?? null
Expand Down