Sessions with more security (#12208)

* Modifying "cookie authentication" to be more secure.

Instead of authenticating by token auth if it exists in the cookie, validate an existing session. If the session
has the user name stored as a session var, it has been authenticated. If the request has the same IP address and
user agent as the request that created the session, the request is from the user that created the session. If
both of these are true, then the session is valid, and we don't need a token auth to authenticate.

If the session is deleted before the Piwik auth cookie expires (due to garbage collection), we attempt to
re-authenticate using a secure hash of the token auth. We don't do this on every request since password_verify()
will, at BEST, add 3.5ms to every request.

* Invalidate existing sessions after user password change.

Invalidation is accomplished w/o having to individually touch sessions by:

1. Using the password hash as the piwik_auth key secret, instead of the token auth. So when a password changes, existing piwik_auth keys are no longer valid. This affects session re-authentication.
2. Saving the session start time & the last time a user's password was modified, and checking that the session start time is always newer than the password modification time.

* Set session.gc_maxlifetime to login_cookie_expire time so session data does not disappear, remove session re-auth functionality & tie cookie hash to password modified time instead of password hash to retain automatic session invalidation on password change.

* In SessionInitializer, clear other cookie values so previously stored token auths will be removed.

* Make sure anonymous user is still default user whan authenticating.

* fixing test failures

* Remove hash checking in piwik_auth cookie.

piwik_auth cookie still required since it's presence indicates we should use SessionAuth instead of the normal authentication mechanism. Since there's always a session, even if you're not logged in, PIWIK_SESSID can't be used by itself to determine this.

* Make sure session auth doesnt break in edge case where ts_password_modified column does not exist.

* Clarify session destruction/invalidation logic in SessionAuth.

* Make UsersManagerTest slightly more comprehensive.

* Use Date::now()->getTimestampUTC() instead of time() in SessionFingerprint::initialize().

* Check getUser returns correct user info in SessionAuth for sanity.

* Add SessionInitializer::getAuthCookie() back since it is @api.

* Remove IP address from session auth info + check.

* Refactor session start changes so it is started in one place only.

* Remove SessionAuthCookieFactory & deprecate auth cookie INI config vars (still needed for SessionInitializer deprectaed method).

* Make sure user can still login if ts_password_modified column is not present in database.

* Rename ts_password_modified Update class.

* Update comment in SessionAuth to include why Piwik tries to create another session.

* Restore 3.x-dev SessionInitializer for BC (deprecated), move new SessionInitializer to core, add tests for both SessionInitializers.

* Change update to 3.5 version.

* Make sure normal auth implementation is used if sessionauth fails so anonymous user can be logged in.

* On logout clear session fingerprint so same session cannot be used to login.

* Change update name + bump version, and make sure Session::rememberMe() is called before session is started (otherwise it has no effect).

* Fixing tests.

* apply review fixes

* remove test

6 files changed, 195 insertions, 61 deletions

diff --git a/plugins/Login/Auth.php b/plugins/Login/Auth.php
index 08bb693108..ab827bf12c 100644
--- a/plugins/Login/Auth.php
+++ b/plugins/Login/Auth.php
@@ -58,7 +58,7 @@ class Auth implements \Piwik\Auth
         } elseif (is_null($this->login)) {
             return $this->authenticateWithToken($this->token_auth);
         } elseif (!empty($this->login)) {
-            return $this->authenticateWithTokenOrHashToken($this->token_auth, $this->login);
+            return $this->authenticateWithLoginAndToken($this->token_auth, $this->login);
         }
 
         return new AuthResult(AuthResult::FAILURE, $this->login, $this->token_auth);
@@ -96,14 +96,13 @@ class Auth implements \Piwik\Auth
         return new AuthResult(AuthResult::FAILURE, null, $token);
     }
 
-    private function authenticateWithTokenOrHashToken($token, $login)
+    private function authenticateWithLoginAndToken($token, $login)
     {
         $user = $this->userModel->getUser($login);
 
         if (!empty($user['token_auth'])
             // authenticate either with the token or the "hash token"
-            && ((SessionInitializer::getHashTokenAuth($login, $user['token_auth']) === $token)
-                || $user['token_auth'] === $token)
+            && $user['token_auth'] === $token
         ) {
             return $this->authenticationSuccess($user);
         }
diff --git a/plugins/Login/Controller.php b/plugins/Login/Controller.php
index c933214b01..3e7eac57a6 100644
--- a/plugins/Login/Controller.php
+++ b/plugins/Login/Controller.php
@@ -39,7 +39,7 @@ class Controller extends \Piwik\Plugin\Controller
     protected $auth;
 
     /**
-     * @var SessionInitializer
+     * @var \Piwik\Session\SessionInitializer
      */
     protected $sessionInitializer;
 
@@ -65,7 +65,7 @@ class Controller extends \Piwik\Plugin\Controller
         $this->auth = $auth;
 
         if (empty($sessionInitializer)) {
-            $sessionInitializer = new SessionInitializer();
+            $sessionInitializer = new \Piwik\Session\SessionInitializer();
         }
         $this->sessionInitializer = $sessionInitializer;
     }
