diff --git a/src/constants.ts b/src/constants.ts index a74bd74..42a3db4 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -1,2 +1,29 @@ export const PLUGIN_NAME = 'graasp-plugin-etherpad'; export const ETHERPAD_API_VERSION = '1.2.13'; + +/** + * User agents are now required to limit the sum of the lengths of the cookie's + * name and value to 4096 bytes, and limit the length of each cookie attribute + * value to 1024 bytes. Any attempt to set a cookie exceeding the name+value + * limit is rejected, and any cookie attribute exceeding the attribute length + * limit is ignored. See https://chromestatus.com/feature/4946713618939904 + */ +export const MAX_COOKIE_VALUE_SIZE_BYTES = 1024; + +/** + * sessionID a string, the unique id of a session. Format is s.16RANDOMCHARS + * for example s.s8oes9dhwrvt0zif (length is 18) + * See https://etherpad.org/doc/v1.8.18/#index_data-types + * We add a comma since the session IDs are joined (length + 1) + * The session cookie can also contain multiple comma-separated sessionIDs + * See https://etherpad.org/doc/v1.8.18/#index_session + */ +export const SESSION_VALUE_COOKIE_LENGTH = 19; + +/** + * Thus the maximum cookie size for etherpad sessions is + * _MAX_COOKIE_VALUE_SIZE_BYTES / SESSION_VALUE_COOKIE_LENGTH_ + */ +export const MAX_SESSIONS_IN_COOKIE = Math.floor( + MAX_COOKIE_VALUE_SIZE_BYTES / SESSION_VALUE_COOKIE_LENGTH, +); diff --git a/src/service-api.ts b/src/service-api.ts index f35b229..c6872a9 100644 --- a/src/service-api.ts +++ b/src/service-api.ts @@ -11,7 +11,7 @@ import { PermissionLevelCompare, } from '@graasp/sdk'; -import { ETHERPAD_API_VERSION } from './constants'; +import { ETHERPAD_API_VERSION, MAX_SESSIONS_IN_COOKIE, PLUGIN_NAME } from './constants'; import { AccessForbiddenError, ItemMissingExtraError, ItemNotFoundError } from './errors'; import { GraaspEtherpad } from './etherpad'; import { createEtherpad, getEtherpadFromItem } from './schemas'; @@ -24,6 +24,7 @@ const plugin: FastifyPluginAsync = async (fastify, option items: { taskManager: itemTaskManager }, itemMemberships: { taskManager: itemMembershipTaskManager }, taskRunner, + log, } = fastify; const { url: etherpadUrl, publicUrl, apiKey, cookieDomain } = validatePluginOptions(options); @@ -93,7 +94,12 @@ const plugin: FastifyPluginAsync = async (fastify, option return await taskRunner.runSingleSequence(createItem); } catch (error) { // create item failed, delete created pad - etherpad.deletePad({ padID: buildPadID({ groupID, padName }) }); + const padID = buildPadID({ groupID, padName }); + etherpad + .deletePad({ padID }) + .catch((e) => + log.error(`${PLUGIN_NAME}: failed to delete orphan etherpad ${padID}`, e), + ); throw error; } }, @@ -179,11 +185,72 @@ const plugin: FastifyPluginAsync = async (fastify, option const { sessionID } = await etherpad.createSession({ authorID, groupID, - validUntil: expiration.toSeconds(), + validUntil: expiration.toUnixInteger(), }); - // set cookie - reply.setCookie('sessionID', sessionID, { + // get available sessions for user + const sessions = (await etherpad.listSessionsOfAuthor({ authorID })) ?? {}; + + // split valid from expired cookies + const now = DateTime.now(); + const { valid, expired } = Object.entries(sessions).reduce( + ({ valid, expired }, [id, { validUntil }]) => { + const isExpired = DateTime.fromSeconds(validUntil) <= now; + isExpired ? expired.add(id) : valid.add(id); + return { valid, expired }; + }, + { + valid: new Set(), + expired: new Set(), + }, + ); + // sanity check, add the new sessionID (should already be part of the set) + valid.add(sessionID); + + // in practice, there is (probably) a limit of 1024B per cookie value + // https://chromestatus.com/feature/4946713618939904 + // so we can only store up to limit / (size of sessionID string + ",") + // assuming that no other cookies are set on the etherpad domain + // to err on the cautious side, we invalidate the oldest cookies in this case + if (valid.size > MAX_SESSIONS_IN_COOKIE) { + const sortedRecent = Array.from(valid).sort((a, b) => { + // return inversed for most recent + const timeA = DateTime.fromSeconds(sessions[a].validUntil); + const timeB = DateTime.fromSeconds(sessions[b].validUntil); + if (timeA < timeB) { + return 1; + } + if (timeA > timeB) { + return -1; + } + return 0; + }); + + const toInvalidate = sortedRecent.slice(MAX_SESSIONS_IN_COOKIE); + + // mutate valid and expired sets in place + toInvalidate.forEach((id) => { + valid.delete(id); + expired.add(id); + }); + } + + // delete expired cookies asynchronously in the background, accept failures by catching + expired.forEach((sessionID) => { + etherpad + .deleteSession({ sessionID }) + .catch((e) => + log.error( + `${PLUGIN_NAME}: failed to delete etherpad session ${sessionID}`, + sessions[sessionID], + e, + ), + ); + }); + + // set cookie with all valid cookies (users should be able to access multiple etherpads simultaneously) + const cookieValue = Array.from(valid).join(','); + reply.setCookie('sessionID', cookieValue, { domain: cookieDomain, path: '/', expires: expiration.toJSDate(), diff --git a/test/app.ts b/test/app.ts index 29b04c6..fa497f1 100644 --- a/test/app.ts +++ b/test/app.ts @@ -27,7 +27,7 @@ type Awaited = T extends PromiseLike ? U : T; export type BuildAppType = Awaited>; export async function buildApp(args?: { options?: EtherpadPluginOptions }) { - const app = fastify(); + const app = fastify({ logger: true }); app.register(fastifyCookie); diff --git a/test/service-api.test.ts b/test/service-api.test.ts index d24eff7..6bbfad1 100644 --- a/test/service-api.test.ts +++ b/test/service-api.test.ts @@ -12,6 +12,7 @@ import { TaskStatus, } from '@graasp/sdk'; +import { MAX_SESSIONS_IN_COOKIE } from '../src/constants'; import plugin from '../src/service-api'; import { setUpApi } from './api'; import { BuildAppType, buildApp } from './app'; @@ -176,6 +177,7 @@ describe('Service API', () => { StatusCodes.OK, { code: 0, message: 'ok', data: { sessionID: MOCK_SESSION_ID } }, ], + listSessionsOfAuthor: [StatusCodes.OK, { code: 0, message: 'ok', data: null }], }); const res = await app.inject(payloadView('read')); @@ -198,6 +200,7 @@ describe('Service API', () => { StatusCodes.OK, { code: 0, message: 'ok', data: { sessionID: MOCK_SESSION_ID } }, ], + listSessionsOfAuthor: [StatusCodes.OK, { code: 0, message: 'ok', data: null }], }); // override get item membership: should return with write permission @@ -220,14 +223,23 @@ describe('Service API', () => { expect(res.json()).toEqual({ padUrl: `${TEST_ENV.url}/p/${MOCK_GROUP_ID}$mock-pad-name`, }); - const mockSessionIdRegex = MOCK_SESSION_ID.replace('.', '\\.'); - const cookieRegex = new RegExp( - `^sessionID=${mockSessionIdRegex}; Domain=localhost; Path=\/; Expires=(.*)$`, - ); - expect(res.headers['set-cookie']).toMatch(cookieRegex); - const matches = res.headers['set-cookie']?.toString().match(cookieRegex) as RegExpMatchArray; - const [_, dateString] = matches; - expect(DateTime.fromRFC2822(dateString).isValid).toBe(true); + + expect(res.cookies.length).toEqual(1); + const { name, value, domain, path, expires } = res.cookies[0] as { + name: string; + value: string; + domain: string; + path: string; + expires: Date; + }; + expect(name).toEqual('sessionID'); + expect(value).toEqual(MOCK_SESSION_ID); + expect(domain).toEqual('localhost'); + expect(path).toEqual('/'); + const expiration = DateTime.fromJSDate(expires); + // since we don't know the exact time the server created the cookie, verify against a range + expect(expiration > DateTime.now().plus({ days: 1 }).minus({ minutes: 1 })).toBeTruthy(); + expect(expiration < DateTime.now().plus({ days: 1 }).plus({ minutes: 1 })).toBeTruthy(); }); it('views a pad in write mode returns a read-only pad ID if user has read permission only', async () => { @@ -245,6 +257,7 @@ describe('Service API', () => { StatusCodes.OK, { code: 0, message: 'ok', data: { sessionID: MOCK_SESSION_ID } }, ], + listSessionsOfAuthor: [StatusCodes.OK, { code: 0, message: 'ok', data: null }], }); const res = await app.inject(payloadView('write')); // <- we request write mode and should get a read ID @@ -256,6 +269,213 @@ describe('Service API', () => { }); }); + it('concatenates existing sessions in cookie', async () => { + const { app } = instance; + const reqParams = setUpApi({ + getReadOnlyID: [ + StatusCodes.OK, + { code: 0, message: 'ok', data: { readOnlyID: MOCK_PAD_READ_ONLY_ID } }, + ], + createAuthorIfNotExistsFor: [ + StatusCodes.OK, + { code: 0, message: 'ok', data: { authorID: MOCK_AUTHOR_ID } }, + ], + createSession: [ + StatusCodes.OK, + { code: 0, message: 'ok', data: { sessionID: MOCK_SESSION_ID } }, + ], + listSessionsOfAuthor: [ + StatusCodes.OK, + { + code: 0, + message: 'ok', + data: { + [MOCK_SESSION_ID]: { + groupID: MOCK_GROUP_ID, + authorID: MOCK_AUTHOR_ID, + validUntil: DateTime.now().plus({ days: 1 }).toUnixInteger(), + }, + ['s.0000000000000000']: { + groupID: MOCK_GROUP_ID, + authorID: MOCK_AUTHOR_ID, + validUntil: DateTime.now().plus({ days: 1 }).toUnixInteger(), + }, + }, + }, + ], + }); + + const res = await app.inject(payloadView('read')); + + const { getReadOnlyID } = await reqParams; + expect(getReadOnlyID?.get('padID')).toEqual(MOCK_PAD_ID); + expect(res.statusCode).toEqual(StatusCodes.OK); + expect(res.json()).toEqual({ + padUrl: `${TEST_ENV.url}/p/${MOCK_PAD_READ_ONLY_ID}`, + }); + + expect(res.cookies.length).toEqual(1); + const { name, value, domain, path, expires } = res.cookies[0] as { + name: string; + value: string; + domain: string; + path: string; + expires: Date; + }; + expect(name).toEqual('sessionID'); + const sessions = value.split(','); + expect(sessions.length).toEqual(2); + expect(sessions.includes(MOCK_SESSION_ID)).toBeTruthy(); + expect(sessions.includes('s.0000000000000000')).toBeTruthy(); + expect(domain).toEqual('localhost'); + expect(path).toEqual('/'); + const expiration = DateTime.fromJSDate(expires); + // since we don't know the exact time the server created the cookie, verify against a range + expect(expiration > DateTime.now().plus({ days: 1 }).minus({ minutes: 1 })).toBeTruthy(); + expect(expiration < DateTime.now().plus({ days: 1 }).plus({ minutes: 1 })).toBeTruthy(); + }); + + it('deletes expired sessions', async () => { + const { app } = instance; + const reqParams = setUpApi({ + getReadOnlyID: [ + StatusCodes.OK, + { code: 0, message: 'ok', data: { readOnlyID: MOCK_PAD_READ_ONLY_ID } }, + ], + createAuthorIfNotExistsFor: [ + StatusCodes.OK, + { code: 0, message: 'ok', data: { authorID: MOCK_AUTHOR_ID } }, + ], + createSession: [ + StatusCodes.OK, + { code: 0, message: 'ok', data: { sessionID: MOCK_SESSION_ID } }, + ], + listSessionsOfAuthor: [ + StatusCodes.OK, + { + code: 0, + message: 'ok', + data: { + [MOCK_SESSION_ID]: { + groupID: MOCK_GROUP_ID, + authorID: MOCK_AUTHOR_ID, + validUntil: DateTime.now().minus({ days: 1 }).toUnixInteger(), + }, + }, + }, + ], + deleteSession: [StatusCodes.OK, { code: 0, message: 'ok', data: null }], + }); + + const res = await app.inject(payloadView('read')); + + const { getReadOnlyID, deleteSession } = await reqParams; + expect(getReadOnlyID?.get('padID')).toEqual(MOCK_PAD_ID); + expect(res.statusCode).toEqual(StatusCodes.OK); + expect(res.json()).toEqual({ + padUrl: `${TEST_ENV.url}/p/${MOCK_PAD_READ_ONLY_ID}`, + }); + + expect(res.cookies.length).toEqual(1); + const { name, value, domain, path, expires } = res.cookies[0] as { + name: string; + value: string; + domain: string; + path: string; + expires: Date; + }; + expect(name).toEqual('sessionID'); + expect(value).toEqual(MOCK_SESSION_ID); + expect(domain).toEqual('localhost'); + expect(path).toEqual('/'); + const expiration = DateTime.fromJSDate(expires); + // since we don't know the exact time the server created the cookie, verify against a range + expect(expiration > DateTime.now().plus({ days: 1 }).minus({ minutes: 1 })).toBeTruthy(); + expect(expiration < DateTime.now().plus({ days: 1 }).plus({ minutes: 1 })).toBeTruthy(); + + expect(deleteSession?.get('sessionID')).toEqual(MOCK_SESSION_ID); + }); + + it('invalidates oldest sessions if the number of sessions exceeds MAX_SESSIONS_IN_COOKIES', async () => { + const { app } = instance; + const reqParams = setUpApi({ + getReadOnlyID: [ + StatusCodes.OK, + { code: 0, message: 'ok', data: { readOnlyID: MOCK_PAD_READ_ONLY_ID } }, + ], + createAuthorIfNotExistsFor: [ + StatusCodes.OK, + { code: 0, message: 'ok', data: { authorID: MOCK_AUTHOR_ID } }, + ], + createSession: [ + StatusCodes.OK, + { code: 0, message: 'ok', data: { sessionID: MOCK_SESSION_ID } }, + ], + listSessionsOfAuthor: [ + StatusCodes.OK, + { + code: 0, + message: 'ok', + data: { + ...Object.fromEntries( + Array.from({ length: MAX_SESSIONS_IN_COOKIE }, (_, i) => [ + `s.${i.toString().padStart(16, '0')}`, + { + groupID: `g.${i.toString().padStart(16, '0')}`, + authorID: MOCK_AUTHOR_ID, + validUntil: DateTime.now().plus({ seconds: i }).toUnixInteger(), + }, + ]), + ), + // this emulates the newly created session + [MOCK_SESSION_ID]: { + groupID: MOCK_GROUP_ID, + authorID: MOCK_AUTHOR_ID, + validUntil: DateTime.now().plus({ days: 1 }).toUnixInteger(), + }, + }, + }, + ], + deleteSession: [StatusCodes.OK, { code: 0, message: 'ok', data: null }], + }); + + const res = await app.inject(payloadView('read')); + + const { getReadOnlyID, deleteSession } = await reqParams; + expect(getReadOnlyID?.get('padID')).toEqual(MOCK_PAD_ID); + expect(res.statusCode).toEqual(StatusCodes.OK); + expect(res.json()).toEqual({ + padUrl: `${TEST_ENV.url}/p/${MOCK_PAD_READ_ONLY_ID}`, + }); + + expect(res.cookies.length).toEqual(1); + const { name, value, domain, path, expires } = res.cookies[0] as { + name: string; + value: string; + domain: string; + path: string; + expires: Date; + }; + expect(name).toEqual('sessionID'); + const sessions = value.split(','); + expect(sessions.length).toEqual(MAX_SESSIONS_IN_COOKIE); + // the first (oldest) session should not be in the cookie + Array.from( + { length: MAX_SESSIONS_IN_COOKIE - 1 }, + (_, i) => `s.${(i + 1).toString().padStart(16, '0')}`, + ).forEach((s) => expect(sessions.includes(s))); + expect(sessions.includes(MOCK_SESSION_ID)).toBeTruthy(); + expect(sessions.includes('s.0000000000000000')).toBeFalsy(); + expect(domain).toEqual('localhost'); + expect(path).toEqual('/'); + const expiration = DateTime.fromJSDate(expires); + // since we don't know the exact time the server created the cookie, verify against a range + expect(expiration > DateTime.now().plus({ days: 1 }).minus({ minutes: 1 })).toBeTruthy(); + expect(expiration < DateTime.now().plus({ days: 1 }).plus({ minutes: 1 })).toBeTruthy(); + // the first (oldest) session should be invalidated + expect(deleteSession?.get('sessionID')).toEqual('s.0000000000000000'); + }); + it.each(MODES)('returns error if item is not found (%p)', async (mode) => { const { app, spies } = instance; setUpApi({