From 2bd229cc5e1eb7c24964ef7af2d1f65f46912785 Mon Sep 17 00:00:00 2001 From: Cedric Hornberger Date: Tue, 5 May 2026 16:59:35 +0200 Subject: [PATCH] Remove docs from tracking --- .../plans/2026-05-04-security-hardening.md | 1462 ----------------- .../2026-05-04-security-hardening-design.md | 166 -- 2 files changed, 1628 deletions(-) delete mode 100644 docs/superpowers/plans/2026-05-04-security-hardening.md delete mode 100644 docs/superpowers/specs/2026-05-04-security-hardening-design.md diff --git a/docs/superpowers/plans/2026-05-04-security-hardening.md b/docs/superpowers/plans/2026-05-04-security-hardening.md deleted file mode 100644 index e1f13eb..0000000 --- a/docs/superpowers/plans/2026-05-04-security-hardening.md +++ /dev/null @@ -1,1462 +0,0 @@ -# Security Hardening Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Add input validation, JWT-based authorization enforcement, and per-user rate limiting to XpenselyServer. - -**Architecture:** Bean Validation annotations on all request models with a `@ControllerAdvice` exception handler for structured 400 responses; an `AuthenticatedUserResolver` component extracts the authenticated `AppUser` from the JWT `sub` claim and guards every endpoint via `assertOwner`/`assertMember` helpers; a `RateLimitFilter` registered inside the Spring Security chain uses Bucket4j in-memory buckets keyed by JWT subject (or IP fallback) to enforce per-endpoint limits. - -**Tech Stack:** Spring Boot 3.4.1, Jakarta Bean Validation (spring-boot-starter-validation), Bucket4j 8.10.1 (bucket4j-core), Spring Security Test (jwt() post processor), JUnit 5, Mockito - ---- - -> **Before starting:** Check out the feature branch. -> ``` -> git checkout feature/security-hardening -> ``` - -> **Client compatibility note:** `GET /api/expenselist/byUser` and `GET /api/expenselist/byUsername` are removed; the Flutter client must be updated to call `GET /api/expenselist/mine` instead. This is out of scope for this plan — coordinate with the client update separately. - ---- - -## File Map - -| File | Action | Responsibility | -|---|---|---| -| `pom.xml` | Modify | Add validation + Bucket4j dependencies | -| `model/AppUserCreateRequest.java` | Modify | Add Bean Validation annotations | -| `model/ExpenseInput.java` | Modify | Add validation; remove stray JPA annotations | -| `model/ExpenseChangeRequest.java` | Modify | Add Bean Validation annotations | -| `model/InviteRequest.java` | Modify | Add validation; remove `userId` field | -| `controller/GlobalExceptionHandler.java` | Create | Structured 400/403 error responses | -| `controller/AppUserController.java` | Modify | Add `@Valid`, ownership guards, JWT user resolution | -| `controller/ExpenseListController.java` | Modify | Add `@Valid`, ownership guards, rename/remove endpoints | -| `security/AuthenticatedUserResolver.java` | Create | Resolve JWT sub claim to `AppUser` entity | -| `security/RateLimitFilter.java` | Create | Per-user/IP Bucket4j rate limiting | -| `test/.../controller/AppUserControllerTest.java` | Create | Validation + authorization tests for user controller | -| `test/.../controller/ExpenseListControllerTest.java` | Create | Validation + authorization tests for expense list controller | -| `test/.../security/AuthenticatedUserResolverTest.java` | Create | Unit tests for JWT resolution | -| `test/.../security/RateLimitFilterTest.java` | Create | Unit tests for rate limiting | - ---- - -## Task 1: Add Maven Dependencies - -**Files:** -- Modify: `pom.xml` - -- [ ] **Step 1: Add dependencies** - -In `pom.xml`, inside ``, add after the existing `spring-boot-starter-security` dependency: - -```xml - - org.springframework.boot - spring-boot-starter-validation - - - com.bucket4j - bucket4j-core - 8.10.1 - -``` - -- [ ] **Step 2: Verify build compiles** - -Run: `mvn compile -q` - -Expected: `BUILD SUCCESS` with no errors. - -- [ ] **Step 3: Commit** - -``` -git add pom.xml -git commit -m "build: add spring-boot-starter-validation and bucket4j-core" -``` - ---- - -## Task 2: Annotate Request Models - -**Files:** -- Modify: `src/main/java/de/zendric/app/xpensely_server/model/AppUserCreateRequest.java` -- Modify: `src/main/java/de/zendric/app/xpensely_server/model/ExpenseInput.java` -- Modify: `src/main/java/de/zendric/app/xpensely_server/model/ExpenseChangeRequest.java` -- Modify: `src/main/java/de/zendric/app/xpensely_server/model/InviteRequest.java` - -- [ ] **Step 1: Update AppUserCreateRequest.java** - -Replace the entire file content with: - -```java -package de.zendric.app.xpensely_server.model; - -import jakarta.validation.constraints.NotBlank; -import jakarta.validation.constraints.Pattern; -import jakarta.validation.constraints.Size; -import lombok.Data; - -@Data -public class AppUserCreateRequest { - - @NotBlank(message = "Username is required") - @Size(min = 3, max = 30, message = "Username must be between 3 and 30 characters") - @Pattern(regexp = "^[a-zA-Z0-9_.\\-]+$", message = "Username may only contain letters, digits, underscores, dots, and hyphens") - private String username; - - @NotBlank(message = "Google ID is required") - private String googleId; - - public AppUser convertToAppUser() { - AppUser appUser = new AppUser(); - appUser.setGoogleId(googleId); - appUser.setUsername(username); - return appUser; - } -} -``` - -- [ ] **Step 2: Update ExpenseInput.java** - -Replace the entire file content with (removes stray `@Id`/`@GeneratedValue`, adds validation): - -```java -package de.zendric.app.xpensely_server.model; - -import java.time.LocalDate; - -import jakarta.validation.constraints.DecimalMin; -import jakarta.validation.constraints.NotBlank; -import jakarta.validation.constraints.NotNull; -import jakarta.validation.constraints.Size; -import lombok.AllArgsConstructor; -import lombok.Getter; -import lombok.NoArgsConstructor; -import lombok.Setter; - -@Getter -@Setter -@AllArgsConstructor -@NoArgsConstructor -public class ExpenseInput { - - private Long id; - - @NotBlank(message = "Title is required") - @Size(max = 100, message = "Title must not exceed 100 characters") - private String title; - - @NotBlank(message = "Owner is required") - private String owner; - - @NotNull(message = "Amount is required") - @DecimalMin(value = "0.01", message = "Amount must be greater than zero") - private Double amount; - - private Double personalUseAmount; - private Double otherPersonAmount; - - @NotNull(message = "Date is required") - private LocalDate date; - - @NotBlank(message = "Category is required") - private String category; - - private ExpenseList expenseList; - - public Expense convertToExpense(Long userId) { - AppUser appUser = new AppUser(); - appUser.setId(userId); - appUser.setUsername(owner); - - Expense expense = new Expense(); - expense.setAmount(amount); - expense.setDate(date); - expense.setPersonalUseAmount(personalUseAmount); - expense.setOtherPersonAmount(otherPersonAmount); - expense.setExpenseList(expenseList); - expense.setId(id); - expense.setOwner(appUser); - expense.setTitle(title); - expense.setCategory(category); - - return expense; - } -} -``` - -- [ ] **Step 3: Update ExpenseChangeRequest.java** - -Replace the entire file content with: - -```java -package de.zendric.app.xpensely_server.model; - -import java.time.LocalDate; - -import jakarta.validation.constraints.DecimalMin; -import jakarta.validation.constraints.NotBlank; -import jakarta.validation.constraints.NotNull; -import jakarta.validation.constraints.Size; -import lombok.AllArgsConstructor; -import lombok.Data; -import lombok.NoArgsConstructor; - -@Data -@AllArgsConstructor -@NoArgsConstructor -public class ExpenseChangeRequest { - - private Long id; - - @NotBlank(message = "Title is required") - @Size(max = 100, message = "Title must not exceed 100 characters") - private String title; - - @NotBlank(message = "Owner name is required") - private String ownerName; - - @NotNull(message = "Amount is required") - @DecimalMin(value = "0.01", message = "Amount must be greater than zero") - private Double amount; - - private Double personalUseAmount; - private Double otherPersonAmount; - - @NotNull(message = "Date is required") - private LocalDate date; - - @NotBlank(message = "Category is required") - private String category; - - public Expense convertToExpense(Long userId, ExpenseList expenseList) { - AppUser appUser = new AppUser(); - appUser.setId(userId); - appUser.setUsername(ownerName); - - Expense expense = new Expense(); - expense.setAmount(amount); - expense.setDate(date); - expense.setPersonalUseAmount(personalUseAmount); - expense.setOtherPersonAmount(otherPersonAmount); - expense.setExpenseList(expenseList); - expense.setId(id); - expense.setOwner(appUser); - expense.setTitle(title); - expense.setCategory(category); - - return expense; - } -} -``` - -- [ ] **Step 4: Update InviteRequest.java** - -Replace the entire file content with (`userId` removed — will be derived from JWT): - -```java -package de.zendric.app.xpensely_server.model; - -import jakarta.validation.constraints.NotBlank; -import jakarta.validation.constraints.Size; -import lombok.AllArgsConstructor; -import lombok.Data; -import lombok.NoArgsConstructor; - -@Data -@AllArgsConstructor -@NoArgsConstructor -public class InviteRequest { - - @NotBlank(message = "Invite code is required") - @Size(min = 6, max = 6, message = "Invite code must be exactly 6 characters") - private String inviteCode; -} -``` - -- [ ] **Step 5: Verify build compiles** - -Run: `mvn compile -q` - -Expected: `BUILD SUCCESS` - -- [ ] **Step 6: Commit** - -``` -git add src/main/java/de/zendric/app/xpensely_server/model/ -git commit -m "feat: add Bean Validation annotations to request models" -``` - ---- - -## Task 3: Add GlobalExceptionHandler - -**Files:** -- Create: `src/main/java/de/zendric/app/xpensely_server/controller/GlobalExceptionHandler.java` -- Create: `src/test/java/de/zendric/app/xpensely_server/controller/AppUserControllerTest.java` - -- [ ] **Step 1: Write the failing test** - -Create `src/test/java/de/zendric/app/xpensely_server/controller/AppUserControllerTest.java`: - -```java -package de.zendric.app.xpensely_server.controller; - -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.WebMvcTest; -import org.springframework.boot.test.mock.mockito.MockBean; -import org.springframework.http.MediaType; -import org.springframework.test.context.ActiveProfiles; -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) -@ActiveProfiles("test") -class AppUserControllerTest { - - @Autowired MockMvc mockMvc; - @MockBean 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()); - } -} -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `mvn test -Dtest=AppUserControllerTest -pl . -q` - -Expected: FAIL — validation is not wired up yet; the controller returns 417 or 500 instead of 400. - -- [ ] **Step 3: Create GlobalExceptionHandler** - -Create `src/main/java/de/zendric/app/xpensely_server/controller/GlobalExceptionHandler.java`: - -```java -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> handleValidationErrors(MethodArgumentNotValidException ex) { - Map 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> handleIllegalArgument(IllegalArgumentException ex) { - return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .body(Map.of("error", ex.getMessage())); - } -} -``` - -- [ ] **Step 4: Add @Valid to AppUserController.createUser** - -In `src/main/java/de/zendric/app/xpensely_server/controller/AppUserController.java`, add `import jakarta.validation.Valid;` and update the `createUser` method signature: - -```java -import jakarta.validation.Valid; - -// Change: -public ResponseEntity createUser(@RequestBody AppUserCreateRequest userRequest) { -// To: -public ResponseEntity createUser(@RequestBody @Valid AppUserCreateRequest userRequest) { -``` - -- [ ] **Step 5: Run tests to verify they pass** - -Run: `mvn test -Dtest=AppUserControllerTest -pl . -q` - -Expected: `Tests run: 4, Failures: 0, Errors: 0` - -- [ ] **Step 6: Commit** - -``` -git add src/main/java/de/zendric/app/xpensely_server/controller/GlobalExceptionHandler.java -git add src/main/java/de/zendric/app/xpensely_server/controller/AppUserController.java -git add src/test/java/de/zendric/app/xpensely_server/controller/AppUserControllerTest.java -git commit -m "feat: add GlobalExceptionHandler and @Valid to user creation endpoint" -``` - ---- - -## Task 4: Add @Valid to ExpenseList Endpoints - -**Files:** -- Modify: `src/main/java/de/zendric/app/xpensely_server/controller/ExpenseListController.java` -- Create: `src/test/java/de/zendric/app/xpensely_server/controller/ExpenseListControllerTest.java` - -- [ ] **Step 1: Write the failing tests** - -Create `src/test/java/de/zendric/app/xpensely_server/controller/ExpenseListControllerTest.java`: - -```java -package de.zendric.app.xpensely_server.controller; - -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; -import org.junit.jupiter.api.Test; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; -import org.springframework.boot.test.mock.mockito.MockBean; -import org.springframework.http.MediaType; -import org.springframework.test.context.ActiveProfiles; -import org.springframework.test.web.servlet.MockMvc; - -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; - -@WebMvcTest(ExpenseListController.class) -@ActiveProfiles("test") -class ExpenseListControllerTest { - - @Autowired MockMvc mockMvc; - @MockBean ExpenseListService expenseListService; - @MockBean UserService userService; - @MockBean CategoryService categoryService; - @MockBean AuthenticatedUserResolver authenticatedUserResolver; - - @Test - void addExpense_blankTitle_returns400() throws Exception { - mockMvc.perform(post("/api/expenselist/1/add") - .contentType(MediaType.APPLICATION_JSON) - .content("{\"title\":\"\",\"owner\":\"alice\",\"amount\":10.0,\"date\":\"2026-05-04\",\"category\":\"Food\"}")) - .andExpect(status().isBadRequest()) - .andExpect(jsonPath("$.title").exists()); - } - - @Test - void addExpense_negativeAmount_returns400() throws Exception { - mockMvc.perform(post("/api/expenselist/1/add") - .contentType(MediaType.APPLICATION_JSON) - .content("{\"title\":\"Lunch\",\"owner\":\"alice\",\"amount\":-5.0,\"date\":\"2026-05-04\",\"category\":\"Food\"}")) - .andExpect(status().isBadRequest()) - .andExpect(jsonPath("$.amount").exists()); - } - - @Test - void addExpense_nullDate_returns400() throws Exception { - mockMvc.perform(post("/api/expenselist/1/add") - .contentType(MediaType.APPLICATION_JSON) - .content("{\"title\":\"Lunch\",\"owner\":\"alice\",\"amount\":10.0,\"category\":\"Food\"}")) - .andExpect(status().isBadRequest()) - .andExpect(jsonPath("$.date").exists()); - } - - @Test - void acceptInvite_blankCode_returns400() throws Exception { - mockMvc.perform(post("/api/expenselist/accept-invite") - .contentType(MediaType.APPLICATION_JSON) - .content("{\"inviteCode\":\"\"}")) - .andExpect(status().isBadRequest()) - .andExpect(jsonPath("$.inviteCode").exists()); - } - - @Test - void acceptInvite_wrongCodeLength_returns400() throws Exception { - mockMvc.perform(post("/api/expenselist/accept-invite") - .contentType(MediaType.APPLICATION_JSON) - .content("{\"inviteCode\":\"ABC\"}")) - .andExpect(status().isBadRequest()) - .andExpect(jsonPath("$.inviteCode").exists()); - } -} -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `mvn test -Dtest=ExpenseListControllerTest -pl . -q` - -Expected: FAIL — `@Valid` not yet added and `AuthenticatedUserResolver` bean does not exist yet. - -- [ ] **Step 3: Create stub AuthenticatedUserResolver** - -Create `src/main/java/de/zendric/app/xpensely_server/security/AuthenticatedUserResolver.java` (full implementation comes in Task 5; create the stub now so the app compiles): - -```java -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"); - } - } -} -``` - -- [ ] **Step 4: Add @Valid to ExpenseListController endpoints** - -In `src/main/java/de/zendric/app/xpensely_server/controller/ExpenseListController.java`, add `import jakarta.validation.Valid;` and update the three `@RequestBody` parameters: - -```java -import jakarta.validation.Valid; - -// Change addExpenseToList signature: -public ResponseEntity addExpenseToList( - @PathVariable("id") Long expenseListId, - @RequestBody @Valid ExpenseInput expenseInput) { - -// Change updateExpenseInList signature: -public ResponseEntity updateExpenseInList( - @PathVariable("id") Long expenseListId, - @RequestBody @Valid ExpenseChangeRequest expenseChangeRequest) { - -// Change acceptInvite signature: -public ResponseEntity acceptInvite(@RequestBody @Valid InviteRequest inviteRequest) { -``` - -- [ ] **Step 5: Run tests to verify they pass** - -Run: `mvn test -Dtest=ExpenseListControllerTest -pl . -q` - -Expected: `Tests run: 5, Failures: 0, Errors: 0` - -- [ ] **Step 6: Run all tests** - -Run: `mvn test -q` - -Expected: All existing tests still pass. - -- [ ] **Step 7: Commit** - -``` -git add src/main/java/de/zendric/app/xpensely_server/controller/ExpenseListController.java -git add src/main/java/de/zendric/app/xpensely_server/security/AuthenticatedUserResolver.java -git add src/test/java/de/zendric/app/xpensely_server/controller/ExpenseListControllerTest.java -git commit -m "feat: add @Valid to expense list endpoints and stub AuthenticatedUserResolver" -``` - ---- - -## Task 5: Test and Complete AuthenticatedUserResolver - -**Files:** -- Modify: `src/main/java/de/zendric/app/xpensely_server/security/AuthenticatedUserResolver.java` (already complete from Task 4) -- Create: `src/test/java/de/zendric/app/xpensely_server/security/AuthenticatedUserResolverTest.java` - -- [ ] **Step 1: Write the failing tests** - -Create `src/test/java/de/zendric/app/xpensely_server/security/AuthenticatedUserResolverTest.java`: - -```java -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.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.springframework.http.HttpStatus; -import org.springframework.security.oauth2.jwt.Jwt; -import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationToken; -import org.springframework.web.server.ResponseStatusException; - -import static org.junit.jupiter.api.Assertions.*; -import static org.mockito.Mockito.*; - -class AuthenticatedUserResolverTest { - - UserService userService; - AuthenticatedUserResolver resolver; - - @BeforeEach - void setUp() { - userService = mock(UserService.class); - resolver = new AuthenticatedUserResolver(userService); - } - - @Test - void resolveCurrentUser_validJwt_returnsAppUser() { - Jwt jwt = Jwt.withTokenValue("token") - .header("alg", "RS256") - .subject("google-id-123") - .build(); - JwtAuthenticationToken auth = new JwtAuthenticationToken(jwt); - - AppUser user = new AppUser(); - user.setId(1L); - user.setGoogleId("google-id-123"); - when(userService.getUserByGoogleId("google-id-123")).thenReturn(user); - - AppUser result = resolver.resolveCurrentUser(auth); - assertEquals(user, result); - } - - @Test - void resolveCurrentUser_userNotFound_throws403() { - Jwt jwt = Jwt.withTokenValue("token") - .header("alg", "RS256") - .subject("unknown-id") - .build(); - JwtAuthenticationToken auth = new JwtAuthenticationToken(jwt); - when(userService.getUserByGoogleId("unknown-id")).thenReturn(null); - - ResponseStatusException ex = assertThrows(ResponseStatusException.class, - () -> resolver.resolveCurrentUser(auth)); - assertEquals(HttpStatus.FORBIDDEN, ex.getStatusCode()); - } - - @Test - void resolveCurrentUser_userServiceThrows_throws403() { - Jwt jwt = Jwt.withTokenValue("token") - .header("alg", "RS256") - .subject("gone-id") - .build(); - JwtAuthenticationToken auth = new JwtAuthenticationToken(jwt); - when(userService.getUserByGoogleId("gone-id")).thenThrow(new IllegalArgumentException("not found")); - - ResponseStatusException ex = assertThrows(ResponseStatusException.class, - () -> resolver.resolveCurrentUser(auth)); - assertEquals(HttpStatus.FORBIDDEN, ex.getStatusCode()); - } - - @Test - void resolveCurrentUser_nullAuthentication_throws403() { - ResponseStatusException ex = assertThrows(ResponseStatusException.class, - () -> resolver.resolveCurrentUser(null)); - assertEquals(HttpStatus.FORBIDDEN, ex.getStatusCode()); - } -} -``` - -- [ ] **Step 2: Run tests to verify they pass** - -Run: `mvn test -Dtest=AuthenticatedUserResolverTest -pl . -q` - -Expected: `Tests run: 4, Failures: 0, Errors: 0` - -(The implementation was already written in Task 4. If tests fail, revisit the resolver logic.) - -- [ ] **Step 3: Commit** - -``` -git add src/test/java/de/zendric/app/xpensely_server/security/AuthenticatedUserResolverTest.java -git commit -m "test: add unit tests for AuthenticatedUserResolver" -``` - ---- - -## Task 6: Add Authorization to ExpenseListController - -**Files:** -- Modify: `src/main/java/de/zendric/app/xpensely_server/controller/ExpenseListController.java` -- Modify: `src/test/java/de/zendric/app/xpensely_server/controller/ExpenseListControllerTest.java` - -- [ ] **Step 1: Write failing authorization tests** - -Add these tests to the existing `ExpenseListControllerTest.java` (append inside the class body, before the closing brace): - -```java - // --- Authorization tests --- - // These run with the test profile (security disabled) but mock the resolver - // to simulate different users, testing the ownership guard logic directly. - - @Test - void getById_authenticatedUserNotMember_returns403() throws Exception { - de.zendric.app.xpensely_server.model.AppUser owner = new de.zendric.app.xpensely_server.model.AppUser(); - owner.setId(1L); - de.zendric.app.xpensely_server.model.AppUser requester = new de.zendric.app.xpensely_server.model.AppUser(); - requester.setId(2L); - - de.zendric.app.xpensely_server.model.ExpenseList list = new de.zendric.app.xpensely_server.model.ExpenseList(); - list.setId(1L); - list.setOwner(owner); - - org.mockito.Mockito.when(expenseListService.findById(1L)).thenReturn(java.util.Optional.of(list)); - org.mockito.Mockito.when(authenticatedUserResolver.resolveCurrentUser(org.mockito.ArgumentMatchers.any())) - .thenReturn(requester); - - mockMvc.perform(get("/api/expenselist/byId").param("id", "1")) - .andExpect(status().isForbidden()); - } - - @Test - void getById_authenticatedUserIsOwner_returns200() throws Exception { - de.zendric.app.xpensely_server.model.AppUser owner = new de.zendric.app.xpensely_server.model.AppUser(); - owner.setId(1L); - - de.zendric.app.xpensely_server.model.ExpenseList list = new de.zendric.app.xpensely_server.model.ExpenseList(); - list.setId(1L); - list.setOwner(owner); - - org.mockito.Mockito.when(expenseListService.findById(1L)).thenReturn(java.util.Optional.of(list)); - org.mockito.Mockito.when(authenticatedUserResolver.resolveCurrentUser(org.mockito.ArgumentMatchers.any())) - .thenReturn(owner); - - mockMvc.perform(get("/api/expenselist/byId").param("id", "1")) - .andExpect(status().isOk()); - } - - @Test - void deleteList_nonOwner_returns403() throws Exception { - de.zendric.app.xpensely_server.model.AppUser owner = new de.zendric.app.xpensely_server.model.AppUser(); - owner.setId(1L); - de.zendric.app.xpensely_server.model.AppUser nonOwner = new de.zendric.app.xpensely_server.model.AppUser(); - nonOwner.setId(2L); - - de.zendric.app.xpensely_server.model.ExpenseList list = new de.zendric.app.xpensely_server.model.ExpenseList(); - list.setId(5L); - list.setOwner(owner); - - org.mockito.Mockito.when(expenseListService.findById(5L)).thenReturn(java.util.Optional.of(list)); - org.mockito.Mockito.when(authenticatedUserResolver.resolveCurrentUser(org.mockito.ArgumentMatchers.any())) - .thenReturn(nonOwner); - - mockMvc.perform(delete("/api/expenselist/5")) - .andExpect(status().isForbidden()); - } - - @Test - void getMine_returnsCurrentUserLists() throws Exception { - de.zendric.app.xpensely_server.model.AppUser user = new de.zendric.app.xpensely_server.model.AppUser(); - user.setId(3L); - - org.mockito.Mockito.when(authenticatedUserResolver.resolveCurrentUser(org.mockito.ArgumentMatchers.any())) - .thenReturn(user); - org.mockito.Mockito.when(expenseListService.findByUserId(3L)) - .thenReturn(java.util.List.of(new de.zendric.app.xpensely_server.model.ExpenseList())); - - mockMvc.perform(get("/api/expenselist/mine")) - .andExpect(status().isOk()); - } -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `mvn test -Dtest=ExpenseListControllerTest -pl . -q` - -Expected: FAIL — `getMine` endpoint doesn't exist, authorization guards not yet implemented. - -- [ ] **Step 3: Rewrite ExpenseListController with authorization** - -Replace the entire content of `src/main/java/de/zendric/app/xpensely_server/controller/ExpenseListController.java`: - -```java -package de.zendric.app.xpensely_server.controller; - -import java.time.LocalDateTime; -import java.util.List; -import java.util.Optional; - -import jakarta.validation.Valid; -import org.springframework.http.HttpStatus; -import org.springframework.http.ResponseEntity; -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.*; -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; - -@RestController -@RequestMapping("/api/expenselist") -class ExpenseListController { - - private final ExpenseListService expenseListService; - private final UserService userService; - private final CategoryService categoryService; - private final AuthenticatedUserResolver authenticatedUserResolver; - - public ExpenseListController(ExpenseListService expenseListService, UserService userService, - CategoryService categoryService, AuthenticatedUserResolver authenticatedUserResolver) { - this.expenseListService = expenseListService; - this.userService = userService; - this.categoryService = categoryService; - this.authenticatedUserResolver = authenticatedUserResolver; - } - - @GetMapping("/mine") - public ResponseEntity> getMine(Authentication authentication) { - AppUser user = authenticatedUserResolver.resolveCurrentUser(authentication); - List items = expenseListService.findByUserId(user.getId()); - if (items.isEmpty()) - return new ResponseEntity<>(HttpStatus.NO_CONTENT); - return new ResponseEntity<>(items, HttpStatus.OK); - } - - @GetMapping("/byId") - public ResponseEntity getById(@RequestParam Long id, Authentication authentication) { - AppUser user = authenticatedUserResolver.resolveCurrentUser(authentication); - Optional existingItemOptional = expenseListService.findById(id); - if (existingItemOptional.isEmpty()) - return new ResponseEntity<>(HttpStatus.NOT_FOUND); - assertMember(user, existingItemOptional.get()); - return new ResponseEntity<>(existingItemOptional.get(), HttpStatus.OK); - } - - @PostMapping("/create") - public ResponseEntity create(@RequestBody ExpenseList expenseList, - Authentication authentication) { - try { - AppUser authenticatedUser = authenticatedUserResolver.resolveCurrentUser(authentication); - expenseList.setOwner(authenticatedUser); - XpenselyStandardCategories standardCategories = categoryService.getDefaultCategories(); - expenseList.setXpenselyStandardCategories(standardCategories); - 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); - } - } - - @DeleteMapping("{id}") - public ResponseEntity delete(@PathVariable("id") Long id, Authentication authentication) { - AppUser user = authenticatedUserResolver.resolveCurrentUser(authentication); - Optional 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); - } catch (Exception e) { - return new ResponseEntity<>(HttpStatus.EXPECTATION_FAILED); - } - } - - @PostMapping("/{id}/add") - public ResponseEntity addExpenseToList( - @PathVariable("id") Long expenseListId, - @RequestBody @Valid ExpenseInput expenseInput, - Authentication authentication) { - AppUser user = authenticatedUserResolver.resolveCurrentUser(authentication); - Optional 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) { - return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR); - } - } - - @PutMapping("/{id}/update") - public ResponseEntity updateExpenseInList( - @PathVariable("id") Long expenseListId, - @RequestBody @Valid ExpenseChangeRequest expenseChangeRequest, - Authentication authentication) { - AppUser user = authenticatedUserResolver.resolveCurrentUser(authentication); - Optional 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()); - 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); - } - } - - @DeleteMapping("/{id}/delete") - public ResponseEntity deleteExpenseFromList( - @PathVariable("id") Long expenseListId, - @RequestParam("expenseId") Long expenseId, - Authentication authentication) { - AppUser user = authenticatedUserResolver.resolveCurrentUser(authentication); - Optional 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); - } catch (Exception e) { - return new ResponseEntity<>(null, HttpStatus.EXPECTATION_FAILED); - } - } - - @PostMapping("/{listId}/invite") - public ResponseEntity generateInvite(@PathVariable Long listId, Authentication authentication) { - AppUser user = authenticatedUserResolver.resolveCurrentUser(authentication); - Optional 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 @Valid InviteRequest inviteRequest, - Authentication authentication) { - AppUser authenticatedUser = authenticatedUserResolver.resolveCurrentUser(authentication); - ExpenseList list = expenseListService.findByInviteCode(inviteRequest.getInviteCode()); - - if (list == null || list.getInviteCodeExpiration() == null || - list.getInviteCodeExpiration().isBefore(LocalDateTime.now())) { - return ResponseEntity.status(HttpStatus.NOT_FOUND).body("Invalid or expired invite code"); - } - if (list.getSharedWith() != null) { - return ResponseEntity.status(HttpStatus.IM_USED).body("List has already been shared"); - } - if (list.getOwner().getId().equals(authenticatedUser.getId())) { - return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("You cannot join your own list"); - } - list.setSharedWith(authenticatedUser); - expenseListService.save(list); - return ResponseEntity.ok("User added to the list"); - } - - // --- Authorization helpers --- - - 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); - } -} -``` - -- [ ] **Step 4: Run tests to verify they pass** - -Run: `mvn test -Dtest=ExpenseListControllerTest -pl . -q` - -Expected: `Tests run: 9, Failures: 0, Errors: 0` - -- [ ] **Step 5: Run all tests** - -Run: `mvn test -q` - -Expected: All tests pass. - -- [ ] **Step 6: Commit** - -``` -git add src/main/java/de/zendric/app/xpensely_server/controller/ExpenseListController.java -git add src/test/java/de/zendric/app/xpensely_server/controller/ExpenseListControllerTest.java -git commit -m "feat: add ownership guards and JWT-based authorization to expense list endpoints" -``` - ---- - -## Task 7: Add Authorization to AppUserController - -**Files:** -- Modify: `src/main/java/de/zendric/app/xpensely_server/controller/AppUserController.java` -- Modify: `src/test/java/de/zendric/app/xpensely_server/controller/AppUserControllerTest.java` - -- [ ] **Step 1: Write failing authorization tests** - -Add these tests inside `AppUserControllerTest.java` (before the closing brace). Add `@MockBean AuthenticatedUserResolver authenticatedUserResolver;` to the class fields first: - -```java - @MockBean de.zendric.app.xpensely_server.security.AuthenticatedUserResolver authenticatedUserResolver; - - @Test - void getUser_differentUser_returns403() throws Exception { - de.zendric.app.xpensely_server.model.AppUser self = new de.zendric.app.xpensely_server.model.AppUser(); - self.setId(1L); - org.mockito.Mockito.when(authenticatedUserResolver.resolveCurrentUser(org.mockito.ArgumentMatchers.any())) - .thenReturn(self); - - mockMvc.perform(org.springframework.test.web.servlet.request.MockMvcRequestBuilders - .get("/api/users").param("id", "99")) - .andExpect(status().isForbidden()); - } - - @Test - void getUser_sameUser_returns200() throws Exception { - de.zendric.app.xpensely_server.model.AppUser self = new de.zendric.app.xpensely_server.model.AppUser(); - self.setId(1L); - org.mockito.Mockito.when(authenticatedUserResolver.resolveCurrentUser(org.mockito.ArgumentMatchers.any())) - .thenReturn(self); - org.mockito.Mockito.when(userService.getUser(1L)).thenReturn(self); - - mockMvc.perform(org.springframework.test.web.servlet.request.MockMvcRequestBuilders - .get("/api/users").param("id", "1")) - .andExpect(status().isOk()); - } - - @Test - void deleteUser_differentUser_returns403() throws Exception { - de.zendric.app.xpensely_server.model.AppUser self = new de.zendric.app.xpensely_server.model.AppUser(); - self.setId(1L); - org.mockito.Mockito.when(authenticatedUserResolver.resolveCurrentUser(org.mockito.ArgumentMatchers.any())) - .thenReturn(self); - - mockMvc.perform(org.springframework.test.web.servlet.request.MockMvcRequestBuilders - .delete("/api/users").param("id", "99")) - .andExpect(status().isForbidden()); - } -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `mvn test -Dtest=AppUserControllerTest -pl . -q` - -Expected: FAIL — authorization guards not yet added. - -- [ ] **Step 3: Rewrite AppUserController with authorization** - -Replace the entire content of `src/main/java/de/zendric/app/xpensely_server/controller/AppUserController.java`: - -```java -package de.zendric.app.xpensely_server.controller; - -import jakarta.validation.Valid; -import org.springframework.http.HttpStatus; -import org.springframework.http.ResponseEntity; -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.AppUserCreateRequest; -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; - -@RestController -@RequestMapping("/api/users") -public class AppUserController { - - private final UserService userService; - private final AuthenticatedUserResolver authenticatedUserResolver; - - public AppUserController(UserService userService, AuthenticatedUserResolver authenticatedUserResolver) { - this.userService = userService; - this.authenticatedUserResolver = authenticatedUserResolver; - } - - @GetMapping - public ResponseEntity getUser(@RequestParam Long id, Authentication authentication) { - AppUser self = authenticatedUserResolver.resolveCurrentUser(authentication); - assertSelf(self, id); - return ResponseEntity.ok(userService.getUser(id)); - } - - @GetMapping("/byName") - public AppUser getUserByName(@RequestParam String username) { - return userService.getUserByName(username); - } - - @GetMapping("/byGoogleId") - public ResponseEntity getUserByGoogleId(@RequestParam String id, Authentication authentication) { - AppUser self = authenticatedUserResolver.resolveCurrentUser(authentication); - if (!self.getGoogleId().equals(id)) - throw new ResponseStatusException(HttpStatus.FORBIDDEN); - return ResponseEntity.ok(self); - } - - @PostMapping("/createUser") - public ResponseEntity createUser(@RequestBody @Valid AppUserCreateRequest userRequest) { - try { - AppUser convertedUser = userRequest.convertToAppUser(); - AppUser nUser = userService.createUser(convertedUser); - return new ResponseEntity<>(nUser, HttpStatus.CREATED); - } catch (UsernameAlreadyExistsException e) { - return new ResponseEntity<>(null, HttpStatus.CONFLICT); - } catch (Exception e) { - return new ResponseEntity<>(null, HttpStatus.BAD_REQUEST); - } - } - - @DeleteMapping - public ResponseEntity deleteUser(@RequestParam Long id, Authentication authentication) { - AppUser self = authenticatedUserResolver.resolveCurrentUser(authentication); - assertSelf(self, id); - AppUser user = userService.deleteUserById(id); - return ResponseEntity.ok("User deleted: " + user.getUsername()); - } - - private void assertSelf(AppUser authenticated, Long requestedId) { - if (!authenticated.getId().equals(requestedId)) - throw new ResponseStatusException(HttpStatus.FORBIDDEN); - } -} -``` - -- [ ] **Step 4: Run tests to verify they pass** - -Run: `mvn test -Dtest=AppUserControllerTest -pl . -q` - -Expected: `Tests run: 7, Failures: 0, Errors: 0` - -- [ ] **Step 5: Run all tests** - -Run: `mvn test -q` - -Expected: All tests pass. - -- [ ] **Step 6: Commit** - -``` -git add src/main/java/de/zendric/app/xpensely_server/controller/AppUserController.java -git add src/test/java/de/zendric/app/xpensely_server/controller/AppUserControllerTest.java -git commit -m "feat: add ownership guards to user endpoints" -``` - ---- - -## Task 8: Add RateLimitFilter - -**Files:** -- Create: `src/main/java/de/zendric/app/xpensely_server/security/RateLimitFilter.java` -- Modify: `src/main/java/de/zendric/app/xpensely_server/security/SecurityConfig.java` -- Create: `src/test/java/de/zendric/app/xpensely_server/security/RateLimitFilterTest.java` - -- [ ] **Step 1: Write the failing tests** - -Create `src/test/java/de/zendric/app/xpensely_server/security/RateLimitFilterTest.java`: - -```java -package de.zendric.app.xpensely_server.security; - -import jakarta.servlet.FilterChain; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.springframework.mock.web.MockHttpServletRequest; -import org.springframework.mock.web.MockHttpServletResponse; - -import static org.junit.jupiter.api.Assertions.*; -import static org.mockito.Mockito.*; - -class RateLimitFilterTest { - - RateLimitFilter filter; - - @BeforeEach - void setUp() { - filter = new RateLimitFilter(); - } - - @Test - void generalEndpoint_underLimit_allowsRequest() throws Exception { - MockHttpServletRequest request = new MockHttpServletRequest("GET", "/api/expenselist/mine"); - request.setRemoteAddr("10.0.0.1"); - MockHttpServletResponse response = new MockHttpServletResponse(); - FilterChain chain = mock(FilterChain.class); - - filter.doFilter(request, response, chain); - - verify(chain).doFilter(request, response); - assertNotEquals(429, response.getStatus()); - } - - @Test - void createUser_exceedsLimit_returns429() throws Exception { - FilterChain chain = mock(FilterChain.class); - - // Exhaust the 3-request limit - for (int i = 0; i < 3; i++) { - MockHttpServletRequest req = new MockHttpServletRequest("POST", "/api/users/createUser"); - req.setRemoteAddr("10.0.0.2"); - MockHttpServletResponse res = new MockHttpServletResponse(); - filter.doFilter(req, res, chain); - assertNotEquals(429, res.getStatus(), "Request " + (i + 1) + " should be allowed"); - } - - MockHttpServletRequest req = new MockHttpServletRequest("POST", "/api/users/createUser"); - req.setRemoteAddr("10.0.0.2"); - MockHttpServletResponse res = new MockHttpServletResponse(); - filter.doFilter(req, res, chain); - - assertEquals(429, res.getStatus()); - assertNotNull(res.getHeader("Retry-After")); - } - - @Test - void createUser_differentIps_separateBuckets() throws Exception { - FilterChain chain = mock(FilterChain.class); - - // Exhaust limit for IP A - for (int i = 0; i < 3; i++) { - MockHttpServletRequest req = new MockHttpServletRequest("POST", "/api/users/createUser"); - req.setRemoteAddr("10.0.0.3"); - filter.doFilter(req, new MockHttpServletResponse(), chain); - } - - // IP B should still be allowed - MockHttpServletRequest req = new MockHttpServletRequest("POST", "/api/users/createUser"); - req.setRemoteAddr("10.0.0.4"); - MockHttpServletResponse res = new MockHttpServletResponse(); - filter.doFilter(req, res, chain); - - assertNotEquals(429, res.getStatus()); - } - - @Test - void exceedLimit_responseHasRetryAfterHeader() throws Exception { - FilterChain chain = mock(FilterChain.class); - for (int i = 0; i < 3; i++) { - MockHttpServletRequest req = new MockHttpServletRequest("POST", "/api/users/createUser"); - req.setRemoteAddr("10.0.0.5"); - filter.doFilter(req, new MockHttpServletResponse(), chain); - } - - MockHttpServletRequest req = new MockHttpServletRequest("POST", "/api/users/createUser"); - req.setRemoteAddr("10.0.0.5"); - MockHttpServletResponse res = new MockHttpServletResponse(); - filter.doFilter(req, res, chain); - - assertNotNull(res.getHeader("Retry-After")); - assertTrue(Integer.parseInt(res.getHeader("Retry-After")) > 0); - } -} -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `mvn test -Dtest=RateLimitFilterTest -pl . -q` - -Expected: FAIL — `RateLimitFilter` does not exist yet. - -- [ ] **Step 3: Create RateLimitFilter** - -Create `src/main/java/de/zendric/app/xpensely_server/security/RateLimitFilter.java`: - -```java -package de.zendric.app.xpensely_server.security; - -import io.github.bucket4j.Bandwidth; -import io.github.bucket4j.Bucket; -import io.github.bucket4j.Refill; -import jakarta.servlet.FilterChain; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; -import org.springframework.security.core.Authentication; -import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.security.oauth2.jwt.Jwt; -import org.springframework.web.filter.OncePerRequestFilter; - -import java.io.IOException; -import java.time.Duration; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.TimeUnit; - -public class RateLimitFilter extends OncePerRequestFilter { - - private final Map generalBuckets = new ConcurrentHashMap<>(); - private final Map inviteBuckets = new ConcurrentHashMap<>(); - private final Map acceptInviteBuckets = new ConcurrentHashMap<>(); - private final Map createUserBuckets = new ConcurrentHashMap<>(); - - @Override - protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, - FilterChain chain) throws ServletException, IOException { - String key = resolveKey(request); - String uri = request.getRequestURI(); - - Bucket bucket = selectBucket(key, uri); - var probe = bucket.tryConsumeAndReturnRemaining(1); - - if (!probe.isConsumed()) { - long retryAfterSeconds = TimeUnit.NANOSECONDS.toSeconds(probe.getNanosToWaitForRefill()); - response.setStatus(429); - response.setHeader("Retry-After", String.valueOf(Math.max(1, retryAfterSeconds))); - response.setContentType("application/json"); - response.getWriter().write("{\"error\":\"Too many requests\"}"); - return; - } - - chain.doFilter(request, response); - } - - private String resolveKey(HttpServletRequest request) { - Authentication auth = SecurityContextHolder.getContext().getAuthentication(); - if (auth != null && auth.isAuthenticated() && auth.getPrincipal() instanceof Jwt jwt) { - return "user:" + jwt.getSubject(); - } - return "ip:" + request.getRemoteAddr(); - } - - private Bucket selectBucket(String key, String uri) { - if (uri.matches(".*/[0-9]+/invite$")) { - return inviteBuckets.computeIfAbsent(key, k -> createBucket(5, Duration.ofMinutes(1))); - } - if (uri.endsWith("/accept-invite")) { - return acceptInviteBuckets.computeIfAbsent(key, k -> createBucket(10, Duration.ofMinutes(1))); - } - if (uri.endsWith("/createUser")) { - return createUserBuckets.computeIfAbsent(key, k -> createBucket(3, Duration.ofMinutes(1))); - } - return generalBuckets.computeIfAbsent(key, k -> createBucket(60, Duration.ofMinutes(1))); - } - - private Bucket createBucket(int capacity, Duration refillPeriod) { - Bandwidth limit = Bandwidth.classic(capacity, Refill.intervally(capacity, refillPeriod)); - return Bucket.builder().addLimit(limit).build(); - } -} -``` - -- [ ] **Step 4: Register RateLimitFilter in SecurityConfig** - -Replace the entire content of `src/main/java/de/zendric/app/xpensely_server/security/SecurityConfig.java`: - -```java -package de.zendric.app.xpensely_server.security; - -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.Profile; -import org.springframework.security.config.Customizer; -import org.springframework.security.config.annotation.web.builders.HttpSecurity; -import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; -import org.springframework.security.oauth2.server.resource.web.authentication.BearerTokenAuthenticationFilter; -import org.springframework.security.web.SecurityFilterChain; - -@Configuration -@EnableWebSecurity -public class SecurityConfig { - - @Bean - @Profile("test") - public SecurityFilterChain testSecurityFilterChain(HttpSecurity http) throws Exception { - http - .authorizeHttpRequests(auth -> auth.anyRequest().permitAll()) - .csrf(csrf -> csrf.disable()); - return http.build(); - } - - @Bean - @Profile("!test") - public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception { - http - .authorizeHttpRequests(auth -> auth.anyRequest().authenticated()) - .oauth2ResourceServer(oauth2 -> oauth2.jwt(Customizer.withDefaults())) - .oauth2Login(Customizer.withDefaults()) - .csrf(csrf -> csrf.disable()) - .addFilterAfter(new RateLimitFilter(), BearerTokenAuthenticationFilter.class); - return http.build(); - } -} -``` - -- [ ] **Step 5: Run RateLimitFilter tests** - -Run: `mvn test -Dtest=RateLimitFilterTest -pl . -q` - -Expected: `Tests run: 4, Failures: 0, Errors: 0` - -- [ ] **Step 6: Run all tests** - -Run: `mvn test -q` - -Expected: All tests pass, `BUILD SUCCESS`. - -- [ ] **Step 7: Commit** - -``` -git add src/main/java/de/zendric/app/xpensely_server/security/RateLimitFilter.java -git add src/main/java/de/zendric/app/xpensely_server/security/SecurityConfig.java -git add src/test/java/de/zendric/app/xpensely_server/security/RateLimitFilterTest.java -git commit -m "feat: add RateLimitFilter with per-user Bucket4j rate limiting" -``` - ---- - -## Done - -All security hardening is complete. Summary of what was implemented: - -| Area | What changed | -|---|---| -| Input validation | Bean Validation on all request models; `@Valid` on all `@RequestBody`; `GlobalExceptionHandler` returns structured 400s | -| Authorization | `AuthenticatedUserResolver` resolves JWT sub to `AppUser`; all endpoints enforce ownership/membership; dangerous endpoints (`/all`, `/byUser`, `/byUsername`) removed or replaced with `/mine` | -| Rate limiting | `RateLimitFilter` inside security chain; per-user buckets (JWT sub) with IP fallback; 60 req/min general, 5/min invite generation, 10/min invite acceptance, 3/min account creation | -| Client compat | Flutter client must be updated: replace calls to `/byUser?userId=X` with `GET /api/expenselist/mine` (no param needed) | diff --git a/docs/superpowers/specs/2026-05-04-security-hardening-design.md b/docs/superpowers/specs/2026-05-04-security-hardening-design.md deleted file mode 100644 index b2f3f04..0000000 --- a/docs/superpowers/specs/2026-05-04-security-hardening-design.md +++ /dev/null @@ -1,166 +0,0 @@ -# Security Hardening Design — XpenselyServer -**Date:** 2026-05-04 -**Scope:** Input validation, authorization enforcement, rate limiting - ---- - -## Problem Statement - -The XpenselyServer has three security gaps: - -1. **No input validation** — request bodies are accepted without any field constraints, allowing null fields, negative amounts, oversized strings, and malformed data to reach the database layer. -2. **Authorization bypass** — every endpoint trusts caller-supplied user IDs from query params or request bodies rather than the authenticated JWT. Any authenticated user can read or destroy another user's expense lists. -3. **No rate limiting** — no protection against brute-force or abuse of sensitive endpoints (invite generation, account creation, invite acceptance). - ---- - -## Section 1 — Input Validation - -### Dependency -Add `spring-boot-starter-validation` to `pom.xml`. - -### Request Model Constraints - -**`AppUserCreateRequest`** -- `username`: `@NotBlank @Size(min=3, max=30) @Pattern(regexp="^[a-zA-Z0-9_.-]+$")` -- `googleId`: `@NotBlank` - -**`ExpenseInput`** -- `title`: `@NotBlank @Size(max=100)` -- `owner`: `@NotBlank` -- `amount`: `@NotNull @DecimalMin("0.01")` -- `date`: `@NotNull` -- `category`: `@NotBlank` - -**`ExpenseChangeRequest`** — same constraints as `ExpenseInput` for the corresponding fields. - -**`InviteRequest`** -- `inviteCode`: `@NotBlank @Size(min=6, max=6)` -- `userId` field removed (derived from JWT — see Section 2) - -### Controller Changes -Add `@Valid` to every `@RequestBody` parameter in `AppUserController` and `ExpenseListController`. - -### Error Handling -Add a `@ControllerAdvice` class `GlobalExceptionHandler` that: -- Catches `MethodArgumentNotValidException` → returns `400 Bad Request` with a map of `{ field: errorMessage }` pairs -- Catches `IllegalArgumentException` → returns `400 Bad Request` with the exception message - -This replaces the current pattern of returning `500 INTERNAL_SERVER_ERROR` or `417 EXPECTATION_FAILED` for validation failures. - -### Cleanup -Remove the stray `@Id` and `@GeneratedValue` JPA annotations from `ExpenseInput` — it is a DTO, not an entity. - ---- - -## Section 2 — Authorization Model - -### Core Principle -Stop trusting caller-supplied user IDs. Derive the authenticated user from the JWT on every request. - -### New Component: `AuthenticatedUserResolver` -A `@Component` with a single method: -```java -AppUser resolveCurrentUser(Authentication auth) -``` -- Extracts the `sub` claim (Google ID) from the JWT -- Calls `UserService.getUserByGoogleId(sub)` to return the `AppUser` -- Throws `ResponseStatusException(403)` if no user is found for the JWT subject - -### Endpoint Changes - -| Endpoint | Change | -|---|---| -| `GET /api/expenselist/all` | **Removed** — no legitimate non-admin use case | -| `GET /api/expenselist/byUser?userId=X` | **Replaced** by `GET /api/expenselist/mine` — returns lists for the JWT user, no param | -| `GET /api/expenselist/byUsername?username=X` | **Removed** — redundant with `/mine` | -| `GET /api/expenselist/byId?id=X` | **Guard added** — 403 if authenticated user is neither owner nor sharedWith | -| `DELETE /api/expenselist/{id}` | **Guard added** — 403 if authenticated user is not the owner | -| `POST /api/expenselist/{id}/add` | **Guard added** — 403 if authenticated user is not owner or sharedWith | -| `PUT /api/expenselist/{id}/update` | **Guard added** — 403 if authenticated user is not owner or sharedWith | -| `DELETE /api/expenselist/{id}/delete` | **Guard added** — 403 if authenticated user is not owner or sharedWith | -| `POST /api/expenselist/{listId}/invite` | **Guard added** — 403 if authenticated user is not the owner | -| `POST /api/expenselist/accept-invite` | **`userId` removed from body** — derived from JWT instead | -| `GET /api/users?id=X` | **Guard added** — 403 if id doesn't match JWT user's id | -| `GET /api/users/byGoogleId?id=X` | **Guard added** — 403 if id doesn't match JWT sub | -| `DELETE /api/users?id=X` | **Guard added** — 403 if id doesn't match JWT user's id | - -### Ownership Check Helper -Each guard is implemented as a private method in its controller (2–3 lines): -```java -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); -} -``` - -### Test Profile -The `@Profile("test")` security chain in `SecurityConfig` is untouched. Existing tests continue to work without authentication. - ---- - -## Section 3 — Rate Limiting - -### Dependency -Add `bucket4j-core` to `pom.xml`. In-memory storage — no external cache needed for single-instance deployment. - -### Implementation -A `RateLimitFilter` extending `OncePerRequestFilter`, registered as a `@Component` with `@Profile("!test")`: - -- **Key for authenticated requests:** JWT `sub` claim (per-user bucket) -- **Key for unauthenticated requests:** remote IP address (pre-auth fallback) -- Buckets stored in a `ConcurrentHashMap` - -### Limits - -| Endpoint pattern | Limit | -|---|---| -| All endpoints (default) | 60 requests / minute | -| `POST /api/expenselist/*/invite` | 5 requests / minute | -| `POST /api/expenselist/accept-invite` | 10 requests / minute | -| `POST /api/users/createUser` | 3 requests / minute | - -Sensitive endpoints get their own per-user bucket independent of the general bucket. - -### Response -When a bucket is exhausted: `429 Too Many Requests` with a `Retry-After: ` header indicating time until refill. - ---- - -## Architecture Summary - -``` -Request - └── RateLimitFilter (per-user/IP buckets) - └── SecurityFilterChain (JWT validation) - └── Controller - ├── @Valid on @RequestBody → GlobalExceptionHandler on failure - ├── AuthenticatedUserResolver → AppUser from JWT sub - └── assertOwner / assertMember → 403 on violation -``` - -No new service layer is introduced. The authorization checks are lightweight and local to each controller method. - ---- - -## Files Affected - -| File | Change | -|---|---| -| `pom.xml` | Add `spring-boot-starter-validation`, `bucket4j-core` | -| `model/AppUserCreateRequest.java` | Add validation annotations | -| `model/ExpenseInput.java` | Add validation annotations, remove JPA annotations | -| `model/ExpenseChangeRequest.java` | Add validation annotations | -| `model/InviteRequest.java` | Add validation annotations, remove `userId` field | -| `controller/AppUserController.java` | Add `@Valid`, ownership guards, use `AuthenticatedUserResolver` | -| `controller/ExpenseListController.java` | Add `@Valid`, ownership guards, remove/rename endpoints, use `AuthenticatedUserResolver` | -| `security/AuthenticatedUserResolver.java` | **New** — resolves JWT sub to `AppUser` | -| `security/RateLimitFilter.java` | **New** — per-user/IP rate limiting | -| `controller/GlobalExceptionHandler.java` | **New** — structured 400/403 error responses |