@@ -100,9 +100,8 @@ class Controller extends \Piwik\Plugin\Controller
                 $login = $this->getLoginFromLoginOrEmail($loginOrEmail);
 
                 $password = $form->getSubmitValue('form_password');
-                $rememberMe = $form->getSubmitValue('form_rememberme') == '1';
                 try {
-                    $this->authenticateAndRedirect($login, $password, $rememberMe);
+                    $this->authenticateAndRedirect($login, $password);
                 } catch (Exception $e) {
                     $messageNoAccess = $e->getMessage();
                 }
@@ -173,7 +172,7 @@ class Controller extends \Piwik\Plugin\Controller
         $urlToRedirect = Common::getRequestVar('url', $currentUrl, 'string');
         $urlToRedirect = Common::unsanitizeInputValue($urlToRedirect);
 
-        $this->authenticateAndRedirect($login, $password, false, $urlToRedirect, $passwordHashed = true);
+        $this->authenticateAndRedirect($login, $password, $urlToRedirect, $passwordHashed = true);
     }
 
     /**
@@ -201,12 +200,11 @@ class Controller extends \Piwik\Plugin\Controller
      *
      * @param string $login user name
      * @param string $password plain-text or hashed password
-     * @param bool $rememberMe Remember me?
      * @param string $urlToRedirect URL to redirect to, if successfully authenticated
      * @param bool $passwordHashed indicates if $password is hashed
      * @return string failure message if unable to authenticate
      */
-    protected function authenticateAndRedirect($login, $password, $rememberMe, $urlToRedirect = false, $passwordHashed = false)
+    protected function authenticateAndRedirect($login, $password, $urlToRedirect = false, $passwordHashed = false)
     {
         Nonce::discardNonce('Login.login');
 
@@ -217,7 +215,7 @@ class Controller extends \Piwik\Plugin\Controller
             $this->auth->setPasswordHash($password);
         }
 
-        $this->sessionInitializer->initSession($this->auth, $rememberMe);
+        $this->sessionInitializer->initSession($this->auth);
 
         // remove password reset entry if it exists
         $this->passwordResetter->removePasswordResetInfo($login);
@@ -357,14 +355,12 @@ class Controller extends \Piwik\Plugin\Controller
     /**
      * Clear session information
      *
-     * @param none
      * @return void
      */
     public static function clearSession()
     {
-        $authCookieName = Config::getInstance()->General['login_cookie_name'];
-        $cookie = new Cookie($authCookieName);
-        $cookie->delete();
+        $sessionFingerprint = new Session\SessionFingerprint();
+        $sessionFingerprint->clear();
 
         Session::expireSessionCookie();
     }
diff --git a/plugins/Login/Login.php b/plugins/Login/Login.php
index c88756a160..9a88e9f48a 100644
--- a/plugins/Login/Login.php
+++ b/plugins/Login/Login.php
@@ -15,6 +15,7 @@ use Piwik\Container\StaticContainer;
 use Piwik\Cookie;
 use Piwik\FrontController;
 use Piwik\Piwik;
