From 48c993f65c2353909b232ceeb43483ba2f0e50bf Mon Sep 17 00:00:00 2001 From: Sae126V Date: Mon, 2 Mar 2026 09:33:51 +0000 Subject: [PATCH 1/3] Add code to Implement account lockout after configured failed attempts - Provide default values to the account lockout config --- .../lockout/AccountLockoutController.java | 61 ++++++ .../lockout/DefaultLoginLockoutService.java | 205 ++++++++++++++++++ .../authn/lockout/LoginLockoutService.java | 50 +++++ .../MultiFactorTotpCheckProvider.java | 15 +- .../it/infn/mw/iam/config/IamProperties.java | 50 +++++ .../infn/mw/iam/config/IamTotpMfaConfig.java | 6 +- .../config/security/IamWebSecurityConfig.java | 6 +- .../core/IamLocalAuthenticationProvider.java | 29 ++- .../src/main/resources/application.yml | 6 + .../user/detail/user.detail.component.html | 21 +- .../user/detail/user.detail.component.js | 2 +- .../userslist/users.userslist.component.html | 8 +- .../userslist/users.userslist.component.js | 15 +- .../dashboard-app/services/user.service.js | 17 +- .../dashboard-app/services/users.service.js | 2 +- .../DefaultLoginLockoutServiceTests.java | 157 ++++++++++++++ .../IamLocalAuthenticationProviderTests.java | 5 +- .../MultiFactorTotpCheckProviderTests.java | 6 +- .../model/IamAccountLoginLockout.java | 146 +++++++++++++ .../IamAccountLoginLockoutRepository.java | 38 ++++ .../db/migration/h2/V119__login_lockout.sql | 13 ++ .../migration/mysql/V119__login_lockout.sql | 13 ++ 22 files changed, 845 insertions(+), 26 deletions(-) create mode 100644 iam-login-service/src/main/java/it/infn/mw/iam/api/account/lockout/AccountLockoutController.java create mode 100644 iam-login-service/src/main/java/it/infn/mw/iam/authn/lockout/DefaultLoginLockoutService.java create mode 100644 iam-login-service/src/main/java/it/infn/mw/iam/authn/lockout/LoginLockoutService.java create mode 100644 iam-login-service/src/test/java/it/infn/mw/iam/test/authn/lockout/DefaultLoginLockoutServiceTests.java create mode 100644 iam-persistence/src/main/java/it/infn/mw/iam/persistence/model/IamAccountLoginLockout.java create mode 100644 iam-persistence/src/main/java/it/infn/mw/iam/persistence/repository/IamAccountLoginLockoutRepository.java create mode 100644 iam-persistence/src/main/resources/db/migration/h2/V119__login_lockout.sql create mode 100644 iam-persistence/src/main/resources/db/migration/mysql/V119__login_lockout.sql diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/account/lockout/AccountLockoutController.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/account/lockout/AccountLockoutController.java new file mode 100644 index 0000000000..285fc10a4d --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/account/lockout/AccountLockoutController.java @@ -0,0 +1,61 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.api.account.lockout; + +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import org.springframework.http.ResponseEntity; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RestController; + +import it.infn.mw.iam.persistence.model.IamAccountLoginLockout; +import it.infn.mw.iam.persistence.repository.IamAccountLoginLockoutRepository; + +@RestController +public class AccountLockoutController { + + private final IamAccountLoginLockoutRepository lockoutRepo; + + public AccountLockoutController(IamAccountLoginLockoutRepository lockoutRepo) { + this.lockoutRepo = lockoutRepo; + } + + @GetMapping("/iam/account/{uuid}/lockout") + @PreAuthorize("#iam.hasScope('iam:admin.read') or #iam.hasDashboardRole('ROLE_ADMIN')") + public ResponseEntity getLockoutStatus(@PathVariable String uuid) { + Optional lockout = lockoutRepo.findByAccountUuid(uuid); + + if (lockout.isPresent() && lockout.get().getSuspendedUntil() != null + && System.currentTimeMillis() < lockout.get().getSuspendedUntil().getTime()) { + return ResponseEntity.ok(Map.of( + "suspended", true, + "suspendedUntil", lockout.get().getSuspendedUntil().getTime())); + } + + return ResponseEntity.ok(Map.of("suspended", false)); + } + + @GetMapping("/iam/account/lockout/suspended") + @PreAuthorize("#iam.hasScope('iam:admin.read') or #iam.hasDashboardRole('ROLE_ADMIN')") + public ResponseEntity getAllSuspendedUsers() { + List suspendedUsers = lockoutRepo.findAllSuspendedUsers(); + return ResponseEntity.ok(suspendedUsers); + } +} diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/authn/lockout/DefaultLoginLockoutService.java b/iam-login-service/src/main/java/it/infn/mw/iam/authn/lockout/DefaultLoginLockoutService.java new file mode 100644 index 0000000000..b036ae4f5c --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/authn/lockout/DefaultLoginLockoutService.java @@ -0,0 +1,205 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.authn.lockout; + +import java.util.Date; +import java.util.Optional; + +import javax.annotation.PostConstruct; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.security.authentication.LockedException; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import it.infn.mw.iam.config.IamProperties; +import it.infn.mw.iam.config.IamProperties.LoginLockoutProperties; +import it.infn.mw.iam.persistence.model.IamAccount; +import it.infn.mw.iam.persistence.model.IamAccountLoginLockout; +import it.infn.mw.iam.persistence.repository.IamAccountRepository; +import it.infn.mw.iam.persistence.repository.IamAccountLoginLockoutRepository; + +@Service +public class DefaultLoginLockoutService implements LoginLockoutService { + + private static final Logger LOG = LoggerFactory.getLogger(DefaultLoginLockoutService.class); + + private final IamAccountLoginLockoutRepository lockoutRepo; + private final IamAccountRepository accountRepo; + private final LoginLockoutProperties properties; + + public DefaultLoginLockoutService(IamAccountLoginLockoutRepository lockoutRepo, + IamAccountRepository accountRepo, IamProperties iamProperties) { + this.lockoutRepo = lockoutRepo; + this.accountRepo = accountRepo; + this.properties = iamProperties.getLoginLockout(); + } + + @PostConstruct + void validateConfiguration() { + if (!properties.isEnabled()) { + LOG.info("Iam Account Login lockout feature is disabled"); + return; + } + + if (properties.getMaxFailedAttempts() < 1) { + throw new IllegalStateException( + "iam.login-lockout.max-failed-attempts must be >= 1. " + + "Please provide the maximum number of failed login attempts allowed before suspension."); + } + + if (properties.getLockoutMinutes() < 1) { + throw new IllegalStateException( + "iam.login-lockout.lockout-minutes must be >= 1. " + + "Please provide the suspension duration in minutes."); + } + + if (properties.getMaxConcurrentFailures() < 1) { + throw new IllegalStateException( + "iam.login-lockout.max-concurrent-failures must be >= 1. " + + "Please provide the maximum number of suspension rounds allowed before the account is permanently disabled."); + } + + LOG.info("Iam Account Login lockout enabled: max-failed-attempts={}, lockout-minutes={}, " + + "max-concurrent-failures={}", properties.getMaxFailedAttempts(), + properties.getLockoutMinutes(), properties.getMaxConcurrentFailures()); + } + + @Override + @Transactional + public void checkIamAccountLockout(String username) { + + if (!properties.isEnabled()) { + return; + } + + Optional lockoutOpt = lockoutRepo.findByAccountUsername(username); + + if (lockoutOpt.isEmpty()) { + return; + } + + IamAccountLoginLockout lockout = lockoutOpt.get(); + + if (isSuspended(lockout)) { + LOG.info("Login blocked: account '{}' is suspended until {}", username, + lockout.getSuspendedUntil()); + throw new LockedException( + "Account is temporarily suspended. Please try again later or contact support for assistance."); + } + + // Previous suspension has expired; reset the attempt counter for a fresh round + if (lockout.getSuspendedUntil() != null) { + LOG.debug("Suspension for '{}' has expired, starting fresh round", username); + lockout.setFailedAttempts(0); + lockout.setFirstFailureTime(null); + lockout.setSuspendedUntil(null); + lockoutRepo.save(lockout); + } + } + + @Override + @Transactional + public void recordFailedAttempt(String username) { + + if (!properties.isEnabled()) { + return; + } + + Optional accountOpt = accountRepo.findByUsername(username); + + if (accountOpt.isEmpty()) { + return; + } + + IamAccount account = accountOpt.get(); + + if (!account.isActive()) { + return; + } + + IamAccountLoginLockout lockout = lockoutRepo.findByAccountUsername(username) + .orElseGet(() -> new IamAccountLoginLockout(account)); + + if (isSuspended(lockout)) { + return; + } + + // if a previous suspension has expired but checkLockout was not called, + // reset the counter so we don't carry over stale failedAttempts from the prior round. + if (lockout.getSuspendedUntil() != null) { + lockout.setFailedAttempts(0); + lockout.setFirstFailureTime(null); + lockout.setSuspendedUntil(null); + } + + Date now = new Date(); + + if (lockout.getFailedAttempts() == 0) { + lockout.setFirstFailureTime(now); + } + + lockout.setFailedAttempts(lockout.getFailedAttempts() + 1); + + LOG.info("Failed login attempt {} of {} for account '{}'", lockout.getFailedAttempts(), + properties.getMaxFailedAttempts(), username); + + if (lockout.getFailedAttempts() >= properties.getMaxFailedAttempts()) { + + lockout.setLockoutCount(lockout.getLockoutCount() + 1); + + if (lockout.getLockoutCount() > properties.getMaxConcurrentFailures()) { + // All suspension rounds exhausted; disable the account and clean up + account.setActive(false); + accountRepo.save(account); + lockoutRepo.delete(lockout); + LOG.warn("Account '{}' disabled after {} suspension rounds", username, + properties.getMaxConcurrentFailures()); + return; + } + + // Suspend for the configured duration + long suspendUntilMs = now.getTime() + ((long) properties.getLockoutMinutes() * 60 * 1000); + lockout.setSuspendedUntil(new Date(suspendUntilMs)); + + LOG.warn("Account '{}' suspended until {} (round {} of {})", username, + lockout.getSuspendedUntil(), lockout.getLockoutCount(), + properties.getMaxConcurrentFailures()); + } + + lockoutRepo.save(lockout); + } + + @Override + @Transactional + public void resetFailedAttempts(String username) { + + if (!properties.isEnabled()) { + return; + } + + lockoutRepo.findByAccountUsername(username).ifPresent(lockout -> { + lockoutRepo.delete(lockout); + LOG.debug("Lockout record deleted for account '{}'", username); + }); + } + + private boolean isSuspended(IamAccountLoginLockout lockout) { + return lockout.getSuspendedUntil() != null + && System.currentTimeMillis() < lockout.getSuspendedUntil().getTime(); + } +} diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/authn/lockout/LoginLockoutService.java b/iam-login-service/src/main/java/it/infn/mw/iam/authn/lockout/LoginLockoutService.java new file mode 100644 index 0000000000..eeecdc4d5f --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/authn/lockout/LoginLockoutService.java @@ -0,0 +1,50 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.authn.lockout; + +/** + * Tracks failed login attempts and enforces temporary suspensions and permanent account disabling. + * + * Password failures and TOTP failures share the same counter. The lifecycle: + * + * User fails {@code max-failed-attempts} times => suspended for {@code lockout-minutes} + * Suspension expires => counter resets, user gets another round of attempts + * After {@code max-concurrent-failures} suspension rounds, the next round of failures + * disables the account ({@code active = false}) and the lockout row is deleted + * An admin can re-enable the account since the lockout row is gone, the user starts fresh + */ +public interface LoginLockoutService { + + /** + * Throws {@link org.springframework.security.authentication.LockedException} if the account + * is currently suspended. If a previous suspension has expired, silently resets the attempt + * counter for a fresh round. + */ + void checkIamAccountLockout(String username); + + /** + * Records a single failed attempt (password or TOTP). When the attempt count reaches the + * threshold the account is suspended. When all suspension rounds are exhausted the account + * is disabled and the lockout row is deleted. + */ + void recordFailedAttempt(String username); + + /** + * Deletes the lockout row for the given username. Called after a fully successful + * authentication (password-only login, or TOTP verification). + */ + void resetFailedAttempts(String username); +} diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/authn/multi_factor_authentication/MultiFactorTotpCheckProvider.java b/iam-login-service/src/main/java/it/infn/mw/iam/authn/multi_factor_authentication/MultiFactorTotpCheckProvider.java index 652a1a8b0d..02f0c0c8f4 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/authn/multi_factor_authentication/MultiFactorTotpCheckProvider.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/authn/multi_factor_authentication/MultiFactorTotpCheckProvider.java @@ -27,6 +27,7 @@ import it.infn.mw.iam.api.account.multi_factor_authentication.IamTotpMfaService; import it.infn.mw.iam.authn.AbstractExternalAuthenticationToken; +import it.infn.mw.iam.authn.lockout.LoginLockoutService; import it.infn.mw.iam.core.ExtendedAuthenticationToken; import it.infn.mw.iam.core.user.exception.MfaSecretNotFoundException; import it.infn.mw.iam.persistence.model.IamAccount; @@ -40,11 +41,13 @@ public class MultiFactorTotpCheckProvider implements AuthenticationProvider { private final IamAccountRepository accountRepo; private final IamTotpMfaService totpMfaService; + private final LoginLockoutService lockoutService; public MultiFactorTotpCheckProvider(IamAccountRepository accountRepo, - IamTotpMfaService totpMfaService) { + IamTotpMfaService totpMfaService, LoginLockoutService lockoutService) { this.accountRepo = accountRepo; this.totpMfaService = totpMfaService; + this.lockoutService = lockoutService; } @Override @@ -62,13 +65,21 @@ private Authentication processAuthentication(Authentication authentication) { return null; } - IamAccount account = accountRepo.findByUsername(authentication.getName()) + String username = authentication.getName(); + + lockoutService.checkIamAccountLockout(username); + + IamAccount account = accountRepo.findByUsername(username) .orElseThrow(() -> new BadCredentialsException("Invalid login details")); if (!isValidTotp(account, totp)) { + lockoutService.recordFailedAttempt(username); throw new BadCredentialsException("Bad TOTP"); } + // TOTP verified; full authentication achieved. Clear lockout state. + lockoutService.resetFailedAttempts(username); + return createSuccessfulAuthentication(authentication); } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/IamProperties.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/IamProperties.java index 9e5066c019..ff57d32a3e 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/IamProperties.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/IamProperties.java @@ -663,6 +663,46 @@ public void setUrnSubnamespaces(String urnSubnamespaces) { } } + public static class LoginLockoutProperties { + + private boolean enabled = false; + private int maxFailedAttempts = 3; + private int lockoutMinutes = 30; + private int maxConcurrentFailures = 2; + + public boolean isEnabled() { + return enabled; + } + + public void setEnabled(boolean enabled) { + this.enabled = enabled; + } + + public int getMaxFailedAttempts() { + return maxFailedAttempts; + } + + public void setMaxFailedAttempts(int maxFailedAttempts) { + this.maxFailedAttempts = maxFailedAttempts; + } + + public int getLockoutMinutes() { + return lockoutMinutes; + } + + public void setLockoutMinutes(int lockoutMinutes) { + this.lockoutMinutes = lockoutMinutes; + } + + public int getMaxConcurrentFailures() { + return maxConcurrentFailures; + } + + public void setMaxConcurrentFailures(int maxConcurrentFailures) { + this.maxConcurrentFailures = maxConcurrentFailures; + } + } + private String host; private String issuer; @@ -727,6 +767,8 @@ public void setUrnSubnamespaces(String urnSubnamespaces) { private AarcProfile aarcProfile = new AarcProfile(); + private LoginLockoutProperties loginLockout = new LoginLockoutProperties(); + public String getBaseUrl() { return baseUrl; } @@ -977,4 +1019,12 @@ public void setAarcProfile(AarcProfile aarcProfile) { this.aarcProfile = aarcProfile; } + public LoginLockoutProperties getLoginLockout() { + return loginLockout; + } + + public void setLoginLockout(LoginLockoutProperties loginLockout) { + this.loginLockout = loginLockout; + } + } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/IamTotpMfaConfig.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/IamTotpMfaConfig.java index f760a760ac..3ad8b103c3 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/IamTotpMfaConfig.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/IamTotpMfaConfig.java @@ -41,6 +41,7 @@ import it.infn.mw.iam.api.account.AccountUtils; import it.infn.mw.iam.api.account.multi_factor_authentication.IamTotpMfaService; import it.infn.mw.iam.service.aup.AUPSignatureCheckService; +import it.infn.mw.iam.authn.lockout.LoginLockoutService; import it.infn.mw.iam.authn.multi_factor_authentication.MultiFactorTotpCheckProvider; import it.infn.mw.iam.authn.multi_factor_authentication.MultiFactorVerificationFilter; import it.infn.mw.iam.authn.multi_factor_authentication.MultiFactorVerificationSuccessHandler; @@ -64,6 +65,9 @@ public class IamTotpMfaConfig { @Autowired private AccountUtils accountUtils; + @Autowired + private LoginLockoutService lockoutService; + /** * Responsible for generating new TOTP secrets * @@ -136,7 +140,7 @@ public AuthenticationFailureHandler failureHandler() { @Bean MultiFactorTotpCheckProvider totpCheckProvider(IamTotpMfaService totpMfaService) { - return new MultiFactorTotpCheckProvider(accountRepo, totpMfaService); + return new MultiFactorTotpCheckProvider(accountRepo, totpMfaService, lockoutService); } } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java index 1ba64efb11..a480128b25 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java @@ -65,6 +65,7 @@ import it.infn.mw.iam.authn.CheckMultiFactorIsEnabledSuccessHandler; import it.infn.mw.iam.authn.ExternalAuthenticationHintService; import it.infn.mw.iam.authn.HintAwareAuthenticationEntryPoint; +import it.infn.mw.iam.authn.lockout.LoginLockoutService; import it.infn.mw.iam.authn.multi_factor_authentication.ExtendedAuthenticationFilter; import it.infn.mw.iam.authn.multi_factor_authentication.ExtendedHttpServletRequestFilter; import it.infn.mw.iam.authn.multi_factor_authentication.MultiFactorVerificationFilter; @@ -147,10 +148,13 @@ public static class UserLoginConfig extends WebSecurityConfigurerAdapter { @Autowired private IamTotpMfaProperties iamTotpMfaProperties; + @Autowired + private LoginLockoutService lockoutService; + @Autowired public void configureGlobal(final AuthenticationManagerBuilder auth) throws Exception { // @formatter:off - auth.authenticationProvider(new IamLocalAuthenticationProvider(iamProperties, iamUserDetailsService, passwordEncoder, accountRepo, iamTotpMfaService, iamTotpMfaProperties)); + auth.authenticationProvider(new IamLocalAuthenticationProvider(iamProperties, iamUserDetailsService, passwordEncoder, accountRepo, iamTotpMfaService, iamTotpMfaProperties, lockoutService)); // @formatter:on } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java index 6adc488feb..4c74283f78 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java @@ -36,6 +36,7 @@ import org.springframework.security.crypto.password.PasswordEncoder; import it.infn.mw.iam.api.account.multi_factor_authentication.IamTotpMfaService; +import it.infn.mw.iam.authn.lockout.LoginLockoutService; import it.infn.mw.iam.authn.multi_factor_authentication.IamAuthenticationMethodReference; import it.infn.mw.iam.authn.util.Authorities; import it.infn.mw.iam.config.IamProperties; @@ -52,6 +53,7 @@ public class IamLocalAuthenticationProvider extends DaoAuthenticationProvider { private final IamAccountRepository accountRepo; private final IamTotpMfaService iamTotpMfaService; private final IamTotpMfaProperties iamTotpMfaProperties; + private final LoginLockoutService lockoutService; private static final Predicate ADMIN_MATCHER = a -> a.getAuthority().equals("ROLE_ADMIN"); @@ -59,13 +61,15 @@ public class IamLocalAuthenticationProvider extends DaoAuthenticationProvider { public IamLocalAuthenticationProvider(IamProperties properties, UserDetailsService uds, PasswordEncoder passwordEncoder, IamAccountRepository accountRepo, - IamTotpMfaService iamTotpMfaService, IamTotpMfaProperties iamTotpMfaProperties) { + IamTotpMfaService iamTotpMfaService, IamTotpMfaProperties iamTotpMfaProperties, + LoginLockoutService lockoutService) { this.allowedUsers = properties.getLocalAuthn().getEnabledFor(); setUserDetailsService(uds); setPasswordEncoder(passwordEncoder); this.accountRepo = accountRepo; this.iamTotpMfaService = iamTotpMfaService; this.iamTotpMfaProperties = iamTotpMfaProperties; + this.lockoutService = lockoutService; } /** @@ -84,14 +88,25 @@ public Authentication authenticate(Authentication authentication) throws Authent isPreAuthenticated = extendedAuthenticationToken.isPreAuthenticated(); } + String username = String.valueOf(authentication.getPrincipal()); + // If not preauthenticated then the first step is to validate the default login credentials. // Therefore, we convert the // authentication to a UsernamePasswordAuthenticationToken and super(authenticate) in the // default manner if (!isPreAuthenticated) { - UsernamePasswordAuthenticationToken userpassToken = new UsernamePasswordAuthenticationToken( - authentication.getPrincipal(), authentication.getCredentials()); - authentication = super.authenticate(userpassToken); + + lockoutService.checkIamAccountLockout(username); + + try { + UsernamePasswordAuthenticationToken userpassToken = + new UsernamePasswordAuthenticationToken( + authentication.getPrincipal(), authentication.getCredentials()); + authentication = super.authenticate(userpassToken); + } catch (BadCredentialsException e) { + lockoutService.recordFailedAttempt(username); + throw e; + } } IamAccount account = accountRepo.findByUsername(authentication.getName()) @@ -120,6 +135,7 @@ public Authentication authenticate(Authentication authentication) throws Authent } // Construct a new authentication object for the PRE_AUTHENTICATED user. + // Don't reset lockout yet -> TOTP verification still pending. token = new ExtendedAuthenticationToken(authentication.getPrincipal(), authentication.getCredentials(), currentAuthorities); token.setAuthenticated(false); @@ -128,7 +144,10 @@ public Authentication authenticate(Authentication authentication) throws Authent token.setDetails(Map.of("acr", ACR_VALUE_MFA)); } else { // MFA is not enabled on this account, construct a new authentication object for the FULLY - // AUTHENTICATED user, granting their normal authorities + // AUTHENTICATED user, granting their normal authorities. + // Full auth clear any lockout state. + lockoutService.resetFailedAttempts(authentication.getName()); + token = new ExtendedAuthenticationToken(authentication.getPrincipal(), authentication.getCredentials(), authentication.getAuthorities()); token.setAuthenticationMethodReferences(refs); diff --git a/iam-login-service/src/main/resources/application.yml b/iam-login-service/src/main/resources/application.yml index 1f97d874fb..78390dbe5e 100644 --- a/iam-login-service/src/main/resources/application.yml +++ b/iam-login-service/src/main/resources/application.yml @@ -189,6 +189,12 @@ iam: client: track-last-used: ${IAM_CLIENT_TRACK_LAST_USED:false} + login-lockout: + enabled: ${IAM_ACCOUNT_LOGIN_LOCKOUT_ENABLED:false} + max-failed-attempts: ${IAM_ACCOUNT_LOGIN_LOCKOUT_MAX_FAILED_ATTEMPTS:3} + lockout-minutes: ${IAM_ACCOUNT_LOGIN_LOCKOUT_MINUTES:30} + max-concurrent-failures: ${IAM_ACCOUNT_LOGIN_LOCKOUT_MAX_CONCURRENT_FAILURES:2} + cache: enabled: ${IAM_CACHE_ENABLED:true} redis: diff --git a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/detail/user.detail.component.html b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/detail/user.detail.component.html index 2568b95aa3..4fa62c4c18 100644 --- a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/detail/user.detail.component.html +++ b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/detail/user.detail.component.html @@ -54,11 +54,14 @@

{{$ctrl.user.name.formatted}}

Status - + Active - - Disabled + + Suspended + + + Disabled @@ -87,6 +90,16 @@

{{$ctrl.user.name.formatted}}

{{$ctrl.user.meta.lastModified | relativeDate }} + + + Suspended Until + + + + {{$ctrl.user.lockoutInfo.suspendedUntil | date:'yyyy-MM-dd HH:mm'}} + + + End time @@ -118,4 +131,4 @@

{{$ctrl.user.name.formatted}}

- \ No newline at end of file + diff --git a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/detail/user.detail.component.js b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/detail/user.detail.component.js index 9933dbd9f3..eaac01d3ce 100644 --- a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/detail/user.detail.component.js +++ b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/detail/user.detail.component.js @@ -62,4 +62,4 @@ }, controller: ['Utils', UserDetailController] }); -})(); \ No newline at end of file +})(); diff --git a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/users/userslist/users.userslist.component.html b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/users/userslist/users.userslist.component.html index feb9b864f2..8f504a5ccd 100644 --- a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/users/userslist/users.userslist.component.html +++ b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/users/userslist/users.userslist.component.html @@ -91,7 +91,11 @@ + + {{user.emails[0].value}} @@ -129,4 +133,4 @@ - \ No newline at end of file + diff --git a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/users/userslist/users.userslist.component.js b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/users/userslist/users.userslist.component.js index 2080c0b57c..78926e4157 100644 --- a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/users/userslist/users.userslist.component.js +++ b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/users/userslist/users.userslist.component.js @@ -112,7 +112,7 @@ } function UsersListController($q, $scope, $rootScope, $uibModal, ModalService, - UsersService, Utils, clipboardService, toaster) { + UsersService, Utils, clipboardService, toaster, $http) { var self = this; @@ -125,6 +125,15 @@ self.totalResults = self.total; self.sortByValue = "name"; self.sortDirection = "asc"; + self.suspendedUuids = []; + + $http.get('/iam/account/lockout/suspended').then(function (r) { + self.suspendedUuids = r.data; + }); + }; + + self.isSuspended = function (user) { + return self.suspendedUuids.indexOf(user.id) !== -1; }; $scope.$on('refreshUsersList', function (e) { @@ -261,6 +270,6 @@ }, templateUrl: '/resources/iam/apps/dashboard-app/components/users/userslist/users.userslist.component.html', controller: ['$q', '$scope', '$rootScope', '$uibModal', 'ModalService', - 'UsersService', 'Utils', 'clipboardService', 'toaster', UsersListController] + 'UsersService', 'Utils', 'clipboardService', 'toaster', '$http', UsersListController] }); -})(); \ No newline at end of file +})(); diff --git a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/services/user.service.js b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/services/user.service.js index ffe07d522e..5bba0f78ec 100644 --- a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/services/user.service.js +++ b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/services/user.service.js @@ -17,9 +17,9 @@ angular.module('dashboardApp').factory('UserService', UserService); -UserService.$inject = ['$q', '$rootScope', 'scimFactory', 'Authorities', 'Utils', 'AupService', 'UsersService', 'GroupsService', 'AuthenticatorAppService']; +UserService.$inject = ['$q', '$rootScope', '$http', 'scimFactory', 'Authorities', 'Utils', 'AupService', 'UsersService', 'GroupsService', 'AuthenticatorAppService']; -function UserService($q, $rootScope, scimFactory, Authorities, Utils, AupService, UsersService, GroupsService, AuthenticatorAppService) { +function UserService($q, $rootScope, $http, scimFactory, Authorities, Utils, AupService, UsersService, GroupsService, AuthenticatorAppService) { var service = { getUser: getUser, getMe: getMe, @@ -61,10 +61,16 @@ function UserService($q, $rootScope, scimFactory, Authorities, Utils, AupService }); } + function getLockoutInfo(userId) { + return $http.get('/iam/account/' + userId + '/lockout') + .then(function (r) { return r.data; }) + .catch(function () { return { suspended: false }; }); + } + function getUser(userId) { return $q .all([scimFactory.getUser(userId), Authorities.getAuthorities(userId), AupService.getAupSignatureForUser(userId), - AuthenticatorAppService.getMfaSettingsForAccount(userId) + AuthenticatorAppService.getMfaSettingsForAccount(userId), getLockoutInfo(userId) ]) .then(function (result) { var user = result[0].data; @@ -82,6 +88,9 @@ function UserService($q, $rootScope, scimFactory, Authorities, Utils, AupService if (result[3] !== null) { user.isMfaActive = result[3]; } + + user.lockoutInfo = result[4]; + return user; }) .catch(function (error) { @@ -122,4 +131,4 @@ function UserService($q, $rootScope, scimFactory, Authorities, Utils, AupService console.error('Error loading logged user info ' + error); }); } -} \ No newline at end of file +} diff --git a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/services/users.service.js b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/services/users.service.js index f7b97fedf2..5de67b1435 100644 --- a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/services/users.service.js +++ b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/services/users.service.js @@ -81,4 +81,4 @@ function UsersService($q, $rootScope, $http, $httpParamSerializer) { 'count': 0 })); } -} \ No newline at end of file +} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/authn/lockout/DefaultLoginLockoutServiceTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/authn/lockout/DefaultLoginLockoutServiceTests.java new file mode 100644 index 0000000000..30aea73fd1 --- /dev/null +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/authn/lockout/DefaultLoginLockoutServiceTests.java @@ -0,0 +1,157 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.test.authn.lockout; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.Date; +import java.util.Optional; + +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.LockedException; + +import it.infn.mw.iam.authn.lockout.DefaultLoginLockoutService; +import it.infn.mw.iam.config.IamProperties; +import it.infn.mw.iam.config.IamProperties.LoginLockoutProperties; +import it.infn.mw.iam.persistence.model.IamAccount; +import it.infn.mw.iam.persistence.model.IamAccountLoginLockout; +import it.infn.mw.iam.persistence.repository.IamAccountRepository; +import it.infn.mw.iam.persistence.repository.IamAccountLoginLockoutRepository; + +@ExtendWith(MockitoExtension.class) +class DefaultLoginLockoutServiceTests { + + private static final String USERNAME = "testuser"; + + @Mock + private IamAccountLoginLockoutRepository lockoutRepo; + + @Mock + private IamAccountRepository accountRepo; + + @Mock + private IamProperties iamProperties; + + private LoginLockoutProperties lockoutProps; + private DefaultLoginLockoutService service; + private IamAccount account; + + @BeforeEach + void setup() { + lockoutProps = new LoginLockoutProperties(); + lockoutProps.setEnabled(true); + lockoutProps.setMaxFailedAttempts(2); + lockoutProps.setLockoutMinutes(30); + lockoutProps.setMaxConcurrentFailures(2); + + when(iamProperties.getLoginLockout()).thenReturn(lockoutProps); + service = new DefaultLoginLockoutService(lockoutRepo, accountRepo, iamProperties); + + account = new IamAccount(); + account.setId(1L); + account.setUsername(USERNAME); + account.setActive(true); + } + + @Test + void doesNothingWhenFeatureDisabled() { + lockoutProps.setEnabled(false); + + assertDoesNotThrow(() -> service.checkIamAccountLockout(USERNAME)); + service.recordFailedAttempt(USERNAME); + service.resetFailedAttempts(USERNAME); + + verify(lockoutRepo, never()).findByAccountUsername(any()); + } + + @Test + void blocksLoginWhenSuspended() { + IamAccountLoginLockout lockout = new IamAccountLoginLockout(account); + lockout.setSuspendedUntil(new Date(System.currentTimeMillis() + 60000)); + when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.of(lockout)); + + assertThrows(LockedException.class, () -> service.checkIamAccountLockout(USERNAME)); + } + + @Test + void successfulLoginDeletesRow() { + IamAccountLoginLockout lockout = new IamAccountLoginLockout(account); + lockout.setFailedAttempts(1); + when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.of(lockout)); + + service.resetFailedAttempts(USERNAME); + verify(lockoutRepo).delete(lockout); + } + + @Test + void fullLifecycleSuspendSuspendDisable() { + // Config: max-failed-attempts=2, max-concurrent-failures=2 + when(accountRepo.findByUsername(USERNAME)).thenReturn(Optional.of(account)); + when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.empty()); + + // ROUND 1: 2 failures -> suspended + service.recordFailedAttempt(USERNAME); + ArgumentCaptor captor = ArgumentCaptor.forClass(IamAccountLoginLockout.class); + verify(lockoutRepo).save(captor.capture()); + IamAccountLoginLockout lockout = captor.getValue(); + + when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.of(lockout)); + service.recordFailedAttempt(USERNAME); + + assertEquals(1, lockout.getLockoutCount()); + assertNotNull(lockout.getSuspendedUntil()); + assertTrue(account.isActive()); + + // Suspension expires + lockout.setSuspendedUntil(new Date(System.currentTimeMillis() - 1000)); + service.checkIamAccountLockout(USERNAME); + assertEquals(0, lockout.getFailedAttempts()); + assertNull(lockout.getSuspendedUntil()); + + // ROUND 2: 2 failures -> suspended again + service.recordFailedAttempt(USERNAME); + service.recordFailedAttempt(USERNAME); + assertEquals(2, lockout.getLockoutCount()); + assertTrue(account.isActive()); + + // Suspension expires again + lockout.setSuspendedUntil(new Date(System.currentTimeMillis() - 1000)); + service.checkIamAccountLockout(USERNAME); + + // ROUND 3: 2 failures -> account disabled, row deleted + service.recordFailedAttempt(USERNAME); + service.recordFailedAttempt(USERNAME); + + assertFalse(account.isActive()); + verify(accountRepo).save(account); + verify(lockoutRepo).delete(lockout); + } +} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/core/IamLocalAuthenticationProviderTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/core/IamLocalAuthenticationProviderTests.java index b416c5c7aa..3801797d74 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/core/IamLocalAuthenticationProviderTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/core/IamLocalAuthenticationProviderTests.java @@ -36,6 +36,7 @@ import org.springframework.security.core.userdetails.UserDetailsService; import org.springframework.security.crypto.password.PasswordEncoder; import it.infn.mw.iam.api.account.multi_factor_authentication.IamTotpMfaService; +import it.infn.mw.iam.authn.lockout.LoginLockoutService; import it.infn.mw.iam.config.IamProperties; import it.infn.mw.iam.config.IamProperties.LocalAuthenticationProperties; import it.infn.mw.iam.config.mfa.IamTotpMfaProperties; @@ -62,6 +63,8 @@ class IamLocalAuthenticationProviderTests { LocalAuthenticationProperties localAuthn; @Mock IamTotpMfaProperties iamTotpMfaProperties; + @Mock + LoginLockoutService lockoutService; IamLocalAuthenticationProvider iamLocalAuthenticationProvider; @@ -69,7 +72,7 @@ class IamLocalAuthenticationProviderTests { void setup() { when(properties.getLocalAuthn()).thenReturn(localAuthn); iamLocalAuthenticationProvider = spy(new IamLocalAuthenticationProvider(properties, uds, passwordEncoder, - accountRepo, iamTotpMfaService, iamTotpMfaProperties)); + accountRepo, iamTotpMfaService, iamTotpMfaProperties, lockoutService)); } private IamAccount newAccount(String username) { diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/MultiFactorTotpCheckProviderTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/MultiFactorTotpCheckProviderTests.java index 664ad4d944..7b97580350 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/MultiFactorTotpCheckProviderTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/MultiFactorTotpCheckProviderTests.java @@ -30,6 +30,7 @@ import org.springframework.security.authentication.BadCredentialsException; import it.infn.mw.iam.api.account.multi_factor_authentication.IamTotpMfaService; +import it.infn.mw.iam.authn.lockout.LoginLockoutService; import it.infn.mw.iam.authn.multi_factor_authentication.MultiFactorTotpCheckProvider; import it.infn.mw.iam.authn.oidc.OidcExternalAuthenticationToken; import it.infn.mw.iam.authn.saml.SamlExternalAuthenticationToken; @@ -48,6 +49,9 @@ class MultiFactorTotpCheckProviderTests extends IamTotpMfaServiceTestSupport { @Mock private IamTotpMfaService totpMfaService; + @Mock + private LoginLockoutService lockoutService; + @Mock private ExtendedAuthenticationToken token; @@ -60,7 +64,7 @@ class MultiFactorTotpCheckProviderTests extends IamTotpMfaServiceTestSupport { @BeforeEach void setup() { MockitoAnnotations.openMocks(this); - multiFactorTotpCheckProvider = new MultiFactorTotpCheckProvider(accountRepo, totpMfaService); + multiFactorTotpCheckProvider = new MultiFactorTotpCheckProvider(accountRepo, totpMfaService, lockoutService); } @Test diff --git a/iam-persistence/src/main/java/it/infn/mw/iam/persistence/model/IamAccountLoginLockout.java b/iam-persistence/src/main/java/it/infn/mw/iam/persistence/model/IamAccountLoginLockout.java new file mode 100644 index 0000000000..44aa7b0b83 --- /dev/null +++ b/iam-persistence/src/main/java/it/infn/mw/iam/persistence/model/IamAccountLoginLockout.java @@ -0,0 +1,146 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.persistence.model; + +import java.io.Serializable; +import java.util.Date; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.JoinColumn; +import javax.persistence.OneToOne; +import javax.persistence.Table; +import javax.persistence.Temporal; +import javax.persistence.TemporalType; + +@Entity +@Table(name = "iam_account_login_lockout") +public class IamAccountLoginLockout implements Serializable { + + private static final long serialVersionUID = 1L; + + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + private Long id; + + @OneToOne + @JoinColumn(name = "account_id", unique = true) + private IamAccount account; + + @Column(name = "failed_attempts", nullable = false) + private int failedAttempts; + + @Temporal(TemporalType.TIMESTAMP) + @Column(name = "first_failure_time") + private Date firstFailureTime; + + @Column(name = "lockout_count", nullable = false) + private int lockoutCount; + + @Temporal(TemporalType.TIMESTAMP) + @Column(name = "suspended_until") + private Date suspendedUntil; + + public IamAccountLoginLockout() { + // empty constructor + } + + public IamAccountLoginLockout(IamAccount account) { + this.account = account; + } + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public IamAccount getAccount() { + return account; + } + + public void setAccount(IamAccount account) { + this.account = account; + } + + public int getFailedAttempts() { + return failedAttempts; + } + + public void setFailedAttempts(int failedAttempts) { + this.failedAttempts = failedAttempts; + } + + public Date getFirstFailureTime() { + return firstFailureTime; + } + + public void setFirstFailureTime(Date firstFailureTime) { + this.firstFailureTime = firstFailureTime; + } + + public int getLockoutCount() { + return lockoutCount; + } + + public void setLockoutCount(int lockoutCount) { + this.lockoutCount = lockoutCount; + } + + public Date getSuspendedUntil() { + return suspendedUntil; + } + + public void setSuspendedUntil(Date suspendedUntil) { + this.suspendedUntil = suspendedUntil; + } + + @Override + public String toString() { + return "IamAccountLoginLockout [id=" + id + ", failedAttempts=" + failedAttempts + ", lockoutCount=" + + lockoutCount + ", suspendedUntil=" + suspendedUntil + "]"; + } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((account == null) ? 0 : account.hashCode()); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + IamAccountLoginLockout other = (IamAccountLoginLockout) obj; + if (account == null) { + if (other.account != null) + return false; + } else if (!account.equals(other.account)) + return false; + return true; + } +} diff --git a/iam-persistence/src/main/java/it/infn/mw/iam/persistence/repository/IamAccountLoginLockoutRepository.java b/iam-persistence/src/main/java/it/infn/mw/iam/persistence/repository/IamAccountLoginLockoutRepository.java new file mode 100644 index 0000000000..221dad4ff6 --- /dev/null +++ b/iam-persistence/src/main/java/it/infn/mw/iam/persistence/repository/IamAccountLoginLockoutRepository.java @@ -0,0 +1,38 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.persistence.repository; + +import java.util.List; +import java.util.Optional; + +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.CrudRepository; + +import it.infn.mw.iam.persistence.model.IamAccountLoginLockout; + +public interface IamAccountLoginLockoutRepository extends CrudRepository { + + Optional findByAccountId(Long id); + + Optional findByAccountUsername(String username); + + Optional findByAccountUuid(String uuid); + + @Query( + "SELECT l.account.uuid FROM IamAccountLoginLockout l WHERE l.suspendedUntil IS NOT NULL AND l.suspendedUntil > CURRENT_TIMESTAMP" + ) + List findAllSuspendedUsers(); +} diff --git a/iam-persistence/src/main/resources/db/migration/h2/V119__login_lockout.sql b/iam-persistence/src/main/resources/db/migration/h2/V119__login_lockout.sql new file mode 100644 index 0000000000..6f7be468c7 --- /dev/null +++ b/iam-persistence/src/main/resources/db/migration/h2/V119__login_lockout.sql @@ -0,0 +1,13 @@ + +CREATE TABLE iam_account_login_lockout ( + ID BIGINT AUTO_INCREMENT NOT NULL, + account_id BIGINT NOT NULL, + failed_attempts INT NOT NULL DEFAULT 0, + first_failure_time TIMESTAMP NULL, + lockout_count INT NOT NULL DEFAULT 0, + suspended_until TIMESTAMP NULL, + PRIMARY KEY (ID), + CONSTRAINT UK_iam_login_lockout_account UNIQUE (account_id) +); + +ALTER TABLE iam_account_login_lockout ADD CONSTRAINT FK_iam_login_lockout_account_id FOREIGN KEY (account_id) REFERENCES iam_account (ID) ON DELETE CASCADE; diff --git a/iam-persistence/src/main/resources/db/migration/mysql/V119__login_lockout.sql b/iam-persistence/src/main/resources/db/migration/mysql/V119__login_lockout.sql new file mode 100644 index 0000000000..6f7be468c7 --- /dev/null +++ b/iam-persistence/src/main/resources/db/migration/mysql/V119__login_lockout.sql @@ -0,0 +1,13 @@ + +CREATE TABLE iam_account_login_lockout ( + ID BIGINT AUTO_INCREMENT NOT NULL, + account_id BIGINT NOT NULL, + failed_attempts INT NOT NULL DEFAULT 0, + first_failure_time TIMESTAMP NULL, + lockout_count INT NOT NULL DEFAULT 0, + suspended_until TIMESTAMP NULL, + PRIMARY KEY (ID), + CONSTRAINT UK_iam_login_lockout_account UNIQUE (account_id) +); + +ALTER TABLE iam_account_login_lockout ADD CONSTRAINT FK_iam_login_lockout_account_id FOREIGN KEY (account_id) REFERENCES iam_account (ID) ON DELETE CASCADE; From 7f97be92118091ceaaf4bf56ac4255e4eee63aad Mon Sep 17 00:00:00 2001 From: Sae126V Date: Wed, 11 Mar 2026 09:40:26 +0000 Subject: [PATCH 2/3] Improve test cases and code coverage --- .../lockout/AccountLockoutController.java | 4 +- .../AccountLockoutControllerTests.java | 101 ++++++++++++++++++ .../DefaultLoginLockoutServiceTests.java | 41 ++++++- .../IamLocalAuthenticationProviderTests.java | 13 +++ .../MultiFactorTotpCheckProviderTests.java | 23 +++- 5 files changed, 178 insertions(+), 4 deletions(-) create mode 100644 iam-login-service/src/test/java/it/infn/mw/iam/test/api/account/lockout/AccountLockoutControllerTests.java diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/account/lockout/AccountLockoutController.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/account/lockout/AccountLockoutController.java index 285fc10a4d..21b627ae6a 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/account/lockout/AccountLockoutController.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/account/lockout/AccountLockoutController.java @@ -39,7 +39,7 @@ public AccountLockoutController(IamAccountLoginLockoutRepository lockoutRepo) { @GetMapping("/iam/account/{uuid}/lockout") @PreAuthorize("#iam.hasScope('iam:admin.read') or #iam.hasDashboardRole('ROLE_ADMIN')") - public ResponseEntity getLockoutStatus(@PathVariable String uuid) { + public ResponseEntity> getLockoutStatus(@PathVariable String uuid) { Optional lockout = lockoutRepo.findByAccountUuid(uuid); if (lockout.isPresent() && lockout.get().getSuspendedUntil() != null @@ -54,7 +54,7 @@ public ResponseEntity getLockoutStatus(@PathVariable String uuid) { @GetMapping("/iam/account/lockout/suspended") @PreAuthorize("#iam.hasScope('iam:admin.read') or #iam.hasDashboardRole('ROLE_ADMIN')") - public ResponseEntity getAllSuspendedUsers() { + public ResponseEntity> getAllSuspendedUsers() { List suspendedUsers = lockoutRepo.findAllSuspendedUsers(); return ResponseEntity.ok(suspendedUsers); } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/account/lockout/AccountLockoutControllerTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/account/lockout/AccountLockoutControllerTests.java new file mode 100644 index 0000000000..e50c8da696 --- /dev/null +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/account/lockout/AccountLockoutControllerTests.java @@ -0,0 +1,101 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.test.api.account.lockout; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.when; + +import java.util.Date; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import org.junit.jupiter.api.BeforeEach; +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.http.ResponseEntity; + +import it.infn.mw.iam.api.account.lockout.AccountLockoutController; +import it.infn.mw.iam.persistence.model.IamAccount; +import it.infn.mw.iam.persistence.model.IamAccountLoginLockout; +import it.infn.mw.iam.persistence.repository.IamAccountLoginLockoutRepository; + +@ExtendWith(MockitoExtension.class) +class AccountLockoutControllerTests { + + @Mock + private IamAccountLoginLockoutRepository lockoutRepo; + + private AccountLockoutController controller; + private IamAccount account; + + @BeforeEach + void setup() { + controller = new AccountLockoutController(lockoutRepo); + account = new IamAccount(); + account.setUuid("uuid-1"); + account.setUsername("testuser"); + } + + @Test + void getLockoutStatusReturnsSuspended() { + IamAccountLoginLockout lockout = new IamAccountLoginLockout(account); + long future = System.currentTimeMillis() + 60_000; + lockout.setSuspendedUntil(new Date(future)); + when(lockoutRepo.findByAccountUuid("uuid-1")).thenReturn(Optional.of(lockout)); + + ResponseEntity> r = controller.getLockoutStatus("uuid-1"); + assertEquals(true, r.getBody().get("suspended")); + assertEquals(future, r.getBody().get("suspendedUntil")); + } + + @Test + void getLockoutStatusReturnsNotSuspendedWhenEmpty() { + when(lockoutRepo.findByAccountUuid("uuid-1")).thenReturn(Optional.empty()); + + ResponseEntity> r = controller.getLockoutStatus("uuid-1"); + assertEquals(false, r.getBody().get("suspended")); + } + + @Test + void getLockoutStatusReturnsNotSuspendedWhenExpired() { + IamAccountLoginLockout lockout = new IamAccountLoginLockout(account); + lockout.setSuspendedUntil(new Date(System.currentTimeMillis() - 1_000)); + when(lockoutRepo.findByAccountUuid("uuid-1")).thenReturn(Optional.of(lockout)); + + ResponseEntity> r = controller.getLockoutStatus("uuid-1"); + assertEquals(false, r.getBody().get("suspended")); + } + + @Test + void getLockoutStatusReturnsNotSuspendedWhenNullSuspendedUntil() { + IamAccountLoginLockout lockout = new IamAccountLoginLockout(account); + when(lockoutRepo.findByAccountUuid("uuid-1")).thenReturn(Optional.of(lockout)); + + ResponseEntity> r = controller.getLockoutStatus("uuid-1"); + assertEquals(false, r.getBody().get("suspended")); + } + + @Test + void getAllSuspendedUsersReturnsUuids() { + when(lockoutRepo.findAllSuspendedUsers()).thenReturn(List.of("uuid-a", "uuid-b")); + + ResponseEntity> r = controller.getAllSuspendedUsers(); + assertEquals(2, r.getBody().size()); + } +} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/authn/lockout/DefaultLoginLockoutServiceTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/authn/lockout/DefaultLoginLockoutServiceTests.java index 30aea73fd1..df65664717 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/authn/lockout/DefaultLoginLockoutServiceTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/authn/lockout/DefaultLoginLockoutServiceTests.java @@ -145,7 +145,6 @@ void fullLifecycleSuspendSuspendDisable() { // Suspension expires again lockout.setSuspendedUntil(new Date(System.currentTimeMillis() - 1000)); service.checkIamAccountLockout(USERNAME); - // ROUND 3: 2 failures -> account disabled, row deleted service.recordFailedAttempt(USERNAME); service.recordFailedAttempt(USERNAME); @@ -154,4 +153,44 @@ void fullLifecycleSuspendSuspendDisable() { verify(accountRepo).save(account); verify(lockoutRepo).delete(lockout); } + + @Test + void checkLockoutNoRecordIsNoop() { + when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.empty()); + assertDoesNotThrow(() -> service.checkIamAccountLockout(USERNAME)); + } + + @Test + void recordFailedAttemptUnknownUserIsNoop() { + when(accountRepo.findByUsername(USERNAME)).thenReturn(Optional.empty()); + service.recordFailedAttempt(USERNAME); + verify(lockoutRepo, never()).save(any()); + } + + @Test + void recordFailedAttemptInactiveAccountIsNoop() { + account.setActive(false); + when(accountRepo.findByUsername(USERNAME)).thenReturn(Optional.of(account)); + service.recordFailedAttempt(USERNAME); + verify(lockoutRepo, never()).save(any()); + } + + @Test + void recordFailedAttemptWhileSuspendedIsNoop() { + IamAccountLoginLockout lockout = new IamAccountLoginLockout(account); + lockout.setSuspendedUntil(new Date(System.currentTimeMillis() + 60_000)); + when(accountRepo.findByUsername(USERNAME)).thenReturn(Optional.of(account)); + when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.of(lockout)); + + service.recordFailedAttempt(USERNAME); + verify(lockoutRepo, never()).save(any()); + } + + @Test + void resetNoRowIsNoop() { + when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.empty()); + service.resetFailedAttempts(USERNAME); + verify(lockoutRepo, never()).delete(any()); + } + } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/core/IamLocalAuthenticationProviderTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/core/IamLocalAuthenticationProviderTests.java index 3801797d74..f7d6e11e69 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/core/IamLocalAuthenticationProviderTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/core/IamLocalAuthenticationProviderTests.java @@ -99,4 +99,17 @@ void testWhenPreAuthenticatedThenAuthenticateSetFalseToAuthenticated() { // Verify that super.authenticate was not called verify(iamLocalAuthenticationProvider, never()).authenticate(any(UsernamePasswordAuthenticationToken.class)); } + + @Test + void resetCalledOnNonMfaSuccess() { + ExtendedAuthenticationToken token = new ExtendedAuthenticationToken("user", "cred"); + token.setPreAuthenticated(true); + IamAccount account = newAccount("user"); + when(accountRepo.findByUsername("user")).thenReturn(Optional.of(account)); + when(iamTotpMfaService.isAuthenticatorAppActive(account)).thenReturn(false); + + iamLocalAuthenticationProvider.authenticate(token); + + verify(lockoutService).resetFailedAttempts("user"); + } } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/MultiFactorTotpCheckProviderTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/MultiFactorTotpCheckProviderTests.java index 7b97580350..95f219c297 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/MultiFactorTotpCheckProviderTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/MultiFactorTotpCheckProviderTests.java @@ -19,6 +19,9 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.Optional; @@ -28,6 +31,7 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.springframework.security.authentication.BadCredentialsException; +import org.springframework.security.authentication.LockedException; import it.infn.mw.iam.api.account.multi_factor_authentication.IamTotpMfaService; import it.infn.mw.iam.authn.lockout.LoginLockoutService; @@ -106,10 +110,13 @@ void authenticateThrowsBadCredentialsExceptionWhenTotpIsInvalid() { assertThrows(BadCredentialsException.class, () -> multiFactorTotpCheckProvider.authenticate(token)); + + verify(lockoutService).recordFailedAttempt("totp"); + verify(lockoutService, never()).resetFailedAttempts(anyString()); } @Test - void authenticateReturnsSuccessfulAuthenticationWhenTotpIsValid() { + void authenticateResetsLockoutWhenTotpIsValid() { IamAccount account = cloneAccount(TOTP_MFA_ACCOUNT); when(token.getName()).thenReturn("totp"); when(token.getTotp()).thenReturn("123456"); @@ -117,6 +124,20 @@ void authenticateReturnsSuccessfulAuthenticationWhenTotpIsValid() { when(totpMfaService.verifyTotp(account, "123456")).thenReturn(true); assertNotNull(multiFactorTotpCheckProvider.authenticate(token)); + + verify(lockoutService).resetFailedAttempts("totp"); + verify(lockoutService, never()).recordFailedAttempt(anyString()); + } + + @Test + void authenticateThrowsLockedExceptionWhenSuspended() { + when(token.getTotp()).thenReturn("123456"); + when(token.getName()).thenReturn("locked"); + doThrow(new LockedException("Suspended")).when(lockoutService).checkIamAccountLockout("locked"); + + assertThrows(LockedException.class, + () -> multiFactorTotpCheckProvider.authenticate(token)); + verify(accountRepo, never()).findByUsername(anyString()); } @Test From 77330599509a1bac6513d7e0316197565a5a9b0d Mon Sep 17 00:00:00 2001 From: Sae126V Date: Mon, 16 Mar 2026 09:32:48 +0000 Subject: [PATCH 3/3] Add code to revoke suspension and an option for admin to configure whether to disable the users --- .../lockout/AccountLockoutController.java | 17 ++- .../lockout/DefaultLoginLockoutService.java | 36 +++-- .../authn/lockout/LoginLockoutService.java | 24 ++- .../it/infn/mw/iam/config/IamProperties.java | 9 ++ .../src/main/resources/application.yml | 1 + .../user/status/user.status.component.html | 13 +- .../user/status/user.status.component.js | 33 ++++- .../dashboard-app/services/user.service.js | 12 +- .../AccountLockoutControllerTests.java | 27 +++- .../DefaultLoginLockoutServiceTests.java | 139 +++++++++++++----- .../IamLocalAuthenticationProviderTests.java | 37 +++-- .../MultiFactorTotpCheckProviderTests.java | 8 +- 12 files changed, 262 insertions(+), 94 deletions(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/account/lockout/AccountLockoutController.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/account/lockout/AccountLockoutController.java index 21b627ae6a..fc60876879 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/account/lockout/AccountLockoutController.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/account/lockout/AccountLockoutController.java @@ -15,16 +15,19 @@ */ package it.infn.mw.iam.api.account.lockout; +import java.time.Instant; import java.util.List; import java.util.Map; import java.util.Optional; import org.springframework.http.ResponseEntity; import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RestController; +import it.infn.mw.iam.authn.lockout.LoginLockoutService; import it.infn.mw.iam.persistence.model.IamAccountLoginLockout; import it.infn.mw.iam.persistence.repository.IamAccountLoginLockoutRepository; @@ -32,9 +35,12 @@ public class AccountLockoutController { private final IamAccountLoginLockoutRepository lockoutRepo; + private final LoginLockoutService lockoutService; - public AccountLockoutController(IamAccountLoginLockoutRepository lockoutRepo) { + public AccountLockoutController(IamAccountLoginLockoutRepository lockoutRepo, + LoginLockoutService lockoutService) { this.lockoutRepo = lockoutRepo; + this.lockoutService = lockoutService; } @GetMapping("/iam/account/{uuid}/lockout") @@ -43,7 +49,7 @@ public ResponseEntity> getLockoutStatus(@PathVariable String Optional lockout = lockoutRepo.findByAccountUuid(uuid); if (lockout.isPresent() && lockout.get().getSuspendedUntil() != null - && System.currentTimeMillis() < lockout.get().getSuspendedUntil().getTime()) { + && Instant.now().isBefore(lockout.get().getSuspendedUntil().toInstant())) { return ResponseEntity.ok(Map.of( "suspended", true, "suspendedUntil", lockout.get().getSuspendedUntil().getTime())); @@ -58,4 +64,11 @@ public ResponseEntity> getAllSuspendedUsers() { List suspendedUsers = lockoutRepo.findAllSuspendedUsers(); return ResponseEntity.ok(suspendedUsers); } + + @DeleteMapping("/iam/account/{uuid}/lockout") + @PreAuthorize("#iam.hasScope('iam:admin.write') or #iam.hasDashboardRole('ROLE_ADMIN')") + public ResponseEntity> revokeLockout(@PathVariable String uuid) { + lockoutService.adminRevokeLockout(uuid); + return ResponseEntity.ok(Map.of("unlocked", true)); + } } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/authn/lockout/DefaultLoginLockoutService.java b/iam-login-service/src/main/java/it/infn/mw/iam/authn/lockout/DefaultLoginLockoutService.java index b036ae4f5c..5c2b871628 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/authn/lockout/DefaultLoginLockoutService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/authn/lockout/DefaultLoginLockoutService.java @@ -15,6 +15,8 @@ */ package it.infn.mw.iam.authn.lockout; +import java.time.Instant; +import java.time.temporal.ChronoUnit; import java.util.Date; import java.util.Optional; @@ -68,15 +70,16 @@ void validateConfiguration() { + "Please provide the suspension duration in minutes."); } - if (properties.getMaxConcurrentFailures() < 1) { + if (properties.isDisableAfterMaxFailures() && properties.getMaxConcurrentFailures() < 1) { throw new IllegalStateException( - "iam.login-lockout.max-concurrent-failures must be >= 1. " + "iam.login-lockout.max-concurrent-failures must be >= 1 when disable-after-max-failures is true. " + "Please provide the maximum number of suspension rounds allowed before the account is permanently disabled."); } LOG.info("Iam Account Login lockout enabled: max-failed-attempts={}, lockout-minutes={}, " - + "max-concurrent-failures={}", properties.getMaxFailedAttempts(), - properties.getLockoutMinutes(), properties.getMaxConcurrentFailures()); + + "max-concurrent-failures={}, disable-after-max-failures={}", + properties.getMaxFailedAttempts(), properties.getLockoutMinutes(), + properties.getMaxConcurrentFailures(), properties.isDisableAfterMaxFailures()); } @Override @@ -139,7 +142,7 @@ public void recordFailedAttempt(String username) { return; } - // if a previous suspension has expired but checkLockout was not called, + // if a previous suspension has expired but checkIamAccountLockout was not called, // reset the counter so we don't carry over stale failedAttempts from the prior round. if (lockout.getSuspendedUntil() != null) { lockout.setFailedAttempts(0); @@ -147,10 +150,10 @@ public void recordFailedAttempt(String username) { lockout.setSuspendedUntil(null); } - Date now = new Date(); + Instant now = Instant.now(); if (lockout.getFailedAttempts() == 0) { - lockout.setFirstFailureTime(now); + lockout.setFirstFailureTime(Date.from(now)); } lockout.setFailedAttempts(lockout.getFailedAttempts() + 1); @@ -162,7 +165,8 @@ public void recordFailedAttempt(String username) { lockout.setLockoutCount(lockout.getLockoutCount() + 1); - if (lockout.getLockoutCount() > properties.getMaxConcurrentFailures()) { + if (properties.isDisableAfterMaxFailures() + && lockout.getLockoutCount() > properties.getMaxConcurrentFailures()) { // All suspension rounds exhausted; disable the account and clean up account.setActive(false); accountRepo.save(account); @@ -173,8 +177,8 @@ public void recordFailedAttempt(String username) { } // Suspend for the configured duration - long suspendUntilMs = now.getTime() + ((long) properties.getLockoutMinutes() * 60 * 1000); - lockout.setSuspendedUntil(new Date(suspendUntilMs)); + Instant suspendUntil = now.plus(properties.getLockoutMinutes(), ChronoUnit.MINUTES); + lockout.setSuspendedUntil(Date.from(suspendUntil)); LOG.warn("Account '{}' suspended until {} (round {} of {})", username, lockout.getSuspendedUntil(), lockout.getLockoutCount(), @@ -198,8 +202,18 @@ public void resetFailedAttempts(String username) { }); } + @Override + @Transactional + public void adminRevokeLockout(String accountUuid) { + + lockoutRepo.findByAccountUuid(accountUuid).ifPresent(lockout -> { + lockoutRepo.delete(lockout); + LOG.info("Admin revoked suspension for account '{}'", lockout.getAccount().getUsername()); + }); + } + private boolean isSuspended(IamAccountLoginLockout lockout) { return lockout.getSuspendedUntil() != null - && System.currentTimeMillis() < lockout.getSuspendedUntil().getTime(); + && Instant.now().isBefore(lockout.getSuspendedUntil().toInstant()); } } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/authn/lockout/LoginLockoutService.java b/iam-login-service/src/main/java/it/infn/mw/iam/authn/lockout/LoginLockoutService.java index eeecdc4d5f..15e7eae6c2 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/authn/lockout/LoginLockoutService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/authn/lockout/LoginLockoutService.java @@ -16,15 +16,17 @@ package it.infn.mw.iam.authn.lockout; /** - * Tracks failed login attempts and enforces temporary suspensions and permanent account disabling. + * Tracks failed login attempts and enforces temporary suspensions + * and permanent account disabling, if `disable-after-max-failures` enabled. * * Password failures and TOTP failures share the same counter. The lifecycle: * - * User fails {@code max-failed-attempts} times => suspended for {@code lockout-minutes} - * Suspension expires => counter resets, user gets another round of attempts - * After {@code max-concurrent-failures} suspension rounds, the next round of failures - * disables the account ({@code active = false}) and the lockout row is deleted - * An admin can re-enable the account since the lockout row is gone, the user starts fresh + * User fails {max-failed-attempts} times => suspended for {lockout-minutes}. + * Suspension expires => counter resets, user gets another round of attempts. + * If {disable-after-max-failures} is true: + * after {max-concurrent-failures} suspension rounds the account is disabled. + * If false (default): the account is never disabled, only repeatedly suspended. + * An admin can clear a lockout at any time. */ public interface LoginLockoutService { @@ -37,8 +39,8 @@ public interface LoginLockoutService { /** * Records a single failed attempt (password or TOTP). When the attempt count reaches the - * threshold the account is suspended. When all suspension rounds are exhausted the account - * is disabled and the lockout row is deleted. + * threshold the account is suspended. When all suspension rounds are exhausted and + * disable-after-max-failures is true, the account is disabled and the lockout row is deleted. */ void recordFailedAttempt(String username); @@ -47,4 +49,10 @@ public interface LoginLockoutService { * authentication (password-only login, or TOTP verification). */ void resetFailedAttempts(String username); + + /** + * Admin-triggered unlock. Deletes the lockout row for the given account, + * clearing any active suspension. + */ + void adminRevokeLockout(String accountUuid); } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/IamProperties.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/IamProperties.java index ff57d32a3e..bf3419c553 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/IamProperties.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/IamProperties.java @@ -669,6 +669,7 @@ public static class LoginLockoutProperties { private int maxFailedAttempts = 3; private int lockoutMinutes = 30; private int maxConcurrentFailures = 2; + private boolean disableAfterMaxFailures = true; public boolean isEnabled() { return enabled; @@ -701,6 +702,14 @@ public int getMaxConcurrentFailures() { public void setMaxConcurrentFailures(int maxConcurrentFailures) { this.maxConcurrentFailures = maxConcurrentFailures; } + + public boolean isDisableAfterMaxFailures() { + return disableAfterMaxFailures; + } + + public void setDisableAfterMaxFailures(boolean disableAfterMaxFailures) { + this.disableAfterMaxFailures = disableAfterMaxFailures; + } } private String host; diff --git a/iam-login-service/src/main/resources/application.yml b/iam-login-service/src/main/resources/application.yml index 78390dbe5e..1b1d54e64d 100644 --- a/iam-login-service/src/main/resources/application.yml +++ b/iam-login-service/src/main/resources/application.yml @@ -191,6 +191,7 @@ iam: login-lockout: enabled: ${IAM_ACCOUNT_LOGIN_LOCKOUT_ENABLED:false} + disable-after-max-failures: ${IAM_ACCOUNT_LOGIN_LOCKOUT_DISABLE_AFTER_MAX_FAILURES:false} max-failed-attempts: ${IAM_ACCOUNT_LOGIN_LOCKOUT_MAX_FAILED_ATTEMPTS:3} lockout-minutes: ${IAM_ACCOUNT_LOGIN_LOCKOUT_MINUTES:30} max-concurrent-failures: ${IAM_ACCOUNT_LOGIN_LOCKOUT_MAX_CONCURRENT_FAILURES:2} diff --git a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/status/user.status.component.html b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/status/user.status.component.html index bee991c087..cf62d4ee8f 100644 --- a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/status/user.status.component.html +++ b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/status/user.status.component.html @@ -35,4 +35,15 @@ name="enable-user-btn"> Restore User - \ No newline at end of file + + + + diff --git a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/status/user.status.component.js b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/status/user.status.component.js index 5e86a75457..e7bf36e9b6 100644 --- a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/status/user.status.component.js +++ b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/status/user.status.component.js @@ -16,7 +16,7 @@ (function() { 'use strict'; - function UserStatusController(toaster, Utils, ModalService, scimFactory) { + function UserStatusController(toaster, Utils, ModalService, scimFactory, $http) { var self = this; self.$onInit = function() { @@ -58,6 +58,20 @@ .catch(self.handleError); }; + self.unlockUser = function() { + self.enabled = false; + $http.delete('/iam/account/' + self.user.id + '/lockout') + .then(function() { + self.enabled = true; + self.userCtrl.loadUser().then(function(user) { + toaster.pop({ + type: 'success', + body: `User '${user.name.formatted}' has been unlocked.` + }); + }); + }) + .catch(self.handleError); + }; self.openDialog = function() { @@ -93,6 +107,19 @@ }); }; + self.openUnlockDialog = function() { + var modalOptions = { + closeButtonText: 'Cancel', + actionButtonText: 'Unlock user', + headerText: 'Unlock ' + self.user.name.formatted, + bodyText: + `Are you sure you want to unlock user '${self.user.name.formatted}'? This will clear the lockout and allow them to log in again.` + }; + + ModalService.showModal({}, modalOptions) + .then(function() { self.unlockUser(); }) + .catch(function() {}); + }; self.isMe = function() { return Utils.isMe(self.user.id); }; } @@ -103,8 +130,8 @@ templateUrl: '/resources/iam/apps/dashboard-app/components/user/status/user.status.component.html', controller: [ - 'toaster', 'Utils', 'ModalService', 'scimFactory', UserStatusController + 'toaster', 'Utils', 'ModalService', 'scimFactory', '$http', UserStatusController ] }); -})(); \ No newline at end of file +})(); diff --git a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/services/user.service.js b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/services/user.service.js index 5bba0f78ec..a93ac17c87 100644 --- a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/services/user.service.js +++ b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/services/user.service.js @@ -54,6 +54,7 @@ function UserService($q, $rootScope, $http, scimFactory, Authorities, Utils, Aup if (result[2] !== null) { user.isMfaActive = result[2]; } + user.lockoutInfo = { suspended: false }; return user; }).catch(function (error) { console.error('Error loading authenticated user information: ', error); @@ -61,16 +62,11 @@ function UserService($q, $rootScope, $http, scimFactory, Authorities, Utils, Aup }); } - function getLockoutInfo(userId) { - return $http.get('/iam/account/' + userId + '/lockout') - .then(function (r) { return r.data; }) - .catch(function () { return { suspended: false }; }); - } - function getUser(userId) { return $q .all([scimFactory.getUser(userId), Authorities.getAuthorities(userId), AupService.getAupSignatureForUser(userId), - AuthenticatorAppService.getMfaSettingsForAccount(userId), getLockoutInfo(userId) + AuthenticatorAppService.getMfaSettingsForAccount(userId), + $http.get('/iam/account/' + userId + '/lockout').catch(function () { return { data: { suspended: false } }; }) ]) .then(function (result) { var user = result[0].data; @@ -89,7 +85,7 @@ function UserService($q, $rootScope, $http, scimFactory, Authorities, Utils, Aup user.isMfaActive = result[3]; } - user.lockoutInfo = result[4]; + user.lockoutInfo = result[4].data; return user; }) diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/account/lockout/AccountLockoutControllerTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/account/lockout/AccountLockoutControllerTests.java index e50c8da696..ff3e5afecb 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/account/lockout/AccountLockoutControllerTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/account/lockout/AccountLockoutControllerTests.java @@ -16,8 +16,10 @@ package it.infn.mw.iam.test.api.account.lockout; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.time.Instant; import java.util.Date; import java.util.List; import java.util.Map; @@ -31,22 +33,27 @@ import org.springframework.http.ResponseEntity; import it.infn.mw.iam.api.account.lockout.AccountLockoutController; +import it.infn.mw.iam.authn.lockout.LoginLockoutService; import it.infn.mw.iam.persistence.model.IamAccount; import it.infn.mw.iam.persistence.model.IamAccountLoginLockout; import it.infn.mw.iam.persistence.repository.IamAccountLoginLockoutRepository; @ExtendWith(MockitoExtension.class) +@SuppressWarnings("unchecked") class AccountLockoutControllerTests { @Mock private IamAccountLoginLockoutRepository lockoutRepo; + @Mock + private LoginLockoutService lockoutService; + private AccountLockoutController controller; private IamAccount account; @BeforeEach void setup() { - controller = new AccountLockoutController(lockoutRepo); + controller = new AccountLockoutController(lockoutRepo, lockoutService); account = new IamAccount(); account.setUuid("uuid-1"); account.setUsername("testuser"); @@ -55,13 +62,13 @@ void setup() { @Test void getLockoutStatusReturnsSuspended() { IamAccountLoginLockout lockout = new IamAccountLoginLockout(account); - long future = System.currentTimeMillis() + 60_000; - lockout.setSuspendedUntil(new Date(future)); + Instant future = Instant.now().plusSeconds(60); + lockout.setSuspendedUntil(Date.from(future)); when(lockoutRepo.findByAccountUuid("uuid-1")).thenReturn(Optional.of(lockout)); ResponseEntity> r = controller.getLockoutStatus("uuid-1"); assertEquals(true, r.getBody().get("suspended")); - assertEquals(future, r.getBody().get("suspendedUntil")); + assertEquals(future.toEpochMilli(), r.getBody().get("suspendedUntil")); } @Test @@ -75,7 +82,7 @@ void getLockoutStatusReturnsNotSuspendedWhenEmpty() { @Test void getLockoutStatusReturnsNotSuspendedWhenExpired() { IamAccountLoginLockout lockout = new IamAccountLoginLockout(account); - lockout.setSuspendedUntil(new Date(System.currentTimeMillis() - 1_000)); + lockout.setSuspendedUntil(Date.from(Instant.now().minusSeconds(1))); when(lockoutRepo.findByAccountUuid("uuid-1")).thenReturn(Optional.of(lockout)); ResponseEntity> r = controller.getLockoutStatus("uuid-1"); @@ -93,9 +100,17 @@ void getLockoutStatusReturnsNotSuspendedWhenNullSuspendedUntil() { @Test void getAllSuspendedUsersReturnsUuids() { - when(lockoutRepo.findAllSuspendedUsers()).thenReturn(List.of("uuid-a", "uuid-b")); + when(lockoutRepo.findAllSuspendedUsers()).thenReturn(List.of("a", "b")); ResponseEntity> r = controller.getAllSuspendedUsers(); assertEquals(2, r.getBody().size()); } + + @Test + void revokeLockoutDelegatesToService() { + ResponseEntity> r = controller.revokeLockout("uuid-1"); + + verify(lockoutService).adminRevokeLockout("uuid-1"); + assertEquals(true, r.getBody().get("unlocked")); + } } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/authn/lockout/DefaultLoginLockoutServiceTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/authn/lockout/DefaultLoginLockoutServiceTests.java index df65664717..269c068f5b 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/authn/lockout/DefaultLoginLockoutServiceTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/authn/lockout/DefaultLoginLockoutServiceTests.java @@ -27,6 +27,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.time.Instant; import java.util.Date; import java.util.Optional; @@ -50,6 +51,7 @@ class DefaultLoginLockoutServiceTests { private static final String USERNAME = "testuser"; + private static final String UUID = "test-uuid-1234"; @Mock private IamAccountLoginLockoutRepository lockoutRepo; @@ -71,6 +73,7 @@ void setup() { lockoutProps.setMaxFailedAttempts(2); lockoutProps.setLockoutMinutes(30); lockoutProps.setMaxConcurrentFailures(2); + lockoutProps.setDisableAfterMaxFailures(true); when(iamProperties.getLoginLockout()).thenReturn(lockoutProps); service = new DefaultLoginLockoutService(lockoutRepo, accountRepo, iamProperties); @@ -78,6 +81,7 @@ void setup() { account = new IamAccount(); account.setId(1L); account.setUsername(USERNAME); + account.setUuid(UUID); account.setActive(true); } @@ -92,19 +96,64 @@ void doesNothingWhenFeatureDisabled() { verify(lockoutRepo, never()).findByAccountUsername(any()); } + @Test + void checkLockoutNoRecordIsNoop() { + when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.empty()); + assertDoesNotThrow(() -> service.checkIamAccountLockout(USERNAME)); + } + @Test void blocksLoginWhenSuspended() { IamAccountLoginLockout lockout = new IamAccountLoginLockout(account); - lockout.setSuspendedUntil(new Date(System.currentTimeMillis() + 60000)); + lockout.setSuspendedUntil(Date.from(Instant.now().plusSeconds(60))); when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.of(lockout)); assertThrows(LockedException.class, () -> service.checkIamAccountLockout(USERNAME)); } + @Test + void checkLockoutResetsExpiredSuspension() { + IamAccountLoginLockout lockout = new IamAccountLoginLockout(account); + lockout.setFailedAttempts(2); + lockout.setFirstFailureTime(Date.from(Instant.now().minusSeconds(120))); + lockout.setSuspendedUntil(Date.from(Instant.now().minusSeconds(1))); + when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.of(lockout)); + + assertDoesNotThrow(() -> service.checkIamAccountLockout(USERNAME)); + assertEquals(0, lockout.getFailedAttempts()); + assertNull(lockout.getSuspendedUntil()); + verify(lockoutRepo).save(lockout); + } + + @Test + void recordFailedAttemptUnknownUserIsNoop() { + when(accountRepo.findByUsername(USERNAME)).thenReturn(Optional.empty()); + service.recordFailedAttempt(USERNAME); + verify(lockoutRepo, never()).save(any()); + } + + @Test + void recordFailedAttemptInactiveAccountIsNoop() { + account.setActive(false); + when(accountRepo.findByUsername(USERNAME)).thenReturn(Optional.of(account)); + service.recordFailedAttempt(USERNAME); + verify(lockoutRepo, never()).save(any()); + } + + @Test + void recordFailedAttemptWhileSuspendedIsNoop() { + IamAccountLoginLockout lockout = new IamAccountLoginLockout(account); + lockout.setSuspendedUntil(Date.from(Instant.now().plusSeconds(60))); + when(accountRepo.findByUsername(USERNAME)).thenReturn(Optional.of(account)); + when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.of(lockout)); + + service.recordFailedAttempt(USERNAME); + verify(lockoutRepo, never()).save(any()); + } + @Test void successfulLoginDeletesRow() { IamAccountLoginLockout lockout = new IamAccountLoginLockout(account); - lockout.setFailedAttempts(1); when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.of(lockout)); service.resetFailedAttempts(USERNAME); @@ -112,12 +161,18 @@ void successfulLoginDeletesRow() { } @Test - void fullLifecycleSuspendSuspendDisable() { - // Config: max-failed-attempts=2, max-concurrent-failures=2 + void resetNoRowIsNoop() { + when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.empty()); + service.resetFailedAttempts(USERNAME); + verify(lockoutRepo, never()).delete(any()); + } + + @Test + void fullLifecycleDisablesAfterMaxRounds() { when(accountRepo.findByUsername(USERNAME)).thenReturn(Optional.of(account)); when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.empty()); - // ROUND 1: 2 failures -> suspended + // ROUND 1: 2 failures => suspended service.recordFailedAttempt(USERNAME); ArgumentCaptor captor = ArgumentCaptor.forClass(IamAccountLoginLockout.class); verify(lockoutRepo).save(captor.capture()); @@ -125,27 +180,25 @@ void fullLifecycleSuspendSuspendDisable() { when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.of(lockout)); service.recordFailedAttempt(USERNAME); - assertEquals(1, lockout.getLockoutCount()); assertNotNull(lockout.getSuspendedUntil()); assertTrue(account.isActive()); // Suspension expires - lockout.setSuspendedUntil(new Date(System.currentTimeMillis() - 1000)); + lockout.setSuspendedUntil(Date.from(Instant.now().minusSeconds(1))); service.checkIamAccountLockout(USERNAME); assertEquals(0, lockout.getFailedAttempts()); - assertNull(lockout.getSuspendedUntil()); - // ROUND 2: 2 failures -> suspended again + // ROUND 2 service.recordFailedAttempt(USERNAME); service.recordFailedAttempt(USERNAME); assertEquals(2, lockout.getLockoutCount()); - assertTrue(account.isActive()); - // Suspension expires again - lockout.setSuspendedUntil(new Date(System.currentTimeMillis() - 1000)); + // Suspension expires + lockout.setSuspendedUntil(Date.from(Instant.now().minusSeconds(1))); service.checkIamAccountLockout(USERNAME); - // ROUND 3: 2 failures -> account disabled, row deleted + + // ROUND 3: exceeds max-concurrent-failures=2 => disabled service.recordFailedAttempt(USERNAME); service.recordFailedAttempt(USERNAME); @@ -155,42 +208,56 @@ void fullLifecycleSuspendSuspendDisable() { } @Test - void checkLockoutNoRecordIsNoop() { + void keepsSuspendingWhenDisableAfterMaxFailuresIsFalse() { + lockoutProps.setDisableAfterMaxFailures(false); + + when(accountRepo.findByUsername(USERNAME)).thenReturn(Optional.of(account)); when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.empty()); - assertDoesNotThrow(() -> service.checkIamAccountLockout(USERNAME)); - } - @Test - void recordFailedAttemptUnknownUserIsNoop() { - when(accountRepo.findByUsername(USERNAME)).thenReturn(Optional.empty()); + // ROUND 1 service.recordFailedAttempt(USERNAME); - verify(lockoutRepo, never()).save(any()); - } + ArgumentCaptor captor = ArgumentCaptor.forClass(IamAccountLoginLockout.class); + verify(lockoutRepo).save(captor.capture()); + IamAccountLoginLockout lockout = captor.getValue(); - @Test - void recordFailedAttemptInactiveAccountIsNoop() { - account.setActive(false); - when(accountRepo.findByUsername(USERNAME)).thenReturn(Optional.of(account)); + when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.of(lockout)); service.recordFailedAttempt(USERNAME); - verify(lockoutRepo, never()).save(any()); + assertEquals(1, lockout.getLockoutCount()); + assertNotNull(lockout.getSuspendedUntil()); + + // Expire + ROUND 2 + lockout.setSuspendedUntil(Date.from(Instant.now().minusSeconds(1))); + service.checkIamAccountLockout(USERNAME); + service.recordFailedAttempt(USERNAME); + service.recordFailedAttempt(USERNAME); + assertEquals(2, lockout.getLockoutCount()); + + // Expire + ROUND 3: would disable with default, but now just suspends again + lockout.setSuspendedUntil(Date.from(Instant.now().minusSeconds(1))); + service.checkIamAccountLockout(USERNAME); + service.recordFailedAttempt(USERNAME); + service.recordFailedAttempt(USERNAME); + + assertEquals(3, lockout.getLockoutCount()); + assertNotNull(lockout.getSuspendedUntil()); + assertTrue(account.isActive()); + verify(accountRepo, never()).save(account); } @Test - void recordFailedAttemptWhileSuspendedIsNoop() { + void adminRevokeLockoutDeletesRow() { IamAccountLoginLockout lockout = new IamAccountLoginLockout(account); - lockout.setSuspendedUntil(new Date(System.currentTimeMillis() + 60_000)); - when(accountRepo.findByUsername(USERNAME)).thenReturn(Optional.of(account)); - when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.of(lockout)); + lockout.setSuspendedUntil(Date.from(Instant.now().plusSeconds(60))); + when(lockoutRepo.findByAccountUuid(UUID)).thenReturn(Optional.of(lockout)); - service.recordFailedAttempt(USERNAME); - verify(lockoutRepo, never()).save(any()); + service.adminRevokeLockout(UUID); + verify(lockoutRepo).delete(lockout); } @Test - void resetNoRowIsNoop() { - when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.empty()); - service.resetFailedAttempts(USERNAME); + void adminRevokeLockoutNoRowPresent() { + when(lockoutRepo.findByAccountUuid(UUID)).thenReturn(Optional.empty()); + service.adminRevokeLockout(UUID); verify(lockoutRepo, never()).delete(any()); } - } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/core/IamLocalAuthenticationProviderTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/core/IamLocalAuthenticationProviderTests.java index f7d6e11e69..23af3d48a7 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/core/IamLocalAuthenticationProviderTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/core/IamLocalAuthenticationProviderTests.java @@ -17,8 +17,10 @@ package it.infn.mw.iam.test.core; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; @@ -32,6 +34,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.authentication.LockedException; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.userdetails.UserDetailsService; import org.springframework.security.crypto.password.PasswordEncoder; @@ -49,22 +52,14 @@ @ExtendWith(MockitoExtension.class) class IamLocalAuthenticationProviderTests { - @Mock - IamProperties properties; - @Mock - UserDetailsService uds; - @Mock - PasswordEncoder passwordEncoder; - @Mock - IamAccountRepository accountRepo; - @Mock - IamTotpMfaService iamTotpMfaService; - @Mock - LocalAuthenticationProperties localAuthn; - @Mock - IamTotpMfaProperties iamTotpMfaProperties; - @Mock - LoginLockoutService lockoutService; + @Mock IamProperties properties; + @Mock UserDetailsService uds; + @Mock PasswordEncoder passwordEncoder; + @Mock IamAccountRepository accountRepo; + @Mock IamTotpMfaService iamTotpMfaService; + @Mock LocalAuthenticationProperties localAuthn; + @Mock IamTotpMfaProperties iamTotpMfaProperties; + @Mock LoginLockoutService lockoutService; IamLocalAuthenticationProvider iamLocalAuthenticationProvider; @@ -100,6 +95,16 @@ void testWhenPreAuthenticatedThenAuthenticateSetFalseToAuthenticated() { verify(iamLocalAuthenticationProvider, never()).authenticate(any(UsernamePasswordAuthenticationToken.class)); } + @Test + void lockedExceptionBlocksAuthentication() { + // Not pre-authenticated; enters the if(!isPreAuthenticated) block where checkIamAccountLockout lives + ExtendedAuthenticationToken token = new ExtendedAuthenticationToken("locked", "cred"); + doThrow(new LockedException("Suspended")).when(lockoutService).checkIamAccountLockout("locked"); + + assertThrows(LockedException.class, () -> iamLocalAuthenticationProvider.authenticate(token)); + verify(accountRepo, never()).findByUsername(anyString()); + } + @Test void resetCalledOnNonMfaSuccess() { ExtendedAuthenticationToken token = new ExtendedAuthenticationToken("user", "cred"); diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/MultiFactorTotpCheckProviderTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/MultiFactorTotpCheckProviderTests.java index 95f219c297..f7a090008f 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/MultiFactorTotpCheckProviderTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/MultiFactorTotpCheckProviderTests.java @@ -75,16 +75,18 @@ void setup() { void authenticateReturnsNullWhenTotpIsNull() { when(token.getTotp()).thenReturn(null); assertNull(multiFactorTotpCheckProvider.authenticate(token)); + verify(lockoutService, never()).checkIamAccountLockout(anyString()); } @Test - void authenticateThrowsBadCredentialsExceptionWhenAccountNotFound() { + void authenticateChecksLockoutBeforeAccountLookup() { when(token.getTotp()).thenReturn("123456"); when(token.getName()).thenReturn("username"); when(accountRepo.findByUsername("username")).thenReturn(Optional.empty()); assertThrows(BadCredentialsException.class, () -> multiFactorTotpCheckProvider.authenticate(token)); + verify(lockoutService).checkIamAccountLockout("username"); } @Test @@ -141,7 +143,7 @@ void authenticateThrowsLockedExceptionWhenSuspended() { } @Test - void authenticateWithOidcTokenReturnsSuccessfulAuthenticationWhenTotpIsValid() { + void authenticateWithOidcTokenSucceeds() { IamAccount account = cloneAccount(TOTP_MFA_ACCOUNT); when(oidcToken.getName()).thenReturn("totp"); when(oidcToken.getTotp()).thenReturn("123456"); @@ -152,7 +154,7 @@ void authenticateWithOidcTokenReturnsSuccessfulAuthenticationWhenTotpIsValid() { } @Test - void authenticateWithSamlTokenReturnsSuccessfulAuthenticationWhenTotpIsValid() { + void authenticateWithSamlTokenSucceeds() { IamAccount account = cloneAccount(TOTP_MFA_ACCOUNT); when(samlToken.getName()).thenReturn("totp"); when(samlToken.getTotp()).thenReturn("123456");