fix(auth): honor delegated admin permissions

Allow manage_users administrators to manage user content filters and allow manage_email administrators to share providers they own. Preserve provider ownership and keep system mail designation restricted to superusers.

Fixes #1401

server/src/modules/email/email-provider.service.test.ts

@@ -4,7 +4,7 @@ import { EmailProviderRepository } from './email-provider.repository';
 import { EmailEncryptionService } from './email-encryption.service';
 import { EmailTransportService } from './email-transport.service';
 import type { RequestUser } from '../../common/types/request-user';
-import { EMPTY_CONTENT_FILTER_RULES } from '@bookorbit/types';
+import { EMPTY_CONTENT_FILTER_RULES, Permission } from '@bookorbit/types';
 
 describe('EmailProviderService', () => {
   let service: EmailProviderService;
@@ -36,6 +36,11 @@ describe('EmailProviderService', () => {
     contentFilters: EMPTY_CONTENT_FILTER_RULES,
   };
 
+  const mockEmailManager: RequestUser = {
+    ...mockUser,
+    permissions: [Permission.ManageEmail],
+  };
+
   const mockProvider = {
     id: 10,
     userId: 1,
@@ -62,6 +67,8 @@ describe('EmailProviderService', () => {
       clearDefault: vi.fn(),
       setDefault: vi.fn().mockResolvedValue([mockProvider]),
       setSharedByOwner: vi.fn().mockResolvedValue([{ ...mockProvider, isShared: true }]),
+      setSystemProvider: vi.fn().mockResolvedValue({ ...mockProvider, isSystemProvider: true }),
+      clearSystemProvider: vi.fn(),
     } as unknown as EmailProviderRepository;
 
     encryption = {
@@ -186,14 +193,43 @@ describe('EmailProviderService', () => {
   });
 
   describe('toggleShared', () => {
-    it('should allow superuser to toggle sharing', async () => {
+    it('allows a superuser to toggle sharing', async () => {
       const result = await service.toggleShared(10, mockAdmin);
       expect(result.isShared).toBe(true);
-      expect(repo.setSharedByOwner).toHaveBeenCalled();
+      expect(repo.setSharedByOwner).toHaveBeenCalledWith(10, mockAdmin.id, true);
     });
 
-    it('should throw ForbiddenException for non-superuser', async () => {
+    it('allows a manage_email user to toggle sharing on an owned provider', async () => {
+      const result = await service.toggleShared(10, mockEmailManager);
+
+      expect(result.isShared).toBe(true);
+      expect(repo.setSharedByOwner).toHaveBeenCalledWith(10, mockEmailManager.id, true);
+    });
+
+    it('rejects a caller without manage_email before looking up the provider', async () => {
       await expect(service.toggleShared(10, mockUser)).rejects.toThrow(ForbiddenException);
+      expect(repo.findById).not.toHaveBeenCalled();
+      expect(repo.setSharedByOwner).not.toHaveBeenCalled();
+    });
+
+    it('does not let manage_email bypass provider ownership', async () => {
+      (repo.findById as vi.Mock).mockResolvedValue([{ ...mockProvider, userId: 99 }]);
+
+      await expect(service.toggleShared(10, mockEmailManager)).rejects.toThrow(ForbiddenException);
+      expect(repo.setSharedByOwner).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('system provider authorization', () => {
+    it('keeps setting the system provider restricted to superusers', async () => {
+      await expect(service.setSystemProvider(10, mockEmailManager)).rejects.toThrow(ForbiddenException);
+      expect(repo.findById).not.toHaveBeenCalled();
+      expect(repo.setSystemProvider).not.toHaveBeenCalled();
+    });
+
+    it('keeps clearing the system provider restricted to superusers', async () => {
+      await expect(service.clearSystemProvider(mockEmailManager)).rejects.toThrow(ForbiddenException);
+      expect(repo.clearSystemProvider).not.toHaveBeenCalled();
     });
   });
 

server/src/modules/email/email-provider.service.ts

@@ -1,4 +1,5 @@
 import { BadRequestException, ConflictException, ForbiddenException, Injectable, NotFoundException } from '@nestjs/common';
+import { Permission } from '@bookorbit/types';
 
 import type { RequestUser } from '../../common/types/request-user';
 import { emailProviders } from '../../db/schema';
@@ -100,7 +101,9 @@ export class EmailProviderService {
   }
 
   async toggleShared(id: number, user: RequestUser) {
-    if (!user.isSuperuser) throw new ForbiddenException('Only superusers can share providers');
+    if (!user.isSuperuser && !user.permissions.includes(Permission.ManageEmail)) {
+      throw new ForbiddenException(`Missing permission: ${Permission.ManageEmail}`);
+    }
     const provider = await this.getOwned(id, user);
     const [updated] = await this.repo.setSharedByOwner(id, user.id, !provider.isShared);
     if (!updated) throw new NotFoundException('Provider not found');

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

@@ -657,10 +657,27 @@ describe('UserService', () => {
     expect(contentFilterRepo.findByUserIdWithNames).toHaveBeenCalledWith(5);
   });
 
-  it('getContentFilters blocks non-superusers from reading another user filters', async () => {
+  it('getContentFilters allows manage_users to read another user filters', async () => {
+    const filters = {
+      includeTags: [{ id: 1, name: 'Sci-Fi' }],
+      excludeTags: [],
+      includeGenres: [],
+      excludeGenres: [],
+    };
     userRepo.findByIdWithPermissions.mockResolvedValue({ id: 5, isSuperuser: false });
+    contentFilterRepo.findByUserIdWithNames.mockResolvedValue(filters);
 
-    await expect(service.getContentFilters(5, reqUser({ id: 1, isSuperuser: false }))).rejects.toBeInstanceOf(ForbiddenException);
+    await expect(service.getContentFilters(5, reqUser({ id: 1, isSuperuser: false, permissions: [Permission.ManageUsers] }))).resolves.toEqual(
+      filters,
+    );
+    expect(contentFilterRepo.findByUserIdWithNames).toHaveBeenCalledWith(5);
+  });
+
+  it('getContentFilters blocks callers without manage_users from reading another user filters', async () => {
+    await expect(
+      service.getContentFilters(5, reqUser({ id: 1, isSuperuser: false, permissions: [Permission.LibraryDownload] })),
+    ).rejects.toBeInstanceOf(ForbiddenException);
+    expect(userRepo.findByIdWithPermissions).not.toHaveBeenCalled();
     expect(contentFilterRepo.findByUserIdWithNames).not.toHaveBeenCalled();
   });
 
@@ -687,17 +704,41 @@ describe('UserService', () => {
     await expect(service.setContentFilters(5, {} as any, reqUser({ isSuperuser: true }))).rejects.toBeInstanceOf(NotFoundException);
   });
 
-  it('setContentFilters blocks non-superusers from updating filters', async () => {
+  it('setContentFilters allows manage_users to update filters', async () => {
     userRepo.findByIdWithPermissions.mockResolvedValue({ id: 5, isSuperuser: false });
 
-    await expect(service.setContentFilters(5, {} as any, reqUser({ isSuperuser: false }))).rejects.toBeInstanceOf(ForbiddenException);
-    expect(contentFilterRepo.replaceFilters).not.toHaveBeenCalled();
+    await expect(
+      service.setContentFilters(
+        5,
+        { includeTagIds: [1], excludeGenreIds: [3], seeOwnRequestedBooks: true } as any,
+        reqUser({ isSuperuser: false, permissions: [Permission.ManageUsers] }),
+      ),
+    ).resolves.toBeUndefined();
+
+    expect(contentFilterRepo.replaceFilters).toHaveBeenCalledWith(5, {
+      includeTagIds: [1],
+      excludeTagIds: [],
+      includeGenreIds: [],
+      excludeGenreIds: [3],
+    });
+    expect(userRepo.update).toHaveBeenCalledWith(5, { seeOwnRequestedBooks: true });
   });
 
-  it('setContentFilters rejects administrator targets', async () => {
+  it('setContentFilters blocks callers without manage_users from updating filters', async () => {
+    await expect(
+      service.setContentFilters(5, {} as any, reqUser({ isSuperuser: false, permissions: [Permission.LibraryDownload] })),
+    ).rejects.toBeInstanceOf(ForbiddenException);
+    expect(userRepo.findByIdWithPermissions).not.toHaveBeenCalled();
+    expect(contentFilterRepo.replaceFilters).not.toHaveBeenCalled();
+    expect(userRepo.update).not.toHaveBeenCalled();
+  });
+
+  it('setContentFilters rejects superuser targets even for manage_users callers', async () => {
     userRepo.findByIdWithPermissions.mockResolvedValue({ id: 5, isSuperuser: true });
 
-    await expect(service.setContentFilters(5, {} as any, reqUser({ isSuperuser: true }))).rejects.toBeInstanceOf(BadRequestException);
+    await expect(
+      service.setContentFilters(5, {} as any, reqUser({ isSuperuser: false, permissions: [Permission.ManageUsers] })),
+    ).rejects.toBeInstanceOf(BadRequestException);
     expect(contentFilterRepo.replaceFilters).not.toHaveBeenCalled();
   });
 

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

@@ -522,20 +522,20 @@ export class UserService {
   }
 
   async getContentFilters(targetUserId: number, requestingUser: RequestUser) {
-    const target = await this.userRepo.findByIdWithPermissions(targetUserId);
-    if (!target) throw new NotFoundException('User not found');
-    if (targetUserId !== requestingUser.id && !requestingUser.isSuperuser) {
+    if (targetUserId !== requestingUser.id && !this.canManageUsers(requestingUser)) {
       throw new ForbiddenException('Cannot view another user content filters');
     }
+    const target = await this.userRepo.findByIdWithPermissions(targetUserId);
+    if (!target) throw new NotFoundException('User not found');
     return this.contentFilterRepo.findByUserIdWithNames(targetUserId);
   }
 
   async setContentFilters(targetUserId: number, dto: SetContentFiltersDto, requestingUser: RequestUser) {
+    if (!this.canManageUsers(requestingUser)) {
+      throw new ForbiddenException(`Missing permission: ${Permission.ManageUsers}`);
+    }
     const target = await this.userRepo.findByIdWithPermissions(targetUserId);
     if (!target) throw new NotFoundException('User not found');
-    if (!requestingUser.isSuperuser) {
-      throw new ForbiddenException('Only administrators can set content filters');
-    }
     if (target.isSuperuser) {
       throw new BadRequestException('Content filters cannot be applied to administrators');
     }
@@ -551,4 +551,8 @@ export class UserService {
       await this.userRepo.update(targetUserId, { seeOwnRequestedBooks: dto.seeOwnRequestedBooks });
     }
   }
+
+  private canManageUsers(user: RequestUser): boolean {
+    return user.isSuperuser || user.permissions.includes(Permission.ManageUsers);
+  }
 }