fix: close permission review gaps

This commit is contained in:
2026-07-23 12:23:33 +08:00
parent c98d37307e
commit f39136d9ce
10 changed files with 116 additions and 92 deletions

View File

@@ -54,14 +54,12 @@ describe('permission state', () => {
expect(container?.textContent).toContain('编辑学生'); 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']); writePermissions(['student:edit']);
beginPermissionVerification(); beginPermissionVerification();
clearPermissions('ready');
expect(readPermissionState()).toEqual({ permissions: [], status: 'ready' }); expect(readPermissionState()).toEqual({ permissions: [], status: 'loading' });
await renderPermissionButton(); await renderPermissionButton();
expect(container?.textContent).not.toContain('编辑学生'); expect(container?.textContent).not.toContain('编辑学生');
expect(localStorage.getItem('permissions')).toBeNull();
}); });
}); });

View File

@@ -1401,9 +1401,12 @@ const StudentProfileContent: React.FC<StudentProfileContentProps> = ({
onClose, onClose,
}) => { }) => {
const { hasPermission, hasAnyPermission } = usePermission(); const { hasPermission, hasAnyPermission } = usePermission();
const canViewOrganizations = hasPermission('organization:view'); const canLoadOrganizations = hasAnyPermission(
const canChooseOrganization = 'organization:view',
canViewOrganizations && hasAnyPermission('student:create', 'student:edit'); 'student:create',
'student:edit',
);
const canChooseOrganization = hasAnyPermission('student:create', 'student:edit');
const [aggregateData, setAggregateData] = useState<StudentProfileAggregate | null>(null); const [aggregateData, setAggregateData] = useState<StudentProfileAggregate | null>(null);
const [organizations, setOrganizations] = useState<Array<{ id: number; name: string }>>([]); const [organizations, setOrganizations] = useState<Array<{ id: number; name: string }>>([]);
const [loading, setLoading] = useState(false); const [loading, setLoading] = useState(false);
@@ -1426,7 +1429,7 @@ const StudentProfileContent: React.FC<StudentProfileContentProps> = ({
}, [fetchData]); }, [fetchData]);
useEffect(() => { useEffect(() => {
if (!canViewOrganizations) { if (!canLoadOrganizations) {
setOrganizations([]); setOrganizations([]);
return; return;
} }
@@ -1436,7 +1439,7 @@ const StudentProfileContent: React.FC<StudentProfileContentProps> = ({
setOrganizations(res as Array<{ id: number; name: string }>); setOrganizations(res as Array<{ id: number; name: string }>);
}) })
.catch(() => {}); .catch(() => {});
}, [canViewOrganizations]); }, [canLoadOrganizations]);
const handlePreviewReport = useCallback(async () => { const handlePreviewReport = useCallback(async () => {
try { try {

View File

@@ -86,25 +86,57 @@ const MainLayout: React.FC = () => {
useEffect(() => { useEffect(() => {
let cancelled = false; let cancelled = false;
beginPermissionVerification(); let retryTimer: number | undefined;
api let verificationInFlight = false;
.get<{ id: number; username: string; permissions: string[]; roles?: string[] }>(
'/auth/profile', const verifyPermissions = () => {
) if (cancelled || verificationInFlight || !localStorage.getItem('token')) return;
.then((profile) => { if (retryTimer !== undefined) {
if (cancelled) return; window.clearTimeout(retryTimer);
writePermissions(profile.permissions || []); retryTimer = undefined;
const cachedUser = JSON.parse(localStorage.getItem('user') || '{}'); }
const nextUser = { ...cachedUser, ...profile }; verificationInFlight = true;
localStorage.setItem('user', JSON.stringify(nextUser)); beginPermissionVerification();
setUser(nextUser); api
}) .get<{ id: number; username: string; permissions: string[]; roles?: string[] }>(
.catch(() => { '/auth/profile',
if (!cancelled) clearPermissions('ready'); )
// The API interceptor handles expired/invalid sessions. .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 () => { return () => {
cancelled = true; cancelled = true;
if (retryTimer !== undefined) window.clearTimeout(retryTimer);
window.removeEventListener('storage', handleStorage);
window.removeEventListener('online', handleOnline);
document.removeEventListener('visibilitychange', handleVisibilityChange);
}; };
}, []); }, []);

View File

@@ -1146,9 +1146,9 @@ const AdminAttendanceArchive: React.FC<{ canEdit: boolean }> = ({ canEdit }) =>
value={studentSearch} value={studentSearch}
onChange={(event) => setStudentSearch(event.target.value)} onChange={(event) => setStudentSearch(event.target.value)}
/> />
<Button icon={<ExportOutlined />} onClick={handleExport}> <PermissionButton permission="attendance:export" icon={<ExportOutlined />} onClick={handleExport}>
</Button> </PermissionButton>
</div> </div>
</header> </header>
<div className="student-legend"> <div className="student-legend">

View File

@@ -85,10 +85,15 @@ interface StudentFilterLookups {
const StudentsPage: React.FC = () => { const StudentsPage: React.FC = () => {
const { modal } = App.useApp(); const { modal } = App.useApp();
const { hasPermission, hasAnyPermission } = usePermission(); const { hasPermission, hasAnyPermission, hasAllPermissions } = usePermission();
const canViewOrganizations = hasPermission('organization:view'); const canViewOrganizations = hasPermission('organization:view');
const canChooseOrganization = const canLoadOrganizations = hasAnyPermission(
canViewOrganizations && hasAnyPermission('student:create', 'student:edit'); 'organization:view',
'student:create',
'student:edit',
);
const canChooseOrganization = hasAnyPermission('student:create', 'student:edit');
const canSyncJinshuju = hasAllPermissions('sync:read', 'sync:trigger');
const [data, setData] = useState<any[]>([]); const [data, setData] = useState<any[]>([]);
const [loading, setLoading] = useState(false); const [loading, setLoading] = useState(false);
const [modalOpen, setModalOpen] = useState(false); const [modalOpen, setModalOpen] = useState(false);
@@ -194,7 +199,7 @@ const StudentsPage: React.FC = () => {
}, [fetchData]); }, [fetchData]);
useEffect(() => { useEffect(() => {
if (canViewOrganizations) { if (canLoadOrganizations) {
api api
.get('/organizations', { params: { includeArchived: 'false' } }) .get('/organizations', { params: { includeArchived: 'false' } })
.then((res: unknown) => { .then((res: unknown) => {
@@ -212,7 +217,7 @@ const StudentsPage: React.FC = () => {
setTeacherOptions(res.teachers || []); setTeacherOptions(res.teachers || []);
}) })
.catch(() => {}); .catch(() => {});
}, [canViewOrganizations]); }, [canLoadOrganizations]);
const handleSave = async () => { const handleSave = async () => {
const values = await form.validateFields(); const values = await form.validateFields();
setSaving(true); setSaving(true);
@@ -814,13 +819,11 @@ const StudentsPage: React.FC = () => {
</Upload> </Upload>
</> </>
) : null} ) : null}
<PermissionButton {canSyncJinshuju ? (
permission="sync:read" <Button icon={<CloudUploadOutlined />} onClick={() => setJinshujuOpen(true)}>
icon={<CloudUploadOutlined />}
onClick={() => setJinshujuOpen(true)} </Button>
> ) : null}
</PermissionButton>
<PermissionButton <PermissionButton
permission="student:view" permission="student:view"
icon={<DownloadOutlined />} icon={<DownloadOutlined />}
@@ -1007,7 +1010,7 @@ const StudentsPage: React.FC = () => {
</Form> </Form>
</Modal> </Modal>
{hasPermission('sync:read') ? ( {canSyncJinshuju ? (
<JinshujuMatchModal <JinshujuMatchModal
open={jinshujuOpen} open={jinshujuOpen}
onClose={() => setJinshujuOpen(false)} onClose={() => setJinshujuOpen(false)}

View File

@@ -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',
]);
});
});

View File

@@ -26,7 +26,7 @@ export class OrganizationsController {
) {} ) {}
@Get() @Get()
@RequirePermission('organization:view') @RequirePermission('organization:view', 'student:create', 'student:edit')
findAll( findAll(
@Query('includeArchived') includeArchived?: string, @Query('includeArchived') includeArchived?: string,
@Query('scope') scope?: 'all' | 'host' | 'external', @Query('scope') scope?: 'all' | 'host' | 'external',

View File

@@ -33,9 +33,8 @@ export class CreateStudentDto {
@IsString() @IsString()
emergencyPhone?: string; emergencyPhone?: string;
@IsOptional()
@IsInt() @IsInt()
organizationId?: number; organizationId: number;
@IsOptional() @IsOptional()
@IsString() @IsString()

View File

@@ -43,21 +43,6 @@ describe('StudentsService — archive lifecycle boundaries', () => {
expect(repo.save).not.toHaveBeenCalled(); 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 () => { it('returns not found for a missing student', async () => {
const repo = { findOne: jest.fn().mockResolvedValue(null) }; const repo = { findOne: jest.fn().mockResolvedValue(null) };
await expect(createService(repo).findOne(404)).rejects.toBeInstanceOf(NotFoundException); 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 () => { it('builds export archive maps from profile and result rows', async () => {
const profileRepo = { const profileRepo = {
find: jest.fn().mockResolvedValue([ find: jest.fn().mockResolvedValue([{
{ studentId: 1,
studentId: 1, targetCollege: '北京大学',
targetCollege: '北京大学', collegeSchool: '北京职业技术学院',
collegeSchool: '北京职业技术学院', }]),
},
]),
}; };
const resultRepo = { const resultRepo = {
find: jest.fn().mockResolvedValue([{ studentId: 1, admissionStatus: 'pending' }]), find: jest.fn().mockResolvedValue([{ studentId: 1, admissionStatus: 'pending' }]),

View File

@@ -164,9 +164,8 @@ export class StudentsService {
} }
async create(dto: CreateStudentDto) { async create(dto: CreateStudentDto) {
const organizationId = dto.organizationId || (await this.getHostOrganizationId()); await this.assertActiveOrganization(dto.organizationId);
await this.assertActiveOrganization(organizationId); return this.repo.save(this.repo.create(dto));
return this.repo.save(this.repo.create({ ...dto, organizationId }));
} }
async update(id: number, dto: UpdateStudentDto) { async update(id: number, dto: UpdateStudentDto) {
@@ -315,9 +314,7 @@ export class StudentsService {
}; };
} }
private normalizeImportData( private normalizeImportData(importData: StudentWorkbookImport | StudentImportRow[]): StudentWorkbookImport {
importData: StudentWorkbookImport | StudentImportRow[],
): StudentWorkbookImport {
if (Array.isArray(importData)) { if (Array.isArray(importData)) {
return { students: importData, enrollments: [], examScores: [], learningRecords: [] }; return { students: importData, enrollments: [], examScores: [], learningRecords: [] };
} }
@@ -373,24 +370,18 @@ export class StudentsService {
if (!phone) return imported; if (!phone) return imported;
const enrollmentByClassName = new Map<string, StudentEnrollment>(); const enrollmentByClassName = new Map<string, StudentEnrollment>();
for (const enrollmentRow of data.enrollments.filter( for (const enrollmentRow of data.enrollments.filter((item) => this.normalizePhone(item.phone) === phone)) {
(item) => this.normalizePhone(item.phone) === phone,
)) {
const enrollment = await this.upsertEnrollmentFromImport(studentId, enrollmentRow); const enrollment = await this.upsertEnrollmentFromImport(studentId, enrollmentRow);
if (!enrollment) continue; if (!enrollment) continue;
if (enrollment.className) enrollmentByClassName.set(enrollment.className, enrollment); if (enrollment.className) enrollmentByClassName.set(enrollment.className, enrollment);
imported++; imported++;
} }
for (const examRow of data.examScores.filter( for (const examRow of data.examScores.filter((item) => this.normalizePhone(item.phone) === phone)) {
(item) => this.normalizePhone(item.phone) === phone,
)) {
if (await this.upsertExamScoreFromImport(studentId, examRow, enrollmentByClassName)) { if (await this.upsertExamScoreFromImport(studentId, examRow, enrollmentByClassName)) {
imported++; imported++;
} }
} }
for (const learningRow of data.learningRecords.filter( for (const learningRow of data.learningRecords.filter((item) => this.normalizePhone(item.phone) === phone)) {
(item) => this.normalizePhone(item.phone) === phone,
)) {
if (await this.upsertLearningRecordFromImport(studentId, learningRow)) { if (await this.upsertLearningRecordFromImport(studentId, learningRow)) {
imported++; imported++;
} }
@@ -399,9 +390,7 @@ export class StudentsService {
} }
private async upsertProfileFromImport(studentId: number, row: StudentImportRow) { private async upsertProfileFromImport(studentId: number, row: StudentImportRow) {
const entity = const entity = (await this.profileRepo.findOne({ where: { studentId } })) || this.profileRepo.create({ studentId });
(await this.profileRepo.findOne({ where: { studentId } })) ||
this.profileRepo.create({ studentId });
if (row.targetCollege?.trim()) entity.targetCollege = row.targetCollege.trim(); if (row.targetCollege?.trim()) entity.targetCollege = row.targetCollege.trim();
if (row.targetMajor?.trim()) entity.targetMajor = row.targetMajor.trim(); if (row.targetMajor?.trim()) entity.targetMajor = row.targetMajor.trim();
if (row.collegeSchool?.trim()) entity.collegeSchool = row.collegeSchool.trim(); if (row.collegeSchool?.trim()) entity.collegeSchool = row.collegeSchool.trim();
@@ -414,12 +403,9 @@ export class StudentsService {
} }
private async upsertResultFromImport(studentId: number, row: StudentImportRow) { private async upsertResultFromImport(studentId: number, row: StudentImportRow) {
const entity = const entity = (await this.resultRepo.findOne({ where: { studentId } })) || this.resultRepo.create({ studentId });
(await this.resultRepo.findOne({ where: { studentId } })) ||
this.resultRepo.create({ studentId });
if (row.cultureFinalScore !== undefined) entity.cultureFinalScore = row.cultureFinalScore; if (row.cultureFinalScore !== undefined) entity.cultureFinalScore = row.cultureFinalScore;
if (row.professionalFinalScore !== undefined) if (row.professionalFinalScore !== undefined) entity.professionalFinalScore = row.professionalFinalScore;
entity.professionalFinalScore = row.professionalFinalScore;
if (row.admissionStatus?.trim()) entity.admissionStatus = row.admissionStatus.trim(); if (row.admissionStatus?.trim()) entity.admissionStatus = row.admissionStatus.trim();
if (row.admittedCollege?.trim()) entity.admittedCollege = row.admittedCollege.trim(); if (row.admittedCollege?.trim()) entity.admittedCollege = row.admittedCollege.trim();
if (row.admittedMajor?.trim()) entity.admittedMajor = row.admittedMajor.trim(); if (row.admittedMajor?.trim()) entity.admittedMajor = row.admittedMajor.trim();
@@ -637,9 +623,10 @@ export class StudentsService {
// ---- Filters ---- // ---- Filters ----
if (query?.keyword) { if (query?.keyword) {
qb.andWhere('(student.name LIKE :keyword OR student.studentNo LIKE :keyword)', { qb.andWhere(
keyword: `%${query.keyword}%`, '(student.name LIKE :keyword OR student.studentNo LIKE :keyword)',
}); { keyword: `%${query.keyword}%` },
);
} }
if (query?.organizationId) { if (query?.organizationId) {
qb.andWhere('student.organizationId = :orgId', { orgId: query.organizationId }); qb.andWhere('student.organizationId = :orgId', { orgId: query.organizationId });