From 6c91f1ba5cc24dcfb018823c285392640642c584 Mon Sep 17 00:00:00 2001 From: Romain Lanz Date: Sat, 17 Oct 2026 15:27:07 +0000 Subject: [PATCH 01/14] fix(stores): do not write back a session destroyed during a request A request that read a session and another that destroyed it (logout, SessionCollection.destroy) could interleave. The first request's write then recreated the session with the data it had read: setex in Redis, an upsert in SQL, a plain file write, a PutItem in DynamoDB. The redis, database, file and dynamodb stores now remember the ids they found while reading. A store serves one request, so a write to one of those ids only updates a session that still exists: SET ... XX in Redis, an UPDATE without INSERT in SQL, a file opened with "r+", and a PutItem guarded by attribute_exists in DynamoDB. A session the store did not read is written as before. SessionCollection is a container singleton, so it now creates a store per operation instead of keeping one for the life of the process. --- src/session_collection.ts | 14 ++++++++--- src/stores/database.ts | 32 +++++++++++++++++------ src/stores/dynamodb.ts | 25 ++++++++++++++++-- src/stores/file.ts | 39 ++++++++++++++++++++++++++--- src/stores/redis.ts | 19 ++++++++++++-- tests/stores/database_store.spec.ts | 24 ++++++++++++++++++ tests/stores/dynamodb_store.spec.ts | 23 +++++++++++++++++ tests/stores/file_store.spec.ts | 26 +++++++++++++++++++ tests/stores/redis_store.spec.ts | 24 ++++++++++++++++++ 9 files changed, 208 insertions(+), 18 deletions(-) diff --git a/src/session_collection.ts b/src/session_collection.ts index f7c61f6..57dc484 100644 --- a/src/session_collection.ts +++ b/src/session_collection.ts @@ -36,7 +36,7 @@ import type { * ``` */ export class SessionCollection { - #store: SessionStoreWithTaggingContract + #config: ResolvedSessionConfig /** * Creates a new SessionCollection instance @@ -44,8 +44,16 @@ export class SessionCollection { * @param config - Resolved session configuration */ constructor(config: ResolvedSessionConfig) { - const storeFactory = config.stores[config.store] - this.#store = storeFactory(null as any, config) as SessionStoreWithTaggingContract + this.#config = config + } + + /** + * A store serves one request and remembers the sessions it read. + * The collection is a singleton, so each operation gets a new store. + */ + get #store(): SessionStoreWithTaggingContract { + const storeFactory = this.#config.stores[this.#config.store] + return storeFactory(null as any, this.#config) as SessionStoreWithTaggingContract } /** diff --git a/src/stores/database.ts b/src/stores/database.ts index c7e80fe..ba394aa 100644 --- a/src/stores/database.ts +++ b/src/stores/database.ts @@ -31,6 +31,13 @@ export class DatabaseStore implements SessionStoreWithTaggingContract { #ttlSeconds: number #gcProbability: number + /** + * Ids of the sessions this store found while reading. A store serves + * one request, so a write to one of them must not bring it back once + * another request destroyed it. + */ + #found = new Set() + /** * Creates a new database store instance * @@ -143,12 +150,14 @@ export class DatabaseStore implements SessionStoreWithTaggingContract { return null } + this.#found.add(sessionId) return this.#parseSessionData(row.data, sessionId) } /** - * Writes session values to the database with expiry. - * Uses UPSERT to handle both new and existing sessions. + * Writes session values to the database with expiry. A session read by + * this store is only updated (no insert), so a logout or a destroy that + * happened during the request is not undone. Other sessions are upserted. * * @param sessionId - Session identifier * @param values - Session data to store @@ -162,12 +171,19 @@ export class DatabaseStore implements SessionStoreWithTaggingContract { const message = new MessageBuilder().build(values, undefined, sessionId) const expiresAt = new Date(Date.now() + this.#ttlSeconds * 1000) - await this.#client - .insertQuery() - .table(this.#tableName) - .insert({ id: sessionId, data: message, expires_at: expiresAt }) - .knexQuery.onConflict('id') - .merge(['data', 'expires_at']) + if (this.#found.has(sessionId)) { + await this.#client + .from(this.#tableName) + .where('id', sessionId) + .update({ data: message, expires_at: expiresAt }) + } else { + await this.#client + .insertQuery() + .table(this.#tableName) + .insert({ id: sessionId, data: message, expires_at: expiresAt }) + .knexQuery.onConflict('id') + .merge(['data', 'expires_at']) + } await this.#collectGarbage() } diff --git a/src/stores/dynamodb.ts b/src/stores/dynamodb.ts index 5966563..4d34c9f 100644 --- a/src/stores/dynamodb.ts +++ b/src/stores/dynamodb.ts @@ -16,6 +16,7 @@ import { PutItemCommand, DeleteItemCommand, UpdateItemCommand, + ConditionalCheckFailedException, } from '@aws-sdk/client-dynamodb' import debug from '../debug.ts' @@ -62,6 +63,13 @@ export class DynamoDBStore implements SessionStoreContract { */ #expiresAtAttribute: string = 'expires_at' + /** + * Ids of the sessions this store found while reading. A store serves + * one request, so a write to one of them must not bring it back once + * another request destroyed it. + */ + #found = new Set() + /** * Creates a new DynamoDB store instance * @@ -129,6 +137,8 @@ export class DynamoDBStore implements SessionStoreContract { return null } + this.#found.add(sessionId) + /** * Verify contents with the session id and return them as an object. The verify * method can fail when the contents is not JSON. @@ -141,7 +151,9 @@ export class DynamoDBStore implements SessionStoreContract { } /** - * Writes session values to DynamoDB with expiry + * Writes session values to DynamoDB with expiry. A session read by this + * store is only updated while it still exists, so a logout or a destroy + * that happened during the request is not undone. * * @param sessionId - Session identifier * @param values - Session data to store @@ -160,9 +172,18 @@ export class DynamoDBStore implements SessionStoreContract { [this.#valueAttribute]: message, [this.#expiresAtAttribute]: Date.now() + this.#ttlSeconds * 1000, }), + ...(this.#found.has(sessionId) && { + ConditionExpression: 'attribute_exists(#key)', + ExpressionAttributeNames: { '#key': this.#keyAttribute }, + }), }) - await this.#client.send(command) + try { + await this.#client.send(command) + } catch (error) { + if (!(error instanceof ConditionalCheckFailedException)) throw error + debug('dynamodb store: skipped write of destroyed session %s', sessionId) + } } /** diff --git a/src/stores/file.ts b/src/stores/file.ts index e0f81ca..24a27bd 100644 --- a/src/stores/file.ts +++ b/src/stores/file.ts @@ -11,7 +11,7 @@ import type { Stats } from 'node:fs' import { dirname, join } from 'node:path' import string from '@adonisjs/core/helpers/string' import { MessageBuilder } from '@adonisjs/core/helpers' -import { access, mkdir, readFile, rm, writeFile, utimes, stat } from 'node:fs/promises' +import { access, mkdir, open, readFile, rm, writeFile, utimes, stat } from 'node:fs/promises' import debug from '../debug.ts' import type { FileStoreConfig, SessionData, SessionStoreContract } from '../types.ts' @@ -36,6 +36,13 @@ export class FileStore implements SessionStoreContract { */ #age: string | number + /** + * Ids of the sessions this store found while reading. A store serves + * one request, so a write to one of them must not bring it back once + * another request destroyed it. + */ + #found = new Set() + /** * Creates a new file store instance * @@ -145,6 +152,8 @@ export class FileStore implements SessionStoreContract { return null } + this.#found.add(sessionId) + /** * Verify contents with the session id and return them as an object. The verify * method can fail when the contents is not JSON> @@ -157,7 +166,9 @@ export class FileStore implements SessionStoreContract { } /** - * Writes the session data to the disk as a string + * Writes the session data to the disk as a string. The file of a session + * read by this store is only rewritten while it still exists, so a logout + * or a destroy that happened during the request is not undone. * * @param sessionId - Session identifier * @param values - Session data to store @@ -171,7 +182,29 @@ export class FileStore implements SessionStoreContract { const filePath = this.#getFilePath(sessionId) const message = new MessageBuilder().build(values, undefined, sessionId) - await this.#outputFile(filePath, message) + if (!this.#found.has(sessionId)) { + await this.#outputFile(filePath, message) + return + } + + /** + * "r+" fails when the file is gone. A file removed after it was + * opened loses the write along with its inode. + */ + let file + try { + file = await open(filePath, 'r+') + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return + throw error + } + + try { + await file.truncate(0) + await file.write(message, 0, 'utf-8') + } finally { + await file.close() + } } /** diff --git a/src/stores/redis.ts b/src/stores/redis.ts index 4ad96e9..1ef93bf 100644 --- a/src/stores/redis.ts +++ b/src/stores/redis.ts @@ -32,6 +32,13 @@ export class RedisStore implements SessionStoreWithTaggingContract { */ #ttlSeconds: number + /** + * Ids of the sessions this store found while reading. A store serves + * one request, so a write to one of them must not bring it back once + * another request destroyed it. + */ + #found = new Set() + /** * Creates a new Redis store instance * @@ -132,6 +139,8 @@ export class RedisStore implements SessionStoreWithTaggingContract { return null } + this.#found.add(sessionId) + /** * Verify contents with the session id and return them as an object. The verify * method can fail when the contents is not JSON> @@ -144,7 +153,9 @@ export class RedisStore implements SessionStoreWithTaggingContract { } /** - * Writes session values to Redis with expiry + * Writes session values to Redis with expiry. A session read by this + * store is only updated while it still exists (`XX`), so a logout or + * a destroy that happened during the request is not undone. * * @param sessionId - Session identifier * @param values - Session data to store @@ -156,7 +167,11 @@ export class RedisStore implements SessionStoreWithTaggingContract { debug('redis store: writing session data %s, %O', sessionId, values) const message = new MessageBuilder().build(values, undefined, sessionId) - await this.#connection.setex(sessionId, this.#ttlSeconds, message) + if (this.#found.has(sessionId)) { + await this.#connection.set(sessionId, message, 'EX', this.#ttlSeconds, 'XX') + } else { + await this.#connection.set(sessionId, message, 'EX', this.#ttlSeconds) + } } /** diff --git a/tests/stores/database_store.spec.ts b/tests/stores/database_store.spec.ts index 54036f4..640bc0d 100644 --- a/tests/stores/database_store.spec.ts +++ b/tests/stores/database_store.spec.ts @@ -168,6 +168,30 @@ test.group('Database store', (group) => { assert.isNull(row) }) + test('do not write back a session destroyed after it was read', async ({ assert }) => { + const request = new DatabaseStore(db.connection(), '2 hours') + const logout = new DatabaseStore(db.connection(), '2 hours') + + await logout.write(sessionId, { user: 1 }) + assert.deepEqual(await request.read(sessionId), { user: 1 }) + + await logout.destroy(sessionId) + await request.write(sessionId, { user: 1, visits: 2 }) + + const row = await db.from('sessions').where('id', sessionId).first() + assert.isNull(row) + }) + + test('update a session read by the store while it exists', async ({ assert }) => { + const store = new DatabaseStore(db.connection(), '2 hours') + + await store.write(sessionId, { visits: 1 }) + await store.read(sessionId) + await store.write(sessionId, { visits: 2 }) + + assert.deepEqual(await store.read(sessionId), { visits: 2 }) + }) + test('update session expiry on touch', async ({ assert }) => { const store = new DatabaseStore(db.connection(), 10) await store.write(sessionId, { message: 'hello-world' }) diff --git a/tests/stores/dynamodb_store.spec.ts b/tests/stores/dynamodb_store.spec.ts index 57ddb53..d0c9598 100644 --- a/tests/stores/dynamodb_store.spec.ts +++ b/tests/stores/dynamodb_store.spec.ts @@ -123,6 +123,29 @@ test.group('DynamoDB store', (group) => { const expiryPostTouch = await getExpiry(client, defaultTableName, defaultKeyName, sessionId) assert.isAbove(expiryPostTouch, expiry) }).timeout(3000) + + test('do not write back a session destroyed after it was read', async ({ assert }) => { + const request = new DynamoDBStore(client, '2 hours') + const logout = new DynamoDBStore(client, '2 hours') + + await logout.write(sessionId, { user: 1 }) + assert.deepEqual(await request.read(sessionId), { user: 1 }) + + await logout.destroy(sessionId) + await request.write(sessionId, { user: 1, visits: 2 }) + + assert.isNull(await getSession(client, defaultTableName, defaultKeyName, sessionId)) + }) + + test('update a session read by the store while it exists', async ({ assert }) => { + const store = new DynamoDBStore(client, '2 hours') + + await store.write(sessionId, { visits: 1 }) + await store.read(sessionId) + await store.write(sessionId, { visits: 2 }) + + assert.deepEqual(await store.read(sessionId), { visits: 2 }) + }) }) test.group('DynamoDB store | Custom table name', (group) => { diff --git a/tests/stores/file_store.spec.ts b/tests/stores/file_store.spec.ts index 89cc0fd..cf97125 100644 --- a/tests/stores/file_store.spec.ts +++ b/tests/stores/file_store.spec.ts @@ -120,6 +120,32 @@ test.group('File store', () => { await assert.fileNotExists('1234.txt') }) + test('do not write back a session destroyed after it was read', async ({ assert, fs }) => { + const request = new FileStore({ location: fs.basePath }, '2 hours') + const logout = new FileStore({ location: fs.basePath }, '2 hours') + + await logout.write('1234', { user: 1 }) + assert.deepEqual(await request.read('1234'), { user: 1 }) + + await logout.destroy('1234') + await request.write('1234', { user: 1, visits: 2 }) + + await assert.fileNotExists('1234.txt') + }) + + test('update a session read by the store while it exists', async ({ assert, fs }) => { + const store = new FileStore({ location: fs.basePath }, '2 hours') + + await store.write('1234', { message: 'a longer message than the next one' }) + await store.read('1234') + await store.write('1234', { message: 'short' }) + + await assert.fileEquals( + '1234.txt', + JSON.stringify({ message: { message: 'short' }, purpose: '1234' }) + ) + }) + test('update session expiry on touch', async ({ assert, fs }) => { const sessionId = '1234' diff --git a/tests/stores/redis_store.spec.ts b/tests/stores/redis_store.spec.ts index 364dd94..7aa8d2d 100644 --- a/tests/stores/redis_store.spec.ts +++ b/tests/stores/redis_store.spec.ts @@ -117,6 +117,30 @@ test.group('Redis store', (group) => { assert.isAtLeast(expiryPostTouch, 9) }).disableTimeout() + test('do not write back a session destroyed after it was read', async ({ assert }) => { + const request = new RedisStore(redis.connection('main'), '2 hours') + const logout = new RedisStore(redis.connection('main'), '2 hours') + + await logout.write(sessionId, { user: 1 }) + assert.deepEqual(await request.read(sessionId), { user: 1 }) + + await logout.destroy(sessionId) + await request.write(sessionId, { user: 1, visits: 2 }) + + assert.isNull(await redis.get(sessionId)) + }) + + test('update a session read by the store while it exists', async ({ assert }) => { + const store = new RedisStore(redis.connection('main'), '2 hours') + + await store.write(sessionId, { visits: 1 }) + await store.read(sessionId) + await store.write(sessionId, { visits: 2 }) + + assert.deepEqual(await store.read(sessionId), { visits: 2 }) + assert.isAbove(await redis.ttl(sessionId), 7000) + }) + test('tag a session with a user id', async ({ assert }) => { const session = new RedisStore(redis.connection('main'), '2 hours') From 8f067b287071885caeba12945058180d0ef4e229 Mon Sep 17 00:00:00 2001 From: Romain Lanz Date: Sat, 17 Oct 2026 15:33:13 +0000 Subject: [PATCH 02/14] fix(session): start a new id when the cookie names an ended session When the session cookie named a session the store no longer had (expired, destroyed by a logout or SessionCollection.destroy), Session kept that id and wrote the new guest session under it. A late request that had read the old session could then find a key to write over, and the id of an ended session lived on. initiate now picks a fresh randomUUID() when the cookie carries an id the store does not find. The session is treated as fresh: it has no previous id to destroy or migrate from. The increment/decrement tests sent a cookie with no stored data and expected the write under that id; they now start without a cookie. --- src/session.ts | 11 ++++++++++ tests/session.spec.ts | 48 +++++++++++++++++++++++++++++++++++-------- 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/src/session.ts b/src/session.ts index 9ecf3a0..2db93ac 100644 --- a/src/session.ts +++ b/src/session.ts @@ -242,6 +242,17 @@ export class Session extends Macroable { const contents = await this.#store.read(this.#sessionId) this.#valuesStore = new ValuesStore(contents) + /** + * The cookie names a session the store does not have (expired or + * destroyed). Start a fresh session under a new id, so the id of + * an ended session is never used again. + */ + if (!contents && this.#sessionIdFromCookie) { + debug('session %s not found, starting a new session', this.#sessionIdFromCookie) + this.#sessionIdFromCookie = undefined + this.#sessionId = randomUUID() + } + /** * Extract flash messages from the store and keep a local * copy of it. diff --git a/tests/session.spec.ts b/tests/session.spec.ts index b411eba..b51347d 100644 --- a/tests/session.spec.ts +++ b/tests/session.spec.ts @@ -265,7 +265,7 @@ test.group('Session', (group) => { }) test('initiate value with 1 on increment', async ({ assert }) => { - let sessionId = randomUUID() + let sessionId: string | undefined const server = httpServer.create(async (req, res) => { const request = new RequestFactory().merge({ req, res, encryption }).create() @@ -276,13 +276,12 @@ test.group('Session', (group) => { await session.initiate(false) session.increment('visits') + sessionId = session.sessionId await session.commit() response.finish() }) - const sessionIdCookie = `adonis_session=${cookieClient.sign('adonis_session', sessionId)}` - - const { headers } = await supertest(server).get('/').set('cookie', `${sessionIdCookie}`) + const { headers } = await supertest(server).get('/') const cookies = setCookieParser.parse(headers['set-cookie'], { map: true }) assert.property(cookies, 'adonis_session') @@ -292,7 +291,7 @@ test.group('Session', (group) => { }) test('initiate value with -1 on decrement', async ({ assert }) => { - let sessionId = randomUUID() + let sessionId: string | undefined const server = httpServer.create(async (req, res) => { const request = new RequestFactory().merge({ req, res, encryption }).create() @@ -303,13 +302,12 @@ test.group('Session', (group) => { await session.initiate(false) session.decrement('visits') + sessionId = session.sessionId await session.commit() response.finish() }) - const sessionIdCookie = `adonis_session=${cookieClient.sign('adonis_session', sessionId)}` - - const { headers } = await supertest(server).get('/').set('cookie', `${sessionIdCookie}`) + const { headers } = await supertest(server).get('/') const cookies = setCookieParser.parse(headers['set-cookie'], { map: true }) assert.property(cookies, 'adonis_session') @@ -318,6 +316,40 @@ test.group('Session', (group) => { }) }) + test('start a new session when the cookie names a session the store does not have', async ({ + assert, + }) => { + const endedSessionId = randomUUID() + let sessionId: string | undefined + let fresh: boolean | undefined + + const server = httpServer.create(async (req, res) => { + const request = new RequestFactory().merge({ req, res, encryption }).create() + const response = new ResponseFactory().merge({ req, res, encryption }).create() + const ctx = new HttpContextFactory().merge({ request, response }).create() + + const session = new Session(sessionConfig, () => new MemoryStore(), emitter, ctx) + await session.initiate(false) + + session.put('visits', 1) + sessionId = session.sessionId + fresh = session.fresh + + await session.commit() + response.finish() + }) + + const sessionIdCookie = `adonis_session=${cookieClient.sign('adonis_session', endedSessionId)}` + const { headers } = await supertest(server).get('/').set('cookie', sessionIdCookie) + + const cookies = setCookieParser.parse(headers['set-cookie'], { map: true }) + assert.notEqual(sessionId, endedSessionId) + assert.isTrue(fresh) + assert.equal(cookieClient.unsign('adonis_session', cookies.adonis_session.value), sessionId) + assert.isFalse(MemoryStore.sessions.has(endedSessionId)) + assert.deepEqual(MemoryStore.sessions.get(sessionId!), { visits: 1 }) + }) + test('touch session store when not modified', async ({ assert }) => { let sessionId = randomUUID() From f48690523f4093f5f5d24214cf2ed3ae6ab22074 Mon Sep 17 00:00:00 2001 From: Romain Lanz Date: Sat, 17 Oct 2026 15:34:54 +0000 Subject: [PATCH 03/14] feat(session): add refreshCookie option to send the cookie on id change Session.commit sends the session id cookie on every response. When requests from one browser overlap, a late response can put back an id the browser already left, for instance the guest id after a login that regenerated the session. With refreshCookie: false, the cookie is only sent when the session id changes (new session, regenerate) or the browser did not send one. It defaults to true, so nothing changes for existing apps. The tradeoff: the cookie Max-Age is no longer refreshed by each request. The option fits with clearWithBrowser, letting the store expire inactive sessions through its age. --- src/session.ts | 7 +++++-- src/types.ts | 15 +++++++++++++++ tests/session.spec.ts | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/src/session.ts b/src/session.ts index 2db93ac..d55b777 100644 --- a/src/session.ts +++ b/src/session.ts @@ -699,9 +699,12 @@ export class Session extends Macroable { debug('committing session data') /** - * Touch the session id cookie to stay alive + * Touch the session id cookie to stay alive. With "refreshCookie" + * disabled, only send it when the browser does not have this id. */ - this.#ctx.response.cookie(this.config.cookieName, this.#sessionId, this.config.cookie!) + if (this.config.refreshCookie !== false || this.#sessionId !== this.#sessionIdFromCookie) { + this.#ctx.response.cookie(this.config.cookieName, this.#sessionId, this.config.cookie!) + } /** * Delete the session data when the session store diff --git a/src/types.ts b/src/types.ts index e154e27..dcb2247 100644 --- a/src/types.ts +++ b/src/types.ts @@ -177,6 +177,21 @@ export interface SessionConfig { * Excludes maxAge and expires as they're controlled by age and clearWithBrowser. */ cookie: Partial + + /** + * Whether to send the session ID cookie on every response, which + * refreshes its Max-Age. Defaults to true. + * + * When false, the cookie is only sent when the session ID changes + * (new session, regenerate) or the browser did not send one. A late + * response then cannot put back in the browser a session ID it has + * left, e.g. the guest ID after a login that regenerated the session. + * + * The cookie Max-Age is no longer refreshed, so the browser drops the + * cookie `age` after the session started. Pair it with + * `clearWithBrowser` and let the store expire inactive sessions. + */ + refreshCookie?: boolean } /** diff --git a/tests/session.spec.ts b/tests/session.spec.ts index b51347d..f585aeb 100644 --- a/tests/session.spec.ts +++ b/tests/session.spec.ts @@ -316,6 +316,44 @@ test.group('Session', (group) => { }) }) + test('do not resend the cookie of an unchanged session id when refreshCookie is false', async ({ + assert, + }) => { + const sessionId = randomUUID() + MemoryStore.sessions.set(sessionId, { visits: 1 }) + + const server = httpServer.create(async (req, res) => { + const request = new RequestFactory().merge({ req, res, encryption }).create() + const response = new ResponseFactory().merge({ req, res, encryption }).create() + const ctx = new HttpContextFactory().merge({ request, response }).create() + + const config = { ...sessionConfig, refreshCookie: false } + const session = new Session(config, () => new MemoryStore(), emitter, ctx) + await session.initiate(false) + + session.increment('visits') + if (request.input('regenerate')) session.regenerate() + + await session.commit() + response.finish() + }) + + const sessionIdCookie = `adonis_session=${cookieClient.sign('adonis_session', sessionId)}` + + const unchanged = await supertest(server).get('/').set('cookie', sessionIdCookie) + assert.notProperty( + setCookieParser.parse(unchanged.headers['set-cookie'] ?? [], { map: true }), + 'adonis_session' + ) + assert.deepEqual(MemoryStore.sessions.get(sessionId), { visits: 2 }) + + const regenerated = await supertest(server).get('/?regenerate=1').set('cookie', sessionIdCookie) + const cookies = setCookieParser.parse(regenerated.headers['set-cookie'], { map: true }) + const newSessionId = cookieClient.unsign('adonis_session', cookies.adonis_session.value) + assert.notEqual(newSessionId, sessionId) + assert.deepEqual(MemoryStore.sessions.get(newSessionId), { visits: 3 }) + }) + test('start a new session when the cookie names a session the store does not have', async ({ assert, }) => { From 1c99c2c2259f3a99f6d738372064b424bce9c38e Mon Sep 17 00:00:00 2001 From: Romain Lanz Date: Sat, 17 Oct 2026 15:39:21 +0000 Subject: [PATCH 04/14] fix(stores): tag a session atomically with its write Session.tag runs before commit writes the session. In Redis, a concurrent tagged() call that ran in between saw an id without data and removed it from the user's set, so the logged-in session was missing from it for good. In SQL, tag upserted a placeholder row with empty data, which tagged() listed and which stayed behind when the session was never written. The redis and database stores now remember the user a session was tagged with. Their write adds the id to the user's set in the same MULTI (Redis) or stores user_id in the same query as the data (SQL). tag still applies right away to a session that exists, so SessionCollection.tag keeps working, but the database store no longer inserts a row for a session without data. --- src/stores/database.ts | 40 +++++++++++++++++------------ src/stores/redis.ts | 35 ++++++++++++++++++++++--- tests/stores/database_store.spec.ts | 12 +++++++-- tests/stores/redis_store.spec.ts | 14 ++++++++++ 4 files changed, 79 insertions(+), 22 deletions(-) diff --git a/src/stores/database.ts b/src/stores/database.ts index ba394aa..df642f9 100644 --- a/src/stores/database.ts +++ b/src/stores/database.ts @@ -38,6 +38,12 @@ export class DatabaseStore implements SessionStoreWithTaggingContract { */ #found = new Set() + /** + * User ids the sessions were tagged with by this store. The write of a + * tagged session stores the user id in the same query as its data. + */ + #tags = new Map() + /** * Creates a new database store instance * @@ -170,19 +176,22 @@ export class DatabaseStore implements SessionStoreWithTaggingContract { const message = new MessageBuilder().build(values, undefined, sessionId) const expiresAt = new Date(Date.now() + this.#ttlSeconds * 1000) + const userId = this.#tags.get(sessionId) + const columns = { + data: message, + expires_at: expiresAt, + ...(userId && { user_id: userId }), + } if (this.#found.has(sessionId)) { - await this.#client - .from(this.#tableName) - .where('id', sessionId) - .update({ data: message, expires_at: expiresAt }) + await this.#client.from(this.#tableName).where('id', sessionId).update(columns) } else { await this.#client .insertQuery() .table(this.#tableName) - .insert({ id: sessionId, data: message, expires_at: expiresAt }) + .insert({ id: sessionId, ...columns }) .knexQuery.onConflict('id') - .merge(['data', 'expires_at']) + .merge(Object.keys(columns)) } await this.#collectGarbage() @@ -222,8 +231,10 @@ export class DatabaseStore implements SessionStoreWithTaggingContract { } /** - * Tags a session with a user ID for tracking user sessions. - * Uses UPSERT to handle both existing and new sessions. + * Tags a session with a user ID for tracking user sessions. Updates the + * row when it exists, and the next write of the session by this store + * stores the user ID along with its data. No row is created for a + * session without data. * * @param sessionId - Session identifier * @param userId - User identifier to tag the session with @@ -234,15 +245,11 @@ export class DatabaseStore implements SessionStoreWithTaggingContract { async tag(sessionId: string, userId: string | number): Promise { debug('database store: associating user %s with session %s', userId, sessionId) - const data = new MessageBuilder().build({}, undefined, sessionId) - const expiresAt = new Date(Date.now() + this.#ttlSeconds * 1000) - + this.#tags.set(sessionId, String(userId)) await this.#client - .insertQuery() - .table(this.#tableName) - .insert({ id: sessionId, user_id: String(userId), data, expires_at: expiresAt }) - .knexQuery.onConflict('id') - .merge(['user_id']) + .from(this.#tableName) + .where('id', sessionId) + .update({ user_id: String(userId) }) } /** @@ -258,6 +265,7 @@ export class DatabaseStore implements SessionStoreWithTaggingContract { async untag(sessionId: string, userId: string | number): Promise { debug('database store: dissociating user %s from session %s', userId, sessionId) + this.#tags.delete(sessionId) await this.#client .query() .from(this.#tableName) diff --git a/src/stores/redis.ts b/src/stores/redis.ts index 1ef93bf..bc20228 100644 --- a/src/stores/redis.ts +++ b/src/stores/redis.ts @@ -39,6 +39,12 @@ export class RedisStore implements SessionStoreWithTaggingContract { */ #found = new Set() + /** + * User ids the sessions were tagged with by this store. The write of a + * tagged session adds it to the tag set in the same transaction. + */ + #tags = new Map() + /** * Creates a new Redis store instance * @@ -167,10 +173,28 @@ export class RedisStore implements SessionStoreWithTaggingContract { debug('redis store: writing session data %s, %O', sessionId, values) const message = new MessageBuilder().build(values, undefined, sessionId) + const transaction = this.#connection.multi() + if (this.#found.has(sessionId)) { - await this.#connection.set(sessionId, message, 'EX', this.#ttlSeconds, 'XX') + transaction.set(sessionId, message, 'EX', this.#ttlSeconds, 'XX') } else { - await this.#connection.set(sessionId, message, 'EX', this.#ttlSeconds) + transaction.set(sessionId, message, 'EX', this.#ttlSeconds) + } + + /** + * A concurrent "tagged" call drops ids that have no data yet. Adding + * the id along with the data keeps it in the set. When "XX" refuses + * the write, the id has no data and the next "tagged" call drops it. + */ + const userId = this.#tags.get(sessionId) + if (userId) { + transaction.sadd(this.#getTagKey(userId), sessionId) + } + + const results = await transaction.exec() + const failure = results?.find(([error]) => error) + if (failure) { + throw failure[0] } } @@ -201,14 +225,16 @@ export class RedisStore implements SessionStoreWithTaggingContract { } /** - * Tag a session with a user ID + * Tag a session with a user ID. The next write of the session by this + * store tags it again, atomically with its data. * * @param sessionId - Session identifier * @param userId - User identifier to tag the session with */ async tag(sessionId: string, userId: string | number): Promise { debug('redis store: tagging session %s with user %s', sessionId, userId) - await this.#connection.sadd(this.#getTagKey(userId.toString()), sessionId) + this.#tags.set(sessionId, String(userId)) + await this.#connection.sadd(this.#getTagKey(String(userId)), sessionId) } /** @@ -219,6 +245,7 @@ export class RedisStore implements SessionStoreWithTaggingContract { */ async untag(sessionId: string, userId: string | number): Promise { debug('redis store: untagging session %s from user %s', sessionId, userId) + this.#tags.delete(sessionId) await this.#connection.srem(this.#getTagKey(userId.toString()), sessionId) } diff --git a/tests/stores/database_store.spec.ts b/tests/stores/database_store.spec.ts index 640bc0d..4455c94 100644 --- a/tests/stores/database_store.spec.ts +++ b/tests/stores/database_store.spec.ts @@ -324,8 +324,8 @@ test.group('Database store', (group) => { /** * Simulate what happens during login lifecycle: * - Session is new (doesnt exist in DB yet) - * - User calls session.tag => create session in database - * - Then commit() calls write() => update data and preserves the rest + * - User calls session.tag => remembered by the store + * - Then commit() calls write() => inserts data and user id together */ test('tag before write creates session and preserves tag', async ({ assert }) => { const store = new DatabaseStore(db.connection(), '2 hours') @@ -340,6 +340,14 @@ test.group('Database store', (group) => { assert.deepEqual(data, { message: 'hello' }) }) + test('do not create a session row when tagging a session without data', async ({ assert }) => { + const store = new DatabaseStore(db.connection(), '2 hours') + await store.tag('new-session', 'user-123') + + assert.isNull(await db.from('sessions').where('id', 'new-session').first()) + assert.deepEqual(await store.tagged('user-123'), []) + }) + test('tag is preserved after write updates session data', async ({ assert }) => { const store = new DatabaseStore(db.connection(), '2 hours') diff --git a/tests/stores/redis_store.spec.ts b/tests/stores/redis_store.spec.ts index 7aa8d2d..825f454 100644 --- a/tests/stores/redis_store.spec.ts +++ b/tests/stores/redis_store.spec.ts @@ -299,6 +299,20 @@ test.group('Redis store', (group) => { assert.deepEqual(sessions, [{ id: 'new-session', data: { message: 'hello' } }]) }) + test('keep a session tagged before its write when tagged runs in between', async ({ assert }) => { + const store = new RedisStore(redis.connection('main'), '2 hours') + const collection = new RedisStore(redis.connection('main'), '2 hours') + + await store.tag('session-1', 'user-123') + assert.deepEqual(await collection.tagged('user-123'), []) + + await store.write('session-1', { message: 'hello' }) + + assert.deepEqual(await collection.tagged('user-123'), [ + { id: 'session-1', data: { message: 'hello' } }, + ]) + }) + test('tag is preserved after write updates session data', async ({ assert }) => { const session = new RedisStore(redis.connection('main'), '2 hours') From 6b4b231b0e439465637ac0c690128d74331fc936 Mon Sep 17 00:00:00 2001 From: Romain Lanz Date: Sat, 17 Oct 2026 15:44:04 +0000 Subject: [PATCH 05/14] feat(stores): add encrypt option to the redis and database stores The redis and database stores only signed the session data with MessageBuilder, so anyone with read access to Redis or the sessions table could read it in clear text. The cookie store already encrypts. stores.redis() and stores.database() accept encrypt: true. The store then encrypts the data with the encryption service, using the session id as purpose, and decrypts it on read and in tagged(). It defaults to false. Sessions written before enabling it no longer decode, so their users start a new session once. --- src/define_config.ts | 8 ++++- src/stores/database.ts | 37 +++++++++++++++++++-- src/stores/redis.ts | 50 +++++++++++++++++++++-------- src/types.ts | 25 +++++++++++++++ tests/define_config.spec.ts | 50 +++++++++++++++++++++++++++++ tests/stores/database_store.spec.ts | 12 +++++++ tests/stores/redis_store.spec.ts | 12 +++++++ 7 files changed, 178 insertions(+), 16 deletions(-) diff --git a/src/define_config.ts b/src/define_config.ts index 60c6472..bffafd2 100644 --- a/src/define_config.ts +++ b/src/define_config.ts @@ -196,9 +196,12 @@ export const stores: { return configProvider.create(async (app) => { const { RedisStore } = await import('./stores/redis.js') const redis = await app.container.make('redis') + const encryption = config.encrypt ? await app.container.make('encryption') : undefined return (_, sessionConfig: SessionConfig) => { - return new RedisStore(redis.connection(config.connection), sessionConfig.age) + return new RedisStore(redis.connection(config.connection), sessionConfig.age, { + encryption, + }) } }) }, @@ -251,10 +254,13 @@ export const stores: { ) } + const encryption = config?.encrypt ? await app.container.make('encryption') : undefined + return (_, sessionConfig: SessionConfig) => { return new DatabaseStore(db.connection(connectionName), sessionConfig.age, { tableName: config?.tableName, gcProbability: config?.gcProbability, + encryption, }) } }) diff --git a/src/stores/database.ts b/src/stores/database.ts index df642f9..abf69e8 100644 --- a/src/stores/database.ts +++ b/src/stores/database.ts @@ -12,7 +12,12 @@ import { MessageBuilder } from '@adonisjs/core/helpers' import type { QueryClientContract } from '@adonisjs/lucid/types/database' import debug from '../debug.ts' -import type { SessionStoreWithTaggingContract, SessionData, TaggedSession } from '../types.ts' +import type { + SessionData, + TaggedSession, + SessionEncryption, + SessionStoreWithTaggingContract, +} from '../types.ts' /** * Database store to read/write session data to SQL databases using Lucid. @@ -31,6 +36,12 @@ export class DatabaseStore implements SessionStoreWithTaggingContract { #ttlSeconds: number #gcProbability: number + /** + * Encrypts the session data when defined. Otherwise the data is + * only signed. + */ + #encryption?: SessionEncryption + /** * Ids of the sessions this store found while reading. A store serves * one request, so a write to one of them must not bring it back once @@ -71,12 +82,18 @@ export class DatabaseStore implements SessionStoreWithTaggingContract { * Defaults to 2 (2% chance) */ gcProbability?: number + + /** + * Encrypts the session data instead of only signing it + */ + encryption?: SessionEncryption } ) { this.#client = client this.#tableName = options?.tableName ?? 'sessions' this.#ttlSeconds = string.seconds.parse(age) this.#gcProbability = options?.gcProbability ?? 2 + this.#encryption = options?.encryption debug('initiating database store') } @@ -107,12 +124,28 @@ export class DatabaseStore implements SessionStoreWithTaggingContract { */ #parseSessionData(contents: string, sessionId: string): SessionData | null { try { + if (this.#encryption) { + return this.#encryption.decrypt(contents, sessionId) + } return new MessageBuilder().verify(contents, sessionId) } catch { return null } } + /** + * Encrypts or signs the session data for the session id + * + * @param values - Session data to serialize + * @param sessionId - Session identifier used as the purpose + */ + #serializeSessionData(values: Object, sessionId: string): string { + if (this.#encryption) { + return this.#encryption.encrypt(values, undefined, sessionId) + } + return new MessageBuilder().build(values, undefined, sessionId) + } + /** * Converts a database row to a TaggedSession object. * Parses the session data and returns null if parsing fails. @@ -174,7 +207,7 @@ export class DatabaseStore implements SessionStoreWithTaggingContract { async write(sessionId: string, values: Object): Promise { debug('database store: writing session data %s, %O', sessionId, values) - const message = new MessageBuilder().build(values, undefined, sessionId) + const message = this.#serializeSessionData(values, sessionId) const expiresAt = new Date(Date.now() + this.#ttlSeconds * 1000) const userId = this.#tags.get(sessionId) const columns = { diff --git a/src/stores/redis.ts b/src/stores/redis.ts index bc20228..c0b95b2 100644 --- a/src/stores/redis.ts +++ b/src/stores/redis.ts @@ -12,7 +12,12 @@ import { MessageBuilder } from '@adonisjs/core/helpers' import type { Connection } from '@adonisjs/redis/types' import debug from '../debug.ts' -import type { SessionData, TaggedSession, SessionStoreWithTaggingContract } from '../types.ts' +import type { + SessionData, + TaggedSession, + SessionEncryption, + SessionStoreWithTaggingContract, +} from '../types.ts' /** * Redis store to read/write session data to Redis server. @@ -32,6 +37,12 @@ export class RedisStore implements SessionStoreWithTaggingContract { */ #ttlSeconds: number + /** + * Encrypts the session data when defined. Otherwise the data is + * only signed. + */ + #encryption?: SessionEncryption + /** * Ids of the sessions this store found while reading. A store serves * one request, so a write to one of them must not bring it back once @@ -50,10 +61,16 @@ export class RedisStore implements SessionStoreWithTaggingContract { * * @param connection - Redis connection instance * @param age - Session age in seconds or time expression (e.g. '2 hours') + * @param options.encryption - Encrypts the session data instead of only signing it */ - constructor(connection: Connection, age: string | number) { + constructor( + connection: Connection, + age: string | number, + options?: { encryption?: SessionEncryption } + ) { this.#connection = connection this.#ttlSeconds = string.seconds.parse(age) + this.#encryption = options?.encryption debug('initiating redis store') } @@ -123,12 +140,28 @@ export class RedisStore implements SessionStoreWithTaggingContract { */ #parseSessionData(contents: string, sessionId: string): SessionData | null { try { + if (this.#encryption) { + return this.#encryption.decrypt(contents, sessionId) + } return new MessageBuilder().verify(contents, sessionId) } catch { return null } } + /** + * Encrypts or signs the session data for the session id + * + * @param values - Session data to serialize + * @param sessionId - Session identifier used as the purpose + */ + #serializeSessionData(values: Object, sessionId: string): string { + if (this.#encryption) { + return this.#encryption.encrypt(values, undefined, sessionId) + } + return new MessageBuilder().build(values, undefined, sessionId) + } + /** * Reads session data from Redis * @@ -146,16 +179,7 @@ export class RedisStore implements SessionStoreWithTaggingContract { } this.#found.add(sessionId) - - /** - * Verify contents with the session id and return them as an object. The verify - * method can fail when the contents is not JSON> - */ - try { - return new MessageBuilder().verify(contents, sessionId) - } catch { - return null - } + return this.#parseSessionData(contents, sessionId) } /** @@ -172,7 +196,7 @@ export class RedisStore implements SessionStoreWithTaggingContract { async write(sessionId: string, values: Object): Promise { debug('redis store: writing session data %s, %O', sessionId, values) - const message = new MessageBuilder().build(values, undefined, sessionId) + const message = this.#serializeSessionData(values, sessionId) const transaction = this.#connection.multi() if (this.#found.has(sessionId)) { diff --git a/src/types.ts b/src/types.ts index dcb2247..df87960 100644 --- a/src/types.ts +++ b/src/types.ts @@ -9,6 +9,7 @@ import { type HttpContext } from '@adonisjs/core/http' import { type RedisConnections } from '@adonisjs/redis/types' +import type { Encryption } from '@adonisjs/core/encryption' import type { CookieOptions } from '@adonisjs/core/types/http' import type { DynamoDBClient, DynamoDBClientConfig } from '@aws-sdk/client-dynamodb' @@ -210,6 +211,12 @@ export type FileStoreConfig = { location: string } +/** + * Encrypter used by the redis and database stores when their + * "encrypt" option is enabled. Satisfied by the encryption service. + */ +export type SessionEncryption = Pick + /** * Configuration for Redis-based session storage. * Sessions are stored in a Redis database with automatic expiry. @@ -224,6 +231,15 @@ export type RedisStoreConfig = { * Name of the Redis connection to use (must match connection name in config/redis.ts) */ connection: keyof RedisConnections + + /** + * Encrypt the session data with the encryption service instead of + * only signing it. Sessions written without encryption cannot be + * read once it is enabled, so users start a new session. + * + * Defaults to false + */ + encrypt?: boolean } /** @@ -317,6 +333,15 @@ export interface DatabaseStoreConfig { * Defaults to 2 (2% chance) */ gcProbability?: number + + /** + * Encrypt the session data with the encryption service instead of + * only signing it. Sessions written without encryption cannot be + * read once it is enabled, so users start a new session. + * + * Defaults to false + */ + encrypt?: boolean } /** diff --git a/tests/define_config.spec.ts b/tests/define_config.spec.ts index 1e7993d..0dde52a 100644 --- a/tests/define_config.spec.ts +++ b/tests/define_config.spec.ts @@ -13,6 +13,8 @@ import { AppFactory } from '@adonisjs/core/factories/app' import { defineConfig as redisConfig } from '@adonisjs/redis' import type { ApplicationService } from '@adonisjs/core/types' import { HttpContextFactory } from '@adonisjs/core/factories/http' +import { EncryptionManager } from '@adonisjs/core/encryption' +import { AES256GCM } from '@adonisjs/core/encryption/drivers/aes_256_gcm' import { FileStore } from '../src/stores/file.ts' import { RedisStore } from '../src/stores/redis.ts' @@ -159,6 +161,54 @@ test.group('Define config', () => { assert.instanceOf(config.stores.redis(ctx, config), RedisStore) }) + test('encrypt redis store data when encrypt is enabled', async ({ assert, cleanup }) => { + const appForRedis = new AppFactory().create(BASE_URL, () => {}) as ApplicationService + appForRedis.rcContents({ + providers: [ + () => import('@adonisjs/core/providers/app_provider'), + () => import('@adonisjs/redis/redis_provider'), + ], + }) + appForRedis.useConfig({ + logger: { default: 'main', loggers: { main: {} } }, + redis: redisConfig({ connection: 'main', connections: { main: {} } }), + }) + await appForRedis.init() + await appForRedis.boot() + + const encryption = new EncryptionManager({ + default: 'gcm', + list: { + gcm: { + driver: (key) => new AES256GCM({ key, id: 'gcm' }), + keys: ['averystrongrandomsecretkey'], + }, + }, + }) + appForRedis.container.bindValue('encryption', encryption) + + const config = await defineConfig({ + store: 'redis', + stores: { + redis: stores.redis({ connection: 'main', encrypt: true } as any), + }, + }).resolver(appForRedis) + + const redis = await appForRedis.container.make('redis') + cleanup(async () => { + await redis.del('encrypted-session') + await redis.disconnectAll() + }) + + const store = config.stores.redis(new HttpContextFactory().create(), config) + await store.write('encrypted-session', { message: 'hello-world' }) + + const contents = await redis.get('encrypted-session') + assert.deepEqual(encryption.decrypt(contents!, 'encrypted-session'), { + message: 'hello-world', + }) + }) + test('transform config with cookie store', async ({ assert }) => { const config = await defineConfig({ store: 'cookie', diff --git a/tests/stores/database_store.spec.ts b/tests/stores/database_store.spec.ts index 4455c94..5bb6408 100644 --- a/tests/stores/database_store.spec.ts +++ b/tests/stores/database_store.spec.ts @@ -11,6 +11,7 @@ import { test } from '@japa/runner' import { setTimeout } from 'node:timers/promises' import { Database } from '@adonisjs/lucid/database' import { AppFactory } from '@adonisjs/core/factories/app' +import { EncryptionFactory } from '@adonisjs/core/factories/encryption' import { LoggerFactory } from '@adonisjs/core/factories/logger' import { EmitterFactory } from '@adonisjs/core/factories/events' @@ -110,6 +111,17 @@ test.group('Database store', (group) => { assert.deepEqual(value, { message: 'hello-world' }) }) + test('encrypt session data when an encrypter is given', async ({ assert }) => { + const encryption = new EncryptionFactory().create() + const store = new DatabaseStore(db.connection(), '2 hours', { encryption }) + await store.write(sessionId, { message: 'hello-world' }) + + const row = await db.from('sessions').where('id', sessionId).first() + assert.notInclude(row.data, 'hello-world') + assert.deepEqual(encryption.decrypt(row.data, sessionId), { message: 'hello-world' }) + assert.deepEqual(await store.read(sessionId), { message: 'hello-world' }) + }) + test('return null when session data is expired', async ({ assert }) => { const store = new DatabaseStore(db.connection(), 1) await store.write(sessionId, { message: 'hello-world' }) diff --git a/tests/stores/redis_store.spec.ts b/tests/stores/redis_store.spec.ts index 825f454..1e4e808 100644 --- a/tests/stores/redis_store.spec.ts +++ b/tests/stores/redis_store.spec.ts @@ -11,6 +11,7 @@ import { test } from '@japa/runner' import { defineConfig } from '@adonisjs/redis' import { setTimeout } from 'node:timers/promises' import { RedisManagerFactory } from '@adonisjs/redis/factories' +import { EncryptionFactory } from '@adonisjs/core/factories/encryption' import { RedisStore } from '../../src/stores/redis.ts' @@ -83,6 +84,17 @@ test.group('Redis store', (group) => { assert.isNull(value) }) + test('encrypt session data when an encrypter is given', async ({ assert }) => { + const encryption = new EncryptionFactory().create() + const session = new RedisStore(redis.connection('main'), '2 hours', { encryption }) + await session.write(sessionId, { message: 'hello-world' }) + + const contents = await redis.get(sessionId) + assert.notInclude(contents, 'hello-world') + assert.deepEqual(encryption.decrypt(contents!, sessionId), { message: 'hello-world' }) + assert.deepEqual(await session.read(sessionId), { message: 'hello-world' }) + }) + test('delete key on destroy', async ({ assert }) => { const session = new RedisStore(redis.connection('main'), '2 hours') From 772debc1345bea675734a01d30238d7376aedae2 Mon Sep 17 00:00:00 2001 From: Romain Lanz Date: Sat, 17 Oct 2026 15:46:18 +0000 Subject: [PATCH 06/14] fix(plugins): write japa test sessions to the configured store The api and browser client plugins always wrote sessions to the memory store, so withSession and setSession only reached the application when its tests also ran on the memory store. An app running its tests on redis or the database could not seed a session. Both plugins now create the store configured by "store", falling back to memory for the cookie store, which needs an HTTP request. SessionClient also awaits the store write and destroy, which async stores need before the request is sent. --- src/client.ts | 4 +-- src/plugins/japa/api_client.ts | 7 +++-- src/plugins/japa/browser_client.ts | 7 +++-- src/plugins/japa/store.ts | 22 +++++++++++++++ tests/plugins/api_client.spec.ts | 38 +++++++++++++++++++++++++- tests/plugins/browser_client.spec.ts | 41 +++++++++++++++++++++++++++- 6 files changed, 109 insertions(+), 10 deletions(-) create mode 100644 src/plugins/japa/store.ts diff --git a/src/client.ts b/src/client.ts index f211eaf..7a9817b 100644 --- a/src/client.ts +++ b/src/client.ts @@ -97,7 +97,7 @@ export class SessionClient { debug('committing session data during api request') if (!this.#valuesStore.isEmpty) { - this.#store.write(this.sessionId, this.#valuesStore.toJSON()) + await this.#store.write(this.sessionId, this.#valuesStore.toJSON()) } } @@ -112,7 +112,7 @@ export class SessionClient { */ async destroy(sessionId?: string) { debug('destroying session data during api request') - this.#store.destroy(sessionId || this.sessionId) + await this.#store.destroy(sessionId || this.sessionId) } /** diff --git a/src/plugins/japa/api_client.ts b/src/plugins/japa/api_client.ts index 17c25ed..9734249 100644 --- a/src/plugins/japa/api_client.ts +++ b/src/plugins/japa/api_client.ts @@ -16,7 +16,8 @@ import type { ApplicationService } from '@adonisjs/core/types' import { ApiClient, ApiRequest, ApiResponse } from '@japa/api-client' import { SessionClient } from '../../client.ts' -import type { SessionData } from '../../types.ts' +import { createTestStore } from './store.ts' +import type { ResolvedSessionConfig, SessionData } from '../../types.ts' declare module '@japa/api-client' { export interface ApiRequest { @@ -127,7 +128,7 @@ export const sessionApiClient = (app: ApplicationService) => { /** * Resolve config from the provider */ - const config = await configProvider.resolve(app, sessionConfigProvider) + const config = await configProvider.resolve(app, sessionConfigProvider) if (!config) { throw new RuntimeException( 'Invalid "config/session.ts" file. Make sure you are using the "defineConfig" method' @@ -142,7 +143,7 @@ export const sessionApiClient = (app: ApplicationService) => { ApiRequest.getter( 'sessionClient', function () { - return new SessionClient(config.stores.memory()) + return new SessionClient(createTestStore(config)) }, true ) diff --git a/src/plugins/japa/browser_client.ts b/src/plugins/japa/browser_client.ts index 088b728..6f42512 100644 --- a/src/plugins/japa/browser_client.ts +++ b/src/plugins/japa/browser_client.ts @@ -16,7 +16,8 @@ import type { ApplicationService } from '@adonisjs/core/types' import type { CookieOptions as AdonisCookieOptions } from '@adonisjs/core/types/http' import { SessionClient } from '../../client.ts' -import type { SessionConfig, SessionData } from '../../types.ts' +import { createTestStore } from './store.ts' +import type { ResolvedSessionConfig, SessionConfig, SessionData } from '../../types.ts' declare module 'playwright' { export interface BrowserContext { @@ -115,7 +116,7 @@ export const sessionBrowserClient = (app: ApplicationService) => { /** * Resolve config from the provider */ - const config = await configProvider.resolve(app, sessionConfigProvider) + const config = await configProvider.resolve(app, sessionConfigProvider) if (!config) { throw new RuntimeException( 'Invalid "config/session.ts" file. Make sure you are using the "defineConfig" method' @@ -127,7 +128,7 @@ export const sessionBrowserClient = (app: ApplicationService) => { /** * Reference to session client per browser context */ - context.sessionClient = new SessionClient(config.stores.memory()) + context.sessionClient = new SessionClient(createTestStore(config)) /** * Initiating session store diff --git a/src/plugins/japa/store.ts b/src/plugins/japa/store.ts new file mode 100644 index 0000000..c79acd0 --- /dev/null +++ b/src/plugins/japa/store.ts @@ -0,0 +1,22 @@ +/* + * @adonisjs/session + * + * (c) AdonisJS + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +import { MemoryStore } from '../../stores/memory.ts' +import { CookieStore } from '../../stores/cookie.ts' +import type { ResolvedSessionConfig, SessionStoreContract } from '../../types.ts' + +/** + * Creates the store the test clients write sessions to: the store the + * application reads them from. The cookie store needs an HTTP request, + * so the clients fall back to the memory store for it. + */ +export function createTestStore(config: ResolvedSessionConfig): SessionStoreContract { + const store = config.stores[config.store](null as any, config) + return store instanceof CookieStore ? new MemoryStore() : store +} diff --git a/tests/plugins/api_client.spec.ts b/tests/plugins/api_client.spec.ts index 37470f4..98d47ec 100644 --- a/tests/plugins/api_client.spec.ts +++ b/tests/plugins/api_client.spec.ts @@ -25,7 +25,8 @@ import { import { Session } from '../../src/session.ts' import { type SessionConfig } from '../../src/types.ts' -import { defineConfig } from '../../src/define_config.ts' +import { FileStore } from '../../src/stores/file.ts' +import { defineConfig, stores } from '../../src/define_config.ts' import { MemoryStore } from '../../src/stores/memory.ts' import { httpServer, runJapaTest } from '../../tests_helpers/index.ts' @@ -229,4 +230,39 @@ test.group('Api client', (group) => { assert.throws(() => response.assertValidationError('username', 'field is missing')) }) }) + + test('write the session to the configured store', async ({ assert, fs, cleanup }) => { + app.config.set( + 'session', + defineConfig({ store: 'file', stores: { file: stores.file({ location: fs.basePath }) } }) + ) + cleanup(() => app.config.set('session', defineConfig({ store: 'memory', stores: {} }))) + + let seen: unknown + const server = httpServer.create(async (req, res) => { + const request = new RequestFactory().merge({ req, res, encryption }).create() + const response = new ResponseFactory().merge({ req, res, encryption }).create() + const ctx = new HttpContextFactory().merge({ request, response }).create() + + const fileStore = () => new FileStore({ location: fs.basePath }, '2 hours') + const session = new Session(sessionConfig, fileStore, emitter, ctx) + + await session.initiate(false) + seen = session.all() + + await session.commit() + response.finish() + }) + + const port = await getPort({ port: 3333 }) + const url = `http://localhost:${port}` + server.listen(port) + + await runJapaTest(app, async ({ client }) => { + await client.get(url).withSession({ username: 'virk' }) + }) + + assert.deepEqual(seen, { username: 'virk' }) + assert.lengthOf(MemoryStore.sessions, 0) + }) }) diff --git a/tests/plugins/browser_client.spec.ts b/tests/plugins/browser_client.spec.ts index d21ab38..b5502ba 100644 --- a/tests/plugins/browser_client.spec.ts +++ b/tests/plugins/browser_client.spec.ts @@ -26,7 +26,8 @@ import { import { Session } from '../../src/session.ts' import { type SessionConfig } from '../../src/types.ts' import { MemoryStore } from '../../src/stores/memory.ts' -import { defineConfig } from '../../src/define_config.ts' +import { FileStore } from '../../src/stores/file.ts' +import { defineConfig, stores } from '../../src/define_config.ts' import { httpServer, runJapaTest } from '../../tests_helpers/index.ts' const app = new AppFactory().create(new URL('./', import.meta.url), () => {}) as ApplicationService @@ -202,4 +203,42 @@ test.group('Browser client', (group) => { assert.lengthOf(MemoryStore.sessions, 0) }) }) + + test('write the session to the configured store', async ({ assert, fs, cleanup }) => { + app.config.set( + 'session', + defineConfig({ store: 'file', stores: { file: stores.file({ location: fs.basePath }) } }) + ) + cleanup(() => app.config.set('session', defineConfig({ store: 'memory', stores: {} }))) + + let seen: unknown + const server = httpServer.create(async (req, res) => { + const request = new RequestFactory().merge({ req, res, encryption }).create() + const response = new ResponseFactory().merge({ req, res, encryption }).create() + const ctx = new HttpContextFactory().merge({ request, response }).create() + + const fileStore = () => new FileStore({ location: fs.basePath }, '2 hours') + const session = new Session(sessionConfig, fileStore, emitter, ctx) + + await session.initiate(false) + seen = session.all() + + await session.commit() + response.finish() + }) + + const port = await getPort({ port: 3333 }) + const url = `http://localhost:${port}` + server.listen(port) + + await runJapaTest(app, async ({ visit, browserContext }) => { + await browserContext.initiateSession({ domain: new URL(url).host, path: '/' }) + await browserContext.setSession({ username: 'virk' }) + await visit(url) + await browserContext.close() + }) + + assert.deepEqual(seen, { username: 'virk' }) + assert.lengthOf(MemoryStore.sessions, 0) + }) }) From ae0a9cd4ffcc168b3fc140456a1de44b34ae1ef6 Mon Sep 17 00:00:00 2001 From: Romain Lanz Date: Sat, 17 Oct 2026 16:12:24 +0000 Subject: [PATCH 07/14] test: skip the redis encrypt config test when redis is not available --- tests/define_config.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/define_config.spec.ts b/tests/define_config.spec.ts index 0dde52a..5f3e119 100644 --- a/tests/define_config.spec.ts +++ b/tests/define_config.spec.ts @@ -207,7 +207,7 @@ test.group('Define config', () => { assert.deepEqual(encryption.decrypt(contents!, 'encrypted-session'), { message: 'hello-world', }) - }) + }).skip(!!process.env.NO_REDIS, 'Redis not available in windows env') test('transform config with cookie store', async ({ assert }) => { const config = await defineConfig({ From d432704dd59b4261b358d1fa584031f3687c94ce Mon Sep 17 00:00:00 2001 From: Romain Lanz Date: Sat, 17 Oct 2026 16:23:28 +0000 Subject: [PATCH 08/14] fix(stores): do not use a cross-slot MULTI on redis cluster Writing a tagged session sent SET and SADD in one MULTI. The session key and the tag set key are in different hash slots, so Redis Cluster refused the transaction with CROSSSLOT, and the commit after a login failed. On a cluster connection, the store now sets the data and then adds the id to the tag set, outside a transaction. A standalone connection keeps the MULTI. On a cluster, a tagged() call that read the set before the data and removes the id after the SADD can still drop it. --- src/stores/redis.ts | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/src/stores/redis.ts b/src/stores/redis.ts index c0b95b2..37ede9f 100644 --- a/src/stores/redis.ts +++ b/src/stores/redis.ts @@ -197,23 +197,39 @@ export class RedisStore implements SessionStoreWithTaggingContract { debug('redis store: writing session data %s, %O', sessionId, values) const message = this.#serializeSessionData(values, sessionId) - const transaction = this.#connection.multi() - - if (this.#found.has(sessionId)) { - transaction.set(sessionId, message, 'EX', this.#ttlSeconds, 'XX') - } else { - transaction.set(sessionId, message, 'EX', this.#ttlSeconds) - } + const onlyIfExists = this.#found.has(sessionId) + const userId = this.#tags.get(sessionId) /** * A concurrent "tagged" call drops ids that have no data yet. Adding * the id along with the data keeps it in the set. When "XX" refuses * the write, the id has no data and the next "tagged" call drops it. + * + * Redis Cluster refuses a MULTI over keys of different slots, so the + * id is added right after the data there. A "tagged" call that read + * the set before the data and removes the id after the SADD can still + * drop it. */ - const userId = this.#tags.get(sessionId) - if (userId) { - transaction.sadd(this.#getTagKey(userId), sessionId) + if (!userId || this.#connection.ioConnection.isCluster) { + if (onlyIfExists) { + await this.#connection.set(sessionId, message, 'EX', this.#ttlSeconds, 'XX') + } else { + await this.#connection.set(sessionId, message, 'EX', this.#ttlSeconds) + } + + if (userId) { + await this.#connection.sadd(this.#getTagKey(userId), sessionId) + } + return + } + + const transaction = this.#connection.multi() + if (onlyIfExists) { + transaction.set(sessionId, message, 'EX', this.#ttlSeconds, 'XX') + } else { + transaction.set(sessionId, message, 'EX', this.#ttlSeconds) } + transaction.sadd(this.#getTagKey(userId), sessionId) const results = await transaction.exec() const failure = results?.find(([error]) => error) From 55411fffe813504f36e2b43e96842748dda12e92 Mon Sep 17 00:00:00 2001 From: Romain Lanz Date: Sat, 17 Oct 2026 16:23:29 +0000 Subject: [PATCH 09/14] fix(plugins): read the response session when refreshCookie is false The api client loaded the response session only from the session cookie of the response. With refreshCookie: false, a request that kept its session id got no cookie, so response.session() was empty. When the response has no session cookie and refreshCookie is false, the client now loads the session id it sent. --- src/plugins/japa/api_client.ts | 12 +++++++++--- tests/plugins/api_client.spec.ts | 29 +++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/plugins/japa/api_client.ts b/src/plugins/japa/api_client.ts index 9734249..977adb6 100644 --- a/src/plugins/japa/api_client.ts +++ b/src/plugins/japa/api_client.ts @@ -243,13 +243,19 @@ export const sessionApiClient = (app: ApplicationService) => { }) request.teardown(async (response) => { - const sessionId = response.cookie(config.cookieName) + /** + * With "refreshCookie" disabled, the response has no session + * cookie when the session id did not change. + */ + const sessionId = + response.cookie(config.cookieName)?.value ?? + (config.refreshCookie === false ? request.sessionClient.sessionId : undefined) /** * Reading session data from the response cookie */ response.sessionBag = sessionId - ? await response.request.sessionClient.load(sessionId.value) + ? await response.request.sessionClient.load(sessionId) : { values: {}, flashMessages: {}, @@ -258,7 +264,7 @@ export const sessionApiClient = (app: ApplicationService) => { /** * Cleanup state */ - await request.sessionClient.destroy(sessionId?.value) + await request.sessionClient.destroy(sessionId) }) }) } diff --git a/tests/plugins/api_client.spec.ts b/tests/plugins/api_client.spec.ts index 98d47ec..d41e28b 100644 --- a/tests/plugins/api_client.spec.ts +++ b/tests/plugins/api_client.spec.ts @@ -265,4 +265,33 @@ test.group('Api client', (group) => { assert.deepEqual(seen, { username: 'virk' }) assert.lengthOf(MemoryStore.sessions, 0) }) + + test('read the response session when refreshCookie is false', async ({ assert, cleanup }) => { + app.config.set('session', defineConfig({ store: 'memory', stores: {}, refreshCookie: false })) + cleanup(() => app.config.set('session', defineConfig({ store: 'memory', stores: {} }))) + + const server = httpServer.create(async (req, res) => { + const request = new RequestFactory().merge({ req, res, encryption }).create() + const response = new ResponseFactory().merge({ req, res, encryption }).create() + const ctx = new HttpContextFactory().merge({ request, response }).create() + + const config = { ...sessionConfig, refreshCookie: false } + const session = new Session(config, () => new MemoryStore(), emitter, ctx) + + await session.initiate(false) + session.increment('visits') + + await session.commit() + response.finish() + }) + + const port = await getPort({ port: 3333 }) + const url = `http://localhost:${port}` + server.listen(port) + + await runJapaTest(app, async ({ client }) => { + const response = await client.get(url).withSession({ user: 42 }) + assert.deepEqual(response.session(), { user: 42, visits: 1 }) + }) + }) }) From 94a1ee925b295792c0f90830d49be688011903fa Mon Sep 17 00:00:00 2001 From: Romain Lanz Date: Sat, 17 Oct 2026 16:23:29 +0000 Subject: [PATCH 10/14] fix(plugins): give each test client call its own store A store remembers the sessions it read and writes them back only while they exist. The browser client kept one store for the whole browser context, so after getSession() and a logout that destroyed the session, a later setSession() was silently dropped. The test store now creates a new store for each read, write, destroy and touch call. --- src/plugins/japa/store.ts | 17 +++++++++-- tests/plugins/browser_client.spec.ts | 43 ++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/src/plugins/japa/store.ts b/src/plugins/japa/store.ts index c79acd0..d0c9a88 100644 --- a/src/plugins/japa/store.ts +++ b/src/plugins/japa/store.ts @@ -15,8 +15,21 @@ import type { ResolvedSessionConfig, SessionStoreContract } from '../../types.ts * Creates the store the test clients write sessions to: the store the * application reads them from. The cookie store needs an HTTP request, * so the clients fall back to the memory store for it. + * + * A store serves one request and remembers the sessions it read, which + * changes how it writes them later. Each call gets a new store, so a + * client that read a session can still write it after a logout. */ export function createTestStore(config: ResolvedSessionConfig): SessionStoreContract { - const store = config.stores[config.store](null as any, config) - return store instanceof CookieStore ? new MemoryStore() : store + const createStore = () => { + const store = config.stores[config.store](null as any, config) + return store instanceof CookieStore ? new MemoryStore() : store + } + + return { + read: (sessionId) => createStore().read(sessionId), + write: (sessionId, data) => createStore().write(sessionId, data), + destroy: (sessionId) => createStore().destroy(sessionId), + touch: (sessionId) => createStore().touch(sessionId), + } } diff --git a/tests/plugins/browser_client.spec.ts b/tests/plugins/browser_client.spec.ts index b5502ba..8142df1 100644 --- a/tests/plugins/browser_client.spec.ts +++ b/tests/plugins/browser_client.spec.ts @@ -241,4 +241,47 @@ test.group('Browser client', (group) => { assert.deepEqual(seen, { username: 'virk' }) assert.lengthOf(MemoryStore.sessions, 0) }) + + test('set the session again after the application destroyed it', async ({ + assert, + fs, + cleanup, + }) => { + app.config.set( + 'session', + defineConfig({ store: 'file', stores: { file: stores.file({ location: fs.basePath }) } }) + ) + cleanup(() => app.config.set('session', defineConfig({ store: 'memory', stores: {} }))) + + const server = httpServer.create(async (req, res) => { + const request = new RequestFactory().merge({ req, res, encryption }).create() + const response = new ResponseFactory().merge({ req, res, encryption }).create() + const ctx = new HttpContextFactory().merge({ request, response }).create() + + const fileStore = () => new FileStore({ location: fs.basePath }, '2 hours') + const session = new Session(sessionConfig, fileStore, emitter, ctx) + + await session.initiate(false) + session.clear() + + await session.commit() + response.finish() + }) + + const port = await getPort({ port: 3333 }) + const url = `http://localhost:${port}` + server.listen(port) + + await runJapaTest(app, async ({ visit, browserContext }) => { + await browserContext.initiateSession({ domain: new URL(url).host, path: '/' }) + await browserContext.setSession({ user: 42 }) + assert.deepEqual(await browserContext.getSession(), { user: 42 }) + + await visit(url) + + await browserContext.setSession({ user: 43 }) + assert.deepEqual(await browserContext.getSession(), { user: 43 }) + await browserContext.close() + }) + }) }) From b9cd22143115c30ce981aca93af27e8ac6edbe32 Mon Sep 17 00:00:00 2001 From: Romain Lanz Date: Sat, 17 Oct 2026 16:37:57 +0000 Subject: [PATCH 11/14] fix(stores): do not replay the tag of a session read by the store tag() applied the tag right away and also kept it for the next write. For a session the store had read, another request could untag it in between, and the write then tagged it again. The redis and database stores now keep the tag for the write only for a session they did not read. A session they read already has data, so the tag applied by tag() is enough. --- src/stores/database.ts | 15 +++++++++++---- src/stores/redis.ts | 17 ++++++++++++----- tests/stores/database_store.spec.ts | 14 ++++++++++++++ tests/stores/redis_store.spec.ts | 14 ++++++++++++++ 4 files changed, 51 insertions(+), 9 deletions(-) diff --git a/src/stores/database.ts b/src/stores/database.ts index abf69e8..265254b 100644 --- a/src/stores/database.ts +++ b/src/stores/database.ts @@ -50,8 +50,8 @@ export class DatabaseStore implements SessionStoreWithTaggingContract { #found = new Set() /** - * User ids the sessions were tagged with by this store. The write of a - * tagged session stores the user id in the same query as its data. + * User ids the new sessions were tagged with by this store. The write of + * a tagged session stores the user id in the same query as its data. */ #tags = new Map() @@ -265,7 +265,7 @@ export class DatabaseStore implements SessionStoreWithTaggingContract { /** * Tags a session with a user ID for tracking user sessions. Updates the - * row when it exists, and the next write of the session by this store + * row when it exists. For a new session, the next write by this store * stores the user ID along with its data. No row is created for a * session without data. * @@ -278,7 +278,14 @@ export class DatabaseStore implements SessionStoreWithTaggingContract { async tag(sessionId: string, userId: string | number): Promise { debug('database store: associating user %s with session %s', userId, sessionId) - this.#tags.set(sessionId, String(userId)) + /** + * A session this store read already has data, so the tag applied + * below is enough. Replaying it on write would undo an untag done + * meanwhile by another request. + */ + if (!this.#found.has(sessionId)) { + this.#tags.set(sessionId, String(userId)) + } await this.#client .from(this.#tableName) .where('id', sessionId) diff --git a/src/stores/redis.ts b/src/stores/redis.ts index 37ede9f..90983e8 100644 --- a/src/stores/redis.ts +++ b/src/stores/redis.ts @@ -51,8 +51,8 @@ export class RedisStore implements SessionStoreWithTaggingContract { #found = new Set() /** - * User ids the sessions were tagged with by this store. The write of a - * tagged session adds it to the tag set in the same transaction. + * User ids the new sessions were tagged with by this store. The write of + * a tagged session adds it to the tag set in the same transaction. */ #tags = new Map() @@ -265,15 +265,22 @@ export class RedisStore implements SessionStoreWithTaggingContract { } /** - * Tag a session with a user ID. The next write of the session by this - * store tags it again, atomically with its data. + * Tag a session with a user ID. For a new session, the next write by + * this store tags it again, atomically with its data. * * @param sessionId - Session identifier * @param userId - User identifier to tag the session with */ async tag(sessionId: string, userId: string | number): Promise { debug('redis store: tagging session %s with user %s', sessionId, userId) - this.#tags.set(sessionId, String(userId)) + /** + * A session this store read already has data, so the tag applied + * below is enough. Replaying it on write would undo an untag done + * meanwhile by another request. + */ + if (!this.#found.has(sessionId)) { + this.#tags.set(sessionId, String(userId)) + } await this.#connection.sadd(this.#getTagKey(String(userId)), sessionId) } diff --git a/tests/stores/database_store.spec.ts b/tests/stores/database_store.spec.ts index 5bb6408..20d158e 100644 --- a/tests/stores/database_store.spec.ts +++ b/tests/stores/database_store.spec.ts @@ -360,6 +360,20 @@ test.group('Database store', (group) => { assert.deepEqual(await store.tagged('user-123'), []) }) + test('do not restore a tag removed while the session was written', async ({ assert }) => { + const request = new DatabaseStore(db.connection(), '2 hours') + const other = new DatabaseStore(db.connection(), '2 hours') + + await other.write('session-1', { message: 'hello' }) + await request.read('session-1') + await request.tag('session-1', 'user-123') + + await other.untag('session-1', 'user-123') + await request.write('session-1', { message: 'updated' }) + + assert.deepEqual(await other.tagged('user-123'), []) + }) + test('tag is preserved after write updates session data', async ({ assert }) => { const store = new DatabaseStore(db.connection(), '2 hours') diff --git a/tests/stores/redis_store.spec.ts b/tests/stores/redis_store.spec.ts index 1e4e808..11e7f65 100644 --- a/tests/stores/redis_store.spec.ts +++ b/tests/stores/redis_store.spec.ts @@ -325,6 +325,20 @@ test.group('Redis store', (group) => { ]) }) + test('do not restore a tag removed while the session was written', async ({ assert }) => { + const request = new RedisStore(redis.connection('main'), '2 hours') + const other = new RedisStore(redis.connection('main'), '2 hours') + + await other.write('session-1', { message: 'hello' }) + await request.read('session-1') + await request.tag('session-1', 'user-123') + + await other.untag('session-1', 'user-123') + await request.write('session-1', { message: 'updated' }) + + assert.deepEqual(await other.tagged('user-123'), []) + }) + test('tag is preserved after write updates session data', async ({ assert }) => { const session = new RedisStore(redis.connection('main'), '2 hours') From 25fc65db53bf1753ae75e519e6954e8eae8af37e Mon Sep 17 00:00:00 2001 From: Romain Lanz Date: Sat, 17 Oct 2026 16:37:58 +0000 Subject: [PATCH 12/14] fix(stores): report partial writes of a read session in the file store The file store rewrote a session it had read with a single FileHandle.write() call, which ignores how many bytes were written. A write cut short (for example by a file size limit) resolved and left a truncated file. It now uses FileHandle.writeFile() on the "r+" handle, which writes all the bytes or rejects, as writeFile() did before. --- src/stores/file.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stores/file.ts b/src/stores/file.ts index 24a27bd..e8972f8 100644 --- a/src/stores/file.ts +++ b/src/stores/file.ts @@ -201,7 +201,7 @@ export class FileStore implements SessionStoreContract { try { await file.truncate(0) - await file.write(message, 0, 'utf-8') + await file.writeFile(message, 'utf-8') } finally { await file.close() } From 011231e49f76051f4363c3526e6a42a349ab63fd Mon Sep 17 00:00:00 2001 From: Romain Lanz Date: Sat, 17 Oct 2026 16:50:08 +0000 Subject: [PATCH 13/14] fix(plugins): destroy the browser test session before close resolves The browser client destroyed the test session in a "close" listener. Playwright does not wait for its listeners, so with an async store the destroy still ran after close() resolved. On Windows CI, it raced the removal of the temporary directory and failed with an unhandled EPERM. close() now destroys the session first, then closes the context. --- src/plugins/japa/browser_client.ts | 10 +++++++--- tests/plugins/browser_client.spec.ts | 2 ++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/plugins/japa/browser_client.ts b/src/plugins/japa/browser_client.ts index 6f42512..dc954f1 100644 --- a/src/plugins/japa/browser_client.ts +++ b/src/plugins/japa/browser_client.ts @@ -184,11 +184,15 @@ export const sessionBrowserClient = (app: ApplicationService) => { } /** - * Destroy session when context is closed + * Destroy session when context is closed. Playwright does not wait + * for "close" listeners, so the destroy runs inside close() to + * finish before it resolves. */ - context.on('close', async function () { + const close = context.close.bind(context) + context.close = async function (options) { await context.sessionClient.destroy() - }) + return close(options) + } }, }) } diff --git a/tests/plugins/browser_client.spec.ts b/tests/plugins/browser_client.spec.ts index 8142df1..c6c9850 100644 --- a/tests/plugins/browser_client.spec.ts +++ b/tests/plugins/browser_client.spec.ts @@ -8,6 +8,7 @@ */ import getPort from 'get-port' +import { readdir } from 'node:fs/promises' import { test } from '@japa/runner' import { Router } from '@adonisjs/core/http' import { Emitter } from '@adonisjs/core/events' @@ -236,6 +237,7 @@ test.group('Browser client', (group) => { await browserContext.setSession({ username: 'virk' }) await visit(url) await browserContext.close() + assert.deepEqual(await readdir(fs.basePath), []) }) assert.deepEqual(seen, { username: 'virk' }) From b34b98458876225e5014e65c9da5832b0604b39e Mon Sep 17 00:00:00 2001 From: Romain Lanz Date: Sat, 17 Oct 2026 17:36:24 +0000 Subject: [PATCH 14/14] feat(session): make the new id for a missing session opt-in Giving a new id when the cookie names a session the store does not have changed a behavior apps can rely on: a session without data kept its id across requests. Apps that use session.sessionId as a key while the session is empty (a guest cart, rate limiting) saw it change on each request. The new id is now behind renewMissingSessionId, which defaults to false. Without it, Session keeps the cookie id as before. --- src/session.ts | 6 +++--- src/types.ts | 12 ++++++++++++ tests/session.spec.ts | 27 +++++++++++++++++++++++++-- 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/session.ts b/src/session.ts index d55b777..96fd37a 100644 --- a/src/session.ts +++ b/src/session.ts @@ -244,10 +244,10 @@ export class Session extends Macroable { /** * The cookie names a session the store does not have (expired or - * destroyed). Start a fresh session under a new id, so the id of - * an ended session is never used again. + * destroyed). With "renewMissingSessionId", start a fresh session + * under a new id, so the id of an ended session is never used again. */ - if (!contents && this.#sessionIdFromCookie) { + if (this.config.renewMissingSessionId && !contents && this.#sessionIdFromCookie) { debug('session %s not found, starting a new session', this.#sessionIdFromCookie) this.#sessionIdFromCookie = undefined this.#sessionId = randomUUID() diff --git a/src/types.ts b/src/types.ts index df87960..857b977 100644 --- a/src/types.ts +++ b/src/types.ts @@ -179,6 +179,18 @@ export interface SessionConfig { */ cookie: Partial + /** + * Whether to give a new session ID when the cookie names a session + * the store does not have (expired or destroyed). Defaults to false. + * + * When true, the ID of an ended session is never used again, so a + * late request that read the old session cannot write it back. The + * session is "fresh". A session without data then gets a new ID on + * each request, so do not use `session.sessionId` as a stable key + * for a session without data. + */ + renewMissingSessionId?: boolean + /** * Whether to send the session ID cookie on every response, which * refreshes its Max-Age. Defaults to true. diff --git a/tests/session.spec.ts b/tests/session.spec.ts index f585aeb..55f3d3e 100644 --- a/tests/session.spec.ts +++ b/tests/session.spec.ts @@ -354,7 +354,7 @@ test.group('Session', (group) => { assert.deepEqual(MemoryStore.sessions.get(newSessionId), { visits: 3 }) }) - test('start a new session when the cookie names a session the store does not have', async ({ + test('start a new session when the cookie names a missing session and renewMissingSessionId is true', async ({ assert, }) => { const endedSessionId = randomUUID() @@ -366,7 +366,8 @@ test.group('Session', (group) => { const response = new ResponseFactory().merge({ req, res, encryption }).create() const ctx = new HttpContextFactory().merge({ request, response }).create() - const session = new Session(sessionConfig, () => new MemoryStore(), emitter, ctx) + const config = { ...sessionConfig, renewMissingSessionId: true } + const session = new Session(config, () => new MemoryStore(), emitter, ctx) await session.initiate(false) session.put('visits', 1) @@ -388,6 +389,28 @@ test.group('Session', (group) => { assert.deepEqual(MemoryStore.sessions.get(sessionId!), { visits: 1 }) }) + test('keep the cookie session id of a missing session by default', async ({ assert }) => { + const sessionId = randomUUID() + + const server = httpServer.create(async (req, res) => { + const request = new RequestFactory().merge({ req, res, encryption }).create() + const response = new ResponseFactory().merge({ req, res, encryption }).create() + const ctx = new HttpContextFactory().merge({ request, response }).create() + + const session = new Session(sessionConfig, () => new MemoryStore(), emitter, ctx) + await session.initiate(false) + + session.put('visits', 1) + await session.commit() + response.finish() + }) + + const sessionIdCookie = `adonis_session=${cookieClient.sign('adonis_session', sessionId)}` + await supertest(server).get('/').set('cookie', sessionIdCookie) + + assert.deepEqual(MemoryStore.sessions.get(sessionId), { visits: 1 }) + }) + test('touch session store when not modified', async ({ assert }) => { let sessionId = randomUUID()