PUT /api/users/{id} and DELETE /api/users/{id} accept any authenticated caller for any id. Should verify that the
authenticated principal owns the id, or has admin role.
⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
Enforce ownership/role authorization on id-based mutation endpoints.
Line 45 and Line 52 accept any authenticated caller for any {id}. Without verifying that the authenticated principal owns that id (or has admin privileges), this is an IDOR authorization flaw.
Suggested direction
-@PutMapping("/{id}")
-public ResponseEntity<UserResponse> updateUser(`@PathVariable` UUID id, `@Valid` `@RequestBody` UserRequest request) {
+@PutMapping("/{id}")
+public ResponseEntity<UserResponse> updateUser(`@PathVariable` UUID id,
+ `@Valid` `@RequestBody` UserRequest request,
+ Authentication auth) {
+ if (!isSelfOrAdmin(id, auth)) {
+ return ResponseEntity.status(HttpStatus.FORBIDDEN).build();
+ }
return userService.updateUser(id, request)
.map(ResponseEntity::ok)
.orElse(ResponseEntity.notFound().build());
}
`@DeleteMapping`("/{id}")
-public ResponseEntity<Void> deleteUser(`@PathVariable` UUID id) {
+public ResponseEntity<Void> deleteUser(`@PathVariable` UUID id, Authentication auth) {
+ if (!isSelfOrAdmin(id, auth)) {
+ return ResponseEntity.status(HttpStatus.FORBIDDEN).build();
+ }
userService.deleteUser(id);
return ResponseEntity.noContent().build();
}
Also applies to: 51-55
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@userService/src/main/java/org/example/userservice/controller/UserController.java`
around lines 44 - 49, The updateUser endpoint in UserController allows any
authenticated caller to mutate any UUID id (IDOR); before calling
userService.updateUser(id, request) validate the authenticated principal (via
`@AuthenticationPrincipal` or SecurityContextHolder) and enforce ownership or
admin role: permit the update only if principal.getId().equals(id) or
principal.hasRole("ROLE_ADMIN"), otherwise return 403 Forbidden (or throw
AccessDeniedException); implement the same check for other id-based mutation
endpoints that call userService (referenced by updateUser and the
userService.updateUser call) to ensure proper authorization.
Originally posted by @coderabbitai[bot] in #12 (comment)
PUT /api/users/{id}andDELETE /api/users/{id}accept any authenticated caller for any id. Should verify that theauthenticated principal owns the id, or has admin role.
Enforce ownership/role authorization on id-based mutation endpoints.
Line 45 and Line 52 accept any authenticated caller for any
{id}. Without verifying that the authenticated principal owns that id (or has admin privileges), this is an IDOR authorization flaw.Suggested direction
Also applies to: 51-55
🤖 Prompt for AI Agents
Originally posted by @coderabbitai[bot] in #12 (comment)