mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(auth): enforce library roles for metadata editing
Library access roles had no effect on metadata editing: only the global Edit metadata permission was checked, so a Viewer could edit every library they could see and an Editor could not be limited to one library. Require Editor or Owner on the book's library for every metadata write, including bulk edits, locks, covers, file renames, entity manager changes and author edits, and hide the matching actions in the web client. Keep existing access levels when an admin saves a user's library list. The old replace reset every row to Viewer, which would now silently revoke editing on each user save. Existing grants are Viewer, so after upgrading, admins must set Editor on each library where a non-superuser should keep editing metadata. The released iOS app already gates editing on Editor. Closes #1778
This commit is contained in:
@@ -65,6 +65,7 @@ describe('AuthorsService', () => {
|
||||
const libraryService = {
|
||||
findAll: vi.fn(),
|
||||
findAccessibleLibraryIds: vi.fn(),
|
||||
findEditableLibraryIds: vi.fn(),
|
||||
};
|
||||
|
||||
const authorMetadataFetchService = {
|
||||
@@ -119,6 +120,7 @@ describe('AuthorsService', () => {
|
||||
);
|
||||
libraryService.findAll.mockResolvedValue([{ id: 1 }, { id: 2 }]);
|
||||
libraryService.findAccessibleLibraryIds.mockResolvedValue([1, 2]);
|
||||
libraryService.findEditableLibraryIds.mockResolvedValue([1, 2]);
|
||||
authorImageStorage.getThumbnailUrlIfExists.mockResolvedValue(null);
|
||||
authorImageStorage.getImageUrlIfExists.mockResolvedValue(null);
|
||||
enrichmentOrchestrator.schedule.mockResolvedValue(1);
|
||||
@@ -198,6 +200,15 @@ describe('AuthorsService', () => {
|
||||
expect(result.affectedBookCount).toBe(8);
|
||||
});
|
||||
|
||||
it('update rejects authors whose books sit in a library the user can only read', async () => {
|
||||
authorsRepo.findVisibleAuthorIds.mockResolvedValue([20]);
|
||||
authorsRepo.findRelatedLibraryIds.mockResolvedValue([1, 2]);
|
||||
libraryService.findEditableLibraryIds.mockResolvedValue([1]);
|
||||
|
||||
await expect(service.update(reqUser(), 20, { name: 'Renamed' })).rejects.toBeInstanceOf(ForbiddenException);
|
||||
expect(authorsRepo.updateAuthorById).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('update normalizes author names and blank optional fields', async () => {
|
||||
authorsRepo.findVisibleAuthorIds.mockResolvedValue([20]);
|
||||
authorsRepo.findRelatedLibraryIds.mockResolvedValue([1]);
|
||||
|
||||
@@ -649,7 +649,7 @@ export class AuthorsService {
|
||||
}
|
||||
|
||||
private async assertMutationAccess(user: RequestUser, authorIds: number[]) {
|
||||
const libraryIds = await this.resolveLibraryIds(user);
|
||||
const [libraryIds, editableLibraryIds] = await Promise.all([this.resolveLibraryIds(user), this.libraryService.findEditableLibraryIds(user)]);
|
||||
const [visible, relatedLibraryIds] = await Promise.all([
|
||||
this.authorsRepo.findVisibleAuthorIds(authorIds, libraryIds),
|
||||
this.authorsRepo.findRelatedLibraryIds(authorIds),
|
||||
@@ -657,8 +657,8 @@ export class AuthorsService {
|
||||
if (visible.length !== authorIds.length) {
|
||||
throw new NotFoundException('Author not found');
|
||||
}
|
||||
const accessibleSet = new Set(libraryIds);
|
||||
if (relatedLibraryIds.some((libraryId) => !accessibleSet.has(libraryId))) {
|
||||
const editableSet = new Set(editableLibraryIds);
|
||||
if (relatedLibraryIds.some((libraryId) => !editableSet.has(libraryId))) {
|
||||
throw new ForbiddenException('Insufficient library access to mutate one or more selected authors');
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,310 @@
|
||||
import { ForbiddenException, NotFoundException } from '@nestjs/common';
|
||||
import { ConfigService } from '@nestjs/config';
|
||||
import { Test } from '@nestjs/testing';
|
||||
import { tmpdir } from 'os';
|
||||
import { EMPTY_CONTENT_FILTER_RULES, Permission, type AccessLevel } from '@bookorbit/types';
|
||||
import type { RequestUser } from '../../common/types/request-user';
|
||||
import { LibraryRepository } from '../library/library.repository';
|
||||
import { LibraryService } from '../library/library.service';
|
||||
import { MetadataService } from '../metadata/metadata.service';
|
||||
import { MetadataFetchPipeline } from '../metadata-fetch/metadata-fetch-pipeline';
|
||||
import { BookMetadataLockService } from '../book-metadata-lock/book-metadata-lock.service';
|
||||
import { BookCoverStore } from '../book-cover-store/book-cover-store.service';
|
||||
import { FileWriteService } from '../file-write/file-write.service';
|
||||
import { FileRenameService } from '../file-write/file-rename.service';
|
||||
import { CoverService } from '../cover/cover.service';
|
||||
import { BulkEditFieldsDto } from './dto/bulk-edit-metadata.dto';
|
||||
import { BookRepository } from './book.repository';
|
||||
import { BookReadService } from './book-read.service';
|
||||
import { BookService } from './book.service';
|
||||
import { BookEmbedderService } from '../embedding/book-embedder.service';
|
||||
import { MetadataScoreService } from '../metadata-score/metadata-score.service';
|
||||
import { AchievementEventsService } from '../achievement/achievement-events.service';
|
||||
import { AudiobookEbookProgressSyncService } from './audiobook-ebook-progress-sync.service';
|
||||
import { UserBookStatusService } from '../user-book-status/user-book-status.service';
|
||||
|
||||
const user: RequestUser = {
|
||||
id: 11,
|
||||
username: 'metadata-editor',
|
||||
name: 'Metadata Editor',
|
||||
email: null,
|
||||
active: true,
|
||||
isSuperuser: false,
|
||||
isDefaultPassword: false,
|
||||
tokenVersion: 1,
|
||||
settings: {},
|
||||
avatarUrl: null,
|
||||
provisioningMethod: 'local',
|
||||
permissions: [Permission.LibraryEditMetadata],
|
||||
contentFilters: EMPTY_CONTENT_FILTER_RULES,
|
||||
};
|
||||
|
||||
type Context = Awaited<ReturnType<typeof createContext>>;
|
||||
|
||||
async function createContext(role: AccessLevel | null = 'viewer') {
|
||||
const roles = new Map<number, AccessLevel | null>([
|
||||
[7, role],
|
||||
[8, 'editor'],
|
||||
[9, 'owner'],
|
||||
]);
|
||||
const bookLibraries = new Map([
|
||||
[1, 7],
|
||||
[2, 8],
|
||||
[3, 9],
|
||||
]);
|
||||
const libraryRepo = {
|
||||
hasUserAccess: vi.fn((_userId: number, libraryId: number) => Boolean(roles.get(libraryId))),
|
||||
findUserAccessLevel: vi.fn((_userId: number, libraryId: number) => roles.get(libraryId) ?? null),
|
||||
};
|
||||
const bookRepo = {
|
||||
findLibraryIdByBookId: vi.fn((id: number) => bookLibraries.get(id) ?? null),
|
||||
findLibraryIdsByBookIds: vi.fn((ids: number[]) =>
|
||||
ids.filter((id) => bookLibraries.has(id)).map((id) => ({ id, libraryId: bookLibraries.get(id)! })),
|
||||
),
|
||||
findById: vi.fn((id: number) => {
|
||||
const libraryId = bookLibraries.get(id);
|
||||
return libraryId === undefined
|
||||
? null
|
||||
: {
|
||||
book: { books: { id, libraryId }, book_metadata: {} },
|
||||
authorRows: [],
|
||||
genreRows: [],
|
||||
communityRatingRows: [],
|
||||
fileRows: [],
|
||||
narratorRows: [],
|
||||
};
|
||||
}),
|
||||
checkBookPassesContentFilters: vi.fn().mockResolvedValue(true),
|
||||
withTransaction: vi.fn(async (callback: (tx: unknown) => Promise<unknown>) => callback({})),
|
||||
updateMetadataFields: vi.fn(),
|
||||
updateAddedAt: vi.fn(),
|
||||
bulkUpdateMetadataFields: vi.fn(),
|
||||
bulkSetRating: vi.fn(),
|
||||
findCoverSourceFilesByBookIds: vi.fn().mockResolvedValue([]),
|
||||
findProgress: vi.fn().mockResolvedValue(null),
|
||||
upsertProgress: vi.fn(),
|
||||
findFileById: vi.fn().mockResolvedValue({ id: 10, bookId: 1, libraryId: 7 }),
|
||||
findProgressByBook: vi.fn().mockResolvedValue(0),
|
||||
};
|
||||
const metadata = { replaceAuthors: vi.fn(), replaceTags: vi.fn(), replaceGenres: vi.fn(), downloadAndSaveCover: vi.fn() };
|
||||
const locks = {
|
||||
assertManualUpdateAllowed: vi.fn(),
|
||||
assertFieldsUnlocked: vi.fn(),
|
||||
replaceLockedFields: vi.fn().mockResolvedValue([]),
|
||||
bulkReplaceLockedFields: vi.fn(),
|
||||
getBookIdsWithLockedField: vi.fn().mockResolvedValue(new Set()),
|
||||
getLockedFieldsMap: vi.fn().mockResolvedValue(new Map()),
|
||||
};
|
||||
const pipeline = { run: vi.fn(), runWithSources: vi.fn() };
|
||||
const coverStore = { saveCustom: vi.fn(), revert: vi.fn(), chooseWriteMedium: vi.fn().mockResolvedValue('ebook') };
|
||||
const fileWrite = {
|
||||
scheduleWrite: vi.fn(),
|
||||
writeToFile: vi.fn(),
|
||||
cancelPendingWrite: vi.fn(),
|
||||
findLibraryWriteSettingsForBook: vi.fn().mockResolvedValue({ fileWriteEnabled: false, fileRenameEnabled: false }),
|
||||
};
|
||||
const fileRename = { scheduleRename: vi.fn(), cancelPendingRename: vi.fn(), performRename: vi.fn() };
|
||||
const score = { calculateAndSave: vi.fn(), calculateAndSaveMany: vi.fn() };
|
||||
const mocks = new Map<unknown, unknown>([
|
||||
[BookEmbedderService, { embedBook: vi.fn().mockResolvedValue(undefined) }],
|
||||
[MetadataScoreService, score],
|
||||
[AchievementEventsService, { emit: vi.fn() }],
|
||||
[AudiobookEbookProgressSyncService, { syncFromEbookProgress: vi.fn() }],
|
||||
[UserBookStatusService, { autoUpdate: vi.fn(), findOne: vi.fn() }],
|
||||
[ConfigService, { get: vi.fn().mockReturnValue(tmpdir()) }],
|
||||
[LibraryRepository, libraryRepo],
|
||||
[BookRepository, bookRepo],
|
||||
[BookReadService, bookRepo],
|
||||
[MetadataService, metadata],
|
||||
[BookMetadataLockService, locks],
|
||||
[MetadataFetchPipeline, pipeline],
|
||||
[BookCoverStore, coverStore],
|
||||
[FileWriteService, fileWrite],
|
||||
[FileRenameService, fileRename],
|
||||
]);
|
||||
const module = await Test.createTestingModule({ providers: [BookService, LibraryService, CoverService] })
|
||||
.useMocker((token) => mocks.get(token) ?? {})
|
||||
.compile();
|
||||
const service = module.get(BookService);
|
||||
vi.spyOn(service, 'getDetail').mockResolvedValue({ id: 1 } as never);
|
||||
return {
|
||||
module,
|
||||
service,
|
||||
covers: module.get(CoverService),
|
||||
roles,
|
||||
bookRepo,
|
||||
libraryRepo,
|
||||
metadata,
|
||||
locks,
|
||||
pipeline,
|
||||
coverStore,
|
||||
fileWrite,
|
||||
fileRename,
|
||||
score,
|
||||
};
|
||||
}
|
||||
|
||||
const singleWrites: { name: string; run: (ctx: Context, caller: RequestUser) => Promise<unknown> }[] = [
|
||||
{ name: 'metadata', run: ({ service }, caller) => service.updateMetadata(1, { title: 'Changed' }, caller) },
|
||||
{
|
||||
name: 'metadata and locks',
|
||||
run: ({ service }, caller) => service.updateMetadataAndLocks(1, { metadata: { title: 'Changed' }, lockedFields: [] }, caller),
|
||||
},
|
||||
{ name: 'locks only', run: ({ service }, caller) => service.updateMetadataLocks(1, [], caller) },
|
||||
{ name: 'added date', run: ({ service }, caller) => service.updateAddedAt(1, { addedAt: '2025-01-01' }, caller) },
|
||||
{ name: 'write and rename', run: ({ service }, caller) => service.writeAndRename(1, caller) },
|
||||
{ name: 'file rename', run: ({ service }, caller) => service.renameFile(10, { filename: 'renamed.epub' }, caller) },
|
||||
{ name: 'metadata refresh', run: ({ service }, caller) => service.refreshMetadata(1, false, caller) },
|
||||
{ name: 'cover regeneration', run: ({ service }, caller) => service.reExtractCover(1, caller) },
|
||||
{ name: 'cover upload', run: ({ covers }, caller) => covers.uploadCover(1, Buffer.from('image'), 'image/png', caller) },
|
||||
{ name: 'cover URL upload', run: ({ covers }, caller) => covers.uploadCoverFromUrl(1, 'https://example.com/cover.png', caller) },
|
||||
{ name: 'cover deletion', run: ({ covers }, caller) => covers.deleteCover(1, caller) },
|
||||
];
|
||||
|
||||
const bulkWrites: { name: string; run: (ctx: Context, ids: number[]) => Promise<unknown> }[] = [
|
||||
{ name: 'set metadata', run: ({ service }, ids) => service.bulkSetMetadata(ids, 'publisher', 'Changed', user) },
|
||||
{ name: 'update tags', run: ({ service }, ids) => service.bulkUpdateTags(ids, 'replace', ['Changed'], user) },
|
||||
{ name: 'set locks', run: ({ service }, ids) => service.bulkSetMetadataLock(ids, true, user) },
|
||||
{
|
||||
name: 'edit metadata',
|
||||
run: ({ service }, ids) => service.bulkEditMetadata(ids, Object.assign(new BulkEditFieldsDto(), { publisher: { value: 'Changed' } }), user),
|
||||
},
|
||||
{ name: 'refresh metadata', run: ({ service }, ids) => service.bulkRefreshMetadata(ids, user) },
|
||||
{ name: 'regenerate covers', run: ({ service }, ids) => service.bulkReExtractCover(ids, user) },
|
||||
];
|
||||
|
||||
function expectNoWrites(ctx: Context) {
|
||||
for (const mock of [
|
||||
ctx.bookRepo.withTransaction,
|
||||
ctx.bookRepo.updateMetadataFields,
|
||||
ctx.bookRepo.updateAddedAt,
|
||||
ctx.bookRepo.bulkUpdateMetadataFields,
|
||||
ctx.metadata.replaceAuthors,
|
||||
ctx.metadata.replaceTags,
|
||||
ctx.metadata.replaceGenres,
|
||||
ctx.metadata.downloadAndSaveCover,
|
||||
ctx.locks.replaceLockedFields,
|
||||
ctx.locks.bulkReplaceLockedFields,
|
||||
ctx.pipeline.run,
|
||||
ctx.pipeline.runWithSources,
|
||||
ctx.coverStore.saveCustom,
|
||||
ctx.coverStore.revert,
|
||||
ctx.fileWrite.scheduleWrite,
|
||||
ctx.fileWrite.writeToFile,
|
||||
ctx.fileWrite.cancelPendingWrite,
|
||||
ctx.fileRename.cancelPendingRename,
|
||||
ctx.fileRename.scheduleRename,
|
||||
ctx.fileRename.performRename,
|
||||
])
|
||||
expect(mock).not.toHaveBeenCalled();
|
||||
}
|
||||
|
||||
describe('library roles for book metadata writes', () => {
|
||||
const modules: Context['module'][] = [];
|
||||
async function context(role: AccessLevel | null = 'viewer') {
|
||||
const ctx = await createContext(role);
|
||||
modules.push(ctx.module);
|
||||
return ctx;
|
||||
}
|
||||
afterEach(async () => {
|
||||
await Promise.all(modules.splice(0).map((module) => module.close()));
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it.each(singleWrites)('rejects a viewer before $name has side effects', async ({ run }) => {
|
||||
const ctx = await context();
|
||||
await expect(run(ctx, user)).rejects.toThrow(ForbiddenException);
|
||||
expectNoWrites(ctx);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ role: 'viewer' as const, ids: [1], rating: 4 },
|
||||
{ role: 'editor' as const, ids: [1], rating: null },
|
||||
{ role: 'owner' as const, ids: [1], rating: 5 },
|
||||
{ role: 'viewer' as const, ids: [1, 2, 3], rating: 3 },
|
||||
])('keeps personal ratings separate from shared metadata effects for $role and $ids', async ({ role, ids, rating }) => {
|
||||
const ctx = await context(role);
|
||||
await ctx.service.bulkSetRating(ids, rating, user);
|
||||
expect(ctx.bookRepo.bulkSetRating).toHaveBeenCalledWith(ids, rating, user.id);
|
||||
expectNoWrites(ctx);
|
||||
expect(ctx.score.calculateAndSaveMany).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rejects a personal-rating batch with an inaccessible book before persisting anything', async () => {
|
||||
const ctx = await context(null);
|
||||
await expect(ctx.service.bulkSetRating([2, 1], 4, user)).rejects.toThrow(ForbiddenException);
|
||||
expect(ctx.bookRepo.bulkSetRating).not.toHaveBeenCalled();
|
||||
expectNoWrites(ctx);
|
||||
});
|
||||
|
||||
it.each(bulkWrites)('rejects a mixed editor/viewer batch before $name writes anything', async ({ run }) => {
|
||||
const ctx = await context();
|
||||
await expect(run(ctx, [2, 1])).rejects.toThrow(ForbiddenException);
|
||||
expectNoWrites(ctx);
|
||||
});
|
||||
|
||||
it.each(['editor', 'owner'] as const)('allows %s to persist metadata', async (role) => {
|
||||
const ctx = await context(role);
|
||||
await ctx.service.updateMetadata(1, { title: 'Changed' }, user);
|
||||
expect(ctx.bookRepo.updateMetadataFields).toHaveBeenCalledWith(1, expect.objectContaining({ title: 'Changed' }), expect.anything());
|
||||
});
|
||||
|
||||
it('allows a superuser without membership to persist metadata', async () => {
|
||||
const ctx = await context(null);
|
||||
await ctx.service.updateMetadata(1, { title: 'Changed' }, { ...user, isSuperuser: true, permissions: [] });
|
||||
expect(ctx.bookRepo.updateMetadataFields).toHaveBeenCalled();
|
||||
expect(ctx.libraryRepo.hasUserAccess).not.toHaveBeenCalled();
|
||||
expect(ctx.libraryRepo.findUserAccessLevel).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('keeps missing membership and missing books concealed as 404', async () => {
|
||||
const ctx = await context(null);
|
||||
await expect(ctx.service.updateMetadata(1, { title: 'Changed' }, user)).rejects.toThrow(NotFoundException);
|
||||
await expect(ctx.service.updateMetadata(99, { title: 'Changed' }, user)).rejects.toThrow(NotFoundException);
|
||||
expectNoWrites(ctx);
|
||||
});
|
||||
|
||||
it('conceals content-filtered books before checking editor access', async () => {
|
||||
const ctx = await context();
|
||||
ctx.bookRepo.checkBookPassesContentFilters.mockResolvedValue(false);
|
||||
await expect(ctx.service.updateMetadata(1, { title: 'Changed' }, user)).rejects.toThrow(NotFoundException);
|
||||
expect(ctx.libraryRepo.findUserAccessLevel).not.toHaveBeenCalled();
|
||||
expectNoWrites(ctx);
|
||||
});
|
||||
|
||||
it('rechecks a downgraded role for each write', async () => {
|
||||
const ctx = await context('editor');
|
||||
await ctx.service.updateMetadata(1, { title: 'Allowed' }, user);
|
||||
ctx.bookRepo.updateMetadataFields.mockClear();
|
||||
ctx.roles.set(7, 'viewer');
|
||||
await expect(ctx.service.updateMetadataAndLocks(1, { metadata: { title: 'Denied' }, lockedFields: [] }, user)).rejects.toThrow(
|
||||
ForbiddenException,
|
||||
);
|
||||
expect(ctx.bookRepo.updateMetadataFields).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('validates every library once for bulk locks and deduplicates book IDs', async () => {
|
||||
const ctx = await context('editor');
|
||||
await ctx.service.bulkSetMetadataLock([1, 2, 3, 1], true, user);
|
||||
expect(ctx.bookRepo.findLibraryIdsByBookIds).toHaveBeenCalledWith([1, 2, 3]);
|
||||
expect(ctx.libraryRepo.findUserAccessLevel.mock.calls).toEqual([
|
||||
[user.id, 7],
|
||||
[user.id, 8],
|
||||
[user.id, 9],
|
||||
]);
|
||||
expect(ctx.locks.bulkReplaceLockedFields).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each(bulkWrites)('rejects missing IDs before bulk $name writes anything', async ({ run }) => {
|
||||
const ctx = await context('editor');
|
||||
await expect(run(ctx, [2, 99])).rejects.toThrow(NotFoundException);
|
||||
expectNoWrites(ctx);
|
||||
});
|
||||
|
||||
it('preserves viewer access to reading progress', async () => {
|
||||
const ctx = await context();
|
||||
await ctx.service.saveProgress(user.id, 10, { percentage: 25 }, user);
|
||||
expect(ctx.bookRepo.upsertProgress).toHaveBeenCalled();
|
||||
expect(ctx.libraryRepo.findUserAccessLevel).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import { BadRequestException, NotFoundException } from '@nestjs/common';
|
||||
import { BadRequestException, ForbiddenException, NotFoundException } from '@nestjs/common';
|
||||
import type { MockedFunction } from 'vitest';
|
||||
import { ZipArchive } from 'archiver';
|
||||
import { createReadStream } from 'fs';
|
||||
@@ -480,6 +480,21 @@ describe('BookController', () => {
|
||||
expect(raw.end).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each(['bulkRefreshMetadata', 'bulkReExtractCover'] as const)('rejects an unauthorized %s selection before starting SSE', async (method) => {
|
||||
const { controller, bookService } = makeController();
|
||||
const { reply, raw } = makeReply();
|
||||
const user = makeUser();
|
||||
const error = new ForbiddenException('Editor access required');
|
||||
bookService.resolveSelectionToIds.mockRejectedValue(error);
|
||||
|
||||
await expect(controller[method]({ bookIds: [9] }, user, reply)).rejects.toBe(error);
|
||||
|
||||
expect(bookService.resolveSelectionToIds).toHaveBeenCalledWith({ bookIds: [9] }, user, 'editor');
|
||||
expect(bookService[method]).not.toHaveBeenCalled();
|
||||
expect(raw.writeHead).not.toHaveBeenCalled();
|
||||
expect(raw.write).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('stops sending SSE done event when client disconnects', async () => {
|
||||
const { controller, bookService } = makeController();
|
||||
const { reply, raw, emitRawEvent } = makeReply();
|
||||
|
||||
@@ -166,7 +166,7 @@ export class BookController {
|
||||
},
|
||||
})
|
||||
async bulkRefreshMetadata(@Body() dto: BulkRefreshMetadataDto, @CurrentUser() user: RequestUser, @Res() reply: FastifyReply) {
|
||||
const ids = await this.bookService.resolveSelectionToIds(dto, user);
|
||||
const ids = await this.bookService.resolveSelectionToIds(dto, user, 'editor');
|
||||
const stream = this.createSseStream(reply);
|
||||
try {
|
||||
const result = await this.bookService.bulkRefreshMetadata(
|
||||
@@ -197,7 +197,7 @@ export class BookController {
|
||||
},
|
||||
})
|
||||
async bulkReExtractCover(@Body() dto: BulkBookIdsDto, @CurrentUser() user: RequestUser, @Res() reply: FastifyReply) {
|
||||
const ids = await this.bookService.resolveSelectionToIds(dto, user);
|
||||
const ids = await this.bookService.resolveSelectionToIds(dto, user, 'editor');
|
||||
const stream = this.createSseStream(reply);
|
||||
try {
|
||||
const result = await this.bookService.bulkReExtractCover(
|
||||
|
||||
@@ -73,6 +73,7 @@ type BookRepositoryTx = Parameters<Parameters<NodePgDatabase<typeof schema>['tra
|
||||
|
||||
type CollapsedRawRow = {
|
||||
id: number | null;
|
||||
library_id: number;
|
||||
status: string;
|
||||
cover_aspect_ratio: string;
|
||||
primary_file_id: number | null;
|
||||
@@ -554,6 +555,7 @@ export class BookRepository {
|
||||
const rows = await this.db
|
||||
.select({
|
||||
id: books.id,
|
||||
libraryId: books.libraryId,
|
||||
status: books.status,
|
||||
coverAspectRatio: libraries.coverAspectRatio,
|
||||
primaryFileId: books.primaryFileId,
|
||||
@@ -1051,6 +1053,7 @@ export class BookRepository {
|
||||
representatives AS (
|
||||
SELECT DISTINCT ON (${sql.raw(COLLAPSE_GROUP_KEY_SQL)})
|
||||
base.id,
|
||||
base.library_id,
|
||||
base.status,
|
||||
base.cover_aspect_ratio,
|
||||
base.primary_file_id,
|
||||
@@ -1131,6 +1134,7 @@ export class BookRepository {
|
||||
|
||||
const mappedRows = rawRows.map((r) => ({
|
||||
id: r.id,
|
||||
libraryId: r.library_id,
|
||||
status: r.status,
|
||||
coverAspectRatio: r.cover_aspect_ratio,
|
||||
primaryFileId: r.primary_file_id,
|
||||
@@ -2053,6 +2057,15 @@ export class BookRepository {
|
||||
return rows.map((r) => r.id);
|
||||
}
|
||||
|
||||
async findDistinctLibraryIdsByWhere(where: SQL | undefined): Promise<number[]> {
|
||||
const rows = await this.db
|
||||
.selectDistinct({ libraryId: books.libraryId })
|
||||
.from(books)
|
||||
.leftJoin(bookMetadata, eq(bookMetadata.bookId, books.id))
|
||||
.where(this.visibleWhere(where));
|
||||
return rows.map((r) => r.libraryId);
|
||||
}
|
||||
|
||||
async findPrimaryFile(bookId: number): Promise<{ absolutePath: string; format: string | null } | null> {
|
||||
const [row] = await this.db
|
||||
.select({ absolutePath: bookFiles.absolutePath, format: bookFiles.format })
|
||||
|
||||
@@ -158,6 +158,7 @@ function makeService(overrides: { bookMetadataLockService?: unknown; appDataPath
|
||||
deleteByIdsAndInvalidateScanState: vi.fn(),
|
||||
findAllIds: vi.fn(),
|
||||
findIdsByWhere: vi.fn(),
|
||||
findDistinctLibraryIdsByWhere: vi.fn(),
|
||||
findCardsCollapsed: vi.fn(),
|
||||
findJumpBuckets: vi.fn(),
|
||||
findJumpBucketsCollapsed: vi.fn(),
|
||||
@@ -167,6 +168,7 @@ function makeService(overrides: { bookMetadataLockService?: unknown; appDataPath
|
||||
};
|
||||
const libraryService = {
|
||||
verifyUserAccess: vi.fn().mockResolvedValue(undefined),
|
||||
verifyUserAccessLevel: vi.fn().mockResolvedValue(undefined),
|
||||
findAll: vi.fn().mockResolvedValue([]),
|
||||
findOne: vi.fn().mockResolvedValue({ readingThreshold: 1, markAsFinishedPercentComplete: 99 }),
|
||||
};
|
||||
@@ -1024,7 +1026,7 @@ describe('BookService', () => {
|
||||
metadata: { title: 'New Title', isbn10: '0306406152', isbn13: '9780306406157' },
|
||||
diagnostics: makeMetadataFetchDiagnostics({ resolvedFieldCount: 3 }),
|
||||
});
|
||||
expect(libraryService.verifyUserAccess).toHaveBeenCalledWith(user.id, 7, false);
|
||||
expect(libraryService.verifyUserAccessLevel).toHaveBeenCalledWith(user.id, 7, false, 'editor');
|
||||
expect(pipeline.runWithSources).toHaveBeenCalledWith(
|
||||
{
|
||||
title: 'Old Title',
|
||||
@@ -1758,7 +1760,7 @@ describe('BookService', () => {
|
||||
{ postSaveMode: 'sync' },
|
||||
);
|
||||
|
||||
expect(verifySpy).toHaveBeenCalledWith(5, user);
|
||||
expect(verifySpy).toHaveBeenCalledWith(5, user, 'editor');
|
||||
expect(bookRepo.withTransaction).toHaveBeenCalledTimes(1);
|
||||
expect(bookRepo.updateMetadataFields).toHaveBeenCalledWith(
|
||||
5,
|
||||
@@ -2395,7 +2397,7 @@ describe('BookService', () => {
|
||||
const result = await service.bulkReExtractCover([1, 2], user, onProgress);
|
||||
|
||||
expect(result).toEqual({ processed: 2, updated: 1 });
|
||||
expect(libraryService.verifyUserAccess).toHaveBeenCalledWith(user.id, 7, false);
|
||||
expect(libraryService.verifyUserAccessLevel).toHaveBeenCalledWith(user.id, 7, false, 'editor');
|
||||
expect(onProgress).toHaveBeenCalledTimes(2);
|
||||
expect(onProgress).toHaveBeenNthCalledWith(1, 1);
|
||||
expect(onProgress).toHaveBeenNthCalledWith(2, 2);
|
||||
@@ -4252,7 +4254,7 @@ describe('BookService', () => {
|
||||
});
|
||||
|
||||
describe('bulk metadata actions', () => {
|
||||
it('bulkSetRating updates ratings and queues file writes and score recalculation', async () => {
|
||||
it('bulkSetRating updates personal ratings without shared metadata effects', async () => {
|
||||
const { service, bookRepo, fileWriteService, fileRenameService, scoreService } = makeService();
|
||||
const user = makeUser({ id: 42 });
|
||||
vi.spyOn(service, 'verifyLibraryAccessForBookIds').mockResolvedValue(undefined);
|
||||
@@ -4260,12 +4262,9 @@ describe('BookService', () => {
|
||||
await service.bulkSetRating([3, 5], 4, user);
|
||||
|
||||
expect(bookRepo.bulkSetRating).toHaveBeenCalledWith([3, 5], 4, 42);
|
||||
expect(fileWriteService.scheduleWrite).toHaveBeenNthCalledWith(1, 3, 'auto', 42);
|
||||
expect(fileWriteService.scheduleWrite).toHaveBeenNthCalledWith(2, 5, 'auto', 42);
|
||||
expect(fileRenameService.scheduleRename).toHaveBeenNthCalledWith(1, 3, 42);
|
||||
expect(fileRenameService.scheduleRename).toHaveBeenNthCalledWith(2, 5, 42);
|
||||
expect(scoreService.calculateAndSave).toHaveBeenNthCalledWith(1, 3);
|
||||
expect(scoreService.calculateAndSave).toHaveBeenNthCalledWith(2, 5);
|
||||
expect(fileWriteService.scheduleWrite).not.toHaveBeenCalled();
|
||||
expect(fileRenameService.scheduleRename).not.toHaveBeenCalled();
|
||||
expect(scoreService.calculateAndSave).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('bulkSetRating skips books where rating is metadata-locked', async () => {
|
||||
@@ -4278,23 +4277,22 @@ describe('BookService', () => {
|
||||
|
||||
expect(bookMetadataLockService.getBookIdsWithLockedField).toHaveBeenCalledWith([3, 5], 'rating');
|
||||
expect(bookRepo.bulkSetRating).toHaveBeenCalledWith([3], 4, 42);
|
||||
expect(fileWriteService.scheduleWrite).toHaveBeenCalledTimes(1);
|
||||
expect(fileWriteService.scheduleWrite).toHaveBeenCalledWith(3, 'auto', 42);
|
||||
expect(fileRenameService.scheduleRename).toHaveBeenCalledTimes(1);
|
||||
expect(fileRenameService.scheduleRename).toHaveBeenCalledWith(3, 42);
|
||||
expect(scoreService.calculateAndSave).toHaveBeenCalledTimes(1);
|
||||
expect(scoreService.calculateAndSave).toHaveBeenCalledWith(3);
|
||||
expect(fileWriteService.scheduleWrite).not.toHaveBeenCalled();
|
||||
expect(fileRenameService.scheduleRename).not.toHaveBeenCalled();
|
||||
expect(scoreService.calculateAndSave).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('bulkSetRating propagates score recalculation failures', async () => {
|
||||
const { service, bookRepo, scoreService } = makeService();
|
||||
it('bulkSetRating propagates personal rating persistence failures', async () => {
|
||||
const { service, bookRepo, fileWriteService, fileRenameService } = makeService();
|
||||
const user = makeUser({ id: 42 });
|
||||
vi.spyOn(service, 'verifyLibraryAccessForBookIds').mockResolvedValue(undefined);
|
||||
scoreService.calculateAndSave.mockRejectedValue(new Error('score exploded'));
|
||||
bookRepo.bulkSetRating.mockRejectedValue(new Error('rating persistence failed'));
|
||||
|
||||
await expect(service.bulkSetRating([3], 4, user)).rejects.toThrow('score exploded');
|
||||
await expect(service.bulkSetRating([3], 4, user)).rejects.toThrow('rating persistence failed');
|
||||
|
||||
expect(bookRepo.bulkSetRating).toHaveBeenCalledWith([3], 4, 42);
|
||||
expect(fileWriteService.scheduleWrite).not.toHaveBeenCalled();
|
||||
expect(fileRenameService.scheduleRename).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('bulkSetMetadata updates a single metadata field and queues follow-up work', async () => {
|
||||
@@ -4737,7 +4735,7 @@ describe('BookService', () => {
|
||||
|
||||
await service.bulkEditMetadata([7, 8, 9], fields, user);
|
||||
|
||||
expect(verifySpy).toHaveBeenCalledWith([7, 8, 9], user);
|
||||
expect(verifySpy).toHaveBeenCalledWith([7, 8, 9], user, 'editor');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -5830,7 +5828,7 @@ describe('BookService', () => {
|
||||
const verifySpy = vi.spyOn(service, 'verifyLibraryAccessForBookIds').mockResolvedValue(undefined);
|
||||
|
||||
await expect(service.resolveSelectionToIds({ bookIds: [1, 2] }, user)).resolves.toEqual([1, 2]);
|
||||
expect(verifySpy).toHaveBeenCalledWith([1, 2], user);
|
||||
expect(verifySpy).toHaveBeenCalledWith([1, 2], user, 'viewer');
|
||||
});
|
||||
|
||||
it('throws when bookIds and query are both provided', async () => {
|
||||
@@ -5847,6 +5845,46 @@ describe('BookService', () => {
|
||||
await expect(service.resolveSelectionToIds({ bookIds: [1, 2] }, makeUser())).rejects.toBe(error);
|
||||
});
|
||||
|
||||
it('requires editor access before returning an explicit metadata selection', async () => {
|
||||
const { service } = makeService();
|
||||
const user = makeUser();
|
||||
const error = new ForbiddenException('Editor access required');
|
||||
const verifySpy = vi.spyOn(service, 'verifyLibraryAccessForBookIds').mockRejectedValue(error);
|
||||
|
||||
await expect(service.resolveSelectionToIds({ bookIds: [10, 11] }, user, 'editor')).rejects.toBe(error);
|
||||
expect(verifySpy).toHaveBeenCalledWith([10, 11], user, 'editor');
|
||||
});
|
||||
|
||||
it('checks the matched libraries, not every matched book id, before resolving a query selection for editing', async () => {
|
||||
const { service, bookRepo, libraryService, queryBuilder } = makeService();
|
||||
const user = makeUser();
|
||||
libraryService.findAll.mockResolvedValue([{ id: 5 }, { id: 9 }]);
|
||||
queryBuilder.buildWhere.mockReturnValue('where-clause');
|
||||
bookRepo.findDistinctLibraryIdsByWhere.mockResolvedValue([5, 9]);
|
||||
const error = new ForbiddenException('Insufficient library access level');
|
||||
libraryService.verifyUserAccessLevel.mockImplementation((_userId: number, libraryId: number) =>
|
||||
libraryId === 9 ? Promise.reject(error) : Promise.resolve(),
|
||||
);
|
||||
|
||||
await expect(service.resolveSelectionToIds({ query: {} }, user, 'editor')).rejects.toBe(error);
|
||||
expect(bookRepo.findDistinctLibraryIdsByWhere).toHaveBeenCalledWith('where-clause');
|
||||
expect(libraryService.verifyUserAccessLevel).toHaveBeenCalledWith(user.id, 5, false, 'editor');
|
||||
expect(libraryService.verifyUserAccessLevel).toHaveBeenCalledWith(user.id, 9, false, 'editor');
|
||||
expect(bookRepo.findIdsByWhere).not.toHaveBeenCalled();
|
||||
expect(bookRepo.findLibraryIdsByBookIds).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('skips the matched-library lookup for superusers resolving a query selection for editing', async () => {
|
||||
const { service, bookRepo, libraryService, queryBuilder } = makeService();
|
||||
const user = makeUser({ isSuperuser: true });
|
||||
libraryService.findAll.mockResolvedValue([{ id: 5 }]);
|
||||
queryBuilder.buildWhere.mockReturnValue('where-clause');
|
||||
bookRepo.findIdsByWhere.mockResolvedValue([10, 11]);
|
||||
|
||||
await expect(service.resolveSelectionToIds({ query: {} }, user, 'editor')).resolves.toEqual([10, 11]);
|
||||
expect(bookRepo.findDistinctLibraryIdsByWhere).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rejects explicit selections when any requested book id is missing', async () => {
|
||||
const { service, bookRepo } = makeService();
|
||||
bookRepo.findLibraryIdsByBookIds.mockResolvedValue([{ id: 1, libraryId: 5 }]);
|
||||
|
||||
@@ -51,6 +51,7 @@ import {
|
||||
resolveUploadPath,
|
||||
} from '@bookorbit/types';
|
||||
import type {
|
||||
AccessLevel,
|
||||
AudiobookChapter,
|
||||
BookCommunityRating,
|
||||
BookKoboState,
|
||||
@@ -336,18 +337,34 @@ export class BookService {
|
||||
return code === 'ENOENT' || code === 'ENOTDIR';
|
||||
}
|
||||
|
||||
private async verifyLibraryAccessForBookIds(bookIds: number[], user: RequestUser): Promise<{ id: number; libraryId: number }[]> {
|
||||
private async verifyLibraryAccessForBookIds(
|
||||
bookIds: number[],
|
||||
user: RequestUser,
|
||||
required: AccessLevel = 'viewer',
|
||||
): Promise<{ id: number; libraryId: number }[]> {
|
||||
const uniqueBookIds = [...new Set(bookIds)];
|
||||
const rows = await this.bookRepo.findLibraryIdsByBookIds(uniqueBookIds);
|
||||
if (rows.length !== uniqueBookIds.length) {
|
||||
throw new NotFoundException('One or more books were not found');
|
||||
}
|
||||
const uniqueLibraryIds = [...new Set(rows.map((row) => row.libraryId))];
|
||||
const isSuperuser = this.isSuperuser(user);
|
||||
await Promise.all(uniqueLibraryIds.map((libraryId) => this.libraryService.verifyUserAccess(user.id, libraryId, isSuperuser)));
|
||||
await this.verifyLibraryAccessLevels(
|
||||
rows.map((row) => row.libraryId),
|
||||
user,
|
||||
required,
|
||||
);
|
||||
return rows;
|
||||
}
|
||||
|
||||
private async verifyLibraryAccessLevels(libraryIds: number[], user: RequestUser, required: AccessLevel): Promise<void> {
|
||||
const isSuperuser = this.isSuperuser(user);
|
||||
if (isSuperuser) return;
|
||||
await mapWithConcurrency([...new Set(libraryIds)], 8, (libraryId) =>
|
||||
required === 'viewer'
|
||||
? this.libraryService.verifyUserAccess(user.id, libraryId, isSuperuser)
|
||||
: this.libraryService.verifyUserAccessLevel(user.id, libraryId, isSuperuser, required),
|
||||
);
|
||||
}
|
||||
|
||||
private collectExistingProviderIds(meta: {
|
||||
googleBooksId?: string | null;
|
||||
goodreadsId?: string | null;
|
||||
@@ -526,7 +543,7 @@ export class BookService {
|
||||
return normalized;
|
||||
}
|
||||
|
||||
async verifyBookAccess(bookId: number, user: RequestUser): Promise<void> {
|
||||
async verifyBookAccess(bookId: number, user: RequestUser, required: AccessLevel = 'viewer'): Promise<void> {
|
||||
const libraryId = await this.bookRepo.findLibraryIdByBookId(bookId);
|
||||
if (libraryId === null) throw new NotFoundException(`Book ${bookId} not found`);
|
||||
// Not found, rather than forbidden. A 403 tells the caller the book exists, and book ids are
|
||||
@@ -547,6 +564,7 @@ export class BookService {
|
||||
const passes = await this.checkBookPassesContentFilters(bookId, user.contentFilters);
|
||||
if (!passes) throw new NotFoundException(`Book ${bookId} not found`);
|
||||
}
|
||||
if (required !== 'viewer') await this.libraryService.verifyUserAccessLevel(user.id, libraryId, this.isSuperuser(user), required);
|
||||
}
|
||||
|
||||
setHardcoverEditionIdIfEmpty(bookId: number, hardcoverEditionId: string): Promise<boolean> {
|
||||
@@ -566,12 +584,12 @@ export class BookService {
|
||||
return file;
|
||||
}
|
||||
|
||||
async resolveSelectionToIds(dto: BulkSelectionDto, user: RequestUser): Promise<number[]> {
|
||||
async resolveSelectionToIds(dto: BulkSelectionDto, user: RequestUser, required: AccessLevel = 'viewer'): Promise<number[]> {
|
||||
if (dto.bookIds && dto.query) {
|
||||
throw new BadRequestException('bookIds and query are mutually exclusive');
|
||||
}
|
||||
if (dto.bookIds) {
|
||||
await this.verifyLibraryAccessForBookIds(dto.bookIds, user);
|
||||
await this.verifyLibraryAccessForBookIds(dto.bookIds, user, required);
|
||||
return dto.bookIds;
|
||||
}
|
||||
if (dto.query) {
|
||||
@@ -592,6 +610,9 @@ export class BookService {
|
||||
timeZone,
|
||||
contentFilters: this.isSuperuser(user) ? undefined : user.contentFilters,
|
||||
});
|
||||
if (required !== 'viewer' && !this.isSuperuser(user)) {
|
||||
await this.verifyLibraryAccessLevels(await this.bookRepo.findDistinctLibraryIdsByWhere(where), user, required);
|
||||
}
|
||||
return this.bookRepo.findIdsByWhere(where);
|
||||
}
|
||||
throw new BadRequestException('Either bookIds or query must be provided');
|
||||
@@ -1565,6 +1586,7 @@ export class BookService {
|
||||
this.logger.log(`[${event}] [start] fileId=${fileId} userId=${user.id} - rename file started`);
|
||||
try {
|
||||
const file = await this.verifyFileAccess(fileId, user);
|
||||
await this.libraryService.verifyUserAccessLevel(user.id, file.libraryId, this.isSuperuser(user), 'editor');
|
||||
|
||||
let newAbsolutePath = file.absolutePath;
|
||||
if (dto.filename && dto.filename !== basename(file.absolutePath)) {
|
||||
@@ -1739,7 +1761,7 @@ export class BookService {
|
||||
const startedAt = Date.now();
|
||||
this.logger.log(`[${event}] [start] bookId=${id} userId=${user.id} - metadata update started`);
|
||||
try {
|
||||
await this.verifyBookAccess(id, user);
|
||||
await this.verifyBookAccess(id, user, 'editor');
|
||||
await this.bookMetadataLockService.assertManualUpdateAllowed(id, dto);
|
||||
const { detail, scalarFieldCount, write, libraryAutoWriteEnabled } = await this.persistMetadataUpdate(id, dto, user, {
|
||||
postSaveMode: options.postSaveMode ?? 'schedule',
|
||||
@@ -1759,7 +1781,7 @@ export class BookService {
|
||||
}
|
||||
|
||||
async updateAddedAt(id: number, dto: UpdateBookAddedAtDto, user: RequestUser): Promise<BookDetailDto> {
|
||||
await this.verifyBookAccess(id, user);
|
||||
await this.verifyBookAccess(id, user, 'editor');
|
||||
const timeZone = this.resolveUserTimeZone(user);
|
||||
if (!isDateKey(dto.addedAt)) throw new BadRequestException('addedAt must be a valid date');
|
||||
if (dto.addedAt > toDateKeyInTimeZone(new Date(), timeZone)) throw new BadRequestException('addedAt cannot be in the future');
|
||||
@@ -1780,7 +1802,7 @@ export class BookService {
|
||||
`[${event}] [start] bookId=${id} userId=${user.id} metadataFields=${Object.keys(metadata).length} lockFields=${dto.lockedFields.length} - metadata and lock update started`,
|
||||
);
|
||||
try {
|
||||
await this.verifyBookAccess(id, user);
|
||||
await this.verifyBookAccess(id, user, 'editor');
|
||||
// No lock guard here: this endpoint sets metadata and locks together from one explicit manual
|
||||
// request, so a field carried in the payload is always an intentional edit (the editor disables
|
||||
// locked inputs). Guarding on the final lock state would reject the legitimate unlock -> edit ->
|
||||
@@ -2058,7 +2080,7 @@ export class BookService {
|
||||
const startedAt = Date.now();
|
||||
this.logger.log(`[${event}] [start] bookId=${id} userId=${user.id} - metadata lock update started`);
|
||||
try {
|
||||
await this.verifyBookAccess(id, user);
|
||||
await this.verifyBookAccess(id, user, 'editor');
|
||||
const normalizedLockedFields = await this.bookMetadataLockService.replaceLockedFields(id, lockedFields);
|
||||
const detail = await this.getDetail(id, user);
|
||||
this.logger.log(
|
||||
@@ -2520,7 +2542,6 @@ export class BookService {
|
||||
const updatableIds = bookIds.filter((bookId) => !lockedIds.has(bookId));
|
||||
if (updatableIds.length > 0) {
|
||||
await this.bookRepo.bulkSetRating(updatableIds, rating, user.id);
|
||||
await this.triggerPostMetadataUpdateEffects(updatableIds, user.id);
|
||||
this.achievementEvents?.emit(ACHIEVEMENT_EVENT_BOOK_RATING_CHANGED, {
|
||||
userId: user.id,
|
||||
bookIds: updatableIds,
|
||||
@@ -2536,7 +2557,7 @@ export class BookService {
|
||||
const event = 'book.bulk.set_metadata';
|
||||
const startedAt = Date.now();
|
||||
this.logger.log(`[${event}] [start] userId=${user.id} count=${bookIds.length} field=${field} - bulk set metadata started`);
|
||||
await this.verifyLibraryAccessForBookIds(bookIds, user);
|
||||
await this.verifyLibraryAccessForBookIds(bookIds, user, 'editor');
|
||||
const lockField = BULK_METADATA_LOCK_FIELD_BY_FIELD[field];
|
||||
const lockedIds = await this.bookMetadataLockService.getBookIdsWithLockedField(bookIds, lockField);
|
||||
const updatableIds = bookIds.filter((bookId) => !lockedIds.has(bookId));
|
||||
@@ -2598,7 +2619,7 @@ export class BookService {
|
||||
const event = 'book.bulk.update_tags';
|
||||
const startedAt = Date.now();
|
||||
this.logger.log(`[${event}] [start] userId=${user.id} count=${bookIds.length} mode=${mode} tagCount=${tags.length} - bulk update tags started`);
|
||||
await this.verifyLibraryAccessForBookIds(bookIds, user);
|
||||
await this.verifyLibraryAccessForBookIds(bookIds, user, 'editor');
|
||||
await this.bookRepo.withTransaction(async (tx) => {
|
||||
if (mode === 'replace') {
|
||||
for (const bookId of bookIds) {
|
||||
@@ -2625,7 +2646,7 @@ export class BookService {
|
||||
const event = 'book.bulk.set_metadata_lock';
|
||||
const startedAt = Date.now();
|
||||
this.logger.log(`[${event}] [start] userId=${user.id} count=${bookIds.length} locked=${locked} - bulk set metadata lock started`);
|
||||
await this.verifyLibraryAccessForBookIds(bookIds, user);
|
||||
await this.verifyLibraryAccessForBookIds(bookIds, user, 'editor');
|
||||
const fields: BookMetadataLockField[] = locked ? [...BOOK_METADATA_LOCK_FIELDS] : [];
|
||||
await this.bookMetadataLockService.bulkReplaceLockedFields(bookIds, fields);
|
||||
this.logger.log(
|
||||
@@ -2646,7 +2667,7 @@ export class BookService {
|
||||
throw new BadRequestException('array fields with add or remove mode must have a non-empty values array');
|
||||
}
|
||||
|
||||
await this.verifyLibraryAccessForBookIds(bookIds, user);
|
||||
await this.verifyLibraryAccessForBookIds(bookIds, user, 'editor');
|
||||
|
||||
const locksMap = await this.bookMetadataLockService.getLockedFieldsMap(bookIds);
|
||||
const fieldResults: Record<string, BulkEditFieldResult> = {};
|
||||
@@ -2921,7 +2942,7 @@ export class BookService {
|
||||
if (!found) throw new NotFoundException(`Book ${id} not found`);
|
||||
|
||||
const { book, authorRows, genreRows, communityRatingRows, fileRows, narratorRows } = found;
|
||||
await this.libraryService.verifyUserAccess(user.id, book.books.libraryId, this.isSuperuser(user));
|
||||
await this.libraryService.verifyUserAccessLevel(user.id, book.books.libraryId, this.isSuperuser(user), 'editor');
|
||||
const meta = book.book_metadata;
|
||||
|
||||
const providerIds = this.collectExistingProviderIds(meta ?? {});
|
||||
@@ -3074,7 +3095,7 @@ export class BookService {
|
||||
this.logger.log(`[${event}] [end] count=0 durationMs=${Date.now() - startedAt} processed=0 failed=0 - bulk refresh metadata completed`);
|
||||
return { processed: 0, failed: 0 };
|
||||
}
|
||||
await this.verifyLibraryAccessForBookIds(bookIds, user);
|
||||
await this.verifyLibraryAccessForBookIds(bookIds, user, 'editor');
|
||||
|
||||
let processed = 0;
|
||||
let failed = 0;
|
||||
@@ -3137,7 +3158,7 @@ export class BookService {
|
||||
this.logger.log(`[${event}] [end] count=0 durationMs=${Date.now() - startedAt} processed=0 updated=0 - bulk re-extract cover completed`);
|
||||
return { processed: 0, updated: 0 };
|
||||
}
|
||||
await this.verifyLibraryAccessForBookIds(bookIds, user);
|
||||
await this.verifyLibraryAccessForBookIds(bookIds, user, 'editor');
|
||||
|
||||
const [files, coverLockedBookIds, audioCoverLockedBookIds] = await Promise.all([
|
||||
this.bookRepo.findCoverSourceFilesByBookIds(bookIds),
|
||||
@@ -3541,7 +3562,7 @@ export class BookService {
|
||||
}
|
||||
|
||||
async writeAndRename(bookId: number, user: RequestUser): Promise<BookWriteAndRenameResult> {
|
||||
await this.verifyBookAccess(bookId, user);
|
||||
await this.verifyBookAccess(bookId, user, 'editor');
|
||||
|
||||
this.fileWriteService?.cancelPendingWrite(bookId);
|
||||
this.fileRenameService?.cancelPendingRename(bookId);
|
||||
|
||||
@@ -29,6 +29,11 @@ function makeBookRow(id: number, overrides?: Partial<Parameters<typeof assembleB
|
||||
}
|
||||
|
||||
describe('assembleBookCards', () => {
|
||||
it('preserves each book library ID for permission checks', () => {
|
||||
const cards = assembleBookCards([makeBookRow(1, { libraryId: 7 }), makeBookRow(2, { libraryId: 8 })], [], [], [], []);
|
||||
expect(cards.map((card) => card.libraryId)).toEqual([7, 8]);
|
||||
});
|
||||
|
||||
it('assembles a simple book card with authors and files', () => {
|
||||
const rows = [makeBookRow(1)];
|
||||
const authorRows = [{ bookId: 1, name: 'Author A' }];
|
||||
|
||||
@@ -13,6 +13,7 @@ function normalizeLockedFields(raw: string[] | null | undefined): BookMetadataLo
|
||||
|
||||
type BookRow = {
|
||||
id: number;
|
||||
libraryId?: number;
|
||||
status: string;
|
||||
coverAspectRatio: string;
|
||||
primaryFileId?: number | null;
|
||||
@@ -209,6 +210,7 @@ export function assembleBookCards(
|
||||
|
||||
return {
|
||||
id: row.id,
|
||||
libraryId: row.libraryId,
|
||||
status: row.status,
|
||||
coverAspectRatio: normalizeCoverAspectRatio(row.coverAspectRatio),
|
||||
title: row.title ?? basename(row.folderPath),
|
||||
|
||||
@@ -99,7 +99,7 @@ function createMutationService(options?: { assertFieldsUnlocked?: ReturnType<typ
|
||||
{ findLibraryIdByBookId: vi.fn().mockResolvedValue(7) } as never,
|
||||
{ assertFieldsUnlocked } as never,
|
||||
fileWriteService as never,
|
||||
{ verifyUserAccess: vi.fn().mockResolvedValue(undefined) } as never,
|
||||
{ verifyUserAccess: vi.fn().mockResolvedValue(undefined), verifyUserAccessLevel: vi.fn().mockResolvedValue(undefined) } as never,
|
||||
options?.providerRegistry ?? ({ select: vi.fn().mockReturnValue([]) } as unknown as CoverProviderRegistry),
|
||||
scoreService as never,
|
||||
coverStore as never,
|
||||
|
||||
@@ -212,7 +212,7 @@ export class CoverService {
|
||||
private async verifyAccess(bookId: number, user: RequestUser): Promise<void> {
|
||||
const libraryId = await this.bookReadService.findLibraryIdByBookId(bookId);
|
||||
if (libraryId === null) throw new NotFoundException(`Book ${bookId} not found`);
|
||||
await this.libraryService.verifyUserAccess(user.id, libraryId, user.isSuperuser);
|
||||
await this.libraryService.verifyUserAccessLevel(user.id, libraryId, user.isSuperuser, 'editor');
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -38,6 +38,7 @@ function makeService() {
|
||||
|
||||
const libraryService = {
|
||||
findAccessibleLibraryIds: vi.fn().mockResolvedValue([1, 2]),
|
||||
findEditableLibraryIds: vi.fn().mockResolvedValue([1, 2]),
|
||||
};
|
||||
|
||||
const fileWriteService = {
|
||||
@@ -604,6 +605,27 @@ describe('EntityManagerService', () => {
|
||||
});
|
||||
|
||||
describe('merge', () => {
|
||||
it('scopes every mutation to libraries the user can edit, not every library they can read', async () => {
|
||||
const { service, strategies, libraryService } = makeService();
|
||||
libraryService.findAccessibleLibraryIds.mockResolvedValue([1, 2, 3]);
|
||||
libraryService.findEditableLibraryIds.mockResolvedValue([2]);
|
||||
(strategies.tag.merge as any).mockResolvedValue({ affectedBookIds: [] });
|
||||
(strategies.tag.rename as any).mockResolvedValue({ affectedBookIds: [] });
|
||||
(strategies.genre.deleteEntity as any).mockResolvedValue({ affectedBookIds: [] });
|
||||
(strategies.author.split as any).mockResolvedValue({ originalName: 'Author', newEntities: [], affectedBookIds: [] });
|
||||
|
||||
await service.merge('tag', mockUser, 1, [2], false);
|
||||
await service.rename('tag', mockUser, 1, 'Renamed', false);
|
||||
await service.deleteEntity('genre', mockUser, 5, 'hard', false);
|
||||
await service.split('author', mockUser, 5, ['Author A', 'Author B'], false);
|
||||
|
||||
expect(strategies.tag.merge).toHaveBeenCalledWith(expect.objectContaining({ libraryIds: [2] }));
|
||||
expect(strategies.tag.rename).toHaveBeenCalledWith(expect.objectContaining({ libraryIds: [2] }));
|
||||
expect(strategies.genre.deleteEntity).toHaveBeenCalledWith(expect.objectContaining({ libraryIds: [2] }));
|
||||
expect(strategies.author.split).toHaveBeenCalledWith(expect.objectContaining({ libraryIds: [2] }));
|
||||
expect(libraryService.findEditableLibraryIds).toHaveBeenCalledTimes(4);
|
||||
});
|
||||
|
||||
it('calls strategy merge and cleans up dismissed pairs for first-class', async () => {
|
||||
const { service, strategies, repo, duplicateCompute, metadataScoreService } = makeService();
|
||||
(strategies.author.merge as any).mockResolvedValue({
|
||||
|
||||
@@ -242,7 +242,7 @@ export class EntityManagerService {
|
||||
|
||||
try {
|
||||
const strategy = this.getStrategy(entityType);
|
||||
const libraryIds = await this.libraryService.findAccessibleLibraryIds(user);
|
||||
const libraryIds = await this.libraryService.findEditableLibraryIds(user);
|
||||
const result = await strategy.merge({ targetId, sourceIds, userId: user.id, libraryIds });
|
||||
|
||||
const nonInlineSourceIds: number[] = [];
|
||||
@@ -291,7 +291,7 @@ export class EntityManagerService {
|
||||
|
||||
try {
|
||||
const strategy = this.getStrategy(entityType);
|
||||
const libraryIds = await this.libraryService.findAccessibleLibraryIds(user);
|
||||
const libraryIds = await this.libraryService.findEditableLibraryIds(user);
|
||||
const result = await strategy.rename({ entityId, newName, userId: user.id, libraryIds });
|
||||
|
||||
if (writeFiles) {
|
||||
@@ -333,7 +333,7 @@ export class EntityManagerService {
|
||||
|
||||
try {
|
||||
const strategy = this.getStrategy(entityType);
|
||||
const libraryIds = await this.libraryService.findAccessibleLibraryIds(user);
|
||||
const libraryIds = await this.libraryService.findEditableLibraryIds(user);
|
||||
const effectiveMode = !strategy.isInline && mode === 'soft' && (await strategy.getBookCount(entityId)) === 0 ? 'hard' : mode;
|
||||
const result = await strategy.deleteEntity({ entityId, mode: effectiveMode, libraryIds });
|
||||
|
||||
@@ -400,7 +400,7 @@ export class EntityManagerService {
|
||||
throw new BadRequestException('Split requires at least 2 new names');
|
||||
}
|
||||
|
||||
const libraryIds = await this.libraryService.findAccessibleLibraryIds(user);
|
||||
const libraryIds = await this.libraryService.findEditableLibraryIds(user);
|
||||
const result = await strategy.split({ entityId, newNames, libraryIds });
|
||||
|
||||
await this.repo.deleteDismissedPairsForEntity(entityType, entityId);
|
||||
|
||||
@@ -49,6 +49,6 @@ export async function assertEntityRelationsWithinLibraries(
|
||||
`);
|
||||
|
||||
if (result.rows.length > 0) {
|
||||
throw new ForbiddenException('Entity is used by a library outside the accessible scope');
|
||||
throw new ForbiddenException('Entity is used by a library you cannot edit');
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import type { AccessLevel, AddedAtSource, CoverAspectRatio, OrganizationMode } from '@bookorbit/types';
|
||||
|
||||
export const LIBRARY_ACCESS_LEVELS = ['viewer', 'editor', 'owner'] as const satisfies readonly AccessLevel[];
|
||||
export const EDITOR_ACCESS_LEVELS = ['editor', 'owner'] as const satisfies readonly AccessLevel[];
|
||||
export const LIBRARY_COVER_ASPECT_RATIOS = ['2/3', '1/1'] as const satisfies readonly CoverAspectRatio[];
|
||||
export const LIBRARY_ORGANIZATION_MODES = ['book_per_file', 'book_per_folder'] as const satisfies readonly OrganizationMode[];
|
||||
export const LIBRARY_ADDED_AT_SOURCES = ['imported', 'file_modified', 'file_created'] as const satisfies readonly AddedAtSource[];
|
||||
|
||||
@@ -97,11 +97,16 @@ export class LibraryRepository {
|
||||
.orderBy(libraries.id);
|
||||
}
|
||||
|
||||
findAccessibleIdsForUser(userId: number) {
|
||||
findAccessibleIdsForUser(userId: number, accessLevels?: readonly AccessLevel[]) {
|
||||
const accessJoin = and(
|
||||
eq(schema.userLibraryAccess.libraryId, libraries.id),
|
||||
eq(schema.userLibraryAccess.userId, userId),
|
||||
accessLevels ? inArray(schema.userLibraryAccess.accessLevel, [...accessLevels]) : undefined,
|
||||
);
|
||||
return this.db
|
||||
.select({ id: libraries.id })
|
||||
.from(libraries)
|
||||
.innerJoin(schema.userLibraryAccess, and(eq(schema.userLibraryAccess.libraryId, libraries.id), eq(schema.userLibraryAccess.userId, userId)))
|
||||
.innerJoin(schema.userLibraryAccess, accessJoin)
|
||||
.where(visibleLibraryCondition)
|
||||
.orderBy(libraries.displayOrder, libraries.name);
|
||||
}
|
||||
|
||||
@@ -170,6 +170,21 @@ describe('LibraryService', () => {
|
||||
).resolves.toEqual([3, 4]);
|
||||
});
|
||||
|
||||
it('findEditableLibraryIds reads all IDs for superusers and editor-or-owner IDs for normal users', async () => {
|
||||
libraryRepo.findAllIds.mockResolvedValue([{ id: 1 }, { id: 2 }]);
|
||||
libraryRepo.findAccessibleIdsForUser.mockResolvedValue([{ id: 4 }]);
|
||||
|
||||
await expect(service.findEditableLibraryIds({ id: 99, isSuperuser: true, contentFilters: EMPTY_CONTENT_FILTER_RULES } as any)).resolves.toEqual([
|
||||
1, 2,
|
||||
]);
|
||||
expect(libraryRepo.findAccessibleIdsForUser).not.toHaveBeenCalled();
|
||||
|
||||
await expect(service.findEditableLibraryIds({ id: 42, isSuperuser: false, contentFilters: EMPTY_CONTENT_FILTER_RULES } as any)).resolves.toEqual([
|
||||
4,
|
||||
]);
|
||||
expect(libraryRepo.findAccessibleIdsForUser).toHaveBeenCalledWith(42, ['editor', 'owner']);
|
||||
});
|
||||
|
||||
it('findOne returns library details and normalizes organization mode', async () => {
|
||||
libraryRepo.findById.mockResolvedValue([{ id: 10, name: 'Main', organizationMode: null }]);
|
||||
libraryRepo.findFoldersByLibrary.mockResolvedValue([{ id: 50, path: '/books/main' }]);
|
||||
|
||||
@@ -35,6 +35,7 @@ import {
|
||||
DEFAULT_LIBRARY_ADDED_AT_SOURCE,
|
||||
DEFAULT_LIBRARY_COVER_ASPECT_RATIO,
|
||||
DEFAULT_LIBRARY_ORGANIZATION_MODE,
|
||||
EDITOR_ACCESS_LEVELS,
|
||||
LIBRARY_METADATA_PRECEDENCE_DEFAULT,
|
||||
} from './library.constants';
|
||||
import { resolveLibraryFolderRoles, type LibraryFolderInput } from './library-folder-roles.utils';
|
||||
@@ -158,6 +159,13 @@ export class LibraryService {
|
||||
return ids.map(({ id }) => id);
|
||||
}
|
||||
|
||||
async findEditableLibraryIds(user: RequestUser): Promise<number[]> {
|
||||
const ids = user.isSuperuser
|
||||
? await this.libraryRepo.findAllIds()
|
||||
: await this.libraryRepo.findAccessibleIdsForUser(user.id, EDITOR_ACCESS_LEVELS);
|
||||
return ids.map(({ id }) => id);
|
||||
}
|
||||
|
||||
async findOne(id: number) {
|
||||
const [library] = await this.libraryRepo.findById(id);
|
||||
if (!library) throw new NotFoundException('Library not found');
|
||||
|
||||
@@ -236,7 +236,7 @@ describe('ScannerRepository', () => {
|
||||
coverAspectRatio: '1/1',
|
||||
}),
|
||||
]);
|
||||
expect(db.select.mock.calls[0]?.[0]).toEqual(expect.objectContaining({ coverAspectRatio: expect.anything() }));
|
||||
expect(db.select.mock.calls[0]?.[0]).toEqual(expect.objectContaining({ libraryId: expect.anything(), coverAspectRatio: expect.anything() }));
|
||||
expect(chains.select[0]?.mocks.innerJoin).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
|
||||
@@ -659,6 +659,7 @@ export class ScannerRepository {
|
||||
this.db
|
||||
.select({
|
||||
id: books.id,
|
||||
libraryId: books.libraryId,
|
||||
status: books.status,
|
||||
coverAspectRatio: libraries.coverAspectRatio,
|
||||
formatPriority: libraries.formatPriority,
|
||||
|
||||
@@ -89,7 +89,7 @@ describe('UploadService', () => {
|
||||
};
|
||||
|
||||
const appSettings = { getUploadPattern: vi.fn(), getUploadPatternBookPerFolder: vi.fn(), isCrossPlatformPathSanitizationEnabled: vi.fn() };
|
||||
const libraryService = { verifyUserAccess: vi.fn() };
|
||||
const libraryService = { verifyUserAccess: vi.fn(), verifyUserAccessLevel: vi.fn() };
|
||||
const validator = {
|
||||
sanitizeFilename: vi.fn(),
|
||||
validateFormat: vi.fn(),
|
||||
@@ -142,6 +142,7 @@ describe('UploadService', () => {
|
||||
storage.cleanup.mockResolvedValue(undefined);
|
||||
processor.createBookRecord.mockResolvedValue({ bookId: 99, created: true });
|
||||
libraryService.verifyUserAccess.mockResolvedValue(undefined);
|
||||
libraryService.verifyUserAccessLevel.mockResolvedValue(undefined);
|
||||
appSettings.getUploadPattern.mockResolvedValue(null);
|
||||
appSettings.getUploadPatternBookPerFolder.mockResolvedValue(null);
|
||||
appSettings.isCrossPlatformPathSanitizationEnabled.mockResolvedValue(false);
|
||||
@@ -1443,9 +1444,44 @@ describe('UploadService', () => {
|
||||
db.select.mockReturnValueOnce(selectChain([{ id: 10, libraryId: 1 }]));
|
||||
|
||||
await expect(service.renameBookFiles(10, user)).resolves.toBeUndefined();
|
||||
expect(libraryService.verifyUserAccessLevel).toHaveBeenCalledWith(user.id, 1, false, 'editor');
|
||||
expect(mockRenameService.performRename).toHaveBeenCalledWith(10, user.id, true, false);
|
||||
});
|
||||
|
||||
it('rejects a viewer before resolving or invoking the rename service', async () => {
|
||||
const renameService = { performRename: vi.fn() };
|
||||
moduleRef.get.mockReturnValue(renameService);
|
||||
db.select.mockReturnValueOnce(selectChain([{ id: 10, libraryId: 1 }]));
|
||||
libraryService.verifyUserAccessLevel.mockRejectedValue(new ForbiddenException('Editor access required'));
|
||||
|
||||
await expect(service.renameBookFiles(10, user)).rejects.toThrow(ForbiddenException);
|
||||
expect(moduleRef.get).not.toHaveBeenCalled();
|
||||
expect(renameService.performRename).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('checks the current role again after a downgrade', async () => {
|
||||
const renameService = { performRename: vi.fn().mockResolvedValue({ status: 'success' }) };
|
||||
moduleRef.get.mockReturnValue(renameService);
|
||||
db.select.mockReturnValue(selectChain([{ id: 10, libraryId: 1 }]));
|
||||
await service.renameBookFiles(10, user);
|
||||
renameService.performRename.mockClear();
|
||||
libraryService.verifyUserAccessLevel.mockRejectedValue(new ForbiddenException('Editor access required'));
|
||||
|
||||
await expect(service.renameBookFiles(10, user)).rejects.toThrow(ForbiddenException);
|
||||
expect(renameService.performRename).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('passes the superuser bypass through the editor authorization check', async () => {
|
||||
const caller = { ...user, isSuperuser: true };
|
||||
const renameService = { performRename: vi.fn().mockResolvedValue({ status: 'success' }) };
|
||||
moduleRef.get.mockReturnValue(renameService);
|
||||
db.select.mockReturnValueOnce(selectChain([{ id: 10, libraryId: 1 }]));
|
||||
|
||||
await service.renameBookFiles(10, caller);
|
||||
expect(libraryService.verifyUserAccessLevel).toHaveBeenCalledWith(caller.id, 1, true, 'editor');
|
||||
expect(renameService.performRename).toHaveBeenCalledWith(10, caller.id, true, false);
|
||||
});
|
||||
|
||||
it('throws NotFoundException when book does not exist', async () => {
|
||||
db.select.mockReturnValueOnce(selectChain([]));
|
||||
|
||||
@@ -1454,7 +1490,7 @@ describe('UploadService', () => {
|
||||
|
||||
it('throws ForbiddenException when user has no library access', async () => {
|
||||
db.select.mockReturnValueOnce(selectChain([{ id: 10, libraryId: 1 }]));
|
||||
libraryService.verifyUserAccess.mockRejectedValue(new ForbiddenException('No access'));
|
||||
libraryService.verifyUserAccessLevel.mockRejectedValue(new ForbiddenException('No access to this library'));
|
||||
|
||||
await expect(service.renameBookFiles(10, user)).rejects.toBeInstanceOf(ForbiddenException);
|
||||
});
|
||||
|
||||
@@ -388,7 +388,7 @@ export class UploadService {
|
||||
|
||||
if (!bookRow) throw new NotFoundException(`Book ${bookId} not found`);
|
||||
|
||||
await this.libraryService.verifyUserAccess(user.id, bookRow.libraryId, user.isSuperuser);
|
||||
await this.libraryService.verifyUserAccessLevel(user.id, bookRow.libraryId, user.isSuperuser, 'editor');
|
||||
|
||||
const fileRenameService = this.resolveFileRenameService();
|
||||
if (!fileRenameService) {
|
||||
|
||||
@@ -8,6 +8,7 @@ vi.mock('drizzle-orm', () => ({
|
||||
inArray: vi.fn((left: unknown, right: unknown) => ({ op: 'inArray', left, right })),
|
||||
isNull: vi.fn((value: unknown) => ({ op: 'isNull', value })),
|
||||
ne: vi.fn((left: unknown, right: unknown) => ({ op: 'ne', left, right })),
|
||||
notInArray: vi.fn((left: unknown, right: unknown) => ({ op: 'notInArray', left, right })),
|
||||
or: vi.fn((...clauses: unknown[]) => ({ op: 'or', clauses })),
|
||||
sql: Object.assign(
|
||||
vi.fn((strings: TemplateStringsArray, ...values: unknown[]) => ({ op: 'sql', text: strings.join(''), values })),
|
||||
@@ -843,7 +844,7 @@ describe('UserRepository', () => {
|
||||
expect(onConflictDoNothing).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('findLibraryIdsByUserId and replaceViewerLibraries manage user-library rows', async () => {
|
||||
it('findLibraryIdsByUserId and replaceLibraries manage user-library rows', async () => {
|
||||
const where = vi.fn().mockResolvedValue([{ libraryId: 10 }, { libraryId: 11 }]);
|
||||
const from = vi.fn().mockReturnValue({ where });
|
||||
select.mockReturnValue({ from });
|
||||
@@ -852,7 +853,8 @@ describe('UserRepository', () => {
|
||||
|
||||
const txDeleteWhere = vi.fn().mockResolvedValue(undefined);
|
||||
const txDelete = vi.fn().mockReturnValue({ where: txDeleteWhere });
|
||||
const txInsertValues = vi.fn().mockResolvedValue(undefined);
|
||||
const txOnConflictDoNothing = vi.fn().mockResolvedValue(undefined);
|
||||
const txInsertValues = vi.fn().mockReturnValue({ onConflictDoNothing: txOnConflictDoNothing });
|
||||
const txInsert = vi.fn().mockReturnValue({ values: txInsertValues });
|
||||
db.transaction.mockImplementation(async (cb: (tx: unknown) => Promise<void>) =>
|
||||
cb({
|
||||
@@ -861,15 +863,24 @@ describe('UserRepository', () => {
|
||||
}),
|
||||
);
|
||||
|
||||
await repo.replaceViewerLibraries(2, []);
|
||||
await repo.replaceLibraries(2, []);
|
||||
expect(txDelete).toHaveBeenCalledWith(schema.userLibraryAccess);
|
||||
expect(txDeleteWhere).toHaveBeenCalledWith(expect.objectContaining({ op: 'eq', left: schema.userLibraryAccess.userId, right: 2 }));
|
||||
expect(txInsert).not.toHaveBeenCalled();
|
||||
|
||||
await repo.replaceViewerLibraries(2, [10, 11]);
|
||||
txDeleteWhere.mockClear();
|
||||
await repo.replaceLibraries(2, [10, 11]);
|
||||
expect(txDeleteWhere).toHaveBeenCalledWith({
|
||||
op: 'and',
|
||||
clauses: [
|
||||
{ op: 'eq', left: schema.userLibraryAccess.userId, right: 2 },
|
||||
{ op: 'notInArray', left: schema.userLibraryAccess.libraryId, right: [10, 11] },
|
||||
],
|
||||
});
|
||||
expect(txInsertValues).toHaveBeenCalledWith([
|
||||
{ userId: 2, libraryId: 10, accessLevel: 'viewer' },
|
||||
{ userId: 2, libraryId: 11, accessLevel: 'viewer' },
|
||||
]);
|
||||
expect(txOnConflictDoNothing).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { Inject, Injectable } from '@nestjs/common';
|
||||
import { createHash, randomBytes, randomUUID } from 'crypto';
|
||||
import { and, asc, count, desc, eq, ilike, inArray, isNull, ne, or, sql } from 'drizzle-orm';
|
||||
import { and, asc, count, desc, eq, ilike, inArray, isNull, ne, notInArray, or, sql } from 'drizzle-orm';
|
||||
import type { SQL } from 'drizzle-orm';
|
||||
import { NodePgDatabase } from 'drizzle-orm/node-postgres';
|
||||
import { hash } from 'bcryptjs';
|
||||
@@ -677,11 +677,21 @@ export class UserRepository {
|
||||
return rows.map((r) => r.id);
|
||||
}
|
||||
|
||||
async replaceViewerLibraries(userId: number, libraryIds: number[]): Promise<void> {
|
||||
/**
|
||||
* Libraries that stay assigned keep their access level: it is set per library from the library's
|
||||
* access settings, and resetting it here would silently revoke editing on every user save.
|
||||
*/
|
||||
async replaceLibraries(userId: number, libraryIds: number[]): Promise<void> {
|
||||
await this.db.transaction(async (tx) => {
|
||||
await tx.delete(schema.userLibraryAccess).where(eq(schema.userLibraryAccess.userId, userId));
|
||||
const ownedByUser = eq(schema.userLibraryAccess.userId, userId);
|
||||
await tx
|
||||
.delete(schema.userLibraryAccess)
|
||||
.where(libraryIds.length > 0 ? and(ownedByUser, notInArray(schema.userLibraryAccess.libraryId, libraryIds)) : ownedByUser);
|
||||
if (libraryIds.length > 0) {
|
||||
await tx.insert(schema.userLibraryAccess).values(libraryIds.map((libraryId) => ({ userId, libraryId, accessLevel: 'viewer' as const })));
|
||||
await tx
|
||||
.insert(schema.userLibraryAccess)
|
||||
.values(libraryIds.map((libraryId) => ({ userId, libraryId, accessLevel: 'viewer' as const })))
|
||||
.onConflictDoNothing();
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
@@ -43,7 +43,7 @@ describe('UserService', () => {
|
||||
setSuperuser: vi.fn(),
|
||||
assignViewerLibraries: vi.fn(),
|
||||
findLibraryIdsByUserId: vi.fn(),
|
||||
replaceViewerLibraries: vi.fn(),
|
||||
replaceLibraries: vi.fn(),
|
||||
findExistingLibraryIds: vi.fn(),
|
||||
findSettingsById: vi.fn(),
|
||||
};
|
||||
@@ -658,7 +658,7 @@ describe('UserService', () => {
|
||||
|
||||
await service.setLibraries(2, [3, 7, 3], reqUser({ isSuperuser: true }));
|
||||
|
||||
expect(userRepo.replaceViewerLibraries).toHaveBeenCalledWith(2, [3, 7]);
|
||||
expect(userRepo.replaceLibraries).toHaveBeenCalledWith(2, [3, 7]);
|
||||
});
|
||||
|
||||
it('getContentFilters throws when the target user does not exist', async () => {
|
||||
@@ -831,7 +831,7 @@ describe('UserService.updateSeriesCollapsePreferences', () => {
|
||||
setSuperuser: vi.fn(),
|
||||
assignViewerLibraries: vi.fn(),
|
||||
findLibraryIdsByUserId: vi.fn(),
|
||||
replaceViewerLibraries: vi.fn(),
|
||||
replaceLibraries: vi.fn(),
|
||||
findExistingLibraryIds: vi.fn(),
|
||||
};
|
||||
const contentFilterRepo = {
|
||||
|
||||
@@ -431,7 +431,7 @@ export class UserService {
|
||||
|
||||
const normalizedLibraryIds = this.uniqueIds(libraryIds);
|
||||
await this.assertKnownLibraryIds(normalizedLibraryIds);
|
||||
await this.userRepo.replaceViewerLibraries(targetUserId, normalizedLibraryIds);
|
||||
await this.userRepo.replaceLibraries(targetUserId, normalizedLibraryIds);
|
||||
}
|
||||
|
||||
async adminResetPassword(targetUserId: number, requestingUser: RequestUser) {
|
||||
|
||||
@@ -8,6 +8,7 @@ import { Permission, type LibraryFileSyncProgressEvent } from '@bookorbit/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 { FileRenameService } from '../src/modules/file-write/file-rename.service';
|
||||
import { extractCb7Metadata, extractCbzMetadata } from '../src/modules/metadata/lib/cbz-metadata';
|
||||
import { extractEpubMetadata } from '../src/modules/metadata/lib/epub';
|
||||
import { parseFb2File } from '../src/modules/metadata/lib/fb2-parser';
|
||||
@@ -1600,6 +1601,147 @@ describe('Metadata write operations (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, ()
|
||||
});
|
||||
|
||||
describe('access control', () => {
|
||||
it('rejects viewer forced renames and keeps personal ratings free of shared file effects', async () => {
|
||||
const library = await createLibraryWithFolder(context, { mode: 'book_per_file' });
|
||||
await createEpubFixture(library.folderPath, 'access/viewer.epub');
|
||||
await triggerAndWaitForLibraryScan(context, library.libraryId);
|
||||
const book = await locateBookFileByRelPath(context, library.libraryId, 'access/viewer.epub');
|
||||
const before = await readFile(book.absolutePath);
|
||||
const metadataBefore = await context.db.query.bookMetadata.findFirst({ where: eq(schema.bookMetadata.bookId, book.bookId) });
|
||||
const caller = await createUserAndLogin(context, { permissions: [Permission.LibraryEditMetadata] });
|
||||
await grantLibraryAccess(context, caller.userId, library.libraryId, 'viewer');
|
||||
await setLibraryFileWriteSettings(context.db, library.libraryId, { fileWriteEnabled: true, fileWriteEpubEnabled: true });
|
||||
await context.db.update(schema.libraries).set({ fileRenameEnabled: true }).where(eq(schema.libraries.id, library.libraryId));
|
||||
const renameService = context.app.get(FileRenameService);
|
||||
const writeService = context.app.get(FileWriteService);
|
||||
const performRename = vi.spyOn(renameService, 'performRename');
|
||||
const scheduleRename = vi.spyOn(renameService, 'scheduleRename');
|
||||
const scheduleWrite = vi.spyOn(writeService, 'scheduleWrite');
|
||||
try {
|
||||
const rename = await context.app.inject({
|
||||
method: 'POST',
|
||||
url: `/api/v1/books/${book.bookId}/rename-files`,
|
||||
headers: authHeader(caller.accessToken),
|
||||
});
|
||||
expect(rename.statusCode).toBe(403);
|
||||
expect(performRename).not.toHaveBeenCalled();
|
||||
for (const rating of [4, null]) {
|
||||
const response = await context.app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/books/bulk-set-rating',
|
||||
headers: authHeader(caller.accessToken),
|
||||
payload: { bookIds: [book.bookId], rating },
|
||||
});
|
||||
expect(response.statusCode).toBe(204);
|
||||
const personal = await context.db.query.userBookRatings.findFirst({ where: eq(schema.userBookRatings.userId, caller.userId) });
|
||||
expect(personal).toMatchObject({ bookId: book.bookId, rating });
|
||||
}
|
||||
expect(scheduleWrite).not.toHaveBeenCalled();
|
||||
expect(scheduleRename).not.toHaveBeenCalled();
|
||||
expect(await readFile(book.absolutePath)).toEqual(before);
|
||||
const metadataAfter = await context.db.query.bookMetadata.findFirst({ where: eq(schema.bookMetadata.bookId, book.bookId) });
|
||||
expect(metadataAfter).toEqual(metadataBefore);
|
||||
} finally {
|
||||
performRename.mockRestore();
|
||||
scheduleRename.mockRestore();
|
||||
scheduleWrite.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it('enforces editor access for every manual metadata save and rejects mixed-library bulk writes atomically', async () => {
|
||||
const editor = await createLibraryWithFolder(context, { mode: 'book_per_folder' });
|
||||
const viewer = await createLibraryWithFolder(context, { mode: 'book_per_folder' });
|
||||
const caller = await createUserAndLogin(context, { permissions: [Permission.LibraryEditMetadata] });
|
||||
await grantLibraryAccess(context, caller.userId, editor.libraryId, 'editor');
|
||||
await grantLibraryAccess(context, caller.userId, viewer.libraryId, 'viewer');
|
||||
const bookIds: number[] = [];
|
||||
for (const library of [editor, viewer]) {
|
||||
const [book] = await context.db
|
||||
.insert(schema.books)
|
||||
.values({
|
||||
libraryId: library.libraryId,
|
||||
libraryFolderId: library.libraryFolderId,
|
||||
folderPath: library.folderPath,
|
||||
})
|
||||
.returning({ id: schema.books.id });
|
||||
await context.db.insert(schema.bookMetadata).values({ bookId: book.id, title: 'Original', publisher: 'Original publisher' });
|
||||
bookIds.push(book.id);
|
||||
}
|
||||
const routes = [
|
||||
{ path: 'metadata', payload: { title: 'Forbidden title' } },
|
||||
{ path: 'metadata-and-locks', payload: { metadata: { title: 'Forbidden title' }, lockedFields: ['title'] } },
|
||||
{ path: 'metadata-locks', payload: { lockedFields: ['title'] } },
|
||||
];
|
||||
for (const route of routes) {
|
||||
const response = await context.app.inject({
|
||||
method: 'PATCH',
|
||||
url: `/api/v1/books/${bookIds[1]}/${route.path}`,
|
||||
headers: authHeader(caller.accessToken),
|
||||
payload: route.payload,
|
||||
});
|
||||
expect(response.statusCode).toBe(403);
|
||||
const metadata = await context.db.query.bookMetadata.findFirst({ where: eq(schema.bookMetadata.bookId, bookIds[1]) });
|
||||
expect(metadata?.title).toBe('Original');
|
||||
expect(metadata?.lockedFields).toEqual([]);
|
||||
}
|
||||
for (const route of ['bulk-refresh-metadata', 'bulk-re-extract-cover']) {
|
||||
const response = await context.app.inject({
|
||||
method: 'POST',
|
||||
url: `/api/v1/books/${route}`,
|
||||
headers: authHeader(caller.accessToken),
|
||||
payload: { bookIds },
|
||||
});
|
||||
expect(response.statusCode).toBe(403);
|
||||
expect(response.headers['content-type']).not.toContain('text/event-stream');
|
||||
}
|
||||
const mixed = await context.app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/books/bulk-set-metadata',
|
||||
headers: authHeader(caller.accessToken),
|
||||
payload: { bookIds, field: 'publisher', value: 'Forbidden publisher' },
|
||||
});
|
||||
expect(mixed.statusCode).toBe(403);
|
||||
for (const bookId of bookIds) {
|
||||
const metadata = await context.db.query.bookMetadata.findFirst({ where: eq(schema.bookMetadata.bookId, bookId) });
|
||||
expect(metadata?.publisher).toBe('Original publisher');
|
||||
}
|
||||
for (const level of ['editor', 'owner'] as const) {
|
||||
await grantLibraryAccess(context, caller.userId, viewer.libraryId, level);
|
||||
const rename = await context.app.inject({
|
||||
method: 'POST',
|
||||
url: `/api/v1/books/${bookIds[1]}/rename-files`,
|
||||
headers: authHeader(caller.accessToken),
|
||||
});
|
||||
expect(rename.statusCode).toBe(204);
|
||||
const response = await context.app.inject({
|
||||
method: 'PATCH',
|
||||
url: `/api/v1/books/${bookIds[1]}/metadata-and-locks`,
|
||||
headers: authHeader(caller.accessToken),
|
||||
payload: { metadata: { title: `Allowed ${level}` }, lockedFields: [] },
|
||||
});
|
||||
expect(response.statusCode).toBe(200);
|
||||
const metadata = await context.db.query.bookMetadata.findFirst({ where: eq(schema.bookMetadata.bookId, bookIds[1]) });
|
||||
expect(metadata?.title).toBe(`Allowed ${level}`);
|
||||
}
|
||||
await grantLibraryAccess(context, caller.userId, viewer.libraryId, 'viewer');
|
||||
const downgraded = await context.app.inject({
|
||||
method: 'PATCH',
|
||||
url: `/api/v1/books/${bookIds[1]}/metadata`,
|
||||
headers: authHeader(caller.accessToken),
|
||||
payload: { title: 'Forbidden after downgrade' },
|
||||
});
|
||||
expect(downgraded.statusCode).toBe(403);
|
||||
const renameAfterDowngrade = await context.app.inject({
|
||||
method: 'POST',
|
||||
url: `/api/v1/books/${bookIds[1]}/rename-files`,
|
||||
headers: authHeader(caller.accessToken),
|
||||
});
|
||||
expect(renameAfterDowngrade.statusCode).toBe(403);
|
||||
const readable = await context.app.inject({ method: 'GET', url: `/api/v1/books/${bookIds[1]}`, headers: authHeader(caller.accessToken) });
|
||||
expect(readable.statusCode).toBe(200);
|
||||
expect(readable.json().title).toBe('Allowed owner');
|
||||
});
|
||||
|
||||
it('enforces permission and library access for PATCH /books/:id/metadata', async () => {
|
||||
const library = await createLibraryWithFolder(context, { mode: 'book_per_file' });
|
||||
await createPdfFixture(library.folderPath, 'access/patch.pdf');
|
||||
|
||||
@@ -295,6 +295,20 @@ describe('Users admin lifecycle (e2e)', { timeout: 180_000 }, () => {
|
||||
});
|
||||
expectError(invalidPermissionUpdate, 400, 'permissionNames');
|
||||
|
||||
await ctx.db
|
||||
.update(schema.userLibraryAccess)
|
||||
.set({ accessLevel: 'editor' })
|
||||
.where(and(eq(schema.userLibraryAccess.userId, created.id), eq(schema.userLibraryAccess.libraryId, libraryA.libraryId)));
|
||||
const readAccessLevels = async () =>
|
||||
Object.fromEntries(
|
||||
(
|
||||
await ctx.db
|
||||
.select({ libraryId: schema.userLibraryAccess.libraryId, accessLevel: schema.userLibraryAccess.accessLevel })
|
||||
.from(schema.userLibraryAccess)
|
||||
.where(eq(schema.userLibraryAccess.userId, created.id))
|
||||
).map((row) => [row.libraryId, row.accessLevel]),
|
||||
);
|
||||
|
||||
const libraryUpdate = await ctx.app.inject({
|
||||
method: 'PUT',
|
||||
url: `/api/v1/users/${created.id}/libraries`,
|
||||
@@ -314,6 +328,25 @@ describe('Users admin lifecycle (e2e)', { timeout: 180_000 }, () => {
|
||||
expect((readLibrariesAfterUpdate.json() as number[]).toSorted((a, b) => a - b)).toEqual(
|
||||
[libraryA.libraryId, libraryB.libraryId].toSorted((a, b) => a - b),
|
||||
);
|
||||
expect(await readAccessLevels()).toEqual({ [libraryA.libraryId]: 'editor', [libraryB.libraryId]: 'viewer' });
|
||||
|
||||
const narrowedLibraryUpdate = await ctx.app.inject({
|
||||
method: 'PUT',
|
||||
url: `/api/v1/users/${created.id}/libraries`,
|
||||
headers: authHeader(manageUsersAdmin.accessToken),
|
||||
payload: { libraryIds: [libraryA.libraryId] },
|
||||
});
|
||||
expect(narrowedLibraryUpdate.statusCode).toBe(204);
|
||||
expect(await readAccessLevels()).toEqual({ [libraryA.libraryId]: 'editor' });
|
||||
|
||||
const restoredLibraryUpdate = await ctx.app.inject({
|
||||
method: 'PUT',
|
||||
url: `/api/v1/users/${created.id}/libraries`,
|
||||
headers: authHeader(manageUsersAdmin.accessToken),
|
||||
payload: { libraryIds: [libraryA.libraryId, libraryB.libraryId] },
|
||||
});
|
||||
expect(restoredLibraryUpdate.statusCode).toBe(204);
|
||||
expect(await readAccessLevels()).toEqual({ [libraryA.libraryId]: 'editor', [libraryB.libraryId]: 'viewer' });
|
||||
|
||||
const deleteResponse = await ctx.app.inject({
|
||||
method: 'DELETE',
|
||||
|
||||
Reference in New Issue
Block a user