From a332e54fc376a2e21859df1d3cbd33c73bef9a11 Mon Sep 17 00:00:00 2001 From: podlLev Date: Thu, 23 Jul 2026 11:10:00 +0300 Subject: [PATCH 1/5] build: add resilience4j-spring-boot3 dependency --- pom.xml | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/pom.xml b/pom.xml index 3d85ea4..8d50792 100644 --- a/pom.xml +++ b/pom.xml @@ -40,6 +40,7 @@ 6.6.7.Final 1.0.1.Final 0.8.12 + 2.2.0 1.5.5.Final 0.2.0 @@ -102,6 +103,15 @@ org.springframework.boot spring-boot-starter-mail + + org.springframework.boot + spring-boot-starter-aop + + + io.github.resilience4j + resilience4j-spring-boot3 + ${resilience4j.version} + org.projectlombok lombok @@ -214,6 +224,26 @@ report + + check + + check + + + + + BUNDLE + + + LINE + COVEREDRATIO + 0.90 + + + + + + From 27d2e4c36a0f4d56f06e76accbc5b370b12bbc26 Mon Sep 17 00:00:00 2001 From: podlLev Date: Thu, 23 Jul 2026 11:10:55 +0300 Subject: [PATCH 2/5] db: add failed_login_attempts and locked_until columns to users --- .../java/com/weatherviewer/model/User.java | 18 +++ .../security/AccountLockoutListener.java | 94 +++++++++++ .../com/weatherviewer/security/SecUser.java | 16 +- src/main/resources/liquibase/changelog.xml | 1 + .../update/07-add-account-lockout.xml | 13 ++ .../security/AccountLockoutListenerTest.java | 151 ++++++++++++++++++ .../weatherviewer/security/SecUserTest.java | 28 ++++ 7 files changed, 316 insertions(+), 5 deletions(-) create mode 100644 src/main/java/com/weatherviewer/security/AccountLockoutListener.java create mode 100644 src/main/resources/liquibase/update/07-add-account-lockout.xml create mode 100644 src/test/java/com/weatherviewer/security/AccountLockoutListenerTest.java diff --git a/src/main/java/com/weatherviewer/model/User.java b/src/main/java/com/weatherviewer/model/User.java index 24b87f7..69755be 100644 --- a/src/main/java/com/weatherviewer/model/User.java +++ b/src/main/java/com/weatherviewer/model/User.java @@ -11,6 +11,7 @@ import org.hibernate.annotations.JdbcType; import org.hibernate.dialect.PostgreSQLEnumJdbcType; +import java.time.LocalDateTime; import java.util.List; /** @@ -57,6 +58,23 @@ public class User extends BaseEntity { @JdbcType(PostgreSQLEnumJdbcType.class) private UnitSystem units = UnitSystem.METRIC; + /** + * Consecutive failed sign-in attempts since the last successful login + * or the last time the account was unlocked. Reset to zero on every + * successful authentication. Tracked by + * {@link com.weatherviewer.security.AccountLockoutListener}. + */ + @Column(nullable = false) + private int failedLoginAttempts = 0; + + /** + * If set and still in the future, the account is temporarily locked out + * of authentication regardless of {@link #status} — see + * {@link com.weatherviewer.security.SecUser#isAccountNonLocked()}. + * {@code null} means the account isn't locked. + */ + private LocalDateTime lockedUntil; + /** Locations saved by this user; removed automatically if the user is deleted. */ @OneToMany(mappedBy = "user", cascade = CascadeType.ALL, orphanRemoval = true) private List locations; diff --git a/src/main/java/com/weatherviewer/security/AccountLockoutListener.java b/src/main/java/com/weatherviewer/security/AccountLockoutListener.java new file mode 100644 index 0000000..5894c8d --- /dev/null +++ b/src/main/java/com/weatherviewer/security/AccountLockoutListener.java @@ -0,0 +1,94 @@ +package com.weatherviewer.security; + +import com.weatherviewer.repository.UserRepository; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.context.event.EventListener; +import org.springframework.security.authentication.event.AuthenticationFailureBadCredentialsEvent; +import org.springframework.security.authentication.event.AuthenticationSuccessEvent; +import org.springframework.stereotype.Component; +import org.springframework.transaction.annotation.Transactional; + +import java.time.LocalDateTime; + +/** + * Tracks consecutive failed sign-in attempts per account and applies a + * temporary lockout once a threshold is reached, mitigating credential + * stuffing / brute-force attempts against a single known email address — + * something IP- or session-based rate limiting alone doesn't fully cover, + * since an attacker can spread attempts across many IPs. + *

+ * Listens to the {@link AuthenticationFailureBadCredentialsEvent} and + * {@link AuthenticationSuccessEvent} that Spring Security's default + * {@code AuthenticationEventPublisher} raises around every + * {@code DaoAuthenticationProvider} attempt. Once an account is locked, + * {@link SecUser#isAccountNonLocked()} makes subsequent attempts fail with + * {@code LockedException} instead of {@code BadCredentialsException} — so + * this listener naturally stops incrementing further while the lock is in + * effect, rather than perpetually extending it. + */ +@Component +@RequiredArgsConstructor +@Slf4j +public class AccountLockoutListener { + + private final UserRepository userRepository; + + /** Consecutive failures before an account is locked. */ + @Value("${security.account-lockout.max-attempts:5}") + private int maxAttempts; + + /** How long an account stays locked once the threshold is hit. */ + @Value("${security.account-lockout.lock-duration-minutes:15}") + private long lockDurationMinutes; + + /** + * On a failed login with valid credentials-format-but-wrong-password + * (or, thanks to {@code hideUserNotFoundExceptions}, an unknown email + * too — though in that case there's no matching row to update), + * increments the account's failure counter and locks it once + * {@link #maxAttempts} is reached. + */ + @EventListener + @Transactional + public void onAuthenticationFailure(AuthenticationFailureBadCredentialsEvent event) { + String email = event.getAuthentication().getName(); + userRepository.findByEmail(email).ifPresent(user -> { + int attempts = user.getFailedLoginAttempts() + 1; + user.setFailedLoginAttempts(attempts); + + if (attempts >= maxAttempts) { + user.setLockedUntil(LocalDateTime.now().plusMinutes(lockDurationMinutes)); + log.warn("Account locked for {} minutes after {} consecutive failed sign-in attempts: {}", + lockDurationMinutes, attempts, email); + } else { + log.debug("Failed sign-in attempt {}/{} for {}", attempts, maxAttempts, email); + } + + userRepository.save(user); + }); + } + + /** + * Clears the failure counter and any active lockout on a successful + * sign-in, so a legitimate user who mistyped their password a few + * times isn't left partway toward a lockout on their next visit. + */ + @EventListener + @Transactional + public void onAuthenticationSuccess(AuthenticationSuccessEvent event) { + if (!(event.getAuthentication().getPrincipal() instanceof SecUser secUser)) { + return; + } + + userRepository.findById(secUser.getId()).ifPresent(user -> { + if (user.getFailedLoginAttempts() != 0 || user.getLockedUntil() != null) { + user.setFailedLoginAttempts(0); + user.setLockedUntil(null); + userRepository.save(user); + } + }); + } + +} diff --git a/src/main/java/com/weatherviewer/security/SecUser.java b/src/main/java/com/weatherviewer/security/SecUser.java index 54854f1..7520886 100644 --- a/src/main/java/com/weatherviewer/security/SecUser.java +++ b/src/main/java/com/weatherviewer/security/SecUser.java @@ -9,6 +9,7 @@ import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.core.userdetails.UserDetails; +import java.time.LocalDateTime; import java.util.Collection; import java.util.Set; import java.util.UUID; @@ -17,11 +18,14 @@ * Spring Security {@link UserDetails} adapter around this application's * {@link User} entity. *

