From 8d3430bd757bcffa97def574d62c1dc2990c1e6f Mon Sep 17 00:00:00 2001 From: Harkamal Randhawa Date: Sun, 5 Apr 2026 23:35:05 -0600 Subject: [PATCH] Enforce pet ownership rules --- .../controller/DropdownController.java | 9 +--- .../backend/service/AppointmentService.java | 8 ++-- .../controller/DropdownControllerTest.java | 12 ++--- .../service/AppointmentServiceTest.java | 48 +------------------ 4 files changed, 10 insertions(+), 67 deletions(-) diff --git a/backend/src/main/java/com/petshop/backend/controller/DropdownController.java b/backend/src/main/java/com/petshop/backend/controller/DropdownController.java index 2217b4e7..409891bc 100644 --- a/backend/src/main/java/com/petshop/backend/controller/DropdownController.java +++ b/backend/src/main/java/com/petshop/backend/controller/DropdownController.java @@ -80,15 +80,8 @@ public class DropdownController { @GetMapping("/appointment-customers") @PreAuthorize("hasAnyRole('STAFF', 'ADMIN')") public ResponseEntity> getAppointmentCustomers() { - User user = com.petshop.backend.util.AuthenticationHelper.getAuthenticatedUser(userRepository); - List customers; - if (user.getRole() == User.Role.ADMIN) { - customers = customerRepository.findAll(); - } else { - customers = customerRepository.findAllWithPets(); - } return ResponseEntity.ok( - customers.stream() + customerRepository.findAllWithPets().stream() .map(c -> new DropdownOption(c.getCustomerId(), c.getFirstName() + " " + c.getLastName())) .collect(Collectors.toList()) ); diff --git a/backend/src/main/java/com/petshop/backend/service/AppointmentService.java b/backend/src/main/java/com/petshop/backend/service/AppointmentService.java index 363cb3a0..8c0e05d8 100644 --- a/backend/src/main/java/com/petshop/backend/service/AppointmentService.java +++ b/backend/src/main/java/com/petshop/backend/service/AppointmentService.java @@ -122,7 +122,7 @@ public class AppointmentService { } Set pets = hasPetIds ? fetchPets(request.getPetIds()) : new HashSet<>(); - Set customerPets = hasCustomerPetIds ? fetchCustomerPets(request.getCustomerPetIds(), customer.getCustomerId(), authenticatedUser.getRole()) : new HashSet<>(); + Set customerPets = hasCustomerPetIds ? fetchCustomerPets(request.getCustomerPetIds(), customer.getCustomerId()) : new HashSet<>(); Employee employee = resolveAppointmentEmployee(request.getEmployeeId(), store.getStoreId()); Appointment appointment = new Appointment(); @@ -170,7 +170,7 @@ public class AppointmentService { } Set pets = hasPetIds ? fetchPets(request.getPetIds()) : new HashSet<>(); - Set customerPets = hasCustomerPetIds ? fetchCustomerPets(request.getCustomerPetIds(), customer.getCustomerId(), authenticatedUser.getRole()) : new HashSet<>(); + Set customerPets = hasCustomerPetIds ? fetchCustomerPets(request.getCustomerPetIds(), customer.getCustomerId()) : new HashSet<>(); Employee employee = resolveAppointmentEmployee(request.getEmployeeId(), store.getStoreId()); appointment.setCustomer(customer); @@ -255,12 +255,12 @@ public class AppointmentService { return pets; } - private Set fetchCustomerPets(List customerPetIds, Long customerId, User.Role authenticatedRole) { + private Set fetchCustomerPets(List customerPetIds, Long customerId) { Set customerPets = new HashSet<>(); for (Long customerPetId : customerPetIds) { CustomerPet customerPet = customerPetRepository.findById(customerPetId) .orElseThrow(() -> new ResourceNotFoundException("Customer pet not found with id: " + customerPetId)); - if (authenticatedRole != User.Role.ADMIN && !customerPet.getCustomer().getCustomerId().equals(customerId)) { + if (!customerPet.getCustomer().getCustomerId().equals(customerId)) { throw new IllegalArgumentException("Selected pet does not belong to the selected customer"); } customerPets.add(customerPet); diff --git a/backend/src/test/java/com/petshop/backend/controller/DropdownControllerTest.java b/backend/src/test/java/com/petshop/backend/controller/DropdownControllerTest.java index 563e5d2f..fa0a0ebe 100644 --- a/backend/src/test/java/com/petshop/backend/controller/DropdownControllerTest.java +++ b/backend/src/test/java/com/petshop/backend/controller/DropdownControllerTest.java @@ -173,7 +173,7 @@ class DropdownControllerTest { } @Test - void getAppointmentCustomersReturnsAllCustomersForAdmin() { + void getAppointmentCustomersReturnsOnlyCustomersWithPetsForAdmin() { User adminUser = new User(); adminUser.setId(88L); adminUser.setRole(User.Role.ADMIN); @@ -185,17 +185,11 @@ class DropdownControllerTest { one.setFirstName("Alex"); one.setLastName("Brown"); - Customer two = new Customer(); - two.setCustomerId(2L); - two.setFirstName("Emily"); - two.setLastName("Clark"); - - when(customerRepository.findAll()).thenReturn(List.of(one, two)); + when(customerRepository.findAllWithPets()).thenReturn(List.of(one)); var response = controller.getAppointmentCustomers(); - assertEquals(2, response.getBody().size()); + assertEquals(1, response.getBody().size()); assertEquals(Long.valueOf(1L), response.getBody().get(0).getId()); - assertEquals(Long.valueOf(2L), response.getBody().get(1).getId()); } } diff --git a/backend/src/test/java/com/petshop/backend/service/AppointmentServiceTest.java b/backend/src/test/java/com/petshop/backend/service/AppointmentServiceTest.java index d4892126..aecb5644 100644 --- a/backend/src/test/java/com/petshop/backend/service/AppointmentServiceTest.java +++ b/backend/src/test/java/com/petshop/backend/service/AppointmentServiceTest.java @@ -188,7 +188,7 @@ class AppointmentServiceTest { } @Test - void createAppointmentAllowsCustomerPetOwnedByDifferentCustomerForAdmin() { + void createAppointmentRejectsCustomerPetOwnedByDifferentCustomer() { setAuthentication(99L, User.Role.ADMIN); Customer otherCustomer = new Customer(); @@ -206,11 +206,6 @@ class AppointmentServiceTest { .thenReturn(List.of(new EmployeeStore(employee, store))); when(appointmentRepository.findByStoreAndDate(1L, date)).thenReturn(List.of()); when(customerPetRepository.findById(22L)).thenReturn(Optional.of(otherCustomerPet)); - when(appointmentRepository.save(any(Appointment.class))).thenAnswer(invocation -> { - Appointment appt = invocation.getArgument(0); - appt.setAppointmentId(101L); - return appt; - }); var request = new com.petshop.backend.dto.appointment.AppointmentRequest(); request.setCustomerId(1L); @@ -221,46 +216,7 @@ class AppointmentServiceTest { request.setAppointmentStatus("Booked"); request.setCustomerPetIds(List.of(22L)); - var response = appointmentService.createAppointment(request); - assertEquals(101L, response.getAppointmentId()); - } - - @Test - void createAppointmentAllowsAnyPetForAdmin() { - setAuthentication(99L, User.Role.ADMIN); - - Customer otherCustomer = new Customer(); - otherCustomer.setCustomerId(22L); - CustomerPet otherCustomerPet = new CustomerPet(); - otherCustomerPet.setCustomerPetId(22L); - otherCustomerPet.setCustomer(otherCustomer); - - when(customerRepository.findById(1L)).thenReturn(Optional.of(customer)); - when(storeRepository.findById(1L)).thenReturn(Optional.of(store)); - when(serviceRepository.findById(1L)).thenReturn(Optional.of(grooming)); - when(employeeStoreRepository.findActiveByStoreStoreIdOrderByEmployeeEmployeeIdAsc(1L)) - .thenReturn(List.of(new EmployeeStore(employee, store))); - when(appointmentRepository.findByStoreAndDate(1L, date)).thenReturn(List.of()); - when(customerPetRepository.findById(22L)).thenReturn(Optional.of(otherCustomerPet)); - when(appointmentRepository.save(any(Appointment.class))).thenAnswer(invocation -> { - Appointment appointment = invocation.getArgument(0); - appointment.setAppointmentId(101L); - return appointment; - }); - - var request = new com.petshop.backend.dto.appointment.AppointmentRequest(); - request.setCustomerId(1L); - request.setStoreId(1L); - request.setServiceId(1L); - request.setAppointmentDate(date); - request.setAppointmentTime(LocalTime.of(10, 0)); - request.setAppointmentStatus("Booked"); - request.setCustomerPetIds(List.of(22L)); - - var response = appointmentService.createAppointment(request); - - assertEquals(101L, response.getAppointmentId()); - assertEquals(1L, response.getCustomerId()); + assertThrows(IllegalArgumentException.class, () -> appointmentService.createAppointment(request)); } @Test