Server: Increase cookies security - set HttpOnly, Secure and SameSite flags

packages/server/src/config.ts

@@ -46,6 +46,8 @@ export interface EnvVariables {
 	SUPPORT_NAME?: string;
 
 	BUSINESS_EMAIL?: string;
+
+	COOKIES_SECURE?: string;
 }
 
 let runningInDocker_: boolean = false;
@@ -168,6 +170,7 @@ export async function initConfig(envType: Env, env: EnvVariables, overrides: any
 		supportEmail,
 		supportName: env.SUPPORT_NAME || appName,
 		businessEmail: env.BUSINESS_EMAIL || supportEmail,
+		cookieSecure: env.COOKIES_SECURE === '1',
 		...overrides,
 	};
 }

packages/server/src/routes/index/login.test.ts

@@ -1,5 +1,6 @@
 import { Session } from '../../db';
 import routeHandler from '../../middleware/routeHandler';
+import { cookieGet } from '../../utils/cookies';
 import { beforeAllDb, afterAllTests, beforeEachDb, koaAppContext, models, parseHtml, createUser } from '../../utils/testing/testUtils';
 import { AppContext } from '../../utils/types';
 
@@ -52,7 +53,7 @@ describe('index_login', function() {
 		const user = await createUser(1);
 
 		const context = await doLogin(user.email, '123456');
-		const sessionId = context.cookies.get('sessionId');
+		const sessionId = cookieGet(context, 'sessionId');
 		const session: Session = await models().session().load(sessionId);
 		expect(session.user_id).toBe(user.id);
 	});
@@ -62,12 +63,12 @@ describe('index_login', function() {
 
 		{
 			const context = await doLogin('bad', '123456');
-			expect(!context.cookies.get('sessionId')).toBe(true);
+			expect(!cookieGet(context, 'sessionId')).toBe(true);
 		}
 
 		{
 			const context = await doLogin(user.email, 'bad');
-			expect(!context.cookies.get('sessionId')).toBe(true);
+			expect(!cookieGet(context, 'sessionId')).toBe(true);
 		}
 	});
 

packages/server/src/routes/index/login.ts

@@ -7,6 +7,7 @@ import config from '../../config';
 import defaultView from '../../utils/defaultView';
 import { View } from '../../services/MustacheService';
 import limiterLoginBruteForce from '../../utils/request/limiterLoginBruteForce';
