From 1b8ff1428f76c9ae0ec4bb38474af1ab7af01839 Mon Sep 17 00:00:00 2001 From: Alice Gaudon Date: Sun, 24 Jan 2021 16:29:23 +0100 Subject: [PATCH] Add persist session checkbox on login Makes session not persistent by default Closes #11 --- config/default.json5 | 4 +- config/test.json5 | 5 ++ src/auth/AuthGuard.ts | 3 + src/auth/magic_link/MagicLinkAuthMethod.ts | 2 + src/auth/magic_link/MagicLinkController.ts | 2 +- src/auth/password/PasswordAuthMethod.ts | 8 +- src/components/RedisComponent.ts | 8 +- src/components/SessionComponent.ts | 14 +++- src/types/Express.d.ts | 4 + test/Authentication.test.ts | 91 ++++++++++++++++++++++ views/auth/auth.njk | 2 + 11 files changed, 132 insertions(+), 11 deletions(-) diff --git a/config/default.json5 b/config/default.json5 index c2b2321..a1c8c4d 100644 --- a/config/default.json5 +++ b/config/default.json5 @@ -31,8 +31,8 @@ secret: 'default', cookie: { secure: false, - maxAge: 2592000000, // 30 days - } + maxAge: 31557600000, // 1 year + }, }, mail: { host: "127.0.0.1", diff --git a/config/test.json5 b/config/test.json5 index e5cac9b..d83228f 100644 --- a/config/test.json5 +++ b/config/test.json5 @@ -6,4 +6,9 @@ database: "swaf_test", create_database_automatically: true, }, + session: { + cookie: { + maxAge: 1000, // 1s + }, + }, } diff --git a/src/auth/AuthGuard.ts b/src/auth/AuthGuard.ts index 09b735e..c165079 100644 --- a/src/auth/AuthGuard.ts +++ b/src/auth/AuthGuard.ts @@ -81,6 +81,7 @@ export default class AuthGuard { if (proofs.length === 0) { session.is_authenticated = false; + session.persistent = false; } return proofs; @@ -107,6 +108,7 @@ export default class AuthGuard { public async authenticateOrRegister( session: Session & Partial, proof: AuthProof, + persistSession: boolean, onLogin?: (user: User) => Promise, beforeRegister?: (connection: Connection, user: User) => Promise, afterRegister?: (connection: Connection, user: User) => Promise, @@ -151,6 +153,7 @@ export default class AuthGuard { // Login session.is_authenticated = true; + session.persistent = persistSession; if (onLogin) await onLogin(user); return user; diff --git a/src/auth/magic_link/MagicLinkAuthMethod.ts b/src/auth/magic_link/MagicLinkAuthMethod.ts index 4716d55..d79748f 100644 --- a/src/auth/magic_link/MagicLinkAuthMethod.ts +++ b/src/auth/magic_link/MagicLinkAuthMethod.ts @@ -98,6 +98,8 @@ export default class MagicLinkAuthMethod implements AuthMethod { }); } + req.getSession().wantsSessionPersistence = !!req.body.persist_session || isRegistration; + await MagicLinkController.sendMagicLink( this.app, req.getSession().id, diff --git a/src/auth/magic_link/MagicLinkController.ts b/src/auth/magic_link/MagicLinkController.ts index e50d9cb..eaa2a07 100644 --- a/src/auth/magic_link/MagicLinkController.ts +++ b/src/auth/magic_link/MagicLinkController.ts @@ -62,7 +62,7 @@ export default class MagicLinkController extends Controll // Auth try { return await req.as(AuthMiddleware).getAuthGuard().authenticateOrRegister( - session, magicLink, undefined, async (connection, user) => { + session, magicLink, !!session.wantsSessionPersistence, undefined, async (connection, user) => { const userNameComponent = user.asOptional(UserNameComponent); if (userNameComponent) userNameComponent.name = magicLink.as(MagicLinkUserNameComponent).username; return []; diff --git a/src/auth/password/PasswordAuthMethod.ts b/src/auth/password/PasswordAuthMethod.ts index dc2a8a6..0c46434 100644 --- a/src/auth/password/PasswordAuthMethod.ts +++ b/src/auth/password/PasswordAuthMethod.ts @@ -58,7 +58,11 @@ export default class PasswordAuthMethod implements AuthMethod await passwordAuthProof.authorize(req.body.password); try { - await this.app.as(AuthComponent).getAuthGuard().authenticateOrRegister(req.getSession(), passwordAuthProof); + await this.app.as(AuthComponent).getAuthGuard().authenticateOrRegister( + req.getSession(), + passwordAuthProof, + !!req.body.persist_session, + ); } catch (e) { if (e instanceof AuthError) { Throttler.throttle('login_failed_attempts_user', 3, 3 * 60 * 1000, // 3min @@ -102,7 +106,7 @@ export default class PasswordAuthMethod implements AuthMethod const passwordAuthProof = PasswordAuthProof.createAuthorizedProofForRegistration(req.getSession()); try { await this.app.as(AuthComponent).getAuthGuard().authenticateOrRegister(req.getSession(), passwordAuthProof, - undefined, async (connection, user) => { + true, undefined, async (connection, user) => { const callbacks: RegisterCallback[] = []; // Password diff --git a/src/components/RedisComponent.ts b/src/components/RedisComponent.ts index a5a986b..85363e8 100644 --- a/src/components/RedisComponent.ts +++ b/src/components/RedisComponent.ts @@ -114,10 +114,10 @@ class RedisStore extends Store { } public get(sid: string, callback: (err?: Error, session?: (session.SessionData | null)) => void): void { - this.redisComponent.get('-session:' + sid) + this.redisComponent.get(`-session:${sid}`) .then(value => { if (value) { - this.redisComponent.persist('-session:' + sid, 2592000000) // 30 days + this.redisComponent.persist(`-session:${sid}`, config.get('session.cookie.maxAge')) .then(() => { callback(undefined, JSON.parse(value)); }) @@ -130,7 +130,7 @@ class RedisStore extends Store { } public set(sid: string, session: session.SessionData, callback?: (err?: Error) => void): void { - this.redisComponent.remember('-session:' + sid, JSON.stringify(session), 2592000000) // 30 days + this.redisComponent.remember(`-session:${sid}`, JSON.stringify(session), config.get('session.cookie.maxAge')) .then(() => { if (callback) callback(); }) @@ -138,7 +138,7 @@ class RedisStore extends Store { } public destroy(sid: string, callback?: (err?: Error) => void): void { - this.redisComponent.forget('-session:' + sid) + this.redisComponent.forget(`-session:${sid}`) .then(() => { if (callback) callback(); }) diff --git a/src/components/SessionComponent.ts b/src/components/SessionComponent.ts index e915397..1c86fe6 100644 --- a/src/components/SessionComponent.ts +++ b/src/components/SessionComponent.ts @@ -30,7 +30,6 @@ export default class SessionComponent extends ApplicationComponent { cookie: { httpOnly: true, secure: config.get('session.cookie.secure'), - maxAge: config.get('session.cookie.maxAge'), }, rolling: true, })); @@ -38,6 +37,7 @@ export default class SessionComponent extends ApplicationComponent { router.use(flash()); router.use((req, res, next) => { + // Request session getters req.getSessionOptional = () => { return req.session; }; @@ -47,8 +47,18 @@ export default class SessionComponent extends ApplicationComponent { return session; }; - res.locals.session = req.getSession(); + // Session persistence + const session = req.getSession(); + if (session.persistent) { + session.cookie.maxAge = config.get('session.cookie.maxAge'); + } else { + session.cookie.maxAge = session.cookie.expires = undefined; + } + // Views session local + res.locals.session = session; + + // Views flash function const _flash: FlashStorage = {}; res.locals.flash = (key?: string): FlashMessages | unknown[] => { if (key !== undefined) { diff --git a/src/types/Express.d.ts b/src/types/Express.d.ts index 6b8b65a..33fba07 100644 --- a/src/types/Express.d.ts +++ b/src/types/Express.d.ts @@ -40,6 +40,10 @@ declare module 'express-session' { previousUrl?: string; + wantsSessionPersistence?: boolean; + + persistent?: boolean; + is_authenticated?: boolean; auth_password_proof?: PasswordAuthProofSessionData; diff --git a/test/Authentication.test.ts b/test/Authentication.test.ts index b1d6692..bc165a6 100644 --- a/test/Authentication.test.ts +++ b/test/Authentication.test.ts @@ -1028,3 +1028,94 @@ describe('Manage email addresses', () => { }); }); }); + +describe('Session persistence', () => { + let cookies: string[], csrf: string; + + test('Not persistent at registration', async () => { + let res = await agent.get('/csrf').expect(200); + cookies = res.get('Set-Cookie'); + csrf = res.text; + + await agent.post('/auth/register') + .set('Cookie', cookies) + .send({ + csrf: csrf, + auth_method: 'magic_link', + identifier: 'zuko@example.org', + name: 'zuko', + }) + .expect(302) + .expect('Location', '/magic/lobby?redirect_uri=%2Fcsrf'); + + await followMagicLinkFromMail(agent, cookies); + + expect(cookies[0]).toMatch(/^connect\.sid=.+; Path=\/; HttpOnly$/); + + res = await agent.get('/csrf') + .set('Cookie', cookies) + .expect(200); + expect(res.get('Set-Cookie')[0]).toMatch(/^connect\.sid=.+; Path=\/; Expires=.+; HttpOnly$/); + + // Logout + await agent.post('/auth/logout') + .set('Cookie', cookies) + .send({csrf}) + .expect(302) + .expect('Location', '/'); + }); + + test('Login with persistence', async () => { + await agent.post('/auth/login') + .set('Cookie', cookies) + .send({ + csrf: csrf, + auth_method: 'magic_link', + identifier: 'zuko@example.org', + persist_session: 'on', + }) + .expect(302) + .expect('Location', '/magic/lobby?redirect_uri=%2Fcsrf'); + + await followMagicLinkFromMail(agent, cookies); + + const res = await agent.get('/csrf') + .set('Cookie', cookies) + .expect(200); + expect(res.get('Set-Cookie')[0]).toMatch(/^connect\.sid=.+; Path=\/; Expires=.+; HttpOnly$/); + + // Logout + await agent.post('/auth/logout') + .set('Cookie', cookies) + .send({csrf}) + .expect(302) + .expect('Location', '/'); + }); + + test('Login without persistence', async () => { + await agent.post('/auth/login') + .set('Cookie', cookies) + .send({ + csrf: csrf, + auth_method: 'magic_link', + identifier: 'zuko@example.org', + persist_session: undefined, + }) + .expect(302) + .expect('Location', '/magic/lobby?redirect_uri=%2Fcsrf'); + + await followMagicLinkFromMail(agent, cookies); + + const res = await agent.get('/csrf') + .set('Cookie', cookies) + .expect(200); + expect(res.get('Set-Cookie')[0]).toMatch(/^connect\.sid=.+; Path=\/; HttpOnly$/); + + // Logout + await agent.post('/auth/logout') + .set('Cookie', cookies) + .send({csrf}) + .expect(302) + .expect('Location', '/'); + }); +}); diff --git a/views/auth/auth.njk b/views/auth/auth.njk index d9a5623..4cb30eb 100644 --- a/views/auth/auth.njk +++ b/views/auth/auth.njk @@ -20,6 +20,8 @@ {{ macros.field(_locals, 'password', 'password', null, 'Your password', 'Do not fill to log in via magic link.') }} + {{ macros.field(_locals, 'checkbox', 'persist_session', null, 'Stay logged in on this computer.') }} + {{ macros.csrf(getCsrfToken) }}