CME-984: Error Messages Enabled by Default

src/contractTest/resources/application.properties

@@ -3,7 +3,7 @@ server.port=4452
 server.servlet.contextPath=
 server.compression.enabled=true
 server.compression.mime-types=application/json,application/xml,text/html,text/xml,text/plain
-server.error.include-message=always
+server.error.include-message=never
 server.max-http-header-size=100KB
 spring.config.import=optional:configtree:/mnt/secrets/ccd/
 

src/main/java/uk/gov/hmcts/ccd/config/SafeErrorAttributes.java

@@ -0,0 +1,38 @@
+package uk.gov.hmcts.ccd.config;
+
+import org.springframework.boot.web.error.ErrorAttributeOptions;
+import org.springframework.boot.web.servlet.error.DefaultErrorAttributes;
+import org.springframework.http.HttpStatus;
+import org.springframework.stereotype.Component;
+import org.springframework.web.context.request.WebRequest;
+import uk.gov.hmcts.ccd.domain.model.common.HttpError;
+
+import java.util.Map;
+
+@Component
+public class SafeErrorAttributes extends DefaultErrorAttributes {
+
+    private static final String ACCESS_DENIED_MESSAGE = "Access Denied";
+
+    @Override
+    public Map<String, Object> getErrorAttributes(WebRequest webRequest, ErrorAttributeOptions options) {
+        Map<String, Object> errorAttributes = super.getErrorAttributes(webRequest, options);
+        errorAttributes.put("message", safeMessage(errorAttributes.get("status")));
+        errorAttributes.remove("exception");
+        return errorAttributes;
+    }
+
+    String safeMessage(Object status) {
+        if (status instanceof Number statusCode) {
+            HttpStatus httpStatus = HttpStatus.resolve(statusCode.intValue());
+            if (HttpStatus.FORBIDDEN.equals(httpStatus)) {
+                return ACCESS_DENIED_MESSAGE;
+            }
+            if (httpStatus != null) {
+                return httpStatus.getReasonPhrase();
+            }
+        }
+
+        return HttpError.DEFAULT_ERROR;
+    }
+}

src/main/java/uk/gov/hmcts/ccd/domain/model/common/HttpError.java

@@ -34,7 +34,7 @@ public HttpError(Exception exception, HttpServletRequest request) {
         this.timestamp = LocalDateTime.now(ZoneOffset.UTC);
         this.status = getStatusFromResponseStatus(responseStatus);
         this.error = getErrorReason(responseStatus);
-        this.message = exception.getMessage();
+        this.message = error;
         this.path = UriUtils.encodePath(request.getRequestURI(), StandardCharsets.UTF_8);
     }
 
@@ -45,7 +45,7 @@ public HttpError(Exception exception, String path, HttpStatus status) {
         this.timestamp = LocalDateTime.now(ZoneOffset.UTC);
         this.status = getStatusFromResponseStatus(responseStatus, status);
         this.error = getErrorReason(responseStatus, status);
-        this.message = exception.getMessage();
+        this.message = error;
         this.path = UriUtils.encodePath(path, StandardCharsets.UTF_8);
     }
 

src/main/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandler.java

@@ -80,6 +80,7 @@ public ResponseEntity<HttpError> handleCaseValidationException(HttpServletReques
             exception.getFields(), exception);
         appInsights.trackException(exception, customProperties, SeverityLevel.Warning);
         final HttpError<Serializable> error = new HttpError<>(exception, request)
+            .withMessage(exception.getMessage())
             .withDetails(exception.getDetails());
         return ResponseEntity
             .status(error.getStatus())

src/main/resources/application.properties

@@ -3,7 +3,7 @@ server.port=4452
 server.servlet.contextPath=
 server.compression.enabled=true
 server.compression.mime-types=application/json,application/xml,text/html,text/xml,text/plain
-server.error.include-message=always
+server.error.include-message=never
 server.max-http-request-header-size=100KB
 spring.config.import=optional:configtree:/mnt/secrets/ccd/
 

src/test/java/uk/gov/hmcts/ccd/ElasticsearchIT.java

@@ -69,13 +69,11 @@
 import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
 import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
 import static org.elasticsearch.index.query.QueryBuilders.matchQuery;
-import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.hasItem;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.hasProperty;
-import static org.hamcrest.Matchers.startsWith;
 import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.mockito.Mockito.doReturn;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
