fix(session): regenerate new session IDs from cookies

.changeset/red-quail-bite.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes an issue where a session ID from a cookie with no matching server-side data was accepted as-is. The session now generates a new ID when the cookie value has no corresponding storage entry.

packages/astro/src/core/session/runtime.ts

@@ -53,6 +53,8 @@ export class AstroSession {
 	#dirty = false;
 	// Whether the session cookie has been set.
 	#cookieSet = false;
+	// Whether the session ID was sourced from a client cookie rather than freshly generated.
+	#sessionIDFromCookie = false;
 	// The local data is "partial" if it has not been loaded from storage yet and only
 	// contains values that have been set or deleted in-memory locally.
 	// We do this to avoid the need to block on loading data when it is only being set.
@@ -242,6 +244,7 @@ export class AstroSession {
 
 		// Create new session
 		this.#sessionID = crypto.randomUUID();
+		this.#sessionIDFromCookie = false;
 		this.#data = data;
 		this.#dirty = true;
 		await this.#setCookie();
@@ -349,6 +352,16 @@ export class AstroSession {
 		// We stored this as a devalue string, but unstorage will have parsed it as JSON
 		const raw = await storage.get<any[]>(this.#ensureSessionID());
 		if (!raw) {
+			if (this.#sessionIDFromCookie) {
+				// The session ID was supplied by the client cookie but has no corresponding
+				// server-side data. Generate a new server-controlled ID rather than
+				// accepting an unrecognized value from the client.
+				this.#sessionID = crypto.randomUUID();
+				this.#sessionIDFromCookie = false;
+				if (this.#cookieSet) {
+					await this.#setCookie();
+				}
+			}
 			// If there is no existing data in storage we don't need to merge anything
 			// and can just return the existing local data.
 			return this.#data;
@@ -404,7 +417,15 @@ export class AstroSession {
 	 * Returns the session ID, generating a new one if it does not exist.
 	 */
 	#ensureSessionID() {
-		this.#sessionID ??= this.#cookies.get(this.#cookieName)?.value ?? crypto.randomUUID();
+		if (!this.#sessionID) {
+			const cookieValue = this.#cookies.get(this.#cookieName)?.value;
+			if (cookieValue) {
+				this.#sessionID = cookieValue;
+				this.#sessionIDFromCookie = true;
+			} else {
+				this.#sessionID = crypto.randomUUID();
+			}
+		}
 		return this.#sessionID;
 	}
 

packages/astro/test/fixtures/sessions/src/pages/login-safe.ts

@@ -0,0 +1,8 @@
+import type { APIRoute } from 'astro';
+
+export const POST: APIRoute = async (context) => {
+	const body = await context.request.json() as any;
+	await context.session.regenerate();
+	context.session.set('user', body.username);
+	return Response.json({ username: body.username });
+};

packages/astro/test/fixtures/sessions/src/pages/login.ts

@@ -0,0 +1,7 @@
+import type { APIRoute } from 'astro';
+
+export const POST: APIRoute = async (context) => {
+	const body = await context.request.json() as any;
+	context.session.set('user', body.username);
+	return Response.json({ username: body.username });
+};

packages/astro/test/sessions.test.js

@@ -101,6 +101,88 @@ describe('Astro.session', () => {
 			);
 		});
 
+		it('generates a new session ID when cookie value has no server-side data', async () => {
+			const unknownId = 'nonexistent-session-id-12345';
+			const response = await fetchResponse('/login', {
+				method: 'POST',
+				headers: {
+					'Content-Type': 'application/json',
+					cookie: `astro-session=${unknownId}`,
+				},
+				body: JSON.stringify({ username: 'testuser' }),
+			});
+			assert.equal(response.ok, true);
+			const headers = Array.from(app.setCookieHeaders(response));
+			const sessionId = headers[0].split(';')[0].split('=')[1];
+			// A new ID should be generated since the cookie value had no stored data
+			assert.notEqual(sessionId, unknownId, 'Should not adopt a session ID with no stored data');
+
+			// The original ID should not give access to the new session's data
+			const secondResponse = await fetchResponse('/update', {
+				method: 'GET',
+				headers: {
+					cookie: `astro-session=${unknownId}`,
+				},
+			});
+			const secondData = await secondResponse.json();
+			assert.equal(secondData.previousValue, 'none', 'Original ID should not have session data');
+		});
+
+		it('preserves session ID when cookie value has existing server-side data', async () => {
+			const firstResponse = await fetchResponse('/update');
+			const firstHeaders = Array.from(app.setCookieHeaders(firstResponse));
+			const firstSessionId = firstHeaders[0].split(';')[0].split('=')[1];
+
+			const secondResponse = await fetchResponse('/update', {
+				method: 'GET',
+				headers: {
+					cookie: `astro-session=${firstSessionId}`,
+				},
+			});
+			const secondHeaders = Array.from(app.setCookieHeaders(secondResponse));
+			const secondSessionId = secondHeaders[0].split(';')[0].split('=')[1];
+			assert.equal(secondSessionId, firstSessionId, 'Valid session ID should be preserved');
+		});
+
+		it('regenerate() creates a new ID and cleans up the old session', async () => {
+			const firstResponse = await fetchResponse('/update', {
+				method: 'GET',
+			});
+			const firstHeaders = Array.from(app.setCookieHeaders(firstResponse));
+			const firstSessionId = firstHeaders[0].split(';')[0].split('=')[1];
+
+			const secondResponse = await fetchResponse('/login-safe', {
+				method: 'POST',
+				headers: {
+					'Content-Type': 'application/json',
+					cookie: `astro-session=${firstSessionId}`,
+				},
+				body: JSON.stringify({ username: 'testuser' }),
+			});
+			assert.equal(secondResponse.ok, true);
+			const secondHeaders = Array.from(app.setCookieHeaders(secondResponse));
+			const secondSessionIds = secondHeaders
+				.filter((h) => h.startsWith('astro-session='))
+				.map((h) => h.split(';')[0].split('=')[1]);
+			const secondSessionId = secondSessionIds[secondSessionIds.length - 1];
+
+			assert.notEqual(secondSessionId, firstSessionId, 'regenerate() should create new ID');
+
+			// Old session ID should no longer have data after regeneration
+			const thirdResponse = await fetchResponse('/update', {
+				method: 'GET',
+				headers: {
+					cookie: `astro-session=${firstSessionId}`,
+				},
+			});
+			const thirdData = await thirdResponse.json();
+			assert.equal(
+				thirdData.previousValue,
+				'none',
+				'Old session ID should have no data after regeneration',
+			);
+		});
+
 		it('can load a session by ID', async () => {
 			const firstResponse = await fetchResponse('/_actions/addToCart', {
 				method: 'POST',