Fix/authz 002

database/02-types.sql

@@ -31,6 +31,7 @@ CREATE TYPE audit_action AS ENUM (
     'USER_REGISTER',
     'USER_UPDATE',
     'USER_DELETE',
+    'ACCOUNT_DEACTIVATE',
     'WALLET_CREATE',
     'WALLET_SUSPEND',
     'WALLET_CLOSE',

database/08-migrations.sql

@@ -35,3 +35,9 @@ ALTER TABLE users
 ALTER TABLE users
     ADD CONSTRAINT users_failed_login_attempts_range
         CHECK (failed_login_attempts >= 0 AND failed_login_attempts <= 5);
+
+-- Migration: represent an administrative account block without implying deletion
+-- Date: 2026-11-03
+-- OWASP A09: financial accounts are retained; audit evidence must distinguish
+-- an is_active=false state change from a hard delete.
+ALTER TYPE audit_action ADD VALUE IF NOT EXISTS 'ACCOUNT_DEACTIVATE';

docs/IMPLEMENTATION-PLAN.md

@@ -291,11 +291,11 @@ Resolved. `JwtAuthFilter` now uses the reloaded `UserDetails` account state befo
 
 **Current problem**
 
-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.
+Resolved. Administrative endpoints enforce `ROLE_ADMIN`; only an ADMIN can deactivate a regular account through `PATCH /users/{id}/deactivate`. The operation changes `isActive` and revokes sessions without deleting financial data, while audit records contain immutable actor and target identifiers.
 
 **Expected behavior**
 
-- `ADMIN` can perform explicitly approved management operations on `USER` accounts.
+- `ADMIN` can deactivate (block) explicitly approved `USER` accounts without deleting them.
 - `USER` cannot invoke administrative operations.
 - No public or administrative API operation creates, promotes, or modifies another `ADMIN`.
 - Every administrative action is audited with actor and target identifiers.
@@ -320,8 +320,8 @@ Resolved. Administrative endpoints enforce `ROLE_ADMIN`, service rules limit cro
 **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.
+- `UserService` accepts ADMIN deactivation of a `USER`, rejects self-service and ADMIN-to-ADMIN deactivation, and registration persists the fixed `USER` role because `RegisterRequest` has no role field.
+- `AuditService.logUserDeactivation()` writes `ACCOUNT_DEACTIVATE` evidence with immutable actor and target UUIDs in the same transaction as the state change.
 - `./scripts/test.sh` passed with 0 failures and 0 errors.
 
 ### AUTHZ-002 — Review wallet permissions and ownership
@@ -330,9 +330,9 @@ Resolved. Administrative endpoints enforce `ROLE_ADMIN`, service rules limit cro
 **Priority:** P1  
 **Depends on:** AUTHZ-001
 
-**Current problem**
+**Current problem — resolved**
 
-Wallet controllers derive identity from the JWT and services include ownership patterns, but complete endpoint-by-endpoint authorization evidence is missing.
+Wallet ownership already came from the authenticated principal and ownership lookups use non-enumerating queries. AUTHZ-002 now also proves the route matrix and preserves the administrator (rather than the wallet owner) as the actor in lifecycle audit records.
 
 **Expected behavior**
 
@@ -352,10 +352,17 @@ Wallet controllers derive identity from the JWT and services include ownership p
 
 **Acceptance criteria**
 
-- [ ] User A cannot read, debit, close, suspend, or restore User B's wallet.
-- [ ] User-controlled input cannot assign a wallet to a different owner.
-- [ ] Administrative actions require `ROLE_ADMIN` and generate audit records.
-- [ ] Automated tests cover every wallet endpoint with owner, non-owner, anonymous, and admin actors.
+- [x] User A cannot read, debit, close, suspend, or restore User B's wallet.
+- [x] User-controlled input cannot assign a wallet to a different owner.
+- [x] Administrative actions require `ROLE_ADMIN` and generate audit records.
+- [x] Automated tests cover every wallet endpoint with owner, non-owner, anonymous, and admin actors.
+
+**Verification note — 2026-11-03**
+
+- Ownership-sensitive wallet reads and transaction validation use `findByIdAndUserId`, returning the same `404` for a nonexistent wallet and another user's wallet; controllers never accept an owner ID in request input.
+- The create DTO exposes only `currency`; the service obtains the owner from the JWT-derived ID and forces balance/status server-side.
+- `@PreAuthorize("hasRole('ADMIN')")` blocks a `USER` with `403` on suspend, restore, and close. Anonymous calls to every wallet route return `401`; an `ADMIN` passes the lifecycle authorization gate.
+- Lifecycle audit events now identify the authenticated administrator as the actor and retain the affected `walletId` in immutable audit details.
 
 ### TX-001 — Verify transactions, concurrency, rollback, and audit behavior
 

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

@@ -154,16 +154,15 @@ public void logRegister(UUID userId, String email, String ipAddress, String user
      * 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) {
+    @Transactional(propagation = Propagation.MANDATORY)
+    public void logUserDeactivation(UUID actorUserId, UUID targetUserId) {
         String details = buildDetails(
                 "\"actorUserId\":\"" + actorUserId + "\"",
                 "\"targetUserId\":\"" + targetUserId + "\"",
-                "\"mode\":\"" + (selfService ? "SELF_SERVICE" : "ADMIN") + "\"",
+                "\"mode\":\"ADMIN\"",
                 "\"outcome\":\"SUCCESS\""
         );
-        save(AuditLog.success(actorUserId, AuditAction.USER_DELETE, LogSeverity.WARNING,
+        save(AuditLog.success(actorUserId, AuditAction.ACCOUNT_DEACTIVATE, LogSeverity.WARNING,
                 details, null, null));
     }
 

src/main/java/com/wallet/secure/common/enums/AuditAction.java

@@ -21,6 +21,7 @@ public enum AuditAction {
     USER_REGISTER,
     USER_UPDATE,
     USER_DELETE,
+    ACCOUNT_DEACTIVATE,
 
     // Wallet lifecycle
     WALLET_CREATE,

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

@@ -9,6 +9,7 @@
 import org.springframework.web.bind.MethodArgumentNotValidException;
 import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.RestControllerAdvice;
+import org.springframework.web.HttpRequestMethodNotSupportedException;
 
 import java.util.HashMap;
 import java.util.Map;
@@ -215,6 +216,22 @@ public ResponseEntity<ApiResponse<Map<String, String>>> handleValidationErrors(
                 .body(ApiResponse.error("Validation failed", errors));
     }
 
+    /**
+     * 405 — The URL exists but does not support the requested HTTP method.
+     *
+     * WHY this is explicit: removing a destructive DELETE route must not make
+     * clients see a misleading 500 through the generic fallback. The API keeps
+     * its response contract while making the absence of deletion intentional.
+     */
+    @ExceptionHandler(HttpRequestMethodNotSupportedException.class)
+    public ResponseEntity<ApiResponse<Void>> handleMethodNotSupported(
+            HttpRequestMethodNotSupportedException ex) {
+        log.warn("Unsupported HTTP method: {}", ex.getMethod());
+        return ResponseEntity
+                .status(HttpStatus.METHOD_NOT_ALLOWED)
+                .body(ApiResponse.error("HTTP method not allowed"));
+    }
+
     // ─── Generic Fallback ─────────────────────────────────────────────────────
 
     /**

src/main/java/com/wallet/secure/user/controller/UserController.java

@@ -92,30 +92,19 @@ public ResponseEntity<ApiResponse<UserResponse>> updateMyProfile(
         return ResponseEntity.ok(userService.updateProfile(userId, request));
     }
 
-    // ─── Account Deactivation ─────────────────────────────────────────────────
+    // ─── Administrative Account Deactivation ──────────────────────────────────
 
     /**
-     * DELETE /users/me
-     * Soft-deactivates the authenticated user's account.
-     * isActive = false — data preserved for audit compliance.
+     * PATCH /users/{id}/deactivate
+     * Deactivates (blocks) an account; it never deletes its financial data.
      *
-     * HTTP 200 with message — not 204, because ApiResponse carries confirmation.
-     * OWASP A09: data retention for financial compliance.
+     * A bank-like wallet must retain users, wallets, transactions and audit
+     * evidence. Therefore this changes only isActive to false and revokes
+     * sessions; only an ADMIN can execute it. PATCH expresses a state change,
+     * unlike DELETE, which would misleadingly imply resource removal.
+     * OWASP A01/A09: ADMIN-only operation with preserved forensic evidence.
      */
-    @DeleteMapping("/me")
-    public ResponseEntity<ApiResponse<Void>> deactivateMyAccount(
-            @AuthenticationPrincipal UserDetails userDetails) {
-
-        UUID userId = resolveUserId(userDetails);
-        return ResponseEntity.ok(userService.deactivateAccount(userId, userId));
-    }
-
-    /**
-     * DELETE /users/{id}
-     * Admin deactivates any user's account.
-     * OWASP A01: ADMIN role required.
-     */
-    @DeleteMapping("/{id}")
+    @PatchMapping("/{id}/deactivate")
     @PreAuthorize("hasRole('ADMIN')")
     public ResponseEntity<ApiResponse<Void>> deactivateUser(
             @PathVariable UUID id,

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

@@ -175,33 +175,26 @@ public ApiResponse<UserResponse> updateProfile(UUID userId, UpdateProfileRequest
      * - OWASP A09: audit trail must be preserved
      *
      * @param userId ID of user to deactivate
-     * @param requesterId ID of who is requesting (must be same user or ADMIN)
+     * @param requesterId authenticated ADMIN performing the action
      */
     @Transactional
     public ApiResponse<Void> deactivateAccount(UUID userId, UUID requesterId) {
         User user = findUserById(userId);
 
-        // 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");
-            }
+        // Defense in depth: @PreAuthorize blocks this at the HTTP boundary, and
+        // this service check also protects future internal callers (OWASP A01).
+        User requester = findUserById(requesterId);
+        if (!UserRole.ADMIN.equals(requester.getRole())) {
+            log.warn("Unauthorized account deactivation attempt: requesterId={} targetUserId={}",
+                    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");
-            }
+        // Privileged accounts cannot be changed through this management route.
+        if (UserRole.ADMIN.equals(user.getRole())) {
+            log.warn("Blocked ADMIN-to-ADMIN deactivation: requesterId={} targetUserId={}",
+                    requesterId, userId);
+            throw new UnauthorizedOperationException("Not authorized to deactivate this account");
         }
 
         user.setIsActive(false);
@@ -212,8 +205,10 @@ public ApiResponse<Void> deactivateAccount(UUID userId, UUID requesterId) {
         // token on its next request using the reloaded isActive state.
         sessionService.revokeAllSessions(userId);
 
-        // OWASP A09: immutable audit evidence records both actor and target.
-        auditService.logUserDeactivation(requesterId, userId, isSelf);
+        // OWASP A09: this audit joins the state-change transaction. Returning a
+        // success response without durable evidence is unacceptable for an
+        // administrative account block.
+        auditService.logUserDeactivation(requesterId, userId);
 
         log.info("Account deactivated: userId={} by requesterId={}", userId, requesterId);
         return ApiResponse.ok("Account deactivated successfully");

src/main/java/com/wallet/secure/wallet/controller/WalletController.java

@@ -120,9 +120,10 @@ public ResponseEntity<ApiResponse<WalletResponse>> getMyWalletByCurrency(@PathVa
      */
     @PatchMapping("/{id}/suspend")
     @PreAuthorize("hasRole('ADMIN')")
-    public ResponseEntity<ApiResponse<WalletResponse>> suspendWallet(@PathVariable UUID id) {
+    public ResponseEntity<ApiResponse<WalletResponse>> suspendWallet(@PathVariable UUID id,
+                                                                      @AuthenticationPrincipal UserDetails userDetails) {
 
-        return ResponseEntity.ok(walletService.suspendWallet(id));
+        return ResponseEntity.ok(walletService.suspendWallet(id, resolveUserId(userDetails)));
     }
 
     /**
@@ -132,9 +133,10 @@ public ResponseEntity<ApiResponse<WalletResponse>> suspendWallet(@PathVariable U
      */
     @PatchMapping("/{id}/restore")
     @PreAuthorize("hasRole('ADMIN')")
-    public ResponseEntity<ApiResponse<WalletResponse>> restoreWallet(@PathVariable UUID id) {
+    public ResponseEntity<ApiResponse<WalletResponse>> restoreWallet(@PathVariable UUID id,
+                                                                      @AuthenticationPrincipal UserDetails userDetails) {
 
-        return ResponseEntity.ok(walletService.restoreWallet(id));
+        return ResponseEntity.ok(walletService.restoreWallet(id, resolveUserId(userDetails)));
     }
 
     /**
@@ -146,9 +148,10 @@ public ResponseEntity<ApiResponse<WalletResponse>> restoreWallet(@PathVariable U
      */
     @PatchMapping("/{id}/close")
     @PreAuthorize("hasRole('ADMIN')")
-    public ResponseEntity<ApiResponse<WalletResponse>> closeWallet(@PathVariable UUID id) {
+    public ResponseEntity<ApiResponse<WalletResponse>> closeWallet(@PathVariable UUID id,
+                                                                    @AuthenticationPrincipal UserDetails userDetails) {
 
-        return ResponseEntity.ok(walletService.closeWallet(id));
+        return ResponseEntity.ok(walletService.closeWallet(id, resolveUserId(userDetails)));
     }
 
     // --- Private Helper

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

@@ -164,10 +164,11 @@ public ApiResponse<WalletResponse> getMyWalletByCurrency(UUID userId, CurrencyCo
      * OWASP A01: @PreAuthorize("hasROle('ADMIN')") enforced at controller level.
      *
      * @param walletId wallet to suspend
+     * @param adminId authenticated administrator's ID, preserved as the audit actor
      * @return ApiResponse with updates WalletResponse
      */
     @Transactional
-    public ApiResponse<WalletResponse> suspendWallet(UUID walletId) {
+    public ApiResponse<WalletResponse> suspendWallet(UUID walletId, UUID adminId) {
 
         Wallet wallet = walletRepository.findById(walletId)
                 .orElseThrow(() -> new ResourceNotFoundException("Wallet not found"));
@@ -178,7 +179,9 @@ public ApiResponse<WalletResponse> suspendWallet(UUID walletId) {
         wallet.setStatus(WalletStatus.SUSPENDED);
         Wallet saved = walletRepository.save(wallet);
 
-        auditService.logWalletSuspended(wallet.getUser().getId(), walletId, null, null);
+        // OWASP A09: record the administrator who made the state change, not
+        // the wallet owner who was affected by it.
+        auditService.logWalletSuspended(adminId, walletId, null, null);
 
         log.warn("Wallet suspended: walletId={}", walletId);
 
@@ -194,10 +197,11 @@ public ApiResponse<WalletResponse> suspendWallet(UUID walletId) {
      * OWASP A01: @PreAuthorize("hasRole('ADMIN')") at controller level
      *
      * @param walletId wallet to close permanently
+     * @param adminId authenticated administrator's ID, preserved as the audit actor
      * @return ApiResponse with updated WalletResponse
      */
     @Transactional
-    public ApiResponse<WalletResponse> closeWallet(UUID walletId) {
+    public ApiResponse<WalletResponse> closeWallet(UUID walletId, UUID adminId) {
 
         Wallet wallet = walletRepository.findById(walletId)
                 .orElseThrow(() -> new ResourceNotFoundException("Wallet not found"));
@@ -213,7 +217,8 @@ public ApiResponse<WalletResponse> closeWallet(UUID walletId) {
         wallet.setStatus(WalletStatus.CLOSED);
         Wallet saved =  walletRepository.save(wallet);
 
-        auditService.logWalletClosed(wallet.getUser().getId(), walletId, null, null);
+        // OWASP A09: the audit actor is the administrator, never the affected owner.
+        auditService.logWalletClosed(adminId, walletId, null, null);
 
         log.warn("Wallet permanently closed: walletId={}", walletId);
 
@@ -225,10 +230,11 @@ public ApiResponse<WalletResponse> closeWallet(UUID walletId) {
      * SUSPENDED -> ACTIVE
      *
      * @param walletId wallet to restore
+     * @param adminId authenticated administrator's ID, preserved as the audit actor
      * @return ApiResponse with updated WalletResponse
      */
     @Transactional
-    public ApiResponse<WalletResponse> restoreWallet(UUID walletId) {
+    public ApiResponse<WalletResponse> restoreWallet(UUID walletId, UUID adminId) {
 
         Wallet wallet =  walletRepository.findById(walletId)
                 .orElseThrow(() -> new ResourceNotFoundException("Wallet not found"));
@@ -241,7 +247,7 @@ public ApiResponse<WalletResponse> restoreWallet(UUID walletId) {
 
         // 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);
+        auditService.logWalletRestored(adminId, walletId, null, null);
 
         log.warn("Wallet restored to ACTIVE: walletId={}", walletId);
 
@@ -285,4 +291,4 @@ public UUID resolveUserId(String email) {
         return userRepository.findByEmail(email)
                 .orElseThrow(() -> new ResourceNotFoundException("User not found")).getId();
     }
-}
+}