@@ -644,7 +642,7 @@ void shouldReturnErrorWithUnsupportedUseCase() throws Exception {
 
             assertAll(
                 () -> assertThat(exceptionNode.get(ERROR_MESSAGE).asText(),
-                    is("The provided use case 'INVALID' is unsupported for case type 'AAT'."))
+                    is("Bad Request"))
             );
         }
 
@@ -657,7 +655,7 @@ void shouldReturnErrorWithMissingQueryField() throws Exception {
 
             assertAll(
                 () -> assertThat(exceptionNode.get(ERROR_MESSAGE).asText(),
-                    is("missing required field 'query'"))
+                    is("Bad Request"))
             );
         }
 
@@ -672,7 +670,7 @@ void shouldReturnErrorWhenElasticsearchErrors() throws Exception {
 
             assertAll(
                 () -> assertThat(exceptionNode.get(ERROR_MESSAGE).asText(),
-                    containsString("No mapping found for [invalid.keyword] in order to sort on"))
+                    is("Bad Request"))
             );
         }
 
@@ -693,7 +691,7 @@ void shouldReturnErrorWhenInvalidJsonIsSent() throws Exception {
 
             assertAll(
                 () -> assertThat(exceptionNode.get(ERROR_MESSAGE).asText(),
-                    containsString("Request requires correctly formatted JSON"))
+                    is("Bad Request"))
             );
         }
 
@@ -1589,7 +1587,7 @@ void shouldErrorWhenInvalidCaseTypeIsProvided() throws Exception {
 
                 assertAll(
                     () -> assertThat(exceptionNode.get("message").asText(),
-                        startsWith("Resource not found when getting case type definition for INVALID"))
+                        is("Not Found"))
                 );
             }
 

src/test/java/uk/gov/hmcts/ccd/config/SafeErrorAttributesTest.java

@@ -0,0 +1,57 @@
+package uk.gov.hmcts.ccd.config;
+
+import org.junit.jupiter.api.Test;
+import org.springframework.boot.web.error.ErrorAttributeOptions;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.http.HttpStatus;
+import org.springframework.web.context.request.ServletWebRequest;
+import uk.gov.hmcts.ccd.domain.model.common.HttpError;
+
+import jakarta.servlet.RequestDispatcher;
+import java.util.Map;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+
+class SafeErrorAttributesTest {
+
+    private final SafeErrorAttributes safeErrorAttributes = new SafeErrorAttributes();
+
+    @Test
+    void shouldUseAccessDeniedForForbiddenResponses() {
+        assertThat(safeErrorAttributes.safeMessage(HttpStatus.FORBIDDEN.value()), is("Access Denied"));
+    }
+
+    @Test
+    void shouldUseReasonPhraseForNonForbiddenResponses() {
+        assertThat(safeErrorAttributes.safeMessage(HttpStatus.BAD_REQUEST.value()), is("Bad Request"));
+    }
+
+    @Test
+    void shouldUseDefaultErrorWhenStatusCannotBeResolved() {
+        assertThat(safeErrorAttributes.safeMessage(null), is(HttpError.DEFAULT_ERROR));
+    }
+
+    @Test
+    void shouldOverrideDefaultErrorAttributesWithSafeMessageAndRemoveException() {
+        Map<String, Object> errorAttributes = safeErrorAttributes.getErrorAttributes(
+            errorRequest(HttpStatus.BAD_REQUEST, "Raw exception message"),
+            ErrorAttributeOptions.defaults().including(
+                ErrorAttributeOptions.Include.MESSAGE,
+                ErrorAttributeOptions.Include.EXCEPTION
+            )
+        );
+
+        assertThat(errorAttributes.get("message"), is("Bad Request"));
+        assertThat(errorAttributes.containsKey("exception"), is(false));
+    }
+
+    private ServletWebRequest errorRequest(HttpStatus status, String message) {
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        request.setRequestURI("/some/path");
+        request.setAttribute(RequestDispatcher.ERROR_STATUS_CODE, status.value());
+        request.setAttribute(RequestDispatcher.ERROR_MESSAGE, message);
+        request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, new IllegalArgumentException(message));
+        return new ServletWebRequest(request);
+    }
+}

src/test/java/uk/gov/hmcts/ccd/domain/model/common/HttpErrorTest.java

@@ -97,11 +97,11 @@ public void shouldExtractErrorFromExceptionAnnotation_default() {
     }
 
     @Test
