From b7635c89445887761c021e354e6ebc9f2356af2d Mon Sep 17 00:00:00 2001 From: fengmk2 Date: Tue, 14 Jan 2025 07:43:26 +0000 Subject: [PATCH] refactor(server): use session model in auth service (#9660) --- .../server/src/__tests__/auth/guard.spec.ts | 30 ++-- .../server/src/__tests__/auth/job.spec.ts | 47 ++++++ .../server/src/__tests__/auth/service.spec.ts | 6 +- .../backend/server/src/core/auth/index.ts | 2 + packages/backend/server/src/core/auth/job.ts | 14 ++ .../backend/server/src/core/auth/service.ts | 159 +++--------------- .../backend/server/src/core/auth/session.ts | 6 +- packages/backend/server/src/models/index.ts | 2 + packages/backend/server/src/models/user.ts | 2 +- 9 files changed, 116 insertions(+), 152 deletions(-) create mode 100644 packages/backend/server/src/__tests__/auth/job.spec.ts create mode 100644 packages/backend/server/src/core/auth/job.ts diff --git a/packages/backend/server/src/__tests__/auth/guard.spec.ts b/packages/backend/server/src/__tests__/auth/guard.spec.ts index f9bd66944e..57f96d3d9f 100644 --- a/packages/backend/server/src/__tests__/auth/guard.spec.ts +++ b/packages/backend/server/src/__tests__/auth/guard.spec.ts @@ -6,6 +6,7 @@ import request from 'supertest'; import { AuthModule, CurrentUser, Public, Session } from '../../core/auth'; import { AuthService } from '../../core/auth/service'; +import { Models } from '../../models'; import { createTestingApp } from '../utils'; @Controller('/') @@ -35,6 +36,8 @@ let server!: any; let auth!: AuthService; let u1!: CurrentUser; +let sessionId = ''; + test.before(async t => { const { app } = await createTestingApp({ imports: [AuthModule], @@ -44,13 +47,10 @@ test.before(async t => { auth = app.get(AuthService); u1 = await auth.signUp('u1@affine.pro', '1'); - const db = app.get(PrismaClient); - await db.session.create({ - data: { - id: '1', - }, - }); - await auth.createUserSession(u1.id, '1'); + const models = app.get(Models); + const session = await models.session.createSession(); + sessionId = session.id; + await auth.createUserSession(u1.id, sessionId); server = app.getHttpServer(); t.context.app = app; @@ -69,7 +69,7 @@ test('should be able to visit public api if not signed in', async t => { test('should be able to visit public api if signed in', async t => { const res = await request(server) .get('/public') - .set('Cookie', `${AuthService.sessionCookieName}=1`) + .set('Cookie', `${AuthService.sessionCookieName}=${sessionId}`) .expect(HttpStatus.OK); t.is(res.body.user.id, u1.id); @@ -90,7 +90,7 @@ test('should not be able to visit private api if not signed in', async t => { test('should be able to visit private api if signed in', async t => { const res = await request(server) .get('/private') - .set('Cookie', `${AuthService.sessionCookieName}=1`) + .set('Cookie', `${AuthService.sessionCookieName}=${sessionId}`) .expect(HttpStatus.OK); t.is(res.body.user.id, u1.id); @@ -100,10 +100,10 @@ test('should be able to parse session cookie', async t => { const spy = Sinon.spy(auth, 'getUserSession'); await request(server) .get('/public') - .set('cookie', `${AuthService.sessionCookieName}=1`) + .set('cookie', `${AuthService.sessionCookieName}=${sessionId}`) .expect(200); - t.deepEqual(spy.firstCall.args, ['1', undefined]); + t.deepEqual(spy.firstCall.args, [sessionId, undefined]); spy.restore(); }); @@ -112,17 +112,17 @@ test('should be able to parse bearer token', async t => { await request(server) .get('/public') - .auth('1', { type: 'bearer' }) + .auth(sessionId, { type: 'bearer' }) .expect(200); - t.deepEqual(spy.firstCall.args, ['1', undefined]); + t.deepEqual(spy.firstCall.args, [sessionId, undefined]); spy.restore(); }); test('should be able to refresh session if needed', async t => { await t.context.app.get(PrismaClient).userSession.updateMany({ where: { - sessionId: '1', + sessionId, }, data: { expiresAt: new Date(Date.now() + 1000 * 60 * 60 /* expires in 1 hour */), @@ -131,7 +131,7 @@ test('should be able to refresh session if needed', async t => { const res = await request(server) .get('/session') - .set('cookie', `${AuthService.sessionCookieName}=1`) + .set('cookie', `${AuthService.sessionCookieName}=${sessionId}`) .expect(200); const cookie = res diff --git a/packages/backend/server/src/__tests__/auth/job.spec.ts b/packages/backend/server/src/__tests__/auth/job.spec.ts new file mode 100644 index 0000000000..930b2b5e7b --- /dev/null +++ b/packages/backend/server/src/__tests__/auth/job.spec.ts @@ -0,0 +1,47 @@ +import { ScheduleModule } from '@nestjs/schedule'; +import { TestingModule } from '@nestjs/testing'; +import { PrismaClient } from '@prisma/client'; +import test from 'ava'; + +import { AuthModule, AuthService } from '../../core/auth'; +import { AuthCronJob } from '../../core/auth/job'; +import { createTestingModule } from '../utils'; + +let m: TestingModule; +let db: PrismaClient; + +test.before(async () => { + m = await createTestingModule({ + imports: [ScheduleModule.forRoot(), AuthModule], + }); + + db = m.get(PrismaClient); +}); + +test.after.always(async () => { + await m.close(); +}); + +test('should clean expired user sessions', async t => { + const auth = m.get(AuthService); + const job = m.get(AuthCronJob); + const user1 = await auth.signUp('u1@affine.pro', '1'); + const user2 = await auth.signUp('u2@affine.pro', '1'); + await auth.createUserSession(user1.id); + await auth.createUserSession(user2.id); + let userSessions = await db.userSession.findMany(); + t.is(userSessions.length, 2); + + // no expired sessions + await job.cleanExpiredUserSessions(); + userSessions = await db.userSession.findMany(); + t.is(userSessions.length, 2); + + // clean all expired sessions + await db.userSession.updateMany({ + data: { expiresAt: new Date(Date.now() - 1000) }, + }); + await job.cleanExpiredUserSessions(); + userSessions = await db.userSession.findMany(); + t.is(userSessions.length, 0); +}); diff --git a/packages/backend/server/src/__tests__/auth/service.spec.ts b/packages/backend/server/src/__tests__/auth/service.spec.ts index 6dc96c80f0..cb6f903ff5 100644 --- a/packages/backend/server/src/__tests__/auth/service.spec.ts +++ b/packages/backend/server/src/__tests__/auth/service.spec.ts @@ -192,8 +192,10 @@ test('should be able to signout multi accounts session', async t => { const session = await auth.createSession(); - await auth.createUserSession(u1.id, session.id); - await auth.createUserSession(u2.id, session.id); + const userSession1 = await auth.createUserSession(u1.id, session.id); + const userSession2 = await auth.createUserSession(u2.id, session.id); + t.not(userSession1.id, userSession2.id); + t.is(userSession1.sessionId, userSession2.sessionId); await auth.signOut(session.id, u1.id); diff --git a/packages/backend/server/src/core/auth/index.ts b/packages/backend/server/src/core/auth/index.ts index d85355295d..94a0e45019 100644 --- a/packages/backend/server/src/core/auth/index.ts +++ b/packages/backend/server/src/core/auth/index.ts @@ -7,6 +7,7 @@ import { QuotaModule } from '../quota'; import { UserModule } from '../user'; import { AuthController } from './controller'; import { AuthGuard, AuthWebsocketOptionsProvider } from './guard'; +import { AuthCronJob } from './job'; import { AuthResolver } from './resolver'; import { AuthService } from './service'; @@ -16,6 +17,7 @@ import { AuthService } from './service'; AuthService, AuthResolver, AuthGuard, + AuthCronJob, AuthWebsocketOptionsProvider, ], exports: [AuthService, AuthGuard, AuthWebsocketOptionsProvider], diff --git a/packages/backend/server/src/core/auth/job.ts b/packages/backend/server/src/core/auth/job.ts new file mode 100644 index 0000000000..1e59279dd5 --- /dev/null +++ b/packages/backend/server/src/core/auth/job.ts @@ -0,0 +1,14 @@ +import { Injectable } from '@nestjs/common'; +import { Cron, CronExpression } from '@nestjs/schedule'; + +import { Models } from '../../models'; + +@Injectable() +export class AuthCronJob { + constructor(private readonly models: Models) {} + + @Cron(CronExpression.EVERY_DAY_AT_MIDNIGHT) + async cleanExpiredUserSessions() { + await this.models.session.cleanExpiredUserSessions(); + } +} diff --git a/packages/backend/server/src/core/auth/service.ts b/packages/backend/server/src/core/auth/service.ts index f0bec9e2f8..de53a620d8 100644 --- a/packages/backend/server/src/core/auth/service.ts +++ b/packages/backend/server/src/core/auth/service.ts @@ -1,11 +1,9 @@ import { Injectable, OnApplicationBootstrap } from '@nestjs/common'; -import { Cron, CronExpression } from '@nestjs/schedule'; -import type { User, UserSession } from '@prisma/client'; -import { PrismaClient } from '@prisma/client'; import type { CookieOptions, Request, Response } from 'express'; import { assign, pick } from 'lodash-es'; import { Config, MailService, SignUpForbidden } from '../../base'; +import { Models, type User, type UserSession } from '../../models'; import { FeatureManagementService } from '../features/management'; import { QuotaService } from '../quota/service'; import { QuotaType } from '../quota/types'; @@ -46,7 +44,7 @@ export class AuthService implements OnApplicationBootstrap { constructor( private readonly config: Config, - private readonly db: PrismaClient, + private readonly models: Models, private readonly mailer: MailService, private readonly feature: FeatureManagementService, private readonly quota: QuotaService, @@ -103,18 +101,9 @@ export class AuthService implements OnApplicationBootstrap { async signOut(sessionId: string, userId?: string) { // sign out all users in the session if (!userId) { - await this.db.session.deleteMany({ - where: { - id: sessionId, - }, - }); + await this.models.session.deleteSession(sessionId); } else { - await this.db.userSession.deleteMany({ - where: { - sessionId, - userId, - }, - }); + await this.models.session.deleteUserSession(userId, sessionId); } } @@ -138,7 +127,7 @@ export class AuthService implements OnApplicationBootstrap { // fallback to the first valid session if user provided userId is invalid if (!userSession) { // checked - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + // oxlint-disable-next-line @typescript-eslint/no-non-null-assertion userSession = sessions.at(-1)!; } @@ -152,127 +141,50 @@ export class AuthService implements OnApplicationBootstrap { } async getUserSessions(sessionId: string) { - return this.db.userSession.findMany({ - where: { - sessionId, - OR: [{ expiresAt: { gt: new Date() } }, { expiresAt: null }], - }, - orderBy: { - createdAt: 'asc', - }, - }); + return await this.models.session.findUserSessionsBySessionId(sessionId); } - async createUserSession( - userId: string, - sessionId?: string, - ttl = this.config.auth.session.ttl - ) { - // check whether given session is valid - if (sessionId) { - const session = await this.db.session.findFirst({ - where: { - id: sessionId, - }, - }); - - if (!session) { - sessionId = undefined; - } - } - - if (!sessionId) { - const session = await this.createSession(); - sessionId = session.id; - } - - const expiresAt = new Date(Date.now() + ttl * 1000); - - return this.db.userSession.upsert({ - where: { - sessionId_userId: { - sessionId, - userId, - }, - }, - update: { - expiresAt, - }, - create: { - sessionId, - userId, - expiresAt, - }, - }); + async createUserSession(userId: string, sessionId?: string, ttl?: number) { + return await this.models.session.createOrRefreshUserSession( + userId, + sessionId, + ttl + ); } async getUserList(sessionId: string) { - const sessions = await this.db.userSession.findMany({ - where: { - sessionId, - OR: [ - { - expiresAt: null, - }, - { - expiresAt: { - gt: new Date(), - }, - }, - ], - }, - include: { + const sessions = await this.models.session.findUserSessionsBySessionId( + sessionId, + { user: true, - }, - orderBy: { - createdAt: 'asc', - }, - }); - + } + ); return sessions.map(({ user }) => sessionUser(user)); } async createSession() { - return this.db.session.create({ - data: {}, - }); + return await this.models.session.createSession(); } async getSession(sessionId: string) { - return this.db.session.findFirst({ - where: { - id: sessionId, - }, - }); + return await this.models.session.getSession(sessionId); } async refreshUserSessionIfNeeded( res: Response, - session: UserSession, - ttr = this.config.auth.session.ttr + userSession: UserSession, + ttr?: number ): Promise { - if ( - session.expiresAt && - session.expiresAt.getTime() - Date.now() > ttr * 1000 - ) { + const newExpiresAt = await this.models.session.refreshUserSessionIfNeeded( + userSession, + ttr + ); + if (!newExpiresAt) { // no need to refresh return false; } - const newExpiresAt = new Date( - Date.now() + this.config.auth.session.ttl * 1000 - ); - - await this.db.userSession.update({ - where: { - id: session.id, - }, - data: { - expiresAt: newExpiresAt, - }, - }); - - res.cookie(AuthService.sessionCookieName, session.sessionId, { + res.cookie(AuthService.sessionCookieName, userSession.sessionId, { expires: newExpiresAt, ...this.cookieOptions, }); @@ -281,11 +193,7 @@ export class AuthService implements OnApplicationBootstrap { } async revokeUserSessions(userId: string) { - return this.db.userSession.deleteMany({ - where: { - userId, - }, - }); + return await this.models.session.deleteUserSession(userId); } getSessionOptionsFromRequest(req: Request) { @@ -425,15 +333,4 @@ export class AuthService implements OnApplicationBootstrap { to: email, }); } - - @Cron(CronExpression.EVERY_DAY_AT_MIDNIGHT) - async cleanExpiredSessions() { - await this.db.userSession.deleteMany({ - where: { - expiresAt: { - lte: new Date(), - }, - }, - }); - } } diff --git a/packages/backend/server/src/core/auth/session.ts b/packages/backend/server/src/core/auth/session.ts index 3707d5aaab..9a42e5fcad 100644 --- a/packages/backend/server/src/core/auth/session.ts +++ b/packages/backend/server/src/core/auth/session.ts @@ -1,8 +1,8 @@ import type { ExecutionContext } from '@nestjs/common'; import { createParamDecorator } from '@nestjs/common'; -import { User, UserSession } from '@prisma/client'; import { getRequestResponseFromContext } from '../../base'; +import type { User, UserSession } from '../../models'; /** * Used to fetch current user from the request context. @@ -37,7 +37,7 @@ import { getRequestResponseFromContext } from '../../base'; * ``` */ // interface and variable don't conflict -// eslint-disable-next-line no-redeclare +// oxlint-disable-next-line no-redeclare export const CurrentUser = createParamDecorator( (_: unknown, context: ExecutionContext) => { return getRequestResponseFromContext(context).req.session?.user; @@ -51,7 +51,7 @@ export interface CurrentUser } // interface and variable don't conflict -// eslint-disable-next-line no-redeclare +// oxlint-disable-next-line no-redeclare export const Session = createParamDecorator( (_: unknown, context: ExecutionContext) => { return getRequestResponseFromContext(context).req.session; diff --git a/packages/backend/server/src/models/index.ts b/packages/backend/server/src/models/index.ts index 0edcc2e90a..0e7ec89f63 100644 --- a/packages/backend/server/src/models/index.ts +++ b/packages/backend/server/src/models/index.ts @@ -4,6 +4,8 @@ import { SessionModel } from './session'; import { UserModel } from './user'; import { VerificationTokenModel } from './verification-token'; +export * from './session'; +export * from './user'; export * from './verification-token'; const models = [UserModel, SessionModel, VerificationTokenModel] as const; diff --git a/packages/backend/server/src/models/user.ts b/packages/backend/server/src/models/user.ts index e579afd08a..d366793fa5 100644 --- a/packages/backend/server/src/models/user.ts +++ b/packages/backend/server/src/models/user.ts @@ -14,7 +14,7 @@ import { } from '../base'; import type { Payload } from '../base/event/def'; import { Permission } from '../core/permission'; -import { Quota_FreePlanV1_1 } from '../core/quota'; +import { Quota_FreePlanV1_1 } from '../core/quota/schema'; const publicUserSelect = { id: true,