diff --git a/services/progress-service/build.gradle.kts b/services/progress-service/build.gradle.kts index 4b39962..47f4bd1 100644 --- a/services/progress-service/build.gradle.kts +++ b/services/progress-service/build.gradle.kts @@ -19,6 +19,7 @@ dependencies { implementation(project(":libs:security-jwt")) testImplementation(project(":libs:test-support")) + testImplementation(libs.springSecurityTest) } springBoot { diff --git a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/AcademicClient.kt b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/AcademicClient.kt index c12021a..2222ebd 100644 --- a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/AcademicClient.kt +++ b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/AcademicClient.kt @@ -20,26 +20,29 @@ class AcademicClient( fun getLab( labId: String, token: String, - ): LabInfo = - client - .get() - .uri("/api/v1/labs/{id}", labId) - .header("Authorization", "Bearer $token") - .retrieve() - .body(LabInfo::class.java) - ?: throw NotFoundException("Лабораторная не найдена") + ): LabInfo = getLabDetail(labId, token) + /** Сводка нужна и для проверки доступа: academic сам ограничивает лабораторные курсами пользователя. */ fun getLabSummary( labId: String, token: String, + ): LabInfo = getLabDetail(labId, token) + + private fun getLabDetail( + labId: String, + token: String, ): LabInfo = - client - .get() - .uri("/api/v1/labs/{id}", labId) - .header("Authorization", "Bearer $token") - .retrieve() - .body(LabInfo::class.java) - ?: throw NotFoundException("Лабораторная не найдена") + try { + client + .get() + .uri("/api/v1/labs/{id}", labId) + .header("Authorization", "Bearer $token") + .retrieve() + .body(LabInfo::class.java) + ?: throw NotFoundException("Лабораторная не найдена") + } catch (ex: RestClientResponseException) { + throw labError(ex) + } /** Курс с группами; academic-service сам проверяет доступ по пользовательскому токену. */ fun getCourse( @@ -101,6 +104,14 @@ private fun courseError(ex: RestClientResponseException): Exception = else -> ex } +/** 403 academic означает чужую лабораторную: для преподавателя это должно быть 403, а не 500. */ +private fun labError(ex: RestClientResponseException): Exception = + when (ex.statusCode.value()) { + 403 -> ForbiddenException("Лабораторная недоступна") + 404 -> NotFoundException("Лабораторная не найдена") + else -> ex + } + @Service class IdentityClient( @Value("\${lms.identity.base-url}") baseUrl: String, diff --git a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/GroupLabDataService.kt b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/GroupLabDataService.kt index b83a8f2..445688c 100644 --- a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/GroupLabDataService.kt +++ b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/GroupLabDataService.kt @@ -29,7 +29,7 @@ class GroupLabDataService( labId = labId.toString(), assignments = progress.assignmentsForLabEnriched(labId, token), levels = - progress.getLevelsForLab(labId).map { sll -> + progress.getLevelsForLab(labId, token).map { sll -> LevelAssignmentResponse( studentId = sll.studentId.toString(), labId = sll.labId.toString(), diff --git a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/ProgressController.kt b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/ProgressController.kt index 9273399..8116231 100644 --- a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/ProgressController.kt +++ b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/ProgressController.kt @@ -34,18 +34,19 @@ class ProgressController( fun meLabs(): List> = listOf() @PostMapping("/levels") - @PreAuthorize("hasAuthority('ROLE_ADMIN')") + @PreAuthorize("hasAnyRole('ADMIN','TEACHER')") fun setLevel( @Valid @RequestBody body: LevelAssignmentRequest, @AuthenticationPrincipal jwt: Jwt, ): LevelAssignmentResponse = service.setLevel(body, jwt.tokenValue) @GetMapping("/levels") - @PreAuthorize("hasAuthority('ROLE_ADMIN')") + @PreAuthorize("hasAnyRole('ADMIN','TEACHER')") fun getLevels( @RequestParam labId: UUID, + @AuthenticationPrincipal jwt: Jwt, ): List = - service.getLevelsForLab(labId).map { sll -> + service.getLevelsForLab(labId, jwt.tokenValue).map { sll -> LevelAssignmentResponse( studentId = sll.studentId.toString(), labId = sll.labId.toString(), @@ -62,14 +63,14 @@ class ProgressController( ): DistributeResult = service.distribute(body, jwt.tokenValue) @PostMapping("/assign") - @PreAuthorize("hasAuthority('ROLE_ADMIN')") + @PreAuthorize("hasAnyRole('ADMIN','TEACHER')") fun assign( @Valid @RequestBody body: AssignRequest, @AuthenticationPrincipal jwt: Jwt, ): VariantAssignmentResponse = service.assignWithInfo(body, jwt.tokenValue) @GetMapping("/assignments") - @PreAuthorize("hasAuthority('ROLE_ADMIN')") + @PreAuthorize("hasAnyRole('ADMIN','TEACHER')") fun assignmentsForLab( @RequestParam labId: UUID, @AuthenticationPrincipal jwt: Jwt, diff --git a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/ProgressService.kt b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/ProgressService.kt index 7a3bf37..caead8f 100644 --- a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/ProgressService.kt +++ b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/ProgressService.kt @@ -24,6 +24,8 @@ class ProgressService( ): LevelAssignmentResponse { val sid = UUID.fromString(request.studentId) val lid = UUID.fromString(request.labId) + // Доступ проверяем до записи: чужой курс должен давать 403, а не менять данные. + val lab = academic.getLab(request.labId, token) val key = StudentLabLevelKey(sid, lid) val existing = levels.findById(key) val entity = existing.orElse(StudentLabLevel(studentId = sid, labId = lid)) @@ -31,10 +33,9 @@ class ProgressService( entity.manual = true levels.save(entity) - // Сразу подбираем вариант из выбранного уровня: админ видит результат, + // Сразу подбираем вариант из выбранного уровня: преподаватель видит результат, // а не «пустой» вариант до нажатия «Распределить». - val lab = runCatching { academic.getLab(request.labId, token) }.getOrNull() - val level = lab?.levels?.firstOrNull { it.id == request.labLevelId } + val level = lab.levels.firstOrNull { it.id == request.labLevelId } if (level != null && level.variants.isNotEmpty()) { val assignment = assignments.findByStudentIdAndLabId(sid, lid) val currentInLevel = @@ -75,8 +76,15 @@ class ProgressService( labId: UUID, ): StudentLabLevel? = levels.findByStudentIdAndLabId(studentId, labId) + /** Уровни лабораторной: доступ проверяется токеном пользователя, чтобы преподаватель не читал чужой курс. */ @Transactional(readOnly = true) - fun getLevelsForLab(labId: UUID): List = levels.findByLabId(labId) + fun getLevelsForLab( + labId: UUID, + token: String, + ): List { + academic.getLabSummary(labId.toString(), token) + return levels.findByLabId(labId) + } @Transactional fun distribute( @@ -225,8 +233,9 @@ class ProgressService( request: AssignRequest, token: String, ): VariantAssignmentResponse { - assign(request) + // Сначала доступ к лабораторной, затем запись: чужой курс не должен менять назначения. val lab = academic.getLab(request.labId, token) + assign(request) val variant = lab.levels.flatMap { it.variants }.firstOrNull { it.id == request.variantId } return VariantAssignmentResponse( studentId = request.studentId, diff --git a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/SubmissionController.kt b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/SubmissionController.kt index 698a278..bddfb07 100644 --- a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/SubmissionController.kt +++ b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/SubmissionController.kt @@ -51,18 +51,19 @@ class SubmissionController( } @GetMapping("/submissions") - @PreAuthorize("hasAuthority('ROLE_ADMIN')") + @PreAuthorize("hasAnyRole('ADMIN','TEACHER')") fun labSubmissions( @RequestParam labId: UUID, - ): List = submissions.latestPerStudent(labId) + @AuthenticationPrincipal jwt: Jwt, + ): List = submissions.labSubmissions(labId, jwt.tokenValue) @GetMapping("/submissions/{id}/download") - @PreAuthorize("hasAuthority('ROLE_ADMIN')") - fun downloadForAdmin( + @PreAuthorize("hasAnyRole('ADMIN','TEACHER')") + fun downloadForStaff( @PathVariable id: UUID, @AuthenticationPrincipal jwt: Jwt, ): ResponseEntity { - val (bytes, name) = submissions.download(UUID.fromString(jwt.subject), id, null, isAdmin = true) + val (bytes, name) = submissions.downloadForStaff(id, jwt.tokenValue) return attachment(bytes, name) } diff --git a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/SubmissionService.kt b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/SubmissionService.kt index 90d750e..60d6065 100644 --- a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/SubmissionService.kt +++ b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/SubmissionService.kt @@ -117,6 +117,37 @@ class SubmissionService( return zip to name } + /** + * Доступ к сдачам лабораторной проверяет academic по токену пользователя: + * преподаватель получает 403 на чужой курс, админ — доступ к любой лабораторной. + */ + fun ensureLabAccess( + labId: UUID, + token: String, + ) { + academic.getLabSummary(labId.toString(), token) + } + + @Transactional(readOnly = true) + fun labSubmissions( + labId: UUID, + token: String, + ): List { + ensureLabAccess(labId, token) + return latestPerStudent(labId) + } + + /** Скачивание сдачи преподавателем/админом: лабораторная сдачи должна быть доступна вызывающему. */ + @Transactional(readOnly = true) + fun downloadForStaff( + submissionId: UUID, + token: String, + ): Pair { + val s = submissions.findById(submissionId).orElseThrow { NotFoundException("Сдача не найдена") } + ensureLabAccess(s.labId, token) + return download(s.studentId, submissionId, null, isAdmin = true) + } + @Transactional(readOnly = true) fun latestPerStudent(labId: UUID): List { val perStudent = mutableMapOf() diff --git a/services/progress-service/src/test/kotlin/ru/ysu/lms/progress/TeacherLabPageAccessIntegrationTest.kt b/services/progress-service/src/test/kotlin/ru/ysu/lms/progress/TeacherLabPageAccessIntegrationTest.kt new file mode 100644 index 0000000..e6099c6 --- /dev/null +++ b/services/progress-service/src/test/kotlin/ru/ysu/lms/progress/TeacherLabPageAccessIntegrationTest.kt @@ -0,0 +1,278 @@ +package ru.ysu.lms.progress + +import org.assertj.core.api.Assertions.assertThat +import org.hamcrest.Matchers.containsString +import org.junit.jupiter.api.Test +import org.mockito.ArgumentMatchers.anyString +import org.mockito.Mockito.`when` +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.test.context.SpringBootTest +import org.springframework.boot.testcontainers.service.connection.ServiceConnection +import org.springframework.http.HttpHeaders +import org.springframework.http.MediaType +import org.springframework.security.core.authority.SimpleGrantedAuthority +import org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.jwt +import org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers.springSecurity +import org.springframework.test.context.DynamicPropertyRegistry +import org.springframework.test.context.DynamicPropertySource +import org.springframework.test.context.bean.override.mockito.MockitoBean +import org.springframework.test.web.servlet.MockMvc +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post +import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content +import org.springframework.test.web.servlet.result.MockMvcResultMatchers.header +import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status +import org.springframework.test.web.servlet.setup.DefaultMockMvcBuilder +import org.springframework.test.web.servlet.setup.MockMvcBuilders +import org.springframework.transaction.annotation.Transactional +import org.springframework.web.context.WebApplicationContext +import org.testcontainers.junit.jupiter.Container +import org.testcontainers.junit.jupiter.Testcontainers +import org.testcontainers.postgresql.PostgreSQLContainer +import ru.ysu.lms.common.ForbiddenException +import ru.ysu.lms.progress.domain.Submission +import ru.ysu.lms.progress.repo.StudentLabLevelRepository +import ru.ysu.lms.progress.repo.SubmissionRepository +import ru.ysu.lms.progress.repo.VariantAssignmentRepository +import java.nio.file.Files +import java.nio.file.Path +import java.util.UUID + +/** + * Страница лабораторной для преподавателя: свой курс разрешён, чужой и студент — 403. + * Доступ к лабораторной проверяет academic-service; здесь он подменён заглушкой, + * которая ведёт себя как реальный сервис (403 на чужую лабораторную). + */ +@Testcontainers +@SpringBootTest +@Transactional +class TeacherLabPageAccessIntegrationTest { + companion object { + @Container + @ServiceConnection + @JvmStatic + val postgres = PostgreSQLContainer("postgres:18") + + private val submissionsDir: Path = Files.createTempDirectory("lms-progress-submissions") + + @JvmStatic + @DynamicPropertySource + fun submissionsDir(registry: DynamicPropertyRegistry) { + registry.add("lms.submissions.dir") { submissionsDir.toString() } + } + } + + @Autowired + private lateinit var context: WebApplicationContext + + @Autowired + private lateinit var storage: SubmissionStorage + + @Autowired + private lateinit var submissions: SubmissionRepository + + @Autowired + private lateinit var assignments: VariantAssignmentRepository + + @Autowired + private lateinit var levels: StudentLabLevelRepository + + @MockitoBean + private lateinit var academic: AcademicClient + + private val ownLab = UUID.randomUUID() + private val foreignLab = UUID.randomUUID() + private val levelId = UUID.randomUUID() + private val variantId = UUID.randomUUID() + + private val mockMvc: MockMvc by lazy { + MockMvcBuilders + .webAppContextSetup(context) + .apply(springSecurity()) + .build() + } + + private fun lab(id: UUID): LabInfo = + LabInfo( + id = id.toString(), + title = "Лабораторная", + levels = + listOf( + LevelInfo( + id = levelId.toString(), + code = "basic", + title = "Основная", + variants = listOf(VariantInfo(id = variantId.toString(), number = "1")), + ), + ), + ) + + private fun mockOwnLab() { + `when`(academic.getLabSummary(anyString(), anyString())).thenReturn(lab(ownLab)) + `when`(academic.getLab(anyString(), anyString())).thenReturn(lab(ownLab)) + } + + private fun mockForeignLab() { + `when`(academic.getLabSummary(anyString(), anyString())) + .thenThrow(ForbiddenException("Лабораторная недоступна")) + `when`(academic.getLab(anyString(), anyString())) + .thenThrow(ForbiddenException("Лабораторная недоступна")) + } + + private fun teacher() = jwtFor("TEACHER") + + private fun admin() = jwtFor("ADMIN") + + private fun jwtFor(role: String) = + jwt() + .jwt { + it + .subject(UUID.randomUUID().toString()) + .claim("role", role) + }.authorities(SimpleGrantedAuthority("ROLE_$role")) + + private fun getLevels( + labId: UUID, + role: String, + ) = get("/api/v1/levels").param("labId", labId.toString()).with(jwtFor(role)) + + private fun getAssignments( + labId: UUID, + role: String, + ) = get("/api/v1/assignments").param("labId", labId.toString()).with(jwtFor(role)) + + private fun getSubmissions( + labId: UUID, + role: String, + ) = get("/api/v1/submissions").param("labId", labId.toString()).with(jwtFor(role)) + + private fun postLevel( + labId: UUID, + role: String, + studentId: UUID = UUID.randomUUID(), + ) = post("/api/v1/levels") + .contentType(MediaType.APPLICATION_JSON) + .content(levelBody(studentId, labId)) + .with(jwtFor(role)) + + private fun postAssign( + labId: UUID, + role: String, + studentId: UUID = UUID.randomUUID(), + ) = post("/api/v1/assign") + .contentType(MediaType.APPLICATION_JSON) + .content(assignBody(studentId, labId)) + .with(jwtFor(role)) + + private fun levelBody( + studentId: UUID, + labId: UUID, + ) = """ + {"studentId":"$studentId","labId":"$labId","labLevelId":"$levelId"} + """.trimIndent() + + private fun assignBody( + studentId: UUID, + labId: UUID, + ) = """ + {"studentId":"$studentId","labId":"$labId","variantId":"$variantId"} + """.trimIndent() + + private fun seedSubmission(labId: UUID): Submission { + val studentId = UUID.randomUUID() + val submission = submissions.saveAndFlush(Submission(studentId = studentId, labId = labId, seq = 1)) + storage.store(studentId, labId, 1, listOf("main.c" to "#include \n".toByteArray())) + return submission + } + + @Test + fun `teacher reads levels, assignments and submissions of own lab`() { + mockOwnLab() + + mockMvc.perform(getLevels(ownLab, "TEACHER")).andExpect(status().isOk) + mockMvc.perform(getAssignments(ownLab, "TEACHER")).andExpect(status().isOk) + mockMvc.perform(getSubmissions(ownLab, "TEACHER")).andExpect(status().isOk) + } + + @Test + fun `teacher sets level and assigns variant of own lab`() { + mockOwnLab() + val studentId = UUID.randomUUID() + + mockMvc.perform(postLevel(ownLab, "TEACHER", studentId)).andExpect(status().isOk) + + val savedLevel = levels.findByStudentIdAndLabId(studentId, ownLab) + assertThat(savedLevel?.labLevelId).isEqualTo(levelId) + assertThat(savedLevel?.manual).isTrue() + + mockMvc.perform(postAssign(ownLab, "TEACHER", studentId)).andExpect(status().isOk) + val savedAssignment = assignments.findByStudentIdAndLabId(studentId, ownLab) + assertThat(savedAssignment?.variantId).isEqualTo(variantId) + assertThat(savedAssignment?.manual).isTrue() + } + + @Test + fun `teacher downloads submission of own lab`() { + mockOwnLab() + val submission = seedSubmission(ownLab) + + mockMvc + .perform( + get("/api/v1/submissions/{id}/download", submission.id) + .with(teacher()), + ).andExpect(status().isOk) + .andExpect(header().string(HttpHeaders.CONTENT_DISPOSITION, containsString("attachment"))) + .andExpect(content().contentType(MediaType.APPLICATION_OCTET_STREAM)) + } + + @Test + fun `teacher is forbidden for foreign lab`() { + mockForeignLab() + val foreignSubmission = seedSubmission(foreignLab) + + mockMvc.perform(getLevels(foreignLab, "TEACHER")).andExpect(status().isForbidden) + mockMvc.perform(postLevel(foreignLab, "TEACHER")).andExpect(status().isForbidden) + mockMvc.perform(postAssign(foreignLab, "TEACHER")).andExpect(status().isForbidden) + mockMvc.perform(getAssignments(foreignLab, "TEACHER")).andExpect(status().isForbidden) + mockMvc.perform(getSubmissions(foreignLab, "TEACHER")).andExpect(status().isForbidden) + mockMvc + .perform( + get("/api/v1/submissions/{id}/download", foreignSubmission.id) + .with(teacher()), + ).andExpect(status().isForbidden) + + assertThat(assignments.findByLabId(foreignLab)).isEmpty() + assertThat(levels.findByLabId(foreignLab)).isEmpty() + } + + @Test + fun `student is forbidden for lab page operations`() { + val submission = seedSubmission(ownLab) + + mockMvc.perform(getLevels(ownLab, "STUDENT")).andExpect(status().isForbidden) + mockMvc.perform(postLevel(ownLab, "STUDENT")).andExpect(status().isForbidden) + mockMvc.perform(postAssign(ownLab, "STUDENT")).andExpect(status().isForbidden) + mockMvc.perform(getAssignments(ownLab, "STUDENT")).andExpect(status().isForbidden) + mockMvc.perform(getSubmissions(ownLab, "STUDENT")).andExpect(status().isForbidden) + mockMvc + .perform( + get("/api/v1/submissions/{id}/download", submission.id) + .with(jwtFor("STUDENT")), + ).andExpect(status().isForbidden) + } + + @Test + fun `admin still reads lab page data`() { + mockOwnLab() + val submission = seedSubmission(ownLab) + + mockMvc.perform(getLevels(ownLab, "ADMIN")).andExpect(status().isOk) + mockMvc.perform(getAssignments(ownLab, "ADMIN")).andExpect(status().isOk) + mockMvc.perform(getSubmissions(ownLab, "ADMIN")).andExpect(status().isOk) + mockMvc + .perform( + get("/api/v1/submissions/{id}/download", submission.id) + .with(admin()), + ).andExpect(status().isOk) + } +}