diff --git a/build.gradle b/build.gradle index f2c57df98d..e1174a42f8 100644 --- a/build.gradle +++ b/build.gradle @@ -336,6 +336,8 @@ dependencies { implementation group: 'com.microsoft.graph', name: 'microsoft-graph', version: '6.24.0' implementation group: 'com.azure', name: 'azure-identity', version: '1.18.2' + implementation group: 'org.jsoup', name: 'jsoup', version: '1.22.1' + developmentOnly 'org.springframework.boot:spring-boot-devtools' annotationProcessor('org.hibernate.orm:hibernate-jpamodelgen:6.6.42.Final') developmentOnly 'org.springframework.boot:spring-boot-devtools' diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/CaptureSessionControllerFT.java b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/CaptureSessionControllerFT.java index 16402ed465..ee0cfd61ee 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/CaptureSessionControllerFT.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/CaptureSessionControllerFT.java @@ -613,4 +613,28 @@ void shouldReturn400WhenCaptureSessionProcessingWithinTimeout() throws JsonProce ); assertResponseCode(registerResponse, 400); } + + @Test + @DisplayName("Scenario: Try to create Capture Session with unsafe data") + void shouldNotCreateCaptureSessionWithUnsafeData() throws JsonProcessingException { + var res = doPostRequest("/testing-support/create-well-formed-booking", null) + .body() + .jsonPath(); + var bookingId = res.getUUID("bookingId"); + + // create capture session + var dto = createCaptureSession(bookingId); + dto.setStatus(RecordingStatus.RECORDING_AVAILABLE); + dto.setIngestAddress("rtmps://.example.com/stream"); + dto.setLiveOutputUrl("https://ep-default-live-pre-mediakind-stgsrc=x onerror=alert(1)>." + + "blob.core.windows.net/ingest"); + + var putResponse = putCaptureSession(dto); + assertResponseCode(putResponse, 400); + assertThat(putResponse.body().jsonPath().getString("ingestAddress")) + .contains("potentially malicious content"); + assertThat(putResponse.body().jsonPath().getString("liveOutputUrl")) + .contains("potentially malicious content"); + assertCaptureSessionExists(dto.getId(), false); + } } diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/CourtControllerFT.java b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/CourtControllerFT.java index 602204b054..d0edd96559 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/CourtControllerFT.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/CourtControllerFT.java @@ -6,10 +6,13 @@ import org.junit.jupiter.api.Test; import uk.gov.hmcts.reform.preapi.controllers.params.TestingSupportRoles; import uk.gov.hmcts.reform.preapi.dto.CourtEmailDTO; +import uk.gov.hmcts.reform.preapi.dto.CreateCourtDTO; +import uk.gov.hmcts.reform.preapi.enums.CourtType; import uk.gov.hmcts.reform.preapi.exception.NotFoundException; import uk.gov.hmcts.reform.preapi.util.FunctionalTestBase; import java.util.List; +import java.util.UUID; import static java.lang.String.format; import static org.assertj.core.api.Assertions.assertThat; @@ -17,6 +20,8 @@ class CourtControllerFT extends FunctionalTestBase { private static final String INPUT_CSV_PATH = "src/functionalTest/resources/test/courts/email_addresses.csv"; + private static final String INPUT_CSV_PATH_UNSAFE = + "src/functionalTest/resources/test/courts/email_addresses_unsafe.csv"; @DisplayName("Scenario: Create and update a court") @Test @@ -73,4 +78,53 @@ void updateCourtEmailAddressesFromCsv() throws JsonProcessingException { assertThat(updatedCourt.getGroupEmail()).isEqualTo("PRE.Edits.Example@justice.gov.uk"); } + @Test + @DisplayName("Should not update court email addresses from a csv file if unsafe data") + void updateCourtEmailAddressesFromCsvUnsafeData() throws JsonProcessingException { + + // create courts + var regionId = doPostRequest("/testing-support/create-region", null) + .body().jsonPath().getUUID("regionId"); + + var dto = new CreateCourtDTO(); + dto.setId(UUID.randomUUID()); + dto.setName("Examples Court"); + dto.setCourtType(CourtType.CROWN); + dto.setRegions(List.of(regionId)); + dto.setLocationCode("113456789"); + + var createResponse = putCourt(dto); + assertResponseCode(createResponse, 201); + var courtResponse1 = assertCourtExists(dto.getId(), true); + assertThat(courtResponse1.body().jsonPath().getString("name")).isEqualTo(dto.getName()); + + Response postResponse = doPostRequestWithMultipart( + COURTS_ENDPOINT + "/email", + MULTIPART_HEADERS, + INPUT_CSV_PATH_UNSAFE, + TestingSupportRoles.SUPER_USER + ); + + assertResponseCode(postResponse, 400); + assertThat(postResponse.getBody().asPrettyString()) + .contains("potentially malicious content"); + } + + @Test + @DisplayName("Should not save court that has unsafe data in fields") + void createCourtWithUnsanitizedFields() throws JsonProcessingException { + CreateCourtDTO dto = createCourt(); + dto.setName("Rejected"); + dto.setLocationCode(""); + dto.setCounty("Admin User"); + + Response response = putCourt(dto); + assertResponseCode(response, 400); + assertThat(response.body().jsonPath().getString("name")) + .contains("potentially malicious content"); + assertThat(response.body().jsonPath().getString("locationCode")) + .contains("potentially malicious content"); + assertThat(response.body().jsonPath().getString("county")) + .contains("potentially malicious content"); + } } diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/EditControllerFT.java b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/EditControllerFT.java index a4c5ca6a4d..295fba7cfd 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/EditControllerFT.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/EditControllerFT.java @@ -1,6 +1,7 @@ package uk.gov.hmcts.reform.preapi.controllers; import com.fasterxml.jackson.core.JsonProcessingException; +import io.restassured.response.Response; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -8,6 +9,7 @@ import org.junit.jupiter.params.provider.NullSource; import org.springframework.test.context.bean.override.mockito.MockitoBean; import uk.gov.hmcts.reform.preapi.controllers.params.TestingSupportRoles; +import uk.gov.hmcts.reform.preapi.dto.CreateEditRequestDTO; import uk.gov.hmcts.reform.preapi.dto.EditCutInstructionDTO; import uk.gov.hmcts.reform.preapi.dto.EditRequestDTO; import uk.gov.hmcts.reform.preapi.dto.FfmpegEditInstructionDTO; @@ -16,13 +18,16 @@ import uk.gov.hmcts.reform.preapi.media.storage.AzureFinalStorageService; import uk.gov.hmcts.reform.preapi.util.FunctionalTestBase; +import java.util.ArrayList; import java.util.List; +import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.when; public class EditControllerFT extends FunctionalTestBase { private static final String VALID_EDIT_CSV = "src/functionalTest/resources/test/edit/edit_from_csv.csv"; + private static final String UNSAFE_EDIT_CSV = "src/functionalTest/resources/test/edit/edit_from_csv_unsafe.csv"; private static final String EDIT_ENDPOINT = "/edits"; @MockitoBean @@ -96,4 +101,61 @@ void editRequestFromCsvForbidden(TestingSupportRoles role) { ); assertResponseCode(response, role == null ? 401 : 403); } + + @Test + @DisplayName("Should not create an edit with a csv that has unsafe data in fields") + void editRequestWithUnsafeDataCsv() throws JsonProcessingException { + CreateRecordingResponse recordingDetails = createRecording(); + RecordingDTO recordingDTO = assertRecordingExists(recordingDetails.recordingId(), true).as(RecordingDTO.class); + + when(azureFinalStorageService.getMp4FileName(recordingDetails.recordingId().toString())) + .thenReturn(recordingDTO.getFilename()); + when(azureFinalStorageService.getRecordingDuration(recordingDetails.recordingId())) + .thenReturn(recordingDTO.getDuration()); + + Response postResponse = doPostRequestWithMultipart( + EDIT_ENDPOINT + "/from-csv/" + recordingDetails.recordingId(), + MULTIPART_HEADERS, + UNSAFE_EDIT_CSV, + TestingSupportRoles.SUPER_USER + ); + + assertResponseCode(postResponse, 400); + } + + @SuppressWarnings("checkstyle:VariableDeclarationUsageDistance") + @Test + @DisplayName("Should not create an edit request with unsafe data in fields") + void editRequestWithUnsafeData() throws JsonProcessingException { + UUID editRequestId = UUID.randomUUID(); + CreateRecordingResponse recordingDetails = createRecording(); + + CreateEditRequestDTO editRequestDTO = new CreateEditRequestDTO(); + editRequestDTO.setSourceRecordingId(recordingDetails.recordingId()); + editRequestDTO.setStatus(EditRequestStatus.PENDING); + editRequestDTO.setId(editRequestId); + List editCutInstructionDTOS = new ArrayList<>(); + EditCutInstructionDTO cutInstruction1 = new EditCutInstructionDTO(); + cutInstruction1.setStartOfCut("00:00:00"); + cutInstruction1.setEndOfCut("00:01:00"); + cutInstruction1.setReason("Unsafe reason"); + editCutInstructionDTOS.add(cutInstruction1); + editRequestDTO.setEditInstructions(editCutInstructionDTOS); + + RecordingDTO recordingDTO = assertRecordingExists(recordingDetails.recordingId(), true).as(RecordingDTO.class); + when(azureFinalStorageService.getMp4FileName(recordingDetails.recordingId().toString())) + .thenReturn(recordingDTO.getFilename()); + when(azureFinalStorageService.getRecordingDuration(recordingDetails.recordingId())) + .thenReturn(recordingDTO.getDuration()); + + Response postResponse = doPutRequest( + EDIT_ENDPOINT + "/" + editRequestId, + OBJECT_MAPPER.writeValueAsString(editRequestDTO), + TestingSupportRoles.SUPER_USER + ); + + assertResponseCode(postResponse, 400); + assertThat(postResponse.getBody().asPrettyString()) + .contains("potentially malicious content"); + } } diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/InviteControllerFT.java b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/InviteControllerFT.java index c3c6a8620a..f6c363c8d9 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/InviteControllerFT.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/InviteControllerFT.java @@ -15,6 +15,7 @@ import static org.assertj.core.api.Assertions.assertThat; class InviteControllerFT extends FunctionalTestBase { + @DisplayName("Create a portal invite for new user") @Test void createPortalInvite() throws JsonProcessingException { @@ -168,6 +169,22 @@ void deleteInviteForNonExistingUser() throws JsonProcessingException { assertResponseCode(deleteResponse, 404); } + @DisplayName("Create a portal invite with unsafe data in fields") + @Test + void createPortalInviteWithUnSafeFields() throws JsonProcessingException { + var dto = createInvite(null); + dto.setOrganisation("Malicious Link"); + dto.setFirstName("Example"); + dto.setLastName("hello"); + assertUserExists(dto.getUserId(), false); + + var putResponse = putInvite(dto); + assertResponseCode(putResponse, 400); + assertThat(putResponse.body().jsonPath().getString("organisation")) + .contains("potentially malicious content"); + assertInviteExists(dto.getUserId(), false); + } + private CreateInviteDTO createInvite(UUID userId) { var dto = new CreateInviteDTO(); dto.setUserId(userId != null ? userId : UUID.randomUUID()); diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/UserControllerFT.java b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/UserControllerFT.java index 39c3da0b43..b756de5687 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/UserControllerFT.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/UserControllerFT.java @@ -304,6 +304,26 @@ void userFilteredByAppActiveStatus() throws JsonProcessingException { .isEqualTo(0); } + @DisplayName("Scenario: Should not create/update a user with un-sanitised data") + @Test + void shouldNoCreateUserWithUnsafeData() throws JsonProcessingException { + var dto = createUserDto(); + dto.setOrganisation(""); + dto.setFirstName("
First
"); + dto.setLastName("Last"); + dto.setPhoneNumber(""); + var createResponse = putUser(dto); + assertResponseCode(createResponse, 400); + assertThat(createResponse.body().jsonPath().getString("organisation")) + .contains("potentially malicious content"); + assertThat(createResponse.body().jsonPath().getString("firstName")) + .contains("potentially malicious content"); + assertThat(createResponse.body().jsonPath().getString("lastName")) + .contains("potentially malicious content"); + assertThat(createResponse.body().jsonPath().getString("phoneNumber")) + .contains("potentially malicious content"); + } + private UserDTO getUserById(UUID userId) { Response response = doGetRequest(USERS_ENDPOINT + "/" + userId, TestingSupportRoles.SUPER_USER); assertResponseCode(response, 200); diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/VfMigrationControllerFT.java b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/VfMigrationControllerFT.java index e02d1d9d5f..ab01cd9807 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/VfMigrationControllerFT.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/VfMigrationControllerFT.java @@ -3,6 +3,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import io.restassured.response.Response; import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; import org.junit.jupiter.params.provider.NullSource; @@ -14,6 +15,8 @@ import java.util.UUID; +import static org.assertj.core.api.Assertions.assertThat; + public class VfMigrationControllerFT extends FunctionalTestBase { private static final String MIGRATION_RECORD_ENDPOINT = "/vf-migration-records"; @@ -48,6 +51,30 @@ void putMigrationRecordsAuth(TestingSupportRoles role) throws JsonProcessingExce assertResponseCode(response, role == null ? 401 : 403); } + @Test + @DisplayName("Should not allow a migration record to be updated with unsafe data") + void putMigrationRecordsAuthWithUnsafeData() throws JsonProcessingException { + CreateVfMigrationRecordDTO dto = new CreateVfMigrationRecordDTO(); + dto.setId(UUID.randomUUID()); + dto.setStatus(VfMigrationStatus.PENDING); + dto.setRecordingVersion(VfMigrationRecordingVersion.ORIG); + dto.setUrn("1234567890"); + dto.setExhibitReference("1234567890"); + dto.setCourtId(UUID.randomUUID()); + dto.setWitnessName(""); + dto.setDefendantName(""); + Response response = doPutRequest( + MIGRATION_RECORD_ENDPOINT + "/" + dto.getId(), + OBJECT_MAPPER.writeValueAsString(dto), + TestingSupportRoles.SUPER_USER + ); + assertResponseCode(response, 400); + assertThat(response.body().jsonPath().getString("witnessName")) + .contains("potentially malicious content"); + assertThat(response.body().jsonPath().getString("defendantName")) + .contains("potentially malicious content"); + } + @NullSource @ParameterizedTest @EnumSource(value = TestingSupportRoles.class, names = "SUPER_USER", mode = EnumSource.Mode.EXCLUDE) diff --git a/src/functionalTest/resources/test/courts/email_addresses_unsafe.csv b/src/functionalTest/resources/test/courts/email_addresses_unsafe.csv new file mode 100644 index 0000000000..1c70369521 --- /dev/null +++ b/src/functionalTest/resources/test/courts/email_addresses_unsafe.csv @@ -0,0 +1,2 @@ +Region,Court,PRE Inbox Address +Last,Examples Court,Last diff --git a/src/functionalTest/resources/test/edit/edit_from_csv_unsafe.csv b/src/functionalTest/resources/test/edit/edit_from_csv_unsafe.csv new file mode 100644 index 0000000000..52517493a3 --- /dev/null +++ b/src/functionalTest/resources/test/edit/edit_from_csv_unsafe.csv @@ -0,0 +1,3 @@ +Edit Number,Start time of cut,End time of cut,Total time removed,Reason +1,00:00:00,00:01:00,00:01:00, +2,00:01:01,00:02:00,00:00:59, diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateAuditDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateAuditDTO.java index e1636a32aa..5022050a95 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateAuditDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateAuditDTO.java @@ -8,6 +8,7 @@ import jakarta.validation.constraints.NotNull; import lombok.Data; import lombok.NoArgsConstructor; +import uk.gov.hmcts.reform.preapi.dto.validators.SanitizedStringConstraint; import uk.gov.hmcts.reform.preapi.entities.Audit; import uk.gov.hmcts.reform.preapi.enums.AuditLogSource; @@ -40,14 +41,16 @@ public class CreateAuditDTO { private String category; @Schema(description = "AuditActivity", examples = {"Create", "Update", "Delete", "Check", "Play", "Locked"}) + @SanitizedStringConstraint private String activity; @Schema(description = "AuditFunctionalArea", examples = {"Registration", "Login", "Video Player", "API", "Admin"}) + @SanitizedStringConstraint private String functionalArea; @Schema(description = "AuditDetailsJSONString") @JsonRawValue - private JsonNode auditDetails; + private JsonNode auditDetails; //TODO: Sanitised annotation to be added later public CreateAuditDTO(Audit auditEntity) { this.id = auditEntity.getId(); diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateCaptureSessionDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateCaptureSessionDTO.java index 311f7f1484..5230d46b46 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateCaptureSessionDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateCaptureSessionDTO.java @@ -6,6 +6,7 @@ import jakarta.validation.constraints.NotNull; import lombok.Data; import lombok.NoArgsConstructor; +import uk.gov.hmcts.reform.preapi.dto.validators.SanitizedStringConstraint; import uk.gov.hmcts.reform.preapi.entities.CaptureSession; import uk.gov.hmcts.reform.preapi.enums.RecordingOrigin; import uk.gov.hmcts.reform.preapi.enums.RecordingStatus; @@ -31,9 +32,11 @@ public class CreateCaptureSessionDTO { private RecordingOrigin origin; @Schema(description = "CreateCaptureSessionIngestAddress") + @SanitizedStringConstraint private String ingestAddress; @Schema(description = "CreateCaptureSessionLiveOutputURL") + @SanitizedStringConstraint private String liveOutputUrl; @Schema(description = "CreateCaptureSessionStartedAt") diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateCourtDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateCourtDTO.java index f757bd4e2e..1a0b61d682 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateCourtDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateCourtDTO.java @@ -8,6 +8,7 @@ import jakarta.validation.constraints.Size; import lombok.Data; import lombok.NoArgsConstructor; +import uk.gov.hmcts.reform.preapi.dto.validators.SanitizedStringConstraint; import uk.gov.hmcts.reform.preapi.enums.CourtType; import java.util.List; @@ -22,6 +23,7 @@ public class CreateCourtDTO { @NotNull(message = "id is required") private UUID id; + @SanitizedStringConstraint @Schema(description = "CreateCourtName") @NotNull(message = "name is required") private String name; @@ -30,10 +32,12 @@ public class CreateCourtDTO { @NotNull(message = "court_type is required") private CourtType courtType; + @SanitizedStringConstraint @Schema(description = "CreateCourtLocationCode") @NotNull(message = "location_code is required") private String locationCode; + @SanitizedStringConstraint @Schema(description = "CreateCourtCounty") private String county; diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateEditRequestDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateEditRequestDTO.java index 4172b50ff2..3f0e9f268c 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateEditRequestDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateEditRequestDTO.java @@ -9,6 +9,7 @@ import lombok.Data; import lombok.NoArgsConstructor; import uk.gov.hmcts.reform.preapi.dto.validators.CreateEditRequestStatusConstraint; +import uk.gov.hmcts.reform.preapi.dto.validators.SanitizedStringConstraint; import uk.gov.hmcts.reform.preapi.enums.EditRequestStatus; import java.sql.Timestamp; @@ -41,6 +42,7 @@ public class CreateEditRequestDTO { private Boolean jointlyAgreed; @Size(max = 512) + @SanitizedStringConstraint @Schema(description = "CreateEditRequestRejectionReason") private String rejectionReason; @@ -48,6 +50,7 @@ public class CreateEditRequestDTO { private Timestamp approvedAt; @Size(max = 100) + @SanitizedStringConstraint @Schema(description = "CreateEditRequestApprovedBy") private String approvedBy; } diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateInviteDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateInviteDTO.java index d30ea69755..f4b037aee5 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateInviteDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateInviteDTO.java @@ -8,6 +8,7 @@ import jakarta.validation.constraints.NotNull; import lombok.Data; import lombok.NoArgsConstructor; +import uk.gov.hmcts.reform.preapi.dto.validators.SanitizedStringConstraint; import java.util.UUID; @@ -22,10 +23,12 @@ public class CreateInviteDTO { @Schema(description = "InviteFirstName") @NotBlank + @SanitizedStringConstraint protected String firstName; @Schema(description = "InviteLastName") @NotBlank + @SanitizedStringConstraint protected String lastName; @Schema(description = "InviteEmail") @@ -34,9 +37,11 @@ public class CreateInviteDTO { protected String email; @Schema(description = "InviteOrganisation") + @SanitizedStringConstraint protected String organisation; @Schema(description = "InvitePhone") - protected String phone; + @SanitizedStringConstraint + protected String phone; //TODO: Sanitised constraint added but needs specific phone number validation in future } diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateParticipantDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateParticipantDTO.java index 47b0571af8..e66aab0ef5 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateParticipantDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/CreateParticipantDTO.java @@ -5,6 +5,7 @@ import io.swagger.v3.oas.annotations.media.Schema; import lombok.Data; import lombok.NoArgsConstructor; +import uk.gov.hmcts.reform.preapi.dto.validators.SanitizedStringConstraint; import uk.gov.hmcts.reform.preapi.entities.Participant; import uk.gov.hmcts.reform.preapi.enums.ParticipantType; @@ -21,9 +22,11 @@ public class CreateParticipantDTO { @Schema(description = "CreateParticipantType") private ParticipantType participantType; + @SanitizedStringConstraint @Schema(description = "CreateParticipantFirstName") private String firstName; + @SanitizedStringConstraint @Schema(description = "CreateParticipantLastName") private String lastName; diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/EditCutInstructionDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/EditCutInstructionDTO.java index cfd52e9a2d..d430ee03b8 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/EditCutInstructionDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/EditCutInstructionDTO.java @@ -10,6 +10,7 @@ import lombok.Builder; import lombok.Data; import lombok.NoArgsConstructor; +import uk.gov.hmcts.reform.preapi.dto.validators.SanitizedStringConstraint; import uk.gov.hmcts.reform.preapi.exception.BadRequestException; @Data @@ -36,6 +37,7 @@ public class EditCutInstructionDTO { private String endOfCut; @CsvBindByName(column = "Reason") + @SanitizedStringConstraint private String reason; @Schema(hidden = true) diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/base/BaseRecordingDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/base/BaseRecordingDTO.java index d77b16c0f0..fb881a4221 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/base/BaseRecordingDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/base/BaseRecordingDTO.java @@ -8,6 +8,7 @@ import lombok.Data; import lombok.NoArgsConstructor; import uk.gov.hmcts.reform.preapi.dto.validators.JsonConstraint; +import uk.gov.hmcts.reform.preapi.dto.validators.SanitizedStringConstraint; import java.time.Duration; import java.util.UUID; @@ -29,10 +30,12 @@ public abstract class BaseRecordingDTO { protected Integer version; @Schema(description = "RecordingURL") + @SanitizedStringConstraint protected String url; // is this not needed now as it's different for every user? @Schema(description = "RecordingFilename") @NotNull(message = "filename is required") + @SanitizedStringConstraint protected String filename; @Schema( diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/base/BaseUserDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/base/BaseUserDTO.java index 344735330f..b5631236fb 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/base/BaseUserDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/base/BaseUserDTO.java @@ -9,6 +9,7 @@ import jakarta.validation.constraints.Size; import lombok.Data; import lombok.NoArgsConstructor; +import uk.gov.hmcts.reform.preapi.dto.validators.SanitizedStringConstraint; import uk.gov.hmcts.reform.preapi.entities.User; import java.util.UUID; @@ -23,10 +24,12 @@ public class BaseUserDTO { @NotNull protected UUID id; + @SanitizedStringConstraint @Schema(description = "UserFirstName") @NotBlank protected String firstName; + @SanitizedStringConstraint @Schema(description = "UserLastName") @NotBlank protected String lastName; @@ -41,9 +44,11 @@ public class BaseUserDTO { @Email protected String alternativeEmail; + @SanitizedStringConstraint @Schema(description = "UserPhoneNumber") protected String phoneNumber; + @SanitizedStringConstraint @Schema(description = "UserOrganisation") protected String organisation; diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/migration/CreateVfMigrationRecordDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/migration/CreateVfMigrationRecordDTO.java index 9687bd043a..5d01e71736 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/migration/CreateVfMigrationRecordDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/migration/CreateVfMigrationRecordDTO.java @@ -11,6 +11,7 @@ import uk.gov.hmcts.reform.preapi.batch.application.enums.VfMigrationRecordingVersion; import uk.gov.hmcts.reform.preapi.batch.application.enums.VfMigrationStatus; import uk.gov.hmcts.reform.preapi.dto.validators.AlphanumericConstraint; +import uk.gov.hmcts.reform.preapi.dto.validators.SanitizedStringConstraint; import java.sql.Timestamp; import java.util.UUID; @@ -39,10 +40,12 @@ public class CreateVfMigrationRecordDTO { @Length(max = 25) @Schema(description = "CreateMigrationRecordDefendantName") + @SanitizedStringConstraint private String defendantName; @Length(max = 25) @Schema(description = "CreateMigrationRecordWitnessName") + @SanitizedStringConstraint private String witnessName; @Schema(description = "CreateMigrationRecordRecordingVersion") diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraint.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraint.java new file mode 100644 index 0000000000..f17a04e892 --- /dev/null +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraint.java @@ -0,0 +1,32 @@ +package uk.gov.hmcts.reform.preapi.dto.validators; + +import jakarta.validation.Constraint; +import jakarta.validation.Payload; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Validates and sanitizes string input to prevent XSS attacks. + * This annotation can be applied to String fields to ensure they are + * properly sanitized before processing. + */ +@Documented +@Constraint(validatedBy = SanitizedStringValidator.class) +@Target({ ElementType.FIELD, ElementType.PARAMETER }) +@Retention(RetentionPolicy.RUNTIME) +public @interface SanitizedStringConstraint { + String message() default "contains potentially malicious content"; + Class[] groups() default {}; + Class[] payload() default {}; + + /** + * Whether to allow basic text formatting (bold, italic, etc.). + * Default is false (strips all HTML) + */ + boolean allowBasicFormatting() default false; +} + diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringValidator.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringValidator.java new file mode 100644 index 0000000000..95a8bd265e --- /dev/null +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringValidator.java @@ -0,0 +1,25 @@ +package uk.gov.hmcts.reform.preapi.dto.validators; + +import jakarta.validation.ConstraintValidator; +import jakarta.validation.ConstraintValidatorContext; +import uk.gov.hmcts.reform.preapi.utils.InputSanitizerUtils; + +/** + * Validator that checks if a string contains potentially malicious content. + * This validator uses JSoup to detect HTML/script content that could lead to XSS attacks. + */ +public class SanitizedStringValidator implements ConstraintValidator { + + private boolean allowBasicFormatting; + + @Override + public void initialize(SanitizedStringConstraint constraint) { + this.allowBasicFormatting = constraint.allowBasicFormatting(); + } + + @Override + public boolean isValid(String value, ConstraintValidatorContext context) { + return InputSanitizerUtils.isValid(value, allowBasicFormatting); + } +} + diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/services/CourtService.java b/src/main/java/uk/gov/hmcts/reform/preapi/services/CourtService.java index 94cf9feccb..a18fb65c89 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/services/CourtService.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/services/CourtService.java @@ -17,10 +17,12 @@ import uk.gov.hmcts.reform.preapi.entities.Region; import uk.gov.hmcts.reform.preapi.enums.CourtType; import uk.gov.hmcts.reform.preapi.enums.UpsertResult; +import uk.gov.hmcts.reform.preapi.exception.BadRequestException; import uk.gov.hmcts.reform.preapi.exception.NotFoundException; import uk.gov.hmcts.reform.preapi.exception.UnknownServerException; import uk.gov.hmcts.reform.preapi.repositories.CourtRepository; import uk.gov.hmcts.reform.preapi.repositories.RegionRepository; +import uk.gov.hmcts.reform.preapi.utils.InputSanitizerUtils; import java.io.BufferedReader; import java.io.InputStreamReader; @@ -107,6 +109,10 @@ public List updateCourtEmails(MultipartFile inputFile) { } Court courtInDb = courtRepository.findFirstByName(court.getName()) .orElseThrow(() -> new NotFoundException("Court does not exist: " + court.getName())); + if (!InputSanitizerUtils.isValid(court.getGroupEmail(), false)) { + throw new BadRequestException("Court email may contain potentially malicious content: " + + court.getGroupEmail()); + } courtInDb.setGroupEmail(court.getGroupEmail()); courtRepository.save(courtInDb); diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/services/EditRequestService.java b/src/main/java/uk/gov/hmcts/reform/preapi/services/EditRequestService.java index 107793bf49..8ee9d870bf 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/services/EditRequestService.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/services/EditRequestService.java @@ -42,6 +42,7 @@ import uk.gov.hmcts.reform.preapi.repositories.EditRequestRepository; import uk.gov.hmcts.reform.preapi.repositories.RecordingRepository; import uk.gov.hmcts.reform.preapi.security.authentication.UserAuthentication; +import uk.gov.hmcts.reform.preapi.utils.InputSanitizerUtils; import java.io.BufferedReader; import java.io.InputStreamReader; @@ -300,6 +301,12 @@ public EditRequestDTO upsert(UUID sourceRecordingId, MultipartFile file) { dto.setSourceRecordingId(sourceRecordingId); dto.setEditInstructions(parseCsv(file)); dto.setStatus(EditRequestStatus.PENDING); + dto.getEditInstructions().forEach(editInstruction -> { + if (!InputSanitizerUtils.isValid(editInstruction.getReason(), false)) { + throw new BadRequestException("Edit instruction reason potentially contains malicious code: " + + editInstruction.getReason()); + } + }); upsert(dto); diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java new file mode 100644 index 0000000000..8c4d3b64da --- /dev/null +++ b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java @@ -0,0 +1,66 @@ +package uk.gov.hmcts.reform.preapi.utils; + +import lombok.experimental.UtilityClass; +import org.jsoup.Jsoup; +import org.jsoup.nodes.Document; +import org.jsoup.safety.Cleaner; +import org.jsoup.safety.Safelist; + +/** + * Utility class for sanitizing user input to prevent XSS and other injection attacks. + * Uses JSoup library to parse and clean HTML content. + */ +@UtilityClass +@SuppressWarnings({"checkstyle:HideUtilityClassConstructor", "checkstyle:JavadocParagraph"}) +public class InputSanitizerUtils { + + // Reuse Cleaner instances for better performance + private static final Cleaner STRICT_CLEANER = new Cleaner(Safelist.none()); + private static final Cleaner BASIC_CLEANER = new Cleaner(Safelist.basic()); + + /** + * Sanitizes input by removing all HTML tags and returning plain text. + * This is the strictest sanitization mode. + * + * @param input The string to sanitize + * @return Sanitized plain text, or null if input is null + */ + public static String sanitize(String input) { + return sanitize(input, false); + } + + /** + * Sanitizes input with optional support for basic text formatting. + * If allowBasicFormatting is true, it allows safe HTML tags + * If allowBasicFormatting is false it will strip HTML/Script tags fron input. + * @param input The string to sanitize + * @param allowBasicFormatting If true, allows safe HTML tags like the ones for bold, emphasis, paragraphs etc. + * @return Sanitized text, or null if input is null + */ + private static String sanitize(String input, boolean allowBasicFormatting) { + if (input == null) { + return null; + } + + Cleaner cleaner = allowBasicFormatting ? BASIC_CLEANER : STRICT_CLEANER; + + // If strict mode (no formatting), return just the text content + if (!allowBasicFormatting) { + return Jsoup.clean(input, "", Safelist.none(), new Document.OutputSettings().prettyPrint(false)); + } + + // Parse the input as HTML and clean it + return cleaner.clean(Jsoup.parse(input)).body().html(); + } + + public static boolean isValid(String value, boolean allowBasicFormatting) { + if (value == null) { + return true; + } + + // Check if sanitization would change the string + // If it changes, it means there was potentially malicious content + String sanitized = sanitize(value, allowBasicFormatting); + return value.equals(sanitized); + } +} diff --git a/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringValidatorTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringValidatorTest.java new file mode 100644 index 0000000000..689a47bcc9 --- /dev/null +++ b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringValidatorTest.java @@ -0,0 +1,141 @@ +package uk.gov.hmcts.reform.preapi.dto.validators; + +import jakarta.validation.ConstraintValidatorContext; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.when; + +class SanitizedStringValidatorTest { + + private SanitizedStringValidator validator; + + @Mock + private SanitizedStringConstraint constraint; + + @Mock + private ConstraintValidatorContext context; + + private AutoCloseable mocks; + + @BeforeEach + void setUp() { + mocks = MockitoAnnotations.openMocks(this); + validator = new SanitizedStringValidator(); + } + + @org.junit.jupiter.api.AfterEach + void tearDown() throws Exception { + if (mocks != null) { + mocks.close(); + } + } + + @Test + @DisplayName("Should return true for null value") + void validateNullValue() { + when(constraint.allowBasicFormatting()).thenReturn(false); + validator.initialize(constraint); + + assertTrue(validator.isValid(null, context)); + } + + @Test + @DisplayName("Should return true for plain text without HTML") + void validatePlainText() { + when(constraint.allowBasicFormatting()).thenReturn(false); + validator.initialize(constraint); + + assertTrue(validator.isValid("Hello World", context)); + } + + @Test + @DisplayName("Should return false for text with script tags") + void validateScriptTag() { + when(constraint.allowBasicFormatting()).thenReturn(false); + validator.initialize(constraint); + + assertFalse(validator.isValid("", context)); + } + + @Test + @DisplayName("Should return false for text with HTML tags in strict mode") + void validateHtmlTagsStrictMode() { + when(constraint.allowBasicFormatting()).thenReturn(false); + validator.initialize(constraint); + + assertFalse(validator.isValid("Bold Text", context)); + } + + @Test + @DisplayName("Should return true for text with safe HTML when basic formatting allowed") + void validateSafeHtmlWithBasicFormatting() { + when(constraint.allowBasicFormatting()).thenReturn(true); + validator.initialize(constraint); + + assertTrue(validator.isValid("Bold and Italic", context)); + } + + @Test + @DisplayName("Should return false for text with dangerous HTML even with basic formatting") + void validateDangerousHtmlWithBasicFormatting() { + when(constraint.allowBasicFormatting()).thenReturn(true); + validator.initialize(constraint); + + assertFalse(validator.isValid("", context)); + } + + @Test + @DisplayName("Should return false for text with event handlers") + void validateEventHandlers() { + when(constraint.allowBasicFormatting()).thenReturn(false); + validator.initialize(constraint); + + assertFalse(validator.isValid("", context)); + } + + @Test + @DisplayName("Should return false for text with JavaScript in href") + void validateJavaScriptHref() { + when(constraint.allowBasicFormatting()).thenReturn(false); + validator.initialize(constraint); + + assertFalse(validator.isValid("Click", context)); + } + + @Test + @DisplayName("Should return true for empty string") + void validateEmptyString() { + when(constraint.allowBasicFormatting()).thenReturn(false); + validator.initialize(constraint); + + assertTrue(validator.isValid("", context)); + } + + @Test + @DisplayName("Should return true, Jsoup should not consider whitespace as malicious content") + void validateWhitespace() { + when(constraint.allowBasicFormatting()).thenReturn(false); + validator.initialize(constraint); + + // JSoup's text() method normalizes whitespace, so " " becomes "" + assertTrue(validator.isValid(" ", context)); + } + + @Test + @DisplayName("Should return true, Jsoup should not consider whitespace as malicious content") + void validateWhitespaceInText() { + when(constraint.allowBasicFormatting()).thenReturn(false); + validator.initialize(constraint); + + // JSoup's text() method normalizes whitespace, so " " becomes "" + assertTrue(validator.isValid(" My Organisation is great ", context)); + } +} + + diff --git a/src/test/java/uk/gov/hmcts/reform/preapi/services/CourtServiceTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/services/CourtServiceTest.java index 085912a61b..2c5c579fe6 100644 --- a/src/test/java/uk/gov/hmcts/reform/preapi/services/CourtServiceTest.java +++ b/src/test/java/uk/gov/hmcts/reform/preapi/services/CourtServiceTest.java @@ -6,6 +6,7 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.data.domain.PageImpl; @@ -19,6 +20,7 @@ import uk.gov.hmcts.reform.preapi.entities.Region; import uk.gov.hmcts.reform.preapi.enums.CourtType; import uk.gov.hmcts.reform.preapi.enums.UpsertResult; +import uk.gov.hmcts.reform.preapi.exception.BadRequestException; import uk.gov.hmcts.reform.preapi.exception.NotFoundException; import uk.gov.hmcts.reform.preapi.exception.UnknownServerException; import uk.gov.hmcts.reform.preapi.repositories.CourtRepository; @@ -283,4 +285,27 @@ void updateCourtEmailAddressesThrowErrorForInvalidCsv() { verifyNoInteractions(courtRepository); } + + @DisplayName("Should not update court addresses with unsafe data") + @Test + void updateCourtEmailAddressesWithUnsafeData() { + final String fileContents = """ +Court,PRE Inbox Address +Example Court,@justice.gov.uk + """; + + MockMultipartFile file = new MockMultipartFile( + "file", "email_addresses.csv", + PreApiController.CSV_FILE_TYPE, fileContents.getBytes() + ); + + when(courtRepository.findFirstByName("Example Court")).thenReturn(Optional.of(courtEntity)); + when(courtRepository.findAll()).thenReturn(List.of(courtEntity)); + + assertThrows( + BadRequestException.class, + () -> courtService.updateCourtEmails(file)); + + verify(courtRepository, Mockito.never()).save(any()); + } } diff --git a/src/test/java/uk/gov/hmcts/reform/preapi/services/EditRequestServiceTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/services/EditRequestServiceTest.java index 4b2433947d..9c4b03fa09 100644 --- a/src/test/java/uk/gov/hmcts/reform/preapi/services/EditRequestServiceTest.java +++ b/src/test/java/uk/gov/hmcts/reform/preapi/services/EditRequestServiceTest.java @@ -1400,6 +1400,26 @@ void upsertOnRejected() { verify(editNotificationService, times(1)).onEditRequestRejected(editRequest); } + @DisplayName("Should throw an exception if edit instructions have unsafe data") + @Test + void upsertEditInstructionsWithUnsafeCSVFile() { + final String fileContents = """ + Edit Number,Start time of cut,End time of cut,Total time removed,Reason + 1,00:00:00,00:00:30,00:30:00, + 2,00:01:01,00:02:00,00:00:59, + """; + + final MockMultipartFile file = new MockMultipartFile( + "file", "edit_instructions.csv", + PreApiController.CSV_FILE_TYPE, fileContents.getBytes() + ); + + assertThrows( + BadRequestException.class, + () -> underTest.upsert(mockRecordingId, file) + ); + } + private static void assertEditInstructionsEq(List expected, List actual) { diff --git a/src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerUtilsTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerUtilsTest.java new file mode 100644 index 0000000000..9f61e1acf3 --- /dev/null +++ b/src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerUtilsTest.java @@ -0,0 +1,37 @@ +package uk.gov.hmcts.reform.preapi.util; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import uk.gov.hmcts.reform.preapi.utils.InputSanitizerUtils; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +public class InputSanitizerUtilsTest { + + @ParameterizedTest + @ValueSource(strings = {"TEST", "TEST", "TEST", "TEST", + "TEST"}) + public void shouldSanitizeHtml(String input) { + String expectedOutput = "TEST"; + String actualOutput = InputSanitizerUtils.sanitize(input); + + assertThat(actualOutput).isEqualTo(expectedOutput); + } + + @Test + public void shouldSanitizeScriptTags() { + String input = + """ + "> + """; + + assertThat(input).isNotEqualTo(InputSanitizerUtils.sanitize(input)); + } + + @Test + public void shouldReturnNullIfInputIsNull() { + String actualOutput = InputSanitizerUtils.sanitize(null); + assertThat(actualOutput).isNull(); + } +}