fix(authz): enforce regular-user administration boundary

docs/IMPLEMENTATION-PLAN.md

@@ -291,7 +291,7 @@ Resolved. `JwtAuthFilter` now uses the reloaded `UserDetails` account state befo
 
 **Current problem**
 
-Administrative read/deactivation operations exist, but there is no consolidated and verified administrative user-management contract. The confirmed model allows an administrator to manage regular users but not create or promote other administrators through the API.
+Resolved. Administrative endpoints enforce `ROLE_ADMIN`, service rules limit cross-account deactivation to regular users, public registration fixes the `USER` role, and deactivation audit records contain immutable actor and target identifiers.
 
 **Expected behavior**
 
@@ -311,11 +311,18 @@ Administrative read/deactivation operations exist, but there is no consolidated
 
 **Acceptance criteria**
 
-- [ ] `USER` receives `403` for every administrative endpoint.
-- [ ] `ADMIN` can manage only operations explicitly allowed for regular users.
-- [ ] API requests containing privileged role changes are rejected or ignored safely and never persisted.
-- [ ] No API route can create or promote another administrator.
-- [ ] Administrative actions produce immutable audit records.
+- [x] `USER` receives `403` for every administrative endpoint.
+- [x] `ADMIN` can manage only operations explicitly allowed for regular users.
+- [x] API requests containing privileged role changes are rejected or ignored safely and never persisted.
+- [x] No API route can create or promote another administrator.
+- [x] Administrative actions produce immutable audit records.
+
+**Verification note — 2026-11-02**
+
+- `@PreAuthorize("hasRole('ADMIN')")` returns `403` for a `USER` on both user-administration endpoints; `GlobalExceptionHandler` now maps Spring Security `AccessDeniedException` to the same consistent `ApiResponse` contract instead of a `500`.
+- `UserService` accepts ADMIN deactivation of a `USER`, rejects attempts to deactivate another `ADMIN`, and registration persists the fixed `USER` role because `RegisterRequest` has no role field.
+- `AuditService.logUserDeactivation()` writes `USER_DELETE` evidence with immutable actor and target UUIDs.
+- `./scripts/test.sh` passed with 0 failures and 0 errors.
 
 ### AUTHZ-002 — Review wallet permissions and ownership
 

src/main/java/com/wallet/secure/audit/service/AuditService.java

@@ -147,6 +147,26 @@ public void logRegister(UUID userId, String email, String ipAddress, String user
                 details, ipAddress, userAgent));
     }
 
+    /**
+     * Records a successful account deactivation with both actor and target IDs.
+     *
+     * WHY userId is the actor: audit_logs.user_id identifies who performed the
+     * sensitive operation; targetUserId remains in immutable JSONB details for
+     * an administrator's forensic timeline (OWASP A01/A09).
+     */
+    @Async
+    @Transactional(propagation = Propagation.REQUIRES_NEW)
+    public void logUserDeactivation(UUID actorUserId, UUID targetUserId, boolean selfService) {
+        String details = buildDetails(
+                "\"actorUserId\":\"" + actorUserId + "\"",
+                "\"targetUserId\":\"" + targetUserId + "\"",
+                "\"mode\":\"" + (selfService ? "SELF_SERVICE" : "ADMIN") + "\"",
+                "\"outcome\":\"SUCCESS\""
+        );
+        save(AuditLog.success(actorUserId, AuditAction.USER_DELETE, LogSeverity.WARNING,
+                details, null, null));
+    }
+
     // ─── Transaction Events
 
     /**

src/main/java/com/wallet/secure/common/exception/GlobalExceptionHandler.java

@@ -5,6 +5,7 @@
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.validation.FieldError;
+import org.springframework.security.access.AccessDeniedException;
 import org.springframework.web.bind.MethodArgumentNotValidException;
 import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.RestControllerAdvice;
@@ -106,7 +107,7 @@ public ResponseEntity<ApiResponse<Void>> handleInvalidCredentials(
 
     /**
      * 423 — Account locked after too many failed attempts.
-     * Triggered by: AuthService (future) when failedLoginAttempts >= 3.
+     * Triggered by: AuthService when failedLoginAttempts reaches the configured threshold.
      * OWASP A07: brute force protection.
      */
     @ExceptionHandler(AccountLockedException.class)
@@ -132,6 +133,21 @@ public ResponseEntity<ApiResponse<Void>> handleUnauthorizedOperation(
                 .body(ApiResponse.error(ex.getMessage()));
     }
 