-    public void shouldExtractMessageFromException() {
+    public void shouldUseSafeDefaultMessageFromException() {
         final uk.gov.hmcts.ccd.domain.model.common.HttpError error =
                 new uk.gov.hmcts.ccd.domain.model.common.HttpError(new IllegalArgumentException(MESSAGE), request);
 
-        assertThat(error.getMessage(), is(equalTo(MESSAGE)));
+        assertThat(error.getMessage(), is(equalTo(uk.gov.hmcts.ccd.domain.model.common.HttpError.DEFAULT_ERROR)));
     }
 
     @Test

src/test/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandlerTest.java

@@ -763,6 +763,7 @@ private void assertHttpErrorResponse(ResultActions result, Exception expectedExc
         // NB: as we cannot mock HttpError generate an equivalent and compare to response
         final HttpError<Serializable> expectedError =
             new HttpError<>(expectedException, mock(HttpServletRequest.class));
+        withExplicitClientVisibleMessage(expectedError, expectedException);
 
         assertBasicHttpErrorResponseProperties(result, expectedException, expectedError);
     }
@@ -776,10 +777,17 @@ private void assertHttpErrorResponse(ResultActions result, Exception expectedExc
         // NB: as we cannot mock HttpError generate an equivalent and compare to response
         final HttpError<Serializable> expectedError =
             new HttpError<>(expectedException, mock(HttpServletRequest.class), expectedHttpStatus);
+        withExplicitClientVisibleMessage(expectedError, expectedException);
 
         assertBasicHttpErrorResponseProperties(result, expectedException, expectedError);
     }
 
+    private void withExplicitClientVisibleMessage(HttpError<Serializable> expectedError, Exception expectedException) {
+        if (expectedException instanceof CaseValidationException) {
+            expectedError.withMessage(expectedException.getMessage());
+        }
+    }
+
     private void assertBasicHttpErrorResponseProperties(ResultActions result, Exception expectedException,
                                                         HttpError<Serializable> expectedHttpError) throws Exception {
 
@@ -788,7 +796,7 @@ private void assertBasicHttpErrorResponseProperties(ResultActions result, Except
         result.andExpect(jsonPath("$.exception").value(expectedException.getClass().getName()));
 
         // check a bit more
-        result.andExpect(jsonPath("$.message").value(expectedException.getMessage()));
+        result.andExpect(jsonPath("$.message").value(expectedHttpError.getMessage()));
     }
 
     private void assertExtraApiExceptionResponseProperties(ResultActions result, ApiException expectedException)

src/test/java/uk/gov/hmcts/ccd/endpoint/std/CallbackTest.java

@@ -620,8 +620,7 @@ public void shouldReturn422WhenPostCreateCaseWithCallbackOverridingDataWithMissi
         ).andReturn();
 
         assertEquals("Incorrect Response Status Code", 422, mvcResult.getResponse().getStatus());
-        assertEquals("Incorrect Error Message", "\"The event cannot be complete due to a callback returned data "
-                + "validation error (c)\"",
+        assertEquals("Incorrect Error Message", "\"Unprocessable Entity\"",
             mapper.readTree(mvcResult.getResponse().getContentAsString()).get("message").toString());
     }
 

src/test/java/uk/gov/hmcts/ccd/endpoint/std/DraftsEndpointIT.java

@@ -162,8 +162,7 @@ public void shouldReturn404WhenUpdateDraftForCaseworkerWithDraftIdNotFound() thr
 
         assertEquals("Incorrect Response Status Code", 404, mvcResult.getResponse().getStatus());
         String actualResponse = mapper.readTree(mvcResult.getResponse().getContentAsString()).toString();
-        assertThat(actualResponse, containsString("\"message\":" +
-            "\"No draft found ( draft reference = '7578590391163133' )\""));
+        assertThat(actualResponse, containsString("\"message\":\"Not Found\""));
     }
 
     @Test
@@ -299,8 +298,7 @@ public void shouldReturn404WhenGetInvalidDraft() throws Exception {
 
         assertEquals("Incorrect Response Status Code", 404, mvcResult.getResponse().getStatus());
         String actualResponse = mapper.readTree(mvcResult.getResponse().getContentAsString()).toString();
-        assertThat(actualResponse, containsString("\"message\":" +
-            "\"No draft found ( draft reference = '7578590391163133' )\""));
+        assertThat(actualResponse, containsString("\"message\":\"Not Found\""));
     }
 
     @Test