From a9c6578569ee819cedaebd8fe85363c394183845 Mon Sep 17 00:00:00 2001 From: xiong Date: Fri, 24 Jul 2026 09:41:37 +0800 Subject: [PATCH] =?UTF-8?q?=E4=BF=AE=E5=A4=8D=E7=AE=A1=E7=90=86=E5=91=98?= =?UTF-8?q?=E7=8F=AD=E7=BA=A7=E5=8F=AF=E8=A7=81=E8=8C=83=E5=9B=B4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/classes/classes.controller.spec.ts | 96 +++++++++++++++++++ apps/server/src/classes/classes.controller.ts | 21 ++-- 2 files changed, 110 insertions(+), 7 deletions(-) create mode 100644 apps/server/src/classes/classes.controller.spec.ts diff --git a/apps/server/src/classes/classes.controller.spec.ts b/apps/server/src/classes/classes.controller.spec.ts new file mode 100644 index 0000000..1c8d7e4 --- /dev/null +++ b/apps/server/src/classes/classes.controller.spec.ts @@ -0,0 +1,96 @@ +import { ClassesController } from './classes.controller'; +import { ClassesService } from './classes.service'; +import { OperationLogsService } from '../operation-logs/operation-logs.service'; +import { NotificationsService } from '../notifications/notifications.service'; +import { CaslAction, SubjectName } from '../authorization'; + +describe('ClassesController - class data scope', () => { + const service = { + getAccessibleClassIds: jest.fn(), + assertClassAccess: jest.fn(), + findAll: jest.fn(), + findOne: jest.fn(), + getSchedule: jest.fn(), + getAttendanceSummary: jest.fn(), + getStudents: jest.fn(), + getTeachers: jest.fn(), + }; + const authzService = { can: jest.fn() }; + const logService = {}; + const notificationsService = {}; + let controller: ClassesController; + + const request = (permissions: string[] = [], isSuperAdmin = false) => ({ + user: { + id: 21, + username: 'user', + permissions, + isSuperAdmin, + roles: [], + }, + }); + + beforeEach(() => { + jest.clearAllMocks(); + authzService.can.mockImplementation((req, action, subject) => { + if (subject !== SubjectName.Class) return false; + if (req.user.isSuperAdmin && action === CaslAction.Manage) return true; + if (action === CaslAction.Create) return req.user.permissions.includes('class:create'); + if (action === CaslAction.Update) return req.user.permissions.includes('class:edit'); + return false; + }); + service.getAccessibleClassIds.mockResolvedValue(undefined); + service.assertClassAccess.mockResolvedValue(undefined); + service.findAll.mockResolvedValue([]); + service.findOne.mockResolvedValue({ id: 8 }); + service.getSchedule.mockResolvedValue([]); + service.getAttendanceSummary.mockResolvedValue({}); + service.getStudents.mockResolvedValue([]); + service.getTeachers.mockResolvedValue([]); + controller = new ClassesController( + service as unknown as ClassesService, + logService as unknown as OperationLogsService, + notificationsService as unknown as NotificationsService, + authzService as never, + ); + }); + + it.each([ + ['class creator', request(['class:view', 'class:create'])], + ['class editor', request(['class:view', 'class:edit'])], + ['super admin', request([], true)], + ])('allows %s to list all classes', async (_label, req) => { + await controller.findAll({}, req); + + expect(service.getAccessibleClassIds).toHaveBeenCalledWith(21, true); + expect(service.findAll).toHaveBeenCalledWith({}, undefined); + }); + + it('keeps view-only users scoped to their assigned classes', async () => { + service.getAccessibleClassIds.mockResolvedValue([8]); + + await controller.findAll({}, request(['class:view'])); + + expect(service.getAccessibleClassIds).toHaveBeenCalledWith(21, false); + expect(service.findAll).toHaveBeenCalledWith({}, [8]); + }); + + it('allows a class creator to read an unassigned class through every detail endpoint', async () => { + const req = request(['class:view', 'class:create']); + + await controller.findOne('8', req); + await controller.getSchedule('8', {}, req); + await controller.getAttendanceSummary('8', {}, req); + await controller.getStudents('8', req); + await controller.getTeachers('8', req); + + expect(service.assertClassAccess).toHaveBeenCalledTimes(5); + expect(service.assertClassAccess).toHaveBeenCalledWith(21, 8, true); + }); + + it('requires a view-only user to be assigned before reading class details', async () => { + await controller.findOne('8', request(['class:view'])); + + expect(service.assertClassAccess).toHaveBeenCalledWith(21, 8, false); + }); +}); diff --git a/apps/server/src/classes/classes.controller.ts b/apps/server/src/classes/classes.controller.ts index a3e1c7e..2dc8dca 100644 --- a/apps/server/src/classes/classes.controller.ts +++ b/apps/server/src/classes/classes.controller.ts @@ -54,12 +54,20 @@ export class ClassesController { private readonly authz: AuthorizationService, ) {} - private assertReadAccess(req: AuthenticatedRequest, classId: number) { - // Legacy: Manage (super_admin) or Update (class:edit) grants broad class access - const canManageAll = + private canManageAllClasses(req: AuthenticatedRequest): boolean { + return ( this.authz.can(req, CaslAction.Manage, SubjectName.Class) || - this.authz.can(req, CaslAction.Update, SubjectName.Class); - return this.service.assertClassAccess(req.user.id, classId, canManageAll); + this.authz.can(req, CaslAction.Create, SubjectName.Class) || + this.authz.can(req, CaslAction.Update, SubjectName.Class) + ); + } + + private assertReadAccess(req: AuthenticatedRequest, classId: number) { + return this.service.assertClassAccess( + req.user.id, + classId, + this.canManageAllClasses(req), + ); } @Get() @@ -67,8 +75,7 @@ export class ClassesController { async findAll(@Query() query: QueryClassDto, @Request() req: AuthenticatedRequest) { const classIds = await this.service.getAccessibleClassIds( req.user.id, - this.authz.can(req, CaslAction.Manage, SubjectName.Class) || - this.authz.can(req, CaslAction.Update, SubjectName.Class), + this.canManageAllClasses(req), ); return this.service.findAll(query, classIds); }