+    /**
+     * 403 — Spring Security method authorization rejected an authenticated user.
+     * Triggered before a @PreAuthorize-protected controller method executes.
+     *
+     * OWASP A01: return a consistent denial response instead of letting the
+     * generic handler turn an authorization denial into a misleading 500.
+     */
+    @ExceptionHandler(AccessDeniedException.class)
+    public ResponseEntity<ApiResponse<Void>> handleAccessDenied(AccessDeniedException ex) {
+        log.warn("Access denied by Spring Security");
+        return ResponseEntity
+                .status(HttpStatus.FORBIDDEN)
+                .body(ApiResponse.error("Access denied"));
+    }
+
     /**
      * 423 — Account locked by Spring Security (LockedException).
      * Thrown by Spring Security when UserDetails.isAccountLocked() = true.

src/main/java/com/wallet/secure/user/service/UserService.java

@@ -40,9 +40,6 @@
 @Log4j2
 public class UserService {
 
-    private static final int MAX_FAILED_ATTEMPTS = 3;
-    private static final int LOCK_DURATION_MINUTES = 30;
-
     private final UserRepository userRepository;
     private final PasswordEncoder passwordEncoder;
     private final AuditService auditService;
@@ -197,6 +194,14 @@ public ApiResponse<Void> deactivateAccount(UUID userId, UUID requesterId) {
                         requesterId, userId);
                 throw new UnauthorizedOperationException("Not authorized to deactivate this account");
             }
+
+            // OWASP A01: administrators may manage regular accounts only. An API
+            // cannot be used to alter another privileged account, even by ADMIN.
+            if (UserRole.ADMIN.equals(user.getRole())) {
+                log.warn("Blocked attempt to deactivate another ADMIN: requesterId={} targetUserId={}",
+                        requesterId, userId);
+                throw new UnauthorizedOperationException("Not authorized to deactivate this account");
+            }
         }
 
         user.setIsActive(false);
@@ -207,12 +212,8 @@ public ApiResponse<Void> deactivateAccount(UUID userId, UUID requesterId) {
         // token on its next request using the reloaded isActive state.
         sessionService.revokeAllSessions(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
-        );
+        // OWASP A09: immutable audit evidence records both actor and target.
+        auditService.logUserDeactivation(requesterId, userId, isSelf);
 
         log.info("Account deactivated: userId={} by requesterId={}", userId, requesterId);
         return ApiResponse.ok("Account deactivated successfully");

src/test/java/com/wallet/secure/audit/service/AuditServiceTest.java

@@ -68,6 +68,30 @@ void setUp() {
 
     // ─── Authentication Events
 
+    @Nested
+    @DisplayName("logUserDeactivation()")
+    class LogUserDeactivationTests {
+
+        @Test
+        @DisplayName("records actor and target IDs as immutable USER_DELETE evidence")
+        void logUserDeactivation_recordsActorAndTarget() {
+            UUID targetUserId = UUID.randomUUID();
+
+            auditService.logUserDeactivation(userId, targetUserId, false);
+
+            ArgumentCaptor<AuditLog> captor = ArgumentCaptor.forClass(AuditLog.class);
+            verify(auditLogRepository).save(captor.capture());
+            AuditLog saved = captor.getValue();
+            assertThat(saved.getUserId()).isEqualTo(userId);
+            assertThat(saved.getAction()).isEqualTo(AuditAction.USER_DELETE);
+            assertThat(saved.getSeverityLevel()).isEqualTo(LogSeverity.WARNING);
+            assertThat(saved.getDetails())
+                    .contains("\"actorUserId\":\"" + userId + "\"")
+                    .contains("\"targetUserId\":\"" + targetUserId + "\"")
+                    .contains("\"mode\":\"ADMIN\"");
+        }
+    }
+
     @Nested
     @DisplayName("logLoginSuccess()")
     class LogLoginSuccessTests {
@@ -500,4 +524,4 @@ void countRecentFailedLogins_passesCorrectTimeWindow() {
             assertThat(since).isBefore(Instant.now());
         }
     }
-}
+}

src/test/java/com/wallet/secure/config/SecurityConfigTest.java

@@ -7,8 +7,12 @@
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.test.context.ActiveProfiles;
 import org.springframework.test.web.servlet.MockMvc;
+import org.springframework.security.test.context.support.WithMockUser;
+
+import java.util.UUID;
 
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
 /**
@@ -43,4 +47,20 @@ void actuatorHealth_isPublic() throws Exception {
         mockMvc.perform(get("/actuator/health"))
                 .andExpect(status().isOk());
     }
+
+    @Test
+    @WithMockUser(roles = "USER")
+    @DisplayName("Regular user receives 403 from an ADMIN user-management endpoint")
+    void adminUserEndpoint_regularUser_returns403() throws Exception {
+        mockMvc.perform(get("/users/{id}", UUID.randomUUID()))
+                .andExpect(status().isForbidden());
+    }
+
+    @Test
+    @WithMockUser(roles = "USER")
+    @DisplayName("Regular user receives 403 when trying to deactivate another account")
+    void adminDeactivationEndpoint_regularUser_returns403() throws Exception {
+        mockMvc.perform(delete("/users/{id}", UUID.randomUUID()))
+                .andExpect(status().isForbidden());
+    }
 }

src/test/java/com/wallet/secure/user/service/UserServiceTest.java

@@ -19,6 +19,7 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.InjectMocks;
+import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.springframework.security.crypto.password.PasswordEncoder;
@@ -112,7 +113,10 @@ void shouldRegisterUserSuccessfully() {
             // Verify BCrypt was called — password must be hashed
             verify(passwordEncoder).encode("SecurePass12!");
             // Verify user was saved to DB
-            verify(userRepository).save(any(User.class));
+            ArgumentCaptor<User> savedUser = ArgumentCaptor.forClass(User.class);
+            verify(userRepository).save(savedUser.capture());
+            // OWASP A01: RegisterRequest has no role field and service fixes USER.
+            assertThat(savedUser.getValue().getRole()).isEqualTo(UserRole.USER);
         }
 
         /**
@@ -309,8 +313,7 @@ void shouldDeactivateOwnAccount() {
             assertThat(response.isSuccess()).isTrue();
             verify(userRepository).save(argThat(u -> !u.getIsActive()));
             verify(sessionService).revokeAllSessions(testUserId);
-            // OWASP A09: deactivation is a critical event — must be audited
-            verify(auditService).logCriticalSecurityEvent(eq(testUserId), anyString(), isNull(), isNull());
+            verify(auditService).logUserDeactivation(testUserId, testUserId, true);
         }
 
         @Test
@@ -337,7 +340,7 @@ void shouldAllowAdminToDeactivateAnotherUser() {
             assertThat(response.isSuccess()).isTrue();
             verify(userRepository).save(argThat(u -> !u.getIsActive()));
             verify(sessionService).revokeAllSessions(testUserId);
-            verify(auditService).logCriticalSecurityEvent(eq(testUserId), anyString(), isNull(), isNull());
+            verify(auditService).logUserDeactivation(adminId, testUserId, false);
         }
 
         @Test
@@ -361,8 +364,30 @@ void shouldThrowWhenNonAdminTriesToDeactivateAnotherUser() {
 
             // 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());
+            verify(auditService, never()).logUserDeactivation(any(), any(), anyBoolean());
+        }
+
+        @Test
+        @DisplayName("Should reject an ADMIN trying to deactivate another ADMIN")
+        void shouldRejectAdminDeactivationOfAnotherAdmin() {
+            // GIVEN
+            UUID requesterId = UUID.randomUUID();
+            User targetAdmin = User.builder().email("target-admin@test.com")
+                    .passwordHash("$2a$12$targetAdminHash").role(UserRole.ADMIN).build();
+            targetAdmin.setId(testUserId);
+            User requesterAdmin = User.builder().email("requester-admin@test.com")
+                    .passwordHash("$2a$12$requesterAdminHash").role(UserRole.ADMIN).build();
+            requesterAdmin.setId(requesterId);
+            when(userRepository.findById(testUserId)).thenReturn(Optional.of(targetAdmin));
+            when(userRepository.findById(requesterId)).thenReturn(Optional.of(requesterAdmin));
+
+            // WHEN / THEN
+            assertThatThrownBy(() -> userService.deactivateAccount(testUserId, requesterId))
+                    .isInstanceOf(UnauthorizedOperationException.class)
+                    .hasMessage("Not authorized to deactivate this account");
+            verify(userRepository, never()).save(any());
+            verify(sessionService, never()).revokeAllSessions(any());
+            verify(auditService, never()).logUserDeactivation(any(), any(), anyBoolean());
         }
     }