From f39136d9ce6ee45ae9435c946c98dc3c6ff1759c Mon Sep 17 00:00:00 2001 From: wangziqi Date: Thu, 23 Jul 2026 12:23:33 +0800 Subject: [PATCH] fix: close permission review gaps --- .../permission-state.integration.test.tsx | 6 +- .../StudentProfileContent/index.tsx | 13 ++-- apps/admin/src/layouts/MainLayout.tsx | 66 ++++++++++++++----- apps/admin/src/pages/Attendance/index.tsx | 4 +- apps/admin/src/pages/Students/index.tsx | 29 ++++---- .../organizations.controller.spec.ts | 19 ++++++ .../organizations/organizations.controller.ts | 2 +- apps/server/src/students/dto/student.dto.ts | 3 +- .../src/students/students.lifecycle.spec.ts | 27 ++------ apps/server/src/students/students.service.ts | 39 ++++------- 10 files changed, 116 insertions(+), 92 deletions(-) create mode 100644 apps/server/src/organizations/organizations.controller.spec.ts diff --git a/apps/admin/src/auth/permission-state.integration.test.tsx b/apps/admin/src/auth/permission-state.integration.test.tsx index bd93b98..04d7198 100644 --- a/apps/admin/src/auth/permission-state.integration.test.tsx +++ b/apps/admin/src/auth/permission-state.integration.test.tsx @@ -54,14 +54,12 @@ describe('permission state', () => { expect(container?.textContent).toContain('编辑学生'); }); - it('fails closed after profile refresh failure', async () => { + it('stays fail-closed while profile verification is retried after a failure', async () => { writePermissions(['student:edit']); beginPermissionVerification(); - clearPermissions('ready'); - expect(readPermissionState()).toEqual({ permissions: [], status: 'ready' }); + expect(readPermissionState()).toEqual({ permissions: [], status: 'loading' }); await renderPermissionButton(); expect(container?.textContent).not.toContain('编辑学生'); - expect(localStorage.getItem('permissions')).toBeNull(); }); }); diff --git a/apps/admin/src/components/StudentProfileContent/index.tsx b/apps/admin/src/components/StudentProfileContent/index.tsx index 398df2c..17e5475 100644 --- a/apps/admin/src/components/StudentProfileContent/index.tsx +++ b/apps/admin/src/components/StudentProfileContent/index.tsx @@ -1401,9 +1401,12 @@ const StudentProfileContent: React.FC = ({ onClose, }) => { const { hasPermission, hasAnyPermission } = usePermission(); - const canViewOrganizations = hasPermission('organization:view'); - const canChooseOrganization = - canViewOrganizations && hasAnyPermission('student:create', 'student:edit'); + const canLoadOrganizations = hasAnyPermission( + 'organization:view', + 'student:create', + 'student:edit', + ); + const canChooseOrganization = hasAnyPermission('student:create', 'student:edit'); const [aggregateData, setAggregateData] = useState(null); const [organizations, setOrganizations] = useState>([]); const [loading, setLoading] = useState(false); @@ -1426,7 +1429,7 @@ const StudentProfileContent: React.FC = ({ }, [fetchData]); useEffect(() => { - if (!canViewOrganizations) { + if (!canLoadOrganizations) { setOrganizations([]); return; } @@ -1436,7 +1439,7 @@ const StudentProfileContent: React.FC = ({ setOrganizations(res as Array<{ id: number; name: string }>); }) .catch(() => {}); - }, [canViewOrganizations]); + }, [canLoadOrganizations]); const handlePreviewReport = useCallback(async () => { try { diff --git a/apps/admin/src/layouts/MainLayout.tsx b/apps/admin/src/layouts/MainLayout.tsx index 3e37c69..15eadd3 100644 --- a/apps/admin/src/layouts/MainLayout.tsx +++ b/apps/admin/src/layouts/MainLayout.tsx @@ -86,25 +86,57 @@ const MainLayout: React.FC = () => { useEffect(() => { let cancelled = false; - beginPermissionVerification(); - api - .get<{ id: number; username: string; permissions: string[]; roles?: string[] }>( - '/auth/profile', - ) - .then((profile) => { - if (cancelled) return; - writePermissions(profile.permissions || []); - const cachedUser = JSON.parse(localStorage.getItem('user') || '{}'); - const nextUser = { ...cachedUser, ...profile }; - localStorage.setItem('user', JSON.stringify(nextUser)); - setUser(nextUser); - }) - .catch(() => { - if (!cancelled) clearPermissions('ready'); - // The API interceptor handles expired/invalid sessions. - }); + let retryTimer: number | undefined; + let verificationInFlight = false; + + const verifyPermissions = () => { + if (cancelled || verificationInFlight || !localStorage.getItem('token')) return; + if (retryTimer !== undefined) { + window.clearTimeout(retryTimer); + retryTimer = undefined; + } + verificationInFlight = true; + beginPermissionVerification(); + api + .get<{ id: number; username: string; permissions: string[]; roles?: string[] }>( + '/auth/profile', + ) + .then((profile) => { + if (cancelled) return; + verificationInFlight = false; + writePermissions(profile.permissions || []); + const cachedUser = JSON.parse(localStorage.getItem('user') || '{}'); + const nextUser = { ...cachedUser, ...profile }; + localStorage.setItem('user', JSON.stringify(nextUser)); + setUser(nextUser); + }) + .catch(() => { + verificationInFlight = false; + if (cancelled || !localStorage.getItem('token')) return; + retryTimer = window.setTimeout(verifyPermissions, 5_000); + }); + }; + + const handleStorage = (event: StorageEvent) => { + if (event.key !== 'token' && event.key !== 'permissions') return; + beginPermissionVerification(); + window.location.reload(); + }; + const handleOnline = () => verifyPermissions(); + const handleVisibilityChange = () => { + if (document.visibilityState === 'visible') verifyPermissions(); + }; + + verifyPermissions(); + window.addEventListener('storage', handleStorage); + window.addEventListener('online', handleOnline); + document.addEventListener('visibilitychange', handleVisibilityChange); return () => { cancelled = true; + if (retryTimer !== undefined) window.clearTimeout(retryTimer); + window.removeEventListener('storage', handleStorage); + window.removeEventListener('online', handleOnline); + document.removeEventListener('visibilitychange', handleVisibilityChange); }; }, []); diff --git a/apps/admin/src/pages/Attendance/index.tsx b/apps/admin/src/pages/Attendance/index.tsx index 3935bdd..98b4c83 100644 --- a/apps/admin/src/pages/Attendance/index.tsx +++ b/apps/admin/src/pages/Attendance/index.tsx @@ -1146,9 +1146,9 @@ const AdminAttendanceArchive: React.FC<{ canEdit: boolean }> = ({ canEdit }) => value={studentSearch} onChange={(event) => setStudentSearch(event.target.value)} /> - +
diff --git a/apps/admin/src/pages/Students/index.tsx b/apps/admin/src/pages/Students/index.tsx index d984946..9959024 100644 --- a/apps/admin/src/pages/Students/index.tsx +++ b/apps/admin/src/pages/Students/index.tsx @@ -85,10 +85,15 @@ interface StudentFilterLookups { const StudentsPage: React.FC = () => { const { modal } = App.useApp(); - const { hasPermission, hasAnyPermission } = usePermission(); + const { hasPermission, hasAnyPermission, hasAllPermissions } = usePermission(); const canViewOrganizations = hasPermission('organization:view'); - const canChooseOrganization = - canViewOrganizations && hasAnyPermission('student:create', 'student:edit'); + const canLoadOrganizations = hasAnyPermission( + 'organization:view', + 'student:create', + 'student:edit', + ); + const canChooseOrganization = hasAnyPermission('student:create', 'student:edit'); + const canSyncJinshuju = hasAllPermissions('sync:read', 'sync:trigger'); const [data, setData] = useState([]); const [loading, setLoading] = useState(false); const [modalOpen, setModalOpen] = useState(false); @@ -194,7 +199,7 @@ const StudentsPage: React.FC = () => { }, [fetchData]); useEffect(() => { - if (canViewOrganizations) { + if (canLoadOrganizations) { api .get('/organizations', { params: { includeArchived: 'false' } }) .then((res: unknown) => { @@ -212,7 +217,7 @@ const StudentsPage: React.FC = () => { setTeacherOptions(res.teachers || []); }) .catch(() => {}); - }, [canViewOrganizations]); + }, [canLoadOrganizations]); const handleSave = async () => { const values = await form.validateFields(); setSaving(true); @@ -814,13 +819,11 @@ const StudentsPage: React.FC = () => { ) : null} - } - onClick={() => setJinshujuOpen(true)} - > - 同步金数据 - + {canSyncJinshuju ? ( + + ) : null} } @@ -1007,7 +1010,7 @@ const StudentsPage: React.FC = () => { - {hasPermission('sync:read') ? ( + {canSyncJinshuju ? ( setJinshujuOpen(false)} diff --git a/apps/server/src/organizations/organizations.controller.spec.ts b/apps/server/src/organizations/organizations.controller.spec.ts new file mode 100644 index 0000000..94ebfd2 --- /dev/null +++ b/apps/server/src/organizations/organizations.controller.spec.ts @@ -0,0 +1,19 @@ +import 'reflect-metadata'; +import { PERMISSION_KEY } from '../auth/decorators/permission.decorator'; +import { OrganizationsController } from './organizations.controller'; + +describe('OrganizationsController permissions', () => { + it('allows student editors to list organization options', () => { + expect(Reflect.getMetadata(PERMISSION_KEY, OrganizationsController.prototype.findAll)).toEqual([ + 'organization:view', + 'student:create', + 'student:edit', + ]); + }); + + it('keeps organization detail restricted to organization viewers', () => { + expect(Reflect.getMetadata(PERMISSION_KEY, OrganizationsController.prototype.findOne)).toEqual([ + 'organization:view', + ]); + }); +}); diff --git a/apps/server/src/organizations/organizations.controller.ts b/apps/server/src/organizations/organizations.controller.ts index 8ab4ac5..cd439f7 100644 --- a/apps/server/src/organizations/organizations.controller.ts +++ b/apps/server/src/organizations/organizations.controller.ts @@ -26,7 +26,7 @@ export class OrganizationsController { ) {} @Get() - @RequirePermission('organization:view') + @RequirePermission('organization:view', 'student:create', 'student:edit') findAll( @Query('includeArchived') includeArchived?: string, @Query('scope') scope?: 'all' | 'host' | 'external', diff --git a/apps/server/src/students/dto/student.dto.ts b/apps/server/src/students/dto/student.dto.ts index 18d53c2..276e1c8 100644 --- a/apps/server/src/students/dto/student.dto.ts +++ b/apps/server/src/students/dto/student.dto.ts @@ -33,9 +33,8 @@ export class CreateStudentDto { @IsString() emergencyPhone?: string; - @IsOptional() @IsInt() - organizationId?: number; + organizationId: number; @IsOptional() @IsString() diff --git a/apps/server/src/students/students.lifecycle.spec.ts b/apps/server/src/students/students.lifecycle.spec.ts index 3a85d87..fbe7b11 100644 --- a/apps/server/src/students/students.lifecycle.spec.ts +++ b/apps/server/src/students/students.lifecycle.spec.ts @@ -43,21 +43,6 @@ describe('StudentsService — archive lifecycle boundaries', () => { expect(repo.save).not.toHaveBeenCalled(); }); - it('defaults a new student to the active host organization when none is supplied', async () => { - const repo = { - create: jest.fn((value) => value), - save: jest.fn(async (value) => ({ ...value, id: 1 })), - }; - const organizationRepo = { - findOne: jest.fn().mockResolvedValue({ id: 7, isHost: true, status: 'active' }), - }; - - await expect(createService(repo, organizationRepo).create({ name: '张三' })).resolves.toEqual( - expect.objectContaining({ organizationId: 7 }), - ); - expect(repo.create).toHaveBeenCalledWith(expect.objectContaining({ organizationId: 7 })); - }); - it('returns not found for a missing student', async () => { const repo = { findOne: jest.fn().mockResolvedValue(null) }; await expect(createService(repo).findOne(404)).rejects.toBeInstanceOf(NotFoundException); @@ -65,13 +50,11 @@ describe('StudentsService — archive lifecycle boundaries', () => { it('builds export archive maps from profile and result rows', async () => { const profileRepo = { - find: jest.fn().mockResolvedValue([ - { - studentId: 1, - targetCollege: '北京大学', - collegeSchool: '北京职业技术学院', - }, - ]), + find: jest.fn().mockResolvedValue([{ + studentId: 1, + targetCollege: '北京大学', + collegeSchool: '北京职业技术学院', + }]), }; const resultRepo = { find: jest.fn().mockResolvedValue([{ studentId: 1, admissionStatus: 'pending' }]), diff --git a/apps/server/src/students/students.service.ts b/apps/server/src/students/students.service.ts index 4deafab..26a2279 100644 --- a/apps/server/src/students/students.service.ts +++ b/apps/server/src/students/students.service.ts @@ -164,9 +164,8 @@ export class StudentsService { } async create(dto: CreateStudentDto) { - const organizationId = dto.organizationId || (await this.getHostOrganizationId()); - await this.assertActiveOrganization(organizationId); - return this.repo.save(this.repo.create({ ...dto, organizationId })); + await this.assertActiveOrganization(dto.organizationId); + return this.repo.save(this.repo.create(dto)); } async update(id: number, dto: UpdateStudentDto) { @@ -315,9 +314,7 @@ export class StudentsService { }; } - private normalizeImportData( - importData: StudentWorkbookImport | StudentImportRow[], - ): StudentWorkbookImport { + private normalizeImportData(importData: StudentWorkbookImport | StudentImportRow[]): StudentWorkbookImport { if (Array.isArray(importData)) { return { students: importData, enrollments: [], examScores: [], learningRecords: [] }; } @@ -373,24 +370,18 @@ export class StudentsService { if (!phone) return imported; const enrollmentByClassName = new Map(); - for (const enrollmentRow of data.enrollments.filter( - (item) => this.normalizePhone(item.phone) === phone, - )) { + for (const enrollmentRow of data.enrollments.filter((item) => this.normalizePhone(item.phone) === phone)) { const enrollment = await this.upsertEnrollmentFromImport(studentId, enrollmentRow); if (!enrollment) continue; if (enrollment.className) enrollmentByClassName.set(enrollment.className, enrollment); imported++; } - for (const examRow of data.examScores.filter( - (item) => this.normalizePhone(item.phone) === phone, - )) { + for (const examRow of data.examScores.filter((item) => this.normalizePhone(item.phone) === phone)) { if (await this.upsertExamScoreFromImport(studentId, examRow, enrollmentByClassName)) { imported++; } } - for (const learningRow of data.learningRecords.filter( - (item) => this.normalizePhone(item.phone) === phone, - )) { + for (const learningRow of data.learningRecords.filter((item) => this.normalizePhone(item.phone) === phone)) { if (await this.upsertLearningRecordFromImport(studentId, learningRow)) { imported++; } @@ -399,9 +390,7 @@ export class StudentsService { } private async upsertProfileFromImport(studentId: number, row: StudentImportRow) { - const entity = - (await this.profileRepo.findOne({ where: { studentId } })) || - this.profileRepo.create({ studentId }); + const entity = (await this.profileRepo.findOne({ where: { studentId } })) || this.profileRepo.create({ studentId }); if (row.targetCollege?.trim()) entity.targetCollege = row.targetCollege.trim(); if (row.targetMajor?.trim()) entity.targetMajor = row.targetMajor.trim(); if (row.collegeSchool?.trim()) entity.collegeSchool = row.collegeSchool.trim(); @@ -414,12 +403,9 @@ export class StudentsService { } private async upsertResultFromImport(studentId: number, row: StudentImportRow) { - const entity = - (await this.resultRepo.findOne({ where: { studentId } })) || - this.resultRepo.create({ studentId }); + const entity = (await this.resultRepo.findOne({ where: { studentId } })) || this.resultRepo.create({ studentId }); if (row.cultureFinalScore !== undefined) entity.cultureFinalScore = row.cultureFinalScore; - if (row.professionalFinalScore !== undefined) - entity.professionalFinalScore = row.professionalFinalScore; + if (row.professionalFinalScore !== undefined) entity.professionalFinalScore = row.professionalFinalScore; if (row.admissionStatus?.trim()) entity.admissionStatus = row.admissionStatus.trim(); if (row.admittedCollege?.trim()) entity.admittedCollege = row.admittedCollege.trim(); if (row.admittedMajor?.trim()) entity.admittedMajor = row.admittedMajor.trim(); @@ -637,9 +623,10 @@ export class StudentsService { // ---- Filters ---- if (query?.keyword) { - qb.andWhere('(student.name LIKE :keyword OR student.studentNo LIKE :keyword)', { - keyword: `%${query.keyword}%`, - }); + qb.andWhere( + '(student.name LIKE :keyword OR student.studentNo LIKE :keyword)', + { keyword: `%${query.keyword}%` }, + ); } if (query?.organizationId) { qb.andWhere('student.organizationId = :orgId', { orgId: query.organizationId });