From 2e1efc06efd6750460f86ea14b285c84dca5bf60 Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Wed, 18 Mar 2026 15:37:47 +0000 Subject: [PATCH 01/28] adds sanitizer --- build.gradle | 2 + .../reform/preapi/utils/InputSanitizer.java | 18 ++++++++ .../preapi/util/InputSanitizerTest.java | 42 +++++++++++++++++++ 3 files changed, 62 insertions(+) create mode 100644 src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java create mode 100644 src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerTest.java diff --git a/build.gradle b/build.gradle index f2c57df98..e1174a42f 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/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java new file mode 100644 index 000000000..4f931ac22 --- /dev/null +++ b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java @@ -0,0 +1,18 @@ +package uk.gov.hmcts.reform.preapi.utils; + +import lombok.experimental.UtilityClass; +import org.jsoup.Jsoup; +import org.jsoup.safety.Cleaner; +import org.jsoup.safety.Safelist; + +@UtilityClass +public class InputSanitizer { + + public static String sanitize(String input) { + + Cleaner cleaner = new Cleaner(Safelist.none()); + String text = cleaner.clean(Jsoup.parse(input)).text(); + + return text; + } +} diff --git a/src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerTest.java new file mode 100644 index 000000000..e142a1173 --- /dev/null +++ b/src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerTest.java @@ -0,0 +1,42 @@ +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.InputSanitizer; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +public class InputSanitizerTest { + + @ParameterizedTest + @ValueSource(strings = {"TEST", "TEST", "TEST"}) + public void shouldSanitizeHtml(String input) { + String expectedOutput = "TEST"; + String actualOutput = InputSanitizer.sanitize(input); + + assertThat(actualOutput).isEqualTo(expectedOutput); + } + + @Test + public void shouldSanitizeScriptTags() { + String input = """ + "> + """; + String expectedOutput = "\">"; + + String actualOutput = InputSanitizer.sanitize(input); + + assertThat(actualOutput).isEqualTo(expectedOutput); + } + + @Test + public void shouldSanitizeLink() { + String input = "http://example.com"; + String expectedOutput = ""; + + String actualOutput = InputSanitizer.sanitize(input); + + assertThat(actualOutput).isEqualTo(expectedOutput); + } +} From 9e1fef9e06f495bf863afc7b63a42bebb0cc8087 Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Wed, 18 Mar 2026 17:22:41 +0000 Subject: [PATCH 02/28] wip --- .../reform/preapi/utils/InputSanitizer.java | 32 +++++++++++++++++++ .../preapi/util/InputSanitizerTest.java | 16 ++++++---- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java index 4f931ac22..0ebf81f28 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java @@ -5,6 +5,11 @@ import org.jsoup.safety.Cleaner; import org.jsoup.safety.Safelist; +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + @UtilityClass public class InputSanitizer { @@ -15,4 +20,31 @@ public static String sanitize(String input) { return text; } + + public static Object sanitizeObject(Object dto) throws IllegalAccessException { + if (dto == null) { + return null; + } + Field[] fields = getAllFields(dto.getClass()); + for (Field field : fields) { + if (field.getType() == String.class && !(field.get(dto) == null)) { + field.setAccessible(true); + field.set(dto, sanitize((String) field.get(dto))); + } + } + + return dto; + } + + + public static Field[] getAllFields(Class type) { + List fields = new ArrayList<>(); + + for (Class c = type; c != null; c = c.getSuperclass()) { + fields.addAll(Arrays.asList(c.getDeclaredFields())); + } + + return fields.toArray(new Field[0]); + } + } diff --git a/src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerTest.java index e142a1173..043ce2e02 100644 --- a/src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerTest.java +++ b/src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerTest.java @@ -3,6 +3,7 @@ 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.dto.UserDTO; import uk.gov.hmcts.reform.preapi.utils.InputSanitizer; import static org.assertj.core.api.AssertionsForClassTypes.assertThat; @@ -10,7 +11,8 @@ public class InputSanitizerTest { @ParameterizedTest - @ValueSource(strings = {"TEST", "TEST", "TEST"}) + @ValueSource(strings = {"TEST", "TEST", "TEST", "TEST", + "TEST"}) public void shouldSanitizeHtml(String input) { String expectedOutput = "TEST"; String actualOutput = InputSanitizer.sanitize(input); @@ -31,12 +33,14 @@ public void shouldSanitizeScriptTags() { } @Test - public void shouldSanitizeLink() { - String input = "http://example.com"; - String expectedOutput = ""; + public void shouldSanitizeObject() throws IllegalAccessException { + UserDTO userDTO = new UserDTO(); + userDTO.setFirstName("Test"); + userDTO.setLastName("Test"); - String actualOutput = InputSanitizer.sanitize(input); + UserDTO sanitizedUserDTO = (UserDTO) InputSanitizer.sanitizeObject(userDTO); - assertThat(actualOutput).isEqualTo(expectedOutput); + assertThat(sanitizedUserDTO.getFirstName()).isEqualTo("Test"); + assertThat(sanitizedUserDTO.getLastName()).isEqualTo("Test"); } } From 7af38deae6112276d70d0b33a2ee8235ebb24112 Mon Sep 17 00:00:00 2001 From: Alex Sealey Date: Fri, 20 Mar 2026 09:07:42 +0000 Subject: [PATCH 03/28] Draft sanitizer via annotation --- .../config/InputSanitizationFilter.java | 96 +++++++++++ .../reform/preapi/dto/CreateCourtDTO.java | 4 + .../preapi/dto/CreateEditRequestDTO.java | 3 + .../preapi/dto/CreateParticipantDTO.java | 3 + .../preapi/dto/ExampleSanitizedDTO.java | 47 ++++++ .../reform/preapi/dto/base/BaseUserDTO.java | 5 + .../validators/SanitizedStringConstraint.java | 32 ++++ .../validators/SanitizedStringValidator.java | 33 ++++ .../reform/preapi/utils/InputSanitizer.java | 42 ++++- ...itizedStringConstraintIntegrationTest.java | 158 ++++++++++++++++++ .../SanitizedStringValidatorTest.java | 131 +++++++++++++++ 11 files changed, 551 insertions(+), 3 deletions(-) create mode 100644 src/main/java/uk/gov/hmcts/reform/preapi/config/InputSanitizationFilter.java create mode 100644 src/main/java/uk/gov/hmcts/reform/preapi/dto/ExampleSanitizedDTO.java create mode 100644 src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraint.java create mode 100644 src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringValidator.java create mode 100644 src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraintIntegrationTest.java create mode 100644 src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringValidatorTest.java diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/config/InputSanitizationFilter.java b/src/main/java/uk/gov/hmcts/reform/preapi/config/InputSanitizationFilter.java new file mode 100644 index 000000000..4eec95d01 --- /dev/null +++ b/src/main/java/uk/gov/hmcts/reform/preapi/config/InputSanitizationFilter.java @@ -0,0 +1,96 @@ +package uk.gov.hmcts.reform.preapi.config; + +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletRequestWrapper; +import jakarta.servlet.http.HttpServletResponse; +import org.springframework.web.filter.OncePerRequestFilter; +import uk.gov.hmcts.reform.preapi.utils.InputSanitizer; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.Map; + +/** + * ALTERNATIVE APPROACH: Filter that sanitizes all request parameters and headers. + * This is commented out by default - use @SanitizedStringConstraint on DTOs instead. + * + * To enable this filter, uncomment the @Component annotation. + * Note: This approach sanitizes ALL string inputs, which may be too aggressive. + */ +// @Component +// @Order(Ordered.HIGHEST_PRECEDENCE) +@SuppressWarnings("unused") // This is an optional alternative approach +public class InputSanitizationFilter extends OncePerRequestFilter { + + @Override + @SuppressWarnings("NullableProblems") // Spring framework guarantees non-null parameters + protected void doFilterInternal(HttpServletRequest request, + HttpServletResponse response, + FilterChain filterChain) + throws ServletException, IOException { + + // Wrap the request to sanitize parameters + SanitizedHttpServletRequest sanitizedRequest = new SanitizedHttpServletRequest(request); + + filterChain.doFilter(sanitizedRequest, response); + } + + /** + * Request wrapper that sanitizes parameter values + */ + private static class SanitizedHttpServletRequest extends HttpServletRequestWrapper { + private final Map sanitizedParameterMap; + + public SanitizedHttpServletRequest(HttpServletRequest request) { + super(request); + this.sanitizedParameterMap = sanitizeParameters(request.getParameterMap()); + } + + @Override + public String getParameter(String name) { + String[] values = sanitizedParameterMap.get(name); + return (values != null && values.length > 0) ? values[0] : null; + } + + @Override + public Map getParameterMap() { + return Collections.unmodifiableMap(sanitizedParameterMap); + } + + @Override + public Enumeration getParameterNames() { + return Collections.enumeration(sanitizedParameterMap.keySet()); + } + + @Override + public String[] getParameterValues(String name) { + return sanitizedParameterMap.get(name); + } + + private Map sanitizeParameters(Map originalMap) { + Map sanitizedMap = new HashMap<>(); + + for (Map.Entry entry : originalMap.entrySet()) { + String key = entry.getKey(); + String[] values = entry.getValue(); + + if (values != null) { + String[] sanitizedValues = Arrays.stream(values) + .map(InputSanitizer::sanitize) + .toArray(String[]::new); + sanitizedMap.put(key, sanitizedValues); + } else { + sanitizedMap.put(key, null); + } + } + + return sanitizedMap; + } + } +} + 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 f757bd4e2..1a0b61d68 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 4172b50ff..4d91dc3bf 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(message = "rejectionReason contains potentially malicious content") @Schema(description = "CreateEditRequestRejectionReason") private String rejectionReason; @@ -48,6 +50,7 @@ public class CreateEditRequestDTO { private Timestamp approvedAt; @Size(max = 100) + @SanitizedStringConstraint(message = "approvedBy contains potentially malicious content") @Schema(description = "CreateEditRequestApprovedBy") private String approvedBy; } 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 47b0571af..e66aab0ef 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/ExampleSanitizedDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/ExampleSanitizedDTO.java new file mode 100644 index 000000000..25e6edaf1 --- /dev/null +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/ExampleSanitizedDTO.java @@ -0,0 +1,47 @@ +package uk.gov.hmcts.reform.preapi.dto; + +import com.fasterxml.jackson.databind.PropertyNamingStrategies; +import com.fasterxml.jackson.databind.annotation.JsonNaming; +import io.swagger.v3.oas.annotations.media.Schema; +import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Size; +import lombok.Data; +import lombok.NoArgsConstructor; +import uk.gov.hmcts.reform.preapi.dto.validators.SanitizedStringConstraint; + +import java.util.UUID; + +/** + * EXAMPLE DTO showing how to use the @SanitizedStringConstraint annotation + * to protect against XSS attacks on user-provided string fields. + * + * This is a sample/reference implementation - not meant to be used directly. + */ +@Data +@NoArgsConstructor +@Schema(description = "ExampleDTO - Shows how to apply input sanitization") +@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class) +@SuppressWarnings("unused") // This is an example/reference implementation +public class ExampleSanitizedDTO { + + @NotNull + @Schema(description = "Example ID") + private UUID id; + + @NotNull + @Size(max = 100) + @SanitizedStringConstraint // Ensures no HTML/scripts can be injected + @Schema(description = "Example Name - Will be validated for XSS attacks") + private String name; + + @Size(max = 500) + @SanitizedStringConstraint(message = "description contains potentially malicious HTML content") + @Schema(description = "Example Description - Strictly sanitized (strict by default)") + private String description; + + @Size(max = 1000) + @SanitizedStringConstraint(allowBasicFormatting = true) // Allow , ,

,
etc. + @Schema(description = "Example Notes - Allows basic formatting") + private String notes; +} + 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 344735330..b5631236f 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/validators/SanitizedStringConstraint.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraint.java new file mode 100644 index 000000000..4de6e6a2a --- /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 000000000..265b8b8dd --- /dev/null +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringValidator.java @@ -0,0 +1,33 @@ +package uk.gov.hmcts.reform.preapi.dto.validators; + +import jakarta.validation.ConstraintValidator; +import jakarta.validation.ConstraintValidatorContext; +import uk.gov.hmcts.reform.preapi.utils.InputSanitizer; + +/** + * 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) { + if (value == null) { + return true; // Use @NotNull for null checks + } + + // Check if sanitization would change the string + // If it changes, it means there was potentially malicious content + String sanitized = InputSanitizer.sanitize(value, allowBasicFormatting); + + return value.equals(sanitized); + } +} + diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java index 4f931ac22..02e831e43 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java @@ -5,14 +5,50 @@ 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 public class InputSanitizer { + // 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. + * + * @param input The string to sanitize + * @param allowBasicFormatting If true, allows safe HTML tags like , ,

,
+ * @return Sanitized text, or null if input is null + */ + public static String sanitize(String input, boolean allowBasicFormatting) { + if (input == null) { + return null; + } + + Cleaner cleaner = allowBasicFormatting ? BASIC_CLEANER : STRICT_CLEANER; + + // Parse the input as HTML and clean it + String cleaned = cleaner.clean(Jsoup.parse(input)).body().html(); - Cleaner cleaner = new Cleaner(Safelist.none()); - String text = cleaner.clean(Jsoup.parse(input)).text(); + // If strict mode (no formatting), return just the text content + if (!allowBasicFormatting) { + return Jsoup.parse(cleaned).text(); + } - return text; + return cleaned; } } diff --git a/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraintIntegrationTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraintIntegrationTest.java new file mode 100644 index 000000000..607ea5af8 --- /dev/null +++ b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraintIntegrationTest.java @@ -0,0 +1,158 @@ +package uk.gov.hmcts.reform.preapi.dto.validators; + +import jakarta.validation.ConstraintViolation; +import jakarta.validation.Validation; +import jakarta.validation.Validator; +import jakarta.validation.ValidatorFactory; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import uk.gov.hmcts.reform.preapi.dto.CreateEditRequestDTO; +import uk.gov.hmcts.reform.preapi.enums.EditRequestStatus; + +import java.util.Set; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Integration test showing how @SanitizedStringConstraint works with real DTOs. + * This demonstrates end-to-end validation as it would happen in a controller. + */ +class SanitizedStringConstraintIntegrationTest { + + private Validator validator; + + @BeforeEach + void setUp() { + try (ValidatorFactory factory = Validation.buildDefaultValidatorFactory()) { + validator = factory.getValidator(); + } + } + + @Test + @DisplayName("Should pass validation when rejectionReason is plain text") + void validatePlainTextRejectionReason() { + CreateEditRequestDTO dto = new CreateEditRequestDTO(); + dto.setId(UUID.randomUUID()); + dto.setSourceRecordingId(UUID.randomUUID()); + dto.setStatus(EditRequestStatus.PENDING); + dto.setRejectionReason("This is a valid rejection reason"); + + Set> violations = validator.validate(dto); + + // Filter to only check rejectionReason violations + long rejectionReasonViolations = violations.stream() + .filter(v -> v.getPropertyPath().toString().equals("rejectionReason")) + .count(); + + assertEquals(0, rejectionReasonViolations, "Plain text should pass validation"); + } + + @Test + @DisplayName("Should fail validation when rejectionReason contains XSS attack") + void validateXssInRejectionReason() { + CreateEditRequestDTO dto = new CreateEditRequestDTO(); + dto.setId(UUID.randomUUID()); + dto.setSourceRecordingId(UUID.randomUUID()); + dto.setStatus(EditRequestStatus.PENDING); + dto.setRejectionReason("Rejected"); + + Set> violations = validator.validate(dto); + + // Check for rejectionReason violations + boolean hasRejectionReasonViolation = violations.stream() + .anyMatch(v -> v.getPropertyPath().toString().equals("rejectionReason") + && v.getMessage().contains("potentially malicious content")); + + assertTrue(hasRejectionReasonViolation, + "XSS content should be rejected with appropriate error message"); + } + + @Test + @DisplayName("Should fail validation when approvedBy contains HTML tags") + void validateHtmlInApprovedBy() { + CreateEditRequestDTO dto = new CreateEditRequestDTO(); + dto.setId(UUID.randomUUID()); + dto.setSourceRecordingId(UUID.randomUUID()); + dto.setStatus(EditRequestStatus.PENDING); + dto.setApprovedBy("Admin User"); + + Set> violations = validator.validate(dto); + + boolean hasApprovedByViolation = violations.stream() + .anyMatch(v -> v.getPropertyPath().toString().equals("approvedBy") + && v.getMessage().contains("potentially malicious content")); + + assertTrue(hasApprovedByViolation, + "HTML tags should be rejected even if not dangerous"); + } + + @Test + @DisplayName("Should fail validation for multiple fields with XSS") + void validateMultipleFieldsWithXss() { + CreateEditRequestDTO dto = new CreateEditRequestDTO(); + dto.setId(UUID.randomUUID()); + dto.setSourceRecordingId(UUID.randomUUID()); + dto.setStatus(EditRequestStatus.PENDING); + dto.setRejectionReason(""); + dto.setApprovedBy(""); + + Set> violations = validator.validate(dto); + + long xssViolations = violations.stream() + .filter(v -> v.getMessage().contains("potentially malicious content")) + .count(); + + assertTrue(xssViolations >= 2, + "Both fields should be rejected for XSS content"); + } + + @Test + @DisplayName("Should pass validation when fields are null") + void validateNullFields() { + CreateEditRequestDTO dto = new CreateEditRequestDTO(); + dto.setId(UUID.randomUUID()); + dto.setSourceRecordingId(UUID.randomUUID()); + dto.setStatus(EditRequestStatus.PENDING); + dto.setRejectionReason(null); + dto.setApprovedBy(null); + + Set> violations = validator.validate(dto); + + boolean hasSanitizationViolation = violations.stream() + .anyMatch(v -> v.getMessage().contains("potentially malicious content")); + + assertFalse(hasSanitizationViolation, + "Null values should not trigger sanitization violations"); + } + + @Test + @DisplayName("Should provide clear error message for XSS attempts") + void validateErrorMessageFormat() { + CreateEditRequestDTO dto = new CreateEditRequestDTO(); + dto.setId(UUID.randomUUID()); + dto.setSourceRecordingId(UUID.randomUUID()); + dto.setStatus(EditRequestStatus.PENDING); + dto.setRejectionReason(""); + + Set> violations = validator.validate(dto); + + ConstraintViolation violation = violations.stream() + .filter(v -> v.getPropertyPath().toString().equals("rejectionReason")) + .findFirst() + .orElse(null); + + if (violation != null) { + assertEquals("rejectionReason contains potentially malicious content", + violation.getMessage(), + "Error message should be clear and specific"); + assertEquals("rejectionReason", + violation.getPropertyPath().toString(), + "Property path should identify the problematic field"); + } + } +} + 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 000000000..c342df98d --- /dev/null +++ b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringValidatorTest.java @@ -0,0 +1,131 @@ +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 false for whitespace - JSoup normalizes it to empty string") + void validateWhitespace() { + when(constraint.allowBasicFormatting()).thenReturn(false); + validator.initialize(constraint); + + // JSoup's text() method normalizes whitespace, so " " becomes "" + assertFalse(validator.isValid(" ", context)); + } +} + + From 0e8d171d436d295b8590fdc795cca9f47a12143a Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Fri, 20 Mar 2026 16:01:36 +0000 Subject: [PATCH 04/28] sanitization annotation --- .../reform/preapi/dto/CreateCourtDTO.java | 4 + .../preapi/dto/CreateEditRequestDTO.java | 3 + .../preapi/dto/CreateParticipantDTO.java | 3 + .../preapi/dto/ExampleSanitizedDTO.java | 47 ++++++ .../reform/preapi/dto/base/BaseUserDTO.java | 5 + .../validators/SanitizedStringConstraint.java | 32 ++++ .../validators/SanitizedStringValidator.java | 33 ++++ .../reform/preapi/utils/InputSanitizer.java | 60 +++---- ...itizedStringConstraintIntegrationTest.java | 158 ++++++++++++++++++ .../SanitizedStringValidatorTest.java | 131 +++++++++++++++ 10 files changed, 448 insertions(+), 28 deletions(-) create mode 100644 src/main/java/uk/gov/hmcts/reform/preapi/dto/ExampleSanitizedDTO.java create mode 100644 src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraint.java create mode 100644 src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringValidator.java create mode 100644 src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraintIntegrationTest.java create mode 100644 src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringValidatorTest.java 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 f757bd4e2..1a0b61d68 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 4172b50ff..4d91dc3bf 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(message = "rejectionReason contains potentially malicious content") @Schema(description = "CreateEditRequestRejectionReason") private String rejectionReason; @@ -48,6 +50,7 @@ public class CreateEditRequestDTO { private Timestamp approvedAt; @Size(max = 100) + @SanitizedStringConstraint(message = "approvedBy contains potentially malicious content") @Schema(description = "CreateEditRequestApprovedBy") private String approvedBy; } 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 47b0571af..e66aab0ef 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/ExampleSanitizedDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/ExampleSanitizedDTO.java new file mode 100644 index 000000000..25e6edaf1 --- /dev/null +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/ExampleSanitizedDTO.java @@ -0,0 +1,47 @@ +package uk.gov.hmcts.reform.preapi.dto; + +import com.fasterxml.jackson.databind.PropertyNamingStrategies; +import com.fasterxml.jackson.databind.annotation.JsonNaming; +import io.swagger.v3.oas.annotations.media.Schema; +import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Size; +import lombok.Data; +import lombok.NoArgsConstructor; +import uk.gov.hmcts.reform.preapi.dto.validators.SanitizedStringConstraint; + +import java.util.UUID; + +/** + * EXAMPLE DTO showing how to use the @SanitizedStringConstraint annotation + * to protect against XSS attacks on user-provided string fields. + * + * This is a sample/reference implementation - not meant to be used directly. + */ +@Data +@NoArgsConstructor +@Schema(description = "ExampleDTO - Shows how to apply input sanitization") +@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class) +@SuppressWarnings("unused") // This is an example/reference implementation +public class ExampleSanitizedDTO { + + @NotNull + @Schema(description = "Example ID") + private UUID id; + + @NotNull + @Size(max = 100) + @SanitizedStringConstraint // Ensures no HTML/scripts can be injected + @Schema(description = "Example Name - Will be validated for XSS attacks") + private String name; + + @Size(max = 500) + @SanitizedStringConstraint(message = "description contains potentially malicious HTML content") + @Schema(description = "Example Description - Strictly sanitized (strict by default)") + private String description; + + @Size(max = 1000) + @SanitizedStringConstraint(allowBasicFormatting = true) // Allow , ,

,
etc. + @Schema(description = "Example Notes - Allows basic formatting") + private String notes; +} + 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 344735330..b5631236f 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/validators/SanitizedStringConstraint.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraint.java new file mode 100644 index 000000000..4de6e6a2a --- /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 000000000..265b8b8dd --- /dev/null +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringValidator.java @@ -0,0 +1,33 @@ +package uk.gov.hmcts.reform.preapi.dto.validators; + +import jakarta.validation.ConstraintValidator; +import jakarta.validation.ConstraintValidatorContext; +import uk.gov.hmcts.reform.preapi.utils.InputSanitizer; + +/** + * 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) { + if (value == null) { + return true; // Use @NotNull for null checks + } + + // Check if sanitization would change the string + // If it changes, it means there was potentially malicious content + String sanitized = InputSanitizer.sanitize(value, allowBasicFormatting); + + return value.equals(sanitized); + } +} + diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java index 0ebf81f28..02e831e43 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java @@ -5,46 +5,50 @@ import org.jsoup.safety.Cleaner; import org.jsoup.safety.Safelist; -import java.lang.reflect.Field; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - +/** + * Utility class for sanitizing user input to prevent XSS and other injection attacks. + * Uses JSoup library to parse and clean HTML content. + */ @UtilityClass public class InputSanitizer { + // 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) { - - Cleaner cleaner = new Cleaner(Safelist.none()); - String text = cleaner.clean(Jsoup.parse(input)).text(); - - return text; + return sanitize(input, false); } - public static Object sanitizeObject(Object dto) throws IllegalAccessException { - if (dto == null) { + /** + * Sanitizes input with optional support for basic text formatting. + * + * @param input The string to sanitize + * @param allowBasicFormatting If true, allows safe HTML tags like , ,

,
+ * @return Sanitized text, or null if input is null + */ + public static String sanitize(String input, boolean allowBasicFormatting) { + if (input == null) { return null; } - Field[] fields = getAllFields(dto.getClass()); - for (Field field : fields) { - if (field.getType() == String.class && !(field.get(dto) == null)) { - field.setAccessible(true); - field.set(dto, sanitize((String) field.get(dto))); - } - } - - return dto; - } + Cleaner cleaner = allowBasicFormatting ? BASIC_CLEANER : STRICT_CLEANER; - public static Field[] getAllFields(Class type) { - List fields = new ArrayList<>(); + // Parse the input as HTML and clean it + String cleaned = cleaner.clean(Jsoup.parse(input)).body().html(); - for (Class c = type; c != null; c = c.getSuperclass()) { - fields.addAll(Arrays.asList(c.getDeclaredFields())); + // If strict mode (no formatting), return just the text content + if (!allowBasicFormatting) { + return Jsoup.parse(cleaned).text(); } - return fields.toArray(new Field[0]); + return cleaned; } - } diff --git a/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraintIntegrationTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraintIntegrationTest.java new file mode 100644 index 000000000..607ea5af8 --- /dev/null +++ b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraintIntegrationTest.java @@ -0,0 +1,158 @@ +package uk.gov.hmcts.reform.preapi.dto.validators; + +import jakarta.validation.ConstraintViolation; +import jakarta.validation.Validation; +import jakarta.validation.Validator; +import jakarta.validation.ValidatorFactory; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import uk.gov.hmcts.reform.preapi.dto.CreateEditRequestDTO; +import uk.gov.hmcts.reform.preapi.enums.EditRequestStatus; + +import java.util.Set; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Integration test showing how @SanitizedStringConstraint works with real DTOs. + * This demonstrates end-to-end validation as it would happen in a controller. + */ +class SanitizedStringConstraintIntegrationTest { + + private Validator validator; + + @BeforeEach + void setUp() { + try (ValidatorFactory factory = Validation.buildDefaultValidatorFactory()) { + validator = factory.getValidator(); + } + } + + @Test + @DisplayName("Should pass validation when rejectionReason is plain text") + void validatePlainTextRejectionReason() { + CreateEditRequestDTO dto = new CreateEditRequestDTO(); + dto.setId(UUID.randomUUID()); + dto.setSourceRecordingId(UUID.randomUUID()); + dto.setStatus(EditRequestStatus.PENDING); + dto.setRejectionReason("This is a valid rejection reason"); + + Set> violations = validator.validate(dto); + + // Filter to only check rejectionReason violations + long rejectionReasonViolations = violations.stream() + .filter(v -> v.getPropertyPath().toString().equals("rejectionReason")) + .count(); + + assertEquals(0, rejectionReasonViolations, "Plain text should pass validation"); + } + + @Test + @DisplayName("Should fail validation when rejectionReason contains XSS attack") + void validateXssInRejectionReason() { + CreateEditRequestDTO dto = new CreateEditRequestDTO(); + dto.setId(UUID.randomUUID()); + dto.setSourceRecordingId(UUID.randomUUID()); + dto.setStatus(EditRequestStatus.PENDING); + dto.setRejectionReason("Rejected"); + + Set> violations = validator.validate(dto); + + // Check for rejectionReason violations + boolean hasRejectionReasonViolation = violations.stream() + .anyMatch(v -> v.getPropertyPath().toString().equals("rejectionReason") + && v.getMessage().contains("potentially malicious content")); + + assertTrue(hasRejectionReasonViolation, + "XSS content should be rejected with appropriate error message"); + } + + @Test + @DisplayName("Should fail validation when approvedBy contains HTML tags") + void validateHtmlInApprovedBy() { + CreateEditRequestDTO dto = new CreateEditRequestDTO(); + dto.setId(UUID.randomUUID()); + dto.setSourceRecordingId(UUID.randomUUID()); + dto.setStatus(EditRequestStatus.PENDING); + dto.setApprovedBy("Admin User"); + + Set> violations = validator.validate(dto); + + boolean hasApprovedByViolation = violations.stream() + .anyMatch(v -> v.getPropertyPath().toString().equals("approvedBy") + && v.getMessage().contains("potentially malicious content")); + + assertTrue(hasApprovedByViolation, + "HTML tags should be rejected even if not dangerous"); + } + + @Test + @DisplayName("Should fail validation for multiple fields with XSS") + void validateMultipleFieldsWithXss() { + CreateEditRequestDTO dto = new CreateEditRequestDTO(); + dto.setId(UUID.randomUUID()); + dto.setSourceRecordingId(UUID.randomUUID()); + dto.setStatus(EditRequestStatus.PENDING); + dto.setRejectionReason(""); + dto.setApprovedBy(""); + + Set> violations = validator.validate(dto); + + long xssViolations = violations.stream() + .filter(v -> v.getMessage().contains("potentially malicious content")) + .count(); + + assertTrue(xssViolations >= 2, + "Both fields should be rejected for XSS content"); + } + + @Test + @DisplayName("Should pass validation when fields are null") + void validateNullFields() { + CreateEditRequestDTO dto = new CreateEditRequestDTO(); + dto.setId(UUID.randomUUID()); + dto.setSourceRecordingId(UUID.randomUUID()); + dto.setStatus(EditRequestStatus.PENDING); + dto.setRejectionReason(null); + dto.setApprovedBy(null); + + Set> violations = validator.validate(dto); + + boolean hasSanitizationViolation = violations.stream() + .anyMatch(v -> v.getMessage().contains("potentially malicious content")); + + assertFalse(hasSanitizationViolation, + "Null values should not trigger sanitization violations"); + } + + @Test + @DisplayName("Should provide clear error message for XSS attempts") + void validateErrorMessageFormat() { + CreateEditRequestDTO dto = new CreateEditRequestDTO(); + dto.setId(UUID.randomUUID()); + dto.setSourceRecordingId(UUID.randomUUID()); + dto.setStatus(EditRequestStatus.PENDING); + dto.setRejectionReason(""); + + Set> violations = validator.validate(dto); + + ConstraintViolation violation = violations.stream() + .filter(v -> v.getPropertyPath().toString().equals("rejectionReason")) + .findFirst() + .orElse(null); + + if (violation != null) { + assertEquals("rejectionReason contains potentially malicious content", + violation.getMessage(), + "Error message should be clear and specific"); + assertEquals("rejectionReason", + violation.getPropertyPath().toString(), + "Property path should identify the problematic field"); + } + } +} + 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 000000000..c342df98d --- /dev/null +++ b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringValidatorTest.java @@ -0,0 +1,131 @@ +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 false for whitespace - JSoup normalizes it to empty string") + void validateWhitespace() { + when(constraint.allowBasicFormatting()).thenReturn(false); + validator.initialize(constraint); + + // JSoup's text() method normalizes whitespace, so " " becomes "" + assertFalse(validator.isValid(" ", context)); + } +} + + From 9d744dd3c5f8f0552774f8bcbb4e9e6ded25e2e9 Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Fri, 20 Mar 2026 16:05:14 +0000 Subject: [PATCH 05/28] delete filter --- .../config/InputSanitizationFilter.java | 96 ------------------- 1 file changed, 96 deletions(-) delete mode 100644 src/main/java/uk/gov/hmcts/reform/preapi/config/InputSanitizationFilter.java diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/config/InputSanitizationFilter.java b/src/main/java/uk/gov/hmcts/reform/preapi/config/InputSanitizationFilter.java deleted file mode 100644 index 4eec95d01..000000000 --- a/src/main/java/uk/gov/hmcts/reform/preapi/config/InputSanitizationFilter.java +++ /dev/null @@ -1,96 +0,0 @@ -package uk.gov.hmcts.reform.preapi.config; - -import jakarta.servlet.FilterChain; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletRequestWrapper; -import jakarta.servlet.http.HttpServletResponse; -import org.springframework.web.filter.OncePerRequestFilter; -import uk.gov.hmcts.reform.preapi.utils.InputSanitizer; - -import java.io.IOException; -import java.util.Arrays; -import java.util.Collections; -import java.util.Enumeration; -import java.util.HashMap; -import java.util.Map; - -/** - * ALTERNATIVE APPROACH: Filter that sanitizes all request parameters and headers. - * This is commented out by default - use @SanitizedStringConstraint on DTOs instead. - * - * To enable this filter, uncomment the @Component annotation. - * Note: This approach sanitizes ALL string inputs, which may be too aggressive. - */ -// @Component -// @Order(Ordered.HIGHEST_PRECEDENCE) -@SuppressWarnings("unused") // This is an optional alternative approach -public class InputSanitizationFilter extends OncePerRequestFilter { - - @Override - @SuppressWarnings("NullableProblems") // Spring framework guarantees non-null parameters - protected void doFilterInternal(HttpServletRequest request, - HttpServletResponse response, - FilterChain filterChain) - throws ServletException, IOException { - - // Wrap the request to sanitize parameters - SanitizedHttpServletRequest sanitizedRequest = new SanitizedHttpServletRequest(request); - - filterChain.doFilter(sanitizedRequest, response); - } - - /** - * Request wrapper that sanitizes parameter values - */ - private static class SanitizedHttpServletRequest extends HttpServletRequestWrapper { - private final Map sanitizedParameterMap; - - public SanitizedHttpServletRequest(HttpServletRequest request) { - super(request); - this.sanitizedParameterMap = sanitizeParameters(request.getParameterMap()); - } - - @Override - public String getParameter(String name) { - String[] values = sanitizedParameterMap.get(name); - return (values != null && values.length > 0) ? values[0] : null; - } - - @Override - public Map getParameterMap() { - return Collections.unmodifiableMap(sanitizedParameterMap); - } - - @Override - public Enumeration getParameterNames() { - return Collections.enumeration(sanitizedParameterMap.keySet()); - } - - @Override - public String[] getParameterValues(String name) { - return sanitizedParameterMap.get(name); - } - - private Map sanitizeParameters(Map originalMap) { - Map sanitizedMap = new HashMap<>(); - - for (Map.Entry entry : originalMap.entrySet()) { - String key = entry.getKey(); - String[] values = entry.getValue(); - - if (values != null) { - String[] sanitizedValues = Arrays.stream(values) - .map(InputSanitizer::sanitize) - .toArray(String[]::new); - sanitizedMap.put(key, sanitizedValues); - } else { - sanitizedMap.put(key, null); - } - } - - return sanitizedMap; - } - } -} - From c25fa62127ff1a50d1dcefd502faab8b33c7c8ea Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Mon, 23 Mar 2026 09:57:10 +0000 Subject: [PATCH 06/28] checkstyle fixes --- .../preapi/dto/ExampleSanitizedDTO.java | 1 - .../validators/SanitizedStringConstraint.java | 2 +- .../validators/SanitizedStringValidator.java | 4 +- ...anitizer.java => InputSanitizerUtils.java} | 5 +- .../preapi/util/InputSanitizerTest.java | 46 ------------------- .../preapi/util/InputSanitizerUtilsTest.java | 34 ++++++++++++++ 6 files changed, 40 insertions(+), 52 deletions(-) rename src/main/java/uk/gov/hmcts/reform/preapi/utils/{InputSanitizer.java => InputSanitizerUtils.java} (90%) delete mode 100644 src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerTest.java create mode 100644 src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerUtilsTest.java diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/ExampleSanitizedDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/ExampleSanitizedDTO.java index 25e6edaf1..7c30f4d74 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/ExampleSanitizedDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/ExampleSanitizedDTO.java @@ -14,7 +14,6 @@ /** * EXAMPLE DTO showing how to use the @SanitizedStringConstraint annotation * to protect against XSS attacks on user-provided string fields. - * * This is a sample/reference implementation - not meant to be used directly. */ @Data 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 index 4de6e6a2a..f17a04e89 100644 --- 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 @@ -24,7 +24,7 @@ Class[] payload() default {}; /** - * Whether to allow basic text formatting (bold, italic, etc.) + * 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 index 265b8b8dd..056b26b2d 100644 --- 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 @@ -2,7 +2,7 @@ import jakarta.validation.ConstraintValidator; import jakarta.validation.ConstraintValidatorContext; -import uk.gov.hmcts.reform.preapi.utils.InputSanitizer; +import uk.gov.hmcts.reform.preapi.utils.InputSanitizerUtils; /** * Validator that checks if a string contains potentially malicious content. @@ -25,7 +25,7 @@ public boolean isValid(String value, ConstraintValidatorContext context) { // Check if sanitization would change the string // If it changes, it means there was potentially malicious content - String sanitized = InputSanitizer.sanitize(value, allowBasicFormatting); + String sanitized = InputSanitizerUtils.sanitize(value, allowBasicFormatting); return value.equals(sanitized); } diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java similarity index 90% rename from src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java rename to src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java index 02e831e43..6fa01ce1d 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizer.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java @@ -10,7 +10,8 @@ * Uses JSoup library to parse and clean HTML content. */ @UtilityClass -public class InputSanitizer { +@SuppressWarnings({"checkstyle:HideUtilityClassConstructor", "checkstyle:JavadocParagraph"}) +public class InputSanitizerUtils { // Reuse Cleaner instances for better performance private static final Cleaner STRICT_CLEANER = new Cleaner(Safelist.none()); @@ -31,7 +32,7 @@ public static String sanitize(String input) { * Sanitizes input with optional support for basic text formatting. * * @param input The string to sanitize - * @param allowBasicFormatting If true, allows safe HTML tags like , ,

,
+ * @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 */ public static String sanitize(String input, boolean allowBasicFormatting) { diff --git a/src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerTest.java deleted file mode 100644 index 043ce2e02..000000000 --- a/src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerTest.java +++ /dev/null @@ -1,46 +0,0 @@ -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.dto.UserDTO; -import uk.gov.hmcts.reform.preapi.utils.InputSanitizer; - -import static org.assertj.core.api.AssertionsForClassTypes.assertThat; - -public class InputSanitizerTest { - - @ParameterizedTest - @ValueSource(strings = {"TEST", "TEST", "TEST", "TEST", - "TEST"}) - public void shouldSanitizeHtml(String input) { - String expectedOutput = "TEST"; - String actualOutput = InputSanitizer.sanitize(input); - - assertThat(actualOutput).isEqualTo(expectedOutput); - } - - @Test - public void shouldSanitizeScriptTags() { - String input = """ - "> - """; - String expectedOutput = "\">"; - - String actualOutput = InputSanitizer.sanitize(input); - - assertThat(actualOutput).isEqualTo(expectedOutput); - } - - @Test - public void shouldSanitizeObject() throws IllegalAccessException { - UserDTO userDTO = new UserDTO(); - userDTO.setFirstName("Test"); - userDTO.setLastName("Test"); - - UserDTO sanitizedUserDTO = (UserDTO) InputSanitizer.sanitizeObject(userDTO); - - assertThat(sanitizedUserDTO.getFirstName()).isEqualTo("Test"); - assertThat(sanitizedUserDTO.getLastName()).isEqualTo("Test"); - } -} 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 000000000..2e8984bab --- /dev/null +++ b/src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerUtilsTest.java @@ -0,0 +1,34 @@ +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 = + """ + "> + """; + String expectedOutput = "\">"; + + String actualOutput = InputSanitizerUtils.sanitize(input); + + assertThat(actualOutput).isEqualTo(expectedOutput); + } +} From 075045a028c4b11af4701b294ac48a720a25f43a Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Mon, 23 Mar 2026 09:57:38 +0000 Subject: [PATCH 07/28] wip CreateAuditDTO --- .../java/uk/gov/hmcts/reform/preapi/dto/CreateAuditDTO.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 e1636a32a..3ee4d2649 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; @@ -47,7 +48,8 @@ public class CreateAuditDTO { @Schema(description = "AuditDetailsJSONString") @JsonRawValue - private JsonNode auditDetails; + @SanitizedStringConstraint + private JsonNode auditDetails; //TODO: How to validate this as annotation cannot work for it public CreateAuditDTO(Audit auditEntity) { this.id = auditEntity.getId(); From 1c3e3640b8f0dd01f0a7462ecfc67b306f5de800 Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Mon, 23 Mar 2026 12:05:01 +0000 Subject: [PATCH 08/28] adds validation to more DTO fields and controller param --- .../hmcts/reform/preapi/controllers/InviteController.java | 5 ++++- .../uk/gov/hmcts/reform/preapi/dto/CaptureSessionDTO.java | 2 +- .../hmcts/reform/preapi/dto/CreateCaptureSessionDTO.java | 4 ++-- .../uk/gov/hmcts/reform/preapi/dto/CreateInviteDTO.java | 6 +++++- .../gov/hmcts/reform/preapi/dto/VerifyEmailRequestDTO.java | 2 ++ 5 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/controllers/InviteController.java b/src/main/java/uk/gov/hmcts/reform/preapi/controllers/InviteController.java index f574141b7..a41a260f5 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/controllers/InviteController.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/controllers/InviteController.java @@ -4,6 +4,7 @@ import io.swagger.v3.oas.annotations.Parameter; import io.swagger.v3.oas.annotations.media.Schema; import jakarta.validation.Valid; +import jakarta.validation.constraints.Email; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; @@ -13,6 +14,7 @@ import org.springframework.http.HttpEntity; import org.springframework.http.ResponseEntity; import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.validation.annotation.Validated; import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.ModelAttribute; @@ -138,6 +140,7 @@ public ResponseEntity deleteInvite(@PathVariable UUID userId) { } @PostMapping("/redeem") + @Validated @Operation(operationId = "redeemInvite", summary = "Redeem an invite") @Parameter( name = "email", @@ -145,7 +148,7 @@ public ResponseEntity deleteInvite(@PathVariable UUID userId) { example = "example@example.com", schema = @Schema(implementation = String.class) ) - public ResponseEntity redeemInvite(@RequestParam String email) { + public ResponseEntity redeemInvite(@RequestParam @Email String email) { return getUpsertResponse(inviteService.redeemInvite(email), null); } } diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/CaptureSessionDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/CaptureSessionDTO.java index bfb37dc03..37a14856d 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/CaptureSessionDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/CaptureSessionDTO.java @@ -22,7 +22,7 @@ public class CaptureSessionDTO extends CreateCaptureSessionDTO { private Timestamp deletedAt; @Schema(description = "RecordingParticipants") // todo change this (might be breaking) - private String courtName; + private String courtName; //TODO: what @Schema(description = "CaptureSessionCaseState") private CaseState caseState; 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 311f7f148..c98c650ce 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 @@ -31,10 +31,10 @@ public class CreateCaptureSessionDTO { private RecordingOrigin origin; @Schema(description = "CreateCaptureSessionIngestAddress") - private String ingestAddress; + private String ingestAddress; //TODO: will sanitise constraint affect? @Schema(description = "CreateCaptureSessionLiveOutputURL") - private String liveOutputUrl; + private String liveOutputUrl; //TODO: will sanitise constraint affect? @Schema(description = "CreateCaptureSessionStartedAt") private Timestamp startedAt; 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 d30ea6975..24c5b53d1 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,10 @@ public class CreateInviteDTO { protected String email; @Schema(description = "InviteOrganisation") + @SanitizedStringConstraint protected String organisation; @Schema(description = "InvitePhone") - protected String phone; + protected String phone; //TODO: add validation for phone number? Safe to add Sanitised here } diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/VerifyEmailRequestDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/VerifyEmailRequestDTO.java index db1f5b81c..7eede0b09 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/VerifyEmailRequestDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/VerifyEmailRequestDTO.java @@ -8,6 +8,7 @@ import jakarta.validation.constraints.Pattern; import lombok.Data; import lombok.NoArgsConstructor; +import uk.gov.hmcts.reform.preapi.dto.validators.SanitizedStringConstraint; @Data @NoArgsConstructor @@ -23,5 +24,6 @@ public class VerifyEmailRequestDTO { @Schema(description = "VerificationCode") @NotBlank @Pattern(regexp = "^[0-9]{6}$", message = "invalid verification code") + @SanitizedStringConstraint private String verificationCode; } From 23b379efeadb823e6bab44efc2995786b288e166 Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Mon, 23 Mar 2026 16:47:10 +0000 Subject: [PATCH 09/28] adds sanitized annotation --- .../uk/gov/hmcts/reform/preapi/dto/CreateAuditDTO.java | 3 +-- .../hmcts/reform/preapi/dto/CreateCaptureSessionDTO.java | 7 +++++-- .../gov/hmcts/reform/preapi/dto/CreateEditRequestDTO.java | 4 ++-- .../uk/gov/hmcts/reform/preapi/dto/CreateInviteDTO.java | 1 + .../gov/hmcts/reform/preapi/dto/base/BaseRecordingDTO.java | 3 +++ .../preapi/dto/migration/CreateVfMigrationRecordDTO.java | 3 +++ 6 files changed, 15 insertions(+), 6 deletions(-) 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 3ee4d2649..64296c29a 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 @@ -48,8 +48,7 @@ public class CreateAuditDTO { @Schema(description = "AuditDetailsJSONString") @JsonRawValue - @SanitizedStringConstraint - private JsonNode auditDetails; //TODO: How to validate this as annotation cannot work for it + private JsonNode auditDetails; //TODO: Needs own validator checking contents is valid JSON and not malicious content 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 c98c650ce..5230d46b4 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,10 +32,12 @@ public class CreateCaptureSessionDTO { private RecordingOrigin origin; @Schema(description = "CreateCaptureSessionIngestAddress") - private String ingestAddress; //TODO: will sanitise constraint affect? + @SanitizedStringConstraint + private String ingestAddress; @Schema(description = "CreateCaptureSessionLiveOutputURL") - private String liveOutputUrl; //TODO: will sanitise constraint affect? + @SanitizedStringConstraint + private String liveOutputUrl; @Schema(description = "CreateCaptureSessionStartedAt") private Timestamp startedAt; 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 4d91dc3bf..3f0e9f268 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 @@ -42,7 +42,7 @@ public class CreateEditRequestDTO { private Boolean jointlyAgreed; @Size(max = 512) - @SanitizedStringConstraint(message = "rejectionReason contains potentially malicious content") + @SanitizedStringConstraint @Schema(description = "CreateEditRequestRejectionReason") private String rejectionReason; @@ -50,7 +50,7 @@ public class CreateEditRequestDTO { private Timestamp approvedAt; @Size(max = 100) - @SanitizedStringConstraint(message = "approvedBy contains potentially malicious content") + @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 24c5b53d1..3fc88f3bb 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 @@ -41,6 +41,7 @@ public class CreateInviteDTO { protected String organisation; @Schema(description = "InvitePhone") + @SanitizedStringConstraint protected String phone; //TODO: add validation for phone number? Safe to add Sanitised here } 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 d77b16c0f..fb881a422 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/migration/CreateVfMigrationRecordDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/migration/CreateVfMigrationRecordDTO.java index 9687bd043..5d01e7173 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") From 001978c55cd70a8115c5867835feba8d8b19dacd Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Mon, 23 Mar 2026 16:47:36 +0000 Subject: [PATCH 10/28] cleanup --- .../reform/preapi/dto/CaptureSessionDTO.java | 2 +- .../validators/SanitizedStringValidator.java | 4 +- .../preapi/utils/InputSanitizerUtils.java | 12 +- ...itizedStringConstraintIntegrationTest.java | 158 ------------------ .../preapi/util/InputSanitizerUtilsTest.java | 2 + 5 files changed, 11 insertions(+), 167 deletions(-) delete mode 100644 src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraintIntegrationTest.java diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/CaptureSessionDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/CaptureSessionDTO.java index 37a14856d..bfb37dc03 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/CaptureSessionDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/CaptureSessionDTO.java @@ -22,7 +22,7 @@ public class CaptureSessionDTO extends CreateCaptureSessionDTO { private Timestamp deletedAt; @Schema(description = "RecordingParticipants") // todo change this (might be breaking) - private String courtName; //TODO: what + private String courtName; @Schema(description = "CaptureSessionCaseState") private CaseState caseState; 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 index 056b26b2d..27b5df7f9 100644 --- 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 @@ -2,6 +2,7 @@ import jakarta.validation.ConstraintValidator; import jakarta.validation.ConstraintValidatorContext; +import lombok.extern.slf4j.Slf4j; import uk.gov.hmcts.reform.preapi.utils.InputSanitizerUtils; /** @@ -20,13 +21,12 @@ public void initialize(SanitizedStringConstraint constraint) { @Override public boolean isValid(String value, ConstraintValidatorContext context) { if (value == null) { - return true; // Use @NotNull for null checks + return true; } // Check if sanitization would change the string // If it changes, it means there was potentially malicious content String sanitized = InputSanitizerUtils.sanitize(value, allowBasicFormatting); - return value.equals(sanitized); } } 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 index 6fa01ce1d..363c6d2bb 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java @@ -2,6 +2,7 @@ import lombok.experimental.UtilityClass; import org.jsoup.Jsoup; +import org.jsoup.nodes.Document; import org.jsoup.safety.Cleaner; import org.jsoup.safety.Safelist; @@ -30,7 +31,8 @@ public static String sanitize(String input) { /** * 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 @@ -42,14 +44,12 @@ public static String sanitize(String input, boolean allowBasicFormatting) { Cleaner cleaner = allowBasicFormatting ? BASIC_CLEANER : STRICT_CLEANER; - // Parse the input as HTML and clean it - String cleaned = cleaner.clean(Jsoup.parse(input)).body().html(); - // If strict mode (no formatting), return just the text content if (!allowBasicFormatting) { - return Jsoup.parse(cleaned).text(); + return Jsoup.clean(input, "", Safelist.none(), new Document.OutputSettings().prettyPrint(false)); } - return cleaned; + // Parse the input as HTML and clean it + return cleaner.clean(Jsoup.parse(input)).body().html(); } } diff --git a/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraintIntegrationTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraintIntegrationTest.java deleted file mode 100644 index 607ea5af8..000000000 --- a/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringConstraintIntegrationTest.java +++ /dev/null @@ -1,158 +0,0 @@ -package uk.gov.hmcts.reform.preapi.dto.validators; - -import jakarta.validation.ConstraintViolation; -import jakarta.validation.Validation; -import jakarta.validation.Validator; -import jakarta.validation.ValidatorFactory; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Test; -import uk.gov.hmcts.reform.preapi.dto.CreateEditRequestDTO; -import uk.gov.hmcts.reform.preapi.enums.EditRequestStatus; - -import java.util.Set; -import java.util.UUID; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; - -/** - * Integration test showing how @SanitizedStringConstraint works with real DTOs. - * This demonstrates end-to-end validation as it would happen in a controller. - */ -class SanitizedStringConstraintIntegrationTest { - - private Validator validator; - - @BeforeEach - void setUp() { - try (ValidatorFactory factory = Validation.buildDefaultValidatorFactory()) { - validator = factory.getValidator(); - } - } - - @Test - @DisplayName("Should pass validation when rejectionReason is plain text") - void validatePlainTextRejectionReason() { - CreateEditRequestDTO dto = new CreateEditRequestDTO(); - dto.setId(UUID.randomUUID()); - dto.setSourceRecordingId(UUID.randomUUID()); - dto.setStatus(EditRequestStatus.PENDING); - dto.setRejectionReason("This is a valid rejection reason"); - - Set> violations = validator.validate(dto); - - // Filter to only check rejectionReason violations - long rejectionReasonViolations = violations.stream() - .filter(v -> v.getPropertyPath().toString().equals("rejectionReason")) - .count(); - - assertEquals(0, rejectionReasonViolations, "Plain text should pass validation"); - } - - @Test - @DisplayName("Should fail validation when rejectionReason contains XSS attack") - void validateXssInRejectionReason() { - CreateEditRequestDTO dto = new CreateEditRequestDTO(); - dto.setId(UUID.randomUUID()); - dto.setSourceRecordingId(UUID.randomUUID()); - dto.setStatus(EditRequestStatus.PENDING); - dto.setRejectionReason("Rejected"); - - Set> violations = validator.validate(dto); - - // Check for rejectionReason violations - boolean hasRejectionReasonViolation = violations.stream() - .anyMatch(v -> v.getPropertyPath().toString().equals("rejectionReason") - && v.getMessage().contains("potentially malicious content")); - - assertTrue(hasRejectionReasonViolation, - "XSS content should be rejected with appropriate error message"); - } - - @Test - @DisplayName("Should fail validation when approvedBy contains HTML tags") - void validateHtmlInApprovedBy() { - CreateEditRequestDTO dto = new CreateEditRequestDTO(); - dto.setId(UUID.randomUUID()); - dto.setSourceRecordingId(UUID.randomUUID()); - dto.setStatus(EditRequestStatus.PENDING); - dto.setApprovedBy("Admin User"); - - Set> violations = validator.validate(dto); - - boolean hasApprovedByViolation = violations.stream() - .anyMatch(v -> v.getPropertyPath().toString().equals("approvedBy") - && v.getMessage().contains("potentially malicious content")); - - assertTrue(hasApprovedByViolation, - "HTML tags should be rejected even if not dangerous"); - } - - @Test - @DisplayName("Should fail validation for multiple fields with XSS") - void validateMultipleFieldsWithXss() { - CreateEditRequestDTO dto = new CreateEditRequestDTO(); - dto.setId(UUID.randomUUID()); - dto.setSourceRecordingId(UUID.randomUUID()); - dto.setStatus(EditRequestStatus.PENDING); - dto.setRejectionReason(""); - dto.setApprovedBy(""); - - Set> violations = validator.validate(dto); - - long xssViolations = violations.stream() - .filter(v -> v.getMessage().contains("potentially malicious content")) - .count(); - - assertTrue(xssViolations >= 2, - "Both fields should be rejected for XSS content"); - } - - @Test - @DisplayName("Should pass validation when fields are null") - void validateNullFields() { - CreateEditRequestDTO dto = new CreateEditRequestDTO(); - dto.setId(UUID.randomUUID()); - dto.setSourceRecordingId(UUID.randomUUID()); - dto.setStatus(EditRequestStatus.PENDING); - dto.setRejectionReason(null); - dto.setApprovedBy(null); - - Set> violations = validator.validate(dto); - - boolean hasSanitizationViolation = violations.stream() - .anyMatch(v -> v.getMessage().contains("potentially malicious content")); - - assertFalse(hasSanitizationViolation, - "Null values should not trigger sanitization violations"); - } - - @Test - @DisplayName("Should provide clear error message for XSS attempts") - void validateErrorMessageFormat() { - CreateEditRequestDTO dto = new CreateEditRequestDTO(); - dto.setId(UUID.randomUUID()); - dto.setSourceRecordingId(UUID.randomUUID()); - dto.setStatus(EditRequestStatus.PENDING); - dto.setRejectionReason(""); - - Set> violations = validator.validate(dto); - - ConstraintViolation violation = violations.stream() - .filter(v -> v.getPropertyPath().toString().equals("rejectionReason")) - .findFirst() - .orElse(null); - - if (violation != null) { - assertEquals("rejectionReason contains potentially malicious content", - violation.getMessage(), - "Error message should be clear and specific"); - assertEquals("rejectionReason", - violation.getPropertyPath().toString(), - "Property path should identify the problematic field"); - } - } -} - 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 index 2e8984bab..202945056 100644 --- a/src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerUtilsTest.java +++ b/src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerUtilsTest.java @@ -9,6 +9,8 @@ public class InputSanitizerUtilsTest { + //TODO: Add more unit tests if needed + @ParameterizedTest @ValueSource(strings = {"TEST", "TEST", "TEST", "TEST", "TEST"}) From 73c700cdaa4d5d59f50d78abf8fba7117c7b7fb3 Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Mon, 23 Mar 2026 16:47:48 +0000 Subject: [PATCH 11/28] add tests --- .../preapi/controllers/CourtControllerFT.java | 18 ++++++++++++++++++ .../SanitizedStringValidatorTest.java | 14 ++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) 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 602204b05..ce47c14c2 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,6 +6,7 @@ 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.exception.NotFoundException; import uk.gov.hmcts.reform.preapi.util.FunctionalTestBase; @@ -73,4 +74,21 @@ void updateCourtEmailAddressesFromCsv() throws JsonProcessingException { assertThat(updatedCourt.getGroupEmail()).isEqualTo("PRE.Edits.Example@justice.gov.uk"); } + @Test + @DisplayName("Should not save court that has unsanitised 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/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringValidatorTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringValidatorTest.java index c342df98d..689a47bcc 100644 --- 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 @@ -118,13 +118,23 @@ void validateEmptyString() { } @Test - @DisplayName("Should return false for whitespace - JSoup normalizes it to empty string") + @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 "" - assertFalse(validator.isValid(" ", context)); + 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)); } } From 6a4a0eb0c9335e828efe4efd307308c5e11c9d4a Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Mon, 23 Mar 2026 17:02:31 +0000 Subject: [PATCH 12/28] add tests --- .../preapi/controllers/AuditControllerFT.java | 18 ++++++++++++++ .../CaptureSessionControllerFT.java | 24 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java index 996ca06e0..6feb717a1 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java @@ -32,6 +32,24 @@ void updateAuditFailure() throws JsonProcessingException { .isEqualTo("Data is immutable and cannot be changed. Id: " + audit.getId()); } + @DisplayName("Should not put an audit with un-sanitised data") + @Test + void updateAuditFailureWithUnsafeData() throws JsonProcessingException { + CreateAuditDTO audit = new CreateAuditDTO(); + audit.setId(UUID.randomUUID()); + audit.setAuditDetails(OBJECT_MAPPER.readTree("{\"test\": \"test\"}")); + audit.setSource(AuditLogSource.AUTO); + audit.setActivity(""); + audit.setFunctionalArea(""); + + var error = putAudit(audit); + assertResponseCode(error, 400); + assertThat(error.body().jsonPath().getString("activity")) + .contains("potentially malicious content"); + assertThat(error.body().jsonPath().getString("functionalArea")) + .contains("potentially malicious content"); + } + private Response putAudit(CreateAuditDTO dto) throws JsonProcessingException { return doPutRequest( AUDIT_ENDPOINT + dto.getId(), 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 16402ed46..ee0cfd61e 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); + } } From 72f79f1ba5847402218e2c8843087e5658177c8c Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Mon, 23 Mar 2026 17:02:54 +0000 Subject: [PATCH 13/28] adds/removes sanitized annotation --- .../java/uk/gov/hmcts/reform/preapi/dto/CreateAuditDTO.java | 2 ++ .../uk/gov/hmcts/reform/preapi/dto/VerifyEmailRequestDTO.java | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) 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 64296c29a..702406a5e 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 @@ -41,9 +41,11 @@ 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") diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/VerifyEmailRequestDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/VerifyEmailRequestDTO.java index 7eede0b09..a667a73de 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/VerifyEmailRequestDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/VerifyEmailRequestDTO.java @@ -24,6 +24,5 @@ public class VerifyEmailRequestDTO { @Schema(description = "VerificationCode") @NotBlank @Pattern(regexp = "^[0-9]{6}$", message = "invalid verification code") - @SanitizedStringConstraint private String verificationCode; } From a5ed242a351da59628507cc40b210a2f2e4a4d59 Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Tue, 24 Mar 2026 12:17:48 +0000 Subject: [PATCH 14/28] wip functional tests --- .../preapi/controllers/CourtControllerFT.java | 2 +- .../preapi/controllers/EditControllerFT.java | 63 +++++++++++++++++++ .../preapi/dto/EditCutInstructionDTO.java | 2 + .../validators/SanitizedStringValidator.java | 10 +-- .../preapi/services/EditRequestService.java | 6 ++ .../preapi/utils/InputSanitizerUtils.java | 12 ++++ 6 files changed, 85 insertions(+), 10 deletions(-) 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 ce47c14c2..a343bc78c 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 @@ -75,7 +75,7 @@ void updateCourtEmailAddressesFromCsv() throws JsonProcessingException { } @Test - @DisplayName("Should not save court that has unsanitised fields") + @DisplayName("Should not save court that has unsafe data in fields") void createCourtWithUnsanitizedFields() throws JsonProcessingException { CreateCourtDTO dto = createCourt(); dto.setName("Rejected"); 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 a4c5ca6a4..78ef24125 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,8 @@ package uk.gov.hmcts.reform.preapi.controllers; import com.fasterxml.jackson.core.JsonProcessingException; +import io.restassured.response.Response; +import lombok.extern.slf4j.Slf4j; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -8,6 +10,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 +19,17 @@ 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; +@Slf4j 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 +103,60 @@ 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); + } + + @Test + @DisplayName("Should not create an edit request with unsafe data in fields") + void editRequestWithUnsafeData() throws JsonProcessingException { + UUID editRequestId = UUID.randomUUID(); + CreateRecordingResponse recordingDetails = createRecording(); + RecordingDTO recordingDTO = assertRecordingExists(recordingDetails.recordingId(), true).as(RecordingDTO.class); + + 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); + + 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().) +// .contains("potentially malicious content"); + } } 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 cfd52e9a2..d430ee03b 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/validators/SanitizedStringValidator.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedStringValidator.java index 27b5df7f9..ed8c5340e 100644 --- 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 @@ -18,16 +18,8 @@ public void initialize(SanitizedStringConstraint constraint) { this.allowBasicFormatting = constraint.allowBasicFormatting(); } - @Override public boolean isValid(String value, ConstraintValidatorContext context) { - if (value == null) { - return true; - } - - // Check if sanitization would change the string - // If it changes, it means there was potentially malicious content - String sanitized = InputSanitizerUtils.sanitize(value, allowBasicFormatting); - return value.equals(sanitized); + return InputSanitizerUtils.isValid(value, allowBasicFormatting); } } 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 107793bf4..de52f4938 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,11 @@ 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 index 363c6d2bb..e9bb5160f 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java @@ -1,5 +1,6 @@ package uk.gov.hmcts.reform.preapi.utils; +import jakarta.validation.ConstraintValidatorContext; import lombok.experimental.UtilityClass; import org.jsoup.Jsoup; import org.jsoup.nodes.Document; @@ -52,4 +53,15 @@ public static String sanitize(String input, boolean allowBasicFormatting) { // 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 = InputSanitizerUtils.sanitize(value, allowBasicFormatting); + return value.equals(sanitized); + } } From b12d14a888c6854ae36c2e791f645927dc3986ac Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Tue, 24 Mar 2026 14:11:25 +0000 Subject: [PATCH 15/28] csv for validation test --- .../resources/test/edit/edit_from_csv_unsafe.csv | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 src/functionalTest/resources/test/edit/edit_from_csv_unsafe.csv 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 000000000..52517493a --- /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, From 33644b5d2d49e7e8b44ead48c5cb47baaeda10ec Mon Sep 17 00:00:00 2001 From: Alex Sealey Date: Tue, 24 Mar 2026 14:23:33 +0000 Subject: [PATCH 16/28] JSON validator --- .../reform/preapi/dto/CreateAuditDTO.java | 4 +- .../SanitizedJsonNodeConstraint.java | 25 +++++++++ .../SanitizedJsonNodeValidator.java | 56 +++++++++++++++++++ 3 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeConstraint.java create mode 100644 src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidator.java 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 702406a5e..144edad9d 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.SanitizedJsonNodeConstraint; 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; @@ -50,7 +51,8 @@ public class CreateAuditDTO { @Schema(description = "AuditDetailsJSONString") @JsonRawValue - private JsonNode auditDetails; //TODO: Needs own validator checking contents is valid JSON and not malicious content + @SanitizedJsonNodeConstraint + private JsonNode auditDetails; public CreateAuditDTO(Audit auditEntity) { this.id = auditEntity.getId(); diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeConstraint.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeConstraint.java new file mode 100644 index 000000000..24b2e1c09 --- /dev/null +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeConstraint.java @@ -0,0 +1,25 @@ +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 JsonNode keys and string values to prevent malicious content. + */ +@Documented +@Constraint(validatedBy = uk.gov.hmcts.reform.preapi.dto.validators.SanitizedJsonNodeValidator.class) +@Target({ ElementType.FIELD, ElementType.PARAMETER }) +@Retention(RetentionPolicy.RUNTIME) +public @interface SanitizedJsonNodeConstraint { + String message() default "contains potentially malicious content"; + Class[] groups() default {}; + Class[] payload() default {}; +} + + diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidator.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidator.java new file mode 100644 index 000000000..59491664a --- /dev/null +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidator.java @@ -0,0 +1,56 @@ +package uk.gov.hmcts.reform.preapi.dto.validators; + +import com.fasterxml.jackson.databind.JsonNode; +import jakarta.validation.ConstraintValidator; +import jakarta.validation.ConstraintValidatorContext; +import uk.gov.hmcts.reform.preapi.utils.InputSanitizerUtils; + +import java.util.Map; + +/** + * Validator that checks JsonNode keys and string values for potentially malicious content. + */ +public class SanitizedJsonNodeValidator implements ConstraintValidator { + + @Override + public boolean isValid(JsonNode value, ConstraintValidatorContext context) { + if (value == null || value.isNull()) { + return true; + } + + return isNodeSanitized(value); + } + + private boolean isNodeSanitized(JsonNode node) { + if (node.isObject()) { + for (Map.Entry field : node.properties()) { + if (!isSanitized(field.getKey()) || !isNodeSanitized(field.getValue())) { + return false; + } + } + return true; + } + + if (node.isArray()) { + for (JsonNode element : node) { + if (!isNodeSanitized(element)) { + return false; + } + } + return true; + } + + if (node.isTextual()) { + return isSanitized(node.textValue()); + } + + return true; + } + + private boolean isSanitized(String value) { + return value.equals(InputSanitizerUtils.sanitize(value)); + } +} + + + From a3e56400899f8b15e79e06467f06dd5a1aa4440a Mon Sep 17 00:00:00 2001 From: Alex Sealey Date: Tue, 24 Mar 2026 14:23:51 +0000 Subject: [PATCH 17/28] JSON validator tests --- .../controller/AuditControllerTest.java | 54 +++++++++++++ .../SanitizedJsonNodeValidatorTest.java | 80 +++++++++++++++++++ 2 files changed, 134 insertions(+) create mode 100644 src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java diff --git a/src/test/java/uk/gov/hmcts/reform/preapi/controller/AuditControllerTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/controller/AuditControllerTest.java index 0fecbdfa9..d797e3064 100644 --- a/src/test/java/uk/gov/hmcts/reform/preapi/controller/AuditControllerTest.java +++ b/src/test/java/uk/gov/hmcts/reform/preapi/controller/AuditControllerTest.java @@ -25,6 +25,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; @@ -158,6 +160,58 @@ void createAuditEndpointNotAcceptable() throws Exception { .andExpect(status().is4xxClientError()); } + @DisplayName("Should fail validation when audit details contains malicious content") + @Test + void createAuditEndpointMaliciousAuditDetailsBadRequest() throws Exception { + + var audit = new CreateAuditDTO(); + audit.setId(UUID.randomUUID()); + audit.setSource(AuditLogSource.AUTO); + audit.setAuditDetails(OBJECT_MAPPER.readTree("{\"details\": \"\"}")); + + var xUserId = UUID.randomUUID(); + + MvcResult response = mockMvc.perform(put(getPath(audit.getId())) + .with(csrf()) + .header(X_USER_ID_HEADER, xUserId) + .content(OBJECT_MAPPER.writeValueAsString(audit)) + .contentType(MediaType.APPLICATION_JSON_VALUE) + .accept(MediaType.APPLICATION_JSON_VALUE)) + .andExpect(status().isBadRequest()) + .andReturn(); + + assertThat(response.getResponse().getContentAsString()) + .isEqualTo("{\"auditDetails\":\"contains potentially malicious content\"}"); + + verify(auditService, never()).upsert(any(), any()); + } + + @DisplayName("Should fail validation when audit details contains malicious key") + @Test + void createAuditEndpointMaliciousAuditDetailsKeyBadRequest() throws Exception { + + var audit = new CreateAuditDTO(); + audit.setId(UUID.randomUUID()); + audit.setSource(AuditLogSource.AUTO); + audit.setAuditDetails(OBJECT_MAPPER.readTree("{\"\": \"safe\"}")); + + var xUserId = UUID.randomUUID(); + + MvcResult response = mockMvc.perform(put(getPath(audit.getId())) + .with(csrf()) + .header(X_USER_ID_HEADER, xUserId) + .content(OBJECT_MAPPER.writeValueAsString(audit)) + .contentType(MediaType.APPLICATION_JSON_VALUE) + .accept(MediaType.APPLICATION_JSON_VALUE)) + .andExpect(status().isBadRequest()) + .andReturn(); + + assertThat(response.getResponse().getContentAsString()) + .isEqualTo("{\"auditDetails\":\"contains potentially malicious content\"}"); + + verify(auditService, never()).upsert(any(), any()); + } + private String getPath(UUID auditId) { return "/audit/" + auditId; } diff --git a/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java new file mode 100644 index 000000000..7f183e33f --- /dev/null +++ b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java @@ -0,0 +1,80 @@ +package uk.gov.hmcts.reform.preapi.dto.validators; + +import com.fasterxml.jackson.databind.ObjectMapper; +import jakarta.validation.ConstraintValidatorContext; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +class SanitizedJsonNodeValidatorTest { + + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + + @Test + @DisplayName("Should return true for null value") + void validateNullValue() { + var validator = new SanitizedJsonNodeValidator(); + var context = mock(ConstraintValidatorContext.class); + + assertTrue(validator.isValid(null, context)); + } + + @Test + @DisplayName("Should return true for safe nested JSON") + void validateSafeNestedJson() throws Exception { + var validator = new SanitizedJsonNodeValidator(); + var context = mock(ConstraintValidatorContext.class); + var jsonNode = OBJECT_MAPPER.readTree(""" + { + "action": "create", + "meta": { + "user": "name", + "array": ["string1", "string2"] + } + } + """); + + assertTrue(validator.isValid(jsonNode, context)); + } + + @Test + @DisplayName("Should return false when key contains script tag") + void validateUnsafeKey() throws Exception { + var validator = new SanitizedJsonNodeValidator(); + var context = mock(ConstraintValidatorContext.class); + var jsonNode = OBJECT_MAPPER.readTree("{\"\": \"value\"}"); + + assertFalse(validator.isValid(jsonNode, context)); + } + + @Test + @DisplayName("Should return false when value contains script tag") + void validateUnsafeValue() throws Exception { + var validator = new SanitizedJsonNodeValidator(); + var context = mock(ConstraintValidatorContext.class); + var jsonNode = OBJECT_MAPPER.readTree("{\"key\": \"\"}"); + + assertFalse(validator.isValid(jsonNode, context)); + } + + @Test + @DisplayName("Should return false when nested value contains malicious content") + void validateUnsafeNestedValue() throws Exception { + var validator = new SanitizedJsonNodeValidator(); + var context = mock(ConstraintValidatorContext.class); + var jsonNode = OBJECT_MAPPER.readTree(""" + { + "details": [ + {"safe": "ok"}, + {"unsafe": ""} + ] + } + """); + + assertFalse(validator.isValid(jsonNode, context)); + } +} + From f63ac48ef9c6dfd835c77056874536946d6e524e Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Tue, 24 Mar 2026 16:08:09 +0000 Subject: [PATCH 18/28] tests and cleanup --- .../preapi/controllers/AuditControllerFT.java | 21 +++++++++++ .../preapi/controllers/CourtControllerFT.java | 35 +++++++++++++++++++ .../preapi/controllers/EditControllerFT.java | 4 +-- .../controllers/InviteControllerFT.java | 17 +++++++++ .../preapi/controllers/UserControllerFT.java | 20 +++++++++++ .../controllers/VfMigrationControllerFT.java | 27 ++++++++++++++ .../test/courts/email_addresses_unsafe.csv | 2 ++ .../reform/preapi/services/CourtService.java | 5 +++ .../preapi/utils/InputSanitizerUtils.java | 2 +- .../SanitizedJsonNodeValidatorTest.java | 16 +++++++++ .../preapi/services/CourtServiceTest.java | 25 +++++++++++++ .../services/EditRequestServiceTest.java | 20 +++++++++++ .../preapi/util/InputSanitizerUtilsTest.java | 11 +++--- 13 files changed, 197 insertions(+), 8 deletions(-) create mode 100644 src/functionalTest/resources/test/courts/email_addresses_unsafe.csv diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java index 6feb717a1..b6a2da13e 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java @@ -1,6 +1,7 @@ package uk.gov.hmcts.reform.preapi.controllers; import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonNode; import io.restassured.response.Response; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -50,6 +51,26 @@ void updateAuditFailureWithUnsafeData() throws JsonProcessingException { .contains("potentially malicious content"); } + @DisplayName("Should not put an audit with un-sanitised audit details JSON") + @Test + void updateAuditFailureWithUnsafeAuditDetailsData() throws JsonProcessingException { + CreateAuditDTO audit = new CreateAuditDTO(); + audit.setId(UUID.randomUUID()); + audit.setAuditDetails(OBJECT_MAPPER.readTree("{\"test\": \"test\"}")); + audit.setSource(AuditLogSource.AUTO); + audit.setActivity("Nice Activity"); + audit.setFunctionalArea("Nice Area"); + JsonNode + unsafeNode = OBJECT_MAPPER.readTree("{\"test\": \"\", " + + "\"test2\": {\"nested\": \"\"}}}"); + audit.setAuditDetails(unsafeNode); + + var error = putAudit(audit); + assertResponseCode(error, 400); + assertThat(error.getBody().asPrettyString()) + .contains("potentially malicious content"); + } + private Response putAudit(CreateAuditDTO dto) throws JsonProcessingException { return doPutRequest( AUDIT_ENDPOINT + dto.getId(), 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 a343bc78c..b1319c139 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 @@ -7,10 +7,12 @@ 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; @@ -18,6 +20,7 @@ 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 @@ -74,6 +77,38 @@ 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 { 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 78ef24125..f984b4106 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 @@ -156,7 +156,7 @@ void editRequestWithUnsafeData() throws JsonProcessingException { ); assertResponseCode(postResponse, 400); -// assertThat(postResponse.getBody().) -// .contains("potentially malicious content"); + 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 c3c6a8620..f6c363c8d 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 39c3da0b4..b756de568 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 e02d1d9d5..ab01cd980 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 000000000..1c7036952 --- /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/main/java/uk/gov/hmcts/reform/preapi/services/CourtService.java b/src/main/java/uk/gov/hmcts/reform/preapi/services/CourtService.java index 94cf9fecc..8455b195b 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,9 @@ 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/utils/InputSanitizerUtils.java b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java index e9bb5160f..84169962c 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java @@ -38,7 +38,7 @@ public static String sanitize(String input) { * @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 */ - public static String sanitize(String input, boolean allowBasicFormatting) { + private static String sanitize(String input, boolean allowBasicFormatting) { if (input == null) { return null; } diff --git a/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java index 7f183e33f..90c8dcd35 100644 --- a/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java +++ b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java @@ -76,5 +76,21 @@ void validateUnsafeNestedValue() throws Exception { assertFalse(validator.isValid(jsonNode, context)); } + + @Test + @DisplayName("Should return false when nested value contains malicious content") + void validateUnsafeDoubleNestedValue() throws Exception { + var validator = new SanitizedJsonNodeValidator(); + var context = mock(ConstraintValidatorContext.class); + var jsonNode = OBJECT_MAPPER.readTree(""" + { + "details": [ + {"unsafe": {"ok": {"yes": ""}}} + ] + } + """); + + assertFalse(validator.isValid(jsonNode, 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 085912a61..2c5c579fe 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 4b2433947..9c4b03fa0 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 index 202945056..9f61e1acf 100644 --- a/src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerUtilsTest.java +++ b/src/test/java/uk/gov/hmcts/reform/preapi/util/InputSanitizerUtilsTest.java @@ -9,8 +9,6 @@ public class InputSanitizerUtilsTest { - //TODO: Add more unit tests if needed - @ParameterizedTest @ValueSource(strings = {"TEST", "TEST", "TEST", "TEST", "TEST"}) @@ -27,10 +25,13 @@ public void shouldSanitizeScriptTags() { """ "> """; - String expectedOutput = "\">"; - String actualOutput = InputSanitizerUtils.sanitize(input); + assertThat(input).isNotEqualTo(InputSanitizerUtils.sanitize(input)); + } - assertThat(actualOutput).isEqualTo(expectedOutput); + @Test + public void shouldReturnNullIfInputIsNull() { + String actualOutput = InputSanitizerUtils.sanitize(null); + assertThat(actualOutput).isNull(); } } From 63dd3bb305a3beafd9097ad3853443c5e952b519 Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Tue, 24 Mar 2026 16:16:23 +0000 Subject: [PATCH 19/28] removes unneeded annotation --- .../uk/gov/hmcts/reform/preapi/controllers/EditControllerFT.java | 1 - 1 file changed, 1 deletion(-) 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 f984b4106..171a19d33 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 @@ -26,7 +26,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.when; -@Slf4j 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"; From 16f503ce4171a8aacc8d5b64775073f195f8ef51 Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Tue, 24 Mar 2026 17:09:53 +0000 Subject: [PATCH 20/28] checkstyle fixes --- .../preapi/controllers/AuditControllerFT.java | 3 +- .../preapi/controllers/CourtControllerFT.java | 15 +++---- .../preapi/controllers/EditControllerFT.java | 4 +- .../preapi/dto/VerifyEmailRequestDTO.java | 1 - .../SanitizedJsonNodeConstraint.java | 2 +- .../SanitizedJsonNodeValidator.java | 40 +++++++++++-------- .../validators/SanitizedStringValidator.java | 1 - .../reform/preapi/services/CourtService.java | 3 +- .../preapi/services/EditRequestService.java | 5 ++- .../preapi/utils/InputSanitizerUtils.java | 1 - .../SanitizedJsonNodeValidatorTest.java | 27 +++++++++++++ 11 files changed, 69 insertions(+), 33 deletions(-) diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java index b6a2da13e..a52bd5311 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java @@ -62,7 +62,8 @@ void updateAuditFailureWithUnsafeAuditDetailsData() throws JsonProcessingExcepti audit.setFunctionalArea("Nice Area"); JsonNode unsafeNode = OBJECT_MAPPER.readTree("{\"test\": \"\", " - + "\"test2\": {\"nested\": \"\"}}}"); + + "\"test2\": {\"nested\": " + + "\"\"}}}"); audit.setAuditDetails(unsafeNode); var error = putAudit(audit); 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 b1319c139..d0edd9655 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 @@ -20,7 +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"; + 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 @@ -117,13 +118,13 @@ void createCourtWithUnsanitizedFields() throws JsonProcessingException { 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")) + Response response = putCourt(dto); + assertResponseCode(response, 400); + assertThat(response.body().jsonPath().getString("name")) .contains("potentially malicious content"); - assertThat(response.body().jsonPath().getString("county")) + 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 171a19d33..295fba7cf 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 @@ -2,7 +2,6 @@ import com.fasterxml.jackson.core.JsonProcessingException; import io.restassured.response.Response; -import lombok.extern.slf4j.Slf4j; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -124,12 +123,12 @@ void editRequestWithUnsafeDataCsv() throws JsonProcessingException { 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(); - RecordingDTO recordingDTO = assertRecordingExists(recordingDetails.recordingId(), true).as(RecordingDTO.class); CreateEditRequestDTO editRequestDTO = new CreateEditRequestDTO(); editRequestDTO.setSourceRecordingId(recordingDetails.recordingId()); @@ -143,6 +142,7 @@ void editRequestWithUnsafeData() throws JsonProcessingException { 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())) diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/VerifyEmailRequestDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/VerifyEmailRequestDTO.java index a667a73de..db1f5b81c 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/VerifyEmailRequestDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/VerifyEmailRequestDTO.java @@ -8,7 +8,6 @@ import jakarta.validation.constraints.Pattern; import lombok.Data; import lombok.NoArgsConstructor; -import uk.gov.hmcts.reform.preapi.dto.validators.SanitizedStringConstraint; @Data @NoArgsConstructor diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeConstraint.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeConstraint.java index 24b2e1c09..efae4e60a 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeConstraint.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeConstraint.java @@ -13,7 +13,7 @@ * Validates JsonNode keys and string values to prevent malicious content. */ @Documented -@Constraint(validatedBy = uk.gov.hmcts.reform.preapi.dto.validators.SanitizedJsonNodeValidator.class) +@Constraint(validatedBy = SanitizedJsonNodeValidator.class) @Target({ ElementType.FIELD, ElementType.PARAMETER }) @Retention(RetentionPolicy.RUNTIME) public @interface SanitizedJsonNodeConstraint { diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidator.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidator.java index 59491664a..189bfd7bf 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidator.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidator.java @@ -22,34 +22,42 @@ public boolean isValid(JsonNode value, ConstraintValidatorContext context) { } private boolean isNodeSanitized(JsonNode node) { - if (node.isObject()) { - for (Map.Entry field : node.properties()) { - if (!isSanitized(field.getKey()) || !isNodeSanitized(field.getValue())) { - return false; - } - } + if (node.isNull()) { return true; } - if (node.isArray()) { - for (JsonNode element : node) { - if (!isNodeSanitized(element)) { - return false; - } - } - return true; + if (node.isObject()) { + return isObjectSanitized(node); } - if (node.isTextual()) { - return isSanitized(node.textValue()); + if (node.isArray()) { + return isArraySanitized(node); } - return true; + return node.isTextual() && isSanitized(node.textValue()); } private boolean isSanitized(String value) { return value.equals(InputSanitizerUtils.sanitize(value)); } + + private boolean isObjectSanitized(JsonNode node) { + for (Map.Entry field : node.properties()) { + if (!isSanitized(field.getKey()) || !isNodeSanitized(field.getValue())) { + return false; + } + } + return true; + } + + private boolean isArraySanitized(JsonNode node) { + for (JsonNode element : node) { + if (!isNodeSanitized(element)) { + return false; + } + } + return true; + } } 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 index ed8c5340e..9a269aec3 100644 --- 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 @@ -2,7 +2,6 @@ import jakarta.validation.ConstraintValidator; import jakarta.validation.ConstraintValidatorContext; -import lombok.extern.slf4j.Slf4j; import uk.gov.hmcts.reform.preapi.utils.InputSanitizerUtils; /** 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 8455b195b..a18fb65c8 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 @@ -110,7 +110,8 @@ 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()); + throw new BadRequestException("Court email may contain potentially malicious content: " + + court.getGroupEmail()); } courtInDb.setGroupEmail(court.getGroupEmail()); 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 de52f4938..8ee9d870b 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 @@ -302,8 +302,9 @@ public EditRequestDTO upsert(UUID sourceRecordingId, MultipartFile file) { 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()); + if (!InputSanitizerUtils.isValid(editInstruction.getReason(), false)) { + throw new BadRequestException("Edit instruction reason potentially contains malicious code: " + + editInstruction.getReason()); } }); 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 index 84169962c..e655a2f33 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java @@ -1,6 +1,5 @@ package uk.gov.hmcts.reform.preapi.utils; -import jakarta.validation.ConstraintValidatorContext; import lombok.experimental.UtilityClass; import org.jsoup.Jsoup; import org.jsoup.nodes.Document; diff --git a/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java index 90c8dcd35..410c9220e 100644 --- a/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java +++ b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java @@ -1,5 +1,6 @@ package uk.gov.hmcts.reform.preapi.dto.validators; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import jakarta.validation.ConstraintValidatorContext; import org.junit.jupiter.api.DisplayName; @@ -92,5 +93,31 @@ void validateUnsafeDoubleNestedValue() throws Exception { assertFalse(validator.isValid(jsonNode, context)); } + + @Test + @DisplayName("Should return true when value is null") + void validateNullNestedValue() throws Exception { + var validator = new SanitizedJsonNodeValidator(); + var context = mock(ConstraintValidatorContext.class); + var jsonNode = OBJECT_MAPPER.readTree(""" + { + "details": [ + {"unsafe": null} + ] + } + """); + + assertTrue(validator.isValid(jsonNode, context)); + } + + @Test + @DisplayName("Should return true when JsonNode is null") + void validateNullJsonNodeValue() throws Exception { + var validator = new SanitizedJsonNodeValidator(); + var context = mock(ConstraintValidatorContext.class); + JsonNode jsonNode = null; + + assertTrue(validator.isValid(jsonNode, context)); + } } From 0aa75e2dcdc04e3c738e3bf4737b753561d08dc2 Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Tue, 24 Mar 2026 17:16:40 +0000 Subject: [PATCH 21/28] pmd fixes --- .../reform/preapi/dto/validators/SanitizedStringValidator.java | 1 + .../uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java | 2 +- .../preapi/dto/validators/SanitizedJsonNodeValidatorTest.java | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) 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 index 9a269aec3..95a8bd265 100644 --- 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 @@ -17,6 +17,7 @@ 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/utils/InputSanitizerUtils.java b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java index e655a2f33..8c4d3b64d 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/utils/InputSanitizerUtils.java @@ -60,7 +60,7 @@ public static boolean isValid(String value, boolean allowBasicFormatting) { // Check if sanitization would change the string // If it changes, it means there was potentially malicious content - String sanitized = InputSanitizerUtils.sanitize(value, allowBasicFormatting); + String sanitized = sanitize(value, allowBasicFormatting); return value.equals(sanitized); } } diff --git a/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java index 410c9220e..d6b88195f 100644 --- a/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java +++ b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java @@ -102,7 +102,7 @@ void validateNullNestedValue() throws Exception { var jsonNode = OBJECT_MAPPER.readTree(""" { "details": [ - {"unsafe": null} + {"safe": null} ] } """); From 18563725a18d0abc2fc91375a2c561891222182e Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Wed, 25 Mar 2026 11:40:55 +0000 Subject: [PATCH 22/28] removes JSON validator to be completed in other ticket --- .../SanitizedJsonNodeConstraint.java | 25 ---- .../SanitizedJsonNodeValidator.java | 64 --------- .../SanitizedJsonNodeValidatorTest.java | 123 ------------------ 3 files changed, 212 deletions(-) delete mode 100644 src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeConstraint.java delete mode 100644 src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidator.java delete mode 100644 src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeConstraint.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeConstraint.java deleted file mode 100644 index efae4e60a..000000000 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeConstraint.java +++ /dev/null @@ -1,25 +0,0 @@ -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 JsonNode keys and string values to prevent malicious content. - */ -@Documented -@Constraint(validatedBy = SanitizedJsonNodeValidator.class) -@Target({ ElementType.FIELD, ElementType.PARAMETER }) -@Retention(RetentionPolicy.RUNTIME) -public @interface SanitizedJsonNodeConstraint { - String message() default "contains potentially malicious content"; - Class[] groups() default {}; - Class[] payload() default {}; -} - - diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidator.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidator.java deleted file mode 100644 index 189bfd7bf..000000000 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidator.java +++ /dev/null @@ -1,64 +0,0 @@ -package uk.gov.hmcts.reform.preapi.dto.validators; - -import com.fasterxml.jackson.databind.JsonNode; -import jakarta.validation.ConstraintValidator; -import jakarta.validation.ConstraintValidatorContext; -import uk.gov.hmcts.reform.preapi.utils.InputSanitizerUtils; - -import java.util.Map; - -/** - * Validator that checks JsonNode keys and string values for potentially malicious content. - */ -public class SanitizedJsonNodeValidator implements ConstraintValidator { - - @Override - public boolean isValid(JsonNode value, ConstraintValidatorContext context) { - if (value == null || value.isNull()) { - return true; - } - - return isNodeSanitized(value); - } - - private boolean isNodeSanitized(JsonNode node) { - if (node.isNull()) { - return true; - } - - if (node.isObject()) { - return isObjectSanitized(node); - } - - if (node.isArray()) { - return isArraySanitized(node); - } - - return node.isTextual() && isSanitized(node.textValue()); - } - - private boolean isSanitized(String value) { - return value.equals(InputSanitizerUtils.sanitize(value)); - } - - private boolean isObjectSanitized(JsonNode node) { - for (Map.Entry field : node.properties()) { - if (!isSanitized(field.getKey()) || !isNodeSanitized(field.getValue())) { - return false; - } - } - return true; - } - - private boolean isArraySanitized(JsonNode node) { - for (JsonNode element : node) { - if (!isNodeSanitized(element)) { - return false; - } - } - return true; - } -} - - - diff --git a/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java deleted file mode 100644 index d6b88195f..000000000 --- a/src/test/java/uk/gov/hmcts/reform/preapi/dto/validators/SanitizedJsonNodeValidatorTest.java +++ /dev/null @@ -1,123 +0,0 @@ -package uk.gov.hmcts.reform.preapi.dto.validators; - -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; -import jakarta.validation.ConstraintValidatorContext; -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.Mockito.mock; - -class SanitizedJsonNodeValidatorTest { - - private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); - - @Test - @DisplayName("Should return true for null value") - void validateNullValue() { - var validator = new SanitizedJsonNodeValidator(); - var context = mock(ConstraintValidatorContext.class); - - assertTrue(validator.isValid(null, context)); - } - - @Test - @DisplayName("Should return true for safe nested JSON") - void validateSafeNestedJson() throws Exception { - var validator = new SanitizedJsonNodeValidator(); - var context = mock(ConstraintValidatorContext.class); - var jsonNode = OBJECT_MAPPER.readTree(""" - { - "action": "create", - "meta": { - "user": "name", - "array": ["string1", "string2"] - } - } - """); - - assertTrue(validator.isValid(jsonNode, context)); - } - - @Test - @DisplayName("Should return false when key contains script tag") - void validateUnsafeKey() throws Exception { - var validator = new SanitizedJsonNodeValidator(); - var context = mock(ConstraintValidatorContext.class); - var jsonNode = OBJECT_MAPPER.readTree("{\"\": \"value\"}"); - - assertFalse(validator.isValid(jsonNode, context)); - } - - @Test - @DisplayName("Should return false when value contains script tag") - void validateUnsafeValue() throws Exception { - var validator = new SanitizedJsonNodeValidator(); - var context = mock(ConstraintValidatorContext.class); - var jsonNode = OBJECT_MAPPER.readTree("{\"key\": \"\"}"); - - assertFalse(validator.isValid(jsonNode, context)); - } - - @Test - @DisplayName("Should return false when nested value contains malicious content") - void validateUnsafeNestedValue() throws Exception { - var validator = new SanitizedJsonNodeValidator(); - var context = mock(ConstraintValidatorContext.class); - var jsonNode = OBJECT_MAPPER.readTree(""" - { - "details": [ - {"safe": "ok"}, - {"unsafe": ""} - ] - } - """); - - assertFalse(validator.isValid(jsonNode, context)); - } - - @Test - @DisplayName("Should return false when nested value contains malicious content") - void validateUnsafeDoubleNestedValue() throws Exception { - var validator = new SanitizedJsonNodeValidator(); - var context = mock(ConstraintValidatorContext.class); - var jsonNode = OBJECT_MAPPER.readTree(""" - { - "details": [ - {"unsafe": {"ok": {"yes": ""}}} - ] - } - """); - - assertFalse(validator.isValid(jsonNode, context)); - } - - @Test - @DisplayName("Should return true when value is null") - void validateNullNestedValue() throws Exception { - var validator = new SanitizedJsonNodeValidator(); - var context = mock(ConstraintValidatorContext.class); - var jsonNode = OBJECT_MAPPER.readTree(""" - { - "details": [ - {"safe": null} - ] - } - """); - - assertTrue(validator.isValid(jsonNode, context)); - } - - @Test - @DisplayName("Should return true when JsonNode is null") - void validateNullJsonNodeValue() throws Exception { - var validator = new SanitizedJsonNodeValidator(); - var context = mock(ConstraintValidatorContext.class); - JsonNode jsonNode = null; - - assertTrue(validator.isValid(jsonNode, context)); - } -} - From a3d70a875224537d5d5d7edce2214375ba3ba48c Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Wed, 25 Mar 2026 11:41:06 +0000 Subject: [PATCH 23/28] removes JSON validator to be completed in other ticket --- .../java/uk/gov/hmcts/reform/preapi/dto/CreateAuditDTO.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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 144edad9d..5022050a9 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,7 +8,6 @@ import jakarta.validation.constraints.NotNull; import lombok.Data; import lombok.NoArgsConstructor; -import uk.gov.hmcts.reform.preapi.dto.validators.SanitizedJsonNodeConstraint; 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; @@ -51,8 +50,7 @@ public class CreateAuditDTO { @Schema(description = "AuditDetailsJSONString") @JsonRawValue - @SanitizedJsonNodeConstraint - private JsonNode auditDetails; + private JsonNode auditDetails; //TODO: Sanitised annotation to be added later public CreateAuditDTO(Audit auditEntity) { this.id = auditEntity.getId(); From 26fab65a7e75671b6a74cd4b33ab888363569169 Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Wed, 25 Mar 2026 12:00:38 +0000 Subject: [PATCH 24/28] cleanup --- .../hmcts/reform/preapi/controllers/InviteController.java | 5 +---- .../java/uk/gov/hmcts/reform/preapi/dto/CreateInviteDTO.java | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/controllers/InviteController.java b/src/main/java/uk/gov/hmcts/reform/preapi/controllers/InviteController.java index a41a260f5..f574141b7 100644 --- a/src/main/java/uk/gov/hmcts/reform/preapi/controllers/InviteController.java +++ b/src/main/java/uk/gov/hmcts/reform/preapi/controllers/InviteController.java @@ -4,7 +4,6 @@ import io.swagger.v3.oas.annotations.Parameter; import io.swagger.v3.oas.annotations.media.Schema; import jakarta.validation.Valid; -import jakarta.validation.constraints.Email; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; @@ -14,7 +13,6 @@ import org.springframework.http.HttpEntity; import org.springframework.http.ResponseEntity; import org.springframework.security.access.prepost.PreAuthorize; -import org.springframework.validation.annotation.Validated; import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.ModelAttribute; @@ -140,7 +138,6 @@ public ResponseEntity deleteInvite(@PathVariable UUID userId) { } @PostMapping("/redeem") - @Validated @Operation(operationId = "redeemInvite", summary = "Redeem an invite") @Parameter( name = "email", @@ -148,7 +145,7 @@ public ResponseEntity deleteInvite(@PathVariable UUID userId) { example = "example@example.com", schema = @Schema(implementation = String.class) ) - public ResponseEntity redeemInvite(@RequestParam @Email String email) { + public ResponseEntity redeemInvite(@RequestParam String email) { return getUpsertResponse(inviteService.redeemInvite(email), null); } } 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 3fc88f3bb..f4b037aee 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 @@ -42,6 +42,6 @@ public class CreateInviteDTO { @Schema(description = "InvitePhone") @SanitizedStringConstraint - protected String phone; //TODO: add validation for phone number? Safe to add Sanitised here + protected String phone; //TODO: Sanitised constraint added but needs specific phone number validation in future } From 9668ab1e17a4fd091a6b8fdfded0fc1332f020f7 Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Wed, 25 Mar 2026 12:05:37 +0000 Subject: [PATCH 25/28] cleanup --- .../preapi/dto/ExampleSanitizedDTO.java | 46 ---------------- .../controller/AuditControllerTest.java | 52 ------------------- 2 files changed, 98 deletions(-) delete mode 100644 src/main/java/uk/gov/hmcts/reform/preapi/dto/ExampleSanitizedDTO.java diff --git a/src/main/java/uk/gov/hmcts/reform/preapi/dto/ExampleSanitizedDTO.java b/src/main/java/uk/gov/hmcts/reform/preapi/dto/ExampleSanitizedDTO.java deleted file mode 100644 index 7c30f4d74..000000000 --- a/src/main/java/uk/gov/hmcts/reform/preapi/dto/ExampleSanitizedDTO.java +++ /dev/null @@ -1,46 +0,0 @@ -package uk.gov.hmcts.reform.preapi.dto; - -import com.fasterxml.jackson.databind.PropertyNamingStrategies; -import com.fasterxml.jackson.databind.annotation.JsonNaming; -import io.swagger.v3.oas.annotations.media.Schema; -import jakarta.validation.constraints.NotNull; -import jakarta.validation.constraints.Size; -import lombok.Data; -import lombok.NoArgsConstructor; -import uk.gov.hmcts.reform.preapi.dto.validators.SanitizedStringConstraint; - -import java.util.UUID; - -/** - * EXAMPLE DTO showing how to use the @SanitizedStringConstraint annotation - * to protect against XSS attacks on user-provided string fields. - * This is a sample/reference implementation - not meant to be used directly. - */ -@Data -@NoArgsConstructor -@Schema(description = "ExampleDTO - Shows how to apply input sanitization") -@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class) -@SuppressWarnings("unused") // This is an example/reference implementation -public class ExampleSanitizedDTO { - - @NotNull - @Schema(description = "Example ID") - private UUID id; - - @NotNull - @Size(max = 100) - @SanitizedStringConstraint // Ensures no HTML/scripts can be injected - @Schema(description = "Example Name - Will be validated for XSS attacks") - private String name; - - @Size(max = 500) - @SanitizedStringConstraint(message = "description contains potentially malicious HTML content") - @Schema(description = "Example Description - Strictly sanitized (strict by default)") - private String description; - - @Size(max = 1000) - @SanitizedStringConstraint(allowBasicFormatting = true) // Allow , ,

,
etc. - @Schema(description = "Example Notes - Allows basic formatting") - private String notes; -} - diff --git a/src/test/java/uk/gov/hmcts/reform/preapi/controller/AuditControllerTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/controller/AuditControllerTest.java index d797e3064..102f5c7aa 100644 --- a/src/test/java/uk/gov/hmcts/reform/preapi/controller/AuditControllerTest.java +++ b/src/test/java/uk/gov/hmcts/reform/preapi/controller/AuditControllerTest.java @@ -160,58 +160,6 @@ void createAuditEndpointNotAcceptable() throws Exception { .andExpect(status().is4xxClientError()); } - @DisplayName("Should fail validation when audit details contains malicious content") - @Test - void createAuditEndpointMaliciousAuditDetailsBadRequest() throws Exception { - - var audit = new CreateAuditDTO(); - audit.setId(UUID.randomUUID()); - audit.setSource(AuditLogSource.AUTO); - audit.setAuditDetails(OBJECT_MAPPER.readTree("{\"details\": \"\"}")); - - var xUserId = UUID.randomUUID(); - - MvcResult response = mockMvc.perform(put(getPath(audit.getId())) - .with(csrf()) - .header(X_USER_ID_HEADER, xUserId) - .content(OBJECT_MAPPER.writeValueAsString(audit)) - .contentType(MediaType.APPLICATION_JSON_VALUE) - .accept(MediaType.APPLICATION_JSON_VALUE)) - .andExpect(status().isBadRequest()) - .andReturn(); - - assertThat(response.getResponse().getContentAsString()) - .isEqualTo("{\"auditDetails\":\"contains potentially malicious content\"}"); - - verify(auditService, never()).upsert(any(), any()); - } - - @DisplayName("Should fail validation when audit details contains malicious key") - @Test - void createAuditEndpointMaliciousAuditDetailsKeyBadRequest() throws Exception { - - var audit = new CreateAuditDTO(); - audit.setId(UUID.randomUUID()); - audit.setSource(AuditLogSource.AUTO); - audit.setAuditDetails(OBJECT_MAPPER.readTree("{\"\": \"safe\"}")); - - var xUserId = UUID.randomUUID(); - - MvcResult response = mockMvc.perform(put(getPath(audit.getId())) - .with(csrf()) - .header(X_USER_ID_HEADER, xUserId) - .content(OBJECT_MAPPER.writeValueAsString(audit)) - .contentType(MediaType.APPLICATION_JSON_VALUE) - .accept(MediaType.APPLICATION_JSON_VALUE)) - .andExpect(status().isBadRequest()) - .andReturn(); - - assertThat(response.getResponse().getContentAsString()) - .isEqualTo("{\"auditDetails\":\"contains potentially malicious content\"}"); - - verify(auditService, never()).upsert(any(), any()); - } - private String getPath(UUID auditId) { return "/audit/" + auditId; } From 2359e66de03ff477d0d10ad2b1b78375ab48e2b2 Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Wed, 25 Mar 2026 12:09:34 +0000 Subject: [PATCH 26/28] checkstyle fix --- .../gov/hmcts/reform/preapi/controller/AuditControllerTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/java/uk/gov/hmcts/reform/preapi/controller/AuditControllerTest.java b/src/test/java/uk/gov/hmcts/reform/preapi/controller/AuditControllerTest.java index 102f5c7aa..0fecbdfa9 100644 --- a/src/test/java/uk/gov/hmcts/reform/preapi/controller/AuditControllerTest.java +++ b/src/test/java/uk/gov/hmcts/reform/preapi/controller/AuditControllerTest.java @@ -25,8 +25,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; From 8610b8daf760ee6fca3e9a3c5831561d8e3c32e3 Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Wed, 25 Mar 2026 13:38:50 +0000 Subject: [PATCH 27/28] remove audit details check to be done in future ticket --- .../preapi/controllers/AuditControllerFT.java | 39 ------------------- 1 file changed, 39 deletions(-) diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java index a52bd5311..be2c1fdbb 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java @@ -33,45 +33,6 @@ void updateAuditFailure() throws JsonProcessingException { .isEqualTo("Data is immutable and cannot be changed. Id: " + audit.getId()); } - @DisplayName("Should not put an audit with un-sanitised data") - @Test - void updateAuditFailureWithUnsafeData() throws JsonProcessingException { - CreateAuditDTO audit = new CreateAuditDTO(); - audit.setId(UUID.randomUUID()); - audit.setAuditDetails(OBJECT_MAPPER.readTree("{\"test\": \"test\"}")); - audit.setSource(AuditLogSource.AUTO); - audit.setActivity(""); - audit.setFunctionalArea(""); - - var error = putAudit(audit); - assertResponseCode(error, 400); - assertThat(error.body().jsonPath().getString("activity")) - .contains("potentially malicious content"); - assertThat(error.body().jsonPath().getString("functionalArea")) - .contains("potentially malicious content"); - } - - @DisplayName("Should not put an audit with un-sanitised audit details JSON") - @Test - void updateAuditFailureWithUnsafeAuditDetailsData() throws JsonProcessingException { - CreateAuditDTO audit = new CreateAuditDTO(); - audit.setId(UUID.randomUUID()); - audit.setAuditDetails(OBJECT_MAPPER.readTree("{\"test\": \"test\"}")); - audit.setSource(AuditLogSource.AUTO); - audit.setActivity("Nice Activity"); - audit.setFunctionalArea("Nice Area"); - JsonNode - unsafeNode = OBJECT_MAPPER.readTree("{\"test\": \"\", " - + "\"test2\": {\"nested\": " - + "\"\"}}}"); - audit.setAuditDetails(unsafeNode); - - var error = putAudit(audit); - assertResponseCode(error, 400); - assertThat(error.getBody().asPrettyString()) - .contains("potentially malicious content"); - } - private Response putAudit(CreateAuditDTO dto) throws JsonProcessingException { return doPutRequest( AUDIT_ENDPOINT + dto.getId(), From a4d6d7c5110b99c4784b1d29973fd96b11fcee6e Mon Sep 17 00:00:00 2001 From: "ruth.bovell" Date: Wed, 25 Mar 2026 13:45:58 +0000 Subject: [PATCH 28/28] checkstyle fix --- .../gov/hmcts/reform/preapi/controllers/AuditControllerFT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java index be2c1fdbb..996ca06e0 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/preapi/controllers/AuditControllerFT.java @@ -1,7 +1,6 @@ package uk.gov.hmcts.reform.preapi.controllers; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.JsonNode; import io.restassured.response.Response; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test;