fix: bug desactiveAccount(), UserService without AuditService and restoreWallet() without audit log

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.';
+    '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';

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
 
     /**

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,

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<UserResponse> 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<UserResponse> updateProfile(UUID userId, UpdateProfileRequest
     public ApiResponse<Void> 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");
     }
 

src/main/java/com/wallet/secure/wallet/service/WalletService.java

@@ -239,6 +239,10 @@ public ApiResponse<WalletResponse> 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));

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<Void> 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<Void> 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());
         }
     }