From 64c0814118a5aba57192f0c3077846bfdbf6b8c7 Mon Sep 17 00:00:00 2001 From: forehalo Date: Thu, 19 Sep 2024 20:54:30 +0800 Subject: [PATCH] feat(server): manage auth cookies --- .../server/src/core/auth/controller.ts | 15 +- .../backend/server/src/core/auth/guard.ts | 21 +-- .../backend/server/src/core/auth/service.ts | 91 ++++++++--- .../server/tests/auth/controller.spec.ts | 151 +++++++++++++++++- 4 files changed, 230 insertions(+), 48 deletions(-) diff --git a/packages/backend/server/src/core/auth/controller.ts b/packages/backend/server/src/core/auth/controller.ts index 7db022278be40..cfe1037ea5092 100644 --- a/packages/backend/server/src/core/auth/controller.ts +++ b/packages/backend/server/src/core/auth/controller.ts @@ -172,16 +172,19 @@ export class AuthController { }); } + @Public() @Get('/sign-out') async signOut( @Res() res: Response, - @Session() session: Session, - @Body() { all }: { all: boolean } + @Session() session: Session | undefined, + @Query('user_id') userId: string | undefined ) { - await this.auth.signOut( - session.sessionId, - all ? undefined : session.userId - ); + if (!session) { + return; + } + + await this.auth.signOut(session.sessionId, userId); + await this.auth.refreshCookies(res, session.sessionId); res.status(HttpStatus.OK).send({}); } diff --git a/packages/backend/server/src/core/auth/guard.ts b/packages/backend/server/src/core/auth/guard.ts index f1039f34b8084..53bf915d9cb9d 100644 --- a/packages/backend/server/src/core/auth/guard.ts +++ b/packages/backend/server/src/core/auth/guard.ts @@ -2,11 +2,10 @@ import type { CanActivate, ExecutionContext, FactoryProvider, - OnModuleInit, } from '@nestjs/common'; import { Injectable, SetMetadata } from '@nestjs/common'; -import { ModuleRef, Reflector } from '@nestjs/core'; -import type { Request } from 'express'; +import { Reflector } from '@nestjs/core'; +import type { Request, Response } from 'express'; import { AuthenticationRequired, @@ -22,22 +21,16 @@ import { Session } from './session'; const PUBLIC_ENTRYPOINT_SYMBOL = Symbol('public'); @Injectable() -export class AuthGuard implements CanActivate, OnModuleInit { - private auth!: AuthService; - +export class AuthGuard implements CanActivate { constructor( - private readonly ref: ModuleRef, + private readonly auth: AuthService, private readonly reflector: Reflector ) {} - onModuleInit() { - this.auth = this.ref.get(AuthService, { strict: false }); - } - async canActivate(context: ExecutionContext) { const { req, res } = getRequestResponseFromContext(context); - const userSession = await this.signIn(req); + const userSession = await this.signIn(req, res); if (res && userSession && userSession.expiresAt) { await this.auth.refreshUserSessionIfNeeded(res, userSession); } @@ -59,7 +52,7 @@ export class AuthGuard implements CanActivate, OnModuleInit { return true; } - async signIn(req: Request): Promise { + async signIn(req: Request, res?: Response): Promise { if (req.session) { return req.session; } @@ -68,7 +61,7 @@ export class AuthGuard implements CanActivate, OnModuleInit { parseCookies(req); // TODO(@forehalo): a cache for user session - const userSession = await this.auth.getUserSessionFromRequest(req); + const userSession = await this.auth.getUserSessionFromRequest(req, res); if (userSession) { req.session = { diff --git a/packages/backend/server/src/core/auth/service.ts b/packages/backend/server/src/core/auth/service.ts index a29897fa633e2..268b10f2d330a 100644 --- a/packages/backend/server/src/core/auth/service.ts +++ b/packages/backend/server/src/core/auth/service.ts @@ -122,35 +122,45 @@ export class AuthService implements OnApplicationBootstrap { sessionId: string, userId?: string ): Promise<{ user: CurrentUser; session: UserSession } | null> { - const userSession = await this.db.userSession.findFirst({ - where: { - sessionId, - userId, - }, - select: { - id: true, - sessionId: true, - userId: true, - createdAt: true, - expiresAt: true, - user: true, - }, - orderBy: { - createdAt: 'asc', - }, - }); + const sessions = await this.getUserSessions(sessionId); - // no such session - if (!userSession) { + if (!sessions.length) { return null; } - // user session expired - if (userSession.expiresAt && userSession.expiresAt <= new Date()) { + let userSession: UserSession | undefined; + + // try read from user provided cookies.userId + if (userId) { + userSession = sessions.find(s => s.userId === userId); + } + + // 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 + userSession = sessions.at(-1)!; + } + + const user = await this.user.findUserById(userSession.userId); + + if (!user) { return null; } - return { user: sessionUser(userSession.user), session: userSession }; + return { user: sessionUser(user), session: userSession }; + } + + async getUserSessions(sessionId: string) { + return this.db.userSession.findMany({ + where: { + sessionId, + OR: [{ expiresAt: { gt: new Date() } }, { expiresAt: null }], + }, + orderBy: { + createdAt: 'asc', + }, + }); } async createUserSession( @@ -309,6 +319,25 @@ export class AuthService implements OnApplicationBootstrap { this.setUserCookie(res, userId); } + async refreshCookies(res: Response, sessionId?: string) { + if (sessionId) { + const users = await this.getUserList(sessionId); + const candidateUser = users.at(-1); + + if (candidateUser) { + this.setUserCookie(res, candidateUser.id); + return; + } + } + + this.clearCookies(res); + } + + private clearCookies(res: Response>) { + res.clearCookie(AuthService.sessionCookieName); + res.clearCookie(AuthService.userCookieName); + } + setUserCookie(res: Response, userId: string) { res.cookie(AuthService.userCookieName, userId, { ...this.cookieOptions, @@ -319,14 +348,28 @@ export class AuthService implements OnApplicationBootstrap { }); } - async getUserSessionFromRequest(req: Request) { + async getUserSessionFromRequest(req: Request, res?: Response) { const { sessionId, userId } = this.getSessionOptionsFromRequest(req); if (!sessionId) { return null; } - return this.getUserSession(sessionId, userId); + const session = await this.getUserSession(sessionId, userId); + + if (res) { + if (session) { + // set user id cookie for fast authentication + if (!userId || userId !== session.user.id) { + this.setUserCookie(res, session.user.id); + } + } else if (sessionId) { + // clear invalid cookies.session and cookies.userId + this.clearCookies(res); + } + } + + return session; } async changePassword( diff --git a/packages/backend/server/tests/auth/controller.spec.ts b/packages/backend/server/tests/auth/controller.spec.ts index cf919966e73b3..bedf7f783ed67 100644 --- a/packages/backend/server/tests/auth/controller.spec.ts +++ b/packages/backend/server/tests/auth/controller.spec.ts @@ -161,12 +161,155 @@ test('should be able to sign out', async t => { t.falsy(session.user); }); -test('should not be able to sign out if not signed in', async t => { - const { app } = t.context; +test('should be able to correct user id cookie', async t => { + const { app, u1 } = t.context; + + const signInRes = await request(app.getHttpServer()) + .post('/api/auth/sign-in') + .send({ email: u1.email, password: '1' }) + .expect(200); + + const cookie = sessionCookie(signInRes.headers); + + let session = await request(app.getHttpServer()) + .get('/api/auth/session') + .set('cookie', cookie) + .expect(200); + + let userIdCookie = session.get('Set-Cookie')?.find(c => { + return c.startsWith(`${AuthService.userCookieName}=`); + }); + + t.true(userIdCookie?.startsWith(`${AuthService.userCookieName}=${u1.id}`)); + + session = await request(app.getHttpServer()) + .get('/api/auth/session') + .set('cookie', `${cookie};${AuthService.userCookieName}=invalid_user_id`) + .expect(200); + + userIdCookie = session.get('Set-Cookie')?.find(c => { + return c.startsWith(`${AuthService.userCookieName}=`); + }); + + t.true(userIdCookie?.startsWith(`${AuthService.userCookieName}=${u1.id}`)); + t.is(session.body.user.id, u1.id); +}); + +// multiple accounts session tests +test('should be able to sign in another account in one session', async t => { + const { app, u1, auth } = t.context; + + const u2 = await auth.signUp('u3@affine.pro', '3'); + + // sign in u1 + const signInRes = await request(app.getHttpServer()) + .post('/api/auth/sign-in') + .send({ email: u1.email, password: '1' }) + .expect(200); + + const cookie = sessionCookie(signInRes.headers); + + // avoid create session at the exact same time, leads to same random session users order + await new Promise(resolve => setTimeout(resolve, 1)); + + // sign in u2 in the same session + await request(app.getHttpServer()) + .post('/api/auth/sign-in') + .set('cookie', cookie) + .send({ email: u2.email, password: '3' }) + .expect(200); + + // list [u1, u2] + const sessions = await request(app.getHttpServer()) + .get('/api/auth/sessions') + .set('cookie', cookie) + .expect(200); + t.is(sessions.body.users.length, 2); + t.is(sessions.body.users[0].id, u1.id); + t.is(sessions.body.users[1].id, u2.id); + + // default to latest signed in user: u2 + let session = await request(app.getHttpServer()) + .get('/api/auth/session') + .set('cookie', cookie) + .expect(200); + + t.is(session.body.user.id, u2.id); + + // switch to u1 + session = await request(app.getHttpServer()) + .get('/api/auth/session') + .set('cookie', `${cookie};${AuthService.userCookieName}=${u1.id}`) + .expect(200); + + t.is(session.body.user.id, u1.id); +}); + +test('should be able to sign out multiple accounts in one session', async t => { + const { app, u1, auth } = t.context; + + const u2 = await auth.signUp('u4@affine.pro', '4'); + + // sign in u1 + const signInRes = await request(app.getHttpServer()) + .post('/api/auth/sign-in') + .send({ email: u1.email, password: '1' }) + .expect(200); + + const cookie = sessionCookie(signInRes.headers); + + await new Promise(resolve => setTimeout(resolve, 1)); + + // sign in u2 in the same session await request(app.getHttpServer()) + .post('/api/auth/sign-in') + .set('cookie', cookie) + .send({ email: u2.email, password: '4' }) + .expect(200); + + // sign out u2 + let signOut = await request(app.getHttpServer()) + .get(`/api/auth/sign-out?user_id=${u2.id}`) + .set('cookie', `${cookie};${AuthService.userCookieName}=${u2.id}`) + .expect(200); + + // auto switch to u1 after sign out u2 + const userIdCookie = signOut.get('Set-Cookie')?.find(c => { + return c.startsWith(`${AuthService.userCookieName}=`); + }); + + t.true(userIdCookie?.startsWith(`${AuthService.userCookieName}=${u1.id}`)); + + // list [u1] + const session = await request(app.getHttpServer()) + .get('/api/auth/session') + .set('cookie', cookie) + .expect(200); + + t.is(session.body.user.id, u1.id); + + // sign in u2 in the same session + await request(app.getHttpServer()) + .post('/api/auth/sign-in') + .set('cookie', cookie) + .send({ email: u2.email, password: '4' }) + .expect(200); + + // sign out all account in session + signOut = await request(app.getHttpServer()) .get('/api/auth/sign-out') - .expect(HttpStatus.UNAUTHORIZED); + .set('cookie', cookie) + .expect(200); - t.assert(true); + t.true( + signOut + .get('Set-Cookie') + ?.some(c => c.startsWith(`${AuthService.sessionCookieName}=;`)) + ); + t.true( + signOut + .get('Set-Cookie') + ?.some(c => c.startsWith(`${AuthService.userCookieName}=;`)) + ); });