diff --git a/database/08-migrations.sql b/database/08-migrations.sql index de56775..1ba226d 100644 --- a/database/08-migrations.sql +++ b/database/08-migrations.sql @@ -11,4 +11,12 @@ CREATE INDEX IF NOT EXISTS idx_users_refresh_token WHERE refresh_token IS NOT NULL; COMMENT ON COLUMN users.refresh_token IS - 'Active refresh token. NULL = logged out. Invalidated on logout.'; \ No newline at end of file + 'Active refresh token. NULL = logged out. Invalidated on logout.'; + +-- Migration: add WALLET_RESTORE to audit_action enum +-- Date: 2026-05-02 +-- Reason: restoreWallet() is an ADMIN action and must be auditable (OWASP A09). +-- All other wallet admin ops (suspend, close) already have their own action. +-- Using ALTER TYPE ... ADD VALUE because PostgreSQL enums are append-only. + +ALTER TYPE audit_action ADD VALUE IF NOT EXISTS 'WALLET_RESTORE'; \ No newline at end of file diff --git a/src/main/java/com/wallet/secure/audit/service/AuditService.java b/src/main/java/com/wallet/secure/audit/service/AuditService.java index b6db81e..dcd99a0 100644 --- a/src/main/java/com/wallet/secure/audit/service/AuditService.java +++ b/src/main/java/com/wallet/secure/audit/service/AuditService.java @@ -227,6 +227,27 @@ public void logWalletClosed(UUID adminId, UUID walletId, details, ipAddress, userAgent)); } + /** + * Logs a wallet restoration from SUSPENDED to ACTIVE — ADMIN action. + * Called by WalletService.restoreWallet(). + * WHY this needs its own audit entry: restoring a wallet re-enables all + * financial operations on it. That is a security-relevant state change + * that must be traceable to the admin who performed it. + * OWASP A09: all wallet state changes by admins must be logged. + */ + @Async + @Transactional(propagation = Propagation.REQUIRES_NEW) + public void logWalletRestored(UUID adminId, UUID walletId, + String ipAddress, String userAgent) { + String details = buildDetails( + "\"walletId\":\"" + walletId + "\"", + "\"adminId\":\"" + adminId + "\"", + "\"outcome\":\"SUCCESS\"" + ); + save(AuditLog.success(adminId, AuditAction.WALLET_RESTORE, LogSeverity.WARNING, + details, ipAddress, userAgent)); + } + // ─── Security Events /** diff --git a/src/main/java/com/wallet/secure/common/enums/AuditAction.java b/src/main/java/com/wallet/secure/common/enums/AuditAction.java index f4cff43..03be828 100644 --- a/src/main/java/com/wallet/secure/common/enums/AuditAction.java +++ b/src/main/java/com/wallet/secure/common/enums/AuditAction.java @@ -25,6 +25,7 @@ public enum AuditAction { WALLET_CREATE, WALLET_SUSPEND, WALLET_CLOSE, + WALLET_RESTORE, // Transaction lifecycle TRANSACTION_CREATE, diff --git a/src/main/java/com/wallet/secure/user/service/UserService.java b/src/main/java/com/wallet/secure/user/service/UserService.java index 2a6fb7c..3296999 100644 --- a/src/main/java/com/wallet/secure/user/service/UserService.java +++ b/src/main/java/com/wallet/secure/user/service/UserService.java @@ -1,10 +1,13 @@ package com.wallet.secure.user.service; +import com.wallet.secure.audit.service.AuditService; +import com.wallet.secure.common.enums.UserRole; import com.wallet.secure.common.exception.EmailAlreadyExistsException; import com.wallet.secure.common.exception.InvalidCredentialsException; import com.wallet.secure.common.exception.UnauthorizedOperationException; import com.wallet.secure.common.exception.UserNotFoundException; import com.wallet.secure.common.response.ApiResponse; +import com.wallet.secure.common.util.LogSanitizer; import com.wallet.secure.user.dto.RegisterRequest; import com.wallet.secure.user.dto.UpdateProfileRequest; import com.wallet.secure.user.dto.UserResponse; @@ -15,7 +18,6 @@ import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; -import com.wallet.secure.common.util.LogSanitizer; import java.util.UUID; @@ -42,6 +44,7 @@ public class UserService { private final UserRepository userRepository; private final PasswordEncoder passwordEncoder; + private final AuditService auditService; // ─── Registration ───────────────────────────────────────────────────────── @@ -149,6 +152,8 @@ public ApiResponse updateProfile(UUID userId, UpdateProfileRequest user.setPasswordHash(passwordEncoder.encode(request.getPassword())); log.info("Password changed: userId={}", userId); updated = true; + // OWASP A09: password change is a high-sensitivity event — always audited + auditService.logPasswordChange(userId, user.getEmail(), null, null); } if (!updated) { @@ -177,18 +182,32 @@ public ApiResponse updateProfile(UUID userId, UpdateProfileRequest public ApiResponse deactivateAccount(UUID userId, UUID requesterId) { User user = findUserById(userId); - // OWASP A01: only the user or an ADMIN can deactivate an account - // Fine-grained check — controller also uses @PreAuthorize - if (!userId.equals(requesterId)) { - log.warn("Unauthorized deactivation attempt: requesterId={} tried userId={}", - requesterId, userId); - throw new UnauthorizedOperationException("Not authorized to deactivate this account"); + // OWASP A01: only the account owner OR an ADMIN can deactivate an account. + // WHY we also load the requester: comparing UUIDs alone is not enough — + // an ADMIN will always have a different UUID than the target user. + // We must verify the requester's role to allow cross-user deactivation. + boolean isSelf = userId.equals(requesterId); + if (!isSelf) { + User requester = findUserById(requesterId); + boolean isAdmin = UserRole.ADMIN.equals(requester.getRole()); + if (!isAdmin) { + log.warn("Unauthorized deactivation attempt: requesterId={} tried userId={}", + requesterId, userId); + throw new UnauthorizedOperationException("Not authorized to deactivate this account"); + } } user.setIsActive(false); userRepository.save(user); - log.info("Account deactivated: userId={}", userId); + // OWASP A09: account deactivation is a high-sensitivity operation — always audited + auditService.logCriticalSecurityEvent( + userId, + "Account deactivated by " + (userId.equals(requesterId) ? "owner" : "ADMIN requesterId=" + requesterId), + null, null + ); + + log.info("Account deactivated: userId={} by requesterId={}", userId, requesterId); return ApiResponse.ok("Account deactivated successfully"); } diff --git a/src/main/java/com/wallet/secure/wallet/service/WalletService.java b/src/main/java/com/wallet/secure/wallet/service/WalletService.java index 6c83ab8..6c160e1 100644 --- a/src/main/java/com/wallet/secure/wallet/service/WalletService.java +++ b/src/main/java/com/wallet/secure/wallet/service/WalletService.java @@ -239,6 +239,10 @@ public ApiResponse restoreWallet(UUID walletId) { wallet.setStatus(WalletStatus.ACTIVE); Wallet saved = walletRepository.save(wallet); + // OWASP A09: wallet restoration re-enables all financial operations on it — + // this is a security-relevant state change that must be traceable to the admin. + auditService.logWalletRestored(wallet.getUser().getId(), walletId, null, null); + log.warn("Wallet restored to ACTIVE: walletId={}", walletId); return ApiResponse.ok("Wallet restored", WalletResponse.fromEntity(saved)); diff --git a/src/test/java/com/wallet/secure/user/service/UserServiceTest.java b/src/test/java/com/wallet/secure/user/service/UserServiceTest.java index 68d74f6..f5781bc 100644 --- a/src/test/java/com/wallet/secure/user/service/UserServiceTest.java +++ b/src/test/java/com/wallet/secure/user/service/UserServiceTest.java @@ -1,7 +1,9 @@ package com.wallet.secure.user.service; +import com.wallet.secure.audit.service.AuditService; import com.wallet.secure.common.exception.EmailAlreadyExistsException; import com.wallet.secure.common.exception.InvalidCredentialsException; +import com.wallet.secure.common.exception.UnauthorizedOperationException; import com.wallet.secure.common.exception.UserNotFoundException; import com.wallet.secure.common.response.ApiResponse; import com.wallet.secure.common.enums.UserRole; @@ -49,6 +51,9 @@ class UserServiceTest { @Mock private PasswordEncoder passwordEncoder; + @Mock + private AuditService auditService; + @InjectMocks private UserService userService; @@ -221,10 +226,8 @@ void shouldUpdatePasswordSuccessfully() { setField(request, "password", "NewPass12!"); when(userRepository.findById(testUserId)).thenReturn(Optional.of(testUser)); - // currentPassword matches the stored hash when(passwordEncoder.matches("OldPass12!", "$2a$12$hashedPassword")) .thenReturn(true); - // new password is different from current when(passwordEncoder.matches("NewPass12!", "$2a$12$hashedPassword")) .thenReturn(false); when(passwordEncoder.encode("NewPass12!")).thenReturn("$2a$12$newHash"); @@ -237,6 +240,8 @@ void shouldUpdatePasswordSuccessfully() { assertThat(response.isSuccess()).isTrue(); verify(passwordEncoder).encode("NewPass12!"); verify(userRepository).save(any(User.class)); + // OWASP A09: password change MUST generate an audit log entry + verify(auditService).logPasswordChange(eq(testUserId), anyString(), isNull(), isNull()); } @Test @@ -289,17 +294,69 @@ class DeactivateTests { @Test @DisplayName("Should deactivate own account successfully") void shouldDeactivateOwnAccount() { - // ARRANGE — user deactivates themselves (userId == requesterId) + // GIVEN — user deactivates themselves (userId == requesterId) when(userRepository.findById(testUserId)).thenReturn(Optional.of(testUser)); when(userRepository.save(any(User.class))).thenReturn(testUser); - // ACT + // WHEN ApiResponse response = userService.deactivateAccount(testUserId, testUserId); - // ASSERT + // THEN + assertThat(response.isSuccess()).isTrue(); + verify(userRepository).save(argThat(u -> !u.getIsActive())); + // OWASP A09: deactivation is a critical event — must be audited + verify(auditService).logCriticalSecurityEvent(eq(testUserId), anyString(), isNull(), isNull()); + } + + @Test + @DisplayName("Should allow ADMIN to deactivate another user") + void shouldAllowAdminToDeactivateAnotherUser() { + // GIVEN — admin (different UUID) deactivates target user + UUID adminId = UUID.randomUUID(); + User adminUser = User.builder() + .email("admin@test.com") + .passwordHash("$2a$12$adminHash") + .role(UserRole.ADMIN) + .build(); + adminUser.setId(adminId); + + // findById is called twice: once for the target, once for the requester + when(userRepository.findById(testUserId)).thenReturn(Optional.of(testUser)); + when(userRepository.findById(adminId)).thenReturn(Optional.of(adminUser)); + when(userRepository.save(any(User.class))).thenReturn(testUser); + + // WHEN + ApiResponse response = userService.deactivateAccount(testUserId, adminId); + + // THEN assertThat(response.isSuccess()).isTrue(); - // Verify isActive was set to false verify(userRepository).save(argThat(u -> !u.getIsActive())); + verify(auditService).logCriticalSecurityEvent(eq(testUserId), anyString(), isNull(), isNull()); + } + + @Test + @DisplayName("Should throw UnauthorizedOperationException when non-ADMIN tries to deactivate another user") + void shouldThrowWhenNonAdminTriesToDeactivateAnotherUser() { + // GIVEN — a regular USER tries to deactivate someone else's account + UUID intruderId = UUID.randomUUID(); + User intruder = User.builder() + .email("intruder@test.com") + .passwordHash("$2a$12$hash") + .role(UserRole.USER) + .build(); + intruder.setId(intruderId); + + when(userRepository.findById(testUserId)).thenReturn(Optional.of(testUser)); + when(userRepository.findById(intruderId)).thenReturn(Optional.of(intruder)); + + // WHEN / THEN + assertThatThrownBy(() -> userService.deactivateAccount(testUserId, intruderId)) + .isInstanceOf(UnauthorizedOperationException.class); + + // Account must NOT have been deactivated + verify(userRepository, never()).save(any()); + // No audit log for a blocked attempt + verify(auditService, never()).logCriticalSecurityEvent(any(), any(), any(), any()); } }