mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
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.
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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],
|
||||
|
||||
@@ -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<User | null> {
|
||||
return verifyJwtAndLoadUser(token, this.usersRepo, this.userSessions);
|
||||
return verifyJwtAndLoadUser(token, this.usersRepo, this.sessions);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<UserSessionsRepository, 'findActive' | 'touchLastSeen'>;
|
||||
|
||||
/**
|
||||
* 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<boolean> {
|
||||
async function sessionIsActive(decoded: { id: number; jti?: unknown }, sessions: SessionLookup): Promise<boolean> {
|
||||
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<User | null> {
|
||||
try {
|
||||
const decoded = jwt.verify(token, JWT_SECRET, { algorithms: ['HS256'] }) as {
|
||||
|
||||
@@ -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<void> {
|
||||
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<UserSession[]> {
|
||||
const rows = await this.sessions.listActiveForUser(userId, dbNow());
|
||||
|
||||
@@ -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<McpRegistry> {
|
||||
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));
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -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));
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user