fix(auth): restore metadata editing lost to the 3.5.0 library roles

Promote Viewer grants to Editor on book libraries for non-superusers who hold Edit metadata but
can edit nowhere, which is how every such account was left after the upgrade. Accounts with any
Editor or Owner role are left alone. The Users drawer now says where Edit metadata takes effect,
read from a new GET /users/:id/library-access route.

Closes #1902

server/src/db/migrations/0131_restore_metadata_editor_access.sql

@@ -0,0 +1,25 @@
+-- Since 3.5.0 metadata editing needs the Editor or Owner role on the book's library as well as the
+-- Edit metadata permission. Every existing grant was Viewer, so the upgrade silently took editing
+-- away from each non-superuser who held Edit metadata (issue 1902). Restore it on book libraries
+-- for the accounts the upgrade left unable to edit anywhere. An account that already holds Editor
+-- or Owner on some book library was set up under the new rules, so its Viewer grants are deliberate.
+UPDATE "user_library_access" AS "access"
+SET "access_level" = 'editor'
+FROM "libraries", "users"
+WHERE "libraries"."id" = "access"."library_id"
+  AND "libraries"."type" = 'books'
+  AND "users"."id" = "access"."user_id"
+  AND "users"."is_superuser" = false
+  AND "access"."access_level" = 'viewer'
+  AND EXISTS (
+    SELECT 1 FROM "user_permissions"
+    WHERE "user_permissions"."user_id" = "access"."user_id"
+      AND "user_permissions"."permission_name" = 'library_edit_metadata'
+  )
+  AND NOT EXISTS (
+    SELECT 1 FROM "user_library_access" AS "editing"
+    INNER JOIN "libraries" AS "editing_library" ON "editing_library"."id" = "editing"."library_id"
+    WHERE "editing"."user_id" = "access"."user_id"
+      AND "editing_library"."type" = 'books'
+      AND "editing"."access_level" IN ('editor', 'owner')
+  );

server/src/db/migrations/meta/_journal.json

@@ -918,6 +918,13 @@
       "when": 1792705454131,
       "tag": "0130_add_metadata_provider_icons",
       "breakpoints": true
+    },
+    {
+      "idx": 131,
+      "version": "7",
+      "when": 1792810767351,
+      "tag": "0131_restore_metadata_editor_access",
+      "breakpoints": true
     }
   ]
 }

server/src/modules/user/user.controller.test.ts

