From d19d633ef05766f6b1bf8e4d77ddc0eb60640ef6 Mon Sep 17 00:00:00 2001 From: Harkamal Randhawa Date: Tue, 14 Apr 2026 15:46:42 -0600 Subject: [PATCH 1/2] use openrouter/free model --- backend/src/main/resources/application.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/main/resources/application.yml b/backend/src/main/resources/application.yml index f16140f7..4f424b93 100644 --- a/backend/src/main/resources/application.yml +++ b/backend/src/main/resources/application.yml @@ -65,7 +65,7 @@ stripe: openrouter: api-key: ${OPENROUTER_API_KEY:} - model: ${OPENROUTER_MODEL:openai/gpt-oss-120b:free} + model: ${OPENROUTER_MODEL:openrouter/free} logging: level: -- 2.49.1 From 7194ba12e78ef22c3d73fb17053eeb4d2f9cdc09 Mon Sep 17 00:00:00 2001 From: Harkamal Randhawa Date: Tue, 14 Apr 2026 16:09:39 -0600 Subject: [PATCH 2/2] Harden admin guards --- backend/.gitignore | 3 +- .../controller/CustomerController.java | 11 +- .../controller/EmployeeController.java | 9 +- .../petshop/backend/service/UserService.java | 111 ++++++++- .../backend/service/UserServiceTest.java | 218 ++++++++++++++++++ 5 files changed, 337 insertions(+), 15 deletions(-) create mode 100644 backend/src/test/java/com/petshop/backend/service/UserServiceTest.java diff --git a/backend/.gitignore b/backend/.gitignore index 0989bc79..c2a0b5de 100644 --- a/backend/.gitignore +++ b/backend/.gitignore @@ -44,7 +44,8 @@ build/ .env ### Project Specific ### -src/test/ +!src/test/ +!src/test/** tmp/ uploads/* !uploads/avatars/ diff --git a/backend/src/main/java/com/petshop/backend/controller/CustomerController.java b/backend/src/main/java/com/petshop/backend/controller/CustomerController.java index 4f17dd4f..534fc06e 100644 --- a/backend/src/main/java/com/petshop/backend/controller/CustomerController.java +++ b/backend/src/main/java/com/petshop/backend/controller/CustomerController.java @@ -3,6 +3,7 @@ package com.petshop.backend.controller; import com.petshop.backend.dto.common.BulkDeleteRequest; import com.petshop.backend.dto.user.UserRequest; import com.petshop.backend.dto.user.UserResponse; +import com.petshop.backend.entity.User; import com.petshop.backend.service.UserService; import jakarta.validation.Valid; import org.springframework.data.domain.Page; @@ -32,30 +33,30 @@ public class CustomerController { @GetMapping("/{id}") public ResponseEntity getCustomerById(@PathVariable Long id) { - return ResponseEntity.ok(userService.getUserById(id)); + return ResponseEntity.ok(userService.getUserById(id, User.Role.CUSTOMER)); } @PostMapping public ResponseEntity createCustomer(@Valid @RequestBody UserRequest request) { - return ResponseEntity.status(HttpStatus.CREATED).body(userService.createUser(request)); + return ResponseEntity.status(HttpStatus.CREATED).body(userService.createUser(request, User.Role.CUSTOMER)); } @PutMapping("/{id}") public ResponseEntity updateCustomer( @PathVariable Long id, @Valid @RequestBody UserRequest request) { - return ResponseEntity.ok(userService.updateUser(id, request)); + return ResponseEntity.ok(userService.updateUser(id, request, User.Role.CUSTOMER)); } @DeleteMapping("/{id}") public ResponseEntity deleteCustomer(@PathVariable Long id) { - userService.deleteUser(id); + userService.deleteUser(id, User.Role.CUSTOMER); return ResponseEntity.noContent().build(); } @PostMapping("/bulk-delete") public ResponseEntity bulkDeleteCustomers(@Valid @RequestBody BulkDeleteRequest request) { - userService.bulkDeleteUsers(request); + userService.bulkDeleteUsers(request, User.Role.CUSTOMER); return ResponseEntity.noContent().build(); } } diff --git a/backend/src/main/java/com/petshop/backend/controller/EmployeeController.java b/backend/src/main/java/com/petshop/backend/controller/EmployeeController.java index 2276e6ad..05cc8553 100644 --- a/backend/src/main/java/com/petshop/backend/controller/EmployeeController.java +++ b/backend/src/main/java/com/petshop/backend/controller/EmployeeController.java @@ -2,6 +2,7 @@ package com.petshop.backend.controller; import com.petshop.backend.dto.user.UserRequest; import com.petshop.backend.dto.user.UserResponse; +import com.petshop.backend.entity.User; import com.petshop.backend.service.UserService; import jakarta.validation.Valid; import org.springframework.data.domain.Page; @@ -31,24 +32,24 @@ public class EmployeeController { @GetMapping("/{id}") public ResponseEntity getEmployeeById(@PathVariable Long id) { - return ResponseEntity.ok(userService.getUserById(id)); + return ResponseEntity.ok(userService.getUserById(id, User.Role.STAFF)); } @PostMapping public ResponseEntity createEmployee(@Valid @RequestBody UserRequest request) { - return ResponseEntity.status(HttpStatus.CREATED).body(userService.createUser(request)); + return ResponseEntity.status(HttpStatus.CREATED).body(userService.createUser(request, User.Role.STAFF)); } @PutMapping("/{id}") public ResponseEntity updateEmployee( @PathVariable Long id, @Valid @RequestBody UserRequest request) { - return ResponseEntity.ok(userService.updateUser(id, request)); + return ResponseEntity.ok(userService.updateUser(id, request, User.Role.STAFF)); } @DeleteMapping("/{id}") public ResponseEntity deleteEmployee(@PathVariable Long id) { - userService.deleteUser(id); + userService.deleteUser(id, User.Role.STAFF); return ResponseEntity.noContent().build(); } } diff --git a/backend/src/main/java/com/petshop/backend/service/UserService.java b/backend/src/main/java/com/petshop/backend/service/UserService.java index 61c6f096..f1aa848b 100644 --- a/backend/src/main/java/com/petshop/backend/service/UserService.java +++ b/backend/src/main/java/com/petshop/backend/service/UserService.java @@ -3,21 +3,27 @@ package com.petshop.backend.service; import com.petshop.backend.dto.common.BulkDeleteRequest; import com.petshop.backend.dto.user.UserRequest; import com.petshop.backend.dto.user.UserResponse; -import com.petshop.backend.repository.ActivityLogRepository; import com.petshop.backend.entity.StoreLocation; import com.petshop.backend.entity.User; +import com.petshop.backend.repository.ActivityLogRepository; import com.petshop.backend.exception.ResourceNotFoundException; import com.petshop.backend.repository.StoreRepository; import com.petshop.backend.repository.UserRepository; +import com.petshop.backend.util.AuthenticationHelper; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; +import org.springframework.security.access.AccessDeniedException; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; import org.springframework.web.server.ResponseStatusException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; import java.util.Locale; import java.util.Objects; +import java.util.Set; import static org.springframework.http.HttpStatus.BAD_REQUEST; import static org.springframework.http.HttpStatus.CONFLICT; @@ -54,13 +60,26 @@ public class UserService { } public UserResponse getUserById(Long id) { + return getUserById(id, null); + } + + @Transactional(readOnly = true) + public UserResponse getUserById(Long id, User.Role requiredRole) { User user = userRepository.findById(id) .orElseThrow(() -> new ResourceNotFoundException("User not found with id: " + id)); + requireRoleOrNotFound(user, requiredRole, id); return mapToResponse(user); } @Transactional public UserResponse createUser(UserRequest request) { + return createUser(request, null); + } + + @Transactional + public UserResponse createUser(UserRequest request, User.Role requiredRole) { + requireRequestedRole(request.getRole(), requiredRole); + User user = new User(); user.setUsername(trimToNull(request.getUsername())); if (request.getPassword() != null && !request.getPassword().trim().isEmpty()) { @@ -88,9 +107,18 @@ public class UserService { @Transactional public UserResponse updateUser(Long id, UserRequest request) { + return updateUser(id, request, null); + } + + @Transactional + public UserResponse updateUser(Long id, UserRequest request, User.Role requiredRole) { User user = userRepository.findById(id) .orElseThrow(() -> new ResourceNotFoundException("User not found with id: " + id)); + requireRoleOrNotFound(user, requiredRole, id); + requireRequestedRole(request.getRole(), requiredRole); + requireAdminMutationAllowed(user, request.getRole()); + boolean invalidateToken = !Objects.equals(user.getUsername(), request.getUsername()) || user.getRole() != request.getRole() @@ -127,9 +155,17 @@ public class UserService { @Transactional public void deleteUser(Long id) { - if (!userRepository.existsById(id)) { - throw new ResourceNotFoundException("User not found with id: " + id); - } + deleteUser(id, null); + } + + @Transactional + public void deleteUser(Long id, User.Role requiredRole) { + User user = userRepository.findById(id) + .orElseThrow(() -> new ResourceNotFoundException("User not found with id: " + id)); + + requireRoleOrNotFound(user, requiredRole, id); + requireAdminDeletionAllowed(user); + if (activityLogRepository.existsByUser_Id(id)) { throw new ResponseStatusException(CONFLICT, "User cannot be deleted because activity logs exist"); } @@ -138,7 +174,27 @@ public class UserService { @Transactional public void bulkDeleteUsers(BulkDeleteRequest request) { - if (request.getIds() != null && request.getIds().stream().anyMatch(activityLogRepository::existsByUser_Id)) { + bulkDeleteUsers(request, null); + } + + @Transactional + public void bulkDeleteUsers(BulkDeleteRequest request, User.Role requiredRole) { + if (request.getIds() == null || request.getIds().isEmpty()) { + throw new ResponseStatusException(BAD_REQUEST, "IDs list cannot be empty"); + } + + Set requestedIds = new HashSet<>(request.getIds()); + ArrayList usersToDelete = new ArrayList<>(); + userRepository.findAllById(requestedIds).forEach(usersToDelete::add); + + if (usersToDelete.size() != requestedIds.size()) { + throw new ResourceNotFoundException("One or more users not found for bulk delete"); + } + + requireRoleOrNotFound(usersToDelete, requiredRole); + requireAdminDeletionAllowed(usersToDelete); + + if (request.getIds().stream().anyMatch(activityLogRepository::existsByUser_Id)) { throw new ResponseStatusException(CONFLICT, "One or more users cannot be deleted because activity logs exist"); } userRepository.deleteAllById(request.getIds()); @@ -169,6 +225,51 @@ public class UserService { .orElseThrow(() -> new ResourceNotFoundException("Store not found with id: " + storeId)); } + private void requireRoleOrNotFound(User user, User.Role requiredRole, Long id) { + if (requiredRole != null && user.getRole() != requiredRole) { + throw new ResourceNotFoundException("User not found with id: " + id); + } + } + + private void requireRoleOrNotFound(Collection users, User.Role requiredRole) { + if (requiredRole == null) { + return; + } + for (User user : users) { + if (user.getRole() != requiredRole) { + throw new ResourceNotFoundException("User not found with id: " + user.getId()); + } + } + } + + private void requireRequestedRole(User.Role requestedRole, User.Role requiredRole) { + if (requiredRole != null && requestedRole != requiredRole) { + throw new AccessDeniedException("Target user must have role " + requiredRole.name()); + } + } + + private void requireAdminMutationAllowed(User target, User.Role requestedRole) { + User actor = AuthenticationHelper.getAuthenticatedUser(userRepository); + if ((target.getRole() == User.Role.ADMIN && !actor.getId().equals(target.getId())) + || (requestedRole == User.Role.ADMIN && !actor.getId().equals(target.getId()))) { + throw new AccessDeniedException("Admins cannot modify other admin accounts"); + } + } + + private void requireAdminDeletionAllowed(User target) { + if (target.getRole() == User.Role.ADMIN) { + throw new AccessDeniedException("Admins cannot delete admin accounts"); + } + } + + private void requireAdminDeletionAllowed(Collection targets) { + for (User target : targets) { + if (target.getRole() == User.Role.ADMIN) { + throw new AccessDeniedException("Admins cannot delete admin accounts"); + } + } + } + private void validateUniquePhone(String phone, Long currentUserId) { if (phone == null || phone.isBlank()) { return; diff --git a/backend/src/test/java/com/petshop/backend/service/UserServiceTest.java b/backend/src/test/java/com/petshop/backend/service/UserServiceTest.java new file mode 100644 index 00000000..aa187b09 --- /dev/null +++ b/backend/src/test/java/com/petshop/backend/service/UserServiceTest.java @@ -0,0 +1,218 @@ +package com.petshop.backend.service; + +import com.petshop.backend.dto.common.BulkDeleteRequest; +import com.petshop.backend.dto.user.UserRequest; +import com.petshop.backend.dto.user.UserResponse; +import com.petshop.backend.exception.ResourceNotFoundException; +import com.petshop.backend.entity.User; +import com.petshop.backend.repository.ActivityLogRepository; +import com.petshop.backend.repository.StoreRepository; +import com.petshop.backend.repository.UserRepository; +import com.petshop.backend.security.AppPrincipal; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.access.AccessDeniedException; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.crypto.password.PasswordEncoder; + +import java.util.List; +import java.util.HashSet; +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class UserServiceTest { + + @Mock private UserRepository userRepository; + @Mock private ActivityLogRepository activityLogRepository; + @Mock private PasswordEncoder passwordEncoder; + @Mock private StoreRepository storeRepository; + + private UserService userService; + + @BeforeEach + void setUp() { + userService = new UserService(userRepository, activityLogRepository, passwordEncoder, storeRepository); + } + + @AfterEach + void tearDown() { + SecurityContextHolder.clearContext(); + } + + @Test + void updateUserDeniesEditingAnotherAdmin() { + User actor = user(1L, User.Role.ADMIN); + User target = user(2L, User.Role.ADMIN); + authenticate(actor); + + when(userRepository.findById(2L)).thenReturn(Optional.of(target)); + when(userRepository.findById(1L)).thenReturn(Optional.of(actor)); + + UserRequest request = request(User.Role.ADMIN); + + assertThrows(AccessDeniedException.class, () -> userService.updateUser(2L, request)); + verify(userRepository, never()).save(any(User.class)); + } + + @Test + void updateUserTreatsWrongScopedRoleAsNotFound() { + User actor = user(1L, User.Role.ADMIN); + User target = user(2L, User.Role.ADMIN); + authenticate(actor); + + when(userRepository.findById(2L)).thenReturn(Optional.of(target)); + + UserRequest request = request(User.Role.CUSTOMER); + + assertThrows(ResourceNotFoundException.class, () -> userService.updateUser(2L, request, User.Role.CUSTOMER)); + verify(userRepository, never()).save(any(User.class)); + } + + @Test + void deleteUserDeniesDeletingAnotherAdmin() { + User actor = user(1L, User.Role.ADMIN); + User target = user(2L, User.Role.ADMIN); + authenticate(actor); + + when(userRepository.findById(2L)).thenReturn(Optional.of(target)); + + assertThrows(AccessDeniedException.class, () -> userService.deleteUser(2L)); + verify(userRepository, never()).deleteById(anyLong()); + } + + @Test + void deleteUserDeniesDeletingSelfAdminAccount() { + User actor = user(1L, User.Role.ADMIN); + authenticate(actor); + + when(userRepository.findById(1L)).thenReturn(Optional.of(actor)); + + assertThrows(AccessDeniedException.class, () -> userService.deleteUser(1L)); + verify(userRepository, never()).deleteById(anyLong()); + } + + @Test + void bulkDeleteUsersDeniesMixedAdminTargets() { + User targetAdmin = user(2L, User.Role.ADMIN); + User customer = user(3L, User.Role.CUSTOMER); + + when(userRepository.findAllById(eq(new HashSet<>(List.of(2L, 3L))))).thenReturn(List.of(targetAdmin, customer)); + + BulkDeleteRequest request = new BulkDeleteRequest(); + request.setIds(List.of(2L, 3L)); + + assertThrows(AccessDeniedException.class, () -> userService.bulkDeleteUsers(request)); + verify(userRepository, never()).deleteAllById(any()); + } + + @Test + void bulkDeleteUsersThrowsWhenAnyIdMissing() { + User customer = user(2L, User.Role.CUSTOMER); + + when(userRepository.findAllById(any())).thenReturn(List.of(customer)); + + BulkDeleteRequest request = new BulkDeleteRequest(); + request.setIds(List.of(2L, 3L)); + + assertThrows(ResourceNotFoundException.class, () -> userService.bulkDeleteUsers(request, User.Role.CUSTOMER)); + verify(userRepository, never()).deleteAllById(any()); + } + + @Test + void createUserDeniesWrongRoleForScopedRoute() { + UserRequest request = request(User.Role.ADMIN); + + assertThrows(AccessDeniedException.class, () -> userService.createUser(request, User.Role.CUSTOMER)); + verify(userRepository, never()).save(any(User.class)); + } + + @Test + void updateUserAllowsEditingOwnAdminAccount() { + User actor = user(1L, User.Role.ADMIN); + authenticate(actor); + + when(userRepository.findById(1L)).thenReturn(Optional.of(actor)); + when(userRepository.save(any(User.class))).thenAnswer(invocation -> invocation.getArgument(0)); + + UserRequest request = request(User.Role.ADMIN); + request.setFirstName("Updated"); + + UserResponse response = userService.updateUser(1L, request); + assertEquals("Updated", response.getFirstName()); + } + + @Test + void updateUserDeniesPromotingAnotherUserToAdmin() { + User actor = user(1L, User.Role.ADMIN); + User target = user(2L, User.Role.CUSTOMER); + authenticate(actor); + + when(userRepository.findById(2L)).thenReturn(Optional.of(target)); + when(userRepository.findById(1L)).thenReturn(Optional.of(actor)); + + UserRequest request = request(User.Role.ADMIN); + + assertThrows(AccessDeniedException.class, () -> userService.updateUser(2L, request)); + verify(userRepository, never()).save(any(User.class)); + } + + @Test + void scopedUpdateDeniesRoleEscalation() { + User actor = user(1L, User.Role.ADMIN); + User target = user(2L, User.Role.CUSTOMER); + authenticate(actor); + + when(userRepository.findById(2L)).thenReturn(Optional.of(target)); + + UserRequest request = request(User.Role.ADMIN); + + assertThrows(AccessDeniedException.class, () -> userService.updateUser(2L, request, User.Role.CUSTOMER)); + verify(userRepository, never()).save(any(User.class)); + } + + private void authenticate(User user) { + AppPrincipal principal = new AppPrincipal(user.getId(), user.getUsername(), user.getRole(), user.getTokenVersion()); + var context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(new UsernamePasswordAuthenticationToken(principal, "n/a", principal.getAuthorities())); + SecurityContextHolder.setContext(context); + } + + private User user(Long id, User.Role role) { + User user = new User(); + user.setId(id); + user.setUsername("user" + id); + user.setFirstName("First"); + user.setLastName("Last"); + user.setEmail("user" + id + "@example.com"); + user.setRole(role); + user.setActive(true); + user.setTokenVersion(0); + return user; + } + + private UserRequest request(User.Role role) { + UserRequest request = new UserRequest(); + request.setUsername("updated-user"); + request.setFirstName("First"); + request.setLastName("Last"); + request.setEmail("updated@example.com"); + request.setRole(role); + request.setActive(true); + return request; + } + +} -- 2.49.1