From 1d41f5de9b6c040e44ff753f0665aca19a5ba196 Mon Sep 17 00:00:00 2001 From: Nicolas Meienberger Date: Mon, 28 Aug 2023 21:44:53 +0200 Subject: [PATCH] refactor: export TipiCache as a non-instantiated class --- __mocks__/redis.ts | 5 +-- .../utils/__tests__/page-helpers.test.ts | 10 ++++-- src/client/utils/page-helpers.ts | 6 ++-- src/pages/api/certificate.ts | 8 +++-- src/server/common/session.helpers.ts | 10 ++++-- src/server/context.ts | 7 +++-- src/server/core/TipiCache/TipiCache.ts | 4 +-- src/server/core/TipiCache/index.ts | 2 +- src/server/routers/auth/auth.router.ts | 2 +- src/server/services/auth/auth.service.test.ts | 31 ++++++++++--------- src/server/services/auth/auth.service.ts | 19 +++++++----- .../services/system/system.service.test.ts | 21 +++++++------ src/server/services/system/system.service.ts | 4 +-- 13 files changed, 78 insertions(+), 51 deletions(-) diff --git a/__mocks__/redis.ts b/__mocks__/redis.ts index b8586df6..5b54149f 100644 --- a/__mocks__/redis.ts +++ b/__mocks__/redis.ts @@ -1,6 +1,7 @@ +const values = new Map(); +const expirations = new Map(); + export const createClient = jest.fn(() => { - const values = new Map(); - const expirations = new Map(); return { isOpen: true, connect: jest.fn(), diff --git a/src/client/utils/__tests__/page-helpers.test.ts b/src/client/utils/__tests__/page-helpers.test.ts index 669076e3..45f7efb2 100644 --- a/src/client/utils/__tests__/page-helpers.test.ts +++ b/src/client/utils/__tests__/page-helpers.test.ts @@ -1,11 +1,17 @@ import merge from 'lodash.merge'; import { deleteCookie, setCookie } from 'cookies-next'; import { fromPartial } from '@total-typescript/shoehorn'; -import TipiCache from '@/server/core/TipiCache/TipiCache'; +import { TipiCache } from '@/server/core/TipiCache'; import { getAuthedPageProps, getMessagesPageProps } from '../page-helpers'; import englishMessages from '../../messages/en.json'; import frenchMessages from '../../messages/fr-FR.json'; +const cache = new TipiCache(); + +afterAll(async () => { + await cache.close(); +}); + describe('test: getAuthedPageProps()', () => { it('should redirect to /login if there is no user id in session', async () => { // arrange @@ -23,7 +29,7 @@ describe('test: getAuthedPageProps()', () => { it('should return props if there is a user id in session', async () => { // arrange const ctx = { req: { headers: { 'x-session-id': '123' } } }; - await TipiCache.set('session:123', '456'); + await cache.set('session:123', '456'); // act // @ts-expect-error - we're passing in a partial context diff --git a/src/client/utils/page-helpers.ts b/src/client/utils/page-helpers.ts index 99594d31..fe5e32f4 100644 --- a/src/client/utils/page-helpers.ts +++ b/src/client/utils/page-helpers.ts @@ -2,11 +2,13 @@ import { GetServerSideProps } from 'next'; import merge from 'lodash.merge'; import { getLocaleFromString } from '@/shared/internationalization/locales'; import { getCookie } from 'cookies-next'; -import TipiCache from '@/server/core/TipiCache/TipiCache'; +import { TipiCache } from '@/server/core/TipiCache'; export const getAuthedPageProps: GetServerSideProps = async (ctx) => { + const cache = new TipiCache(); const sessionId = ctx.req.headers['x-session-id']; - const userId = await TipiCache.get(`session:${sessionId}`); + const userId = await cache.get(`session:${sessionId}`); + await cache.close(); if (!userId) { return { diff --git a/src/pages/api/certificate.ts b/src/pages/api/certificate.ts index fb49721a..bc89517d 100644 --- a/src/pages/api/certificate.ts +++ b/src/pages/api/certificate.ts @@ -1,5 +1,5 @@ import { getConfig } from '@/server/core/TipiConfig/TipiConfig'; -import TipiCache from '@/server/core/TipiCache/TipiCache'; +import { TipiCache } from '@/server/core/TipiCache/TipiCache'; import { AuthQueries } from '@/server/queries/auth/auth.queries'; import { db } from '@/server/db'; @@ -13,12 +13,16 @@ import fs from 'fs-extra'; * @param {NextApiResponse} res - The response */ export default async function handler(req: NextApiRequest, res: NextApiResponse) { + const cache = new TipiCache(); + const authService = new AuthQueries(db); const sessionId = req.headers['x-session-id']; - const userId = await TipiCache.get(`session:${sessionId}`); + const userId = await cache.get(`session:${sessionId}`); const user = await authService.getUserById(Number(userId)); + await cache.close(); + if (user?.operator) { const filePath = `${getConfig().rootFolder}/traefik/tls/cert.pem`; diff --git a/src/server/common/session.helpers.ts b/src/server/common/session.helpers.ts index 59000cd2..132d4220 100644 --- a/src/server/common/session.helpers.ts +++ b/src/server/common/session.helpers.ts @@ -1,7 +1,7 @@ import { setCookie } from 'cookies-next'; import { NextApiRequest, NextApiResponse } from 'next'; import { v4 } from 'uuid'; -import TipiCache from '../core/TipiCache/TipiCache'; +import { TipiCache } from '../core/TipiCache/TipiCache'; const COOKIE_MAX_AGE = 60 * 60 * 24; // 1 day const COOKIE_NAME = 'tipi.sid'; @@ -11,10 +11,14 @@ export const generateSessionId = (prefix: string) => { }; export const setSession = async (sessionId: string, userId: string, req: NextApiRequest, res: NextApiResponse) => { + const cache = new TipiCache(); + setCookie(COOKIE_NAME, sessionId, { req, res, maxAge: COOKIE_MAX_AGE, httpOnly: true, secure: true, sameSite: false }); const sessionKey = `session:${sessionId}`; - await TipiCache.set(sessionKey, userId); - await TipiCache.set(`session:${userId}:${sessionId}`, sessionKey); + await cache.set(sessionKey, userId); + await cache.set(`session:${userId}:${sessionId}`, sessionKey); + + await cache.close(); }; diff --git a/src/server/context.ts b/src/server/context.ts index 37c5738a..7f54f905 100644 --- a/src/server/context.ts +++ b/src/server/context.ts @@ -1,6 +1,6 @@ import { inferAsyncReturnType } from '@trpc/server'; import { CreateNextContextOptions } from '@trpc/server/adapters/next'; -import TipiCache from './core/TipiCache/TipiCache'; +import { TipiCache } from './core/TipiCache/TipiCache'; type CreateContextOptions = { req: CreateNextContextOptions['req']; @@ -27,11 +27,14 @@ const createContextInner = async (opts: CreateContextOptions) => ({ * @param {CreateNextContextOptions} opts - options */ export const createContext = async (opts: CreateNextContextOptions) => { + const cache = new TipiCache(); const { req, res } = opts; const sessionId = req.headers['x-session-id'] as string; - const userId = await TipiCache.get(`session:${sessionId}`); + const userId = await cache.get(`session:${sessionId}`); + + await cache.close(); return createContextInner({ req, diff --git a/src/server/core/TipiCache/TipiCache.ts b/src/server/core/TipiCache/TipiCache.ts index 1111ffae..1b98e521 100644 --- a/src/server/core/TipiCache/TipiCache.ts +++ b/src/server/core/TipiCache/TipiCache.ts @@ -4,7 +4,7 @@ import { getConfig } from '../TipiConfig'; const ONE_DAY_IN_SECONDS = 60 * 60 * 24; -class TipiCache { +export class TipiCache { private static instance: TipiCache; private client: RedisClientType; @@ -78,5 +78,3 @@ class TipiCache { return client.ttl(key); } } - -export default TipiCache.getInstance(); diff --git a/src/server/core/TipiCache/index.ts b/src/server/core/TipiCache/index.ts index 9447c7bf..45a114a9 100644 --- a/src/server/core/TipiCache/index.ts +++ b/src/server/core/TipiCache/index.ts @@ -1 +1 @@ -export { default } from './TipiCache'; +export { TipiCache } from './TipiCache'; diff --git a/src/server/routers/auth/auth.router.ts b/src/server/routers/auth/auth.router.ts index 09375baa..03c86c63 100644 --- a/src/server/routers/auth/auth.router.ts +++ b/src/server/routers/auth/auth.router.ts @@ -7,7 +7,7 @@ const AuthService = new AuthServiceClass(db); export const authRouter = router({ login: publicProcedure.input(z.object({ username: z.string(), password: z.string() })).mutation(async ({ input, ctx }) => AuthService.login({ ...input }, ctx.req, ctx.res)), - logout: protectedProcedure.mutation(async ({ ctx }) => AuthServiceClass.logout(ctx.sessionId)), + logout: protectedProcedure.mutation(async ({ ctx }) => AuthService.logout(ctx.sessionId)), register: publicProcedure .input(z.object({ username: z.string(), password: z.string(), locale: z.string() })) .mutation(async ({ input, ctx }) => AuthService.register({ ...input }, ctx.req, ctx.res)), diff --git a/src/server/services/auth/auth.service.test.ts b/src/server/services/auth/auth.service.test.ts index 817b3f87..72032e6c 100644 --- a/src/server/services/auth/auth.service.test.ts +++ b/src/server/services/auth/auth.service.test.ts @@ -11,12 +11,14 @@ import { encrypt } from '../../utils/encryption'; import { setConfig } from '../../core/TipiConfig'; import { createUser, getUserByEmail, getUserById } from '../../tests/user.factory'; import { AuthServiceClass } from './auth.service'; -import TipiCache from '../../core/TipiCache'; +import { TipiCache } from '../../core/TipiCache'; let AuthService: AuthServiceClass; let database: TestDatabase; const TEST_SUITE = 'authservice'; +const cache = new TipiCache(); + beforeAll(async () => { setConfig('jwtSecret', 'test'); database = await createDatabase(TEST_SUITE); @@ -30,6 +32,7 @@ beforeEach(async () => { afterAll(async () => { await closeDatabase(database); + await cache.close(); }); describe('Login', () => { @@ -51,7 +54,7 @@ describe('Login', () => { const sessionId = session.split(';')[0]?.split('=')[1]; const sessionKey = `session:${sessionId}`; - const userId = await TipiCache.get(sessionKey); + const userId = await cache.get(sessionKey); // assert expect(userId).toBeDefined(); @@ -105,12 +108,12 @@ describe('Test: verifyTotp', () => { const totpSessionId = generateSessionId('otp'); const otp = TotpAuthenticator.generate(totpSecret); - await TipiCache.set(totpSessionId, user.id.toString()); + await cache.set(totpSessionId, user.id.toString()); // act const result = await AuthService.verifyTotp({ totpSessionId, totpCode: otp }, fromPartial({}), fromPartial(res)); const sessionId = session.split(';')[0]?.split('=')[1]; - const userId = await TipiCache.get(`session:${sessionId}`); + const userId = await cache.get(`session:${sessionId}`); // assert expect(result).toBeTruthy(); @@ -128,7 +131,7 @@ describe('Test: verifyTotp', () => { const encryptedTotpSecret = encrypt(totpSecret, salt); const user = await createUser({ email, totpEnabled: true, totpSecret: encryptedTotpSecret, salt }, database); const totpSessionId = generateSessionId('otp'); - await TipiCache.set(totpSessionId, user.id.toString()); + await cache.set(totpSessionId, user.id.toString()); // act & assert await expect(AuthService.verifyTotp({ totpSessionId, totpCode: 'wrong' }, fromPartial({}), fromPartial({}))).rejects.toThrowError('server-messages.errors.totp-invalid-code'); @@ -144,7 +147,7 @@ describe('Test: verifyTotp', () => { const totpSessionId = generateSessionId('otp'); const otp = TotpAuthenticator.generate(totpSecret); - await TipiCache.set(totpSessionId, user.id.toString()); + await cache.set(totpSessionId, user.id.toString()); // act & assert await expect(AuthService.verifyTotp({ totpSessionId: 'wrong', totpCode: otp }, fromPartial({}), fromPartial({}))).rejects.toThrowError('server-messages.errors.totp-session-not-found'); @@ -153,7 +156,7 @@ describe('Test: verifyTotp', () => { it('should throw if the user does not exist', async () => { // arrange const totpSessionId = generateSessionId('otp'); - await TipiCache.set(totpSessionId, '1234'); + await cache.set(totpSessionId, '1234'); // act & assert await expect(AuthService.verifyTotp({ totpSessionId, totpCode: '1234' }, fromPartial({}), fromPartial({}))).rejects.toThrowError('server-messages.errors.user-not-found'); @@ -169,7 +172,7 @@ describe('Test: verifyTotp', () => { const totpSessionId = generateSessionId('otp'); const otp = TotpAuthenticator.generate(totpSecret); - await TipiCache.set(totpSessionId, user.id.toString()); + await cache.set(totpSessionId, user.id.toString()); // act & assert await expect(AuthService.verifyTotp({ totpSessionId, totpCode: otp }, fromPartial({}), fromPartial({}))).rejects.toThrowError('server-messages.errors.totp-not-enabled'); @@ -477,7 +480,7 @@ describe('Register', () => { describe('Test: logout', () => { it('Should return true if there is no session to delete', async () => { // act - const result = await AuthServiceClass.logout('session'); + const result = await AuthService.logout('session'); // assert expect(result).toBe(true); @@ -487,11 +490,11 @@ describe('Test: logout', () => { // arrange const sessionId = v4(); - await TipiCache.set(`session:${sessionId}`, '1'); + await cache.set(`session:${sessionId}`, '1'); // act - const result = await AuthServiceClass.logout(sessionId); - const session = await TipiCache.get(`session:${sessionId}`); + const result = await AuthService.logout(sessionId); + const session = await cache.get(`session:${sessionId}`); // assert expect(result).toBe(true); @@ -715,14 +718,14 @@ describe('Test: changePassword', () => { const email = faker.internet.email(); const user = await createUser({ email }, database); const newPassword = faker.internet.password(); - await TipiCache.set(`session:${user.id}:${faker.lorem.word()}`, 'test'); + await cache.set(`session:${user.id}:${faker.lorem.word()}`, 'test'); // act await AuthService.changePassword({ userId: user.id, newPassword, currentPassword: 'password' }); // assert // eslint-disable-next-line testing-library/no-await-sync-query - const sessions = await TipiCache.getByPrefix(`session:${user.id}:`); + const sessions = await cache.getByPrefix(`session:${user.id}:`); expect(sessions).toHaveLength(0); }); }); diff --git a/src/server/services/auth/auth.service.ts b/src/server/services/auth/auth.service.ts index 733200ad..209d0258 100644 --- a/src/server/services/auth/auth.service.ts +++ b/src/server/services/auth/auth.service.ts @@ -9,7 +9,7 @@ import { generateSessionId, setSession } from '@/server/common/session.helpers'; import { Database } from '@/server/db'; import { NextApiRequest, NextApiResponse } from 'next'; import { getConfig } from '../../core/TipiConfig'; -import TipiCache from '../../core/TipiCache'; +import { TipiCache } from '../../core/TipiCache'; import { fileExists, unlinkFile } from '../../common/fs.helpers'; import { decrypt, encrypt } from '../../utils/encryption'; @@ -22,8 +22,11 @@ type UsernamePasswordInput = { export class AuthServiceClass { private queries; + private cache; + constructor(p: Database) { this.queries = new AuthQueries(p); + this.cache = new TipiCache(); } /** @@ -49,7 +52,7 @@ export class AuthServiceClass { if (user.totpEnabled) { const totpSessionId = generateSessionId('otp'); - await TipiCache.set(totpSessionId, user.id.toString()); + await this.cache.set(totpSessionId, user.id.toString()); return { totpSessionId }; } @@ -70,7 +73,7 @@ export class AuthServiceClass { */ public verifyTotp = async (params: { totpSessionId: string; totpCode: string }, req: NextApiRequest, res: NextApiResponse) => { const { totpSessionId, totpCode } = params; - const userId = await TipiCache.get(totpSessionId); + const userId = await this.cache.get(totpSessionId); if (!userId) { throw new TranslatedError('server-messages.errors.totp-session-not-found'); @@ -261,8 +264,8 @@ export class AuthServiceClass { * @param {string} sessionId - The session token to remove * @returns {Promise} - Returns true if the session token is removed successfully */ - public static logout = async (sessionId: string): Promise => { - await TipiCache.del(`session:${sessionId}`); + public logout = async (sessionId: string): Promise => { + await this.cache.del(`session:${sessionId}`); return true; }; @@ -341,12 +344,12 @@ export class AuthServiceClass { * @param {number} userId - The user ID */ private destroyAllSessionsByUserId = async (userId: number) => { - const sessions = await TipiCache.getByPrefix(`session:${userId}:`); + const sessions = await this.cache.getByPrefix(`session:${userId}:`); await Promise.all( sessions.map(async (session) => { - await TipiCache.del(session.key); - if (session.val) await TipiCache.del(session.val); + await this.cache.del(session.key); + if (session.val) await this.cache.del(session.val); }), ); }; diff --git a/src/server/services/system/system.service.test.ts b/src/server/services/system/system.service.test.ts index c6a57693..3bcec3d0 100644 --- a/src/server/services/system/system.service.test.ts +++ b/src/server/services/system/system.service.test.ts @@ -5,7 +5,7 @@ import semver from 'semver'; import { faker } from '@faker-js/faker'; import { EventDispatcher } from '../../core/EventDispatcher'; import { setConfig } from '../../core/TipiConfig'; -import TipiCache from '../../core/TipiCache'; +import { TipiCache } from '../../core/TipiCache'; import { SystemServiceClass } from '.'; jest.mock('redis'); @@ -14,6 +14,8 @@ const SystemService = new SystemServiceClass(); const server = setupServer(); +const cache = new TipiCache(); + beforeEach(async () => { await setConfig('demoMode', false); @@ -71,14 +73,15 @@ describe('Test: getVersion', () => { server.listen(); }); - beforeEach(() => { + beforeEach(async () => { server.resetHandlers(); - TipiCache.del('latestVersion'); + await cache.del('latestVersion'); }); - afterAll(() => { + afterAll(async () => { server.close(); jest.restoreAllMocks(); + await cache.close(); }); it('It should return version with body', async () => { @@ -163,7 +166,7 @@ describe('Test: update', () => { // Arrange EventDispatcher.dispatchEventAsync = jest.fn().mockResolvedValueOnce({ success: true }); setConfig('version', '0.0.1'); - TipiCache.set('latestVersion', '0.0.2'); + await cache.set('latestVersion', '0.0.2'); // Act const update = await SystemService.update(); @@ -174,7 +177,7 @@ describe('Test: update', () => { it('Should throw an error if latest version is not set', async () => { // Arrange - TipiCache.del('latestVersion'); + await cache.del('latestVersion'); server.use( rest.get('https://api.github.com/repos/meienberger/runtipi/releases/latest', (_, res, ctx) => { return res(ctx.json({ name: null })); @@ -189,7 +192,7 @@ describe('Test: update', () => { it('Should throw if current version is higher than latest', async () => { // Arrange setConfig('version', '0.0.2'); - TipiCache.set('latestVersion', '0.0.1'); + await cache.set('latestVersion', '0.0.1'); // Act & Assert await expect(SystemService.update()).rejects.toThrow('server-messages.errors.current-version-is-latest'); @@ -198,7 +201,7 @@ describe('Test: update', () => { it('Should throw if current version is equal to latest', async () => { // Arrange setConfig('version', '0.0.1'); - TipiCache.set('latestVersion', '0.0.1'); + await cache.set('latestVersion', '0.0.1'); // Act & Assert await expect(SystemService.update()).rejects.toThrow('server-messages.errors.current-version-is-latest'); @@ -207,7 +210,7 @@ describe('Test: update', () => { it('Should throw an error if there is a major version difference', async () => { // Arrange setConfig('version', '0.0.1'); - TipiCache.set('latestVersion', '1.0.0'); + await cache.set('latestVersion', '1.0.0'); // Act & Assert await expect(SystemService.update()).rejects.toThrow('server-messages.errors.major-version-update'); diff --git a/src/server/services/system/system.service.ts b/src/server/services/system/system.service.ts index 6b118be5..7663b9b7 100644 --- a/src/server/services/system/system.service.ts +++ b/src/server/services/system/system.service.ts @@ -5,7 +5,7 @@ import { TranslatedError } from '@/server/utils/errors'; import { readJsonFile } from '../../common/fs.helpers'; import { EventDispatcher } from '../../core/EventDispatcher'; import { Logger } from '../../core/Logger'; -import TipiCache from '../../core/TipiCache'; +import { TipiCache } from '../../core/TipiCache'; import * as TipiConfig from '../../core/TipiConfig'; const SYSTEM_STATUS = ['UPDATING', 'RESTARTING', 'RUNNING'] as const; @@ -33,7 +33,7 @@ export class SystemServiceClass { private dispatcher; constructor() { - this.cache = TipiCache; + this.cache = new TipiCache(); this.dispatcher = EventDispatcher; }