4 Commits

8 changed files with 180 additions and 1719 deletions
File diff suppressed because it is too large Load Diff
@@ -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 (23 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<String, Bucket>`
### 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: <seconds>` 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 |
@@ -0,0 +1,11 @@
package de.zendric.app.xpensely_server.model.Exception;
import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.ResponseStatus;
@ResponseStatus(HttpStatus.NOT_FOUND)
public class ResourceNotFoundException extends RuntimeException {
public ResourceNotFoundException(String message) {
super(message);
}
}
@@ -3,13 +3,22 @@ package de.zendric.app.xpensely_server.repo;
import java.util.List;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;
import org.springframework.stereotype.Repository;
import de.zendric.app.xpensely_server.model.ExpenseList;
@Repository
public interface ExpenseListRepository extends JpaRepository<ExpenseList, Long> {
List<ExpenseList> findByOwnerId(Long ownerId);
ExpenseList findByInviteCode(String inviteCode);
@Query("SELECT el FROM ExpenseList el WHERE el.owner.id = :userId OR el.sharedWith.id = :userId")
List<ExpenseList> findByOwnerIdOrSharedWithId(@Param("userId") Long userId);
@Query("SELECT el FROM ExpenseList el WHERE el.owner.username = :username OR el.sharedWith.username = :username")
List<ExpenseList> findByOwnerUsernameOrSharedWithUsername(@Param("username") String username);
}
@@ -1,37 +1,29 @@
package de.zendric.app.xpensely_server.services;
import java.time.LocalDateTime;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.UUID;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import de.zendric.app.xpensely_server.model.AppUser;
import de.zendric.app.xpensely_server.model.Expense;
import de.zendric.app.xpensely_server.model.ExpenseList;
import de.zendric.app.xpensely_server.model.Exception.ResourceNotFoundException;
import de.zendric.app.xpensely_server.model.XpenselyCustomCategory;
import de.zendric.app.xpensely_server.repo.ExpenseListRepository;
import de.zendric.app.xpensely_server.repo.ExpenseRepository;
import de.zendric.app.xpensely_server.repo.XpenselyCustomCategoryRepository;
import jakarta.persistence.EntityManager;
@Service
@Transactional
public class ExpenseListService {
private ExpenseListRepository repository;
private final ExpenseListRepository repository;
private final ExpenseRepository expenseRepository;
private XpenselyCustomCategoryRepository customCategoryRepository;
private final XpenselyCustomCategoryRepository customCategoryRepository;
@Autowired
private EntityManager entityManager;
@Autowired
public ExpenseListService(ExpenseListRepository repository, ExpenseRepository expenseRepository,
XpenselyCustomCategoryRepository customCategoryRepository) {
this.repository = repository;
@@ -68,67 +60,24 @@ public class ExpenseListService {
}
public List<ExpenseList> findByUserId(Long id) {
List<ExpenseList> allLists = repository.findAll();
List<ExpenseList> userSpecificList = new ArrayList<>();
for (ExpenseList expenseList : allLists) {
AppUser sharedWith = expenseList.getSharedWith();
if (expenseList.getOwner().getId().equals(id)) {
userSpecificList.add(expenseList);
} else {
if (sharedWith != null && sharedWith.getId().equals(id)) {
userSpecificList.add(expenseList);
}
}
}
return userSpecificList;
return repository.findByOwnerIdOrSharedWithId(id);
}
public List<ExpenseList> findByUsername(String username) {
List<ExpenseList> allLists = repository.findAll();
List<ExpenseList> userSpecificList = new ArrayList<>();
for (ExpenseList expenseList : allLists) {
AppUser sharedWith = expenseList.getSharedWith();
if (expenseList.getOwner().getUsername().equals(username)) {
userSpecificList.add(expenseList);
} else {
if (sharedWith != null && sharedWith.getUsername().equals(username)) {
userSpecificList.add(expenseList);
}
}
}
return userSpecificList;
return repository.findByOwnerUsernameOrSharedWithUsername(username);
}
public Expense addExpenseToList(Long expenseListId, Expense expense) {
// find expenseList
ExpenseList expenseList = repository.findById(expenseListId)
.orElseThrow(() -> new RuntimeException("ExpenseList not found with id: " + expenseListId));
// get all added expenses
HashSet<Long> existingId = new HashSet<>();
for (Expense e : expenseList.getExpenses()) {
existingId.add(e.getId());
}
// add the new expense
.orElseThrow(() -> new ResourceNotFoundException("ExpenseList not found with id: " + expenseListId));
expenseList.addExpense(expense);
// save
repository.save(expenseList);
Expense newExpense = new Expense();
for (Expense e : expenseList.getExpenses()) {
if (!existingId.contains(e.getId())) {
newExpense = e;
break;
}
}
return newExpense;
return expense;
}
public void deleteExpenseFromList(Long expenseListId, Long expenseId) {
ExpenseList expenseList = repository.findById(expenseListId)
.orElseThrow(() -> new RuntimeException("ExpenseList not found with id: " + expenseListId));
.orElseThrow(() -> new ResourceNotFoundException("ExpenseList not found with id: " + expenseListId));
Expense expenseToRemove = null;
for (Expense expense : expenseList.getExpenses()) {
if (expense.getId().equals(expenseId)) {
@@ -139,14 +88,14 @@ public class ExpenseListService {
if (expenseToRemove != null) {
expenseList.removeExpense(expenseToRemove);
} else {
throw new RuntimeException("Expense not found with id: " + expenseId);
throw new ResourceNotFoundException("Expense not found with id: " + expenseId);
}
repository.save(expenseList);
}
public String generateInviteCode(Long listId) {
ExpenseList list = repository.findById(listId)
.orElseThrow(() -> new RuntimeException("List not found"));
.orElseThrow(() -> new ResourceNotFoundException("List not found"));
String inviteCode;
if (list.getInviteCode() == null || list.getInviteCodeExpiration().isBefore(LocalDateTime.now())) {
@@ -168,7 +117,7 @@ public class ExpenseListService {
public Expense updateExpense(Long expenseListId, Expense updatedExpense) {
ExpenseList expenseList = repository.findById(expenseListId)
.orElseThrow(() -> new IllegalArgumentException("ExpenseList not found"));
.orElseThrow(() -> new ResourceNotFoundException("ExpenseList not found with id: " + expenseListId));
if (!expenseList.getExpenses().stream()
.anyMatch(expense -> expense.getId().equals(updatedExpense.getId()))) {
@@ -176,7 +125,7 @@ public class ExpenseListService {
}
Expense existingExpense = expenseRepository.findById(updatedExpense.getId())
.orElseThrow(() -> new IllegalArgumentException("Expense not found"));
.orElseThrow(() -> new ResourceNotFoundException("Expense not found with id: " + updatedExpense.getId()));
existingExpense.setTitle(updatedExpense.getTitle());
existingExpense.setAmount(updatedExpense.getAmount());
existingExpense.setPersonalUseAmount(updatedExpense.getPersonalUseAmount());
@@ -191,7 +140,7 @@ public class ExpenseListService {
// TODO implement API for this
public XpenselyCustomCategory addCustomCategory(Long expenseListId, XpenselyCustomCategory customCategory) {
ExpenseList expenseList = repository.findById(expenseListId)
.orElseThrow(() -> new RuntimeException("Expense List not found"));
.orElseThrow(() -> new ResourceNotFoundException("Expense List not found"));
customCategory.setExpenseList(expenseList);
return customCategoryRepository.save(customCategory);
@@ -200,9 +149,9 @@ public class ExpenseListService {
// TODO implement API for this
public void deleteCustomCategory(Long expenseListId, Long categoryId) {
XpenselyCustomCategory category = customCategoryRepository.findById(categoryId)
.orElseThrow(() -> new RuntimeException("Custom Category not found"));
.orElseThrow(() -> new ResourceNotFoundException("Custom Category not found"));
if (!category.getExpenseList().getId().equals(expenseListId)) {
throw new RuntimeException("Category does not belong to the specified Expense List");
throw new IllegalArgumentException("Category does not belong to the specified Expense List");
}
customCategoryRepository.delete(category);
}
@@ -1,11 +1,11 @@
package de.zendric.app.xpensely_server.services;
import java.util.List;
import java.util.Optional;
import org.springframework.stereotype.Service;
import de.zendric.app.xpensely_server.model.AppUser;
import de.zendric.app.xpensely_server.model.Exception.ResourceNotFoundException;
import de.zendric.app.xpensely_server.model.Exception.UsernameAlreadyExistsException;
import de.zendric.app.xpensely_server.repo.UserRepository;
@@ -29,36 +29,24 @@ public class UserService {
}
public AppUser getUser(Long id) {
Optional<AppUser> user = userRepository.findById(id);
if (user.isPresent()) {
return user.get();
} else
return null;
return userRepository.findById(id)
.orElseThrow(() -> new ResourceNotFoundException("User not found with id: " + id));
}
public AppUser deleteUserById(Long id) {
Optional<AppUser> user = userRepository.findById(id);
if (user.isPresent()) {
AppUser user = userRepository.findById(id)
.orElseThrow(() -> new ResourceNotFoundException("User not found with id: " + id));
userRepository.deleteById(id);
return user.get();
} else
return null;
return user;
}
public AppUser getUserByName(String username) {
Optional<AppUser> optUser = userRepository.findByUsername(username);
if (optUser.isPresent()) {
return optUser.get();
} else
return null;
return userRepository.findByUsername(username)
.orElseThrow(() -> new ResourceNotFoundException("User not found: " + username));
}
public AppUser getUserByGoogleId(String id) {
Optional<AppUser> optUser = userRepository.findByGoogleId(id);
if (optUser.isPresent()) {
return optUser.get();
} else
return null;
return userRepository.findByGoogleId(id)
.orElseThrow(() -> new ResourceNotFoundException("User not found with Google ID: " + id));
}
}
@@ -0,0 +1,57 @@
package de.zendric.app.xpensely_Server.services;
import de.zendric.app.xpensely_server.model.AppUser;
import de.zendric.app.xpensely_server.model.ExpenseList;
import de.zendric.app.xpensely_server.repo.ExpenseListRepository;
import de.zendric.app.xpensely_server.repo.ExpenseRepository;
import de.zendric.app.xpensely_server.repo.XpenselyCustomCategoryRepository;
import de.zendric.app.xpensely_server.services.ExpenseListService;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import java.util.List;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.never;
@ExtendWith(MockitoExtension.class)
class ExpenseListServiceTest {
@Mock ExpenseListRepository repository;
@Mock ExpenseRepository expenseRepository;
@Mock XpenselyCustomCategoryRepository customCategoryRepository;
@InjectMocks
ExpenseListService service;
@Test
void findByUserId_usesRepositoryQuery_notFindAll() {
AppUser owner = new AppUser(); owner.setId(1L);
ExpenseList list = new ExpenseList(); list.setId(10L); list.setOwner(owner);
when(repository.findByOwnerIdOrSharedWithId(1L)).thenReturn(List.of(list));
List<ExpenseList> result = service.findByUserId(1L);
assertThat(result).hasSize(1);
verify(repository).findByOwnerIdOrSharedWithId(1L);
verify(repository, never()).findAll();
}
@Test
void findByUsername_usesRepositoryQuery_notFindAll() {
AppUser owner = new AppUser(); owner.setId(1L); owner.setUsername("alice");
ExpenseList list = new ExpenseList(); list.setId(10L); list.setOwner(owner);
when(repository.findByOwnerUsernameOrSharedWithUsername("alice")).thenReturn(List.of(list));
List<ExpenseList> result = service.findByUsername("alice");
assertThat(result).hasSize(1);
verify(repository).findByOwnerUsernameOrSharedWithUsername("alice");
verify(repository, never()).findAll();
}
}
@@ -0,0 +1,75 @@
package de.zendric.app.xpensely_Server.services;
import de.zendric.app.xpensely_server.model.AppUser;
import de.zendric.app.xpensely_server.model.Exception.ResourceNotFoundException;
import de.zendric.app.xpensely_server.repo.UserRepository;
import de.zendric.app.xpensely_server.services.UserService;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import java.util.Optional;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.Mockito.when;
@ExtendWith(MockitoExtension.class)
class UserServiceTest {
@Mock
UserRepository userRepository;
@InjectMocks
UserService userService;
@Test
void getUserByName_throwsResourceNotFound_whenUserMissing() {
when(userRepository.findByUsername("ghost")).thenReturn(Optional.empty());
assertThatThrownBy(() -> userService.getUserByName("ghost"))
.isInstanceOf(ResourceNotFoundException.class)
.hasMessageContaining("ghost");
}
@Test
void getUserByName_returnsUser_whenFound() {
AppUser user = new AppUser();
user.setId(1L);
user.setUsername("alice");
when(userRepository.findByUsername("alice")).thenReturn(Optional.of(user));
AppUser result = userService.getUserByName("alice");
assertThat(result.getUsername()).isEqualTo("alice");
}
@Test
void getUser_throwsResourceNotFound_whenIdMissing() {
when(userRepository.findById(99L)).thenReturn(Optional.empty());
assertThatThrownBy(() -> userService.getUser(99L))
.isInstanceOf(ResourceNotFoundException.class)
.hasMessageContaining("99");
}
@Test
void deleteUserById_throwsResourceNotFound_whenIdMissing() {
when(userRepository.findById(99L)).thenReturn(Optional.empty());
assertThatThrownBy(() -> userService.deleteUserById(99L))
.isInstanceOf(ResourceNotFoundException.class)
.hasMessageContaining("99");
}
@Test
void getUserByGoogleId_throwsResourceNotFound_whenMissing() {
when(userRepository.findByGoogleId("gid-404")).thenReturn(Optional.empty());
assertThatThrownBy(() -> userService.getUserByGoogleId("gid-404"))
.isInstanceOf(ResourceNotFoundException.class)
.hasMessageContaining("gid-404");
}
}