diff --git a/server/src/nest/auth/auth.controller.ts b/server/src/nest/auth/auth.controller.ts index b692eb235..ca31d5561 100644 --- a/server/src/nest/auth/auth.controller.ts +++ b/server/src/nest/auth/auth.controller.ts @@ -17,15 +17,13 @@ import { import { FileInterceptor } from '@nestjs/platform-express'; import { isDemoWriteBlocked, DEMO_WRITE_ERROR } from '../common/demo-write'; import { RuntimeEnvService } from '../app-config/runtime-env.service'; -import { diskStorage } from 'multer'; +import type { Options } from 'multer'; import type { Request, Response } from 'express'; import path from 'path'; -import fs from 'fs'; -import { v4 as uuid } from 'uuid'; import { AuthService } from './auth.service'; import { TokenService } from '../tokens/token.service'; import { UserProfileService } from './user-profile.service'; -import { avatarDir } from './auth.helpers'; +import { StorageService } from '../storage/storage.service'; import { ChangePasswordDto, MapsKeyUpdateDto, @@ -49,21 +47,17 @@ import { ManagedForbidden } from '../common/managed'; const WINDOW = 15 * 60 * 1000; const ALLOWED_AVATAR_EXTS = ['.jpg', '.jpeg', '.png', '.gif', '.webp']; -const AVATAR_UPLOAD = { - storage: diskStorage({ - destination: (_req, _file, cb) => { if (!fs.existsSync(avatarDir)) fs.mkdirSync(avatarDir, { recursive: true }); cb(null, avatarDir); }, - filename: (_req, file, cb) => cb(null, uuid() + path.extname(file.originalname)), - }), - limits: { fileSize: 5 * 1024 * 1024 }, - fileFilter: (_req: unknown, file: Express.Multer.File, cb: (err: Error | null, accept: boolean) => void) => { - const ext = path.extname(file.originalname).toLowerCase(); - if (!file.mimetype.startsWith('image/') || !ALLOWED_AVATAR_EXTS.includes(ext)) { - const err: Error & { statusCode?: number } = new Error('Only image files (jpg, png, gif, webp) are allowed'); - err.statusCode = 400; - return cb(err, false); - } - cb(null, true); - }, +export const MAX_AVATAR_SIZE = 5 * 1024 * 1024; +// Consumed by auth.module.ts's MulterModule factory; the storage engine (spool +// destination + UUID filename) comes from the storage upload factory. +export const AVATAR_FILE_FILTER: Options['fileFilter'] = (_req, file, cb) => { + const ext = path.extname(file.originalname).toLowerCase(); + if (!file.mimetype.startsWith('image/') || !ALLOWED_AVATAR_EXTS.includes(ext)) { + const err: Error & { statusCode?: number } = new Error('Only image files (jpg, png, gif, webp) are allowed'); + err.statusCode = 400; + return cb(err); + } + cb(null, true); }; /** @@ -77,7 +71,7 @@ const AVATAR_UPLOAD = { @Controller('api/auth') @UseGuards(JwtAuthGuard) export class AuthController { - constructor(private readonly auth: AuthService, private readonly profile: UserProfileService, private readonly tokens: TokenService, private readonly rl: RateLimitService, private readonly audit: AuditService, private readonly env: RuntimeEnvService) {} + constructor(private readonly auth: AuthService, private readonly profile: UserProfileService, private readonly tokens: TokenService, private readonly rl: RateLimitService, private readonly audit: AuditService, private readonly env: RuntimeEnvService, private readonly storage: StorageService) {} private limit(bucket: string, req: Request, max: number): void { if (!this.rl.check(bucket, req.ip || 'unknown', max, WINDOW, Date.now())) { @@ -153,7 +147,7 @@ export class AuthController { @Post('avatar') @HttpCode(200) - @UseInterceptors(FileInterceptor('avatar', AVATAR_UPLOAD)) + @UseInterceptors(FileInterceptor('avatar')) async avatar(@CurrentUser() user: User, @UploadedFile() file: Express.Multer.File | undefined) { if (isDemoWriteBlocked(this.env, user.email)) { throw new HttpException(DEMO_WRITE_ERROR, 403); @@ -161,6 +155,9 @@ export class AuthController { if (!file) { throw new HttpException({ error: 'No image uploaded' }, 400); } + // Commit the spooled upload to its final storage location (atomic + // same-volume rename) before the DB row references the final path. + await this.storage.put('avatars', file.filename, { tmpPath: file.path }); return this.profile.saveAvatar(user.id, file.filename); } diff --git a/server/src/nest/auth/auth.module.ts b/server/src/nest/auth/auth.module.ts index 059add436..25344f8d0 100644 --- a/server/src/nest/auth/auth.module.ts +++ b/server/src/nest/auth/auth.module.ts @@ -18,6 +18,11 @@ import { MailerModule } from '../notifications/mailer/mailer.module'; import { PermissionsModule } from '../permissions/permissions.module'; import { TripMembershipModule } from '../trip-membership/trip-membership.module'; import { EphemeralTokenModule } from './ephemeral-token.module'; +import { MulterModule } from '@nestjs/platform-express'; +import { StorageModule } from '../storage/storage.module'; +import { StorageService } from '../storage/storage.service'; +import { buildStorageUploadOptions } from '../storage/storage-upload.factory'; +import { AVATAR_FILE_FILTER, MAX_AVATAR_SIZE } from './auth.controller'; /** * Auth module — public flows (login/register/reset/mfa-verify/logout) and the @@ -43,7 +48,19 @@ import { EphemeralTokenModule } from './ephemeral-token.module'; * through auth.bridge.ts. */ @Module({ - imports: [EphemeralTokenModule, RateLimitModule, AuditModule, PermissionsModule, TripMembershipModule, MailerModule, AppConfigModule, TokensModule, BudgetModule], + imports: [ + MulterModule.registerAsync({ + imports: [StorageModule], + inject: [StorageService], + useFactory: (storage: StorageService) => + buildStorageUploadOptions(storage, { + category: 'avatars', + maxSize: MAX_AVATAR_SIZE, + fileFilter: AVATAR_FILE_FILTER, + }), + }), + StorageModule, + EphemeralTokenModule, RateLimitModule, AuditModule, PermissionsModule, TripMembershipModule, MailerModule, AppConfigModule, TokensModule, BudgetModule], controllers: [AuthPublicController, AuthController, PasskeyController], providers: [AuthService, UserProfileService, RegistrationInvitesService, PasskeyService, UserCleanupService, WebauthnConfigService, AuthMcp], exports: [AuthService, RegistrationInvitesService, PasskeyService, UserCleanupService], diff --git a/server/tests/integration/profile.test.ts b/server/tests/integration/profile.test.ts index 4a71a82c0..4b79ab541 100644 --- a/server/tests/integration/profile.test.ts +++ b/server/tests/integration/profile.test.ts @@ -108,6 +108,32 @@ describe('Avatar', () => { expect(typeof res.body.avatar_url).toBe('string'); }); + it('PROFILE-P01 — avatar bytes land at uploads/avatars under a bare uuid name', async () => { + const fsMod = require('fs') as typeof import('fs'); + const pathMod = require('path') as typeof import('path'); + const avatarsDir = pathMod.join(__dirname, '../../uploads/avatars'); + const { user } = createUser(testDb); + + const res = await request(app) + .post('/api/auth/avatar') + .set('Cookie', authCookie(user.id)) + .attach('avatar', FIXTURE_JPEG); + expect(res.status).toBe(200); + const first = pathMod.basename(res.body.avatar_url); + expect(first).toMatch(/^[0-9a-f-]{36}\.jpg$/); + expect(fsMod.existsSync(pathMod.join(avatarsDir, first))).toBe(true); + + // Re-upload removes the previous file (saveAvatar's cleanup branch). + const second = await request(app) + .post('/api/auth/avatar') + .set('Cookie', authCookie(user.id)) + .attach('avatar', FIXTURE_JPEG); + expect(second.status).toBe(200); + const secondName = pathMod.basename(second.body.avatar_url); + expect(fsMod.existsSync(pathMod.join(avatarsDir, secondName))).toBe(true); + expect(fsMod.existsSync(pathMod.join(avatarsDir, first))).toBe(false); + }); + it('PROFILE-003 — uploading non-image (PDF) is rejected', async () => { const { user } = createUser(testDb); const res = await request(app) diff --git a/server/tests/unit/nest/auth.controller.test.ts b/server/tests/unit/nest/auth.controller.test.ts index 23a26b742..6f8578e26 100644 --- a/server/tests/unit/nest/auth.controller.test.ts +++ b/server/tests/unit/nest/auth.controller.test.ts @@ -35,8 +35,9 @@ const audit = { writeAudit } as unknown as AuditService; const apc = (a: AuthService, limiter: RateLimitService) => new AuthPublicController(a, limiter, audit); // Tokens moved to TokenService; the controller takes it second. Stubbed via a // third, optional argument so every non-token call site stays as it was. +const storageStub = { put: vi.fn().mockResolvedValue(undefined) } as unknown as import('../../../src/nest/storage/storage.service').StorageService; const ac = (a: AuthService, limiter: RateLimitService, t: Partial = {}, pr: Partial = {}) => - new AuthController(a, pr as UserProfileService, t as TokenService, limiter, audit, new RuntimeEnvService()); + new AuthController(a, pr as UserProfileService, t as TokenService, limiter, audit, new RuntimeEnvService(), storageStub); function thrown(fn: () => unknown): { status: number; body: unknown } { try { fn(); } catch (err) {