Make most cookies HTTP only by default. (#1243)

config/vufind/config.ini

@@ -178,6 +178,9 @@ secure = false
 ; the browser from ever sending cookies over an unencrypted connection (i.e.
 ; before being redirected to HTTPS). Default is false.
 ;only_secure = true
+; Whether to set cookies set by the server (apart from cart function) "HTTP only" so
+; that they cannot be accessed by scripts. Default is true.
+;http_only = false
 ; Set the domain used for cookies (sometimes useful for sharing the cookies across
 ; subdomains); by default, cookies will be restricted to the current hostname.
 ;domain = ".example.edu"

module/VuFind/src/VuFind/Cart.php

@@ -312,9 +312,9 @@ class Cart
 
         // Save the cookies:
         $cookie = implode(self::CART_COOKIE_DELIM, $ids);
-        $this->cookieManager->set(self::CART_COOKIE, $cookie, 0);
+        $this->cookieManager->set(self::CART_COOKIE, $cookie, 0, false);
         $srcCookie = implode(self::CART_COOKIE_DELIM, $sources);
-        $this->cookieManager->set(self::CART_COOKIE_SOURCES, $srcCookie, 0);
+        $this->cookieManager->set(self::CART_COOKIE_SOURCES, $srcCookie, 0, false);
     }
 
     /**

module/VuFind/src/VuFind/Cookie/CookieManager.php

@@ -59,6 +59,13 @@ class CookieManager
      */
     protected $secure;
 
+    /**
+     * Are cookies HTTP only?
+     *
+     * @var bool
+     */
+    protected $httpOnly;
+
     /**
      * The name of the session cookie
      *
@@ -75,14 +82,16 @@ class CookieManager
      * @param bool   $secure      Are cookies secure only? (default = false)
      * @param string $sessionName Session cookie name (if null defaults to PHP
      * settings)
+     * @param bool   $httpOnly    Are cookies HTTP only? (default = true)
      */
     public function __construct($cookies, $path = '/', $domain = null,
-        $secure = false, $sessionName = null
+        $secure = false, $sessionName = null, $httpOnly = true
     ) {
         $this->cookies = $cookies;
         $this->path = $path;
         $this->domain = $domain;
         $this->secure = $secure;
+        $this->httpOnly = $httpOnly;
         $this->sessionName = $sessionName;
     }
 
@@ -126,6 +135,16 @@ class CookieManager
         return $this->secure;
     }
 
+    /**
+     * Are cookies set to "HTTP only" mode?
+     *
+     * @return bool
+     */
+    public function isHttpOnly()
+    {
+        return $this->httpOnly;
+    }
+
     /**
      * Get the name of the cookie
      *
@@ -152,18 +171,23 @@ class CookieManager
     /**
      * Support method for set() -- set the actual cookie in PHP.
      *
-     * @param string $key    Name of cookie to set
-     * @param mixed  $value  Value to set
-     * @param int    $expire Cookie expiration time
+     * @param string    $key      Name of cookie to set
+     * @param mixed     $value    Value to set
+     * @param int       $expire   Cookie expiration time
+     * @param null|bool $httpOnly Whether the cookie should be "HTTP only"
      *
      * @return bool
      */
-    public function setGlobalCookie($key, $value, $expire)
+    public function setGlobalCookie($key, $value, $expire, $httpOnly = null)
     {
+        if (null === $httpOnly) {
+            $httpOnly = $this->httpOnly;
+        }
         // Simple case: flat value.
         if (!is_array($value)) {
             return $this->proxySetCookie(
-                $key, $value, $expire, $this->path, $this->domain, $this->secure
+                $key, $value, $expire, $this->path, $this->domain, $this->secure,
+                $httpOnly
             );
         }
 
@@ -172,7 +196,7 @@ class CookieManager
         foreach ($value as $i => $curr) {
             $lastSuccess = $this->proxySetCookie(
                 $key . '[' . $i . ']', $curr, $expire,
-                $this->path, $this->domain, $this->secure
+                $this->path, $this->domain, $this->secure, $httpOnly
             );
             if (!$lastSuccess) {
                 $success = false;
@@ -184,15 +208,16 @@ class CookieManager
     /**
      * Set a cookie.
      *
-     * @param string $key    Name of cookie to set
-     * @param mixed  $value  Value to set
-     * @param int    $expire Cookie expiration time
+     * @param string    $key      Name of cookie to set
+     * @param mixed     $value    Value to set
+     * @param int       $expire   Cookie expiration time
+     * @param null|bool $httpOnly Whether the cookie should be "HTTP only"
      *
      * @return bool
      */
-    public function set($key, $value, $expire = 0)
+    public function set($key, $value, $expire = 0, $httpOnly = null)
     {
-        if ($success = $this->setGlobalCookie($key, $value, $expire)) {
+        if ($success = $this->setGlobalCookie($key, $value, $expire, $httpOnly)) {
             $this->cookies[$key] = $value;
         }
         return $success;

module/VuFind/src/VuFind/Cookie/CookieManagerFactory.php

@@ -64,24 +64,19 @@ class CookieManagerFactory implements FactoryInterface
         }
         $config = $container->get('VuFind\Config\PluginManager')->get('config');
         $path = '/';
-        if (isset($config->Cookies->limit_by_path)
-            && $config->Cookies->limit_by_path
-        ) {
+        if ($config->Cookies->limit_by_path ?? false) {
             $path = Console::isConsole()
                 ? '' : $container->get('Request')->getBasePath();
             if (empty($path)) {
                 $path = '/';
             }
         }
-        $secure = isset($config->Cookies->only_secure)
-            ? $config->Cookies->only_secure
-            : false;
-        $domain = isset($config->Cookies->domain)
-            ? $config->Cookies->domain
-            : null;
-        $session_name = isset($config->Cookies->session_name)
-            ? $config->Cookies->session_name
-            : null;
-        return new $requestedName($_COOKIE, $path, $domain, $secure, $session_name);
+        $secure = $config->Cookies->only_secure ?? false;
+        $httpOnly = $config->Cookies->http_only ?? true;
+        $domain = $config->Cookies->domain ?? null;
+        $session_name = $config->Cookies->session_name ?? null;
+        return new $requestedName(
+            $_COOKIE, $path, $domain, $secure, $session_name, $httpOnly
+        );
     }
 }

module/VuFind/src/VuFind/Session/ManagerFactory.php

@@ -55,6 +55,7 @@ class ManagerFactory implements FactoryInterface
     {
         $cookieManager = $container->get('VuFind\Cookie\CookieManager');
         $options = [
+            'cookie_httponly' => $cookieManager->isHttpOnly(),
             'cookie_path' => $cookieManager->getPath(),
             'cookie_secure' => $cookieManager->isSecure()
         ];

module/VuFind/tests/unit-tests/src/VuFindTest/CartTest.php

@@ -73,11 +73,11 @@ class CartTest extends \PHPUnit\Framework\TestCase
      * @return CookieManager
      */
     protected function getMockCookieManager($cookies = [], $path = '/',
-        $domain = null, $secure = false
+        $domain = null, $secure = false, $httpOnly = false
     ) {
         return $this->getMockBuilder('VuFind\Cookie\CookieManager')
             ->setMethods(['set'])
-            ->setConstructorArgs([$cookies, $path, $domain, $secure])
+            ->setConstructorArgs([$cookies, $path, $domain, $secure, $httpOnly])
             ->getMock();
     }