RecentRunner/group-2-threaded-project-petshop
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Harden admin guards

Browse Source
This commit is contained in:
Harkamal Randhawa
2026-04-14 16:09:39 -06:00
parent 5964528524
commit 758b44d6d6
5 changed files with 337 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
backend/.gitignore vendored
View File

@@ -44,7 +44,8 @@ build/
.env
### Project Specific ###
src/test/
!src/test/
!src/test/**
tmp/
uploads/*
!uploads/avatars/

11
backend/src/main/java/com/petshop/backend/controller/CustomerController.java
View File

@@ -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<UserResponse> getCustomerById(@PathVariable Long id) {
return ResponseEntity.ok(userService.getUserById(id));
return ResponseEntity.ok(userService.getUserById(id, User.Role.CUSTOMER));
}
@PostMapping
public ResponseEntity<UserResponse> 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<UserResponse> 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<Void> deleteCustomer(@PathVariable Long id) {
userService.deleteUser(id);
userService.deleteUser(id, User.Role.CUSTOMER);
return ResponseEntity.noContent().build();
}
@PostMapping("/bulk-delete")
public ResponseEntity<Void> bulkDeleteCustomers(@Valid @RequestBody BulkDeleteRequest request) {
userService.bulkDeleteUsers(request);
userService.bulkDeleteUsers(request, User.Role.CUSTOMER);
return ResponseEntity.noContent().build();
}
}

9
backend/src/main/java/com/petshop/backend/controller/EmployeeController.java
View File

@@ -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<UserResponse> getEmployeeById(@PathVariable Long id) {
return ResponseEntity.ok(userService.getUserById(id));
return ResponseEntity.ok(userService.getUserById(id, User.Role.STAFF));
}
@PostMapping
public ResponseEntity<UserResponse> 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<UserResponse> 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<Void> deleteEmployee(@PathVariable Long id) {
userService.deleteUser(id);
userService.deleteUser(id, User.Role.STAFF);
return ResponseEntity.noContent().build();
}
}

109
backend/src/main/java/com/petshop/backend/service/UserService.java
View File

@@ -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<Long> requestedIds = new HashSet<>(request.getIds());
ArrayList<User> 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<User> 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<User> 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;

218
backend/src/test/java/com/petshop/backend/service/UserServiceTest.java Normal file
View File

@@ -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;
}
}