From b872bb48a6243657134264b21afe04fdc4efad9f Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 8 Oct 2026 17:36:53 +0200 Subject: [PATCH] refactor(auth): check sessions through SessionsService, not its repository The sessions domain owns user_sessions, yet AuthService injected UserSessionsRepository next to SessionsService and AuthModule registered the entity in its own forFeature, only so verifyJwtToken could hand the repository to verifyJwtAndLoadUser. verifyJwtAndLoadUser now takes a SessionLookup, the two repository methods the session check uses (findActive, touchLastSeen). SessionsService answers both, so AuthService passes itself through the sessions domain's API and drops the repository and the forFeature entry. The guards keep handing in the repository from their own entity manager, which satisfies the same type. auth.service.ts lost lines, so its size baseline goes down with it. --- server/scripts/size-baseline.json | 2 +- server/src/nest/auth/auth.module.ts | 7 +++---- server/src/nest/auth/auth.service.ts | 5 +---- server/src/nest/auth/jwt-verify.ts | 12 +++++++++-- server/src/nest/sessions/sessions.service.ts | 19 +++++++++++++++++ server/tests/helpers/mcp-test-controllers.ts | 3 +-- server/tests/unit/nest/admin.service.test.ts | 3 +-- .../nest/admin.version-notification.test.ts | 3 +-- server/tests/unit/nest/auth.service.test.ts | 1 - server/tests/unit/nest/oidc.service.test.ts | 3 +-- .../tests/unit/nest/passkey.service.test.ts | 3 +-- .../nest/sessions/sessions.service.test.ts | 21 +++++++++++++++++++ 12 files changed, 60 insertions(+), 22 deletions(-) diff --git a/server/scripts/size-baseline.json b/server/scripts/size-baseline.json index 49ddfd068..034ce75db 100644 --- a/server/scripts/size-baseline.json +++ b/server/scripts/size-baseline.json @@ -7,7 +7,7 @@ "src/db/repositories/Users.repository.ts": 1710, "src/nest/atlas/atlas-geo.ts": 1005, "src/nest/atlas/atlas.service.ts": 1182, - "src/nest/auth/auth.service.ts": 1165, + "src/nest/auth/auth.service.ts": 1163, "src/nest/budget/budget.service.ts": 1702, "src/nest/collections/collections.service.ts": 1488, "src/nest/doc-sync/doc-sync.service.ts": 1060, diff --git a/server/src/nest/auth/auth.module.ts b/server/src/nest/auth/auth.module.ts index 1563a19e0..ab9482dc4 100644 --- a/server/src/nest/auth/auth.module.ts +++ b/server/src/nest/auth/auth.module.ts @@ -21,7 +21,6 @@ import { TripMembers } from '../../db/entities/TripMembers.entity'; import { Plugins } from '../../db/entities/Plugins.entity'; import { PluginUserErasureQueue } from '../../db/entities/PluginUserErasureQueue.entity'; import { PushSubscriptions } from '../../db/entities/PushSubscriptions.entity'; -import { UserSessions } from '../../db/entities/UserSessions.entity'; import { TokensModule } from '../tokens/tokens.module'; import { AuthPublicController } from './auth-public.controller'; import { AuthController } from './auth.controller'; @@ -121,9 +120,9 @@ import { AllowedFileTypesModule } from '../files/allowed-file-types.module'; // .enqueueUserErasure` rather than re-implementing the same filter twice. // PushSubscriptions: AuthService drops every Web Push device of the user // on password change/reset (Web Push, #894), owned by `nest/notifications`. - // UserSessions: AuthService.verifyJwtToken hands it to verifyJwtAndLoadUser, - // owned by `nest/sessions`. - MikroOrmModule.forFeature([AppSettings, Users, WebauthnCredentials, WebauthnChallenges, InviteTokens, McpTokens, OauthTokens, PasswordResetTokens, BudgetItems, BudgetSettlements, JourneyShareTokens, Journeys, JourneyEntries, JourneyContributors, ShareTokens, Trips, TripMembers, Plugins, PluginUserErasureQueue, PushSubscriptions, UserSessions])], + // UserSessions is not here: AuthService reaches the session rows through + // SessionsService, which owns them (`nest/sessions`). + MikroOrmModule.forFeature([AppSettings, Users, WebauthnCredentials, WebauthnChallenges, InviteTokens, McpTokens, OauthTokens, PasswordResetTokens, BudgetItems, BudgetSettlements, JourneyShareTokens, Journeys, JourneyEntries, JourneyContributors, ShareTokens, Trips, TripMembers, Plugins, PluginUserErasureQueue, PushSubscriptions])], controllers: [AuthPublicController, AuthController, PasskeyController, SessionsController], providers: [AuthService, UserProfileService, RegistrationInvitesService, PasskeyService, UserCleanupService, WebauthnConfigService, AuthMcp], exports: [AuthService, RegistrationInvitesService, PasskeyService, UserCleanupService], diff --git a/server/src/nest/auth/auth.service.ts b/server/src/nest/auth/auth.service.ts index d267bd8a2..f95b9b56f 100644 --- a/server/src/nest/auth/auth.service.ts +++ b/server/src/nest/auth/auth.service.ts @@ -33,8 +33,6 @@ import { PasswordResetTokens } from '../../db/entities/PasswordResetTokens.entit import type { PasswordResetTokensRepository } from '../../db/repositories/PasswordResetTokens.repository'; import { PushSubscriptions } from '../../db/entities/PushSubscriptions.entity'; import type { PushSubscriptionsRepository } from '../../db/repositories/PushSubscriptions.repository'; -import { UserSessions } from '../../db/entities/UserSessions.entity'; -import type { UserSessionsRepository } from '../../db/repositories/UserSessions.repository'; import { SessionsService, type SessionClient } from '../sessions/sessions.service'; // Type-and-guard only: the app-config read reports the provider choice, it does // not construct one, so this does not pull the maps domain into auth. @@ -188,7 +186,6 @@ export class AuthService { @InjectRepository(WebauthnCredentials) private readonly webauthnCredentials: WebauthnCredentialsRepository, @InjectRepository(PasswordResetTokens) private readonly passwordResetTokens: PasswordResetTokensRepository, @InjectRepository(PushSubscriptions) private readonly pushSubscriptions: PushSubscriptionsRepository, - @InjectRepository(UserSessions) private readonly userSessions: UserSessionsRepository, private readonly sessions: SessionsService, ) {} @@ -1134,6 +1131,6 @@ export class AuthService { * route) should go through. */ async verifyJwtToken(token: string): Promise { - return verifyJwtAndLoadUser(token, this.usersRepo, this.userSessions); + return verifyJwtAndLoadUser(token, this.usersRepo, this.sessions); } } diff --git a/server/src/nest/auth/jwt-verify.ts b/server/src/nest/auth/jwt-verify.ts index e5ae0c894..047ecc447 100644 --- a/server/src/nest/auth/jwt-verify.ts +++ b/server/src/nest/auth/jwt-verify.ts @@ -102,6 +102,14 @@ export function verifiedSessionClaims(token: string | null | undefined): (Sessio } } +/** + * What the session check reads and writes: the two `UserSessionsRepository` + * methods. The guards hand in the repository from their own entity manager; + * `AuthService` hands in `SessionsService`, which answers the same two calls, + * so auth reaches the table through the sessions domain that owns it. + */ +export type SessionLookup = Pick; + /** * The session half of the check. A token with a `jti` must name an active * session of its own user: one revoked (logout, a password change, "sign out @@ -109,7 +117,7 @@ export function verifiedSessionClaims(token: string | null | undefined): (Sessio * was issued before sessions were tracked and passes until its own expiry, so * the upgrade signs nobody out; a password change still ends it through `pv`. */ -async function sessionIsActive(decoded: { id: number; jti?: unknown }, sessions: UserSessionsRepository): Promise { +async function sessionIsActive(decoded: { id: number; jti?: unknown }, sessions: SessionLookup): Promise { if (decoded.jti === undefined) return true; if (typeof decoded.jti !== 'string') return false; const now = new Date(); @@ -130,7 +138,7 @@ async function sessionIsActive(decoded: { id: number; jti?: unknown }, sessions: export async function verifyJwtAndLoadUser( token: string, users: UsersRepository, - sessions: UserSessionsRepository, + sessions: SessionLookup, ): Promise { try { const decoded = jwt.verify(token, JWT_SECRET, { algorithms: ['HS256'] }) as { diff --git a/server/src/nest/sessions/sessions.service.ts b/server/src/nest/sessions/sessions.service.ts index 57362d6d1..3d0957b49 100644 --- a/server/src/nest/sessions/sessions.service.ts +++ b/server/src/nest/sessions/sessions.service.ts @@ -137,6 +137,25 @@ export class SessionsService { return (await this.sessions.findActive(jti, user.id, dbNow())) ? token : null; } + /** + * The active session a token names, for the session check in + * `verifyJwtAndLoadUser` (auth/jwt-verify.ts, its `SessionLookup`): null + * when it is revoked, expired, gone or another user's. + */ + async findActive(id: string, userId: number, now: string): Promise<{ id: string; last_seen_at: string } | null> { + return this.sessions.findActive(id, userId, now); + } + + /** + * Refresh when a session was last used; the same check calls it at most + * every few minutes. + * + * @txStandalone bookkeeping for the session list that stands on its own. + */ + async touchLastSeen(id: string, now: string): Promise { + await this.sessions.touchLastSeen(id, now); + } + /** The user's active sessions, most recently used first, the one in `currentId` flagged. */ async list(userId: number, currentId?: string): Promise { const rows = await this.sessions.listActiveForUser(userId, dbNow()); diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index d686c79a4..33dbb552b 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -190,7 +190,7 @@ import { createTestPushSubscriptionsRepo } from './notifications-repos'; import { createTestToursRepo } from './tours-repos'; import { createTestBudgetSettlementsRepo } from './budget-repos'; import { MaintenanceRepository } from '../../src/db/repositories/MaintenanceRepository'; -import { createTestSessionsService, createTestUserSessionsRepo } from './sessions'; +import { createTestSessionsService } from './sessions'; /** * Hand-wired counterpart of the boot-time discovery in McpRegistryService, @@ -241,7 +241,6 @@ export async function createMcpTestRegistry(): Promise { new AllowedFileTypesService(appSettings), await createTestUnitOfWork(db), appSettings, usersRepo, inviteTokensRepo, mcpTokensRepoForAuth, oauthTokensRepo, webauthnCredentialsRepoForAuth, passwordResetTokensRepo, await createTestPushSubscriptionsRepo(db), - await createTestUserSessionsRepo(db), await createTestSessionsService(db), ); const queryHelpersService = new QueryHelpersService(await createTestTagsRepo(db), await createTestPlaceRatingsRepo(db), await createTestAssignmentParticipantsRepo(db)); diff --git a/server/tests/unit/nest/admin.service.test.ts b/server/tests/unit/nest/admin.service.test.ts index d22569a77..b5472d326 100644 --- a/server/tests/unit/nest/admin.service.test.ts +++ b/server/tests/unit/nest/admin.service.test.ts @@ -117,7 +117,7 @@ import { createTestPushSubscriptionsRepo } from '../../helpers/notifications-rep import { createTestBudgetSettlementsRepo } from '../../helpers/budget-repos'; import { MaintenanceRepository } from '../../../src/db/repositories/MaintenanceRepository'; import type { DatabaseBackupStrategy } from '../../../src/nest/database/database-backup.interface'; -import { createTestSessionsService, createTestUserSessionsRepo } from '../../helpers/sessions'; +import { createTestSessionsService } from '../../helpers/sessions'; const realtime = new RealtimeService(); @@ -152,7 +152,6 @@ beforeAll(async () => { await createTestAppSettingsRepo(testDb), await createTestUsersRepo(testDb), await createTestInviteTokensRepo(testDb), await createTestMcpTokensRepo(testDb), await createTestOauthTokensRepo(testDb), await createTestWebauthnCredentialsRepo(testDb), await createTestPasswordResetTokensRepo(testDb), await createTestPushSubscriptionsRepo(testDb), - await createTestUserSessionsRepo(testDb), await createTestSessionsService(testDb), ); const t = await sharedTestOrm(testDb); diff --git a/server/tests/unit/nest/admin.version-notification.test.ts b/server/tests/unit/nest/admin.version-notification.test.ts index 2e292f129..0bd80ac06 100644 --- a/server/tests/unit/nest/admin.version-notification.test.ts +++ b/server/tests/unit/nest/admin.version-notification.test.ts @@ -90,7 +90,7 @@ import { createTestPushSubscriptionsRepo } from '../../helpers/notifications-rep import { createTestBudgetSettlementsRepo } from '../../helpers/budget-repos'; import { MaintenanceRepository } from '../../../src/db/repositories/MaintenanceRepository'; import type { DatabaseBackupStrategy } from '../../../src/nest/database/database-backup.interface'; -import { createTestSessionsService, createTestUserSessionsRepo } from '../../helpers/sessions'; +import { createTestSessionsService } from '../../helpers/sessions'; const realtime = new RealtimeService(); @@ -116,7 +116,6 @@ beforeAll(async () => { await createTestAppSettingsRepo(testDb), await createTestUsersRepo(testDb), await createTestInviteTokensRepo(testDb), await createTestMcpTokensRepo(testDb), await createTestOauthTokensRepo(testDb), await createTestWebauthnCredentialsRepo(testDb), await createTestPasswordResetTokensRepo(testDb), await createTestPushSubscriptionsRepo(testDb), - await createTestUserSessionsRepo(testDb), await createTestSessionsService(testDb), ); const t = await sharedTestOrm(testDb); diff --git a/server/tests/unit/nest/auth.service.test.ts b/server/tests/unit/nest/auth.service.test.ts index dae120578..36382fae7 100644 --- a/server/tests/unit/nest/auth.service.test.ts +++ b/server/tests/unit/nest/auth.service.test.ts @@ -131,7 +131,6 @@ beforeAll(async () => { await createTestInviteTokensRepo(testDb), await createTestMcpTokensRepo(testDb), await createTestOauthTokensRepo(testDb), await createTestWebauthnCredentialsRepo(testDb), await createTestPasswordResetTokensRepo(testDb), await createTestPushSubscriptionsRepo(testDb), - await createTestUserSessionsRepo(testDb), await createTestSessionsService(testDb), ); }); diff --git a/server/tests/unit/nest/oidc.service.test.ts b/server/tests/unit/nest/oidc.service.test.ts index c2f297b44..2d0360975 100644 --- a/server/tests/unit/nest/oidc.service.test.ts +++ b/server/tests/unit/nest/oidc.service.test.ts @@ -123,7 +123,7 @@ import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-re import { createTestPushSubscriptionsRepo } from '../../helpers/notifications-repos'; import { createTestBudgetSettlementsRepo } from '../../helpers/budget-repos'; import { MaintenanceRepository } from '../../../src/db/repositories/MaintenanceRepository'; -import { createTestSessionsService, createTestUserSessionsRepo } from '../../helpers/sessions'; +import { createTestSessionsService } from '../../helpers/sessions'; // MailerService is injected since the notifications fold — a stub instead of a // module mock. sendPasswordResetEmail is the only thing auth reaches for. @@ -151,7 +151,6 @@ beforeAll(async () => { await createTestInviteTokensRepo(testDb), await createTestMcpTokensRepo(testDb), await createTestOauthTokensRepo(testDb), await createTestWebauthnCredentialsRepo(testDb), await createTestPasswordResetTokensRepo(testDb), await createTestPushSubscriptionsRepo(testDb), - await createTestUserSessionsRepo(testDb), await createTestSessionsService(testDb), ); svc = new OidcService(auth, membership, await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestInviteTokensRepo(testDb), await createTestAppSettingsRepo(testDb)); diff --git a/server/tests/unit/nest/passkey.service.test.ts b/server/tests/unit/nest/passkey.service.test.ts index b610dfa7d..502250f3f 100644 --- a/server/tests/unit/nest/passkey.service.test.ts +++ b/server/tests/unit/nest/passkey.service.test.ts @@ -107,7 +107,7 @@ import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-re import { createTestPushSubscriptionsRepo } from '../../helpers/notifications-repos'; import { createTestBudgetSettlementsRepo } from '../../helpers/budget-repos'; import { MaintenanceRepository } from '../../../src/db/repositories/MaintenanceRepository'; -import { createTestSessionsService, createTestUserSessionsRepo } from '../../helpers/sessions'; +import { createTestSessionsService } from '../../helpers/sessions'; // MailerService is injected since the notifications fold — a stub instead of a // module mock. sendPasswordResetEmail is the only thing auth reaches for. @@ -144,7 +144,6 @@ beforeAll(async () => { await createTestInviteTokensRepo(testDb), await createTestMcpTokensRepo(testDb), await createTestOauthTokensRepo(testDb), await createTestWebauthnCredentialsRepo(testDb), await createTestPasswordResetTokensRepo(testDb), await createTestPushSubscriptionsRepo(testDb), - await createTestUserSessionsRepo(testDb), await createTestSessionsService(testDb), ); svc = new PasskeyService( diff --git a/server/tests/unit/nest/sessions/sessions.service.test.ts b/server/tests/unit/nest/sessions/sessions.service.test.ts index ebd416a3f..a36062e43 100644 --- a/server/tests/unit/nest/sessions/sessions.service.test.ts +++ b/server/tests/unit/nest/sessions/sessions.service.test.ts @@ -8,6 +8,8 @@ import { createUser } from '../../../helpers/factories'; import { UserSessions } from '../../../../src/db/entities/UserSessions.entity'; import { SessionsService, USER_AGENT_MAX_LENGTH, legacySessionId, sessionClientFrom } from '../../../../src/nest/sessions/sessions.service'; import { userSessionIdSchema } from '@trek/shared'; +import { verifyJwtAndLoadUser } from '../../../../src/nest/auth/jwt-verify'; +import { Users } from '../../../../src/db/entities/Users.entity'; import type { Request } from 'express'; import { JWT_SECRET, SESSION_DURATION_REMEMBER_SECONDS, SESSION_DURATION_SECONDS } from '../../../../src/config'; @@ -257,3 +259,22 @@ describe('SessionsService list and revoke', () => { expect(rowOf(short.jti)).toBeUndefined(); }); }); + +describe('SessionsService as the session check', () => { + it('SESS-016: answers findActive and touchLastSeen for verifyJwtAndLoadUser', async () => { + const { user } = createUser(testDb); + const live = claimsOf(await svc.issue({ id: user.id, pv: 0 })); + const ended = claimsOf(await svc.issue({ id: user.id, pv: 0 })); + await svc.revoke(user.id, ended.jti); + const now = textOf(Math.floor(Date.now() / 1000)); + + expect(await svc.findActive(live.jti, user.id, now)).toEqual({ id: live.jti, last_seen_at: textOf(live.iat) }); + expect(await svc.findActive(ended.jti, user.id, now)).toBeNull(); + await svc.touchLastSeen(live.jti, '2030-01-01 00:00:00'); + expect(rowOf(live.jti)?.last_seen_at).toBe('2030-01-01 00:00:00'); + + const sign = (jti: string) => jwt.sign({ id: user.id, pv: 0 }, JWT_SECRET, { algorithm: 'HS256', expiresIn: 600, jwtid: jti }); + expect((await verifyJwtAndLoadUser(sign(live.jti), t.repo(Users), svc))?.id).toBe(user.id); + expect(await verifyJwtAndLoadUser(sign(ended.jti), t.repo(Users), svc)).toBeNull(); + }); +});