From 342b8c4a426c4b2a8f951a4f513c2116b01f5cd7 Mon Sep 17 00:00:00 2001
From: Pierre Rudloff <contact@rudloff.pro>
Date: Wed, 21 Oct 2020 23:04:29 +0200
Subject: [PATCH] Use secure session cookies (fixes #321)

---
 classes/Factory/SessionFactory.php | 18 ++++++++++++++++--
 index.php                          |  2 +-
 tests/ControllerTest.php           |  2 +-
 tests/LocaleManagerTest.php        |  3 ++-
 tests/LocaleMiddlewareTest.php     |  2 +-
 tests/ViewFactoryTest.php          |  4 ++--
 6 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/classes/Factory/SessionFactory.php b/classes/Factory/SessionFactory.php
index 3d291b7..b819b2f 100644
--- a/classes/Factory/SessionFactory.php
+++ b/classes/Factory/SessionFactory.php
@@ -7,6 +7,7 @@
 namespace Alltube\Factory;
 
 use Aura\Session\Session;
+use Slim\Container;
 
 /**
  * Manage sessions.
@@ -17,11 +18,24 @@ class SessionFactory
     /**
      * Get the current session.
      *
+     * @param Container $container
      * @return Session
      */
-    public static function create()
+    public static function create(Container $container)
     {
         $session_factory = new \Aura\Session\SessionFactory();
-        return $session_factory->newInstance($_COOKIE);
+        $session = $session_factory->newInstance($_COOKIE);
+
+        $session->setCookieParams(['httponly' => true]);
+
+        $request = $container->get('request');
+        if (
+            in_array('https', $request->getHeader('X-Forwarded-Proto'))
+            || $request->getUri()->getScheme() == 'https'
+        ) {
+            $session->setCookieParams(['secure' => true]);
+        }
+
+        return $session;
     }
 }
diff --git a/index.php b/index.php
index d6cdd7c..f386247 100644
--- a/index.php
+++ b/index.php
@@ -34,7 +34,7 @@ try {
     $container['config'] = ConfigFactory::create($container);
 
     // Session.
-    $container['session'] = SessionFactory::create();
+    $container['session'] = SessionFactory::create($container);
 
     // Locales.
     $container['locale'] = LocaleManagerFactory::create($container);
diff --git a/tests/ControllerTest.php b/tests/ControllerTest.php
index 535df78..b2695b7 100644
--- a/tests/ControllerTest.php
+++ b/tests/ControllerTest.php
@@ -68,7 +68,7 @@ abstract class ControllerTest extends BaseTest
         $this->request = Request::createFromEnvironment(Environment::mock());
         $this->response = new Response();
         $this->container['config'] = Config::fromFile($this->getConfigFile());
-        $this->container['session'] = SessionFactory::create();
+        $this->container['session'] = SessionFactory::create($this->container);
         $this->container['locale'] = LocaleManagerFactory::create($this->container);
         $this->container['view'] = ViewFactory::create($this->container, $this->request);
         $this->container['logger'] = new NullLogger();
diff --git a/tests/LocaleManagerTest.php b/tests/LocaleManagerTest.php
index b2010a4..d06af1f 100644
--- a/tests/LocaleManagerTest.php
+++ b/tests/LocaleManagerTest.php
@@ -9,6 +9,7 @@ namespace Alltube\Test;
 use Alltube\Factory\SessionFactory;
 use Alltube\Locale;
 use Alltube\LocaleManager;
+use Slim\Container;
 
 /**
  * Unit tests for the LocaleManagerTest class.
@@ -28,7 +29,7 @@ class LocaleManagerTest extends BaseTest
     protected function setUp(): void
     {
         $_SESSION[LocaleManager::class]['locale'] = 'foo_BAR';
-        $this->localeManager = new LocaleManager(SessionFactory::create());
+        $this->localeManager = new LocaleManager(SessionFactory::create(new Container()));
     }
 
     /**
diff --git a/tests/LocaleMiddlewareTest.php b/tests/LocaleMiddlewareTest.php
index 2ea66da..48977d8 100644
--- a/tests/LocaleMiddlewareTest.php
+++ b/tests/LocaleMiddlewareTest.php
@@ -41,7 +41,7 @@ class LocaleMiddlewareTest extends BaseTest
     protected function setUp(): void
     {
         $this->container = new Container();
-        $this->container['session'] = SessionFactory::create();
+        $this->container['session'] = SessionFactory::create($this->container);
         $this->container['locale'] = LocaleManagerFactory::create($this->container);
         $this->middleware = new LocaleMiddleware($this->container);
     }
diff --git a/tests/ViewFactoryTest.php b/tests/ViewFactoryTest.php
index 62990ef..a4d4f54 100644
--- a/tests/ViewFactoryTest.php
+++ b/tests/ViewFactoryTest.php
@@ -31,7 +31,7 @@ class ViewFactoryTest extends BaseTest
     public function testCreate()
     {
         $container = new Container();
-        $container['session'] = SessionFactory::create();
+        $container['session'] = SessionFactory::create($container);
         $container['locale'] = LocaleManagerFactory::create($container);
         $view = ViewFactory::create($container);
         $this->assertInstanceOf(Smarty::class, $view);
@@ -47,7 +47,7 @@ class ViewFactoryTest extends BaseTest
     public function testCreateWithXForwardedProto()
     {
         $container = new Container();
-        $container['session'] = SessionFactory::create();
+        $container['session'] = SessionFactory::create($container);
         $container['locale'] = LocaleManagerFactory::create($container);
         $request = Request::createFromEnvironment(Environment::mock());
         $view = ViewFactory::create($container, $request->withHeader('X-Forwarded-Proto', 'https'));