@@ -34,6 +34,7 @@ describe('UserController', () => {
     setPermissions: vi.fn(),
     setSuperuser: vi.fn(),
     getLibraryIds: vi.fn(),
+    getLibraryGrants: vi.fn(),
     setLibraries: vi.fn(),
     adminResetPassword: vi.fn(),
     unlockUser: vi.fn(),
@@ -174,6 +175,7 @@ describe('UserController', () => {
     await controller.setPermissions(8, dto as any, requester);
     await controller.setSuperuser(8, { isSuperuser: true }, requester);
     await controller.getLibraries(8);
+    await controller.getLibraryGrants(8);
     await controller.setLibraries(8, setLibrariesDto as any, requester);
     await controller.adminResetPassword(8, requester);
     await controller.unlockUser(8, requester);
@@ -185,6 +187,7 @@ describe('UserController', () => {
     expect(userService.setPermissions).toHaveBeenCalledWith(8, dto, requester);
     expect(userService.setSuperuser).toHaveBeenCalledWith(8, true, requester);
     expect(userService.getLibraryIds).toHaveBeenCalledWith(8);
+    expect(userService.getLibraryGrants).toHaveBeenCalledWith(8);
     expect(userService.setLibraries).toHaveBeenCalledWith(8, [1, 2], requester);
     expect(userService.adminResetPassword).toHaveBeenCalledWith(8, requester);
     expect(userService.unlockUser).toHaveBeenCalledWith(8, requester);

server/src/modules/user/user.controller.ts

@@ -277,6 +277,12 @@ export class UserController {
     return this.userService.getLibraryIds(id);
   }
 
+  @Get(':id/library-access')
+  @RequirePermission(Permission.ManageUsers)
+  getLibraryGrants(@Param('id', ParseIntPipe) id: number) {
+    return this.userService.getLibraryGrants(id);
+  }
+
   @Put(':id/libraries')
   @HttpCode(HttpStatus.NO_CONTENT)
   @RequirePermission(Permission.ManageUsers)

server/src/modules/user/user.repository.test.ts

@@ -851,6 +851,11 @@ describe('UserRepository', () => {
 
     await expect(repo.findLibraryIdsByUserId(2)).resolves.toEqual([10, 11]);
 
+    where.mockResolvedValueOnce([{ libraryId: 10, accessLevel: 'editor' }]);
+    await expect(repo.findLibraryGrantsByUserId(2)).resolves.toEqual([{ libraryId: 10, accessLevel: 'editor' }]);
+    expect(select).toHaveBeenLastCalledWith({ libraryId: schema.userLibraryAccess.libraryId, accessLevel: schema.userLibraryAccess.accessLevel });
+    expect(where).toHaveBeenLastCalledWith(expect.objectContaining({ op: 'eq', left: schema.userLibraryAccess.userId, right: 2 }));
+
     const txDeleteWhere = vi.fn().mockResolvedValue(undefined);
     const txDelete = vi.fn().mockReturnValue({ where: txDeleteWhere });
     const txOnConflictDoNothing = vi.fn().mockResolvedValue(undefined);

server/src/modules/user/user.repository.ts

@@ -6,7 +6,14 @@ import { NodePgDatabase } from 'drizzle-orm/node-postgres';
 import { hash } from 'bcryptjs';
 
 import { Permission, withRequiredPermissions } from '@bookorbit/types';
-import type { UserAttentionReason, UserListSortDirection, UserListSortField, UserListState, UserListSummary } from '@bookorbit/types';
+import type {
+  UserAttentionReason,
+  UserLibraryGrant,
+  UserListSortDirection,
+  UserListSortField,
+  UserListState,
+  UserListSummary,
+} from '@bookorbit/types';
 import { RequestUser } from '../../common/types/request-user';
 import { DB } from '../../db';
 import * as schema from '../../db/schema';
@@ -671,6 +678,13 @@ export class UserRepository {
     return rows.map((r) => r.libraryId);
   }
 
+  findLibraryGrantsByUserId(userId: number): Promise<UserLibraryGrant[]> {
+    return this.db
+      .select({ libraryId: schema.userLibraryAccess.libraryId, accessLevel: schema.userLibraryAccess.accessLevel })
+      .from(schema.userLibraryAccess)
+      .where(eq(schema.userLibraryAccess.userId, userId));
+  }
+
   async findExistingLibraryIds(libraryIds: number[]): Promise<number[]> {
     if (libraryIds.length === 0) return [];
     const rows = await this.db.select({ id: schema.libraries.id }).from(schema.libraries).where(inArray(schema.libraries.id, libraryIds));

server/src/modules/user/user.service.test.ts

@@ -43,6 +43,7 @@ describe('UserService', () => {
     setSuperuser: vi.fn(),
     assignViewerLibraries: vi.fn(),
     findLibraryIdsByUserId: vi.fn(),
+    findLibraryGrantsByUserId: vi.fn(),
     replaceLibraries: vi.fn(),
     findExistingLibraryIds: vi.fn(),
     findSettingsById: vi.fn(),
@@ -639,6 +640,25 @@ describe('UserService', () => {
     await expect(service.getLibraryIds(4)).resolves.toEqual([11, 12]);
   });
 
+  it('getLibraryGrants throws when target user does not exist', async () => {
+    userRepo.findByIdWithPermissions.mockResolvedValue(null);
+
+    await expect(service.getLibraryGrants(4)).rejects.toBeInstanceOf(NotFoundException);
+    expect(userRepo.findLibraryGrantsByUserId).not.toHaveBeenCalled();
+  });
+
+  it('getLibraryGrants returns each library with its access level', async () => {
+    const grants = [
+      { libraryId: 11, accessLevel: 'viewer' },
+      { libraryId: 12, accessLevel: 'editor' },
+    ];
+    userRepo.findByIdWithPermissions.mockResolvedValue({ id: 4, isSuperuser: false });
+    userRepo.findLibraryGrantsByUserId.mockResolvedValue(grants);
+
+    await expect(service.getLibraryGrants(4)).resolves.toEqual(grants);
+    expect(userRepo.findLibraryGrantsByUserId).toHaveBeenCalledWith(4);
+  });
+
   it('setLibraries rejects unknown libraries', async () => {
     userRepo.findByIdWithPermissions.mockResolvedValue({ id: 2, isSuperuser: false });
     userRepo.findExistingLibraryIds.mockResolvedValue([3]);
@@ -831,6 +851,7 @@ describe('UserService.updateSeriesCollapsePreferences', () => {
     setSuperuser: vi.fn(),
     assignViewerLibraries: vi.fn(),
     findLibraryIdsByUserId: vi.fn(),
+    findLibraryGrantsByUserId: vi.fn(),
     replaceLibraries: vi.fn(),
     findExistingLibraryIds: vi.fn(),
   };

server/src/modules/user/user.service.ts

@@ -3,7 +3,7 @@ import type { ConfigType } from '@nestjs/config';
 import { hash } from 'bcryptjs';
 import { randomBytes } from 'crypto';
 import { Permission } from '@bookorbit/types';
-import type { ProvisioningMethod, UserAttentionResponse, UserListSummary, UserSettings } from '@bookorbit/types';
+import type { ProvisioningMethod, UserAttentionResponse, UserLibraryGrant, UserListSummary, UserSettings } from '@bookorbit/types';
 
 import type { RequestUser } from '../../common/types/request-user';
 import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils';
@@ -422,6 +422,12 @@ export class UserService {
     return this.userRepo.findLibraryIdsByUserId(userId);
   }
 
+  async getLibraryGrants(userId: number): Promise<UserLibraryGrant[]> {
+    const user = await this.userRepo.findByIdWithPermissions(userId);
+    if (!user) throw new NotFoundException('User not found');
+    return this.userRepo.findLibraryGrantsByUserId(userId);
+  }
+
   async setLibraries(targetUserId: number, libraryIds: number[], requestingUser: RequestUser): Promise<void> {
     const target = await this.userRepo.findByIdWithPermissions(targetUserId);
     if (!target) throw new NotFoundException('User not found');

server/test/e2e/authorization-matrix/route-inventory/manifest.json

@@ -1,5 +1,5 @@
 {
-  "totalRoutes": 711,
+  "totalRoutes": 712,
   "byPermission": {
     "(none)": 298,
     "Permission.ManageMetadataConfig": 47,
@@ -8,7 +8,7 @@
     "Permission.BookDockAccess": 17,
     "Permission.ManageAppSettings": 62,
     "Permission.ManageLibraries": 24,
-    "Permission.ManageUsers": 13,
+    "Permission.ManageUsers": 14,
     "Permission.ManageEmail": 8,
     "Permission.KoboSync": 13,
     "Permission.NotificationAccess": 6,

server/test/e2e/authorization-matrix/route-inventory/routes-06.json

@@ -766,5 +766,17 @@
     "isPublic": false,
     "allowDefault": false,
     "useGuards": []
+  },
+  {
+    "file": "server/src/modules/user/user.controller.ts",
+    "className": "UserController",
+    "methodName": "getLibraryGrants",
+    "httpMethod": "GET",
+    "path": "/users/:id/library-access",
+    "permissions": ["Permission.ManageUsers"],
+    "libraryAccess": [],
+    "isPublic": false,
+    "allowDefault": false,
+    "useGuards": []
   }
 ]

server/test/metadata-editor-access-migration.e2e-spec.ts

@@ -0,0 +1,111 @@
+import { randomUUID } from 'crypto';
+import { readFile, readdir } from 'fs/promises';
+import { dirname, join } from 'path';
+import { fileURLToPath } from 'url';
+import { Client } from 'pg';
+
+const MIGRATIONS_DIRECTORY = join(dirname(fileURLToPath(import.meta.url)), '../src/db/migrations');
+const BACKFILL_MIGRATION = '0131_restore_metadata_editor_access.sql';
+
+function escapeIdentifier(identifier: string): string {
+  return `"${identifier.replace(/"/g, '""')}"`;
+}
+
+async function applyMigrations(client: Client, filenames: string[]): Promise<void> {
+  for (const filename of filenames) {
+    await client.query(await readFile(join(MIGRATIONS_DIRECTORY, filename), 'utf8'));
+  }
+}
+
+describe('Metadata editor access migration (e2e)', { timeout: 60_000 }, () => {
+  it('restores Editor on book libraries only for accounts the role enforcement left unable to edit', async () => {
+    const baseUrl = new URL(process.env.DATABASE_URL!);
+    const databaseName = `editor_access_${randomUUID().replace(/-/g, '').slice(0, 12)}_e2e`;
+    const targetUrl = new URL(baseUrl);
+    targetUrl.pathname = `/${databaseName}`;
+    const adminUrl = new URL(baseUrl);
+    adminUrl.pathname = '/postgres';
+
+    const admin = new Client({ connectionString: adminUrl.toString() });
+    let target: Client | undefined;
+    await admin.connect();
+
+    try {
+      await admin.query(`CREATE DATABASE ${escapeIdentifier(databaseName)}`);
+      target = new Client({ connectionString: targetUrl.toString() });
+      await target.connect();
+      await target.query(`
+        CREATE EXTENSION IF NOT EXISTS "uuid-ossp";
+        CREATE EXTENSION IF NOT EXISTS "pg_trgm";
+        CREATE EXTENSION IF NOT EXISTS "unaccent";
+        CREATE EXTENSION IF NOT EXISTS "vector";
+        CREATE OR REPLACE FUNCTION public.bookorbit_unaccent(value text)
+        RETURNS text
+        LANGUAGE sql
+        IMMUTABLE
+        PARALLEL SAFE
+        STRICT
+        AS $function$ SELECT public.unaccent('public.unaccent', value) $function$;
+      `);
+
+      const migrationFiles = (await readdir(MIGRATIONS_DIRECTORY)).filter((filename) => filename.endsWith('.sql')).sort();
+      await applyMigrations(
+        target,
+        migrationFiles.filter((filename) => filename < BACKFILL_MIGRATION),
+      );
+      await target.query(`
+        INSERT INTO libraries (id, type, name) VALUES (1, 'books', 'Fiction'), (2, 'books', 'Comics'), (3, 'podcasts', 'Shows');
+        INSERT INTO users (id, username, name, password_hash, is_superuser, active) VALUES
+          (1, 'upgraded', 'Upgraded', 'x', false, true),
+          (2, 'configured', 'Configured', 'x', false, true),
+          (3, 'reader', 'Reader', 'x', false, true),
+          (4, 'superuser', 'Superuser', 'x', true, true),
+          (5, 'podcast-owner', 'Podcast owner', 'x', false, true),
+          (6, 'suspended', 'Suspended', 'x', false, false);
+        INSERT INTO user_permissions (user_id, permission_name) VALUES
+          (1, 'library_edit_metadata'), (1, 'library_download'),
+          (2, 'library_edit_metadata'),
+          (3, 'library_download'),
+          (4, 'library_edit_metadata'),
+          (5, 'library_edit_metadata'),
+          (6, 'library_edit_metadata');
+        INSERT INTO user_library_access (user_id, library_id, access_level) VALUES
+          (1, 1, 'viewer'), (1, 2, 'viewer'), (1, 3, 'viewer'),
+          (2, 1, 'editor'), (2, 2, 'viewer'),
+          (3, 1, 'viewer'),
+          (4, 1, 'viewer'),
+          (5, 1, 'viewer'), (5, 3, 'owner'),
+          (6, 1, 'viewer');
+      `);
+
+      await applyMigrations(
+        target,
+        migrationFiles.filter((filename) => filename >= BACKFILL_MIGRATION),
+      );
+
+      const grants = await target.query<{ username: string; library: string; accessLevel: string }>(`
+        SELECT users.username, libraries.name AS library, user_library_access.access_level AS "accessLevel"
+        FROM user_library_access
+        INNER JOIN users ON users.id = user_library_access.user_id
+        INNER JOIN libraries ON libraries.id = user_library_access.library_id
+        ORDER BY users.id, libraries.id
+      `);
+      expect(grants.rows).toEqual([
+        { username: 'upgraded', library: 'Fiction', accessLevel: 'editor' },
+        { username: 'upgraded', library: 'Comics', accessLevel: 'editor' },
+        { username: 'upgraded', library: 'Shows', accessLevel: 'viewer' },
+        { username: 'configured', library: 'Fiction', accessLevel: 'editor' },
+        { username: 'configured', library: 'Comics', accessLevel: 'viewer' },
+        { username: 'reader', library: 'Fiction', accessLevel: 'viewer' },
+        { username: 'superuser', library: 'Fiction', accessLevel: 'viewer' },
+        { username: 'podcast-owner', library: 'Fiction', accessLevel: 'editor' },
+        { username: 'podcast-owner', library: 'Shows', accessLevel: 'owner' },
+        { username: 'suspended', library: 'Fiction', accessLevel: 'editor' },
+      ]);
+    } finally {
+      await target?.end();
+      await admin.query(`DROP DATABASE IF EXISTS ${escapeIdentifier(databaseName)} WITH (FORCE)`);
+      await admin.end();
+    }
+  });
+});