fix(education): fail closed on unsafe question data
This commit is contained in:
@@ -397,7 +397,7 @@ public class JavaCatalogProvider implements CatalogProvider, QuestionCatalogProv
|
|||||||
|
|
||||||
@SuppressWarnings({"unchecked", "rawtypes"})
|
@SuppressWarnings({"unchecked", "rawtypes"})
|
||||||
private List<CatalogQuestionDTO.QuestionOptionDTO> parseOptions(String optionsJson) {
|
private List<CatalogQuestionDTO.QuestionOptionDTO> parseOptions(String optionsJson) {
|
||||||
if (optionsJson == null || optionsJson.isEmpty()) return Collections.emptyList();
|
if (optionsJson == null || optionsJson.isBlank()) return Collections.emptyList();
|
||||||
try {
|
try {
|
||||||
com.fasterxml.jackson.databind.ObjectMapper mapper = new com.fasterxml.jackson.databind.ObjectMapper();
|
com.fasterxml.jackson.databind.ObjectMapper mapper = new com.fasterxml.jackson.databind.ObjectMapper();
|
||||||
List<java.util.Map> rawList = mapper.readValue(optionsJson, List.class);
|
List<java.util.Map> rawList = mapper.readValue(optionsJson, List.class);
|
||||||
@@ -414,7 +414,7 @@ public class JavaCatalogProvider implements CatalogProvider, QuestionCatalogProv
|
|||||||
}
|
}
|
||||||
return result;
|
return result;
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
return Collections.emptyList();
|
throw exception(UNSAFE_PROVIDER_PAYLOAD);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -579,8 +579,11 @@ public class PracticeSessionServiceImpl implements PracticeSessionService {
|
|||||||
|| response.getDetails() == null
|
|| response.getDetails() == null
|
||||||
|| !Objects.equals(existing.getReportId(), response.getReportId())
|
|| !Objects.equals(existing.getReportId(), response.getReportId())
|
||||||
|| !Objects.equals(existing.getSessionId(), response.getSessionId())) {
|
|| !Objects.equals(existing.getSessionId(), response.getSessionId())) {
|
||||||
log.warn("Invalid submit replay payload: id={}, storedReportId={}, storedSessionId={}, response={}",
|
log.warn("Invalid submit replay payload: id={}, storedReportId={}, storedSessionId={}, " +
|
||||||
existing.getId(), existing.getReportId(), existing.getSessionId(), existing.getResponseJson());
|
"responseReportId={}, responseSessionId={}",
|
||||||
|
existing.getId(), existing.getReportId(), existing.getSessionId(),
|
||||||
|
response != null ? response.getReportId() : null,
|
||||||
|
response != null ? response.getSessionId() : null);
|
||||||
throw exception(SUBMIT_IDEMPOTENCY_REPLAY_INVALID);
|
throw exception(SUBMIT_IDEMPOTENCY_REPLAY_INVALID);
|
||||||
}
|
}
|
||||||
return response;
|
return response;
|
||||||
|
|||||||
@@ -33,8 +33,8 @@ public final class QuestionContentSafety {
|
|||||||
*/
|
*/
|
||||||
public static void validateVisibleProviderQuestion(CatalogQuestionDTO question, ErrorCode visibilityError) {
|
public static void validateVisibleProviderQuestion(CatalogQuestionDTO question, ErrorCode visibilityError) {
|
||||||
if (question == null || question.getIsPublished() == null || !question.getIsPublished()
|
if (question == null || question.getIsPublished() == null || !question.getIsPublished()
|
||||||
|| "hidden".equalsIgnoreCase(question.getStatus())
|
|| (question.getStatus() != null && !question.getStatus().isBlank()
|
||||||
|| "inactive".equalsIgnoreCase(question.getStatus())) {
|
&& !"published".equalsIgnoreCase(question.getStatus()))) {
|
||||||
throw exception(visibilityError);
|
throw exception(visibilityError);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -298,12 +298,12 @@ class QuestionCatalogServiceImplTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void shouldAllowAllPublishedAndActiveItems() {
|
void shouldAllowPublishedItemsWithPublishedOrAbsentStatus() {
|
||||||
// Mixed types but all published and status != hidden/inactive → all pass
|
// Providers may omit status, but any explicit student-visible status must be PUBLISHED.
|
||||||
CatalogQuestionDTO q1 = questionDto("q1", "Q1", "fill", "easy", true, null, null, null, null, null);
|
CatalogQuestionDTO q1 = questionDto("q1", "Q1", "fill", "easy", true, null, null, null, null, null);
|
||||||
CatalogQuestionDTO q2 = CatalogQuestionDTO.builder()
|
CatalogQuestionDTO q2 = CatalogQuestionDTO.builder()
|
||||||
.id("q2").stem("Q2").type("fill").difficulty("medium")
|
.id("q2").stem("Q2").type("fill").difficulty("medium")
|
||||||
.isPublished(true).status("active").build();
|
.isPublished(true).status("PUBLISHED").build();
|
||||||
CatalogQuestionDTO q3 = CatalogQuestionDTO.builder()
|
CatalogQuestionDTO q3 = CatalogQuestionDTO.builder()
|
||||||
.id("q3").stem("Q3").type("short_answer").difficulty("hard")
|
.id("q3").stem("Q3").type("short_answer").difficulty("hard")
|
||||||
.isPublished(true).status(null).build();
|
.isPublished(true).status(null).build();
|
||||||
|
|||||||
@@ -14,6 +14,20 @@ import static org.junit.jupiter.api.Assertions.*;
|
|||||||
|
|
||||||
class QuestionContentSafetyTest {
|
class QuestionContentSafetyTest {
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void shouldAllowOnlyPublishedProviderStatusWhenPresent() {
|
||||||
|
CatalogQuestionDTO published = CatalogQuestionDTO.builder().isPublished(true).status("PUBLISHED").build();
|
||||||
|
assertDoesNotThrow(() -> QuestionContentSafety.validateVisibleProviderQuestion(
|
||||||
|
published, UNSAFE_PROVIDER_PAYLOAD));
|
||||||
|
|
||||||
|
for (String status : List.of("DRAFT", "DISABLED", "UNKNOWN", "HIDDEN", "INACTIVE")) {
|
||||||
|
CatalogQuestionDTO unsafe = CatalogQuestionDTO.builder().isPublished(true).status(status).build();
|
||||||
|
ServiceException ex = assertThrows(ServiceException.class,
|
||||||
|
() -> QuestionContentSafety.validateVisibleProviderQuestion(unsafe, UNSAFE_PROVIDER_PAYLOAD));
|
||||||
|
assertEquals(UNSAFE_PROVIDER_PAYLOAD.getCode(), ex.getCode());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void shouldAcceptAndOrderValidOptionBackedQuestion() {
|
void shouldAcceptAndOrderValidOptionBackedQuestion() {
|
||||||
List<SafeOption> options = QuestionContentSafety.validateProviderOptions(" choice ", List.of(
|
List<SafeOption> options = QuestionContentSafety.validateProviderOptions(" choice ", List.of(
|
||||||
|
|||||||
Reference in New Issue
Block a user