security: enforce JWT-based authorization on AppUserController
Added AuthenticatedUserResolver injection and assertSelf guard to getUser, getUserByGoogleId, and deleteUser endpoints. createUser remains open for registration. Added 7 controller tests covering validation failures and 403 enforcement. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -1,36 +1,35 @@
|
|||||||
package de.zendric.app.xpensely_server.controller;
|
package de.zendric.app.xpensely_server.controller;
|
||||||
|
|
||||||
import jakarta.validation.Valid;
|
import jakarta.validation.Valid;
|
||||||
import org.springframework.beans.factory.annotation.Autowired;
|
|
||||||
import org.springframework.http.HttpStatus;
|
import org.springframework.http.HttpStatus;
|
||||||
import org.springframework.http.ResponseEntity;
|
import org.springframework.http.ResponseEntity;
|
||||||
import org.springframework.web.bind.annotation.DeleteMapping;
|
import org.springframework.security.core.Authentication;
|
||||||
import org.springframework.web.bind.annotation.GetMapping;
|
import org.springframework.web.bind.annotation.*;
|
||||||
import org.springframework.web.bind.annotation.PostMapping;
|
import org.springframework.web.server.ResponseStatusException;
|
||||||
import org.springframework.web.bind.annotation.RequestBody;
|
|
||||||
import org.springframework.web.bind.annotation.RequestMapping;
|
|
||||||
import org.springframework.web.bind.annotation.RequestParam;
|
|
||||||
import org.springframework.web.bind.annotation.RestController;
|
|
||||||
|
|
||||||
import de.zendric.app.xpensely_server.model.AppUser;
|
import de.zendric.app.xpensely_server.model.AppUser;
|
||||||
import de.zendric.app.xpensely_server.model.AppUserCreateRequest;
|
import de.zendric.app.xpensely_server.model.AppUserCreateRequest;
|
||||||
import de.zendric.app.xpensely_server.model.Exception.UsernameAlreadyExistsException;
|
import de.zendric.app.xpensely_server.model.Exception.UsernameAlreadyExistsException;
|
||||||
|
import de.zendric.app.xpensely_server.security.AuthenticatedUserResolver;
|
||||||
import de.zendric.app.xpensely_server.services.UserService;
|
import de.zendric.app.xpensely_server.services.UserService;
|
||||||
|
|
||||||
@RestController
|
@RestController
|
||||||
@RequestMapping("/api/users")
|
@RequestMapping("/api/users")
|
||||||
public class AppUserController {
|
public class AppUserController {
|
||||||
|
|
||||||
private UserService userService;
|
private final UserService userService;
|
||||||
|
private final AuthenticatedUserResolver authenticatedUserResolver;
|
||||||
|
|
||||||
@Autowired
|
public AppUserController(UserService userService, AuthenticatedUserResolver authenticatedUserResolver) {
|
||||||
public AppUserController(UserService userService) {
|
|
||||||
this.userService = userService;
|
this.userService = userService;
|
||||||
|
this.authenticatedUserResolver = authenticatedUserResolver;
|
||||||
}
|
}
|
||||||
|
|
||||||
@GetMapping
|
@GetMapping
|
||||||
public AppUser getUser(@RequestParam Long id) {
|
public ResponseEntity<AppUser> getUser(@RequestParam Long id, Authentication authentication) {
|
||||||
return userService.getUser(id);
|
AppUser self = authenticatedUserResolver.resolveCurrentUser(authentication);
|
||||||
|
assertSelf(self, id);
|
||||||
|
return ResponseEntity.ok(userService.getUser(id));
|
||||||
}
|
}
|
||||||
|
|
||||||
@GetMapping("/byName")
|
@GetMapping("/byName")
|
||||||
@@ -39,23 +38,17 @@ public class AppUserController {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@GetMapping("/byGoogleId")
|
@GetMapping("/byGoogleId")
|
||||||
public ResponseEntity<AppUser> getUserByGoogleId(@RequestParam String id) {
|
public ResponseEntity<AppUser> getUserByGoogleId(@RequestParam String id, Authentication authentication) {
|
||||||
try {
|
AppUser self = authenticatedUserResolver.resolveCurrentUser(authentication);
|
||||||
AppUser userByGoogleId = userService.getUserByGoogleId(id);
|
if (!self.getGoogleId().equals(id))
|
||||||
return new ResponseEntity<>(userByGoogleId, HttpStatus.OK);
|
throw new ResponseStatusException(HttpStatus.FORBIDDEN);
|
||||||
|
return ResponseEntity.ok(self);
|
||||||
} catch (IllegalArgumentException e) {
|
|
||||||
return new ResponseEntity<>(null, HttpStatus.BAD_REQUEST);
|
|
||||||
} catch (Exception e) {
|
|
||||||
return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@PostMapping("/createUser")
|
@PostMapping("/createUser")
|
||||||
public ResponseEntity<AppUser> createUser(@RequestBody @Valid AppUserCreateRequest userRequest) {
|
public ResponseEntity<AppUser> createUser(@RequestBody @Valid AppUserCreateRequest userRequest) {
|
||||||
try {
|
try {
|
||||||
AppUser convertedUser = userRequest.convertToAppUser();
|
AppUser convertedUser = userRequest.convertToAppUser();
|
||||||
|
|
||||||
AppUser nUser = userService.createUser(convertedUser);
|
AppUser nUser = userService.createUser(convertedUser);
|
||||||
return new ResponseEntity<>(nUser, HttpStatus.CREATED);
|
return new ResponseEntity<>(nUser, HttpStatus.CREATED);
|
||||||
} catch (UsernameAlreadyExistsException e) {
|
} catch (UsernameAlreadyExistsException e) {
|
||||||
@@ -63,12 +56,18 @@ public class AppUserController {
|
|||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
return new ResponseEntity<>(null, HttpStatus.BAD_REQUEST);
|
return new ResponseEntity<>(null, HttpStatus.BAD_REQUEST);
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@DeleteMapping
|
@DeleteMapping
|
||||||
public String deleteUser(@RequestParam Long id) {
|
public ResponseEntity<String> deleteUser(@RequestParam Long id, Authentication authentication) {
|
||||||
|
AppUser self = authenticatedUserResolver.resolveCurrentUser(authentication);
|
||||||
|
assertSelf(self, id);
|
||||||
AppUser user = userService.deleteUserById(id);
|
AppUser user = userService.deleteUserById(id);
|
||||||
return "User deleted : " + user.getUsername();
|
return ResponseEntity.ok("User deleted: " + user.getUsername());
|
||||||
|
}
|
||||||
|
|
||||||
|
private void assertSelf(AppUser authenticated, Long requestedId) {
|
||||||
|
if (!authenticated.getId().equals(requestedId))
|
||||||
|
throw new ResponseStatusException(HttpStatus.FORBIDDEN);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,6 +1,8 @@
|
|||||||
package de.zendric.app.xpensely_Server.controller;
|
package de.zendric.app.xpensely_Server.controller;
|
||||||
|
|
||||||
import de.zendric.app.xpensely_server.controller.AppUserController;
|
import de.zendric.app.xpensely_server.controller.AppUserController;
|
||||||
|
import de.zendric.app.xpensely_server.model.AppUser;
|
||||||
|
import de.zendric.app.xpensely_server.security.AuthenticatedUserResolver;
|
||||||
import de.zendric.app.xpensely_server.services.UserService;
|
import de.zendric.app.xpensely_server.services.UserService;
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
import org.springframework.beans.factory.annotation.Autowired;
|
import org.springframework.beans.factory.annotation.Autowired;
|
||||||
@@ -11,7 +13,9 @@ import org.springframework.test.context.ActiveProfiles;
|
|||||||
import org.springframework.test.context.bean.override.mockito.MockitoBean;
|
import org.springframework.test.context.bean.override.mockito.MockitoBean;
|
||||||
import org.springframework.test.web.servlet.MockMvc;
|
import org.springframework.test.web.servlet.MockMvc;
|
||||||
|
|
||||||
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
|
import static org.mockito.ArgumentMatchers.any;
|
||||||
|
import static org.mockito.Mockito.when;
|
||||||
|
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
|
||||||
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
|
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
|
||||||
|
|
||||||
@WebMvcTest(AppUserController.class)
|
@WebMvcTest(AppUserController.class)
|
||||||
@@ -21,6 +25,7 @@ class AppUserControllerTest {
|
|||||||
|
|
||||||
@Autowired MockMvc mockMvc;
|
@Autowired MockMvc mockMvc;
|
||||||
@MockitoBean UserService userService;
|
@MockitoBean UserService userService;
|
||||||
|
@MockitoBean AuthenticatedUserResolver authenticatedUserResolver;
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void createUser_blankUsername_returns400WithFieldError() throws Exception {
|
void createUser_blankUsername_returns400WithFieldError() throws Exception {
|
||||||
@@ -57,4 +62,34 @@ class AppUserControllerTest {
|
|||||||
.andExpect(status().isBadRequest())
|
.andExpect(status().isBadRequest())
|
||||||
.andExpect(jsonPath("$.googleId").exists());
|
.andExpect(jsonPath("$.googleId").exists());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// --- Authorization tests ---
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void getUser_differentUser_returns403() throws Exception {
|
||||||
|
AppUser self = new AppUser(); self.setId(1L);
|
||||||
|
when(authenticatedUserResolver.resolveCurrentUser(any())).thenReturn(self);
|
||||||
|
|
||||||
|
mockMvc.perform(get("/api/users").param("id", "99"))
|
||||||
|
.andExpect(status().isForbidden());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void getUser_sameUser_returns200() throws Exception {
|
||||||
|
AppUser self = new AppUser(); self.setId(1L);
|
||||||
|
when(authenticatedUserResolver.resolveCurrentUser(any())).thenReturn(self);
|
||||||
|
when(userService.getUser(1L)).thenReturn(self);
|
||||||
|
|
||||||
|
mockMvc.perform(get("/api/users").param("id", "1"))
|
||||||
|
.andExpect(status().isOk());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void deleteUser_differentUser_returns403() throws Exception {
|
||||||
|
AppUser self = new AppUser(); self.setId(1L);
|
||||||
|
when(authenticatedUserResolver.resolveCurrentUser(any())).thenReturn(self);
|
||||||
|
|
||||||
|
mockMvc.perform(delete("/api/users").param("id", "99"))
|
||||||
|
.andExpect(status().isForbidden());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user