diff --git a/docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md b/docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md index dced0ad87..76a03518a 100644 --- a/docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md +++ b/docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md @@ -705,6 +705,20 @@ SilverStripe\Control\Session: cookie_secure: true ``` +The same treatment should be applied to the cookie responsible for remembering logins across sessions: + +```yml +--- +Name: secure-alc +Except: + environment: dev +--- +SilverStripe\Core\Injector\Injector: + SilverStripe\Security\MemberAuthenticator\CookieAuthenticationHandler: + properties: + TokenCookieSecure: true +``` + For other cookies set by your application we should also ensure the users are provided with secure cookies by setting the "Secure" and "HTTPOnly" flags. These flags prevent them from being stolen by an attacker through javascript. diff --git a/src/Security/MemberAuthenticator/CookieAuthenticationHandler.php b/src/Security/MemberAuthenticator/CookieAuthenticationHandler.php index d481f43bb..2594d4b1f 100644 --- a/src/Security/MemberAuthenticator/CookieAuthenticationHandler.php +++ b/src/Security/MemberAuthenticator/CookieAuthenticationHandler.php @@ -27,6 +27,11 @@ class CookieAuthenticationHandler implements AuthenticationHandler */ private $tokenCookieName; + /** + * @var boolean + */ + private $tokenCookieSecure = false; + /** * @var IdentityStore */ @@ -76,6 +81,28 @@ class CookieAuthenticationHandler implements AuthenticationHandler return $this; } + /** + * Get the name of the cookie used to store an login token + * + * @return string + */ + public function getTokenCookieSecure() + { + return $this->tokenCookieSecure; + } + + /** + * Set cookie with HTTPS only flag + * + * @param string $tokenCookieSecure + * @return $this + */ + public function setTokenCookieSecure($tokenCookieSecure) + { + $this->tokenCookieSecure = $tokenCookieSecure; + return $this; + } + /** * Once a member is found by authenticateRequest() pass it to this identity store * @@ -128,11 +155,11 @@ class CookieAuthenticationHandler implements AuthenticationHandler /** @var RememberLoginHash $rememberLoginHash */ $rememberLoginHash = RememberLoginHash::get() - ->filter(array( + ->filter([ 'MemberID' => $member->ID, 'DeviceID' => $deviceID, - 'Hash' => $hash - ))->first(); + 'Hash' => $hash, + ])->first(); if (!$rememberLoginHash) { return null; } @@ -189,13 +216,14 @@ class CookieAuthenticationHandler implements AuthenticationHandler $rememberLoginHash = RememberLoginHash::generate($member); $tokenExpiryDays = RememberLoginHash::config()->uninherited('token_expiry_days'); $deviceExpiryDays = RememberLoginHash::config()->uninherited('device_expiry_days'); + $secure = $this->getTokenCookieSecure(); Cookie::set( $this->getTokenCookieName(), $member->ID . ':' . $rememberLoginHash->getToken(), $tokenExpiryDays, null, null, - null, + $secure, true ); Cookie::set( @@ -204,7 +232,7 @@ class CookieAuthenticationHandler implements AuthenticationHandler $deviceExpiryDays, null, null, - null, + $secure, true ); } else { @@ -220,7 +248,7 @@ class CookieAuthenticationHandler implements AuthenticationHandler { $member = Security::getCurrentUser(); if ($member) { - RememberLoginHash::clear($member, Cookie::get('alc_device')); + RememberLoginHash::clear($member, Cookie::get($this->getDeviceCookieName())); } $this->clearCookies(); @@ -236,9 +264,10 @@ class CookieAuthenticationHandler implements AuthenticationHandler */ protected function clearCookies() { - Cookie::set($this->getTokenCookieName(), null); - Cookie::set($this->getDeviceCookieName(), null); - Cookie::force_expiry($this->getTokenCookieName()); - Cookie::force_expiry($this->getDeviceCookieName()); + $secure = $this->getTokenCookieSecure(); + Cookie::set($this->getTokenCookieName(), null, null, null, null, $secure); + Cookie::set($this->getDeviceCookieName(), null, null, null, null, $secure); + Cookie::force_expiry($this->getTokenCookieName(), null, null, null, null, $secure); + Cookie::force_expiry($this->getDeviceCookieName(), null, null, null, null, $secure); } }