From 0ada1dfb469eea9e433443ee5a43d6fac29ab16a Mon Sep 17 00:00:00 2001
From: Laurent Cozic <laurent@cozic.net>
Date: Tue, 26 Oct 2021 12:34:22 +0100
Subject: [PATCH] Server: Expire sessions after 12 hours

---
 .../server/src/models/SessionModel.test.ts    | 46 +++++++++++++++++++
 packages/server/src/models/SessionModel.ts    |  8 ++++
 packages/server/src/services/TaskService.ts   |  1 +
 packages/server/src/utils/routeUtils.ts       | 12 ++++-
 packages/server/src/utils/setupTaskService.ts |  7 +++
 5 files changed, 73 insertions(+), 1 deletion(-)
 create mode 100644 packages/server/src/models/SessionModel.test.ts

diff --git a/packages/server/src/models/SessionModel.test.ts b/packages/server/src/models/SessionModel.test.ts
new file mode 100644
index 00000000000..5f797db793a
--- /dev/null
+++ b/packages/server/src/models/SessionModel.test.ts
@@ -0,0 +1,46 @@
+import { createUserAndSession, beforeAllDb, afterAllTests, beforeEachDb, models } from '../utils/testing/testUtils';
+import { defaultSessionTtl } from './SessionModel';
+
+describe('SessionModel', function() {
+
+	beforeAll(async () => {
+		await beforeAllDb('SessionModel');
+	});
+
+	afterAll(async () => {
+		await afterAllTests();
+	});
+
+	beforeEach(async () => {
+		await beforeEachDb();
+	});
+
+	test('should delete expired sessions', async function() {
+		jest.useFakeTimers('modern');
+
+		const t0 = new Date('2020-01-01T00:00:00').getTime();
+		jest.setSystemTime(t0);
+
+		const { user, password } = await createUserAndSession(1);
+		await models().session().authenticate(user.email, password);
+
+		jest.setSystemTime(new Date(t0 + defaultSessionTtl + 10));
+
+		const lastSession = await models().session().authenticate(user.email, password);
+
+		expect(await models().session().count()).toBe(3);
+
+		await models().session().deleteExpiredSessions();
+
+		expect(await models().session().count()).toBe(1);
+		expect((await models().session().all())[0].id).toBe(lastSession.id);
+
+		await models().session().authenticate(user.email, password);
+		await models().session().deleteExpiredSessions();
+
+		expect(await models().session().count()).toBe(2);
+
+		jest.useRealTimers();
+	});
+
+});
diff --git a/packages/server/src/models/SessionModel.ts b/packages/server/src/models/SessionModel.ts
index 5d1bb9a6e83..64185c9db7f 100644
--- a/packages/server/src/models/SessionModel.ts
+++ b/packages/server/src/models/SessionModel.ts
@@ -2,6 +2,9 @@ import BaseModel from './BaseModel';
 import { User, Session, Uuid } from '../services/database/types';
 import uuidgen from '../utils/uuidgen';
 import { ErrorForbidden } from '../utils/errors';
+import { Hour } from '../utils/time';
+
+export const defaultSessionTtl = 12 * Hour;
 
 export default class SessionModel extends BaseModel<Session> {
 
@@ -40,4 +43,9 @@ export default class SessionModel extends BaseModel<Session> {
 		await query.delete();
 	}
 
+	public async deleteExpiredSessions() {
+		const cutOffTime = Date.now() - defaultSessionTtl;
+		await this.db(this.tableName).where('created_time', '<', cutOffTime).delete();
+	}
+
 }
diff --git a/packages/server/src/services/TaskService.ts b/packages/server/src/services/TaskService.ts
index da551172b54..d224ba95d25 100644
--- a/packages/server/src/services/TaskService.ts
+++ b/packages/server/src/services/TaskService.ts
@@ -12,6 +12,7 @@ export enum TaskId {
 	HandleOversizedAccounts = 3,
 	HandleBetaUserEmails = 4,
 	HandleFailedPaymentSubscriptions = 5,
+	DeleteExpiredSessions = 6,
 }
 
 export enum RunType {
diff --git a/packages/server/src/utils/routeUtils.ts b/packages/server/src/utils/routeUtils.ts
index bdc78d90260..3cde05f392c 100644
--- a/packages/server/src/utils/routeUtils.ts
+++ b/packages/server/src/utils/routeUtils.ts
@@ -5,6 +5,7 @@ import Router from './Router';
 import { AppContext, HttpMethod, RouteType } from './types';
 import { URL } from 'url';
 import { csrfCheck } from './csrf';
+import { contextSessionId } from './requestUtils';
 
 const { ltrimSlashes, rtrimSlashes } = require('@joplin/lib/path-utils');
 
@@ -200,7 +201,16 @@ export async function execRequest(routes: Routers, ctx: AppContext) {
 	// This is a generic catch-all for all private end points - if we
 	// couldn't get a valid session, we exit now. Individual end points
 	// might have additional permission checks depending on the action.
-	if (!isPublicRoute && !ctx.joplin.owner) throw new ErrorForbidden();
+	if (!isPublicRoute && !ctx.joplin.owner) {
+		if (contextSessionId(ctx, false)) {
+			// If we have a session but not a user it means the session was
+			// invalid or has expired, so display a special message, since this
+			// is also going to be displayed on the website.
+			throw new ErrorForbidden('Your session has expired. Please login again.');
+		} else {
+			throw new ErrorForbidden();
+		}
+	}
 
 	await csrfCheck(ctx, isPublicRoute);
 	disabledAccountCheck(match, ctx.joplin.owner);
diff --git a/packages/server/src/utils/setupTaskService.ts b/packages/server/src/utils/setupTaskService.ts
index cf54ec60f73..3d3ee73dcd8 100644
--- a/packages/server/src/utils/setupTaskService.ts
+++ b/packages/server/src/utils/setupTaskService.ts
@@ -29,6 +29,13 @@ export default function(env: Env, models: Models, config: Config): TaskService {
 			schedule: '0 */2 30 * *',
 			run: (models: Models) => models.user().handleOversizedAccounts(),
 		},
+
+		{
+			id: TaskId.DeleteExpiredSessions,
+			description: 'Delete expired sessions',
+			schedule: '0 */6 * * *',
+			run: (models: Models) => models.session().deleteExpiredSessions(),
+		},
 	];
 
 	if (config.isJoplinCloud) {