Regenerate the session id on login

src/CatLab/OpenIDClient/Module.php

@@ -112,6 +112,10 @@ public function setUserMapper ()
      */
 	public function login (Request $request, User $user)
 	{
+		// A logged in session gets a new id. The session data (such as
+		// the post-login redirect) is kept.
+		$this->regenerateSessionId ($request);
+
 		$request->getSession ()->set ('catlab-user-id', $user->getId ());
 		$request->getSession ()->set ('catlab-openid-access-token', $user->getAccessToken ());
 
@@ -120,6 +124,36 @@ public function login (Request $request, User $user)
 		return $this->postLogin ($request, $user);
 	}
 
+	/**
+	 * Give the active session a new id, keeping its data.
+	 * Uses Session::regenerate () where Neuron has it; older Neuron
+	 * versions get the same through PHP directly.
+	 * @param Request $request
+	 * @throws DataNotSet
+	 */
+	private function regenerateSessionId (Request $request)
+	{
+		$session = $request->getSession ();
+
+		if (method_exists ($session, 'regenerate'))
+		{
+			$session->regenerate ();
+			return;
+		}
+
+		if (session_status () !== PHP_SESSION_ACTIVE)
+		{
+			return;
+		}
+
+		if (headers_sent () && ini_get ('session.use_cookies'))
+		{
+			return;
+		}
+
+		session_regenerate_id (true);
+	}
+
 	/**
 	 * Logout user
 	 * @param Request $request

tests/Fakes/RecordingSession.php

@@ -0,0 +1,33 @@
+<?php
+
+namespace Tests\Fakes;
+
+use Neuron\Net\Session;
+use Neuron\SessionHandlers\SessionHandler;
+
+/**
+ * A Session that has regenerate() (as newer Neuron versions do) and
+ * records the order of regenerate() and set() calls.
+ */
+class RecordingSession extends Session
+{
+    /** @var string[] */
+    public $calls = [];
+
+    public function __construct()
+    {
+        parent::__construct(new SessionHandler());
+    }
+
+    public function regenerate()
+    {
+        $this->calls[] = 'regenerate';
+        return true;
+    }
+
+    public function set($key, $value)
+    {
+        $this->calls[] = 'set:' . $key;
+        parent::set($key, $value);
+    }
+}

tests/ModuleTest.php

@@ -9,6 +9,7 @@
 use Neuron\Net\Session;
 use Neuron\SessionHandlers\SessionHandler;
 use PHPUnit\Framework\TestCase;
+use Tests\Fakes\RecordingSession;
 
 /**
  * Session bookkeeping done by the module on login/logout. Neuron's Session
@@ -55,6 +56,38 @@ public function testLoginStoresIdentityInSessionAndNotifiesListeners()
         $this->assertSame(302, $response->getStatus());
     }
 
+    public function testLoginRegeneratesTheSessionIdBeforeStoringTheIdentity()
+    {
+        $_SESSION['post-login-redirect'] = 'https://app.example.com/after-login';
+
+        $session = new RecordingSession();
+        $request = new Request();
+        $request->setSession($session);
+
+        $response = (new Module())->login($request, $this->user(7, 'tok'));
+
+        $this->assertSame('regenerate', $session->calls[0]);
+        $this->assertSame(1, count(array_keys($session->calls, 'regenerate')));
+        $this->assertSame(
+            [ 'regenerate', 'set:catlab-user-id', 'set:catlab-openid-access-token' ],
+            array_slice($session->calls, 0, 3)
+        );
+
+        // What was in the session before login is still there afterwards.
+        $this->assertSame(7, $_SESSION['catlab-user-id']);
+        $this->assertSame('https://app.example.com/after-login', $response->getHeaders()['Location']);
+    }
+
+    public function testLoginWithoutAnActiveSessionDoesNotStartOne()
+    {
+        $this->assertSame(PHP_SESSION_NONE, session_status());
+
+        (new Module())->login($this->request(), $this->user(7, 'tok'));
+
+        $this->assertSame(PHP_SESSION_NONE, session_status());
+        $this->assertSame(7, $_SESSION['catlab-user-id']);
+    }
+
     public function testLoginRedirectsToTheStoredReturnUrlOnce()
     {
         $_SESSION['post-login-redirect'] = 'https://app.example.com/after-login';