From 114b0a5ea7ea6b8f33b8c9b8d1611e5ee6619a1c Mon Sep 17 00:00:00 2001
From: Ingo Schommer <me@chillu.com>
Date: Fri, 13 Jul 2018 11:08:47 +1200
Subject: [PATCH] NEW Option for secure "remember me" cookie

Fixes #8234
---
 .../09_Security/04_Secure_Coding.md           | 14 ++++++
 .../CookieAuthenticationHandler.php           | 49 +++++++++++++++----
 2 files changed, 53 insertions(+), 10 deletions(-)

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