diff --git a/server/src/modules/auth/auth.service.ts b/server/src/modules/auth/auth.service.ts index 3423b2165..ac60134d9 100644 --- a/server/src/modules/auth/auth.service.ts +++ b/server/src/modules/auth/auth.service.ts @@ -6,6 +6,7 @@ import { InternalServerErrorException, Injectable, Logger, + NotFoundException, ServiceUnavailableException, UnauthorizedException, } from '@nestjs/common'; @@ -265,9 +266,6 @@ export class AuthService { throw new UnauthorizedException(); } - // Rotate: revoke old, issue new - await this.db.update(schema.refreshTokens).set({ revokedAt: new Date() }).where(eq(schema.refreshTokens.id, row.id)); - const userForToken = await this.db.query.users.findFirst({ where: eq(schema.users.id, row.userId) }); if (!userForToken) throw new UnauthorizedException(); if (!userForToken.active) { @@ -277,6 +275,9 @@ export class AuthService { throw new UnauthorizedException('Account disabled'); } + // Rotate: revoke old, issue new + await this.db.update(schema.refreshTokens).set({ revokedAt: new Date() }).where(eq(schema.refreshTokens.id, row.id)); + const { accessToken, rawRefreshToken } = await this.issueTokenPair(row.userId, userForToken.tokenVersion); this.setRefreshCookie(reply, rawRefreshToken); this.setAccessCookie(reply, accessToken); @@ -325,6 +326,7 @@ export class AuthService { await this.oidcSessionRepo.revokeByUserId(userId); if (!oidcSession.idTokenHint) return {}; + const oidcLogoutStart = Date.now(); try { const config = await this.appSettings.getOidcConfig(); if (!config.enabled) return {}; @@ -340,7 +342,7 @@ export class AuthService { return { logoutUrl: `${disc.endSessionEndpoint}?${params.toString()}` }; } catch (error) { - const durationMs = 0; + const durationMs = Date.now() - oidcLogoutStart; const errorClass = error instanceof Error ? error.name : 'UnknownError'; const errorMessage = error instanceof Error ? error.message : 'unknown error'; this.logger.warn( @@ -368,7 +370,7 @@ export class AuthService { const row = await this.db.query.refreshTokens.findFirst({ where: eq(schema.refreshTokens.id, sessionId), }); - if (!row) throw new UnauthorizedException(); + if (!row) throw new NotFoundException('Session not found'); if (row.userId !== userId) throw new ForbiddenException('You do not have access to this session'); await this.db.update(schema.refreshTokens).set({ revokedAt: new Date() }).where(eq(schema.refreshTokens.id, sessionId)); } diff --git a/server/src/modules/book-bucket/book-bucket-finalize.service.ts b/server/src/modules/book-bucket/book-bucket-finalize.service.ts index eee883e71..a3c3d4dd1 100644 --- a/server/src/modules/book-bucket/book-bucket-finalize.service.ts +++ b/server/src/modules/book-bucket/book-bucket-finalize.service.ts @@ -1,7 +1,7 @@ import { BadRequestException, Inject, Injectable, Logger, NotFoundException, OnModuleInit } from '@nestjs/common'; import { basename, dirname, extname, join } from 'path'; import { access as fsAccess, readFile, stat, unlink } from 'fs/promises'; -import { and, eq, ilike, inArray, or } from 'drizzle-orm'; +import { and, eq, inArray, or, sql } from 'drizzle-orm'; import { NodePgDatabase } from 'drizzle-orm/node-postgres'; import type { BookBucketAutoFinalizeMetadataMode, BookBucketFinalizeFileResult, BookBucketFinalizeResult, BookBucketMetadata } from '@projectx/types'; @@ -169,7 +169,7 @@ export class BookBucketFinalizeService implements OnModuleInit { let bookId: number; try { const { size } = await stat(destPath); - const bookFolderPath = dirname(destPath) === folder.path ? destPath : dirname(destPath); + const bookFolderPath = dirname(destPath); ({ bookId } = await this.processor.createBookRecord( libraryId, folder.id, @@ -278,7 +278,7 @@ export class BookBucketFinalizeService implements OnModuleInit { .select({ bookId: bookMetadata.bookId }) .from(bookMetadata) .innerJoin(books, eq(books.id, bookMetadata.bookId)) - .where(and(eq(books.libraryId, libraryId), ilike(bookMetadata.title, meta.title))) + .where(and(eq(books.libraryId, libraryId), sql`lower(${bookMetadata.title}) = lower(${meta.title})`)) .limit(1); if (existing) return existing.bookId; diff --git a/server/src/modules/book-bucket/dto/index.ts b/server/src/modules/book-bucket/dto/index.ts index bdf90ed04..f47b96f21 100644 --- a/server/src/modules/book-bucket/dto/index.ts +++ b/server/src/modules/book-bucket/dto/index.ts @@ -1,7 +1,48 @@ -import { IsArray, IsBoolean, IsInt, IsNumber, IsObject, IsOptional, IsString, ValidateIf, ValidateNested } from 'class-validator'; +import { + IsArray, + IsBoolean, + IsInt, + IsNumber, + IsObject, + IsOptional, + IsString, + ValidateIf, + ValidateNested, + type ValidationArguments, + registerDecorator, + type ValidationOptions, + type ValidatorConstraintInterface, + ValidatorConstraint, +} from 'class-validator'; import { Type } from 'class-transformer'; import type { BookBucketMetadata } from '@projectx/types'; +@ValidatorConstraint({ name: 'hasCompleteDefaultDestination', async: false }) +class HasCompleteDefaultDestinationConstraint implements ValidatorConstraintInterface { + validate(_: unknown, args?: ValidationArguments): boolean { + const dto = args?.object as FinalizeBookBucketDto | undefined; + if (!dto) return false; + return (dto.defaultLibraryId === undefined) === (dto.defaultFolderId === undefined); + } + + defaultMessage(): string { + return 'defaultLibraryId and defaultFolderId must either both be provided or both be omitted'; + } +} + +function HasCompleteDefaultDestination(options?: ValidationOptions) { + return function (constructor: new (...args: unknown[]) => unknown) { + registerDecorator({ + name: 'hasCompleteDefaultDestination', + target: constructor, + propertyName: '', + options, + constraints: [], + validator: HasCompleteDefaultDestinationConstraint, + }); + }; +} + export class UpdateBookBucketFileDto { @IsOptional() @IsObject() @@ -31,6 +72,7 @@ class FinalizeOverrideDto { folderId?: number; } +@HasCompleteDefaultDestination() export class FinalizeBookBucketDto { @IsOptional() @IsArray() diff --git a/server/src/modules/email/email-provider.repository.test.ts b/server/src/modules/email/email-provider.repository.test.ts index 26fb0640c..089eb1f08 100644 --- a/server/src/modules/email/email-provider.repository.test.ts +++ b/server/src/modules/email/email-provider.repository.test.ts @@ -122,17 +122,20 @@ describe('EmailProviderRepository', () => { }); }); - it('setDefault marks only owner record and returns updated provider', () => { + it('setDefault atomically flips defaults within the owner scope and returns updated rows', () => { const { db, updateBuilder } = makeDb(); const repo = new EmailProviderRepository(db as never); void repo.setDefault(9, 4); expect(db.update).toHaveBeenCalledWith(emailProviders); - expect(updateBuilder.set).toHaveBeenCalledWith({ - isDefault: true, - updatedAt: expect.objectContaining({ op: 'sql', text: 'now()' }), - }); + expect(updateBuilder.set).toHaveBeenCalledWith( + expect.objectContaining({ + isDefault: expect.objectContaining({ op: 'sql', text: 'case when = then true else false end' }), + updatedAt: expect.objectContaining({ op: 'sql', text: 'now()' }), + }), + ); + expect(updateBuilder.where).toHaveBeenCalledWith({ op: 'eq', left: emailProviders.userId, right: 4 }); expect(updateBuilder.returning).toHaveBeenCalled(); }); diff --git a/server/src/modules/email/email-provider.repository.ts b/server/src/modules/email/email-provider.repository.ts index 063b242ca..f4b76b858 100644 --- a/server/src/modules/email/email-provider.repository.ts +++ b/server/src/modules/email/email-provider.repository.ts @@ -46,8 +46,11 @@ export class EmailProviderRepository { setDefault(id: number, userId: number) { return this.db .update(emailProviders) - .set({ isDefault: true, updatedAt: sql`now()` }) - .where(and(eq(emailProviders.id, id), eq(emailProviders.userId, userId))) + .set({ + isDefault: sql`case when ${emailProviders.id} = ${id} then true else false end`, + updatedAt: sql`now()`, + }) + .where(eq(emailProviders.userId, userId)) .returning(); } diff --git a/server/src/modules/email/email-provider.service.test.ts b/server/src/modules/email/email-provider.service.test.ts index 7d7737487..3957acfd8 100644 --- a/server/src/modules/email/email-provider.service.test.ts +++ b/server/src/modules/email/email-provider.service.test.ts @@ -142,7 +142,6 @@ describe('EmailProviderService', () => { describe('setDefault', () => { it('should set default provider', async () => { const result = await service.setDefault(10, mockUser); - expect(repo.clearDefault).toHaveBeenCalledWith(1); expect(repo.setDefault).toHaveBeenCalledWith(10, 1); expect(result.id).toBe(10); }); diff --git a/server/src/modules/email/email-provider.service.ts b/server/src/modules/email/email-provider.service.ts index 908ea9036..48ccb545f 100644 --- a/server/src/modules/email/email-provider.service.ts +++ b/server/src/modules/email/email-provider.service.ts @@ -92,8 +92,7 @@ export class EmailProviderService { async setDefault(id: number, user: RequestUser) { await this.getOwned(id, user); - await this.repo.clearDefault(user.id); - const [updated] = await this.repo.setDefault(id, user.id); + const updated = (await this.repo.setDefault(id, user.id)).find((provider) => provider.id === id); if (!updated) throw new NotFoundException('Provider not found'); return this.sanitize(updated); } diff --git a/server/src/modules/email/email-recipient-group.service.test.ts b/server/src/modules/email/email-recipient-group.service.test.ts index 4850c9c20..0c13f2b1c 100644 --- a/server/src/modules/email/email-recipient-group.service.test.ts +++ b/server/src/modules/email/email-recipient-group.service.test.ts @@ -3,12 +3,14 @@ import { Test, TestingModule } from '@nestjs/testing'; import { EmailRecipientGroupService } from './email-recipient-group.service'; import { EmailRecipientGroupRepository } from './email-recipient-group.repository'; import { EmailRecipientRepository } from './email-recipient.repository'; +import { EmailTemplateService } from './email-template.service'; import type { RequestUser } from '../../common/types/request-user'; describe('EmailRecipientGroupService', () => { let service: EmailRecipientGroupService; let repo: EmailRecipientGroupRepository; let recipientRepo: EmailRecipientRepository; + let templateService: EmailTemplateService; const mockUser: RequestUser = { id: 1, @@ -52,12 +54,19 @@ describe('EmailRecipientGroupService', () => { findById: vi.fn().mockResolvedValue([{ id: 100, userId: 1 }]), }, }, + { + provide: EmailTemplateService, + useValue: { + findOne: vi.fn().mockResolvedValue({ id: 8 }), + }, + }, ], }).compile(); service = module.get(EmailRecipientGroupService); repo = module.get(EmailRecipientGroupRepository); recipientRepo = module.get(EmailRecipientRepository); + templateService = module.get(EmailTemplateService); }); describe('findAll', () => { @@ -86,6 +95,11 @@ describe('EmailRecipientGroupService', () => { expect(result.id).toBe(10); }); + it('should validate template access when creating a group with a default template', async () => { + await service.create({ name: 'New Group', defaultTemplateId: 8 }, mockUser); + expect(templateService.findOne).toHaveBeenCalledWith(8, mockUser); + }); + it('should map duplicate group names to ConflictException', async () => { (repo.insert as vi.Mock).mockRejectedValue({ code: '23505' }); await expect(service.create({ name: 'New Group' }, mockUser)).rejects.toThrow(ConflictException); @@ -100,6 +114,11 @@ describe('EmailRecipientGroupService', () => { expect(result.id).toBe(10); }); + it('should validate template access when updating a group default template', async () => { + await service.update(10, { defaultTemplateId: 8 }, mockUser); + expect(templateService.findOne).toHaveBeenCalledWith(8, mockUser); + }); + it('should map duplicate group names to ConflictException', async () => { (repo.update as vi.Mock).mockRejectedValue({ code: '23505' }); await expect(service.update(10, { name: 'Updated' }, mockUser)).rejects.toThrow(ConflictException); diff --git a/server/src/modules/email/email-recipient-group.service.ts b/server/src/modules/email/email-recipient-group.service.ts index 806f18c89..7551a1bc3 100644 --- a/server/src/modules/email/email-recipient-group.service.ts +++ b/server/src/modules/email/email-recipient-group.service.ts @@ -3,6 +3,7 @@ import { ConflictException, ForbiddenException, Injectable, NotFoundException } import type { RequestUser } from '../../common/types/request-user'; import { EmailRecipientGroupRepository } from './email-recipient-group.repository'; import { EmailRecipientRepository } from './email-recipient.repository'; +import { EmailTemplateService } from './email-template.service'; import { CreateEmailRecipientGroupDto } from './dto/create-email-recipient-group.dto'; import { UpdateEmailRecipientGroupDto } from './dto/update-email-recipient-group.dto'; import type { EmailRecipientGroup } from '../../db/schema'; @@ -13,6 +14,7 @@ export class EmailRecipientGroupService { constructor( private readonly repo: EmailRecipientGroupRepository, private readonly recipientRepo: EmailRecipientRepository, + private readonly templateService: EmailTemplateService, ) {} async findAll(user: RequestUser) { @@ -43,6 +45,7 @@ export class EmailRecipientGroupService { } async create(dto: CreateEmailRecipientGroupDto, user: RequestUser) { + await this.validateDefaultTemplate(dto.defaultTemplateId, user); try { const [created] = await this.repo.insert({ userId: user.id, @@ -60,6 +63,7 @@ export class EmailRecipientGroupService { async update(id: number, dto: UpdateEmailRecipientGroupDto, user: RequestUser) { await this.getOwned(id, user); + await this.validateDefaultTemplate(dto.defaultTemplateId, user); try { const [updated] = await this.repo.update(id, user.id, dto); if (!updated) throw new NotFoundException('Group not found'); @@ -103,4 +107,9 @@ export class EmailRecipientGroupService { if (group.userId !== user.id) throw new ForbiddenException('Cannot modify this group'); return group; } + + private async validateDefaultTemplate(templateId: number | null | undefined, user: RequestUser): Promise { + if (templateId === null || templateId === undefined) return; + await this.templateService.findOne(templateId, user); + } } diff --git a/server/src/modules/email/email-recipient.repository.test.ts b/server/src/modules/email/email-recipient.repository.test.ts index ce5f348ea..a27953485 100644 --- a/server/src/modules/email/email-recipient.repository.test.ts +++ b/server/src/modules/email/email-recipient.repository.test.ts @@ -142,7 +142,13 @@ describe('EmailRecipientRepository', () => { void repo.setDefault(11, 3); expect(db.update).toHaveBeenCalledWith(emailRecipients); - expect(updateBuilder.set).toHaveBeenCalledWith({ isDefault: true }); + expect(updateBuilder.set).toHaveBeenCalledWith( + expect.objectContaining({ + isDefault: expect.objectContaining({ op: 'sql', parts: expect.any(Array), values: [emailRecipients.id, 11] }), + updatedAt: expect.objectContaining({ op: 'sql', parts: expect.any(Array), values: [] }), + }), + ); + expect(updateBuilder.where).toHaveBeenCalledWith({ op: 'eq', left: emailRecipients.userId, right: 3 }); expect(updateBuilder.returning).toHaveBeenCalled(); }); diff --git a/server/src/modules/email/email-recipient.repository.ts b/server/src/modules/email/email-recipient.repository.ts index 3d9a2c1f9..1701d5228 100644 --- a/server/src/modules/email/email-recipient.repository.ts +++ b/server/src/modules/email/email-recipient.repository.ts @@ -1,5 +1,5 @@ import { Inject, Injectable } from '@nestjs/common'; -import { and, eq, inArray } from 'drizzle-orm'; +import { and, eq, inArray, sql } from 'drizzle-orm'; import { NodePgDatabase } from 'drizzle-orm/node-postgres'; import { DB } from '../../db'; @@ -48,8 +48,11 @@ export class EmailRecipientRepository { setDefault(id: number, userId: number): Promise { return this.db .update(emailRecipients) - .set({ isDefault: true }) - .where(and(eq(emailRecipients.id, id), eq(emailRecipients.userId, userId))) + .set({ + isDefault: sql`case when ${emailRecipients.id} = ${id} then true else false end`, + updatedAt: sql`now()`, + }) + .where(eq(emailRecipients.userId, userId)) .returning(); } diff --git a/server/src/modules/email/email-recipient.service.test.ts b/server/src/modules/email/email-recipient.service.test.ts index 929f38e52..668d8b031 100644 --- a/server/src/modules/email/email-recipient.service.test.ts +++ b/server/src/modules/email/email-recipient.service.test.ts @@ -1,12 +1,14 @@ -import { ConflictException, NotFoundException } from '@nestjs/common'; +import { ConflictException, ForbiddenException, NotFoundException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { EmailRecipientService } from './email-recipient.service'; import { EmailRecipientRepository } from './email-recipient.repository'; +import { EmailTemplateService } from './email-template.service'; import type { RequestUser } from '../../common/types/request-user'; describe('EmailRecipientService', () => { let service: EmailRecipientService; let repo: EmailRecipientRepository; + let templateService: EmailTemplateService; const mockUser: RequestUser = { id: 1, @@ -42,11 +44,18 @@ describe('EmailRecipientService', () => { setDefault: vi.fn().mockResolvedValue([mockRecipient]), }, }, + { + provide: EmailTemplateService, + useValue: { + findOne: vi.fn().mockResolvedValue({ id: 99 }), + }, + }, ], }).compile(); service = module.get(EmailRecipientService); repo = module.get(EmailRecipientRepository); + templateService = module.get(EmailTemplateService); }); it('should find all for user', async () => { @@ -67,6 +76,16 @@ describe('EmailRecipientService', () => { expect(result.id).toBe(10); }); + it('should validate template access when creating a recipient with a default template', async () => { + await service.create({ name: 'New', email: 'new@test.com', defaultTemplateId: 99 }, mockUser); + expect(templateService.findOne).toHaveBeenCalledWith(99, mockUser); + }); + + it('should propagate template access errors when creating a recipient with a default template', async () => { + (templateService.findOne as vi.Mock).mockRejectedValueOnce(new ForbiddenException('No access to this template')); + await expect(service.create({ name: 'New', email: 'new@test.com', defaultTemplateId: 99 }, mockUser)).rejects.toThrow(ForbiddenException); + }); + it('should map duplicate recipient emails to ConflictException on create', async () => { (repo.insert as vi.Mock).mockRejectedValue({ code: '23505' }); await expect(service.create({ name: 'New', email: 'new@test.com' }, mockUser)).rejects.toThrow(ConflictException); @@ -79,6 +98,11 @@ describe('EmailRecipientService', () => { expect(result.id).toBe(10); }); + it('should validate template access when updating a recipient default template', async () => { + await service.update(10, { defaultTemplateId: 99 }, mockUser); + expect(templateService.findOne).toHaveBeenCalledWith(99, mockUser); + }); + it('should remove a recipient', async () => { await service.remove(10, mockUser); expect(repo.delete).toHaveBeenCalledWith(10, 1); @@ -86,7 +110,6 @@ describe('EmailRecipientService', () => { it('should set default recipient', async () => { const result = await service.setDefault(10, mockUser); - expect(repo.clearDefault).toHaveBeenCalledWith(1); expect(repo.setDefault).toHaveBeenCalledWith(10, 1); expect(result.id).toBe(10); }); diff --git a/server/src/modules/email/email-recipient.service.ts b/server/src/modules/email/email-recipient.service.ts index 2a328fcc2..818bce6f9 100644 --- a/server/src/modules/email/email-recipient.service.ts +++ b/server/src/modules/email/email-recipient.service.ts @@ -2,6 +2,7 @@ import { ConflictException, ForbiddenException, Injectable, NotFoundException } import type { RequestUser } from '../../common/types/request-user'; import { EmailRecipientRepository } from './email-recipient.repository'; +import { EmailTemplateService } from './email-template.service'; import { CreateEmailRecipientDto } from './dto/create-email-recipient.dto'; import { UpdateEmailRecipientDto } from './dto/update-email-recipient.dto'; import type { EmailRecipient } from '../../db/schema'; @@ -9,7 +10,10 @@ import { isUniqueViolation } from './email-db-error.util'; @Injectable() export class EmailRecipientService { - constructor(private readonly repo: EmailRecipientRepository) {} + constructor( + private readonly repo: EmailRecipientRepository, + private readonly templateService: EmailTemplateService, + ) {} findAll(user: RequestUser) { return this.repo.findAllForUser(user.id); @@ -20,6 +24,7 @@ export class EmailRecipientService { } async create(dto: CreateEmailRecipientDto, user: RequestUser) { + await this.validateDefaultTemplate(dto.defaultTemplateId, user); try { const [created] = await this.repo.insert({ userId: user.id, @@ -40,6 +45,7 @@ export class EmailRecipientService { async update(id: number, dto: UpdateEmailRecipientDto, user: RequestUser) { await this.getOwnedById(id, user); + await this.validateDefaultTemplate(dto.defaultTemplateId, user); try { const [updated] = await this.repo.update(id, user.id, dto); if (!updated) throw new NotFoundException('Recipient not found'); @@ -59,8 +65,7 @@ export class EmailRecipientService { async setDefault(id: number, user: RequestUser) { await this.getOwnedById(id, user); - await this.repo.clearDefault(user.id); - const [updated] = await this.repo.setDefault(id, user.id); + const updated = (await this.repo.setDefault(id, user.id)).find((recipient) => recipient.id === id); if (!updated) throw new NotFoundException('Recipient not found'); return updated; } @@ -85,4 +90,9 @@ export class EmailRecipientService { return uniqueIds.map((id) => recipientById.get(id)!); } + + private async validateDefaultTemplate(templateId: number | null | undefined, user: RequestUser): Promise { + if (templateId === null || templateId === undefined) return; + await this.templateService.findOne(templateId, user); + } } diff --git a/server/src/modules/email/email-template.repository.test.ts b/server/src/modules/email/email-template.repository.test.ts index aed36f710..8a11be32f 100644 --- a/server/src/modules/email/email-template.repository.test.ts +++ b/server/src/modules/email/email-template.repository.test.ts @@ -127,16 +127,16 @@ describe('EmailTemplateRepository', () => { }); void repo.setDefault(22, 4); - expect(updateBuilder.set).toHaveBeenLastCalledWith({ - isDefault: true, - updatedAt: expect.objectContaining({ op: 'sql', text: 'now()' }), - }); + expect(updateBuilder.set).toHaveBeenLastCalledWith( + expect.objectContaining({ + isDefault: expect.objectContaining({ op: 'sql', text: 'case when = then true else false end' }), + updatedAt: expect.objectContaining({ op: 'sql', text: 'now()' }), + }), + ); expect(updateBuilder.where).toHaveBeenLastCalledWith({ - op: 'and', - clauses: [ - { op: 'eq', left: emailTemplates.id, right: 22 }, - { op: 'eq', left: emailTemplates.userId, right: 4 }, - ], + op: 'eq', + left: emailTemplates.userId, + right: 4, }); void repo.delete(22, 4); diff --git a/server/src/modules/email/email-template.repository.ts b/server/src/modules/email/email-template.repository.ts index 2e40f3700..2d6bd9572 100644 --- a/server/src/modules/email/email-template.repository.ts +++ b/server/src/modules/email/email-template.repository.ts @@ -70,8 +70,11 @@ export class EmailTemplateRepository { setDefault(id: number, userId: number) { return this.db .update(emailTemplates) - .set({ isDefault: true, updatedAt: sql`now()` }) - .where(and(eq(emailTemplates.id, id), eq(emailTemplates.userId, userId))) + .set({ + isDefault: sql`case when ${emailTemplates.id} = ${id} then true else false end`, + updatedAt: sql`now()`, + }) + .where(eq(emailTemplates.userId, userId)) .returning(); } diff --git a/server/src/modules/email/email-template.service.test.ts b/server/src/modules/email/email-template.service.test.ts index 02672c97b..74d84fbde 100644 --- a/server/src/modules/email/email-template.service.test.ts +++ b/server/src/modules/email/email-template.service.test.ts @@ -1,4 +1,4 @@ -import { ConflictException, ForbiddenException } from '@nestjs/common'; +import { ConflictException, ForbiddenException, NotFoundException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { EmailTemplateService } from './email-template.service'; import { EmailTemplateRepository } from './email-template.repository'; @@ -144,7 +144,6 @@ describe('EmailTemplateService', () => { describe('setDefault', () => { it('should set owned template as default', async () => { const result = await service.setDefault(10, mockUser); - expect(repo.clearDefault).toHaveBeenCalledWith(1); expect(repo.setDefault).toHaveBeenCalledWith(10, 1); expect(result.id).toBe(10); }); @@ -186,6 +185,11 @@ describe('EmailTemplateService', () => { await expect(service.resolveTemplate(10, mockUser)).rejects.toThrow(ForbiddenException); }); + it('should throw NotFoundException if a specific template is requested but missing', async () => { + (repo.findById as vi.Mock).mockResolvedValue([]); + await expect(service.resolveTemplate(999, mockUser)).rejects.toThrow(NotFoundException); + }); + it('should return user default if no template requested', async () => { const result = await service.resolveTemplate(null, mockUser); expect(result.id).toBe(10); diff --git a/server/src/modules/email/email-template.service.ts b/server/src/modules/email/email-template.service.ts index 2149d7933..c4e267c4a 100644 --- a/server/src/modules/email/email-template.service.ts +++ b/server/src/modules/email/email-template.service.ts @@ -79,12 +79,12 @@ export class EmailTemplateService { if (template.userId !== user.id && !template.isSystem) { throw new ForbiddenException('Cannot set this template as default'); } - await this.repo.clearDefault(user.id); if (!template.isSystem) { - const [updated] = await this.repo.setDefault(id, user.id); + const updated = (await this.repo.setDefault(id, user.id)).find((row) => row.id === id); if (!updated) throw new NotFoundException('Template not found'); return updated; } + await this.repo.clearDefault(user.id); return template; } @@ -96,14 +96,15 @@ export class EmailTemplateService { } async resolveTemplate(templateId: number | null | undefined, user: RequestUser): Promise { - if (templateId) { + if (templateId !== null && templateId !== undefined) { const [tpl] = await this.repo.findById(templateId); - if (tpl) { - if (tpl.userId !== null && tpl.userId !== user.id) { - throw new ForbiddenException('No access to the specified email template'); - } - return tpl; + if (!tpl) { + throw new NotFoundException('Template not found'); } + if (tpl.userId !== null && tpl.userId !== user.id) { + throw new ForbiddenException('No access to the specified email template'); + } + return tpl; } const [userDefault] = await this.repo.findUserDefault(user.id); if (userDefault) return userDefault; diff --git a/server/src/modules/file-write/file-write.repository.ts b/server/src/modules/file-write/file-write.repository.ts index 2041e449d..b9bb11839 100644 --- a/server/src/modules/file-write/file-write.repository.ts +++ b/server/src/modules/file-write/file-write.repository.ts @@ -40,7 +40,8 @@ export class FileWriteRepository { }) .from(books) .innerJoin(bookFiles, eq(bookFiles.id, books.primaryFileId)) - .where(and(eq(books.libraryId, libraryId), ne(books.status, 'missing'))); + .where(and(eq(books.libraryId, libraryId), ne(books.status, 'missing'))) + .orderBy(asc(books.id)); } async loadPayload(bookId: number) { diff --git a/server/src/modules/file-write/file-write.service.ts b/server/src/modules/file-write/file-write.service.ts index d766e65f3..6d98d8087 100644 --- a/server/src/modules/file-write/file-write.service.ts +++ b/server/src/modules/file-write/file-write.service.ts @@ -25,6 +25,7 @@ export class FileWriteService implements OnModuleDestroy { private readonly debounceMs: number; private readonly maxConcurrentWrites: number; private readonly debounceMap = new Map(); + private readonly scheduledWriteRuns = new Set>(); private readonly writeQueue: Array<() => void> = []; private activeWrites = 0; @@ -53,24 +54,40 @@ export class FileWriteService implements OnModuleDestroy { this.logger.debug( `[${FILE_WRITE_SCHEDULE_EVENT}] [end] bookId=${bookId} triggeredBy=${triggeredBy} userId=${formatUserId(userId)} - scheduled file write fired`, ); - this.writeToFile(bookId, triggeredBy, userId).catch((err: Error) => - this.logger.warn( - `[${FILE_WRITE_SCHEDULE_EVENT}] [fail] bookId=${bookId} triggeredBy=${triggeredBy} userId=${formatUserId(userId)} errorClass=${err.name} error="${sanitizeErrorMessage(err.message)}" - scheduled file write failed`, - ), - ); + const run = this.writeToFile(bookId, triggeredBy, userId) + .catch((err: Error) => + this.logger.warn( + `[${FILE_WRITE_SCHEDULE_EVENT}] [fail] bookId=${bookId} triggeredBy=${triggeredBy} userId=${formatUserId(userId)} errorClass=${err.name} error="${sanitizeErrorMessage(err.message)}" - scheduled file write failed`, + ), + ) + .finally(() => { + this.scheduledWriteRuns.delete(run); + }); + this.scheduledWriteRuns.add(run); }, this.debounceMs); this.debounceMap.set(bookId, timer); } onModuleDestroy(): void { - for (const timer of this.debounceMap.values()) clearTimeout(timer); - this.debounceMap.clear(); + this.clearScheduledWrites(); for (const release of this.writeQueue) { release(); } this.writeQueue.length = 0; } + async drainScheduledWritesForTests(): Promise { + this.clearScheduledWrites(); + while (this.scheduledWriteRuns.size > 0) { + await Promise.allSettled([...this.scheduledWriteRuns]); + } + } + + private clearScheduledWrites(): void { + for (const timer of this.debounceMap.values()) clearTimeout(timer); + this.debounceMap.clear(); + } + async writeToFile(bookId: number, triggeredBy: 'auto' | 'sync', userId?: number, dryRun = false): Promise { await this.acquireWriteSlot(); diff --git a/server/src/modules/kobo/services/kobo-book-access.service.test.ts b/server/src/modules/kobo/services/kobo-book-access.service.test.ts new file mode 100644 index 000000000..97b3b7083 --- /dev/null +++ b/server/src/modules/kobo/services/kobo-book-access.service.test.ts @@ -0,0 +1,61 @@ +import { ForbiddenException, NotFoundException } from '@nestjs/common'; + +import { KoboBookAccessService } from './kobo-book-access.service'; + +describe('KoboBookAccessService', () => { + function createService(options: { + user?: { isSuperuser: boolean } | undefined; + accessRows?: Array<{ libraryId: number }>; + book?: { id: number; libraryId: number } | undefined; + }) { + const db = { + query: { + users: { + findFirst: vi.fn().mockResolvedValue(options.user), + }, + books: { + findFirst: vi.fn().mockResolvedValue(options.book), + }, + }, + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockResolvedValue(options.accessRows ?? []), + }), + }), + }; + + return { + service: new KoboBookAccessService(db as never), + db, + }; + } + + it('throws not found when the book does not exist', async () => { + const { service } = createService({ + user: { isSuperuser: false }, + book: undefined, + }); + + await expect(service.assertBookAccessible(7, 99)).rejects.toThrow(NotFoundException); + }); + + it('throws forbidden when the user lacks access to the book library', async () => { + const { service } = createService({ + user: { isSuperuser: false }, + accessRows: [{ libraryId: 1 }], + book: { id: 42, libraryId: 2 }, + }); + + await expect(service.assertBookAccessible(7, 42)).rejects.toThrow(ForbiddenException); + }); + + it('allows access when the user can reach the book library', async () => { + const { service } = createService({ + user: { isSuperuser: false }, + accessRows: [{ libraryId: 2 }], + book: { id: 42, libraryId: 2 }, + }); + + await expect(service.assertBookAccessible(7, 42)).resolves.toBeUndefined(); + }); +}); diff --git a/server/src/modules/kobo/services/kobo-book-access.service.ts b/server/src/modules/kobo/services/kobo-book-access.service.ts index ba4490b41..6c4ab61dd 100644 --- a/server/src/modules/kobo/services/kobo-book-access.service.ts +++ b/server/src/modules/kobo/services/kobo-book-access.service.ts @@ -1,4 +1,4 @@ -import { ForbiddenException, Inject, Injectable } from '@nestjs/common'; +import { ForbiddenException, Inject, Injectable, NotFoundException } from '@nestjs/common'; import { eq } from 'drizzle-orm'; import type { NodePgDatabase } from 'drizzle-orm/node-postgres'; @@ -31,7 +31,9 @@ export class KoboBookAccessService { columns: { id: true, libraryId: true }, }); - if (!book) return; + if (!book) { + throw new NotFoundException('Book not found'); + } const accessibleLibraryIds = await this.getAccessibleLibraryIds(userId); if (accessibleLibraryIds !== null && !accessibleLibraryIds.includes(book.libraryId)) { diff --git a/server/src/modules/library/library.service.test.ts b/server/src/modules/library/library.service.test.ts index 47be77318..7a21c34d6 100644 --- a/server/src/modules/library/library.service.test.ts +++ b/server/src/modules/library/library.service.test.ts @@ -276,12 +276,21 @@ describe('LibraryService', () => { }); it('writeMetadataToFiles blocks non-dry-run when file write is disabled', async () => { + libraryRepo.findById.mockResolvedValue([{ id: 1, name: 'L' }]); fileWriteService.resolveSettings.mockResolvedValue({ enabled: false }); await expect(service.writeMetadataToFiles(1, 7, false)).rejects.toBeInstanceOf(BadRequestException); }); + it('writeMetadataToFiles throws when the library does not exist', async () => { + libraryRepo.findById.mockResolvedValue([]); + + await expect(service.writeMetadataToFiles(404, 7, true)).rejects.toBeInstanceOf(NotFoundException); + expect(fileWriteService.findNonMissingPrimaryFilesByLibrary).not.toHaveBeenCalled(); + }); + it('writeMetadataToFiles emits progress and returns summary counters', async () => { + libraryRepo.findById.mockResolvedValue([{ id: 1, name: 'L' }]); fileWriteService.resolveSettings.mockResolvedValue({ enabled: true }); fileWriteService.findNonMissingPrimaryFilesByLibrary.mockResolvedValue([{ bookId: 1 }, { bookId: 2 }, { bookId: 3 }]); fileWriteService.writeToFile @@ -299,6 +308,7 @@ describe('LibraryService', () => { }); it('writeMetadataToFiles stops when cancellation is requested', async () => { + libraryRepo.findById.mockResolvedValue([{ id: 1, name: 'L' }]); fileWriteService.resolveSettings.mockResolvedValue({ enabled: true }); fileWriteService.findNonMissingPrimaryFilesByLibrary.mockResolvedValue([{ bookId: 1 }, { bookId: 2 }]); fileWriteService.writeToFile.mockResolvedValue({ status: 'success', fieldsWritten: [], durationMs: 1 }); diff --git a/server/src/modules/library/library.service.ts b/server/src/modules/library/library.service.ts index 569af1a44..a621f244c 100644 --- a/server/src/modules/library/library.service.ts +++ b/server/src/modules/library/library.service.ts @@ -280,6 +280,9 @@ export class LibraryService { dryRun: boolean, options: LibraryMetadataWriteStreamOptions = {}, ): Promise { + const [library] = await this.libraryRepo.findById(libraryId); + if (!library) throw new NotFoundException('Library not found'); + if (!dryRun) { const settings = await this.fileWriteService.resolveSettings(libraryId); if (!settings.enabled) { diff --git a/server/src/modules/metadata/lib/pdf-parser.test.ts b/server/src/modules/metadata/lib/pdf-parser.test.ts index 67ce17911..93bdd132d 100644 --- a/server/src/modules/metadata/lib/pdf-parser.test.ts +++ b/server/src/modules/metadata/lib/pdf-parser.test.ts @@ -118,6 +118,16 @@ describe('parsePdfFile', () => { ); }); + it('can skip cover extraction when only metadata is needed', async () => { + mockExtractXmpXml.mockReturnValue(null); + + const parsed = await parsePdfFile('/books/book.pdf', { extractCover: false }); + + expect(parsed).toEqual(expect.objectContaining({ title: 'Info Title', coverBuffer: null })); + expect(mockExecFile).not.toHaveBeenCalled(); + expect(mockReadFile).toHaveBeenCalledTimes(1); + }); + it('returns null coverBuffer when pdftoppm command fails, but still returns metadata', async () => { mockExtractXmpXml.mockReturnValue(null); mockExecFile.mockImplementation((_file: string, _args: string[], cb: (err: Error | null) => void) => cb(new Error('pdftoppm missing'))); diff --git a/server/src/modules/metadata/lib/pdf-parser.ts b/server/src/modules/metadata/lib/pdf-parser.ts index 19892f9ed..60df95c4e 100644 --- a/server/src/modules/metadata/lib/pdf-parser.ts +++ b/server/src/modules/metadata/lib/pdf-parser.ts @@ -34,6 +34,10 @@ export interface PdfParsed { coverBuffer: Buffer | null; } +interface PdfParseOptions { + extractCover?: boolean; +} + function clean(value: string | undefined): string | null { if (!value) return null; const s = value.trim(); @@ -64,7 +68,7 @@ async function extractPdfCover(absolutePath: string): Promise { } } -export async function parsePdfFile(absolutePath: string): Promise { +export async function parsePdfFile(absolutePath: string, options: PdfParseOptions = {}): Promise { try { const buf = await readFile(absolutePath); const doc = await PDFDocument.load(buf, { ignoreEncryption: true }); @@ -88,7 +92,7 @@ export async function parsePdfFile(absolutePath: string): Promise ({ name, sortName: null })) : []; - const coverBuffer = await extractPdfCover(absolutePath); + const coverBuffer = options.extractCover === false ? null : await extractPdfCover(absolutePath); return { title: hasXmp ? xmp.title : infoTitle, diff --git a/server/src/modules/opds/__tests__/opds-auth.guard.test.ts b/server/src/modules/opds/__tests__/opds-auth.guard.test.ts index bbb82a54e..e4b707b7e 100644 --- a/server/src/modules/opds/__tests__/opds-auth.guard.test.ts +++ b/server/src/modules/opds/__tests__/opds-auth.guard.test.ts @@ -142,6 +142,14 @@ describe('OpdsAuthGuard', () => { expect((request.opdsUser as OpdsRequestUser).userId).toBe(1); }); + it('rejects a same-length tampered cover token', async () => { + const guard = makeGuard(); + const token = createCoverToken(1, TEST_SECRET); + const tamperedToken = `${token.slice(0, -1)}${token.endsWith('0') ? '1' : '0'}`; + const { context } = mockContext(undefined, tamperedToken, '/api/v1/opds/42/cover'); + await expect(guard.canActivate(context)).rejects.toThrow(UnauthorizedException); + }); + it('attaches opdsUser to request on success', async () => { const guard = makeGuard(); const { context, request } = mockContext(basicHeader('reader', 'pass')); diff --git a/server/src/modules/opds/opds-auth.guard.ts b/server/src/modules/opds/opds-auth.guard.ts index 9212577f1..ce186095f 100644 --- a/server/src/modules/opds/opds-auth.guard.ts +++ b/server/src/modules/opds/opds-auth.guard.ts @@ -1,4 +1,4 @@ -import { createHmac } from 'crypto'; +import { createHmac, timingSafeEqual } from 'crypto'; import { CanActivate, ExecutionContext, ForbiddenException, Injectable, UnauthorizedException } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; @@ -39,7 +39,11 @@ function parseCoverToken(token: string, secret: string): number | null { const userId = parseInt(token.slice(0, dot), 10); if (isNaN(userId) || userId <= 0) return null; const expected = createCoverToken(userId, secret); - if (token !== expected) return null; + const tokenBuffer = Buffer.from(token); + const expectedBuffer = Buffer.from(expected); + if (tokenBuffer.length !== expectedBuffer.length || !timingSafeEqual(tokenBuffer, expectedBuffer)) { + return null; + } return userId; } diff --git a/server/src/modules/reader-preferences/dto/upsert-preference.dto.ts b/server/src/modules/reader-preferences/dto/upsert-preference.dto.ts index e23e8b1fc..42e248d1d 100644 --- a/server/src/modules/reader-preferences/dto/upsert-preference.dto.ts +++ b/server/src/modules/reader-preferences/dto/upsert-preference.dto.ts @@ -2,5 +2,5 @@ import { IsObject } from 'class-validator'; export class UpsertPreferenceDto { @IsObject() - settings: Record; + settings!: Record; } diff --git a/server/src/modules/scanner/file-event-processor.service.ts b/server/src/modules/scanner/file-event-processor.service.ts index 91fd3bc3a..74553298f 100644 --- a/server/src/modules/scanner/file-event-processor.service.ts +++ b/server/src/modules/scanner/file-event-processor.service.ts @@ -225,7 +225,7 @@ export class FileEventProcessorService { if (!match || match.file.absolutePath === newAbsolutePath) return { type: 'noop' }; const { file, libraryId: rowLibraryId, folderPath: oldFolderPath, libraryFolderPath } = match; - const newFolderPath = oldFolderPath === file.absolutePath ? newAbsolutePath : dirname(newAbsolutePath); + const newFolderPath = dirname(newAbsolutePath) === libraryFolderPath ? newAbsolutePath : dirname(newAbsolutePath); if (newFolderPath !== oldFolderPath) { const targetBooks = await this.scannerRepo.findBooksByFolderPath(newFolderPath, rowLibraryId); const hasFolderCollision = targetBooks.some((book) => book.id !== file.bookId && book.folderPath === newFolderPath); diff --git a/server/src/modules/scanner/lib/walk.ts b/server/src/modules/scanner/lib/walk.ts index a388c158c..7730421a4 100644 --- a/server/src/modules/scanner/lib/walk.ts +++ b/server/src/modules/scanner/lib/walk.ts @@ -51,16 +51,19 @@ function naturalCompare(a: string, b: string): number { return 0; } -function matchesExcludePattern(name: string, patterns: string[]): boolean { - for (const pattern of patterns) { - if (!pattern.includes('*')) { - if (name === pattern) return true; - } else { - const escaped = pattern.replace(/[.+^${}()|[\]\\]/g, '\\$&').replace(/\*/g, '.*'); - if (new RegExp(`^${escaped}$`).test(name)) return true; +function buildExcludeMatcher(patterns: string[]): (name: string) => boolean { + if (patterns.length === 0) return () => false; + const compiled = patterns.map((p) => { + if (!p.includes('*')) return { literal: p, regex: null as RegExp | null }; + const escaped = p.replace(/[.+^${}()|[\]\\]/g, '\\$&').replace(/\*/g, '.*'); + return { literal: null as string | null, regex: new RegExp(`^${escaped}$`) }; + }); + return (name: string) => { + for (const { literal, regex } of compiled) { + if (literal !== null ? name === literal : regex!.test(name)) return true; } - } - return false; + return false; + }; } // Recursively collect files, grouped by their parent directory. @@ -68,7 +71,7 @@ async function collectByDir( dir: string, libraryRoot: string, acc: Map, - excludePatterns: string[], + shouldExclude: (name: string) => boolean, logger?: (msg: string) => void, ): Promise { let entries; @@ -90,7 +93,7 @@ async function collectByDir( const full = join(dir, entry.name); if (entry.name.startsWith('.')) continue; - if (excludePatterns.length > 0 && matchesExcludePattern(entry.name, excludePatterns)) continue; + if (shouldExclude(entry.name)) continue; if (entry.isDirectory()) { subdirs.push(full); @@ -104,8 +107,18 @@ async function collectByDir( } if (filePaths.length > 0) { - const stats = await Promise.all(filePaths.map(async (full) => ({ full, s: await stat(full) }))); - for (const { full, s } of stats) { + const statResults = await Promise.all( + filePaths.map(async (full) => { + const s = await stat(full).catch((err: NodeJS.ErrnoException) => { + if (err.code === 'ENOENT') return null; + throw err; + }); + return s ? { full, s } : null; + }), + ); + for (const entry of statResults) { + if (!entry) continue; + const { full, s } = entry; if (!acc.has(dir)) acc.set(dir, []); acc.get(dir)!.push({ absolutePath: full, @@ -117,7 +130,7 @@ async function collectByDir( } } - await Promise.all(subdirs.map((full) => collectByDir(full, libraryRoot, acc, excludePatterns, logger))); + await Promise.all(subdirs.map((full) => collectByDir(full, libraryRoot, acc, shouldExclude, logger))); } /** @@ -143,7 +156,8 @@ export async function findBookCandidates( logger?: (msg: string) => void, ): Promise { const byDir = new Map(); - await collectByDir(libraryFolderPath, libraryFolderPath, byDir, excludePatterns, logger); + const shouldExclude = buildExcludeMatcher(excludePatterns); + await collectByDir(libraryFolderPath, libraryFolderPath, byDir, shouldExclude, logger); // Flatten disc subdirectories (e.g. "CD 1", "Disc 2") into their parent. // Collect disc dirs first to avoid mutating the map while iterating. @@ -234,7 +248,8 @@ export async function findLooseFileCandidates( logger?: (msg: string) => void, ): Promise { const byDir = new Map(); - await collectByDir(libraryFolderPath, libraryFolderPath, byDir, excludePatterns, logger); + const shouldExclude = buildExcludeMatcher(excludePatterns); + await collectByDir(libraryFolderPath, libraryFolderPath, byDir, shouldExclude, logger); const candidates: BookCandidate[] = []; @@ -272,13 +287,14 @@ export async function buildSingleBookCandidate( return null; } + const shouldExclude = buildExcludeMatcher(excludePatterns); const filePaths: string[] = []; const discDirs: string[] = []; const nonDiscDirs: { name: string; path: string }[] = []; for (const entry of entries) { if (entry.name.startsWith('.')) continue; - if (excludePatterns.length > 0 && matchesExcludePattern(entry.name, excludePatterns)) continue; + if (shouldExclude(entry.name)) continue; const full = join(folderPath, entry.name); if (entry.isDirectory()) { if (isDiscDirectory(entry.name)) { @@ -295,6 +311,7 @@ export async function buildSingleBookCandidate( const discEntries = await readdir(discDir, { withFileTypes: true }).catch(() => []); for (const entry of discEntries) { if (!entry.isFile() || entry.name.startsWith('.')) continue; + if (shouldExclude(entry.name)) continue; const full = join(discDir, entry.name); if (full.length <= MAX_PATH_LENGTH) filePaths.push(full); } @@ -310,6 +327,7 @@ export async function buildSingleBookCandidate( const stemEntries = await readdir(stemDir, { withFileTypes: true }).catch(() => []); for (const entry of stemEntries) { if (!entry.isFile() || entry.name.startsWith('.')) continue; + if (shouldExclude(entry.name)) continue; const full = join(stemDir, entry.name); if (full.length <= MAX_PATH_LENGTH) filePaths.push(full); } diff --git a/server/test/auth-recovery-oidc-logout.e2e-spec.ts b/server/test/auth-recovery-oidc-logout.e2e-spec.ts index 0aaaf815b..d0119ad4e 100644 --- a/server/test/auth-recovery-oidc-logout.e2e-spec.ts +++ b/server/test/auth-recovery-oidc-logout.e2e-spec.ts @@ -12,7 +12,7 @@ import * as schema from '../src/db/schema'; import { MetadataService } from '../src/modules/metadata/metadata.service'; import { OidcDiscoveryService } from '../src/modules/auth/oidc/oidc-discovery.service'; import { SystemMailService } from '../src/modules/email/system-mail.service'; -import type { Db } from './e2e/app-harness'; +import { makeMetadataNoopMock, type Db } from './e2e/app-harness'; type CookieJar = Map; @@ -94,19 +94,6 @@ function cookieHeader(jar: CookieJar): string { .join('; '); } -function makeMetadataNoopMock(): Pick< - MetadataService, - 'extractAndSave' | 'refreshCoverForBook' | 'extractAudioFileDuration' | 'aggregateAudioDuration' | 'extractAudioChaptersAndNarrators' -> { - return { - extractAndSave: () => Promise.resolve(undefined), - refreshCoverForBook: () => Promise.resolve(false), - extractAudioFileDuration: () => Promise.resolve(undefined), - aggregateAudioDuration: () => Promise.resolve(undefined), - extractAudioChaptersAndNarrators: () => Promise.resolve(undefined), - }; -} - function defaultDiscoveryDoc() { return { issuer: 'https://issuer.example', @@ -586,6 +573,30 @@ describe('Auth recovery and OIDC logout hardening (e2e)', () => { }); describe('OIDC logout', () => { + it('clears cookies and revokes session for local user without OIDC session', async () => { + const user = await createUser(context.db); + const session = await login(context.app, user.username, user.password); + + const logoutResponse = await context.app.inject({ + method: 'POST', + url: '/api/v1/auth/logout', + headers: { cookie: cookieHeader(session.jar) }, + }); + expect(logoutResponse.statusCode).toBe(200); + expect(logoutResponse.json()).toEqual({}); + + const logoutCookies = getSetCookieLines(logoutResponse.headers); + expect(cookieValue(logoutCookies, 'refresh_token')).toBe(''); + expect(cookieValue(logoutCookies, 'access_token')).toBe(''); + + const refreshAfterLogout = await context.app.inject({ + method: 'POST', + url: '/api/v1/auth/refresh', + headers: { cookie: `refresh_token=${session.refreshToken}` }, + }); + expect(refreshAfterLogout.statusCode).toBe(401); + }); + it('returns provider logout URL and revokes local OIDC session', async () => { const user = await createUser(context.db); const session = await login(context.app, user.username, user.password); diff --git a/server/test/auth-session-security.e2e-spec.ts b/server/test/auth-session-security.e2e-spec.ts index 25bba0454..b98968a7c 100644 --- a/server/test/auth-session-security.e2e-spec.ts +++ b/server/test/auth-session-security.e2e-spec.ts @@ -10,6 +10,7 @@ import { AppModule } from '../src/app.module'; import { DB } from '../src/db'; import * as schema from '../src/db/schema'; import { MetadataService } from '../src/modules/metadata/metadata.service'; +import { makeMetadataNoopMock } from './e2e/app-harness'; import type { Db } from './e2e/app-harness'; type CookieJar = Map; @@ -101,19 +102,6 @@ function cookieHeader(jar: CookieJar): string { .join('; '); } -function makeMetadataNoopMock(): Pick< - MetadataService, - 'extractAndSave' | 'refreshCoverForBook' | 'extractAudioFileDuration' | 'aggregateAudioDuration' | 'extractAudioChaptersAndNarrators' -> { - return { - extractAndSave: () => Promise.resolve(undefined), - refreshCoverForBook: () => Promise.resolve(false), - extractAudioFileDuration: () => Promise.resolve(undefined), - aggregateAudioDuration: () => Promise.resolve(undefined), - extractAudioChaptersAndNarrators: () => Promise.resolve(undefined), - }; -} - async function createAuthE2EContext(): Promise { const moduleFixture = await Test.createTestingModule({ imports: [AppModule], @@ -241,7 +229,11 @@ describe('Auth session security (e2e)', () => { expect(refreshCookieLine).toBeDefined(); expect(accessCookieLine).toBeDefined(); expect(cookieAttribute(refreshCookieLine!, 'path')).toBe('/api/v1/auth'); + expect(cookieAttribute(refreshCookieLine!, 'httponly')).toBe(''); + expect(cookieAttribute(refreshCookieLine!, 'samesite')).toBe('Strict'); expect(cookieAttribute(accessCookieLine!, 'path')).toBe('/api'); + expect(cookieAttribute(accessCookieLine!, 'httponly')).toBe(''); + expect(cookieAttribute(accessCookieLine!, 'samesite')).toBe('Lax'); const statusAfter = await context.app.inject({ method: 'GET', @@ -369,6 +361,9 @@ describe('Auth session security (e2e)', () => { expect(cookieValue(responseCookies, 'refresh_token')).toBe(''); expect(cookieValue(responseCookies, 'access_token')).toBe(''); + const tokenRow = await findRefreshTokenByRawToken(context.db, loginResult.refreshToken); + expect(tokenRow?.revokedAt).toBeNull(); + const meResponse = await context.app.inject({ method: 'GET', url: '/api/v1/auth/me', @@ -704,7 +699,7 @@ describe('Auth session security (e2e)', () => { authorization: `Bearer ${sessionB.accessToken}`, }, }); - expect(missingSessionResponse.statusCode).toBe(401); + expect(missingSessionResponse.statusCode).toBe(404); const activeSessionRefresh = await context.app.inject({ method: 'POST', diff --git a/server/test/authorization-matrix.e2e-spec.ts b/server/test/authorization-matrix.e2e-spec.ts index b957d1081..71415d7e1 100644 --- a/server/test/authorization-matrix.e2e-spec.ts +++ b/server/test/authorization-matrix.e2e-spec.ts @@ -804,7 +804,10 @@ describe('Authorization matrix (e2e)', () => { method: 'POST', url: '/api/v1/collections', headers: authHeader(personas.ownerUser.accessToken), - payload: { name: `authz-collection-${randomUUID()}` }, + payload: { + name: `authz-collection-${randomUUID()}`, + icon: 'FolderOpen', + }, }); expect(createdCollection.statusCode).toBe(201); const collectionId = (createdCollection.json() as { id: number }).id; diff --git a/server/test/book-bucket-ingest-finalize.e2e-spec.ts b/server/test/book-bucket-ingest-finalize.e2e-spec.ts index 26893a6a8..60973fa3b 100644 --- a/server/test/book-bucket-ingest-finalize.e2e-spec.ts +++ b/server/test/book-bucket-ingest-finalize.e2e-spec.ts @@ -308,6 +308,7 @@ describe('Book Bucket ingest + finalize (e2e)', () => { .select({ id: schema.books.id, libraryId: schema.books.libraryId, + folderPath: schema.books.folderPath, }) .from(schema.books) .where(eq(schema.books.id, finalizedBookId)) @@ -324,6 +325,7 @@ describe('Book Bucket ingest + finalize (e2e)', () => { .limit(1); expect(bookFile).toBeDefined(); expect(bookFile!.absolutePath.startsWith(destination.folderPath)).toBe(true); + expect(book?.folderPath).toBe(dirname(bookFile!.absolutePath)); expect(await fileExists(bookFile!.absolutePath)).toBe(true); expect(await fileExists(bookBucketRow.absolutePath)).toBe(false); @@ -423,6 +425,93 @@ describe('Book Bucket ingest + finalize (e2e)', () => { expect(await getBookBucketRow(context, successRow.id)).toBeUndefined(); }); + it('finalize stores a folder path even when the naming pattern is flat', async () => { + const destination = await createLibraryWithFolder(context); + await context.db.update(schema.libraries).set({ fileNamingPattern: '{title}' }).where(eq(schema.libraries.id, destination.libraryId)); + + const bookBucketRow = await createBookBucketRow(context, { + fileName: 'flat-pattern.fb2', + selectedMetadata: { + title: 'Flat Pattern Title', + authors: ['Flat Pattern Author'], + }, + targetLibraryId: destination.libraryId, + targetFolderId: destination.libraryFolderId, + }); + + const response = await context.app.inject({ + method: 'POST', + url: '/api/v1/book-bucket/finalize', + headers: authHeader(context.adminToken), + payload: { + fileIds: [bookBucketRow.id], + }, + }); + + expect(response.statusCode).toBe(201); + const body = response.json() as BookBucketFinalizeResult; + expect(body).toMatchObject({ total: 1, succeeded: 1, failed: 0 }); + + const finalizedBookId = body.results[0]!.bookId!; + const [book] = await context.db + .select({ + folderPath: schema.books.folderPath, + }) + .from(schema.books) + .where(eq(schema.books.id, finalizedBookId)) + .limit(1); + const [bookFile] = await context.db + .select({ + absolutePath: schema.bookFiles.absolutePath, + }) + .from(schema.bookFiles) + .where(eq(schema.bookFiles.bookId, finalizedBookId)) + .limit(1); + + expect(book?.folderPath).toBe(destination.folderPath); + expect(book?.folderPath).toBe(dirname(bookFile!.absolutePath)); + }); + + it('finalize treats title duplicates as exact matches instead of wildcard patterns', async () => { + const destination = await createLibraryWithFolder(context); + + const seedRow = await createBookBucketRow(context, { + fileName: 'seed-wildcard-duplicate.fb2', + selectedMetadata: { title: 'The Real Title', authors: ['Seed Author'] }, + targetLibraryId: destination.libraryId, + targetFolderId: destination.libraryFolderId, + }); + + const seedResponse = await context.app.inject({ + method: 'POST', + url: '/api/v1/book-bucket/finalize', + headers: authHeader(context.adminToken), + payload: { fileIds: [seedRow.id] }, + }); + expect(seedResponse.statusCode).toBe(201); + expect((seedResponse.json() as BookBucketFinalizeResult).results[0]?.success).toBe(true); + + const wildcardTitleRow = await createBookBucketRow(context, { + fileName: 'wildcard-title.fb2', + selectedMetadata: { title: 'The %', authors: ['Wildcard Author'] }, + targetLibraryId: destination.libraryId, + targetFolderId: destination.libraryFolderId, + }); + + const response = await context.app.inject({ + method: 'POST', + url: '/api/v1/book-bucket/finalize', + headers: authHeader(context.adminToken), + payload: { fileIds: [wildcardTitleRow.id] }, + }); + + expect(response.statusCode).toBe(201); + const body = response.json() as BookBucketFinalizeResult; + expect(body).toMatchObject({ total: 1, succeeded: 1, failed: 0 }); + expect(body.results[0]).toMatchObject({ fileId: wildcardTitleRow.id, success: true }); + expect(body.results[0]?.isDuplicate).toBeUndefined(); + }); + it('finalize selectAll honors status/search filters and excluded ids', async () => { const destination = await createLibraryWithFolder(context); @@ -586,6 +675,21 @@ describe('Book Bucket ingest + finalize (e2e)', () => { message: 'targetLibraryId and targetFolderId must both be set or both be null', }); + const finalizeValidationResponse = await context.app.inject({ + method: 'POST', + url: '/api/v1/book-bucket/finalize', + headers: authHeader(context.adminToken), + payload: { + fileIds: [123], + defaultLibraryId: 1, + }, + }); + + expect(finalizeValidationResponse.statusCode).toBe(400); + expect(finalizeValidationResponse.json()).toMatchObject({ + message: expect.arrayContaining(['defaultLibraryId and defaultFolderId must either both be provided or both be omitted']), + }); + const retriableErrorRow = await createBookBucketRow(context, { fileName: 'retry-fetch.fb2', status: 'error', diff --git a/server/test/e2e/app-harness.ts b/server/test/e2e/app-harness.ts index 00060f4e1..dcbaf7e28 100644 --- a/server/test/e2e/app-harness.ts +++ b/server/test/e2e/app-harness.ts @@ -1,7 +1,7 @@ import { randomUUID } from 'crypto'; import { expect } from 'vitest'; import fastifyCookie from '@fastify/cookie'; -import { count, eq, isNull, sql } from 'drizzle-orm'; +import { count, eq, inArray, isNull, sql } from 'drizzle-orm'; import type { NodePgDatabase } from 'drizzle-orm/node-postgres'; import { FastifyAdapter, type NestFastifyApplication } from '@nestjs/platform-fastify'; import { Test } from '@nestjs/testing'; @@ -27,7 +27,7 @@ function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } -function makeMetadataNoopMock(): Pick< +export function makeMetadataNoopMock(): Pick< MetadataService, 'extractAndSave' | 'refreshCoverForBook' | 'extractAudioFileDuration' | 'aggregateAudioDuration' | 'extractAudioChaptersAndNarrators' > { @@ -181,26 +181,30 @@ export async function loadLibraryBookState(db: Db, libraryId: number): Promise r.id); + const allFileRows = await db + .select({ bookId: bookFiles.bookId, id: bookFiles.id, absolutePath: bookFiles.absolutePath }) + .from(bookFiles) + .where(inArray(bookFiles.bookId, bookIds)); - let primaryPath: string | null = null; - if (row.primaryFileId !== null) { - const [primary] = await db.select({ absolutePath: bookFiles.absolutePath }).from(bookFiles).where(eq(bookFiles.id, row.primaryFileId)).limit(1); - primaryPath = primary?.absolutePath ?? null; - } - - states.push({ - folderPath: row.folderPath, - status: row.status, - primaryPath, - filePaths: fileRows.map((fileRow) => fileRow.absolutePath).sort(), - }); + const filesByBookId = new Map(); + const fileById = new Map(); + for (const f of allFileRows) { + if (!filesByBookId.has(f.bookId)) filesByBookId.set(f.bookId, []); + filesByBookId.get(f.bookId)!.push({ id: f.id, absolutePath: f.absolutePath }); + fileById.set(f.id, f.absolutePath); } - return states.sort((a, b) => a.folderPath.localeCompare(b.folderPath)); + return bookRows + .map((row) => ({ + folderPath: row.folderPath, + status: row.status, + primaryPath: row.primaryFileId !== null ? (fileById.get(row.primaryFileId) ?? null) : null, + filePaths: (filesByBookId.get(row.id) ?? []).map((f) => f.absolutePath).sort(), + })) + .sort((a, b) => a.folderPath.localeCompare(b.folderPath)); } export interface IntegritySnapshot { diff --git a/server/test/e2e/authorization-matrix/authorization-matrix-harness.ts b/server/test/e2e/authorization-matrix/authorization-matrix-harness.ts index b50268eac..ac81b3249 100644 --- a/server/test/e2e/authorization-matrix/authorization-matrix-harness.ts +++ b/server/test/e2e/authorization-matrix/authorization-matrix-harness.ts @@ -2,6 +2,7 @@ import { randomUUID } from 'crypto'; import { hash } from 'bcryptjs'; import fastifyCookie from '@fastify/cookie'; import fastifyMultipart from '@fastify/multipart'; +import { ValidationPipe } from '@nestjs/common'; import { Test } from '@nestjs/testing'; import { FastifyAdapter, type NestFastifyApplication } from '@nestjs/platform-fastify'; import { mkdir, stat } from 'fs/promises'; @@ -10,6 +11,7 @@ import type { NodePgDatabase } from 'drizzle-orm/node-postgres'; import type { Permission, BookBucketMetadata } from '@projectx/types'; import { AppModule } from '../../../src/app.module'; +import { GlobalExceptionFilter } from '../../../src/common/filters/http-exception.filter'; import { DB } from '../../../src/db'; import * as schema from '../../../src/db/schema'; import { MetadataService } from '../../../src/modules/metadata/metadata.service'; @@ -23,6 +25,7 @@ import { } from './authorization-matrix-fixture-builder'; type Db = NodePgDatabase; +const TEST_PASSWORD_HASH_ROUNDS = 4; const ADMIN_SETUP_DTO = { username: 'authorization-matrix-e2e-admin', @@ -114,6 +117,14 @@ export async function createAuthorizationMatrixE2EContext(): Promise(new FastifyAdapter()); app.setGlobalPrefix('api/v1'); + app.useGlobalPipes( + new ValidationPipe({ + whitelist: true, + forbidNonWhitelisted: true, + transform: true, + }), + ); + app.useGlobalFilters(new GlobalExceptionFilter()); await app.register(fastifyCookie as never); await app.register(fastifyMultipart as never); await app.init(); @@ -207,7 +218,7 @@ export async function createUserAndLogin( const username = options.username ?? `authorization-matrix-user-${suffix}`; const password = options.password ?? 'AuthorizationMatrixUser123'; const email = options.email ?? `${username}@example.com`; - const passwordHash = await hash(password, 12); + const passwordHash = await hash(password, TEST_PASSWORD_HASH_ROUNDS); const [created] = await ctx.db .insert(schema.users) @@ -370,7 +381,7 @@ export async function createOpdsUser( ): Promise<{ row: typeof schema.opdsUsers.$inferSelect; password: string }> { const username = input.username ?? `opds-${randomUUID().replaceAll('-', '').slice(0, 12)}`; const password = input.password ?? 'OpdsPassword123'; - const passwordHash = await hash(password, 12); + const passwordHash = await hash(password, TEST_PASSWORD_HASH_ROUNDS); const [row] = await ctx.db .insert(schema.opdsUsers) @@ -474,7 +485,7 @@ async function getAdminToken(app: NestFastifyApplication, db: Db): Promise row.name).sort(), narrators: narratorRows.map((row) => row.name), comicMetadata: comic ? { - issueNumber: comic.issueNumber ?? null, - volumeName: comic.volumeName ?? null, + issueNumber: comic.issueNumber, + volumeName: comic.volumeName, storyArcs: [...(comic.storyArcs ?? [])].sort(), } : null, diff --git a/server/test/e2e/metadata-write/metadata-write-fixture-builder.ts b/server/test/e2e/metadata-write/metadata-write-fixture-builder.ts index 9091c3cb5..3daa84b0b 100644 --- a/server/test/e2e/metadata-write/metadata-write-fixture-builder.ts +++ b/server/test/e2e/metadata-write/metadata-write-fixture-builder.ts @@ -3,7 +3,7 @@ import { randomUUID } from 'crypto'; import { createWriteStream } from 'fs'; import { mkdir, mkdtemp, rm, writeFile } from 'fs/promises'; import { tmpdir } from 'os'; -import { dirname, join } from 'path'; +import { dirname, isAbsolute, join, normalize } from 'path'; import { PDFDocument } from 'pdf-lib'; import { getSevenZip } from '../../../src/common/sevenzip'; @@ -33,9 +33,10 @@ interface ComicFixtureInput { year?: number; } -function assertRelativePath(path: string): void { - if (path.startsWith('/')) { - throw new Error(`Fixture paths must be relative. Received "${path}"`); +function assertRelativePath(relativePath: string): void { + const normalized = normalize(relativePath); + if (isAbsolute(relativePath) || normalized === '..' || normalized.startsWith('../') || normalized.startsWith('..\\')) { + throw new Error(`Fixture paths must be relative. Received "${relativePath}"`); } } diff --git a/server/test/e2e/metadata-write/metadata-write-harness.ts b/server/test/e2e/metadata-write/metadata-write-harness.ts index 6bf2567c7..592b13584 100644 --- a/server/test/e2e/metadata-write/metadata-write-harness.ts +++ b/server/test/e2e/metadata-write/metadata-write-harness.ts @@ -71,7 +71,7 @@ export async function createMetadataWriteE2EContext(): Promise; @@ -27,6 +27,7 @@ const ADMIN_SETUP_DTO = { password: 'UsersAdminLifecycleE2E123', }; const MAX_MULTIPART_BYTES = 20 * 1024 * 1024; +const BCRYPT_TEST_ROUNDS = 4; interface EnvSnapshot { booksPath: string | undefined; @@ -157,7 +158,7 @@ export async function createUserAndLogin( const username = options.username ?? `users-admin-lifecycle-${suffix}`; const password = options.password ?? 'UsersAdminLifecycleUser123'; const email = options.email ?? `${username}@example.com`; - const passwordHash = await hash(password, 12); + const passwordHash = await hash(password, BCRYPT_TEST_ROUNDS); const [created] = await ctx.db .insert(schema.users) @@ -201,7 +202,7 @@ export async function createOidcUser( const suffix = randomUUID().replaceAll('-', ''); const username = options.username ?? `users-admin-lifecycle-oidc-${suffix}`; const email = options.email ?? `${username}@example.com`; - const passwordHash = await hash(`OIDC_${suffix}`, 12); + const passwordHash = await hash(`OIDC_${suffix}`, BCRYPT_TEST_ROUNDS); const [created] = await ctx.db .insert(schema.users) @@ -263,7 +264,7 @@ async function getAdminToken(app: NestFastifyApplication, db: Db): Promise { - return { - extractAndSave: () => Promise.resolve(undefined), - refreshCoverForBook: () => Promise.resolve(false), - extractAudioFileDuration: () => Promise.resolve(undefined), - aggregateAudioDuration: () => Promise.resolve(undefined), - extractAudioChaptersAndNarrators: () => Promise.resolve(undefined), - }; -} - function restoreEnv(snapshot: EnvSnapshot): void { if (snapshot.booksPath === undefined) delete process.env.BOOKS_PATH; else process.env.BOOKS_PATH = snapshot.booksPath; diff --git a/server/test/email-lifecycle.e2e-spec.ts b/server/test/email-lifecycle.e2e-spec.ts index 7e833957e..2bf1c19ec 100644 --- a/server/test/email-lifecycle.e2e-spec.ts +++ b/server/test/email-lifecycle.e2e-spec.ts @@ -504,6 +504,19 @@ describe('Email lifecycle (e2e)', { timeout: 180_000 }, () => { defaultTemplateId: foreignTemplate.id, }); expectError(foreignTemplatePreferenceResponse, 403, 'No access to this template'); + + const recipientWithForeignTemplate = await apiAs(senderOnly, 'POST', '/api/v1/email/recipients', { + name: 'Recipient With Foreign Template', + email: 'recipient-with-foreign-template@example.com', + defaultTemplateId: foreignTemplate.id, + }); + expectError(recipientWithForeignTemplate, 403, 'No access to this template'); + + const groupWithForeignTemplate = await apiAs(senderOnly, 'POST', '/api/v1/email/recipient-groups', { + name: `foreign-template-group-${randomUUID().slice(0, 8)}`, + defaultTemplateId: foreignTemplate.id, + }); + expectError(groupWithForeignTemplate, 403, 'No access to this template'); }); }); @@ -565,6 +578,14 @@ describe('Email lifecycle (e2e)', { timeout: 180_000 }, () => { expect(fromHeader).toContain('ProjectX Bot '); expect(subjectHeader).toBe('Override Email Lifecycle Book'); expect(subjectHeader).not.toContain('Recipient'); + + const missingTemplateSendResponse = await apiAs(senderOnly, 'POST', '/api/v1/email/send', { + bookIds: [book.bookId], + recipientIds: [recipient.id], + providerId: provider.id, + templateId: 999_999_999, + }); + expectError(missingTemplateSendResponse, 404, 'Template not found'); }); it('keeps group defaultTemplateId non-operational in send resolution', async () => { diff --git a/server/test/metadata-lock.e2e-spec.ts b/server/test/metadata-lock.e2e-spec.ts index d5d9aaec3..15a388736 100644 --- a/server/test/metadata-lock.e2e-spec.ts +++ b/server/test/metadata-lock.e2e-spec.ts @@ -343,6 +343,7 @@ describe('metadata-lock e2e', { timeout: 30_000 }, () => { const lock = await patchLocks(initial.bookId, ['title']); expect(lock.statusCode).toBe(200); + // Filesystem timestamp resolution is 1s on many OSes; wait so the new file gets a newer mtime. await pause(1_100); await createEpubWithCoverFixture(library.folderPath, epubRelPath, { title: 'Scanner EPUB Winner', diff --git a/server/test/metadata-write.e2e-spec.ts b/server/test/metadata-write.e2e-spec.ts index 5e3a3ebea..c3dd6352d 100644 --- a/server/test/metadata-write.e2e-spec.ts +++ b/server/test/metadata-write.e2e-spec.ts @@ -6,6 +6,7 @@ import { Permission, type LibraryFileSyncProgressEvent } from '@projectx/types'; import * as schema from '../src/db/schema'; import { BookService, type MetadataUpdateFailpoint } from '../src/modules/book/book.service'; +import { FileWriteService } from '../src/modules/file-write/file-write.service'; import { extractCb7Metadata, extractCbzMetadata } from '../src/modules/metadata/lib/cbz-metadata'; import { extractEpubMetadata } from '../src/modules/metadata/lib/epub'; import { parsePdfFile } from '../src/modules/metadata/lib/pdf-parser'; @@ -203,9 +204,10 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () context = await createMetadataWriteE2EContext(); }); - afterEach((taskContext) => { + afterEach(async (taskContext) => { if (context) { context.app.get(BookService).clearMetadataUpdateFailpointForTests(); + await drainScheduledFileWrites(context); } const result = taskContext.task.result; @@ -231,7 +233,7 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () beforeEach(async () => { scenarioStartedAt = Date.now(); await setFileWriteSettings(context.db, { - enabled: true, + enabled: false, writeCover: false, epub: { enabled: true }, pdf: { enabled: true }, @@ -441,6 +443,8 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () describe('auto-write on metadata patch', () => { it('writes PDF metadata to disk and records auto write-log entry', async () => { + await setFileWriteSettings(context.db, { enabled: true }); + const library = await createLibraryWithFolder(context, { mode: 'book_per_file' }); await createPdfFixture(library.folderPath, 'auto-write/book.pdf', 'Auto Seed PDF'); await triggerAndWaitForLibraryScan(context, library.libraryId); @@ -466,7 +470,7 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () }); expect(logEntry.format).toBe('pdf'); - const parsedPdf = await parsePdfFile(book.absolutePath); + const parsedPdf = await parsePdfFile(book.absolutePath, { extractCover: false }); expect(parsedPdf?.title).toBe('Auto PDF Title'); expect(parsedPdf?.authors.map((author) => author.name)).toContain('Auto PDF Author'); expect(parsedPdf?.description).toBe('Auto PDF Description'); @@ -480,6 +484,8 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () }); it('writes EPUB metadata to disk and records auto write-log entry', async () => { + await setFileWriteSettings(context.db, { enabled: true }); + const library = await createLibraryWithFolder(context, { mode: 'book_per_file' }); await createEpubFixture(library.folderPath, 'auto-write/book.epub', { title: 'Auto Seed EPUB' }); await triggerAndWaitForLibraryScan(context, library.libraryId); @@ -541,7 +547,7 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () expect(patchResponse.statusCode).toBe(200); } - await sleep(250); + await drainScheduledFileWrites(context); await setFileWriteSettings(context.db, { enabled: true, @@ -574,7 +580,7 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () } } - const parsedPdf = await parsePdfFile(prepared.books.pdf.absolutePath); + const parsedPdf = await parsePdfFile(prepared.books.pdf.absolutePath, { extractCover: false }); expect(parsedPdf).toMatchObject({ title: MATRIX_SET_PAYLOAD.title, subtitle: MATRIX_SET_PAYLOAD.subtitle, @@ -763,7 +769,7 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () expect(tagRows).toHaveLength(0); } - await sleep(250); + await drainScheduledFileWrites(context); await setFileWriteSettings(context.db, { enabled: true, @@ -784,7 +790,7 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () ); expect(doneEvent).toMatchObject({ processed: 4, succeeded: 4, failed: 0, skipped: 0 }); - const parsedPdf = await parsePdfFile(prepared.books.pdf.absolutePath); + const parsedPdf = await parsePdfFile(prepared.books.pdf.absolutePath, { extractCover: false }); expect(parsedPdf).toMatchObject({ title: null, subtitle: null, @@ -903,7 +909,7 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () }); expect(patchResponse.statusCode).toBe(200); - await sleep(250); + await drainScheduledFileWrites(context); await setFileWriteSettings(context.db, { enabled: true }); @@ -979,7 +985,7 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () expect(patchResponse.statusCode).toBe(200); } - await sleep(250); + await drainScheduledFileWrites(context); await setFileWriteSettings(context.db, { enabled: true, cbx: { enabled: true, formats: ['cbz', 'cb7'] } }); @@ -1001,13 +1007,15 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () expect(successLog.errorMessage ?? null).toBeNull(); expect(failureLog.errorMessage).toBeTruthy(); - const parsedPdf = await parsePdfFile(okBook.absolutePath); + const parsedPdf = await parsePdfFile(okBook.absolutePath, { extractCover: false }); expect(parsedPdf?.title).toBe('Failure Isolation Success'); }); }); describe('concurrency hardening', () => { it('resolves parallel PATCH conflicts to one complete payload shape', async () => { + await setFileWriteSettings(context.db, { enabled: true }); + const library = await createLibraryWithFolder(context, { mode: 'book_per_file' }); await createPdfFixture(library.folderPath, 'concurrency/parallel-patch.pdf', 'Parallel Patch Seed'); await triggerAndWaitForLibraryScan(context, library.libraryId); @@ -1089,7 +1097,7 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () const winningPayload = matchesBookWriteState(dbState, payloadA) ? payloadA : matchesBookWriteState(dbState, payloadB) ? payloadB : null; expect(winningPayload).not.toBeNull(); - const parsedPdf = await parsePdfFile(book.absolutePath); + const parsedPdf = await parsePdfFile(book.absolutePath, { extractCover: false }); expect(parsedPdf).toMatchObject({ title: winningPayload!.title, subtitle: winningPayload!.subtitle, @@ -1139,7 +1147,7 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () expect(seedResponse.statusCode).toBe(200); } - await sleep(250); + await drainScheduledFileWrites(context); await setFileWriteSettings(context.db, { enabled: true, @@ -1173,23 +1181,58 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () itunesId: 'overlap-itunes', } as const; - const syncPromise = context.app.inject({ - method: 'POST', - url: `/api/v1/libraries/${library.libraryId}/write-metadata-to-files`, - headers: authHeader(context.adminToken), + const fileWriteService = context.app.get(FileWriteService); + const originalWriteToFile = fileWriteService.writeToFile.bind(fileWriteService); + let releaseFirstSyncWrite!: () => void; + let resolveFirstSyncWriteStarted!: () => void; + const firstSyncWriteStarted = new Promise((resolve) => { + resolveFirstSyncWriteStarted = resolve; }); - - await sleep(75); - - const patchResponse = await context.app.inject({ - method: 'PATCH', - url: `/api/v1/books/${targetBook.bookId}/metadata`, - headers: authHeader(context.adminToken), - payload: overlapPayload, + const releaseFirstSyncWritePromise = new Promise((resolve) => { + releaseFirstSyncWrite = resolve; }); - expect(patchResponse.statusCode).toBe(200); + let blockedFirstSyncWrite = false; + let releasedFirstSyncWrite = false; + + const writeSpy = vi + .spyOn(fileWriteService, 'writeToFile') + .mockImplementation(async (bookId: number, triggeredBy: 'auto' | 'sync', userId?: number, dryRun?: boolean) => { + if (triggeredBy === 'sync' && !blockedFirstSyncWrite) { + blockedFirstSyncWrite = true; + resolveFirstSyncWriteStarted(); + await releaseFirstSyncWritePromise; + } + return originalWriteToFile(bookId, triggeredBy, userId, dryRun); + }); + + let syncResponse!: Awaited>; + try { + const syncPromise = context.app.inject({ + method: 'POST', + url: `/api/v1/libraries/${library.libraryId}/write-metadata-to-files`, + headers: authHeader(context.adminToken), + }); + + await firstSyncWriteStarted; + + const patchResponse = await context.app.inject({ + method: 'PATCH', + url: `/api/v1/books/${targetBook.bookId}/metadata`, + headers: authHeader(context.adminToken), + payload: overlapPayload, + }); + expect(patchResponse.statusCode).toBe(200); + + releasedFirstSyncWrite = true; + releaseFirstSyncWrite(); + syncResponse = await syncPromise; + } finally { + if (blockedFirstSyncWrite && !releasedFirstSyncWrite) { + releaseFirstSyncWrite(); + } + writeSpy.mockRestore(); + } - const syncResponse = await syncPromise; expect(syncResponse.statusCode).toBe(200); const syncEvents = parseSseEvents(syncResponse.body); const doneEvent = syncEvents.find( @@ -1206,7 +1249,7 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () const targetState = await readBookWriteState(context, targetBook.bookId); expect(matchesBookWriteState(targetState, overlapPayload)).toBe(true); - const parsedPdf = await parsePdfFile(targetBook.absolutePath); + const parsedPdf = await parsePdfFile(targetBook.absolutePath, { extractCover: false }); expect(parsedPdf).toMatchObject({ title: overlapPayload.title, subtitle: overlapPayload.subtitle, @@ -1252,7 +1295,7 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () expect(patchResponse.statusCode).toBe(200); } - await sleep(250); + await drainScheduledFileWrites(context); await setFileWriteSettings(context.db, { enabled: true, @@ -1334,6 +1377,19 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () expect(success.statusCode).toBe(200); }); + it('returns 404 for sync requests targeting a missing library', async () => { + await setFileWriteSettings(context.db, { enabled: true }); + + const response = await context.app.inject({ + method: 'POST', + url: '/api/v1/libraries/999999/write-metadata-to-files', + headers: authHeader(context.adminToken), + }); + + expect(response.statusCode).toBe(404); + expect(response.json()).toMatchObject({ message: 'Library not found' }); + }); + it('enforces permission and editor-level library access for sync endpoint', async () => { const library = await createLibraryWithFolder(context, { mode: 'book_per_file' }); await createPdfFixture(library.folderPath, 'access/sync.pdf'); @@ -1369,6 +1425,7 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () expect(viewerAccess.statusCode).toBe(403); await grantLibraryAccess(context, withPermissionUser.userId, library.libraryId, 'editor'); + await setFileWriteSettings(context.db, { enabled: true }); const editorAccess = await context.app.inject({ method: 'POST', url: `/api/v1/libraries/${library.libraryId}/write-metadata-to-files`, @@ -1427,6 +1484,10 @@ function parseSseEvents(rawBody: string): LibraryFileSyncProgressEvent[] { return events; } +async function drainScheduledFileWrites(context: MetadataWriteE2EContext): Promise { + await context.app.get(FileWriteService).drainScheduledWritesForTests(); +} + function buildAtomicityFailurePayload(failpoint: MetadataUpdateFailpoint) { const token = failpoint .replace(/[^a-z]/gi, '') @@ -1646,7 +1707,3 @@ async function countSyncSuccessLogs(context: MetadataWriteE2EContext, bookId: nu return rows.filter((row) => row.triggeredBy === 'sync' && row.status === 'success').length; } - -function sleep(ms: number): Promise { - return new Promise((resolve) => setTimeout(resolve, ms)); -} diff --git a/server/test/reader-state-isolation.e2e-spec.ts b/server/test/reader-state-isolation.e2e-spec.ts index 819f05d8d..b55738006 100644 --- a/server/test/reader-state-isolation.e2e-spec.ts +++ b/server/test/reader-state-isolation.e2e-spec.ts @@ -231,6 +231,17 @@ describe('Reader state isolation (e2e)', { timeout: 120_000 }, () => { describe('reader preferences ownership', () => { it('isolates per-book and default reader preferences per user', async () => { + const preCustomizationResponse = await ctx.app.inject({ + method: 'GET', + url: `/api/v1/reader/preferences/${sharedEpub.bookFileId}`, + headers: authHeader(userA.accessToken), + }); + expect(preCustomizationResponse.statusCode).toBe(200); + expect(preCustomizationResponse.json()).toEqual({ + settings: null, + isCustomized: false, + }); + const userASettings = { themeName: 'dark', fontSize: 20, @@ -539,6 +550,13 @@ describe('Reader state isolation (e2e)', { timeout: 120_000 }, () => { }); expect(bookmarkResponse.statusCode).toBe(403); + const annotationResponse = await ctx.app.inject({ + method: 'GET', + url: `/api/v1/books/${sharedEpub.bookId}/annotations`, + headers: authHeader(outsider.accessToken), + }); + expect(annotationResponse.statusCode).toBe(403); + const preferenceResponse = await ctx.app.inject({ method: 'GET', url: `/api/v1/reader/preferences/${sharedEpub.bookFileId}`, @@ -664,13 +682,15 @@ describe('Reader state isolation (e2e)', { timeout: 120_000 }, () => { }); async function resetReaderState(ctx: ReaderStateIsolationE2EContext): Promise { - await ctx.db.delete(schema.readingSessions); - await ctx.db.delete(schema.userReadingDailyStats); - await ctx.db.delete(schema.readingProgress); - await ctx.db.delete(schema.audiobookProgress); - await ctx.db.delete(schema.bookmarks); - await ctx.db.delete(schema.annotations); - await ctx.db.delete(schema.readerPreferences); - await ctx.db.delete(schema.readerDefaultPreferences); - await ctx.db.delete(schema.userBookStatus); + await Promise.all([ + ctx.db.delete(schema.readingSessions), + ctx.db.delete(schema.userReadingDailyStats), + ctx.db.delete(schema.readingProgress), + ctx.db.delete(schema.audiobookProgress), + ctx.db.delete(schema.bookmarks), + ctx.db.delete(schema.annotations), + ctx.db.delete(schema.readerPreferences), + ctx.db.delete(schema.readerDefaultPreferences), + ctx.db.delete(schema.userBookStatus), + ]); } diff --git a/server/test/scanner-file-operations.e2e-spec.ts b/server/test/scanner-file-operations.e2e-spec.ts index 3f40bb98b..e5170bb93 100644 --- a/server/test/scanner-file-operations.e2e-spec.ts +++ b/server/test/scanner-file-operations.e2e-spec.ts @@ -468,6 +468,7 @@ const structuralScenarios: StructuralScenario[] = [ statusByFolder: { Shelf: 'present' }, fileOwners: { 'Shelf/book.epub': 'Shelf' }, presentCount: 1, + missingCount: 0, }, }, }, @@ -482,6 +483,7 @@ const structuralScenarios: StructuralScenario[] = [ statusByFolder: { 'book.epub': 'present' }, fileOwners: { 'book.epub': 'book.epub' }, presentCount: 1, + missingCount: 0, }, }, }, diff --git a/server/test/users-admin-lifecycle.e2e-spec.ts b/server/test/users-admin-lifecycle.e2e-spec.ts index e9219b6dd..dfacc67d7 100644 --- a/server/test/users-admin-lifecycle.e2e-spec.ts +++ b/server/test/users-admin-lifecycle.e2e-spec.ts @@ -264,7 +264,7 @@ describe('Users admin lifecycle (e2e)', { timeout: 180_000 }, () => { }); expect(readAfterPermissionUpdate.statusCode).toBe(200); const afterPermissionUpdateBody = readAfterPermissionUpdate.json() as { permissions: Permission[] }; - expect(afterPermissionUpdateBody.permissions).toEqual([PermissionEnum.LibraryDownload, PermissionEnum.KoboSync]); + expect(afterPermissionUpdateBody.permissions.toSorted()).toEqual([PermissionEnum.LibraryDownload, PermissionEnum.KoboSync].toSorted()); const invalidPermissionUpdate = await ctx.app.inject({ method: 'PUT', @@ -310,6 +310,66 @@ describe('Users admin lifecycle (e2e)', { timeout: 180_000 }, () => { }); expectError(readAfterDelete, 404, 'User not found'); }); + + it('rejects duplicate username on create', async () => { + const response = await ctx.app.inject({ + method: 'POST', + url: '/api/v1/users', + headers: authHeader(manageUsersAdmin.accessToken), + payload: { + username: regularA.username, + name: 'Duplicate Username User', + email: `dup-username-${randomUUID().slice(0, 8)}@example.com`, + }, + }); + expectError(response, 409, 'Username already taken'); + }); + + it('returns 401 for unauthenticated requests to admin routes', async () => { + const cases: Array<{ + method: 'GET' | 'POST' | 'PATCH' | 'PUT' | 'DELETE'; + url: string; + payload?: Record; + }> = [ + { method: 'GET', url: '/api/v1/users' }, + { method: 'POST', url: '/api/v1/users', payload: { username: 'noauth', name: 'No Auth', email: 'noauth@test.com' } }, + { method: 'GET', url: `/api/v1/users/${regularA.userId}` }, + { method: 'PATCH', url: `/api/v1/users/${regularA.userId}`, payload: { name: 'hacked' } }, + { method: 'DELETE', url: `/api/v1/users/${regularA.userId}` }, + ]; + + for (const entry of cases) { + const response = await ctx.app.inject({ + method: entry.method, + url: entry.url, + ...(entry.payload ? { payload: entry.payload } : {}), + }); + expect(response.statusCode).toBe(401); + } + }); + it('returns 403 for authenticated users lacking manage_users permission', async () => { + const cases: Array<{ + method: 'GET' | 'POST' | 'PATCH' | 'PUT' | 'DELETE'; + url: string; + payload?: Record; + }> = [ + { method: 'GET', url: '/api/v1/users' }, + { method: 'POST', url: '/api/v1/users', payload: { username: 'noperm', name: 'No Perm', email: 'noperm@test.com' } }, + { method: 'GET', url: `/api/v1/users/${regularA.userId}` }, + { method: 'PATCH', url: `/api/v1/users/${regularA.userId}`, payload: { name: 'hacked' } }, + { method: 'DELETE', url: `/api/v1/users/${regularA.userId}` }, + ]; + + for (const entry of cases) { + const response = await ctx.app.inject({ + method: entry.method, + url: entry.url, + headers: authHeader(regularB.accessToken), + ...(entry.payload ? { payload: entry.payload } : {}), + }); + expectError(response, 403, 'Missing permission: manage_users'); + } + }); }); describe('protection rules', () => { @@ -421,6 +481,16 @@ describe('Users admin lifecycle (e2e)', { timeout: 180_000 }, () => { expect(readAfterDemote.statusCode).toBe(200); expect((readAfterDemote.json() as { isSuperuser: boolean }).isSuperuser).toBe(false); }); + + it('blocks self-superuser toggle by a superuser', async () => { + const selfToggle = await ctx.app.inject({ + method: 'PUT', + url: `/api/v1/users/${superuserTarget.userId}/superuser`, + headers: authHeader(superuserTarget.accessToken), + payload: { isSuperuser: false }, + }); + expectError(selfToggle, 409, 'You cannot change your own superuser status'); + }); }); describe('profile and avatar behavior', () => { @@ -569,7 +639,7 @@ describe('Users admin lifecycle (e2e)', { timeout: 180_000 }, () => { url: `/api/v1/users/${created.id}/reset-password`, headers: authHeader(manageUsersAdmin.accessToken), }); - expect([200, 201]).toContain(secondResetResponse.statusCode); + expect(secondResetResponse.statusCode).toBe(201); const secondToken = parseTokenFromResetUrl((secondResetResponse.json() as { resetUrl: string }).resetUrl); const reuseFirstToken = await ctx.app.inject({ @@ -654,6 +724,18 @@ describe('Users admin lifecycle (e2e)', { timeout: 180_000 }, () => { }); expect(sessionsAfterChange.statusCode).toBe(200); }); + + it('rejects login for inactive users', async () => { + const inactiveLogin = await ctx.app.inject({ + method: 'POST', + url: '/api/v1/auth/login', + payload: { + username: inactiveAssignable.username, + password: inactiveAssignable.password, + }, + }); + expect(inactiveLogin.statusCode).toBe(401); + }); }); describe('assignable users contract', () => {