+import { cookieSet } from '../../utils/cookies';
 
 function makeView(error: any = null): View {
 	const view = defaultView('login', 'Login');
@@ -32,7 +33,7 @@ router.post('login', async (_path: SubPath, ctx: AppContext) => {
 		const body = await formParse(ctx.req);
 
 		const session = await ctx.joplin.models.session().authenticate(body.fields.email, body.fields.password);
-		ctx.cookies.set('sessionId', session.id);
+		cookieSet(ctx, 'sessionId', session.id);
 		return redirect(ctx, `${config().baseUrl}/home`);
 	} catch (error) {
 		return makeView(error);

packages/server/src/routes/index/logout.test.ts

@@ -1,4 +1,5 @@
 import routeHandler from '../../middleware/routeHandler';
+import { cookieGet } from '../../utils/cookies';
 import { beforeAllDb, afterAllTests, beforeEachDb, koaAppContext, models, createUserAndSession } from '../../utils/testing/testUtils';
 
 describe('index_logout', function() {
@@ -26,11 +27,11 @@ describe('index_logout', function() {
 			},
 		});
 
-		expect(context.cookies.get('sessionId')).toBe(session.id);
+		expect(cookieGet(context, 'sessionId')).toBe(session.id);
 		expect(!!(await models().session().load(session.id))).toBe(true);
 		await routeHandler(context);
 
-		expect(!context.cookies.get('sessionId')).toBe(true);
+		expect(!cookieGet(context, 'sessionId')).toBe(true);
 		expect(!!(await models().session().load(session.id))).toBe(false);
 	});
 

packages/server/src/routes/index/logout.ts

@@ -4,12 +4,13 @@ import { RouteType } from '../../utils/types';
 import { AppContext } from '../../utils/types';
 import config from '../../config';
 import { contextSessionId } from '../../utils/requestUtils';
+import { cookieSet } from '../../utils/cookies';
 
 const router = new Router(RouteType.Web);
 
 router.post('logout', async (_path: SubPath, ctx: AppContext) => {
 	const sessionId = contextSessionId(ctx, false);
-	ctx.cookies.set('sessionId', '');
+	cookieSet(ctx, 'sessionId', '');
 	await ctx.joplin.models.session().logout(sessionId);
 	return redirect(ctx, `${config().baseUrl}/login`);
 });

packages/server/src/routes/index/signup.test.ts

@@ -3,6 +3,7 @@ import { NotificationKey } from '../../models/NotificationModel';
 import { AccountType } from '../../models/UserModel';
 import { getCanShareFolder, getMaxItemSize } from '../../models/utils/user';
 import { MB } from '../../utils/bytes';
+import { cookieGet } from '../../utils/cookies';
 import { execRequestC } from '../../utils/testing/apiUtils';
 import { beforeAllDb, afterAllTests, beforeEachDb, models } from '../../utils/testing/testUtils';
 import uuidgen from '../../utils/uuidgen';
@@ -50,7 +51,7 @@ describe('index_signup', function() {
 		expect(getMaxItemSize(user)).toBe(10 * MB);
 
 		// Check that the user is logged in
-		const session = await models().session().load(context.cookies.get('sessionId'));
+		const session = await models().session().load(cookieGet(context, 'sessionId'));
 		expect(session.user_id).toBe(user.id);
 
 		// Check that the notification has been created

packages/server/src/routes/index/signup.ts

@@ -10,6 +10,7 @@ import { checkRepeatPassword } from './users';
 import { NotificationKey } from '../../models/NotificationModel';
 import { AccountType } from '../../models/UserModel';
 import { ErrorForbidden } from '../../utils/errors';
+import { cookieSet } from '../../utils/cookies';
 
 function makeView(error: Error = null): View {
 	const view = defaultView('signup', 'Sign Up');
@@ -51,7 +52,7 @@ router.post('signup', async (_path: SubPath, ctx: AppContext) => {
 		});
 
 		const session = await ctx.joplin.models.session().createUserSession(user.id);
-		ctx.cookies.set('sessionId', session.id);
+		cookieSet(ctx, 'sessionId', session.id);
 
 		await ctx.joplin.models.notification().add(user.id, NotificationKey.ConfirmEmail);
 

packages/server/src/routes/index/users.test.ts

@@ -1,6 +1,7 @@
 import { User } from '../../db';
 import routeHandler from '../../middleware/routeHandler';
 import { NotificationKey } from '../../models/NotificationModel';
+import { cookieGet } from '../../utils/cookies';
 import { ErrorForbidden } from '../../utils/errors';
 import { execRequest, execRequestC } from '../../utils/testing/apiUtils';
 import { beforeAllDb, afterAllTests, beforeEachDb, koaAppContext, createUserAndSession, models, parseHtml, checkContextError, expectHttpError } from '../../utils/testing/testUtils';
@@ -240,7 +241,7 @@ describe('index/users', function() {
 				password: newPassword,
 				password2: newPassword,
 			});
-			const sessionId = context.cookies.get('sessionId');
+			const sessionId = cookieGet(context, 'sessionId');
 			expect(sessionId).toBeFalsy();
 		}
 
@@ -253,7 +254,7 @@ describe('index/users', function() {
 				password2: newPassword,
 				token: token2,
 			});
-			const sessionId = context.cookies.get('sessionId');
+			const sessionId = cookieGet(context, 'sessionId');
 			expect(sessionId).toBeFalsy();
 		}
 
@@ -266,7 +267,7 @@ describe('index/users', function() {
 		});
 
 		// Check that the user has been logged in
-		const sessionId = context.cookies.get('sessionId');
+		const sessionId = cookieGet(context, 'sessionId');
 		const session = await models().session().load(sessionId);
 		expect(session.user_id).toBe(user1.id);
 
@@ -303,7 +304,7 @@ describe('index/users', function() {
 		user1 = await models().user().load(user1.id);
 
 		// Check that the user has been logged in
-		const sessionId = context.cookies.get('sessionId');
+		const sessionId = cookieGet(context, 'sessionId');
 		expect(sessionId).toBeFalsy();
 
 		// Check that the email has been verified

packages/server/src/routes/index/users.ts

@@ -19,6 +19,7 @@ import { confirmUrl } from '../../utils/urlUtils';
 import { cancelSubscriptionByUserId, updateSubscriptionType } from '../../utils/stripe';
 import { createCsrfTag } from '../../utils/csrf';
 import { formatDateTime } from '../../utils/time';
+import { cookieSet } from '../../utils/cookies';
 
 export interface CheckRepeatPasswordInput {
 	password: string;
@@ -236,7 +237,7 @@ router.post('users/:id/confirm', async (path: SubPath, ctx: AppContext) => {
 		await ctx.joplin.models.token().deleteByValue(userId, fields.token);
 
 		const session = await ctx.joplin.models.session().createUserSession(userId);
-		ctx.cookies.set('sessionId', session.id);
+		cookieSet(ctx, 'sessionId', session.id);
 
 		await ctx.joplin.models.notification().add(userId, NotificationKey.PasswordSet);
 

packages/server/src/utils/cookies.ts

@@ -0,0 +1,17 @@
+import config from '../config';
+import { AppContext } from './types';
+
+export function cookieSet(ctx: AppContext, name: string, value: string) {
+	ctx.cookies.set(name, value, {
+		// Means that the cookies cannot be accessed from JavaScript
+		httpOnly: true,
+		// Can only be transferred over https
+		secure: config().cookieSecure,
+		// Prevent cookies from being sent in cross-site requests
+		sameSite: true,
+	});
+}
+
+export function cookieGet(ctx: AppContext, name: string) {
+	return ctx.cookies.get(name);
+}

packages/server/src/utils/requestUtils.ts

@@ -1,3 +1,4 @@
+import { cookieGet } from './cookies';
 import { ErrorForbidden } from './errors';
 import { AppContext } from './types';
 
@@ -61,7 +62,7 @@ export function headerSessionId(headers: any): string {
 export function contextSessionId(ctx: AppContext, throwIfNotFound = true): string {
 	if (ctx.headers['x-api-auth']) return ctx.headers['x-api-auth'];
 
-	const id = ctx.cookies.get('sessionId');
+	const id = cookieGet(ctx, 'sessionId');
 	if (!id && throwIfNotFound) throw new ErrorForbidden('Invalid or missing session');
 	return id;
 }

packages/server/src/utils/testing/testUtils.ts

@@ -21,6 +21,7 @@ import { initializeJoplinUtils } from '../joplinUtils';
 import MustacheService from '../../services/MustacheService';
 import uuidgen from '../uuidgen';
 import { createCsrfToken } from '../csrf';
+import { cookieSet } from '../cookies';
 
 // Takes into account the fact that this file will be inside the /dist directory
 // when it runs.
@@ -211,7 +212,7 @@ export async function koaAppContext(options: AppContextTestOptions = null): Prom
 	};
 
 	if (options.sessionId) {
-		appContext.cookies.set('sessionId', options.sessionId);
+		cookieSet(appContext, 'sessionId', options.sessionId);
 	}
 
 	return appContext as AppContext;

packages/server/src/utils/types.ts

@@ -108,6 +108,7 @@ export interface Config {
 	supportName: string;
 	businessEmail: string;
 	isJoplinCloud: boolean;
+	cookieSecure: boolean;
 }
 
 export enum HttpMethod {