feat: add GlobalExceptionHandler, @Valid to user creation, AuthenticatedUserResolver stub, and rewrite ExpenseListController with authorization

2026-05-04 22:44:37 +02:00
parent 3bea06fead
commit a948bca2fc
5 changed files with 216 additions and 117 deletions
@@ -1,5 +1,6 @@
 package de.zendric.app.xpensely_server.controller;

+import jakarta.validation.Valid;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
@@ -51,7 +52,7 @@ public class AppUserController {
    }

    @PostMapping("/createUser")
-    public ResponseEntity<AppUser> createUser(@RequestBody AppUserCreateRequest userRequest) {
+    public ResponseEntity<AppUser> createUser(@RequestBody @Valid AppUserCreateRequest userRequest) {
        try {
            AppUser convertedUser = userRequest.convertToAppUser();

@@ -1,30 +1,18 @@
 package de.zendric.app.xpensely_server.controller;

 import java.time.LocalDateTime;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;

-import org.springframework.beans.factory.annotation.Autowired;
+import jakarta.validation.Valid;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
-import org.springframework.web.bind.annotation.DeleteMapping;
-import org.springframework.web.bind.annotation.GetMapping;
-import org.springframework.web.bind.annotation.PathVariable;
-import org.springframework.web.bind.annotation.PostMapping;
-import org.springframework.web.bind.annotation.PutMapping;
-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 org.springframework.security.core.Authentication;
+import org.springframework.web.bind.annotation.*;
+import org.springframework.web.server.ResponseStatusException;

-import de.zendric.app.xpensely_server.model.AppUser;
-import de.zendric.app.xpensely_server.model.Expense;
-import de.zendric.app.xpensely_server.model.ExpenseChangeRequest;
-import de.zendric.app.xpensely_server.model.ExpenseInput;
-import de.zendric.app.xpensely_server.model.ExpenseList;
-import de.zendric.app.xpensely_server.model.InviteRequest;
-import de.zendric.app.xpensely_server.model.XpenselyStandardCategories;
+import de.zendric.app.xpensely_server.model.*;
+import de.zendric.app.xpensely_server.security.AuthenticatedUserResolver;
 import de.zendric.app.xpensely_server.services.CategoryService;
 import de.zendric.app.xpensely_server.services.ExpenseListService;
 import de.zendric.app.xpensely_server.services.UserService;
@@ -33,93 +21,51 @@ import de.zendric.app.xpensely_server.services.UserService;
@RequestMapping("/api/expenselist")
 class ExpenseListController {

-    private ExpenseListService expenseListService;
-    private UserService userService;
-    private CategoryService categoryService;
+    private final ExpenseListService expenseListService;
+    private final UserService userService;
+    private final CategoryService categoryService;
+    private final AuthenticatedUserResolver authenticatedUserResolver;

-    @Autowired
    public ExpenseListController(ExpenseListService expenseListService, UserService userService,
-            CategoryService categoryService) {
+            CategoryService categoryService, AuthenticatedUserResolver authenticatedUserResolver) {
        this.expenseListService = expenseListService;
        this.userService = userService;
        this.categoryService = categoryService;
+        this.authenticatedUserResolver = authenticatedUserResolver;
    }

-    @GetMapping("/all")
-    public ResponseEntity<List<ExpenseList>> getAll() {
-        try {
-            List<ExpenseList> items = new ArrayList<>();
-
-            expenseListService.findAll().forEach(items::add);
-
+    @GetMapping("/mine")
+    public ResponseEntity<List<ExpenseList>> getMine(Authentication authentication) {
+        AppUser user = authenticatedUserResolver.resolveCurrentUser(authentication);
+        List<ExpenseList> items = expenseListService.findByUserId(user.getId());
        if (items.isEmpty())
            return new ResponseEntity<>(HttpStatus.NO_CONTENT);
-
        return new ResponseEntity<>(items, HttpStatus.OK);
-        } catch (Exception e) {
-            return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR);
-        }
-    }
-
-    @GetMapping("/byUser")
-    public ResponseEntity<List<ExpenseList>> getByUser(@RequestParam Long userId) {
-        try {
-            List<ExpenseList> items = expenseListService.findByUserId(userId);
-
-            if (items.isEmpty())
-                return new ResponseEntity<>(HttpStatus.NO_CONTENT);
-
-            return new ResponseEntity<>(items, HttpStatus.OK);
-        } catch (Exception e) {
-            return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR);
-        }
-    }
-
-    @GetMapping("/byUsername")
-    public ResponseEntity<List<ExpenseList>> getByUser(@RequestParam String username) {
-        try {
-            List<ExpenseList> items = expenseListService.findByUsername(username);
-
-            if (items.isEmpty())
-                return new ResponseEntity<>(HttpStatus.NO_CONTENT);
-
-            return new ResponseEntity<>(items, HttpStatus.OK);
-        } catch (Exception e) {
-            return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR);
-        }
    }

    @GetMapping("/byId")
-    public ResponseEntity<ExpenseList> getById(@RequestParam Long id) {
+    public ResponseEntity<ExpenseList> getById(@RequestParam Long id, Authentication authentication) {
+        AppUser user = authenticatedUserResolver.resolveCurrentUser(authentication);
        Optional<ExpenseList> existingItemOptional = expenseListService.findById(id);
-
-        if (existingItemOptional.isPresent()) {
-            return new ResponseEntity<>(existingItemOptional.get(), HttpStatus.OK);
-        } else {
+        if (existingItemOptional.isEmpty())
            return new ResponseEntity<>(HttpStatus.NOT_FOUND);
-        }
+        assertMember(user, existingItemOptional.get());
+        return new ResponseEntity<>(existingItemOptional.get(), HttpStatus.OK);
    }

    @PostMapping("/create")
-    // TODO add handling of categories by using DTO
-    public ResponseEntity<ExpenseList> create(@RequestBody ExpenseList expenseList) {
+    public ResponseEntity<ExpenseList> create(@RequestBody ExpenseList expenseList,
+            Authentication authentication) {
        try {
-            if (expenseList.getOwner() != null) {
-                AppUser existingOwner = userService.getUser(expenseList.getOwner().getId());
-                if (existingOwner == null) {
-                    throw new IllegalArgumentException("Owner does not exist.");
-                }
-                expenseList.setOwner(existingOwner);
+            AppUser authenticatedUser = authenticatedUserResolver.resolveCurrentUser(authentication);
+            expenseList.setOwner(authenticatedUser);
            XpenselyStandardCategories standardCategories = categoryService.getDefaultCategories();
            expenseList.setXpenselyStandardCategories(standardCategories);
-            } else {
-                throw new IllegalArgumentException("Owner is required.");
-            }
-
            expenseList.setSharedWith(null);
-
            ExpenseList savedItem = expenseListService.createList(expenseList);
            return new ResponseEntity<>(savedItem, HttpStatus.CREATED);
+        } catch (ResponseStatusException e) {
+            throw e;
        } catch (Exception e) {
            e.printStackTrace();
            return new ResponseEntity<>(null, HttpStatus.EXPECTATION_FAILED);
@@ -127,7 +73,12 @@ class ExpenseListController {
    }

    @DeleteMapping("{id}")
-    public ResponseEntity<HttpStatus> delete(@PathVariable("id") Long id) {
+    public ResponseEntity<HttpStatus> delete(@PathVariable("id") Long id, Authentication authentication) {
+        AppUser user = authenticatedUserResolver.resolveCurrentUser(authentication);
+        Optional<ExpenseList> listOpt = expenseListService.findById(id);
+        if (listOpt.isEmpty())
+            return new ResponseEntity<>(HttpStatus.NOT_FOUND);
+        assertOwner(user, listOpt.get());
        try {
            expenseListService.deleteById(id);
            return new ResponseEntity<>(HttpStatus.NO_CONTENT);
@@ -139,11 +90,16 @@ class ExpenseListController {
    @PostMapping("/{id}/add")
    public ResponseEntity<Expense> addExpenseToList(
            @PathVariable("id") Long expenseListId,
-            @RequestBody ExpenseInput expenseInput) {
+            @RequestBody @Valid ExpenseInput expenseInput,
+            Authentication authentication) {
+        AppUser user = authenticatedUserResolver.resolveCurrentUser(authentication);
+        Optional<ExpenseList> listOpt = expenseListService.findById(expenseListId);
+        if (listOpt.isEmpty())
+            return new ResponseEntity<>(HttpStatus.NOT_FOUND);
+        assertMember(user, listOpt.get());
        try {
            AppUser expenseOwner = userService.getUserByName(expenseInput.getOwner());
            Expense expense = expenseInput.convertToExpense(expenseOwner.getId());
-
            Expense addedExpense = expenseListService.addExpenseToList(expenseListId, expense);
            return new ResponseEntity<>(addedExpense, HttpStatus.CREATED);
        } catch (Exception e) {
@@ -154,18 +110,18 @@ class ExpenseListController {
    @PutMapping("/{id}/update")
    public ResponseEntity<Expense> updateExpenseInList(
            @PathVariable("id") Long expenseListId,
-            @RequestBody ExpenseChangeRequest expenseChangeRequest) {
+            @RequestBody @Valid ExpenseChangeRequest expenseChangeRequest,
+            Authentication authentication) {
+        AppUser user = authenticatedUserResolver.resolveCurrentUser(authentication);
+        Optional<ExpenseList> expenseListOpt = expenseListService.findById(expenseListId);
+        if (expenseListOpt.isEmpty())
+            return new ResponseEntity<>(null, HttpStatus.NOT_FOUND);
+        assertMember(user, expenseListOpt.get());
        try {
            AppUser expenseOwner = userService.getUserByName(expenseChangeRequest.getOwnerName());
-            Optional<ExpenseList> expenseList = expenseListService.findById(expenseListId);
-            if (expenseList.isPresent()) {
-                Expense expense = expenseChangeRequest.convertToExpense(expenseOwner.getId(), expenseList.get());
-
-                Expense addedExpense = expenseListService.updateExpense(expenseListId, expense);
-                return new ResponseEntity<>(addedExpense, HttpStatus.CREATED);
-            }
-            return new ResponseEntity<>(null, HttpStatus.BAD_REQUEST);
-
+            Expense expense = expenseChangeRequest.convertToExpense(expenseOwner.getId(), expenseListOpt.get());
+            Expense updatedExpense = expenseListService.updateExpense(expenseListId, expense);
+            return new ResponseEntity<>(updatedExpense, HttpStatus.OK);
        } catch (Exception e) {
            return new ResponseEntity<>(null, HttpStatus.BAD_REQUEST);
        }
@@ -174,7 +130,13 @@ class ExpenseListController {
    @DeleteMapping("/{id}/delete")
    public ResponseEntity<Expense> deleteExpenseFromList(
            @PathVariable("id") Long expenseListId,
-            @RequestParam("expenseId") Long expenseId) {
+            @RequestParam("expenseId") Long expenseId,
+            Authentication authentication) {
+        AppUser user = authenticatedUserResolver.resolveCurrentUser(authentication);
+        Optional<ExpenseList> listOpt = expenseListService.findById(expenseListId);
+        if (listOpt.isEmpty())
+            return new ResponseEntity<>(HttpStatus.NOT_FOUND);
+        assertMember(user, listOpt.get());
        try {
            expenseListService.deleteExpenseFromList(expenseListId, expenseId);
            return new ResponseEntity<>(HttpStatus.NO_CONTENT);
@@ -184,13 +146,20 @@ class ExpenseListController {
    }

    @PostMapping("/{listId}/invite")
-    public ResponseEntity<String> generateInvite(@PathVariable Long listId) {
+    public ResponseEntity<String> generateInvite(@PathVariable Long listId, Authentication authentication) {
+        AppUser user = authenticatedUserResolver.resolveCurrentUser(authentication);
+        Optional<ExpenseList> listOpt = expenseListService.findById(listId);
+        if (listOpt.isEmpty())
+            return new ResponseEntity<>(HttpStatus.NOT_FOUND);
+        assertOwner(user, listOpt.get());
        String inviteCode = expenseListService.generateInviteCode(listId);
        return ResponseEntity.ok(inviteCode);
    }

    @PostMapping("/accept-invite")
-    public ResponseEntity<?> acceptInvite(@RequestBody InviteRequest inviteRequest) {
+    public ResponseEntity<?> acceptInvite(@RequestBody @Valid InviteRequest inviteRequest,
+            Authentication authentication) {
+        AppUser authenticatedUser = authenticatedUserResolver.resolveCurrentUser(authentication);
        ExpenseList list = expenseListService.findByInviteCode(inviteRequest.getInviteCode());

        if (list == null || list.getInviteCodeExpiration() == null ||
@@ -200,21 +169,24 @@ class ExpenseListController {
        if (list.getSharedWith() != null) {
            return ResponseEntity.status(HttpStatus.IM_USED).body("List has already been shared");
        }
-        if (list.getOwner().getId() == inviteRequest.getUserId()) {
-            return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("You cant join your own List");
+        if (list.getOwner().getId().equals(authenticatedUser.getId())) {
+            return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("You cannot join your own list");
        }
-        AppUser user = null;
-        try {
-            user = userService.getUser(inviteRequest.getUserId());
-        } catch (Exception e) {
-            throw new RuntimeException("User not found");
-        }
-        if (user != null) {
-            list.setSharedWith(user);
+        list.setSharedWith(authenticatedUser);
        expenseListService.save(list);
-        } else {
-            throw new RuntimeException("User not found");
-        }
        return ResponseEntity.ok("User added to the list");
    }
+
+    private void assertOwner(AppUser authenticated, ExpenseList list) {
+        if (!list.getOwner().getId().equals(authenticated.getId()))
+            throw new ResponseStatusException(HttpStatus.FORBIDDEN);
+    }
+
+    private void assertMember(AppUser authenticated, ExpenseList list) {
+        boolean isOwner = list.getOwner().getId().equals(authenticated.getId());
+        boolean isShared = list.getSharedWith() != null
+                && list.getSharedWith().getId().equals(authenticated.getId());
+        if (!isOwner && !isShared)
+            throw new ResponseStatusException(HttpStatus.FORBIDDEN);
+    }
 }
@@ -0,0 +1,30 @@
+package de.zendric.app.xpensely_server.controller;
+
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
+import org.springframework.validation.FieldError;
+import org.springframework.web.bind.MethodArgumentNotValidException;
+import org.springframework.web.bind.annotation.ExceptionHandler;
+import org.springframework.web.bind.annotation.RestControllerAdvice;
+
+import java.util.HashMap;
+import java.util.Map;
+
+@RestControllerAdvice
+public class GlobalExceptionHandler {
+
+    @ExceptionHandler(MethodArgumentNotValidException.class)
+    public ResponseEntity<Map<String, String>> handleValidationErrors(MethodArgumentNotValidException ex) {
+        Map<String, String> errors = new HashMap<>();
+        for (FieldError fieldError : ex.getBindingResult().getFieldErrors()) {
+            errors.put(fieldError.getField(), fieldError.getDefaultMessage());
+        }
+        return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(errors);
+    }
+
+    @ExceptionHandler(IllegalArgumentException.class)
+    public ResponseEntity<Map<String, String>> handleIllegalArgument(IllegalArgumentException ex) {
+        return ResponseEntity.status(HttpStatus.BAD_REQUEST)
+                .body(Map.of("error", ex.getMessage()));
+    }
+}
@@ -0,0 +1,36 @@
+package de.zendric.app.xpensely_server.security;
+
+import de.zendric.app.xpensely_server.model.AppUser;
+import de.zendric.app.xpensely_server.services.UserService;
+import org.springframework.http.HttpStatus;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.oauth2.jwt.Jwt;
+import org.springframework.stereotype.Component;
+import org.springframework.web.server.ResponseStatusException;
+
+@Component
+public class AuthenticatedUserResolver {
+
+    private final UserService userService;
+
+    public AuthenticatedUserResolver(UserService userService) {
+        this.userService = userService;
+    }
+
+    public AppUser resolveCurrentUser(Authentication authentication) {
+        if (authentication == null) {
+            throw new ResponseStatusException(HttpStatus.FORBIDDEN, "Not authenticated");
+        }
+        Jwt jwt = (Jwt) authentication.getPrincipal();
+        String googleId = jwt.getSubject();
+        try {
+            AppUser user = userService.getUserByGoogleId(googleId);
+            if (user == null) {
+                throw new ResponseStatusException(HttpStatus.FORBIDDEN, "User not registered");
+            }
+            return user;
+        } catch (IllegalArgumentException e) {
+            throw new ResponseStatusException(HttpStatus.FORBIDDEN, "User not registered");
+        }
+    }
+}
@@ -0,0 +1,60 @@
+package de.zendric.app.xpensely_Server.controller;
+
+import de.zendric.app.xpensely_server.controller.AppUserController;
+import de.zendric.app.xpensely_server.services.UserService;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
+import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
+import org.springframework.http.MediaType;
+import org.springframework.test.context.ActiveProfiles;
+import org.springframework.test.context.bean.override.mockito.MockitoBean;
+import org.springframework.test.web.servlet.MockMvc;
+
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
+
+@WebMvcTest(AppUserController.class)
+@AutoConfigureMockMvc(addFilters = false)
+@ActiveProfiles("test")
+class AppUserControllerTest {
+
+    @Autowired MockMvc mockMvc;
+    @MockitoBean UserService userService;
+
+    @Test
+    void createUser_blankUsername_returns400WithFieldError() throws Exception {
+        mockMvc.perform(post("/api/users/createUser")
+                .contentType(MediaType.APPLICATION_JSON)
+                .content("{\"username\":\"\",\"googleId\":\"gid123\"}"))
+                .andExpect(status().isBadRequest())
+                .andExpect(jsonPath("$.username").exists());
+    }
+
+    @Test
+    void createUser_invalidUsernamePattern_returns400() throws Exception {
+        mockMvc.perform(post("/api/users/createUser")
+                .contentType(MediaType.APPLICATION_JSON)
+                .content("{\"username\":\"hello world!\",\"googleId\":\"gid123\"}"))
+                .andExpect(status().isBadRequest())
+                .andExpect(jsonPath("$.username").exists());
+    }
+
+    @Test
+    void createUser_usernameTooShort_returns400() throws Exception {
+        mockMvc.perform(post("/api/users/createUser")
+                .contentType(MediaType.APPLICATION_JSON)
+                .content("{\"username\":\"ab\",\"googleId\":\"gid123\"}"))
+                .andExpect(status().isBadRequest())
+                .andExpect(jsonPath("$.username").exists());
+    }
+
+    @Test
+    void createUser_blankGoogleId_returns400() throws Exception {
+        mockMvc.perform(post("/api/users/createUser")
+                .contentType(MediaType.APPLICATION_JSON)
+                .content("{\"username\":\"validuser\",\"googleId\":\"\"}"))
+                .andExpect(status().isBadRequest())
+                .andExpect(jsonPath("$.googleId").exists());
+    }
+}