diff --git a/src/lib/routes/index.ts b/src/lib/routes/index.ts index 71b98693d4..4b4e345365 100644 --- a/src/lib/routes/index.ts +++ b/src/lib/routes/index.ts @@ -18,7 +18,7 @@ class IndexRouter extends Controller { this.use('/health', new HealthCheckController(config, services).router); this.use('/internal-backstage', new BackstageController(config).router); - this.use('/logout', new LogoutController(config).router); + this.use('/logout', new LogoutController(config, services).router); this.use( '/auth/simple', new SimplePasswordProvider(config, services).router, diff --git a/src/lib/routes/logout.test.ts b/src/lib/routes/logout.test.ts index 8756676117..e2e464532e 100644 --- a/src/lib/routes/logout.test.ts +++ b/src/lib/routes/logout.test.ts @@ -4,12 +4,26 @@ import { createTestConfig } from '../../test/config/test-config'; import LogoutController from './logout'; import { IAuthRequest } from './unleash-types'; +import SessionService from '../services/session-service'; +import FakeSessionStore from '../../test/fixtures/fake-session-store'; +import noLogger from '../../test/fixtures/no-logger'; +import { addDays } from 'date-fns'; test('should redirect to "/" after logout', async () => { const baseUriPath = ''; const app = express(); const config = createTestConfig({ server: { baseUriPath } }); - app.use('/logout', new LogoutController(config).router); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService( + { sessionStore }, + { getLogger: noLogger }, + ); + app.use( + '/logout', + new LogoutController(config, { + sessionService, + }).router, + ); const request = supertest(app); expect.assertions(0); await request @@ -22,7 +36,12 @@ test('should redirect to "/basePath" after logout when baseUriPath is set', asyn const baseUriPath = '/basePath'; const app = express(); const config = createTestConfig({ server: { baseUriPath } }); - app.use('/logout', new LogoutController(config).router); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService( + { sessionStore }, + { getLogger: noLogger }, + ); + app.use('/logout', new LogoutController(config, { sessionService }).router); const request = supertest(app); expect.assertions(0); await request @@ -35,7 +54,13 @@ test('should set "Clear-Site-Data" header', async () => { const baseUriPath = ''; const app = express(); const config = createTestConfig({ server: { baseUriPath } }); - app.use('/logout', new LogoutController(config).router); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService( + { sessionStore }, + { getLogger: noLogger }, + ); + + app.use('/logout', new LogoutController(config, { sessionService }).router); const request = supertest(app); expect.assertions(0); await request @@ -51,7 +76,13 @@ test('should not set "Clear-Site-Data" header', async () => { server: { baseUriPath }, session: { clearSiteDataOnLogout: false }, }); - app.use('/logout', new LogoutController(config).router); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService( + { sessionStore }, + { getLogger: noLogger }, + ); + + app.use('/logout', new LogoutController(config, { sessionService }).router); const request = supertest(app); expect.assertions(1); await request @@ -66,7 +97,14 @@ test('should clear "unleash-session" cookies', async () => { const baseUriPath = ''; const app = express(); const config = createTestConfig({ server: { baseUriPath } }); - app.use('/logout', new LogoutController(config).router); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService( + { sessionStore }, + { getLogger: noLogger }, + ); + + app.use('/logout', new LogoutController(config, { sessionService }).router); + const request = supertest(app); expect.assertions(0); await request @@ -85,7 +123,14 @@ test('should clear "unleash-session" cookie even when disabled clear site data', server: { baseUriPath }, session: { clearSiteDataOnLogout: false }, }); - app.use('/logout', new LogoutController(config).router); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService( + { sessionStore }, + { getLogger: noLogger }, + ); + + app.use('/logout', new LogoutController(config, { sessionService }).router); + const request = supertest(app); expect.assertions(0); await request @@ -108,7 +153,14 @@ test('should call destroy on session', async () => { req.session = fakeSession; next(); }); - app.use('/logout', new LogoutController(config).router); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService( + { sessionStore }, + { getLogger: noLogger }, + ); + + app.use('/logout', new LogoutController(config, { sessionService }).router); + const request = supertest(app); await request.get(`${baseUriPath}/logout`); @@ -125,7 +177,14 @@ test('should handle req.logout with callback function', async () => { req.logout = logoutFunction; next(); }); - app.use('/logout', new LogoutController(config).router); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService( + { sessionStore }, + { getLogger: noLogger }, + ); + + app.use('/logout', new LogoutController(config, { sessionService }).router); + const request = supertest(app); await request.get(`${baseUriPath}/logout`); @@ -143,7 +202,14 @@ test('should handle req.logout without callback function', async () => { req.logout = logoutFunction; next(); }); - app.use('/logout', new LogoutController(config).router); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService( + { sessionStore }, + { getLogger: noLogger }, + ); + + app.use('/logout', new LogoutController(config, { sessionService }).router); + const request = supertest(app); await request.get(`${baseUriPath}/logout`); @@ -162,10 +228,61 @@ test('should redirect to alternative logoutUrl', async () => { req.session = fakeSession; next(); }); - app.use('/logout', new LogoutController(config).router); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService( + { sessionStore }, + { getLogger: noLogger }, + ); + + app.use('/logout', new LogoutController(config, { sessionService }).router); + const request = supertest(app); await request .get('/logout') .expect(302) .expect('Location', '/some-other-path'); }); + +test('Should destroy sessions for user', async () => { + const app = express(); + const config = createTestConfig(); + const fakeSession = { + destroy: jest.fn(), + user: { + id: 1, + }, + }; + app.use((req: IAuthRequest, res, next) => { + req.session = fakeSession; + next(); + }); + const sessionStore = new FakeSessionStore(); + const sessionService = new SessionService( + { sessionStore }, + { getLogger: noLogger }, + ); + await sessionStore.insertSession({ + sid: '1', + sess: { + user: { + id: 1, + }, + }, + expired: addDays(new Date(), 2), + }); + await sessionStore.insertSession({ + sid: '2', + sess: { + user: { + id: 1, + }, + }, + expired: addDays(new Date(), 2), + }); + let activeSessionsBeforeLogout = await sessionStore.getSessionsForUser(1); + expect(activeSessionsBeforeLogout).toHaveLength(2); + app.use('/logout', new LogoutController(config, { sessionService }).router); + await supertest(app).get('/logout').expect(302); + let activeSessions = await sessionStore.getSessionsForUser(1); + expect(activeSessions).toHaveLength(0); +}); diff --git a/src/lib/routes/logout.ts b/src/lib/routes/logout.ts index cdf5388ca0..98a597c9af 100644 --- a/src/lib/routes/logout.ts +++ b/src/lib/routes/logout.ts @@ -3,6 +3,8 @@ import { promisify } from 'util'; import { IUnleashConfig } from '../types/option'; import Controller from './controller'; import { IAuthRequest } from './unleash-types'; +import { IUnleashServices } from '../types'; +import SessionService from '../services/session-service'; class LogoutController extends Controller { private clearSiteDataOnLogout: boolean; @@ -11,8 +13,14 @@ class LogoutController extends Controller { private baseUri: string; - constructor(config: IUnleashConfig) { + private sessionService: SessionService; + + constructor( + config: IUnleashConfig, + { sessionService }: Pick, + ) { super(config); + this.sessionService = sessionService; this.baseUri = config.server.baseUriPath; this.clearSiteDataOnLogout = config.session.clearSiteDataOnLogout; this.cookieName = config.session.cookieName; @@ -41,6 +49,11 @@ class LogoutController extends Controller { } if (req.session) { + if (req.session.user?.id) { + await this.sessionService.deleteSessionsForUser( + req.session.user.id, + ); + } req.session.destroy(); } res.clearCookie(this.cookieName); @@ -48,7 +61,9 @@ class LogoutController extends Controller { if (this.clearSiteDataOnLogout) { res.set('Clear-Site-Data', '"cookies", "storage"'); } - + if (req.user?.id) { + await this.sessionService.deleteSessionsForUser(req.user.id); + } res.redirect(`${this.baseUri}/`); } diff --git a/src/lib/services/user-service.ts b/src/lib/services/user-service.ts index 8534099d4b..9d76707c7d 100644 --- a/src/lib/services/user-service.ts +++ b/src/lib/services/user-service.ts @@ -340,14 +340,15 @@ class UserService { throw e; } } - this.store.successfullyLogin(user); + await this.store.successfullyLogin(user); return user; } async changePassword(userId: number, password: string): Promise { this.validatePassword(password); const passwordHash = await bcrypt.hash(password, saltRounds); - return this.store.setPasswordHash(userId, passwordHash); + await this.store.setPasswordHash(userId, passwordHash); + await this.sessionService.deleteSessionsForUser(userId); } async getUserForToken(token: string): Promise { diff --git a/src/test/e2e/api/admin/user-admin.e2e.test.ts b/src/test/e2e/api/admin/user-admin.e2e.test.ts index 948db2d757..224555f552 100644 --- a/src/test/e2e/api/admin/user-admin.e2e.test.ts +++ b/src/test/e2e/api/admin/user-admin.e2e.test.ts @@ -13,6 +13,7 @@ import { RoleName } from '../../../../lib/types/model'; import { IRoleStore } from 'lib/types/stores/role-store'; import { randomId } from '../../../../lib/util/random-id'; import { omitKeys } from '../../../../lib/util/omit-keys'; +import { ISessionStore } from '../../../../lib/types/stores/session-store'; let stores; let db; @@ -21,6 +22,7 @@ let app; let userStore: IUserStore; let eventStore: IEventStore; let roleStore: IRoleStore; +let sessionStore: ISessionStore; let editorRole: IRole; let adminRole: IRole; @@ -32,6 +34,7 @@ beforeAll(async () => { userStore = stores.userStore; eventStore = stores.eventStore; roleStore = stores.roleStore; + sessionStore = stores.sessionStore; const roles = await roleStore.getRootRoles(); editorRole = roles.find((r) => r.name === RoleName.EDITOR); adminRole = roles.find((r) => r.name === RoleName.ADMIN); @@ -231,11 +234,12 @@ test('validator should accept strong password', async () => { test('should change password', async () => { const user = await userStore.insert({ email: 'some@mail.com' }); - - return app.request + const spy = jest.spyOn(sessionStore, 'deleteSessionsForUser'); + await app.request .post(`/api/admin/user-admin/${user.id}/change-password`) .send({ password: 'simple123-_ASsad' }) .expect(200); + expect(spy).toHaveBeenCalled(); }); test('should search for users', async () => {