From 578078e03f94ac3de447fc3484322e027b3bf0e3 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Tue, 27 Apr 2021 09:16:44 +0200 Subject: [PATCH] fix: active sessions are now destroyed if auth/reset and auth/validate endpoints are used (#806) --- src/lib/db/index.ts | 47 ++++---- src/lib/db/session-store.ts | 105 ++++++++++++++++ src/lib/routes/admin-api/user-admin.ts | 17 +++ src/lib/routes/admin-api/user.ts | 30 ++++- .../routes/auth/reset-password-controller.ts | 25 ++-- src/lib/services/addon-service.js | 10 +- src/lib/services/addon-service.test.js | 6 +- src/lib/services/index.ts | 42 +++---- src/lib/services/session-service.ts | 47 ++++++++ src/lib/services/user-service.test.ts | 27 +++++ src/lib/services/user-service.ts | 37 ++++-- src/lib/types/services.ts | 2 + src/lib/types/stores.ts | 2 + .../reset-password-controller.e2e.test.ts | 79 +++++++++++- .../services/reset-token-service.e2e.test.ts | 5 +- .../e2e/services/session-service.e2e.test.ts | 113 ++++++++++++++++++ .../e2e/services/user-service.e2e.test.ts | 3 + src/test/fixtures/fake-session-store.ts | 24 ++++ 18 files changed, 545 insertions(+), 76 deletions(-) create mode 100644 src/lib/db/session-store.ts create mode 100644 src/lib/services/session-service.ts create mode 100644 src/test/e2e/services/session-service.e2e.test.ts create mode 100644 src/test/fixtures/fake-session-store.ts diff --git a/src/lib/db/index.ts b/src/lib/db/index.ts index ec14cf54d9..472d27e6da 100644 --- a/src/lib/db/index.ts +++ b/src/lib/db/index.ts @@ -2,29 +2,29 @@ // eslint-disable-next-line import EventEmitter from "events"; -import { Knex } from 'knex'; -import { AccessStore } from './access-store'; -import { ResetTokenStore } from './reset-token-store'; import { IUnleashConfig } from '../types/option'; import { IUnleashStores } from '../types/stores'; -const { createDb } = require('./db-pool'); -const EventStore = require('./event-store'); -const FeatureToggleStore = require('./feature-toggle-store'); -const FeatureTypeStore = require('./feature-type-store'); -const StrategyStore = require('./strategy-store'); -const ClientInstanceStore = require('./client-instance-store'); -const ClientMetricsDb = require('./client-metrics-db'); -const ClientMetricsStore = require('./client-metrics-store'); -const ClientApplicationsStore = require('./client-applications-store'); -const ContextFieldStore = require('./context-field-store'); -const SettingStore = require('./setting-store'); -const UserStore = require('./user-store'); -const ProjectStore = require('./project-store'); -const TagStore = require('./tag-store'); -const TagTypeStore = require('./tag-type-store'); -const AddonStore = require('./addon-store'); -const { ApiTokenStore } = require('./api-token-store'); +import { createDb } from './db-pool'; +import EventStore from './event-store'; +import FeatureToggleStore from './feature-toggle-store'; +import FeatureTypeStore from './feature-type-store'; +import StrategyStore from './strategy-store'; +import ClientInstanceStore from './client-instance-store'; +import ClientMetricsDb from './client-metrics-db'; +import ClientMetricsStore from './client-metrics-store'; +import ClientApplicationsStore from './client-applications-store'; +import ContextFieldStore from './context-field-store'; +import SettingStore from './setting-store'; +import UserStore from './user-store'; +import ProjectStore from './project-store'; +import TagStore from './tag-store'; +import TagTypeStore from './tag-type-store'; +import AddonStore from './addon-store'; +import { ApiTokenStore } from './api-token-store'; +import SessionStore from './session-store'; +import { AccessStore } from './access-store'; +import { ResetTokenStore } from './reset-token-store'; export const createStores = ( config: IUnleashConfig, @@ -41,11 +41,7 @@ export const createStores = ( featureToggleStore: new FeatureToggleStore(db, eventBus, getLogger), featureTypeStore: new FeatureTypeStore(db, getLogger), strategyStore: new StrategyStore(db, getLogger), - clientApplicationsStore: new ClientApplicationsStore( - db, - eventBus, - getLogger, - ), + clientApplicationsStore: new ClientApplicationsStore(db, eventBus), clientInstanceStore: new ClientInstanceStore(db, eventBus, getLogger), clientMetricsStore: new ClientMetricsStore( clientMetricsDb, @@ -62,6 +58,7 @@ export const createStores = ( accessStore: new AccessStore(db, eventBus, getLogger), apiTokenStore: new ApiTokenStore(db, eventBus, getLogger), resetTokenStore: new ResetTokenStore(db, eventBus, getLogger), + sessionStore: new SessionStore(db, eventBus, getLogger), }; }; diff --git a/src/lib/db/session-store.ts b/src/lib/db/session-store.ts new file mode 100644 index 0000000000..e340a10c86 --- /dev/null +++ b/src/lib/db/session-store.ts @@ -0,0 +1,105 @@ +import EventEmitter from 'events'; +import { Knex } from 'knex'; +import { Logger, LogProvider } from '../logger'; +import NotFoundError from '../error/notfound-error'; + +const TABLE = 'unleash_session'; + +interface ISessionRow { + sid: string; + sess: string; + created_at: Date; + expired?: Date; +} + +export interface ISession { + sid: string; + sess: any; + createdAt: Date; + expired?: Date; +} + +export default class SessionStore { + private logger: Logger; + + private eventBus: EventEmitter; + + private db: Knex; + + constructor(db: Knex, eventBus: EventEmitter, getLogger: LogProvider) { + this.db = db; + this.eventBus = eventBus; + this.logger = getLogger('lib/db/session-store.ts'); + } + + async getActiveSessions(): Promise { + const rows = await this.db(TABLE) + .whereNull('expired') + .orWhere('expired', '>', new Date()) + .orderBy('created_at', 'desc'); + return rows.map(this.rowToSession); + } + + async getSessionsForUser(userId: number): Promise { + const rows = await this.db( + TABLE, + ).whereRaw(`(sess -> 'user' ->> 'id')::int = ?`, [userId]); + if (rows && rows.length > 0) { + return rows.map(this.rowToSession); + } + throw new NotFoundError( + `Could not find sessions for user with id ${userId}`, + ); + } + + async getSession(sid: string): Promise { + const row = await this.db(TABLE) + .where('sid', '=', sid) + .first(); + if (row) { + return this.rowToSession(row); + } + throw new NotFoundError(`Could not find session with sid ${sid}`); + } + + async deleteSessionsForUser(userId: number): Promise { + await this.db(TABLE) + .whereRaw(`(sess -> 'user' ->> 'id')::int = ?`, [userId]) + .del(); + } + + async deleteSession(sid: string): Promise { + await this.db(TABLE) + .where('sid', '=', sid) + .del(); + } + + async insertSession(data: Omit): Promise { + const row = await this.db(TABLE) + .insert({ + sid: data.sid, + sess: JSON.stringify(data.sess), + expired: data.expired || new Date(Date.now() + 86400000), + }) + .returning(['sid', 'sess', 'created_at', 'expired']); + if (row) { + return this.rowToSession(row); + } + throw new Error('Could not insert session'); + } + + async deleteAll(): Promise { + await this.db(TABLE).del(); + } + + private rowToSession(row: ISessionRow): ISession { + return { + sid: row.sid, + sess: row.sess, + createdAt: row.created_at, + expired: row.expired, + }; + } +} + +module.exports = SessionStore; diff --git a/src/lib/routes/admin-api/user-admin.ts b/src/lib/routes/admin-api/user-admin.ts index bc613c1da3..018ae1a44a 100644 --- a/src/lib/routes/admin-api/user-admin.ts +++ b/src/lib/routes/admin-api/user-admin.ts @@ -1,3 +1,4 @@ +import { Request, Response } from 'express'; import Controller from '../controller'; import { ADMIN } from '../../permissions'; import UserService from '../../services/user-service'; @@ -8,6 +9,7 @@ import { IUnleashConfig } from '../../types/option'; import { EmailService, MAIL_ACCEPTED } from '../../services/email-service'; import ResetTokenService from '../../services/reset-token-service'; import { IUnleashServices } from '../../types/services'; +import SessionService from '../../services/session-service'; const getCreatorUsernameOrPassword = req => req.user.username || req.user.email; @@ -22,6 +24,8 @@ export default class UserAdminController extends Controller { private resetTokenService: ResetTokenService; + private sessionService: SessionService; + constructor( config: IUnleashConfig, { @@ -29,12 +33,14 @@ export default class UserAdminController extends Controller { accessService, emailService, resetTokenService, + sessionService, }: Pick< IUnleashServices, | 'userService' | 'accessService' | 'emailService' | 'resetTokenService' + | 'sessionService' >, ) { super(config); @@ -43,6 +49,7 @@ export default class UserAdminController extends Controller { this.logger = config.getLogger('routes/user-controller.ts'); this.emailService = emailService; this.resetTokenService = resetTokenService; + this.sessionService = sessionService; this.get('/', this.getUsers, ADMIN); this.get('/search', this.search); @@ -52,6 +59,7 @@ export default class UserAdminController extends Controller { this.post('/:id/change-password', this.changePassword, ADMIN); this.delete('/:id', this.deleteUser, ADMIN); this.post('/reset-password', this.resetPassword); + this.get('/active-sessions', this.getActiveSessions, ADMIN); } // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types @@ -88,6 +96,15 @@ export default class UserAdminController extends Controller { } } + async getActiveSessions(req: Request, res: Response): Promise { + try { + const sessions = await this.sessionService.getActiveSessions(); + res.json(sessions); + } catch (error) { + handleErrors(res, this.logger, error); + } + } + // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types async search(req, res): Promise { const { q } = req.query; diff --git a/src/lib/routes/admin-api/user.ts b/src/lib/routes/admin-api/user.ts index 8cfbe40502..75fd155707 100644 --- a/src/lib/routes/admin-api/user.ts +++ b/src/lib/routes/admin-api/user.ts @@ -10,6 +10,7 @@ import UserService from '../../services/user-service'; import User from '../../types/user'; import { Logger } from '../../logger'; import { handleErrors } from './util'; +import SessionService from '../../services/session-service'; interface IChangeUserRequest { password: string; @@ -26,6 +27,8 @@ class UserController extends Controller { private userService: UserService; + private sessionService: SessionService; + private logger: Logger; constructor( @@ -33,15 +36,21 @@ class UserController extends Controller { { accessService, userService, - }: Pick, + sessionService, + }: Pick< + IUnleashServices, + 'accessService' | 'userService' | 'sessionService' + >, ) { super(config); this.accessService = accessService; this.userService = userService; + this.sessionService = sessionService; this.logger = config.getLogger('lib/routes/admin-api/user.ts'); this.get('/', this.getUser); this.post('/change-password', this.updateUserPass); + this.get('/my-sessions', this.mySessions); } async getUser(req: IAuthRequest, res: Response): Promise { @@ -81,6 +90,25 @@ class UserController extends Controller { res.status(401).end(); } } + + async mySessions( + req: UserRequest, + res: Response, + ): Promise { + const { user } = req; + if (user) { + try { + const sessions = await this.sessionService.getSessionsForUser( + user.id, + ); + res.json(sessions); + } catch (e) { + handleErrors(res, this.logger, e); + } + } else { + res.status(401).end(); + } + } } module.exports = UserController; diff --git a/src/lib/routes/auth/reset-password-controller.ts b/src/lib/routes/auth/reset-password-controller.ts index e0f82d3cce..173003741c 100644 --- a/src/lib/routes/auth/reset-password-controller.ts +++ b/src/lib/routes/auth/reset-password-controller.ts @@ -4,10 +4,7 @@ import UserService from '../../services/user-service'; import { Logger } from '../../logger'; import { handleErrors } from '../admin-api/util'; import { IUnleashConfig } from '../../types/option'; - -interface IServices { - userService: UserService; -} +import { IUnleashServices } from '../../types/services'; interface IValidateQuery { token: string; @@ -18,13 +15,19 @@ interface IChangePasswordBody { password: string; } +interface SessionRequest + extends Request { + session?; + user?; +} + const UNLEASH = 'Unleash'; class ResetPasswordController extends Controller { - userService: UserService; + private userService: UserService; - logger: Logger; + private logger: Logger; - constructor(config: IUnleashConfig, { userService }: IServices) { + constructor(config: IUnleashConfig, { userService }: IUnleashServices) { super(config); this.logger = config.getLogger( 'lib/routes/auth/reset-password-controller.ts', @@ -65,6 +68,7 @@ class ResetPasswordController extends Controller { const { token } = req.query; try { const user = await this.userService.getUserForToken(token); + await this.logout(req); res.status(200).json(user); } catch (e) { handleErrors(res, this.logger, e); @@ -75,6 +79,7 @@ class ResetPasswordController extends Controller { req: Request, res: Response, ): Promise { + await this.logout(req); const { token, password } = req.body; try { await this.userService.resetPassword(token, password); @@ -83,6 +88,12 @@ class ResetPasswordController extends Controller { handleErrors(res, this.logger, e); } } + + private async logout(req: SessionRequest) { + if (req.session) { + req.session.destroy(); + } + } } export default ResetPasswordController; diff --git a/src/lib/services/addon-service.js b/src/lib/services/addon-service.js index 807eb4e956..7f998ce44f 100644 --- a/src/lib/services/addon-service.js +++ b/src/lib/services/addon-service.js @@ -16,19 +16,19 @@ const MASKED_VALUE = '*****'; class AddonService { constructor( { addonStore, eventStore, featureToggleStore }, - { getLogger, unleashUrl }, + config, tagTypeService, ) { this.eventStore = eventStore; this.addonStore = addonStore; this.featureToggleStore = featureToggleStore; - this.getLogger = getLogger; - this.logger = getLogger('services/addon-service.js'); + this.getLogger = config.getLogger; + this.logger = config.getLogger('services/addon-service.js'); this.tagTypeService = tagTypeService; this.addonProviders = this.loadProviders({ - getLogger, - unleashUrl, + getLogger: config.getLogger, + unleashUrl: config.server.unleashUrl, }); this.sensitiveParams = this.loadSensitiveParams(this.addonProviders); if (addonStore) { diff --git a/src/lib/services/addon-service.test.js b/src/lib/services/addon-service.test.js index 67c5881934..8a32c15354 100644 --- a/src/lib/services/addon-service.test.js +++ b/src/lib/services/addon-service.test.js @@ -88,7 +88,11 @@ function getSetup() { const stores = store.createStores(); const tagTypeService = new TagTypeService(stores, { getLogger }); return { - addonService: new AddonService(stores, { getLogger }, tagTypeService), + addonService: new AddonService( + stores, + { getLogger, server: { unleashUrl: 'http://test' } }, + tagTypeService, + ), stores, tagTypeService, }; diff --git a/src/lib/services/index.ts b/src/lib/services/index.ts index bd9f35aa60..0dd3f46792 100644 --- a/src/lib/services/index.ts +++ b/src/lib/services/index.ts @@ -5,22 +5,23 @@ import FeatureTypeService from './feature-type-service'; import EventService from './event-service'; import HealthService from './health-service'; -const FeatureToggleService = require('./feature-toggle-service'); -const ProjectService = require('./project-service'); -const StateService = require('./state-service'); -const ClientMetricsService = require('./client-metrics'); -const TagTypeService = require('./tag-type-service'); -const TagService = require('./tag-service'); -const StrategyService = require('./strategy-service'); -const AddonService = require('./addon-service'); -const ContextService = require('./context-service'); -const VersionService = require('./version-service'); -const { EmailService } = require('./email-service'); -const { AccessService } = require('./access-service'); -const { ApiTokenService } = require('./api-token-service'); -const UserService = require('./user-service'); -const ResetTokenService = require('./reset-token-service'); -const SettingService = require('./setting-service'); +import FeatureToggleService from './feature-toggle-service'; +import ProjectService from './project-service'; +import StateService from './state-service'; +import ClientMetricsService from './client-metrics'; +import TagTypeService from './tag-type-service'; +import TagService from './tag-service'; +import StrategyService from './strategy-service'; +import AddonService from './addon-service'; +import ContextService from './context-service'; +import VersionService from './version-service'; +import { EmailService } from './email-service'; +import { AccessService } from './access-service'; +import { ApiTokenService } from './api-token-service'; +import UserService from './user-service'; +import ResetTokenService from './reset-token-service'; +import SettingService from './setting-service'; +import SessionService from './session-service'; export const createServices = ( stores: IUnleashStores, @@ -32,11 +33,7 @@ export const createServices = ( const contextService = new ContextService(stores, config); const emailService = new EmailService(config.email, config.getLogger); const eventService = new EventService(stores, config); - const featureToggleService = new FeatureToggleService( - stores, - config, - accessService, - ); + const featureToggleService = new FeatureToggleService(stores, config); const featureTypeService = new FeatureTypeService(stores, config); const projectService = new ProjectService(stores, config, accessService); const resetTokenService = new ResetTokenService(stores, config); @@ -45,10 +42,12 @@ export const createServices = ( const tagService = new TagService(stores, config); const tagTypeService = new TagTypeService(stores, config); const addonService = new AddonService(stores, config, tagTypeService); + const sessionService = new SessionService(stores, config); const userService = new UserService(stores, config, { accessService, resetTokenService, emailService, + sessionService, }); const versionService = new VersionService(stores, config); const healthService = new HealthService(stores, config); @@ -74,6 +73,7 @@ export const createServices = ( resetTokenService, eventService, settingService, + sessionService, }; }; diff --git a/src/lib/services/session-service.ts b/src/lib/services/session-service.ts new file mode 100644 index 0000000000..fab8cce7c6 --- /dev/null +++ b/src/lib/services/session-service.ts @@ -0,0 +1,47 @@ +import { IUnleashStores } from '../types/stores'; +import { IUnleashConfig } from '../types/option'; +import { Logger } from '../logger'; +import SessionStore, { ISession } from '../db/session-store'; + +export default class SessionService { + private logger: Logger; + + private sessionStore: SessionStore; + + constructor( + { sessionStore }: Pick, + { getLogger }: Pick, + ) { + this.logger = getLogger('lib/services/session-service.ts'); + this.sessionStore = sessionStore; + } + + async getActiveSessions(): Promise { + return this.sessionStore.getActiveSessions(); + } + + async getSessionsForUser(userId: number): Promise { + return this.sessionStore.getSessionsForUser(userId); + } + + async getSession(sid: string): Promise { + return this.sessionStore.getSession(sid); + } + + async deleteSessionsForUser(userId: number): Promise { + return this.sessionStore.deleteSessionsForUser(userId); + } + + async deleteSession(sid: string): Promise { + return this.sessionStore.deleteSession(sid); + } + + async insertSession({ + sid, + sess, + }: Pick): Promise { + return this.sessionStore.insertSession({ sid, sess }); + } +} + +module.exports = SessionService; diff --git a/src/lib/services/user-service.test.ts b/src/lib/services/user-service.test.ts index 886e2cb85e..20e2a9d7ba 100644 --- a/src/lib/services/user-service.test.ts +++ b/src/lib/services/user-service.test.ts @@ -8,6 +8,8 @@ import { EmailService } from './email-service'; import OwaspValidationError from '../error/owasp-validation-error'; import { IUnleashConfig } from '../types/option'; import { createTestConfig } from '../../test/config/test-config'; +import SessionService from './session-service'; +import FakeSessionStore from '../../test/fixtures/fake-session-store'; const config: IUnleashConfig = createTestConfig(); @@ -19,12 +21,15 @@ test('Should create new user', async t => { { userStore, resetTokenStore }, config, ); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService({ sessionStore }, config); const emailService = new EmailService(config.email, config.getLogger); const service = new UserService({ userStore }, config, { accessService, resetTokenService, emailService, + sessionService, }); const user = await service.createUser({ username: 'test', @@ -48,11 +53,14 @@ test('Should create default user', async t => { config, ); const emailService = new EmailService(config.email, config.getLogger); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService({ sessionStore }, config); const service = new UserService({ userStore }, config, { accessService, resetTokenService, emailService, + sessionService, }); await service.initAdminUser(); @@ -71,11 +79,14 @@ test('Should be a valid password', async t => { ); const emailService = new EmailService(config.email, config.getLogger); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService({ sessionStore }, config); const service = new UserService({ userStore }, config, { accessService, resetTokenService, emailService, + sessionService, }); const valid = service.validatePassword('this is a strong password!'); @@ -92,11 +103,14 @@ test('Password must be at least 10 chars', async t => { config, ); const emailService = new EmailService(config.email, config.getLogger); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService({ sessionStore }, config); const service = new UserService({ userStore }, config, { accessService, resetTokenService, emailService, + sessionService, }); t.throws(() => service.validatePassword('admin'), { @@ -114,11 +128,14 @@ test('The password must contain at least one uppercase letter.', async t => { config, ); const emailService = new EmailService(config.email, config.getLogger); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService({ sessionStore }, config); const service = new UserService({ userStore }, config, { accessService, resetTokenService, emailService, + sessionService, }); t.throws(() => service.validatePassword('qwertyabcde'), { @@ -137,10 +154,14 @@ test('The password must contain at least one number', async t => { ); const emailService = new EmailService(config.email, config.getLogger); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService({ sessionStore }, config); + const service = new UserService({ userStore }, config, { accessService, resetTokenService, emailService, + sessionService, }); t.throws(() => service.validatePassword('qwertyabcdE'), { @@ -158,11 +179,14 @@ test('The password must contain at least one special character', async t => { config, ); const emailService = new EmailService(config.email, config.getLogger); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService({ sessionStore }, config); const service = new UserService({ userStore }, config, { accessService, resetTokenService, emailService, + sessionService, }); t.throws(() => service.validatePassword('qwertyabcdE2'), { @@ -180,11 +204,14 @@ test('Should be a valid password with special chars', async t => { config, ); const emailService = new EmailService(config.email, config.getLogger); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService({ sessionStore }, config); const service = new UserService({ userStore }, config, { accessService, resetTokenService, emailService, + sessionService, }); const valid = service.validatePassword('this is a strong password!'); diff --git a/src/lib/services/user-service.ts b/src/lib/services/user-service.ts index c566f1366d..63fd394d5e 100644 --- a/src/lib/services/user-service.ts +++ b/src/lib/services/user-service.ts @@ -15,6 +15,9 @@ import NotFoundError from '../error/notfound-error'; import OwaspValidationError from '../error/owasp-validation-error'; import { EmailService } from './email-service'; import { IUnleashConfig } from '../types/option'; +import SessionService from './session-service'; +import { IUnleashServices } from '../types/services'; +import { IUnleashStores } from '../types/stores'; export interface ICreateUser { name?: string; @@ -45,16 +48,6 @@ interface ITokenUser extends IUpdateUser { role: IRoleDescription; } -interface IStores { - userStore: UserStore; -} - -interface IServices { - accessService: AccessService; - resetTokenService: ResetTokenService; - emailService: EmailService; -} - const saltRounds = 10; class UserService { @@ -66,21 +59,35 @@ class UserService { private resetTokenService: ResetTokenService; + private sessionService: SessionService; + private emailService: EmailService; constructor( - stores: IStores, + stores: Pick, { getLogger, authentication, }: Pick, - { accessService, resetTokenService, emailService }: IServices, + { + accessService, + resetTokenService, + emailService, + sessionService, + }: Pick< + IUnleashServices, + | 'accessService' + | 'resetTokenService' + | 'emailService' + | 'sessionService' + >, ) { this.logger = getLogger('service/user-service.js'); this.store = stores.userStore; this.accessService = accessService; this.resetTokenService = resetTokenService; this.emailService = emailService; + this.sessionService = sessionService; if (authentication && authentication.createAdminUser) { process.nextTick(() => this.initAdminUser()); } @@ -288,6 +295,11 @@ class UserService { }; } + /** + * If the password is a strong password will update password and delete all sessions for the user we're changing the password for + * @param token - the token authenticating this request + * @param password - new password + */ async resetPassword(token: string, password: string): Promise { this.validatePassword(password); const user = await this.getUserForToken(token); @@ -297,6 +309,7 @@ class UserService { }); if (allowed) { await this.changePassword(user.id, password); + await this.sessionService.deleteSessionsForUser(user.id); } else { throw new InvalidTokenError(); } diff --git a/src/lib/types/services.ts b/src/lib/types/services.ts index 6f6b0ad337..2bb0dc8584 100644 --- a/src/lib/types/services.ts +++ b/src/lib/types/services.ts @@ -17,6 +17,7 @@ import FeatureTypeService from '../services/feature-type-service'; import EventService from '../services/event-service'; import HealthService from '../services/health-service'; import SettingService from '../services/setting-service'; +import SessionService from '../services/session-service'; export interface IUnleashServices { accessService: AccessService; @@ -31,6 +32,7 @@ export interface IUnleashServices { healthService: HealthService; projectService: ProjectService; resetTokenService: ResetTokenService; + sessionService: SessionService; settingService: SettingService; stateService: StateService; strategyService: StrategyService; diff --git a/src/lib/types/stores.ts b/src/lib/types/stores.ts index c426586831..e593a59305 100644 --- a/src/lib/types/stores.ts +++ b/src/lib/types/stores.ts @@ -16,6 +16,7 @@ import AddonStore from '../db/addon-store'; import { AccessStore } from '../db/access-store'; import { ApiTokenStore } from '../db/api-token-store'; import { ResetTokenStore } from '../db/reset-token-store'; +import SessionStore from '../db/session-store'; export interface IUnleashStores { projectStore: ProjectStore; @@ -28,6 +29,7 @@ export interface IUnleashStores { featureToggleStore: FeatureToggleStore; contextFieldStore: ContextFieldStore; settingStore: SettingStore; + sessionStore: SessionStore; userStore: UserStore; tagStore: TagStore; tagTypeStore: TagTypeStore; diff --git a/src/test/e2e/api/auth/reset-password-controller.e2e.test.ts b/src/test/e2e/api/auth/reset-password-controller.e2e.test.ts index af91f3e327..2be5f42a53 100644 --- a/src/test/e2e/api/auth/reset-password-controller.e2e.test.ts +++ b/src/test/e2e/api/auth/reset-password-controller.e2e.test.ts @@ -1,5 +1,7 @@ import test from 'ava'; +import sinon from 'sinon'; import { URL } from 'url'; +import EventEmitter from 'events'; import dbInit from '../../helpers/database-init'; import getLogger from '../../../fixtures/no-logger'; @@ -9,11 +11,13 @@ import { } from '../../../../lib/services/access-service'; import ResetTokenService from '../../../../lib/services/reset-token-service'; import UserService from '../../../../lib/services/user-service'; -import { setupApp } from '../../helpers/test-helper'; +import { setupApp, setupAppWithAuth } from '../../helpers/test-helper'; import { EmailService } from '../../../../lib/services/email-service'; import User from '../../../../lib/types/user'; import { IUnleashConfig } from '../../../../lib/types/option'; import { createTestConfig } from '../../../config/test-config'; +import SessionStore from '../../../../lib/db/session-store'; +import SessionService from '../../../../lib/services/session-service'; let stores; let db; @@ -46,11 +50,17 @@ test.before(async () => { stores = db.stores; accessService = new AccessService(stores, config); const emailService = new EmailService(config.email, config.getLogger); - + const sessionStore = new SessionStore( + db, + new EventEmitter(), + config.getLogger, + ); + const sessionService = new SessionService({ sessionStore }, config); userService = new UserService(stores, config, { accessService, resetTokenService, emailService, + sessionService, }); resetTokenService = new ResetTokenService(stores, config); const adminRole = await accessService.getRootRole(RoleName.ADMIN); @@ -99,7 +109,7 @@ test.serial('Can use token to reset password', async t => { ); const relative = getBackendResetUrl(url); // Can't login before reset - t.throwsAsync( + await t.throwsAsync( async () => userService.loginUser(user.email, password), { instanceOf: Error, @@ -171,6 +181,69 @@ test.serial('Invalid token should yield 401', async t => { }); }); +test.serial( + 'Calling validate endpoint with already existing session should destroy session', + async t => { + t.plan(0); + const request = await setupAppWithAuth(stores); + await request + .post('/api/admin/login') + .send({ + email: 'user@mail.com', + }) + .expect(200); + await request.get('/api/admin/features').expect(200); + const url = await resetTokenService.createResetPasswordUrl( + user.id, + adminUser.username, + ); + const relative = getBackendResetUrl(url); + + await request + .get(relative) + .expect(200) + .expect('Content-Type', /json/); + await request.get('/api/admin/features').expect(401); // we no longer should have a valid session + }, +); + +test.serial( + 'Calling reset endpoint with already existing session should logout/destroy existing session', + async t => { + t.plan(0); + const request = await setupAppWithAuth(stores); + const url = await resetTokenService.createResetPasswordUrl( + user.id, + adminUser.username, + ); + const relative = getBackendResetUrl(url); + let token; + await request + .get(relative) + .expect(200) + .expect('Content-Type', /json/) + .expect(res => { + token = res.body.token; + }); + await request + .post('/api/admin/login') + .send({ + email: 'user@mail.com', + }) + .expect(200); + await request.get('/api/admin/features').expect(200); // If we login we can access features endpoint + await request + .post('/auth/reset/password') + .send({ + email: user.email, + token, + password, + }) + .expect(200); + await request.get('/api/admin/features').expect(401); // we no longer have a valid session after using the reset password endpoint + }, +); + test.serial( 'Trying to change password with an invalid token should yield 401', async t => { diff --git a/src/test/e2e/services/reset-token-service.e2e.test.ts b/src/test/e2e/services/reset-token-service.e2e.test.ts index 2571475ef3..09501076d5 100644 --- a/src/test/e2e/services/reset-token-service.e2e.test.ts +++ b/src/test/e2e/services/reset-token-service.e2e.test.ts @@ -9,6 +9,7 @@ import { EmailService } from '../../../lib/services/email-service'; import User from '../../../lib/types/user'; import { IUnleashConfig } from '../../../lib/types/option'; import { createTestConfig } from '../../config/test-config'; +import SessionService from '../../../lib/services/session-service'; const config: IUnleashConfig = createTestConfig(); @@ -20,18 +21,20 @@ let userIdToCreateResetFor: number; let accessService: AccessService; let userService: UserService; let resetTokenService: ResetTokenService; +let sessionService: SessionService; test.before(async () => { db = await dbInit('reset_token_service_serial', getLogger); stores = db.stores; accessService = new AccessService(stores, config); resetTokenService = new ResetTokenService(stores, config); - + sessionService = new SessionService(stores, config); const emailService = new EmailService(undefined, config.getLogger); userService = new UserService(stores, config, { accessService, resetTokenService, emailService, + sessionService, }); adminUser = await userService.createUser({ diff --git a/src/test/e2e/services/session-service.e2e.test.ts b/src/test/e2e/services/session-service.e2e.test.ts new file mode 100644 index 0000000000..5f624e7818 --- /dev/null +++ b/src/test/e2e/services/session-service.e2e.test.ts @@ -0,0 +1,113 @@ +import test, { after, before, beforeEach } from 'ava'; +import noLoggerProvider from '../../fixtures/no-logger'; +import dbInit from '../helpers/database-init'; +import SessionService from '../../../lib/services/session-service'; +import NotFoundError from '../../../lib/error/notfound-error'; + +let stores; +let db; +let sessionService; +const newSession = { + sid: 'abc123', + sess: { + cookie: { + originalMaxAge: 2880000, + expires: new Date(Date.now() + 86_400_000).toDateString(), + secure: false, + httpOnly: true, + path: '/', + }, + user: { + id: 1, + username: 'admin', + imageUrl: + 'https://gravatar.com/avatar/21232f297a57a5a743894a0e4a801fc3?size=42&default=retro', + seenAt: '2021-04-26T10:59:18.782Z', + loginAttempts: 0, + createdAt: '2021-04-22T05:12:54.368Z', + }, + }, +}; +const otherSession = { + sid: 'xyz321', + sess: { + cookie: { + originalMaxAge: 2880000, + expires: new Date(Date.now() + 86400000).toDateString(), + secure: false, + httpOnly: true, + path: '/', + }, + user: { + id: 2, + username: 'editor', + imageUrl: + 'https://gravatar.com/avatar/21232f297a57a5a743894a0e4a801fc3?size=42&default=retro', + seenAt: '2021-04-26T10:59:18.782Z', + loginAttempts: 0, + createdAt: '2021-04-22T05:12:54.368Z', + }, + }, +}; +before(async () => { + db = await dbInit('session_service_serial', noLoggerProvider); + stores = db.stores; + sessionService = new SessionService(stores, { + getLogger: noLoggerProvider, + }); +}); + +beforeEach(async () => { + await db.stores.sessionStore.deleteAll(); +}); + +after.always(async () => { + await db.destroy(); +}); + +test.serial('should list active sessions', async t => { + await sessionService.insertSession(newSession); + await sessionService.insertSession(otherSession); + + const sessions = await sessionService.getActiveSessions(); + t.is(sessions.length, 2); + t.deepEqual(sessions[0].sess, otherSession.sess); // Ordered newest first + t.deepEqual(sessions[1].sess, newSession.sess); +}); + +test.serial('Should list active sessions for user', async t => { + await sessionService.insertSession(newSession); + await sessionService.insertSession(otherSession); + + const sessions = await sessionService.getSessionsForUser(2); // editor session + t.is(sessions.length, 1); + t.deepEqual(sessions[0].sess, otherSession.sess); +}); + +test.serial('Can delete sessions by user', async t => { + await sessionService.insertSession(newSession); + await sessionService.insertSession(otherSession); + + const sessions = await sessionService.getActiveSessions(); + t.is(sessions.length, 2); + await sessionService.deleteSessionsForUser(2); + await t.throwsAsync( + async () => { + await sessionService.getSessionsForUser(2); + }, + { instanceOf: NotFoundError }, + ); +}); + +test.serial('Can delete session by sid', async t => { + await sessionService.insertSession(newSession); + await sessionService.insertSession(otherSession); + + const sessions = await sessionService.getActiveSessions(); + t.is(sessions.length, 2); + + await sessionService.deleteSession('abc123'); + await t.throwsAsync(async () => sessionService.getSession('abc123'), { + instanceOf: NotFoundError, + }); +}); diff --git a/src/test/e2e/services/user-service.e2e.test.ts b/src/test/e2e/services/user-service.e2e.test.ts index 1d40f03706..908ac4ffa5 100644 --- a/src/test/e2e/services/user-service.e2e.test.ts +++ b/src/test/e2e/services/user-service.e2e.test.ts @@ -9,6 +9,7 @@ import { IRole } from '../../../lib/db/access-store'; import ResetTokenService from '../../../lib/services/reset-token-service'; import { EmailService } from '../../../lib/services/email-service'; import { createTestConfig } from '../../config/test-config'; +import SessionService from '../../../lib/services/session-service'; let db; let stores; @@ -23,11 +24,13 @@ test.before(async () => { const accessService = new AccessService(stores, config); const resetTokenService = new ResetTokenService(stores, config); const emailService = new EmailService(undefined, config.getLogger); + const sessionService = new SessionService(stores, config); userService = new UserService(stores, config, { accessService, resetTokenService, emailService, + sessionService, }); userStore = stores.userStore; const rootRoles = await accessService.getRootRoles(); diff --git a/src/test/fixtures/fake-session-store.ts b/src/test/fixtures/fake-session-store.ts new file mode 100644 index 0000000000..79abf56142 --- /dev/null +++ b/src/test/fixtures/fake-session-store.ts @@ -0,0 +1,24 @@ +import SessionStore, { ISession } from '../../lib/db/session-store'; +import noLoggerProvider from './no-logger'; + +export default class FakeSessionStore extends SessionStore { + private sessions: ISession[] = []; + + constructor() { + super(undefined, undefined, noLoggerProvider); + } + + async getActiveSessions(): Promise { + return this.sessions.filter(session => session.expired != null); + } + + async getSessionsForUser(userId: number): Promise { + return this.sessions.filter(session => session.sess.user.id === userId); + } + + async deleteSessionsForUser(userId: number): Promise { + this.sessions = this.sessions.filter( + session => session.sess.user.id !== userId, + ); + } +}