+use Piwik\Session;
 
 /**
  *
@@ -27,11 +28,11 @@ class Login extends \Piwik\Plugin
     public function registerEvents()
     {
         $hooks = array(
-            'Request.initAuthenticationObject' => 'initAuthenticationObject',
             'User.isNotAuthorized'             => 'noAccess',
             'API.Request.authenticate'         => 'ApiRequestAuthenticate',
             'AssetManager.getJavaScriptFiles'  => 'getJsFiles',
-            'AssetManager.getStylesheetFiles'  => 'getStylesheetFiles'
+            'AssetManager.getStylesheetFiles'  => 'getStylesheetFiles',
+            'Session.beforeSessionStart'       => 'beforeSessionStart',
         );
         return $hooks;
     }
@@ -47,6 +48,26 @@ class Login extends \Piwik\Plugin
         $stylesheetFiles[] = "plugins/Login/stylesheets/variables.less";
     }
 
+    public function beforeSessionStart()
+    {
+        if (!$this->shouldHandleRememberMe()) {
+            return;
+        }
+
+        // if this is a login request & form_rememberme was set, change the session cookie expire time before starting the session
+        $rememberMe = isset($_POST['form_rememberme']) ? $_POST['form_rememberme'] : null;
+        if ($rememberMe == '1') {
+            Session::rememberMe(Config::getInstance()->General['login_cookie_expire']);
+        }
+    }
+
+    private function shouldHandleRememberMe()
+    {
+        $module = Common::getRequestVar('module', false);
+        $action = Common::getRequestVar('action', false);
+        return $module == 'Login' && (empty($action) || $action == 'login');
+    }
+
     /**
      * Redirects to Login form with error message.
      * Listens to User.isNotAuthorized hook.
@@ -82,35 +103,12 @@ class Login extends \Piwik\Plugin
     }
 
     /**
-     * Initializes the authentication object.
-     * Listens to Request.initAuthenticationObject hook.
-     */
-    function initAuthenticationObject($activateCookieAuth = false)
-    {
-        $this->initAuthenticationFromCookie(StaticContainer::getContainer()->get('Piwik\Auth'), $activateCookieAuth);
-    }
-
-    /**
      * @param $auth
+     * @deprecated authenticating via cookie is handled in core by SessionAuth
      */
     public static function initAuthenticationFromCookie(\Piwik\Auth $auth, $activateCookieAuth)
     {
-        if (self::isModuleIsAPI() && !$activateCookieAuth) {
-            return;
-        }
-
-        $authCookieName = Config::getInstance()->General['login_cookie_name'];
-        $authCookieExpiry = 0;
-        $authCookiePath = Config::getInstance()->General['login_cookie_path'];
-        $authCookie = new Cookie($authCookieName, $authCookieExpiry, $authCookiePath);
-        $defaultLogin = 'anonymous';
-        $defaultTokenAuth = 'anonymous';
-        if ($authCookie->isCookieFound()) {
-            $defaultLogin = $authCookie->get('login');
-            $defaultTokenAuth = $authCookie->get('token_auth');
-        }
-        $auth->setLogin($defaultLogin);
-        $auth->setTokenAuth($defaultTokenAuth);
+        // empty
     }
 
 }
diff --git a/plugins/Login/SessionInitializer.php b/plugins/Login/SessionInitializer.php
index 9db9da7dec..e011186ae7 100644
--- a/plugins/Login/SessionInitializer.php
+++ b/plugins/Login/SessionInitializer.php
@@ -21,15 +21,10 @@ use Piwik\ProxyHttp;
 use Piwik\Session;
 
 /**
- * Initializes authenticated sessions using an Auth implementation.
- *
- * If a user is authenticated, a browser cookie is created so the user will be remembered
- * until the cookie is destroyed.
- *
- * Plugins can override SessionInitializer behavior by extending this class and
- * overriding methods. In order for these changes to have effect, however, an instance of
- * the derived class must be used by the Login/Controller.
+ * This SessionInitializer is no longer used, but is kept for backwards compatibility.
+ * Session management no longer uses the piwik_auth cookie.
  *
+ * @deprecated
  * @api
  */
 class SessionInitializer
@@ -192,6 +187,8 @@ class SessionInitializer
         $cookie->setSecure(ProxyHttp::isHttps());
         $cookie->setHttpOnly(true);
         $cookie->save();
+
+        return $cookie;
     }
 
     protected function regenerateSessionId()
diff --git a/plugins/Login/tests/Integration/LoginTest.php b/plugins/Login/tests/Integration/LoginTest.php
index 9a80b2aaf8..9f43512f0b 100644
--- a/plugins/Login/tests/Integration/LoginTest.php
+++ b/plugins/Login/tests/Integration/LoginTest.php
@@ -261,16 +261,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertSuperUserLogin($rc, 'user');
     }
 
