From b3655da012b1ad37dc903ea86d72c3ebd298de07 Mon Sep 17 00:00:00 2001 From: wangziqi Date: Mon, 6 Jul 2026 00:38:51 +0800 Subject: [PATCH] fix: resolve 9 P0+P1 review findings (bills decorators, campus isolation bypass, ding raw crash, batch import counter, SSE multi-tab, interval leak, dept permissions, cycle prevention, tree depth) --- .../admin/src/components/NotificationBell.tsx | 10 ++++- .../src/attendance/attendance.service.ts | 2 +- apps/server/src/bills/bills.controller.ts | 2 + apps/server/src/common/campus-scope.ts | 8 +++- .../src/departments/departments.controller.ts | 10 +++++ .../src/departments/departments.service.ts | 38 ++++++++++++++++++- .../notifications/notifications.service.ts | 10 +++++ apps/server/src/students/students.service.ts | 1 + 8 files changed, 75 insertions(+), 6 deletions(-) diff --git a/apps/admin/src/components/NotificationBell.tsx b/apps/admin/src/components/NotificationBell.tsx index 8ca53e3..53d3fdb 100644 --- a/apps/admin/src/components/NotificationBell.tsx +++ b/apps/admin/src/components/NotificationBell.tsx @@ -58,6 +58,7 @@ const NotificationBell: React.FC = () => { }; const openRef = useRef(open); openRef.current = open; + const retryRef = useRef(null); // SSE connection — decoupled from popover open state useEffect(() => { @@ -74,9 +75,14 @@ const NotificationBell: React.FC = () => { }; es.onerror = () => { es.close(); - setInterval(fetchUnread, 60_000); + if (retryRef.current !== null) clearInterval(retryRef.current); + retryRef.current = window.setInterval(fetchUnread, 60_000); + }; + return () => { + es.close(); + clearInterval(retryRef.current ?? undefined); + retryRef.current = null; }; - return () => es.close(); }, []); const handleOpen = (visible: boolean) => { diff --git a/apps/server/src/attendance/attendance.service.ts b/apps/server/src/attendance/attendance.service.ts index b5a2cb5..1592437 100644 --- a/apps/server/src/attendance/attendance.service.ts +++ b/apps/server/src/attendance/attendance.service.ts @@ -233,7 +233,7 @@ export class AttendanceService { } return this.dingRawRepo.find({ - where: await this.scope.filter(where), + where, relations: ['matchedStudent'], order: { attendanceDate: 'DESC', checkInTime: 'ASC' }, }); diff --git a/apps/server/src/bills/bills.controller.ts b/apps/server/src/bills/bills.controller.ts index 76f9828..6aaa1b9 100644 --- a/apps/server/src/bills/bills.controller.ts +++ b/apps/server/src/bills/bills.controller.ts @@ -27,6 +27,8 @@ import { extractRequestInfo } from '../common/request-utils'; import { RequirePermission } from '../auth/decorators/permission.decorator'; import type { Response } from 'express'; +@UseGuards(JwtAuthGuard) +@Controller('bills') export class BillsController { constructor( private service: BillsService, diff --git a/apps/server/src/common/campus-scope.ts b/apps/server/src/common/campus-scope.ts index 3b6e494..684f600 100644 --- a/apps/server/src/common/campus-scope.ts +++ b/apps/server/src/common/campus-scope.ts @@ -44,7 +44,13 @@ export class CampusScope { } const ids = await this.getEffectiveScopeIds(); - if (ids.length === 0) return where; + if (ids.length === 0) { + // Non-super-admin with no scoping → match nothing, never leak unfiltered data + if (!this.isSuperAdmin) { + return { ...where, departmentId: In([]) } as unknown as T; + } + return where; + } return { ...where, departmentId: In(ids) } as unknown as T; } diff --git a/apps/server/src/departments/departments.controller.ts b/apps/server/src/departments/departments.controller.ts index 4b44e01..6b0c4e6 100644 --- a/apps/server/src/departments/departments.controller.ts +++ b/apps/server/src/departments/departments.controller.ts @@ -16,6 +16,7 @@ import { AssignUserDto, } from './dto/department.dto'; import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard'; +import { RequirePermission } from '../auth/decorators/permission.decorator'; @UseGuards(JwtAuthGuard) @Controller('departments') @@ -23,26 +24,31 @@ export class DepartmentsController { constructor(private readonly departmentsService: DepartmentsService) {} @Get() + @RequirePermission('department:view') findAll() { return this.departmentsService.findAll(); } @Get('tree') + @RequirePermission('department:view') findTree() { return this.departmentsService.findTree(); } @Get(':id') + @RequirePermission('department:view') findOne(@Param('id', ParseIntPipe) id: number) { return this.departmentsService.findOne(id); } @Post() + @RequirePermission('department:edit') create(@Body() dto: CreateDepartmentDto) { return this.departmentsService.create(dto); } @Put(':id') + @RequirePermission('department:edit') update( @Param('id', ParseIntPipe) id: number, @Body() dto: UpdateDepartmentDto, @@ -51,16 +57,19 @@ export class DepartmentsController { } @Delete(':id') + @RequirePermission('department:delete') remove(@Param('id', ParseIntPipe) id: number) { return this.departmentsService.remove(id); } @Get(':id/users') + @RequirePermission('department:view') getUsers(@Param('id', ParseIntPipe) id: number) { return this.departmentsService.getUsers(id); } @Post(':id/users') + @RequirePermission('department:edit') assignUser( @Param('id', ParseIntPipe) id: number, @Body() dto: AssignUserDto, @@ -69,6 +78,7 @@ export class DepartmentsController { } @Delete(':id/users/:userId') + @RequirePermission('department:delete') removeUser( @Param('id', ParseIntPipe) id: number, @Param('userId', ParseIntPipe) userId: number, diff --git a/apps/server/src/departments/departments.service.ts b/apps/server/src/departments/departments.service.ts index 2342425..d653fed 100644 --- a/apps/server/src/departments/departments.service.ts +++ b/apps/server/src/departments/departments.service.ts @@ -22,12 +22,33 @@ export class DepartmentsService { } async findTree(): Promise { + // Load ALL departments flat (no relations — avoids N+1 and only loads 1 level), + // then build the full tree in memory. const all = await this.deptRepo.find({ where: { status: 'active' }, order: { sortOrder: 'ASC', name: 'ASC' }, - relations: ['children'], }); - return all.filter((d) => d.parentId === null); + + const byParent = new Map(); + for (const dept of all) { + const key = dept.parentId ?? null; + const list = byParent.get(key); + if (list) { + list.push(dept); + } else { + byParent.set(key, [dept]); + } + } + + const attachChildren = (dept: Department): void => { + const children = byParent.get(dept.id) ?? []; + dept.children = children; + for (const child of children) attachChildren(child); + }; + + const roots = byParent.get(null) ?? []; + for (const root of roots) attachChildren(root); + return roots; } async findOne(id: number): Promise { @@ -43,6 +64,19 @@ export class DepartmentsService { async update(id: number, dto: UpdateDepartmentDto): Promise { const dept = await this.findOne(id); + + // Prevent parent cycles: parentId must not be the dept itself or one of its descendants + if (dto.parentId !== undefined && dto.parentId !== null) { + const newParentId = dto.parentId; + if (newParentId === id) { + throw new ConflictException('不能将部门的上级设为自身'); + } + const descendantIds = await this.getDescendantIds(id); + if (descendantIds.includes(newParentId)) { + throw new ConflictException('不能将部门的上级设为其子部门，会形成循环'); + } + } + Object.assign(dept, dto); return this.deptRepo.save(dept); } diff --git a/apps/server/src/notifications/notifications.service.ts b/apps/server/src/notifications/notifications.service.ts index 3bb5d00..f9581ba 100644 --- a/apps/server/src/notifications/notifications.service.ts +++ b/apps/server/src/notifications/notifications.service.ts @@ -9,6 +9,7 @@ import { CreateNotificationDto } from './dto/notification.dto'; @Injectable() export class NotificationsService { private subjects = new Map>(); + private subscriberCounts = new Map(); constructor( @InjectRepository(Notification) @@ -76,11 +77,20 @@ export class NotificationsService { subscribe(userId: number): Observable { if (!this.subjects.has(userId)) { this.subjects.set(userId, new Subject()); + this.subscriberCounts.set(userId, 0); } + this.subscriberCounts.set(userId, (this.subscriberCounts.get(userId) ?? 0) + 1); return this.subjects.get(userId)!.asObservable(); } unsubscribe(userId: number): void { + const count = (this.subscriberCounts.get(userId) ?? 0) - 1; + if (count > 0) { + this.subscriberCounts.set(userId, count); + return; + } + // Last subscriber gone → complete + clean up + this.subscriberCounts.delete(userId); const subj = this.subjects.get(userId); if (subj) { subj.complete(); diff --git a/apps/server/src/students/students.service.ts b/apps/server/src/students/students.service.ts index 86c6456..d87e4ef 100644 --- a/apps/server/src/students/students.service.ts +++ b/apps/server/src/students/students.service.ts @@ -142,6 +142,7 @@ export class StudentsService { tenantId: row.tenantId || undefined, }), ); + imported++; } return { message: `成功导入 ${imported} 名学生，跳过 ${skipped} 条（重复或空行）`,