Merge pull request #302 from San-43/fix-287-hide-technical-error-details

Fix #287 hide technical exception messages in API error responses (SQL / Hibernate details)

Author: vfedoriv

src/main/java/org/springframework/samples/petclinic/rest/advice/ExceptionControllerAdvice.java

@@ -86,7 +86,11 @@ public ResponseEntity<ProblemDetail> handleGeneralException(Exception e, HttpSer
     @ResponseBody
     public ResponseEntity<ProblemDetail> handleDataIntegrityViolationException(DataIntegrityViolationException ex, HttpServletRequest request) {
         HttpStatus status = HttpStatus.NOT_FOUND;
-        ProblemDetail detail = this.detailBuild(ex, status, request.getRequestURL());
+        ProblemDetail detail = ProblemDetail.forStatus(status);
+        detail.setType(URI.create(request.getRequestURL().toString()));
+        detail.setTitle(ex.getClass().getSimpleName());
+        detail.setDetail("Request could not be processed");
+        detail.setProperty("timestamp", Instant.now());
         return ResponseEntity.status(status).body(detail);
     }
 

src/main/java/org/springframework/samples/petclinic/rest/controller/OwnerRestController.java

@@ -134,9 +134,12 @@ public ResponseEntity<OwnerDto> deleteOwner(Integer ownerId) {
     @PreAuthorize("hasRole(@roles.OWNER_ADMIN)")
     @Override
     public ResponseEntity<PetDto> addPetToOwner(Integer ownerId, PetFieldsDto petFieldsDto) {
+        Owner owner = this.clinicService.findOwnerById(ownerId);
+        if (owner == null) {
+            return new ResponseEntity<>(HttpStatus.NOT_FOUND);
+        }
         HttpHeaders headers = new HttpHeaders();
         Pet pet = petMapper.toPet(petFieldsDto);
-        Owner owner = new Owner();
         owner.setId(ownerId);
         pet.setOwner(owner);
         pet.getType().setName(null);

src/test/java/org/springframework/samples/petclinic/rest/controller/OwnerRestControllerTests.java

@@ -16,6 +16,7 @@
 
 package org.springframework.samples.petclinic.rest.controller;
 
+import org.springframework.dao.DataIntegrityViolationException;
 import tools.jackson.databind.ObjectMapper;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -47,7 +48,9 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.doThrow;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
 
@@ -341,6 +344,8 @@ void testDeleteOwnerError() throws Exception {
     @Test
     @WithMockUser(roles = "OWNER_ADMIN")
     void testCreatePetSuccess() throws Exception {
+        final Owner owner = ownerMapper.toOwner(owners.get(0));
+        given(this.clinicService.findOwnerById(1)).willReturn(owner);
         PetDto newPet = pets.get(0);
         newPet.setId(999);
         ObjectMapper mapper =  JsonMapper.builder()
@@ -368,6 +373,40 @@ void testCreatePetError() throws Exception {
             .andExpect(status().isBadRequest()).andDo(MockMvcResultHandlers.print());
     }
 
+    @Test
+    @WithMockUser(roles = "OWNER_ADMIN")
+    void testCreatePetShouldNotExposeTechnicalDetails() throws Exception {
+        PetDto newPet = pets.get(0);
+        newPet.setId(null);
+        ObjectMapper mapper =  JsonMapper.builder()
+            .defaultDateFormat(new SimpleDateFormat("dd/MM/yyyy"))
+            .build();
+        String newPetAsJSON = mapper.writeValueAsString(newPet);
+        String technicalMessage = "could not execute statement; SQL [insert into pets ...]; constraint [fk_pet_owner]";
+        given(this.clinicService.findOwnerById(1)).willReturn(ownerMapper.toOwner(owners.get(0)));
+        doThrow(new DataIntegrityViolationException(technicalMessage)).when(this.clinicService).savePet(any());
+        this.mockMvc.perform(post("/api/owners/1/pets")
+                .content(newPetAsJSON).accept(MediaType.APPLICATION_JSON_VALUE).contentType(MediaType.APPLICATION_JSON_VALUE))
+            .andDo(MockMvcResultHandlers.print())
+            .andExpect(status().isNotFound())
+            .andExpect(jsonPath("$.detail").value("Request could not be processed"));
+    }
+
+    @Test
+    @WithMockUser(roles = "OWNER_ADMIN")
+    void testCreatePetWithUnknownOwnerShouldReturnNotFound() throws Exception {
+        PetDto newPet = pets.get(0);
+        newPet.setId(null);
+        ObjectMapper mapper = JsonMapper.builder()
+            .defaultDateFormat(new SimpleDateFormat("dd/MM/yyyy"))
+            .build();
+        String newPetAsJSON = mapper.writeValueAsString(newPet);
+        given(this.clinicService.findOwnerById(1000000)).willReturn(null);
+        this.mockMvc.perform(post("/api/owners/1000000/pets")
+                .content(newPetAsJSON).accept(MediaType.APPLICATION_JSON_VALUE).contentType(MediaType.APPLICATION_JSON_VALUE))
+            .andExpect(status().isNotFound());
+    }
+
     @Test
     @WithMockUser(roles = "OWNER_ADMIN")
     void testCreateVisitSuccess() throws Exception {