- * All four account-state checks ({@code isAccountNonExpired}, - * {@code isAccountNonLocked}, {@code isCredentialsNonExpired}, - * {@code isEnabled}) are backed by the single {@link #isActive} flag — + * {@code isAccountNonExpired}, {@code isCredentialsNonExpired}, and + * {@code isEnabled} are all backed by the single {@link #isActive} flag — * this app doesn't distinguish between those states, so any non-{@code ACTIVE} * {@link UserStatus} simply locks the account out of authentication. + * {@code isAccountNonLocked} is separate: it reflects the temporary, + * self-clearing lockout applied after repeated failed sign-in attempts + * (see {@link com.weatherviewer.security.AccountLockoutListener}), not the + * account's persistent {@link UserStatus}. */ @Getter @RequiredArgsConstructor @@ -34,6 +38,7 @@ public class SecUser implements UserDetails { private final Boolean isActive; private final String fullName; private final UnitSystem units; + private final LocalDateTime lockedUntil; @Override public Collection getAuthorities() { @@ -57,7 +62,7 @@ public boolean isAccountNonExpired() { @Override public boolean isAccountNonLocked() { - return isActive; + return isActive && (lockedUntil == null || lockedUntil.isBefore(LocalDateTime.now())); } @Override @@ -84,7 +89,8 @@ public static SecUser fromUser(User user) { user.getRole().getAuthority(), user.getStatus() == UserStatus.ACTIVE, user.getFullName(), - user.getUnits() + user.getUnits(), + user.getLockedUntil() ); } diff --git a/src/main/resources/liquibase/changelog.xml b/src/main/resources/liquibase/changelog.xml index 51be9e7..e5d9933 100644 --- a/src/main/resources/liquibase/changelog.xml +++ b/src/main/resources/liquibase/changelog.xml @@ -9,5 +9,6 @@ + diff --git a/src/main/resources/liquibase/update/07-add-account-lockout.xml b/src/main/resources/liquibase/update/07-add-account-lockout.xml new file mode 100644 index 0000000..57d4c82 --- /dev/null +++ b/src/main/resources/liquibase/update/07-add-account-lockout.xml @@ -0,0 +1,13 @@ + + + + + + ALTER TABLE users ADD COLUMN failed_login_attempts INT NOT NULL DEFAULT 0; + ALTER TABLE users ADD COLUMN locked_until TIMESTAMP NULL; + + + + diff --git a/src/test/java/com/weatherviewer/security/AccountLockoutListenerTest.java b/src/test/java/com/weatherviewer/security/AccountLockoutListenerTest.java new file mode 100644 index 0000000..651eabd --- /dev/null +++ b/src/test/java/com/weatherviewer/security/AccountLockoutListenerTest.java @@ -0,0 +1,151 @@ +package com.weatherviewer.security; + +import com.weatherviewer.model.User; +import com.weatherviewer.model.enums.Role; +import com.weatherviewer.model.enums.UserStatus; +import com.weatherviewer.repository.UserRepository; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.authentication.event.AuthenticationFailureBadCredentialsEvent; +import org.springframework.security.authentication.event.AuthenticationSuccessEvent; +import org.springframework.security.core.Authentication; +import org.springframework.test.util.ReflectionTestUtils; + +import java.time.LocalDateTime; +import java.util.Optional; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.*; + +@ExtendWith(MockitoExtension.class) +class AccountLockoutListenerTest { + + private static final int MAX_ATTEMPTS = 5; + private static final long LOCK_DURATION_MINUTES = 15; + + @Mock + private UserRepository userRepository; + + private AccountLockoutListener listener; + + @BeforeEach + void setUp() { + listener = new AccountLockoutListener(userRepository); + ReflectionTestUtils.setField(listener, "maxAttempts", MAX_ATTEMPTS); + ReflectionTestUtils.setField(listener, "lockDurationMinutes", LOCK_DURATION_MINUTES); + } + + private User user(int failedAttempts) { + User user = (User) new User() + .setEmail("john@example.com") + .setPassword("hashed") + .setStatus(UserStatus.ACTIVE) + .setRole(Role.USER) + .setId(UUID.randomUUID()); + user.setFailedLoginAttempts(failedAttempts); + return user; + } + + @Test + void onAuthenticationFailure_belowThreshold_incrementsWithoutLocking() { + User user = user(1); + when(userRepository.findByEmail("john@example.com")).thenReturn(Optional.of(user)); + + listener.onAuthenticationFailure(failureEvent("john@example.com")); + + ArgumentCaptor captor = ArgumentCaptor.forClass(User.class); + verify(userRepository).save(captor.capture()); + assertThat(captor.getValue().getFailedLoginAttempts()).isEqualTo(2); + assertThat(captor.getValue().getLockedUntil()).isNull(); + } + + @Test + void onAuthenticationFailure_reachesThreshold_locksAccount() { + User user = user(MAX_ATTEMPTS - 1); + when(userRepository.findByEmail("john@example.com")).thenReturn(Optional.of(user)); + + listener.onAuthenticationFailure(failureEvent("john@example.com")); + + ArgumentCaptor captor = ArgumentCaptor.forClass(User.class); + verify(userRepository).save(captor.capture()); + assertThat(captor.getValue().getFailedLoginAttempts()).isEqualTo(MAX_ATTEMPTS); + assertThat(captor.getValue().getLockedUntil()) + .isAfter(LocalDateTime.now().plusMinutes(LOCK_DURATION_MINUTES).minusSeconds(5)); + } + + @Test + void onAuthenticationFailure_unknownEmail_doesNothing() { + when(userRepository.findByEmail("ghost@example.com")).thenReturn(Optional.empty()); + + listener.onAuthenticationFailure(failureEvent("ghost@example.com")); + + verify(userRepository, never()).save(any()); + } + + @Test + void onAuthenticationSuccess_clearsCounterAndLock() { + User user = user(3); + user.setLockedUntil(LocalDateTime.now().minusMinutes(1)); + when(userRepository.findById(user.getId())).thenReturn(Optional.of(user)); + + listener.onAuthenticationSuccess(successEvent(user)); + + ArgumentCaptor captor = ArgumentCaptor.forClass(User.class); + verify(userRepository).save(captor.capture()); + assertThat(captor.getValue().getFailedLoginAttempts()).isZero(); + assertThat(captor.getValue().getLockedUntil()).isNull(); + } + + @Test + void onAuthenticationSuccess_noPriorFailures_doesNotWriteToDatabase() { + User user = user(0); + when(userRepository.findById(user.getId())).thenReturn(Optional.of(user)); + + listener.onAuthenticationSuccess(successEvent(user)); + + verify(userRepository, never()).save(any()); + } + + @Test + void onAuthenticationSuccess_principalNotSecUser_doesNothing() { + Authentication authentication = new UsernamePasswordAuthenticationToken("anonymousUser", null); + AuthenticationSuccessEvent event = new AuthenticationSuccessEvent(authentication); + + listener.onAuthenticationSuccess(event); + + verify(userRepository, never()).findById(any()); + verify(userRepository, never()).save(any()); + } + + @Test + void onAuthenticationSuccess_attemptsZeroButLockedUntilSet_resetsAndSaves() { + User user = user(0); + user.setLockedUntil(LocalDateTime.now().minusMinutes(1)); + when(userRepository.findById(user.getId())).thenReturn(Optional.of(user)); + + listener.onAuthenticationSuccess(successEvent(user)); + + ArgumentCaptor captor = ArgumentCaptor.forClass(User.class); + verify(userRepository).save(captor.capture()); + assertThat(captor.getValue().getFailedLoginAttempts()).isZero(); + assertThat(captor.getValue().getLockedUntil()).isNull(); + } + + private AuthenticationFailureBadCredentialsEvent failureEvent(String email) { + Authentication authentication = new UsernamePasswordAuthenticationToken(email, "wrong-password"); + return new AuthenticationFailureBadCredentialsEvent(authentication, new org.springframework.security.authentication.BadCredentialsException("bad credentials")); + } + + private AuthenticationSuccessEvent successEvent(User user) { + SecUser secUser = SecUser.fromUser(user); + Authentication authentication = new UsernamePasswordAuthenticationToken(secUser, null, secUser.getAuthorities()); + return new AuthenticationSuccessEvent(authentication); + } + +} diff --git a/src/test/java/com/weatherviewer/security/SecUserTest.java b/src/test/java/com/weatherviewer/security/SecUserTest.java index 7a41525..dcbbcb0 100644 --- a/src/test/java/com/weatherviewer/security/SecUserTest.java +++ b/src/test/java/com/weatherviewer/security/SecUserTest.java @@ -5,6 +5,7 @@ import com.weatherviewer.model.enums.UserStatus; import org.junit.jupiter.api.Test; +import java.time.LocalDateTime; import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; @@ -80,4 +81,31 @@ void fromUser_userRole_doesNotHaveWriteAuthority() { .doesNotContain("users:write"); } + @Test + void isAccountNonLocked_lockedUntilInFuture_returnsFalse() { + User user = user(UserStatus.ACTIVE, Role.USER); + user.setLockedUntil(LocalDateTime.now().plusMinutes(15)); + SecUser secUser = SecUser.fromUser(user); + + assertThat(secUser.isAccountNonLocked()).isFalse(); + } + + @Test + void isAccountNonLocked_lockedUntilInPast_returnsTrue() { + User user = user(UserStatus.ACTIVE, Role.USER); + user.setLockedUntil(LocalDateTime.now().minusMinutes(5)); + SecUser secUser = SecUser.fromUser(user); + + assertThat(secUser.isAccountNonLocked()).isTrue(); + } + + @Test + void isAccountNonLocked_inactiveStatusWithNullLockedUntil_returnsFalse() { + User user = user(UserStatus.PENDING, Role.USER); + user.setLockedUntil(null); + SecUser secUser = SecUser.fromUser(user); + + assertThat(secUser.isAccountNonLocked()).isFalse(); + } + } From 517a5dd7da1555f05a2ee2789ef0a456e5672bf9 Mon Sep 17 00:00:00 2001 From: podlLev Date: Thu, 23 Jul 2026 11:12:06 +0300 Subject: [PATCH 3/5] feat(security): lock account after repeated failed sign-in attempts --- .../com/weatherviewer/config/AppConfig.java | 20 ++++++++++++++--- .../controller/AuthController.java | 13 ++++++++--- .../exception/ExternalHttpCallException.java | 22 +++++++++++++++++++ .../security/CustomAuthFailureHandler.java | 19 +++++++++++++++- .../controller/AuthControllerTest.java | 21 ++++++++++++++++++ 5 files changed, 88 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/weatherviewer/config/AppConfig.java b/src/main/java/com/weatherviewer/config/AppConfig.java index 01dee09..262fc7c 100644 --- a/src/main/java/com/weatherviewer/config/AppConfig.java +++ b/src/main/java/com/weatherviewer/config/AppConfig.java @@ -1,7 +1,9 @@ package com.weatherviewer.config; +import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.http.client.SimpleClientHttpRequestFactory; import org.springframework.security.web.DefaultRedirectStrategy; import org.springframework.security.web.RedirectStrategy; import org.springframework.web.client.RestClient; @@ -17,10 +19,22 @@ @Configuration public class AppConfig { - /** HTTP client used by {@link com.weatherviewer.service.integration.WeatherApiClient} to call OpenWeatherMap. */ + /** + * HTTP client used by {@link com.weatherviewer.service.integration.WeatherApiClient} to + * call OpenWeatherMap. Explicit connect/read timeouts are required here: without them the + * underlying JDK client factory has no default timeout, so a hung or slow OpenWeatherMap + * response would block the calling request thread indefinitely and, under load, exhaust + * the server's thread pool. + */ @Bean - public RestClient restClient(RestClient.Builder restClientBuilder) { - return restClientBuilder.build(); + public RestClient restClient( + RestClient.Builder restClientBuilder, + @Value("${weather.api.connect-timeout-ms:3000}") int connectTimeoutMs, + @Value("${weather.api.read-timeout-ms:5000}") int readTimeoutMs) { + SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpRequestFactory(); + requestFactory.setConnectTimeout(connectTimeoutMs); + requestFactory.setReadTimeout(readTimeoutMs); + return restClientBuilder.requestFactory(requestFactory).build(); } /** Default Spring Security redirect strategy, used by {@link com.weatherviewer.security.CustomAuthSuccessHandler}. */ diff --git a/src/main/java/com/weatherviewer/controller/AuthController.java b/src/main/java/com/weatherviewer/controller/AuthController.java index a0f1eb1..6d59f52 100644 --- a/src/main/java/com/weatherviewer/controller/AuthController.java +++ b/src/main/java/com/weatherviewer/controller/AuthController.java @@ -49,15 +49,22 @@ public String signIn(@RequestParam(required = false) String redirect, Model mode /** Landing target Spring Security redirects to after a failed login attempt; re-renders sign-in with an error. */ @GetMapping("/sign-in-failure") public String signInFailure(@RequestParam(required = false) Boolean unverified, + @RequestParam(required = false) Boolean locked, @RequestParam(required = false) String email, RedirectAttributes redirectAttributes) { - log.info("Sign-in failed, unverified={}", unverified); + log.info("Sign-in failed, unverified={}, locked={}", unverified, locked); + String emailParam = email != null && !email.isBlank() ? "&email=" + email : ""; if (Boolean.TRUE.equals(unverified)) { redirectAttributes.addFlashAttribute("errorMessage", "Please verify your email before signing in."); - return "redirect:/sign-in?unverified=true" - + (email != null && !email.isBlank() ? "&email=" + email : ""); + return "redirect:/sign-in?unverified=true" + emailParam; + } + + if (Boolean.TRUE.equals(locked)) { + redirectAttributes.addFlashAttribute("errorMessage", + "Too many failed sign-in attempts. Your account is temporarily locked — please try again in a few minutes."); + return "redirect:/sign-in?locked=true" + emailParam; } redirectAttributes.addFlashAttribute("errorMessage", "Invalid email or password. Please try again."); diff --git a/src/main/java/com/weatherviewer/exception/ExternalHttpCallException.java b/src/main/java/com/weatherviewer/exception/ExternalHttpCallException.java index b0142e8..2a84363 100644 --- a/src/main/java/com/weatherviewer/exception/ExternalHttpCallException.java +++ b/src/main/java/com/weatherviewer/exception/ExternalHttpCallException.java @@ -1,5 +1,7 @@ package com.weatherviewer.exception; +import lombok.Getter; + /** * Thrown when a call to an external HTTP service fails — in practice, the * OpenWeatherMap API accessed by @@ -8,13 +10,33 @@ * {@link ControllerExceptionHandler} into a {@code 503 Service Unavailable} * response. */ +@Getter public class ExternalHttpCallException extends RuntimeException { + /** + * Whether this failure is worth retrying. Network errors, timeouts, and + * 5xx responses from the provider are transient and {@code retryable}; + * 4xx responses (bad request, unauthorized, not found) reflect a + * problem with the request itself and won't succeed on a second try, so + * they're marked {@code false} and skipped by + * {@link com.weatherviewer.service.integration.WeatherApiRetryPredicate}. + */ + private final boolean retryable; + /** * @param message description of what went wrong calling the external service */ public ExternalHttpCallException(String message) { + this(message, true); + } + + /** + * @param message description of what went wrong calling the external service + * @param retryable whether a retry might succeed + */ + public ExternalHttpCallException(String message, boolean retryable) { super(message); + this.retryable = retryable; } } diff --git a/src/main/java/com/weatherviewer/security/CustomAuthFailureHandler.java b/src/main/java/com/weatherviewer/security/CustomAuthFailureHandler.java index 29ccbbf..7d66e64 100644 --- a/src/main/java/com/weatherviewer/security/CustomAuthFailureHandler.java +++ b/src/main/java/com/weatherviewer/security/CustomAuthFailureHandler.java @@ -4,6 +4,7 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import org.springframework.security.authentication.DisabledException; +import org.springframework.security.authentication.LockedException; import org.springframework.security.core.AuthenticationException; import org.springframework.security.web.authentication.SimpleUrlAuthenticationFailureHandler; import org.springframework.stereotype.Component; @@ -22,6 +23,12 @@ * verification. That case is distinguished from ordinary bad-credentials * failures via an {@code unverified} query parameter, so the sign-in page * can offer to resend the verification email instead of just "try again". + *

+ * A {@link LockedException} means {@link com.weatherviewer.security.SecUser#isAccountNonLocked()} + * is {@code false} — {@link com.weatherviewer.security.AccountLockoutListener} + * locked the account after too many recent failed attempts. That's + * distinguished via a {@code locked} query parameter, so the sign-in page + * can tell the user to wait rather than implying their password is wrong. */ @Component public class CustomAuthFailureHandler extends SimpleUrlAuthenticationFailureHandler { @@ -33,13 +40,23 @@ public CustomAuthFailureHandler() { @Override public void onAuthenticationFailure(HttpServletRequest request, HttpServletResponse response, AuthenticationException exception) throws IOException, ServletException { + if (exception instanceof LockedException) { + redirectWithParam(request, response, "locked"); + return; + } + if (!(exception instanceof DisabledException)) { super.onAuthenticationFailure(request, response, exception); return; } + redirectWithParam(request, response, "unverified"); + } + + private void redirectWithParam(HttpServletRequest request, HttpServletResponse response, String param) + throws IOException { UriComponentsBuilder targetUrl = UriComponentsBuilder.fromPath("/sign-in-failure") - .queryParam("unverified", "true"); + .queryParam(param, "true"); String email = request.getParameter("email"); if (email != null && !email.isBlank()) { diff --git a/src/test/java/com/weatherviewer/controller/AuthControllerTest.java b/src/test/java/com/weatherviewer/controller/AuthControllerTest.java index ad8c08f..63d79a4 100644 --- a/src/test/java/com/weatherviewer/controller/AuthControllerTest.java +++ b/src/test/java/com/weatherviewer/controller/AuthControllerTest.java @@ -301,4 +301,25 @@ void signInFailure_unverifiedTrue_withBlankEmail_redirectsWithUnverifiedQueryPar .andExpect(flash().attribute("errorMessage", "Please verify your email before signing in.")); } + @Test + @WithMockUser + void signInFailure_lockedTrue_withEmail_redirectsWithLockedAndEmailQueryParam() throws Exception { + mockMvc.perform(get("/sign-in-failure") + .param("locked", "true") + .param("email", "john@example.com")) + .andExpect(status().is3xxRedirection()) + .andExpect(redirectedUrl("/sign-in?locked=true&email=john@example.com")) + .andExpect(flash().attributeExists("errorMessage")); + } + + @Test + @WithMockUser + void signInFailure_lockedTrue_withoutEmail_redirectsWithLockedQueryParamOnly() throws Exception { + mockMvc.perform(get("/sign-in-failure") + .param("locked", "true")) + .andExpect(status().is3xxRedirection()) + .andExpect(redirectedUrl("/sign-in?locked=true")) + .andExpect(flash().attributeExists("errorMessage")); + } + } From a31a1a94fb419f86271f46214d293c50e9fe849e Mon Sep 17 00:00:00 2001 From: podlLev Date: Thu, 23 Jul 2026 11:12:22 +0300 Subject: [PATCH 4/5] feat(resilience): add WeatherApiRetryPredicate for weatherApi retries --- .../integration/WeatherApiRetryPredicate.java | 30 +++++++++++++++++++ .../WeatherApiRetryPredicateTest.java | 29 ++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 src/main/java/com/weatherviewer/service/integration/WeatherApiRetryPredicate.java create mode 100644 src/test/java/com/weatherviewer/service/integration/WeatherApiRetryPredicateTest.java diff --git a/src/main/java/com/weatherviewer/service/integration/WeatherApiRetryPredicate.java b/src/main/java/com/weatherviewer/service/integration/WeatherApiRetryPredicate.java new file mode 100644 index 0000000..a678045 --- /dev/null +++ b/src/main/java/com/weatherviewer/service/integration/WeatherApiRetryPredicate.java @@ -0,0 +1,30 @@ +package com.weatherviewer.service.integration; + +import com.weatherviewer.exception.ExternalHttpCallException; + +import java.util.function.Predicate; + +/** + * Decides, for the {@code weatherApi} resilience4j retry instance + * (configured in {@code application.properties} via + * {@code resilience4j.retry.instances.weatherApi.retry-exception-predicate}), + * whether a failure from {@link WeatherApiClient} is worth retrying. + *

+ * Transient failures — network errors, timeouts, malformed responses, and + * 5xx status codes — are retried. Client errors (bad request, invalid API + * key, unknown city) are marked non-retryable on the exception itself by + * {@link WeatherApiClient}, since retrying an inherently-wrong request + * three times only adds latency and burns the rate-limited provider quota + * without any chance of succeeding. + */ +public class WeatherApiRetryPredicate implements Predicate { + + @Override + public boolean test(Throwable throwable) { + if (throwable instanceof ExternalHttpCallException externalHttpCallException) { + return externalHttpCallException.isRetryable(); + } + return true; + } + +} diff --git a/src/test/java/com/weatherviewer/service/integration/WeatherApiRetryPredicateTest.java b/src/test/java/com/weatherviewer/service/integration/WeatherApiRetryPredicateTest.java new file mode 100644 index 0000000..983688e --- /dev/null +++ b/src/test/java/com/weatherviewer/service/integration/WeatherApiRetryPredicateTest.java @@ -0,0 +1,29 @@ +package com.weatherviewer.service.integration; + +import com.weatherviewer.exception.ExternalHttpCallException; +import org.junit.jupiter.api.Test; + +import java.io.IOException; + +import static org.assertj.core.api.Assertions.assertThat; + +class WeatherApiRetryPredicateTest { + + private final WeatherApiRetryPredicate predicate = new WeatherApiRetryPredicate(); + + @Test + void test_retryableExternalHttpCallException_returnsTrue() { + assertThat(predicate.test(new ExternalHttpCallException("5xx from provider", true))).isTrue(); + } + + @Test + void test_nonRetryableExternalHttpCallException_returnsFalse() { + assertThat(predicate.test(new ExternalHttpCallException("404 unknown city", false))).isFalse(); + } + + @Test + void test_otherExceptionTypes_defaultsToRetryable() { + assertThat(predicate.test(new IOException("connection reset"))).isTrue(); + } + +} From aae505036ac63ad2d7a85f4438a7e1a8e9aeffdd Mon Sep 17 00:00:00 2001 From: podlLev Date: Thu, 23 Jul 2026 11:13:20 +0300 Subject: [PATCH 5/5] config: configure weatherApi retry, circuit breaker and add unit tests --- .../service/integration/WeatherApiClient.java | 65 +++++- src/main/resources/application.properties | 21 ++ .../controller/ForecastControllerTest.java | 3 +- .../controller/HomeControllerTest.java | 3 +- .../controller/ProfileControllerTest.java | 3 +- .../controller/SearchControllerTest.java | 3 +- .../ExternalHttpCallExceptionTest.java | 15 ++ .../integration/ForecastIntegrationTest.java | 3 +- .../integration/LocationIntegrationTest.java | 3 +- .../integration/ProfileIntegrationTest.java | 3 +- .../integration/SearchIntegrationTest.java | 5 +- .../WeatherApiIntegrationTest.java | 5 +- .../ratelimit/RateLimitingFilterTest.java | 11 +- .../rest/LocationControllerTest.java | 3 +- .../CustomAuthFailureHandlerTest.java | 24 ++- .../integration/WeatherApiClientTest.java | 203 +++++++++++++++++- 16 files changed, 354 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/weatherviewer/service/integration/WeatherApiClient.java b/src/main/java/com/weatherviewer/service/integration/WeatherApiClient.java index 2fef711..64c297b 100644 --- a/src/main/java/com/weatherviewer/service/integration/WeatherApiClient.java +++ b/src/main/java/com/weatherviewer/service/integration/WeatherApiClient.java @@ -4,6 +4,8 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.weatherviewer.exception.ExternalHttpCallException; +import io.github.resilience4j.circuitbreaker.annotation.CircuitBreaker; +import io.github.resilience4j.retry.annotation.Retry; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Value; @@ -51,7 +53,17 @@ public class WeatherApiClient { @Value("${geo.api.url.suffix}") private String geocodingApiUrlSuffix; - /** Fetches current weather for a city by name. */ + /** + * Fetches current weather for a city by name. + *

+ * Wrapped with the {@code weatherApi} retry (transient failures only, + * see {@link WeatherApiRetryPredicate}) and circuit breaker: once the + * provider is failing consistently, the breaker opens and short-circuits + * straight to {@link #fallbackWeatherByCity} instead of piling up more + * slow/failing calls. + */ + @Retry(name = "weatherApi") + @CircuitBreaker(name = "weatherApi", fallbackMethod = "fallbackWeatherByCity") public JsonNode fetchCurrentWeatherByCity(String city) { log.debug("Building URL and fetching current weather for city: {}", city); String url = buildCityUrl(weatherApiUrlSuffix, city); @@ -59,6 +71,8 @@ public JsonNode fetchCurrentWeatherByCity(String city) { } /** Fetches current weather for a coordinate pair. */ + @Retry(name = "weatherApi") + @CircuitBreaker(name = "weatherApi", fallbackMethod = "fallbackWeatherByCoordinates") public JsonNode fetchCurrentWeatherByCoordinates(double latitude, double longitude) { log.debug("Building URL and fetching current weather for lat: {}, lon: {}", latitude, longitude); String url = buildCoordsUrl(weatherApiUrlSuffix, latitude, longitude); @@ -66,6 +80,8 @@ public JsonNode fetchCurrentWeatherByCoordinates(double latitude, double longitu } /** Fetches the raw 3-hour-step forecast for a city by name. */ + @Retry(name = "weatherApi") + @CircuitBreaker(name = "weatherApi", fallbackMethod = "fallbackForecastByCity") public JsonNode fetchForecastByCity(String city) { log.debug("Building URL and fetching forecast for city: {}", city); String url = buildCityUrl(forecastApiUrlSuffix, city); @@ -73,6 +89,8 @@ public JsonNode fetchForecastByCity(String city) { } /** Fetches the raw 3-hour-step forecast for a coordinate pair. */ + @Retry(name = "weatherApi") + @CircuitBreaker(name = "weatherApi", fallbackMethod = "fallbackForecastByCoordinates") public JsonNode fetchForecastByCoordinates(double latitude, double longitude) { log.debug("Building URL and fetching forecast for lat: {}, lon: {}", latitude, longitude); String url = buildCoordsUrl(forecastApiUrlSuffix, latitude, longitude); @@ -80,12 +98,54 @@ public JsonNode fetchForecastByCoordinates(double latitude, double longitude) { } /** Geocodes a free-text city name into candidate locations. */ + @Retry(name = "weatherApi") + @CircuitBreaker(name = "weatherApi", fallbackMethod = "fallbackGeocodingByCity") public JsonNode fetchGeocodingByCity(String city) { log.debug("Building URL and fetching geocoding data for city: {}", city); String url = buildCityUrl(geocodingApiUrlSuffix, city); return fetchJsonNode(url); } + /** + * Fallback invoked once retries are exhausted or the {@code weatherApi} + * circuit breaker is open. Resilience4j matches this by parameter list + * (original method's args, plus the triggering {@link Throwable}), so + * each public fetch method needs its own overload here even though the + * bodies are identical. + */ + private JsonNode fallbackWeatherByCity(String city, Throwable t) { + return handleFallback("current weather", "city=" + city, t); + } + + private JsonNode fallbackWeatherByCoordinates(double latitude, double longitude, Throwable t) { + return handleFallback("current weather", "lat=" + latitude + ", lon=" + longitude, t); + } + + private JsonNode fallbackForecastByCity(String city, Throwable t) { + return handleFallback("forecast", "city=" + city, t); + } + + private JsonNode fallbackForecastByCoordinates(double latitude, double longitude, Throwable t) { + return handleFallback("forecast", "lat=" + latitude + ", lon=" + longitude, t); + } + + private JsonNode fallbackGeocodingByCity(String city, Throwable t) { + return handleFallback("geocoding", "city=" + city, t); + } + + /** + * Logs the exhausted call and surfaces a single, consistent + * {@link ExternalHttpCallException} regardless of whether we got here + * via a retryable exception running out of attempts or via an open + * circuit breaker ({@code CallNotPermittedException}). Callers (and + * {@link com.weatherviewer.exception.ControllerExceptionHandler}) only + * ever need to handle one exception type. + */ + private JsonNode handleFallback(String operation, String params, Throwable t) { + log.error("Weather API {} call failed after retries/circuit breaker for {}: {}", operation, params, t.toString()); + throw new ExternalHttpCallException("Weather service is temporarily unavailable, please try again shortly", false); + } + /** * Issues the GET request and parses the response body as JSON. * @@ -104,7 +164,8 @@ private JsonNode fetchJsonNode(String url) { throw new ExternalHttpCallException(message); } catch (RestClientResponseException e) { log.error("Weather API returned {} for URL: {}", e.getStatusCode(), maskApiKey(url), e); - throw new ExternalHttpCallException("Weather API error: " + e.getStatusCode()); + boolean retryable = e.getStatusCode() == null || !e.getStatusCode().is4xxClientError(); + throw new ExternalHttpCallException("Weather API error: " + e.getStatusCode(), retryable); } catch (Exception e) { String message = "External HTTP call failed due to network or connection issues"; log.error("{} for URL: {}", message, maskApiKey(url), e); diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 1e10548..54482fe 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -24,6 +24,10 @@ server.servlet.session.cookie.same-site=strict security.remember-me.key=${REMEMBER_ME_KEY} security.remember-me.token-validity-seconds=1209600 +# --- Account lockout (protects a single account against distributed brute force) --- +security.account-lockout.max-attempts=${ACCOUNT_LOCKOUT_MAX_ATTEMPTS:5} +security.account-lockout.lock-duration-minutes=${ACCOUNT_LOCKOUT_DURATION_MINUTES:15} + spring.data.web.pageable.default-page-size=20 spring.data.web.pageable.max-page-size=100 spring.data.web.pageable.one-indexed-parameters=false @@ -54,6 +58,23 @@ weather.api.url.suffix=/data/2.5/weather forecast.api.url.suffix=/data/2.5/forecast geo.api.url.suffix=/geo/1.0/direct weather.api.key=${WEATHER_API_KEY} +weather.api.connect-timeout-ms=${WEATHER_API_CONNECT_TIMEOUT_MS:3000} +weather.api.read-timeout-ms=${WEATHER_API_READ_TIMEOUT_MS:5000} + +# --- Resilience (retry + circuit breaker around the OpenWeatherMap client) --- +resilience4j.retry.instances.weatherApi.max-attempts=3 +resilience4j.retry.instances.weatherApi.wait-duration=500ms +resilience4j.retry.instances.weatherApi.enable-exponential-backoff=true +resilience4j.retry.instances.weatherApi.exponential-backoff-multiplier=2 +resilience4j.retry.instances.weatherApi.retry-exception-predicate=com.weatherviewer.service.integration.WeatherApiRetryPredicate + +resilience4j.circuitbreaker.instances.weatherApi.sliding-window-type=COUNT_BASED +resilience4j.circuitbreaker.instances.weatherApi.sliding-window-size=20 +resilience4j.circuitbreaker.instances.weatherApi.minimum-number-of-calls=10 +resilience4j.circuitbreaker.instances.weatherApi.failure-rate-threshold=50 +resilience4j.circuitbreaker.instances.weatherApi.wait-duration-in-open-state=30s +resilience4j.circuitbreaker.instances.weatherApi.permitted-number-of-calls-in-half-open-state=5 +resilience4j.circuitbreaker.instances.weatherApi.automatic-transition-from-open-to-half-open-enabled=true # --- Redis / caching --- spring.data.redis.host=${SPRING_DATA_REDIS_HOST:localhost} diff --git a/src/test/java/com/weatherviewer/controller/ForecastControllerTest.java b/src/test/java/com/weatherviewer/controller/ForecastControllerTest.java index 26052a3..65a9963 100644 --- a/src/test/java/com/weatherviewer/controller/ForecastControllerTest.java +++ b/src/test/java/com/weatherviewer/controller/ForecastControllerTest.java @@ -67,7 +67,8 @@ private SecUser secUser() { Set.of(), true, "John Doe", - UnitSystem.METRIC + UnitSystem.METRIC, + null ); } diff --git a/src/test/java/com/weatherviewer/controller/HomeControllerTest.java b/src/test/java/com/weatherviewer/controller/HomeControllerTest.java index f1c0e97..b8d95f1 100644 --- a/src/test/java/com/weatherviewer/controller/HomeControllerTest.java +++ b/src/test/java/com/weatherviewer/controller/HomeControllerTest.java @@ -48,7 +48,8 @@ private SecUser secUser() { Set.of(), true, "John Doe", - UnitSystem.METRIC + UnitSystem.METRIC, + null ); } diff --git a/src/test/java/com/weatherviewer/controller/ProfileControllerTest.java b/src/test/java/com/weatherviewer/controller/ProfileControllerTest.java index 4339257..1c10e96 100644 --- a/src/test/java/com/weatherviewer/controller/ProfileControllerTest.java +++ b/src/test/java/com/weatherviewer/controller/ProfileControllerTest.java @@ -52,7 +52,8 @@ private SecUser secUser() { Set.of(), true, "John Doe", - UnitSystem.METRIC + UnitSystem.METRIC, + null ); } diff --git a/src/test/java/com/weatherviewer/controller/SearchControllerTest.java b/src/test/java/com/weatherviewer/controller/SearchControllerTest.java index 2b030e6..332655a 100644 --- a/src/test/java/com/weatherviewer/controller/SearchControllerTest.java +++ b/src/test/java/com/weatherviewer/controller/SearchControllerTest.java @@ -63,7 +63,8 @@ private SecUser secUser() { Set.of(), true, "John Doe", - UnitSystem.METRIC + UnitSystem.METRIC, + null ); } diff --git a/src/test/java/com/weatherviewer/exception/ExternalHttpCallExceptionTest.java b/src/test/java/com/weatherviewer/exception/ExternalHttpCallExceptionTest.java index 8871dc5..6668070 100644 --- a/src/test/java/com/weatherviewer/exception/ExternalHttpCallExceptionTest.java +++ b/src/test/java/com/weatherviewer/exception/ExternalHttpCallExceptionTest.java @@ -26,4 +26,19 @@ void thrown_canBeCaughtAsRuntimeException() { .hasMessage("External call failed"); } + @Test + void singleArgConstructor_defaultsToRetryable() { + ExternalHttpCallException ex = new ExternalHttpCallException("Timeout"); + assertThat(ex.isRetryable()).isTrue(); + } + + @Test + void twoArgConstructor_setsRetryableFlag() { + ExternalHttpCallException retryable = new ExternalHttpCallException("5xx from provider", true); + ExternalHttpCallException notRetryable = new ExternalHttpCallException("404 unknown city", false); + + assertThat(retryable.isRetryable()).isTrue(); + assertThat(notRetryable.isRetryable()).isFalse(); + } + } diff --git a/src/test/java/com/weatherviewer/integration/ForecastIntegrationTest.java b/src/test/java/com/weatherviewer/integration/ForecastIntegrationTest.java index 8178220..fc64bde 100644 --- a/src/test/java/com/weatherviewer/integration/ForecastIntegrationTest.java +++ b/src/test/java/com/weatherviewer/integration/ForecastIntegrationTest.java @@ -58,7 +58,8 @@ void setUp() { Set.of(), true, savedUser.getFullName(), - UnitSystem.METRIC + UnitSystem.METRIC, + null ); } diff --git a/src/test/java/com/weatherviewer/integration/LocationIntegrationTest.java b/src/test/java/com/weatherviewer/integration/LocationIntegrationTest.java index 3311e1d..efa8110 100644 --- a/src/test/java/com/weatherviewer/integration/LocationIntegrationTest.java +++ b/src/test/java/com/weatherviewer/integration/LocationIntegrationTest.java @@ -63,7 +63,8 @@ void setUp() { Set.of(), true, savedUser.getFullName(), - UnitSystem.METRIC + UnitSystem.METRIC, + null ); } diff --git a/src/test/java/com/weatherviewer/integration/ProfileIntegrationTest.java b/src/test/java/com/weatherviewer/integration/ProfileIntegrationTest.java index b8d4406..c412a1d 100644 --- a/src/test/java/com/weatherviewer/integration/ProfileIntegrationTest.java +++ b/src/test/java/com/weatherviewer/integration/ProfileIntegrationTest.java @@ -57,7 +57,8 @@ void setUp() { Set.of(), true, savedUser.getFullName(), - UnitSystem.METRIC + UnitSystem.METRIC, + null ); } diff --git a/src/test/java/com/weatherviewer/integration/SearchIntegrationTest.java b/src/test/java/com/weatherviewer/integration/SearchIntegrationTest.java index 8cad246..64d536b 100644 --- a/src/test/java/com/weatherviewer/integration/SearchIntegrationTest.java +++ b/src/test/java/com/weatherviewer/integration/SearchIntegrationTest.java @@ -70,7 +70,8 @@ void setUp() { Set.of(), true, savedUser.getFullName(), - UnitSystem.METRIC + UnitSystem.METRIC, + null ); } @@ -181,7 +182,7 @@ void addLocation_sameCoordinatesDifferentUser_bothSaved() throws Exception { SecUser otherSecUser = new SecUser( otherUser.getId(), otherUser.getEmail(), otherUser.getPassword(), Set.of(), true, otherUser.getFullName(), - UnitSystem.METRIC + UnitSystem.METRIC, null ); mockMvc.perform(post("/search/add") diff --git a/src/test/java/com/weatherviewer/integration/WeatherApiIntegrationTest.java b/src/test/java/com/weatherviewer/integration/WeatherApiIntegrationTest.java index e3c916a..9745893 100644 --- a/src/test/java/com/weatherviewer/integration/WeatherApiIntegrationTest.java +++ b/src/test/java/com/weatherviewer/integration/WeatherApiIntegrationTest.java @@ -12,6 +12,7 @@ import org.springframework.cache.CacheManager; import org.springframework.http.MediaType; import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.test.web.client.ExpectedCount; import org.springframework.test.web.client.MockRestServiceServer; import org.springframework.test.web.client.response.MockRestResponseCreators; import org.springframework.web.client.RestClient; @@ -96,7 +97,7 @@ void getWeatherByCoordinates_realHttpFlow_returnsMappedWeatherDto() { @Test void getWeatherByCity_apiReturnsError_throwsExternalHttpCallException() { - mockServer.expect(method(GET)) + mockServer.expect(ExpectedCount.times(3), method(GET)) .andRespond(MockRestResponseCreators.withServerError()); assertThatThrownBy(() -> weatherApiService.getWeatherByCity("Kyiv")) @@ -107,7 +108,7 @@ void getWeatherByCity_apiReturnsError_throwsExternalHttpCallException() { @Test void getWeatherByCity_malformedJson_throwsExternalHttpCallException() { - mockServer.expect(method(GET)) + mockServer.expect(ExpectedCount.times(3), method(GET)) .andRespond(withSuccess("not-valid-json{{{", MediaType.APPLICATION_JSON)); assertThatThrownBy(() -> weatherApiService.getWeatherByCity("Kyiv")) diff --git a/src/test/java/com/weatherviewer/ratelimit/RateLimitingFilterTest.java b/src/test/java/com/weatherviewer/ratelimit/RateLimitingFilterTest.java index a0253bb..8bb49a9 100644 --- a/src/test/java/com/weatherviewer/ratelimit/RateLimitingFilterTest.java +++ b/src/test/java/com/weatherviewer/ratelimit/RateLimitingFilterTest.java @@ -1,5 +1,6 @@ package com.weatherviewer.ratelimit; +import com.weatherviewer.model.enums.UnitSystem; import com.weatherviewer.security.SecUser; import jakarta.servlet.FilterChain; import org.junit.jupiter.api.AfterEach; @@ -55,7 +56,15 @@ private RateLimitProperties.Rule rule(String prefix, int limit) { } private void authenticateAs(UUID userId) { - SecUser secUser = new SecUser(userId, "john@example.com", "hashed", java.util.Set.of(), true, "John Doe", com.weatherviewer.model.enums.UnitSystem.METRIC); + SecUser secUser = new SecUser( + userId, + "john@example.com", + "hashed", + java.util.Set.of(), + true, + "John Doe", + UnitSystem.METRIC, + null); SecurityContextHolder.getContext().setAuthentication( new UsernamePasswordAuthenticationToken(secUser, null, secUser.getAuthorities())); } diff --git a/src/test/java/com/weatherviewer/rest/LocationControllerTest.java b/src/test/java/com/weatherviewer/rest/LocationControllerTest.java index 6792e51..b2417f8 100644 --- a/src/test/java/com/weatherviewer/rest/LocationControllerTest.java +++ b/src/test/java/com/weatherviewer/rest/LocationControllerTest.java @@ -55,7 +55,8 @@ void setUp() { Set.of(), true, "John Doe", - UnitSystem.METRIC + UnitSystem.METRIC, + null ); } diff --git a/src/test/java/com/weatherviewer/security/CustomAuthFailureHandlerTest.java b/src/test/java/com/weatherviewer/security/CustomAuthFailureHandlerTest.java index 60b8339..cae2e38 100644 --- a/src/test/java/com/weatherviewer/security/CustomAuthFailureHandlerTest.java +++ b/src/test/java/com/weatherviewer/security/CustomAuthFailureHandlerTest.java @@ -9,6 +9,7 @@ import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.authentication.DisabledException; +import org.springframework.security.authentication.LockedException; import org.springframework.security.core.AuthenticationException; import java.io.IOException; @@ -71,7 +72,28 @@ void onAuthenticationFailure_disabledException_withoutEmail_redirectsWithoutEmai } @Test - void onAuthenticationFailure_disabledException_withBlankEmail_redirectsWithoutEmailParam() + void onAuthenticationFailure_lockedException_redirectsWithLockedAndEmailParam() + throws IOException, ServletException { + request.setParameter("email", "john@example.com"); + AuthenticationException exception = new LockedException("Account is locked"); + + failureHandler.onAuthenticationFailure(request, response, exception); + + assertEquals("/sign-in-failure?locked=true&email=john@example.com", response.getRedirectedUrl()); + } + + @Test + void onAuthenticationFailure_lockedException_withoutEmail_redirectsWithLockedParamOnly() + throws IOException, ServletException { + AuthenticationException exception = new LockedException("Account is locked"); + + failureHandler.onAuthenticationFailure(request, response, exception); + + assertEquals("/sign-in-failure?locked=true", response.getRedirectedUrl()); + } + + @Test + void onAuthenticationFailure_disabledException_withBlankEmail_redirectsWithUnverifiedParamOnly() throws IOException, ServletException { request.setParameter("email", " "); AuthenticationException exception = new DisabledException("Account is disabled"); diff --git a/src/test/java/com/weatherviewer/service/integration/WeatherApiClientTest.java b/src/test/java/com/weatherviewer/service/integration/WeatherApiClientTest.java index eaaef2d..5eb62e7 100644 --- a/src/test/java/com/weatherviewer/service/integration/WeatherApiClientTest.java +++ b/src/test/java/com/weatherviewer/service/integration/WeatherApiClientTest.java @@ -20,6 +20,7 @@ import org.springframework.web.client.RestClient; import org.springframework.web.client.RestClientResponseException; +import java.lang.reflect.Method; import java.net.URI; import java.util.List; @@ -61,9 +62,9 @@ void setUp() { ReflectionTestUtils.setField(client, "forecastApiUrlSuffix", "/data/2.5/forecast"); ReflectionTestUtils.setField(client, "geocodingApiUrlSuffix", "/geo/1.0/direct"); - doReturn(requestHeadersUriSpec).when(restClient).get(); - doReturn(requestHeadersSpec).when(requestHeadersUriSpec).uri(any(URI.class)); - when(requestHeadersSpec.retrieve()).thenReturn(responseSpec); + lenient().doReturn(requestHeadersUriSpec).when(restClient).get(); + lenient().doReturn(requestHeadersSpec).when(requestHeadersUriSpec).uri(any(URI.class)); + lenient().when(requestHeadersSpec.retrieve()).thenReturn(responseSpec); logAppender = new ListAppender<>(); logAppender.start(); @@ -222,6 +223,202 @@ void fetchJsonNode_errorLogging_masksApiKeyRegardlessOfSuffixOrParamOrder() { assertLoggedMessagesContainNoApiKeyAndAreMasked(); } + @Test + void fetchCurrentWeatherByCity_buildsUrlWithMetricUnitsAndEnglishLang() throws Exception { + JsonNode mockNode = mock(JsonNode.class); + when(responseSpec.body(String.class)).thenReturn("{}"); + when(objectMapper.readTree(anyString())).thenReturn(mockNode); + + client.fetchCurrentWeatherByCity("Kyiv"); + + ArgumentCaptor uriCaptor = ArgumentCaptor.forClass(URI.class); + verify(requestHeadersUriSpec).uri(uriCaptor.capture()); + String uri = uriCaptor.getValue().toString(); + + assertThat(uri).startsWith("https://api.openweathermap.org/data/2.5/weather"); + assertThat(uri).contains("units=metric"); + assertThat(uri).contains("lang=en"); + assertThat(uri).contains("q=Kyiv"); + assertThat(uri).contains("appid=" + SECRET_API_KEY); + } + + @Test + void fetchForecastByCity_buildsUrlWithForecastSuffix() throws Exception { + JsonNode mockNode = mock(JsonNode.class); + when(responseSpec.body(String.class)).thenReturn("{}"); + when(objectMapper.readTree(anyString())).thenReturn(mockNode); + + client.fetchForecastByCity("Kyiv"); + + ArgumentCaptor uriCaptor = ArgumentCaptor.forClass(URI.class); + verify(requestHeadersUriSpec).uri(uriCaptor.capture()); + String uri = uriCaptor.getValue().toString(); + + assertThat(uri).startsWith("https://api.openweathermap.org/data/2.5/forecast"); + assertThat(uri).contains("units=metric"); + assertThat(uri).contains("lang=en"); + assertThat(uri).contains("q=Kyiv"); + } + + @Test + void fetchGeocodingByCity_buildsUrlWithGeoSuffix() throws Exception { + JsonNode mockNode = mock(JsonNode.class); + when(responseSpec.body(String.class)).thenReturn("[]"); + when(objectMapper.readTree(anyString())).thenReturn(mockNode); + + client.fetchGeocodingByCity("Kyiv"); + + ArgumentCaptor uriCaptor = ArgumentCaptor.forClass(URI.class); + verify(requestHeadersUriSpec).uri(uriCaptor.capture()); + String uri = uriCaptor.getValue().toString(); + + assertThat(uri).startsWith("https://api.openweathermap.org/geo/1.0/direct"); + assertThat(uri).contains("q=Kyiv"); + } + + @Test + void fetchCurrentWeatherByCoordinates_buildsUrlWithLatLonAndNoQParam() throws Exception { + JsonNode mockNode = mock(JsonNode.class); + when(responseSpec.body(String.class)).thenReturn("{}"); + when(objectMapper.readTree(anyString())).thenReturn(mockNode); + + client.fetchCurrentWeatherByCoordinates(50.45, 30.52); + + ArgumentCaptor uriCaptor = ArgumentCaptor.forClass(URI.class); + verify(requestHeadersUriSpec).uri(uriCaptor.capture()); + String uri = uriCaptor.getValue().toString(); + + assertThat(uri).contains("lat=50.45"); + assertThat(uri).contains("lon=30.52"); + assertThat(uri).contains("units=metric"); + assertThat(uri).contains("lang=en"); + assertThat(uri).doesNotContain("q="); + } + + @Test + void fetchForecastByCoordinates_buildsUrlWithLatLonAndForecastSuffix() throws Exception { + JsonNode mockNode = mock(JsonNode.class); + when(responseSpec.body(String.class)).thenReturn("{}"); + when(objectMapper.readTree(anyString())).thenReturn(mockNode); + + client.fetchForecastByCoordinates(50.45, 30.52); + + ArgumentCaptor uriCaptor = ArgumentCaptor.forClass(URI.class); + verify(requestHeadersUriSpec).uri(uriCaptor.capture()); + String uri = uriCaptor.getValue().toString(); + + assertThat(uri).startsWith("https://api.openweathermap.org/data/2.5/forecast"); + assertThat(uri).contains("lat=50.45"); + assertThat(uri).contains("lon=30.52"); + } + + @Test + void fetchCurrentWeatherByCoordinates_encodesNegativeAndDecimalCoordinatesCorrectly() throws Exception { + JsonNode mockNode = mock(JsonNode.class); + when(responseSpec.body(String.class)).thenReturn("{}"); + when(objectMapper.readTree(anyString())).thenReturn(mockNode); + + client.fetchCurrentWeatherByCoordinates(-33.8688, 151.2093); + + ArgumentCaptor uriCaptor = ArgumentCaptor.forClass(URI.class); + verify(requestHeadersUriSpec).uri(uriCaptor.capture()); + String uri = uriCaptor.getValue().toString(); + + assertThat(uri).contains("lat=-33.8688"); + assertThat(uri).contains("lon=151.2093"); + assertThat(uri).doesNotContain("%25"); + } + + @Test + void fetchJsonNode_restClientResponseExceptionWith4xxStatus_isNotRetryable() { + RestClientResponseException ex = mock(RestClientResponseException.class); + when(ex.getStatusCode()).thenReturn(org.springframework.http.HttpStatus.NOT_FOUND); + when(responseSpec.body(String.class)).thenThrow(ex); + + assertThatThrownBy(() -> client.fetchCurrentWeatherByCity("Kyiv")) + .isInstanceOf(ExternalHttpCallException.class) + .satisfies(e -> assertThat(((ExternalHttpCallException) e).isRetryable()).isFalse()); + } + + @Test + void fetchJsonNode_restClientResponseExceptionWith5xxStatus_isRetryable() { + RestClientResponseException ex = mock(RestClientResponseException.class); + when(ex.getStatusCode()).thenReturn(org.springframework.http.HttpStatus.SERVICE_UNAVAILABLE); + when(responseSpec.body(String.class)).thenThrow(ex); + + assertThatThrownBy(() -> client.fetchCurrentWeatherByCity("Kyiv")) + .isInstanceOf(ExternalHttpCallException.class) + .satisfies(e -> assertThat(((ExternalHttpCallException) e).isRetryable()).isTrue()); + } + + @Test + void fetchJsonNode_restClientResponseExceptionWith429_isRetryable() { + RestClientResponseException ex = mock(RestClientResponseException.class); + when(ex.getStatusCode()).thenReturn(org.springframework.http.HttpStatus.TOO_MANY_REQUESTS); + when(responseSpec.body(String.class)).thenThrow(ex); + + assertThatThrownBy(() -> client.fetchCurrentWeatherByCity("Kyiv")) + .isInstanceOf(ExternalHttpCallException.class) + .satisfies(e -> assertThat(((ExternalHttpCallException) e).isRetryable()).isFalse()); + } + + @Test + void maskApiKey_masksAppidRegardlessOfCaseAndPosition() throws Exception { + Method maskApiKey = WeatherApiClient.class.getDeclaredMethod("maskApiKey", String.class); + maskApiKey.setAccessible(true); + + assertThat((String) maskApiKey.invoke(null, "https://x.com/y?appid=secret123&units=metric")) + .isEqualTo("https://x.com/y?appid=***&units=metric"); + + assertThat((String) maskApiKey.invoke(null, "https://x.com/y?units=metric&appid=secret123")) + .isEqualTo("https://x.com/y?units=metric&appid=***"); + + assertThat((String) maskApiKey.invoke(null, "https://x.com/y?APPID=secret123")) + .isEqualTo("https://x.com/y?APPID=***"); + + assertThat((String) maskApiKey.invoke(null, "https://x.com/y?units=metric&appid=secret123&lang=en")) + .isEqualTo("https://x.com/y?units=metric&appid=***&lang=en"); + } + + @Test + void maskApiKey_leavesUrlUnchangedWhenNoAppidPresent() throws Exception { + Method maskApiKey = WeatherApiClient.class.getDeclaredMethod("maskApiKey", String.class); + maskApiKey.setAccessible(true); + + String url = "https://x.com/y?units=metric&lang=en"; + assertThat((String) maskApiKey.invoke(null, url)).isEqualTo(url); + } + + @Test + void fetchForecastByCity_networkException_throwsExternalHttpCallException() { + when(responseSpec.body(String.class)) + .thenThrow(new RuntimeException("Connection refused")); + + assertThatThrownBy(() -> client.fetchForecastByCity("Kyiv")) + .isInstanceOf(ExternalHttpCallException.class) + .hasMessageContaining("External HTTP call failed"); + } + + @Test + void fetchGeocodingByCity_networkException_throwsExternalHttpCallException() { + when(responseSpec.body(String.class)) + .thenThrow(new RuntimeException("Connection refused")); + + assertThatThrownBy(() -> client.fetchGeocodingByCity("Kyiv")) + .isInstanceOf(ExternalHttpCallException.class) + .hasMessageContaining("External HTTP call failed"); + } + + @Test + void fetchForecastByCoordinates_networkException_throwsExternalHttpCallException() { + when(responseSpec.body(String.class)) + .thenThrow(new RuntimeException("Connection refused")); + + assertThatThrownBy(() -> client.fetchForecastByCoordinates(50.45, 30.52)) + .isInstanceOf(ExternalHttpCallException.class) + .hasMessageContaining("External HTTP call failed"); + } + private void assertLoggedMessagesContainNoApiKeyAndAreMasked() { List events = logAppender.list; assertThat(events).isNotEmpty();