Skip to content
Open
15 changes: 15 additions & 0 deletions src/main/java/de/tum/cit/aet/artemis/exam/web/ExamResource.java
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ public ResponseEntity<ExamDTO> createExam(@PathVariable Long courseId, @RequestB
if (examDTO.id() != null) {
throw new BadRequestAlertException("A new exam cannot already have an ID", ENTITY_NAME, "idExists");
}
checkExamTitleIsPresentElseThrow(examDTO.title());

examAccessService.checkCourseAccessForInstructorElseThrow(courseId);

Expand Down Expand Up @@ -287,6 +288,7 @@ public ResponseEntity<ExamDTO> updateExam(@PathVariable Long courseId, @RequestB
if (examUpdateDTO.id() == null) {
throw new BadRequestAlertException("An exam update must have an ID", ENTITY_NAME, "idMissing");
}
checkExamTitleIsPresentElseThrow(examUpdateDTO.title());

examAccessService.checkCourseAndExamAccessForInstructorElseThrow(courseId, examUpdateDTO.id());

Expand Down Expand Up @@ -465,6 +467,7 @@ public ResponseEntity<ExamWideAnnouncementEventDTO> createExamAnnouncement(@Path
public ResponseEntity<ExamImportResultDTO> importExamWithExercises(@PathVariable Long courseId, @RequestBody ExamImportDTO examImportDTO,
@RequestParam(required = false) String importId) throws URISyntaxException, IOException {
log.debug("REST request to import an exam : {}", examImportDTO);
checkExamTitleIsPresentElseThrow(examImportDTO.title());

examAccessService.checkCourseAccessForInstructorElseThrow(courseId);

Expand Down Expand Up @@ -548,6 +551,18 @@ private void checkExamTitleLengthElseThrow(Exam exam) {
}
}

/**
* Checks that the exam title is present, so an exam is never created or updated with a missing or blank title. The client marks this too, but crafted requests and import
* payloads bypass the UI. This validates the raw request title before it is mapped to an entity, because {@link Exam#setTitle} would throw on a null title during mapping.
*
* @param title the exam title from the request
*/
private void checkExamTitleIsPresentElseThrow(String title) {
if (title == null || title.isBlank()) {
throw new BadRequestAlertException("The exam title must not be empty.", ENTITY_NAME, "examTitleEmpty");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

examTitleEmpty has no error.json entry in en or de.

checkExamTitleLengthElseThrow right above this (examTitleTooLong) has both, and so does every other exam validation error in this class (noCourse, wrongCourseId, examTimes, negativePoints, correctionRoundViolation, attendanceCheckViolation).

confirmed at runtime: fetch('/i18n/en.json') and de.json both lack error.examTitleEmpty while error.examTitleTooLong is present in both. means a crafted request or any non-UI caller gets the raw english exception title in the toast, untranslated for german users. two-line fix, same pattern as examTitleTooLong.

}
}

/**
* Validates numeric field limits for exam configuration.
* Maximum values:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ <h5 class="pb-1" jhiTranslate="artemisApp.examManagement.sections.configuration"
[hideChannelName]="hideChannelNameInput()"
[initChannelName]="isImport() || exam.id === undefined"
/>
@if (!exam.title?.trim()) {
Comment thread
Claudia-Anthropica marked this conversation as resolved.
<div class="text-state-danger text-sm mt-1 block" data-testid="title-validation-message" jhiTranslate="artemisApp.examManagement.titleMissingOrNotValid"></div>
}
</div>
<div class="form-group">
<div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,18 @@ describe('ExamUpdateComponent', () => {
expect(component.exam.workingTime).toBe(0);
});

it('should show the title validation message when the title is missing and hide it once a title is set', () => {
fixture.detectChanges();
const titleValidationSelector = By.css('[data-testid="title-validation-message"]');
expect(fixture.debugElement.query(titleValidationSelector)).not.toBeNull();

const examWithTitle = cloneDeep(component.exam);
examWithTitle.title = 'A valid exam title';
component.exam = examWithTitle;
fixture.detectChanges();
expect(fixture.debugElement.query(titleValidationSelector)).toBeNull();
});

it('should validate the dates correctly', () => {
examWithoutExercises.visibleDate = dayjs().add(1, 'hours');
examWithoutExercises.startDate = dayjs().add(2, 'hours');
Expand Down
1 change: 1 addition & 0 deletions src/main/webapp/i18n/de/exam.json
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,7 @@
"examManagement": {
"reviewDatesInvalidWarning": "Das Ende der Klausureinsicht ist falsch definiert oder fehlt",
"reviewDatesInvalidExplanation": "Ist ein Beginn der Klausureinsicht definiert, muss auch ein Ende definiert werden. Zusätzlich muss das Ende der Klausureinsicht immer nach dem Beginn datiert sein.",
"titleMissingOrNotValid": "'Titel' fehlt/ungültig",
"sections": {
"configuration": "Klausurkonfiguration",
"conduction": "Klausurdurchführung",
Expand Down
1 change: 1 addition & 0 deletions src/main/webapp/i18n/en/exam.json
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,7 @@
"examManagement": {
"reviewDatesInvalidWarning": "End of Student Review is set incorrect or missing",
"reviewDatesInvalidExplanation": "If the Begin of Student Review is set, the End of Student Review has to be set as well. Furthermore, the End of Student Review has to be chronologically after its Begin.",
"titleMissingOrNotValid": "'Title' is missing/invalid",
"sections": {
"configuration": "Exam Configuration",
"conduction": "Exam Conduction",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,17 @@ public void putAndExpectError(String path, Object body, HttpStatus expectedStatu
assertThat(response.getHeader(errorHeader)).isEqualTo(fullErrorKey);
}

public void postAndExpectError(String path, Object body, HttpStatus expectedStatus, String expectedErrorKey) throws Exception {
final var jsonBody = mapper.writeValueAsString(body);
final var response = performMvcRequest(MockMvcRequestBuilders.post(new URI(path)).contentType(MediaType.APPLICATION_JSON).content(jsonBody))
.andExpect(status().is(expectedStatus.value())).andReturn().getResponse();
restoreSecurityContext();

final var fullErrorKey = "error." + expectedErrorKey;
final var errorHeader = "X-" + APPLICATION_NAME + "-error";
assertThat(response.getHeader(errorHeader)).isEqualTo(fullErrorKey);
}

public <T> T get(String path, HttpStatus expectedStatus, Class<T> responseType) throws Exception {
return get(path, expectedStatus, responseType, new LinkedMultiValueMap<>());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.NullSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;
Expand Down Expand Up @@ -678,6 +679,55 @@ void testCreateExam_failsWithExamMaxPointsTooHigh() throws Exception {
request.post("/api/exam/courses/" + course1.getId() + "/exams", ExamUpdateDTO.of(exam), HttpStatus.BAD_REQUEST);
}

@ParameterizedTest(name = "title=\"{0}\"")
@NullSource
@ValueSource(strings = { "", " " })
@WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
void testCreateExam_failsWithMissingOrBlankTitle(String title) throws Exception {
// A missing (null), empty or whitespace-only title has to be rejected with a clean 400, not persisted and not failing while mapping the null title to the entity
ObjectNode examJson = examBodyWithTitle(ExamUpdateDTO.of(ExamFactory.generateExam(course1, "examTitleValidationTest")), title);

request.postAndExpectError("/api/exam/courses/" + course1.getId() + "/exams", examJson, HttpStatus.BAD_REQUEST, "examTitleEmpty");
}

@ParameterizedTest(name = "title=\"{0}\"")
@NullSource
@ValueSource(strings = { "", " " })
@WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
void testUpdateExam_failsWithMissingOrBlankTitle(String title) throws Exception {
ObjectNode examJson = examBodyWithTitle(ExamUpdateDTO.of(exam1), title);

request.putAndExpectError("/api/exam/courses/" + course1.getId() + "/exams", examJson, HttpStatus.BAD_REQUEST, "examTitleEmpty");
}

@ParameterizedTest(name = "title=\"{0}\"")
@NullSource
@ValueSource(strings = { "", " " })
@WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
void testImportExam_failsWithMissingOrBlankTitle(String title) throws Exception {
ObjectNode examJson = examBodyWithTitle(ExamImportDTO.of(exam1, course1.getId()), title);

request.postAndExpectError("/api/exam/courses/" + course1.getId() + "/exam-import", examJson, HttpStatus.BAD_REQUEST, "examTitleEmpty");
}

/**
* Serialises the given exam DTO and overwrites its title, so a missing (null), empty or whitespace-only title can be sent as a raw request body.
*
* @param examDto the exam create/update/import DTO to serialise
* @param title the title to set, or null to omit it
* @return the request body as a JSON object with the adjusted title
*/
private ObjectNode examBodyWithTitle(Object examDto, String title) {
ObjectNode examJson = request.getObjectMapper().valueToTree(examDto);
if (title == null) {
examJson.putNull("title");
}
else {
examJson.put("title", title);
}
return examJson;
}

@Test
@WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
void testCreateExam_failsWithGracePeriodTooHigh() throws Exception {
Expand Down
Loading