From 25a875cbbc0f0f04d3e786da1263e6f3e5bc3b79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20S=C3=B3jko?= Date: Wed, 11 Oct 2023 10:12:25 +0200 Subject: [PATCH] fix(auth): reduce session select queries in favor of insert/update model --- .../EphemeralSessionRepositoryInterface.ts | 3 +- .../RevokedSessionRepositoryInterface.ts | 3 +- .../Session/SessionRepositoryInterface.ts | 3 +- .../src/Domain/Session/SessionService.spec.ts | 37 ++++++++++--------- .../auth/src/Domain/Session/SessionService.ts | 16 +++++--- .../Redis/RedisEphemeralSessionRepository.ts | 8 +++- .../TypeORMEphemeralSessionRepository.ts | 6 ++- .../TypeORMRevokedSessionRepository.ts | 10 ++++- .../Infra/TypeORM/TypeORMSessionRepository.ts | 12 +++++- 9 files changed, 65 insertions(+), 33 deletions(-) diff --git a/packages/auth/src/Domain/Session/EphemeralSessionRepositoryInterface.ts b/packages/auth/src/Domain/Session/EphemeralSessionRepositoryInterface.ts index 770615b96..276160cec 100644 --- a/packages/auth/src/Domain/Session/EphemeralSessionRepositoryInterface.ts +++ b/packages/auth/src/Domain/Session/EphemeralSessionRepositoryInterface.ts @@ -5,5 +5,6 @@ export interface EphemeralSessionRepositoryInterface { findOneByUuidAndUserUuid(uuid: string, userUuid: string): Promise findAllByUserUuid(userUuid: string): Promise> deleteOne(uuid: string, userUuid: string): Promise - save(ephemeralSession: EphemeralSession): Promise + insert(ephemeralSession: EphemeralSession): Promise + update(ephemeralSession: EphemeralSession): Promise } diff --git a/packages/auth/src/Domain/Session/RevokedSessionRepositoryInterface.ts b/packages/auth/src/Domain/Session/RevokedSessionRepositoryInterface.ts index b73d848cb..9277bb373 100644 --- a/packages/auth/src/Domain/Session/RevokedSessionRepositoryInterface.ts +++ b/packages/auth/src/Domain/Session/RevokedSessionRepositoryInterface.ts @@ -3,7 +3,8 @@ import { RevokedSession } from './RevokedSession' export interface RevokedSessionRepositoryInterface { findOneByUuid(uuid: string): Promise findAllByUserUuid(userUuid: string): Promise> - save(revokedSession: RevokedSession): Promise + insert(revokedSession: RevokedSession): Promise + update(revokedSession: RevokedSession): Promise remove(revokedSession: RevokedSession): Promise clearUserAgentByUserUuid(userUuid: string): Promise } diff --git a/packages/auth/src/Domain/Session/SessionRepositoryInterface.ts b/packages/auth/src/Domain/Session/SessionRepositoryInterface.ts index 08fa32434..a75402668 100644 --- a/packages/auth/src/Domain/Session/SessionRepositoryInterface.ts +++ b/packages/auth/src/Domain/Session/SessionRepositoryInterface.ts @@ -9,7 +9,8 @@ export interface SessionRepositoryInterface { findAllByUserUuid(userUuid: string): Promise> deleteAllByUserUuidExceptOne(dto: { userUuid: Uuid; currentSessionUuid: Uuid }): Promise deleteOneByUuid(uuid: string): Promise - save(session: Session): Promise + insert(session: Session): Promise + update(session: Session): Promise remove(session: Session): Promise clearUserAgentByUserUuid(userUuid: string): Promise removeExpiredBefore(date: Date): Promise diff --git a/packages/auth/src/Domain/Session/SessionService.spec.ts b/packages/auth/src/Domain/Session/SessionService.spec.ts index d25e650fa..d97aaf629 100644 --- a/packages/auth/src/Domain/Session/SessionService.spec.ts +++ b/packages/auth/src/Domain/Session/SessionService.spec.ts @@ -69,18 +69,21 @@ describe('SessionService', () => { sessionRepository = {} as jest.Mocked sessionRepository.findOneByUuid = jest.fn().mockReturnValue(null) sessionRepository.deleteOneByUuid = jest.fn() - sessionRepository.save = jest.fn().mockReturnValue(existingSession) + sessionRepository.insert = jest.fn() + sessionRepository.update = jest.fn() settingService = {} as jest.Mocked settingService.findSettingWithDecryptedValue = jest.fn().mockReturnValue(null) ephemeralSessionRepository = {} as jest.Mocked - ephemeralSessionRepository.save = jest.fn() + ephemeralSessionRepository.insert = jest.fn() + ephemeralSessionRepository.update = jest.fn() ephemeralSessionRepository.findOneByUuid = jest.fn() ephemeralSessionRepository.deleteOne = jest.fn() revokedSessionRepository = {} as jest.Mocked - revokedSessionRepository.save = jest.fn() + revokedSessionRepository.insert = jest.fn() + revokedSessionRepository.update = jest.fn() existingEphemeralSession = {} as jest.Mocked existingEphemeralSession.uuid = '2-3-4' @@ -129,7 +132,7 @@ describe('SessionService', () => { it('should mark a revoked session as received', async () => { await createService().markRevokedSessionAsReceived(revokedSession) - expect(revokedSessionRepository.save).toHaveBeenCalledWith({ + expect(revokedSessionRepository.update).toHaveBeenCalledWith({ uuid: '2e1e43', received: true, receivedAt: new Date(1), @@ -145,8 +148,8 @@ describe('SessionService', () => { readonly_access: false, }) - expect(sessionRepository.save).toHaveBeenCalled() - expect(ephemeralSessionRepository.save).not.toHaveBeenCalled() + expect(sessionRepository.update).toHaveBeenCalled() + expect(ephemeralSessionRepository.update).not.toHaveBeenCalled() }) it('should refresh access and refresh tokens for an ephemeral session', async () => { @@ -158,8 +161,8 @@ describe('SessionService', () => { readonly_access: false, }) - expect(sessionRepository.save).not.toHaveBeenCalled() - expect(ephemeralSessionRepository.save).toHaveBeenCalled() + expect(sessionRepository.update).not.toHaveBeenCalled() + expect(ephemeralSessionRepository.update).toHaveBeenCalled() }) it('should create new session for a user', async () => { @@ -173,8 +176,8 @@ describe('SessionService', () => { readonlyAccess: false, }) - expect(sessionRepository.save).toHaveBeenCalledWith(expect.any(Session)) - expect(sessionRepository.save).toHaveBeenCalledWith({ + expect(sessionRepository.insert).toHaveBeenCalledWith(expect.any(Session)) + expect(sessionRepository.insert).toHaveBeenCalledWith({ accessExpiration: expect.any(Date), apiVersion: '003', createdAt: expect.any(Date), @@ -209,8 +212,8 @@ describe('SessionService', () => { readonlyAccess: false, }) - expect(sessionRepository.save).toHaveBeenCalledWith(expect.any(Session)) - expect(sessionRepository.save).toHaveBeenCalledWith({ + expect(sessionRepository.insert).toHaveBeenCalledWith(expect.any(Session)) + expect(sessionRepository.insert).toHaveBeenCalledWith({ accessExpiration: expect.any(Date), apiVersion: '003', createdAt: expect.any(Date), @@ -248,8 +251,8 @@ describe('SessionService', () => { readonlyAccess: false, }) - expect(sessionRepository.save).toHaveBeenCalledWith(expect.any(Session)) - expect(sessionRepository.save).toHaveBeenCalledWith({ + expect(sessionRepository.insert).toHaveBeenCalledWith(expect.any(Session)) + expect(sessionRepository.insert).toHaveBeenCalledWith({ accessExpiration: expect.any(Date), apiVersion: '003', createdAt: expect.any(Date), @@ -405,8 +408,8 @@ describe('SessionService', () => { readonlyAccess: false, }) - expect(ephemeralSessionRepository.save).toHaveBeenCalledWith(expect.any(EphemeralSession)) - expect(ephemeralSessionRepository.save).toHaveBeenCalledWith({ + expect(ephemeralSessionRepository.insert).toHaveBeenCalledWith(expect.any(EphemeralSession)) + expect(ephemeralSessionRepository.insert).toHaveBeenCalledWith({ accessExpiration: expect.any(Date), apiVersion: '003', createdAt: expect.any(Date), @@ -684,7 +687,7 @@ describe('SessionService', () => { it('should revoked a session', async () => { await createService().createRevokedSession(existingSession) - expect(revokedSessionRepository.save).toHaveBeenCalledWith({ + expect(revokedSessionRepository.insert).toHaveBeenCalledWith({ uuid: '2e1e43', userUuid: '1-2-3', userAgent: 'Chrome', diff --git a/packages/auth/src/Domain/Session/SessionService.ts b/packages/auth/src/Domain/Session/SessionService.ts index f96729ff5..afad94c73 100644 --- a/packages/auth/src/Domain/Session/SessionService.ts +++ b/packages/auth/src/Domain/Session/SessionService.ts @@ -57,7 +57,7 @@ export class SessionService implements SessionServiceInterface { const sessionPayload = await this.createTokens(session) - await this.sessionRepository.save(session) + await this.sessionRepository.insert(session) try { const userSubscription = await this.userSubscriptionRepository.findOneByUserUuid(dto.user.uuid) @@ -92,7 +92,7 @@ export class SessionService implements SessionServiceInterface { const sessionPayload = await this.createTokens(ephemeralSession) - await this.ephemeralSessionRepository.save(ephemeralSession) + await this.ephemeralSessionRepository.insert(ephemeralSession) return { sessionHttpRepresentation: sessionPayload, @@ -104,9 +104,9 @@ export class SessionService implements SessionServiceInterface { const sessionPayload = await this.createTokens(dto.session) if (dto.isEphemeral) { - await this.ephemeralSessionRepository.save(dto.session) + await this.ephemeralSessionRepository.update(dto.session) } else { - await this.sessionRepository.save(dto.session) + await this.sessionRepository.update(dto.session) } return sessionPayload @@ -221,7 +221,9 @@ export class SessionService implements SessionServiceInterface { revokedSession.received = true revokedSession.receivedAt = this.timer.getUTCDate() - return this.revokedSessionRepository.save(revokedSession) + await this.revokedSessionRepository.update(revokedSession) + + return revokedSession } async deleteSessionByToken(token: string): Promise { @@ -248,7 +250,9 @@ export class SessionService implements SessionServiceInterface { revokedSession.apiVersion = session.apiVersion revokedSession.userAgent = session.userAgent - return this.revokedSessionRepository.save(revokedSession) + await this.revokedSessionRepository.insert(revokedSession) + + return revokedSession } private async createSession(dto: { diff --git a/packages/auth/src/Infra/Redis/RedisEphemeralSessionRepository.ts b/packages/auth/src/Infra/Redis/RedisEphemeralSessionRepository.ts index 22ea0176c..e6451b27e 100644 --- a/packages/auth/src/Infra/Redis/RedisEphemeralSessionRepository.ts +++ b/packages/auth/src/Infra/Redis/RedisEphemeralSessionRepository.ts @@ -42,7 +42,7 @@ export class RedisEphemeralSessionRepository implements EphemeralSessionReposito session.accessExpiration = accessExpiration session.refreshExpiration = refreshExpiration - await this.save(session) + await this.update(session) } async findAllByUserUuid(userUuid: string): Promise> { @@ -77,7 +77,7 @@ export class RedisEphemeralSessionRepository implements EphemeralSessionReposito return JSON.parse(stringifiedSession) } - async save(ephemeralSession: EphemeralSession): Promise { + async insert(ephemeralSession: EphemeralSession): Promise { const ttl = this.ephemeralSessionAge const stringifiedSession = JSON.stringify(ephemeralSession) @@ -92,4 +92,8 @@ export class RedisEphemeralSessionRepository implements EphemeralSessionReposito await pipeline.exec() } + + async update(ephemeralSession: EphemeralSession): Promise { + return this.insert(ephemeralSession) + } } diff --git a/packages/auth/src/Infra/TypeORM/TypeORMEphemeralSessionRepository.ts b/packages/auth/src/Infra/TypeORM/TypeORMEphemeralSessionRepository.ts index 18ecfa776..fbf99c9bb 100644 --- a/packages/auth/src/Infra/TypeORM/TypeORMEphemeralSessionRepository.ts +++ b/packages/auth/src/Infra/TypeORM/TypeORMEphemeralSessionRepository.ts @@ -71,7 +71,11 @@ export class TypeORMEphemeralSessionRepository implements EphemeralSessionReposi return JSON.parse(stringifiedSession.props.value) } - async save(ephemeralSession: EphemeralSession): Promise { + async update(ephemeralSession: EphemeralSession): Promise { + return this.insert(ephemeralSession) + } + + async insert(ephemeralSession: EphemeralSession): Promise { const ttl = this.ephemeralSessionAge ephemeralSession.updatedAt = this.timer.getUTCDate() diff --git a/packages/auth/src/Infra/TypeORM/TypeORMRevokedSessionRepository.ts b/packages/auth/src/Infra/TypeORM/TypeORMRevokedSessionRepository.ts index 44eec6b41..4e30177d2 100644 --- a/packages/auth/src/Infra/TypeORM/TypeORMRevokedSessionRepository.ts +++ b/packages/auth/src/Infra/TypeORM/TypeORMRevokedSessionRepository.ts @@ -12,8 +12,14 @@ export class TypeORMRevokedSessionRepository implements RevokedSessionRepository private ormRepository: Repository, ) {} - async save(revokedSession: RevokedSession): Promise { - return this.ormRepository.save(revokedSession) + async insert(revokedSession: RevokedSession): Promise { + await this.ormRepository.insert(revokedSession) + } + + async update(revokedSession: RevokedSession): Promise { + const { uuid, ...revokedSessionProps } = revokedSession + + await this.ormRepository.update({ uuid }, revokedSessionProps) } async remove(revokedSession: RevokedSession): Promise { diff --git a/packages/auth/src/Infra/TypeORM/TypeORMSessionRepository.ts b/packages/auth/src/Infra/TypeORM/TypeORMSessionRepository.ts index d4cbb300d..7ddcf4a48 100644 --- a/packages/auth/src/Infra/TypeORM/TypeORMSessionRepository.ts +++ b/packages/auth/src/Infra/TypeORM/TypeORMSessionRepository.ts @@ -17,10 +17,18 @@ export class TypeORMSessionRepository implements SessionRepositoryInterface { @inject(TYPES.Auth_Timer) private timer: TimerInterface, ) {} - async save(session: Session): Promise { + async insert(session: Session): Promise { session.updatedAt = this.timer.getUTCDate() - return this.ormRepository.save(session) + await this.ormRepository.insert(session) + } + + async update(session: Session): Promise { + session.updatedAt = this.timer.getUTCDate() + + const { uuid, ...sessionProps } = session + + await this.ormRepository.update({ uuid }, sessionProps) } async remove(session: Session): Promise {