diff --git a/docs/IMPLEMENTATION-PLAN.md b/docs/IMPLEMENTATION-PLAN.md index 779dc72..df865fe 100644 --- a/docs/IMPLEMENTATION-PLAN.md +++ b/docs/IMPLEMENTATION-PLAN.md @@ -250,7 +250,7 @@ Resolved in code. Failed attempts are serialized with a pessimistic user-row loc **Current problem** -Password authentication maps active/locked state into Spring Security. During access-token authentication, however, `JwtAuthFilter` constructs an authenticated token after loading `UserDetails` without explicitly rejecting disabled or locked state. +Resolved. `JwtAuthFilter` now uses the reloaded `UserDetails` account state before creating the `SecurityContext`, and account deactivation revokes all refresh sessions. **Expected behavior** @@ -268,10 +268,16 @@ Password authentication maps active/locked state into Spring Security. During ac **Acceptance criteria** -- [ ] An existing access token fails after its user is disabled. -- [ ] An existing access token fails while its user is locked. -- [ ] Disabling an account revokes all refresh sessions. -- [ ] Active, unlocked users continue to authenticate normally. +- [x] An existing access token fails after its user is disabled. +- [x] An existing access token fails while its user is locked. +- [x] Disabling an account revokes all refresh sessions. +- [x] Active, unlocked users continue to authenticate normally. + +**Verification note — 2026-11-01** + +- `JwtAuthFilterTest` proves that valid access tokens do not populate the `SecurityContext` for disabled or locked users, while active/unlocked users still authenticate. +- `UserServiceTest` proves that self-service and ADMIN deactivation revoke all target refresh sessions. +- `./scripts/test.sh` passed: 121 tests, 0 failures and 0 errors. --- diff --git a/src/main/java/com/wallet/secure/auth/security/JwtAuthFilter.java b/src/main/java/com/wallet/secure/auth/security/JwtAuthFilter.java index 7bf3edc..f466219 100644 --- a/src/main/java/com/wallet/secure/auth/security/JwtAuthFilter.java +++ b/src/main/java/com/wallet/secure/auth/security/JwtAuthFilter.java @@ -51,7 +51,8 @@ public class JwtAuthFilter extends OncePerRequestFilter { * 1. Extract token from Authorization header * 2. Validate token signature and expiration * 3. Load user from DB by email in token - * 4. Populate SecurityContext if everything is valid + * 4. Reject disabled or locked accounts, even if their access JWT is valid + * 5. Populate SecurityContext only for an active, unlocked account * 5. Continue filter chain regardless of result * (Spring Security handles the 401/403 after this) * @@ -96,7 +97,17 @@ protected void doFilterInternal( UserDetails userDetails = userDetailsService.loadUserByUsername(email); - // Step 4 — Build authentication object and populate SecurityContext + // OWASP A07: JWT signature/expiry prove that the token was issued, not + // that its owner is still allowed to use it. Reloaded account state is + // authoritative, so deactivation or a temporary lock takes effect on + // the next request without waiting for access-token expiry. + if (!userDetails.isEnabled() || !userDetails.isAccountNonLocked()) { + log.warn("JWT rejected for disabled or locked account"); + filterChain.doFilter(request, response); + return; + } + + // Step 5 — Build authentication object and populate SecurityContext // UsernamePasswordAuthenticationToken(principal, credentials, authorities) // credentials = null — we don't need the password after token validation UsernamePasswordAuthenticationToken authToken = @@ -146,4 +157,4 @@ private String extractToken(HttpServletRequest request) { // Remove "Bearer " prefix (7 characters) to get the raw token return authHeader.substring(BEARER_PREFIX.length()); } -} \ No newline at end of file +} 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 3296999..4d13327 100644 --- a/src/main/java/com/wallet/secure/user/service/UserService.java +++ b/src/main/java/com/wallet/secure/user/service/UserService.java @@ -1,6 +1,7 @@ package com.wallet.secure.user.service; import com.wallet.secure.audit.service.AuditService; +import com.wallet.secure.auth.service.SessionService; import com.wallet.secure.common.enums.UserRole; import com.wallet.secure.common.exception.EmailAlreadyExistsException; import com.wallet.secure.common.exception.InvalidCredentialsException; @@ -45,6 +46,7 @@ public class UserService { private final UserRepository userRepository; private final PasswordEncoder passwordEncoder; private final AuditService auditService; + private final SessionService sessionService; // ─── Registration ───────────────────────────────────────────────────────── @@ -200,6 +202,11 @@ public ApiResponse deactivateAccount(UUID userId, UUID requesterId) { user.setIsActive(false); userRepository.save(user); + // OWASP A07: ending refresh sessions prevents a disabled account from + // obtaining new access tokens. JwtAuthFilter rejects any current access + // 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, diff --git a/src/test/java/com/wallet/secure/auth/security/JwtAuthFilterTest.java b/src/test/java/com/wallet/secure/auth/security/JwtAuthFilterTest.java new file mode 100644 index 0000000..8c833c1 --- /dev/null +++ b/src/test/java/com/wallet/secure/auth/security/JwtAuthFilterTest.java @@ -0,0 +1,95 @@ +package com.wallet.secure.auth.security; + +import jakarta.servlet.FilterChain; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.core.userdetails.UserDetails; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.*; + +@ExtendWith(MockitoExtension.class) +@DisplayName("JwtAuthFilter") +class JwtAuthFilterTest { + + private static final String TOKEN = "valid.access.token"; + private static final String EMAIL = "tests@test.com"; + + @Mock private JwtService jwtService; + @Mock private UserDetailsServiceImpl userDetailsService; + @Mock private HttpServletRequest request; + @Mock private HttpServletResponse response; + @Mock private FilterChain filterChain; + @Mock private UserDetails userDetails; + + @AfterEach + void clearSecurityContext() { + SecurityContextHolder.clearContext(); + } + + @Test + @DisplayName("rejects a valid access token after its account is disabled — OWASP A07") + void doFilterInternal_disabledUser_doesNotAuthenticate() throws Exception { + // GIVEN + JwtAuthFilter filter = validTokenFilter(); + when(userDetails.isEnabled()).thenReturn(false); + + // WHEN + filter.doFilterInternal(request, response, filterChain); + + // THEN + assertThat(SecurityContextHolder.getContext().getAuthentication()).isNull(); + verify(filterChain).doFilter(request, response); + } + + @Test + @DisplayName("rejects a valid access token while its account is locked — OWASP A07") + void doFilterInternal_lockedUser_doesNotAuthenticate() throws Exception { + // GIVEN + JwtAuthFilter filter = validTokenFilter(); + when(userDetails.isEnabled()).thenReturn(true); + when(userDetails.isAccountNonLocked()).thenReturn(false); + + // WHEN + filter.doFilterInternal(request, response, filterChain); + + // THEN + assertThat(SecurityContextHolder.getContext().getAuthentication()).isNull(); + verify(filterChain).doFilter(request, response); + } + + @Test + @DisplayName("authenticates an active and unlocked account with a valid access token") + void doFilterInternal_activeUnlockedUser_populatesSecurityContext() throws Exception { + // GIVEN + JwtAuthFilter filter = validTokenFilter(); + when(userDetails.isEnabled()).thenReturn(true); + when(userDetails.isAccountNonLocked()).thenReturn(true); + when(userDetails.getAuthorities()).thenReturn(java.util.List.of()); + + // WHEN + filter.doFilterInternal(request, response, filterChain); + + // THEN + assertThat(SecurityContextHolder.getContext().getAuthentication()) + .isNotNull() + .extracting(authentication -> authentication.getPrincipal()) + .isSameAs(userDetails); + verify(filterChain).doFilter(request, response); + } + + private JwtAuthFilter validTokenFilter() { + when(request.getHeader("Authorization")).thenReturn("Bearer " + TOKEN); + when(jwtService.isAccessTokenValid(TOKEN)).thenReturn(true); + when(jwtService.extractEmail(TOKEN)).thenReturn(EMAIL); + when(userDetailsService.loadUserByUsername(EMAIL)).thenReturn(userDetails); + return new JwtAuthFilter(jwtService, userDetailsService); + } +} 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 f5781bc..27a8982 100644 --- a/src/test/java/com/wallet/secure/user/service/UserServiceTest.java +++ b/src/test/java/com/wallet/secure/user/service/UserServiceTest.java @@ -1,6 +1,7 @@ package com.wallet.secure.user.service; import com.wallet.secure.audit.service.AuditService; +import com.wallet.secure.auth.service.SessionService; import com.wallet.secure.common.exception.EmailAlreadyExistsException; import com.wallet.secure.common.exception.InvalidCredentialsException; import com.wallet.secure.common.exception.UnauthorizedOperationException; @@ -54,6 +55,9 @@ class UserServiceTest { @Mock private AuditService auditService; + @Mock + private SessionService sessionService; + @InjectMocks private UserService userService; @@ -304,6 +308,7 @@ void shouldDeactivateOwnAccount() { // THEN 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()); } @@ -331,6 +336,7 @@ void shouldAllowAdminToDeactivateAnotherUser() { // THEN assertThat(response.isSuccess()).isTrue(); verify(userRepository).save(argThat(u -> !u.getIsActive())); + verify(sessionService).revokeAllSessions(testUserId); verify(auditService).logCriticalSecurityEvent(eq(testUserId), anyString(), isNull(), isNull()); }