-    public function test_authenticate_successLoginAndHashedTokenAuth()
-    {
-        $user = $this->_setUpUser();
-        $hash = \Piwik\Plugins\Login\SessionInitializer::getHashTokenAuth($user['login'], $user['tokenAuth']);
-
-        // valid login & hashed token auth
-        $rc = $this->authenticate($user['login'], $tokenAuth = $hash);
-        $this->assertUserLogin($rc);
-    }
-
     public function test_authenticate_successWithValidPassword()
     {
         $user = $this->_setUpUser();
diff --git a/plugins/Login/tests/Integration/SessionInitializerTest.php b/plugins/Login/tests/Integration/SessionInitializerTest.php
new file mode 100644
index 0000000000..9d2f7bb098
--- /dev/null
+++ b/plugins/Login/tests/Integration/SessionInitializerTest.php
@@ -0,0 +1,154 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ *
+ */
+
+namespace Piwik\Plugins\Login\tests\Integration;
+
+use Piwik\Auth;
+use Piwik\AuthResult;
+use Piwik\Container\StaticContainer;
+use Piwik\Cookie;
+use Piwik\Plugins\Login\SessionInitializer;
+use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
+
+/**
+ * Since the original SessionInitializer is still in use, it needs to
+ * work. These light tests ensure it's still working.
+ */
+class SessionInitializerTest extends IntegrationTestCase
+{
+    public function setUp()
+    {
+        parent::setUp();
+
+        // AuthResult is in Auth.php, so we need to make sure that class gets loaded
+        // by loading Auth.
+        StaticContainer::get(Auth::class);
+    }
+
+    public function test_initSession_CreatesCookie_WhenAuthenticationIsSuccessful()
+    {
+        $this->assertAuthCookieIsAbsent();
+
+        $sessionInitializer = new TestSessionInitializer();
+        $sessionInitializer->initSession($this->makeMockAuth(AuthResult::SUCCESS), true);
+
+        $this->assertAuthCookieIsCreated($sessionInitializer->cookie);
+    }
+
+    public function test_initSession_DeletesCookie_WhenAuthenticationFailed()
+    {
+        $this->createAuthCookie();
+
+        try {
+            $sessionInitializer = new TestSessionInitializer();
+            $sessionInitializer->initSession($this->makeMockAuth(AuthResult::FAILURE), true);
+
+            $this->fail('Expected exception to be thrown.');
+        } catch (\Exception $ex) {
+            // empty
+        }
+
+        $this->assertAuthCookieIsDeleted($sessionInitializer->cookie);
+    }
+
+    private function makeMockAuth($resultCode)
+    {
+        return new MockAuth($resultCode);
+    }
+
+    private function assertAuthCookieIsAbsent()
+    {
+        $this->assertArrayNotHasKey('piwik_auth', $_COOKIE);
+    }
+
+    private function assertAuthCookieIsCreated(Cookie $cookie)
+    {
+        $this->assertContains('login=czo5OiJ0ZXN0bG9naW4iOw==:token_auth=czozMjoiOWU5MDYxZjk2MDI0YTY3NWFmOGFkNWZmNmNiZGY2ZGMiOw==',
+            $cookie->generateContentString());
+    }
+
+    private function createAuthCookie()
+    {
+        $_COOKIE['piwik_auth'] = 'login=czo5OiJ0ZXN0bG9naW4iOw==:token_auth=czozMjoiOWU5MDYxZjk2MDI0YTY3NWFmOGFkNWZmNmNiZGY2ZGMiOw==';
+    }
+
+    private function assertAuthCookieIsDeleted(Cookie $cookie)
+    {
+        $this->assertEquals('', $cookie->generateContentString());
+    }
+}
+
+class TestSessionInitializer extends SessionInitializer
+{
+    /**
+     * @var Cookie
+     */
+    public $cookie;
+
+    protected function regenerateSessionId()
+    {
+        // empty
+    }
+
+    protected function getAuthCookie($rememberMe)
+    {
+        $this->cookie = parent::getAuthCookie($rememberMe);
+        return $this->cookie;
+    }
+}
+
+class MockAuth implements Auth
+{
+    private $result;
+
+    public function __construct($resultCode)
+    {
+        $this->result = new AuthResult($resultCode, 'testlogin', 'dummytokenauth');
+    }
+
+    public function getName()
+    {
+        // empty
+    }
+
+    public function setTokenAuth($token_auth)
+    {
+        // empty
+    }
+
+    public function getLogin()
+    {
+        // empty
+    }
+
+    public function getTokenAuthSecret()
+    {
+        // empty
+    }
+
+    public function setLogin($login)
+    {
+        // empty
+    }
+
+    public function setPassword($password)
+    {
+        // empty
+    }
+
+    public function setPasswordHash($passwordHash)
+    {
+        // empty
+    }
+
+    public function authenticate()
+    {
+        return $this->result;
+    }
+}
\ No newline at end of file