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..fc60876879 --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/account/lockout/AccountLockoutController.java @@ -0,0 +1,74 @@ +/** + * 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.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; + +@RestController +public class AccountLockoutController { + + private final IamAccountLoginLockoutRepository lockoutRepo; + private final LoginLockoutService lockoutService; + + public AccountLockoutController(IamAccountLoginLockoutRepository lockoutRepo, + LoginLockoutService lockoutService) { + this.lockoutRepo = lockoutRepo; + this.lockoutService = lockoutService; + } + + @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 + && Instant.now().isBefore(lockout.get().getSuspendedUntil().toInstant())) { + 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); + } + + @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 new file mode 100644 index 0000000000..5c2b871628 --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/authn/lockout/DefaultLoginLockoutService.java @@ -0,0 +1,219 @@ +/** + * 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.time.Instant; +import java.time.temporal.ChronoUnit; +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.isDisableAfterMaxFailures() && properties.getMaxConcurrentFailures() < 1) { + throw new IllegalStateException( + "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={}, disable-after-max-failures={}", + properties.getMaxFailedAttempts(), properties.getLockoutMinutes(), + properties.getMaxConcurrentFailures(), properties.isDisableAfterMaxFailures()); + } + + @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 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); + lockout.setFirstFailureTime(null); + lockout.setSuspendedUntil(null); + } + + Instant now = Instant.now(); + + if (lockout.getFailedAttempts() == 0) { + lockout.setFirstFailureTime(Date.from(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 (properties.isDisableAfterMaxFailures() + && 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 + 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(), + 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); + }); + } + + @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 + && 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 new file mode 100644 index 0000000000..15e7eae6c2 --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/authn/lockout/LoginLockoutService.java @@ -0,0 +1,58 @@ +/** + * 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, if `disable-after-max-failures` enabled. + * + * Password failures and TOTP failures share the same counter. The lifecycle: + * + * 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 { + + /** + * 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 and + * disable-after-max-failures is true, 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); + + /** + * 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/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..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 @@ -663,6 +663,55 @@ 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; + private boolean disableAfterMaxFailures = true; + + 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; + } + + public boolean isDisableAfterMaxFailures() { + return disableAfterMaxFailures; + } + + public void setDisableAfterMaxFailures(boolean disableAfterMaxFailures) { + this.disableAfterMaxFailures = disableAfterMaxFailures; + } + } + private String host; private String issuer; @@ -727,6 +776,8 @@ public void setUrnSubnamespaces(String urnSubnamespaces) { private AarcProfile aarcProfile = new AarcProfile(); + private LoginLockoutProperties loginLockout = new LoginLockoutProperties(); + public String getBaseUrl() { return baseUrl; } @@ -977,4 +1028,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..1b1d54e64d 100644 --- a/iam-login-service/src/main/resources/application.yml +++ b/iam-login-service/src/main/resources/application.yml @@ -189,6 +189,13 @@ iam: client: track-last-used: ${IAM_CLIENT_TRACK_LAST_USED:false} + 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} + 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/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/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..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 @@ -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, @@ -54,6 +54,7 @@ function UserService($q, $rootScope, scimFactory, Authorities, Utils, AupService 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); @@ -64,7 +65,8 @@ function UserService($q, $rootScope, scimFactory, Authorities, Utils, AupService function getUser(userId) { return $q .all([scimFactory.getUser(userId), Authorities.getAuthorities(userId), AupService.getAupSignatureForUser(userId), - AuthenticatorAppService.getMfaSettingsForAccount(userId) + AuthenticatorAppService.getMfaSettingsForAccount(userId), + $http.get('/iam/account/' + userId + '/lockout').catch(function () { return { data: { suspended: false } }; }) ]) .then(function (result) { var user = result[0].data; @@ -82,6 +84,9 @@ function UserService($q, $rootScope, scimFactory, Authorities, Utils, AupService if (result[3] !== null) { user.isMfaActive = result[3]; } + + user.lockoutInfo = result[4].data; + return user; }) .catch(function (error) { @@ -122,4 +127,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/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..ff3e5afecb --- /dev/null +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/account/lockout/AccountLockoutControllerTests.java @@ -0,0 +1,116 @@ +/** + * 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.verify; +import static org.mockito.Mockito.when; + +import java.time.Instant; +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.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, lockoutService); + account = new IamAccount(); + account.setUuid("uuid-1"); + account.setUsername("testuser"); + } + + @Test + void getLockoutStatusReturnsSuspended() { + IamAccountLoginLockout lockout = new IamAccountLoginLockout(account); + 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.toEpochMilli(), 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(Date.from(Instant.now().minusSeconds(1))); + 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("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 new file mode 100644 index 0000000000..269c068f5b --- /dev/null +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/authn/lockout/DefaultLoginLockoutServiceTests.java @@ -0,0 +1,263 @@ +/** + * 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.time.Instant; +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"; + private static final String UUID = "test-uuid-1234"; + + @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); + lockoutProps.setDisableAfterMaxFailures(true); + + when(iamProperties.getLoginLockout()).thenReturn(lockoutProps); + service = new DefaultLoginLockoutService(lockoutRepo, accountRepo, iamProperties); + + account = new IamAccount(); + account.setId(1L); + account.setUsername(USERNAME); + account.setUuid(UUID); + 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 checkLockoutNoRecordIsNoop() { + when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.empty()); + assertDoesNotThrow(() -> service.checkIamAccountLockout(USERNAME)); + } + + @Test + void blocksLoginWhenSuspended() { + IamAccountLoginLockout lockout = new IamAccountLoginLockout(account); + 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); + when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.of(lockout)); + + service.resetFailedAttempts(USERNAME); + verify(lockoutRepo).delete(lockout); + } + + @Test + 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 + 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(Date.from(Instant.now().minusSeconds(1))); + service.checkIamAccountLockout(USERNAME); + assertEquals(0, lockout.getFailedAttempts()); + + // ROUND 2 + service.recordFailedAttempt(USERNAME); + service.recordFailedAttempt(USERNAME); + assertEquals(2, lockout.getLockoutCount()); + + // Suspension expires + lockout.setSuspendedUntil(Date.from(Instant.now().minusSeconds(1))); + service.checkIamAccountLockout(USERNAME); + + // ROUND 3: exceeds max-concurrent-failures=2 => disabled + service.recordFailedAttempt(USERNAME); + service.recordFailedAttempt(USERNAME); + + assertFalse(account.isActive()); + verify(accountRepo).save(account); + verify(lockoutRepo).delete(lockout); + } + + @Test + void keepsSuspendingWhenDisableAfterMaxFailuresIsFalse() { + lockoutProps.setDisableAfterMaxFailures(false); + + when(accountRepo.findByUsername(USERNAME)).thenReturn(Optional.of(account)); + when(lockoutRepo.findByAccountUsername(USERNAME)).thenReturn(Optional.empty()); + + // ROUND 1 + 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()); + + // 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 adminRevokeLockoutDeletesRow() { + IamAccountLoginLockout lockout = new IamAccountLoginLockout(account); + lockout.setSuspendedUntil(Date.from(Instant.now().plusSeconds(60))); + when(lockoutRepo.findByAccountUuid(UUID)).thenReturn(Optional.of(lockout)); + + service.adminRevokeLockout(UUID); + verify(lockoutRepo).delete(lockout); + } + + @Test + 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 b416c5c7aa..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,10 +34,12 @@ 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; 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; @@ -48,20 +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 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; @@ -69,7 +67,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) { @@ -96,4 +94,27 @@ void testWhenPreAuthenticatedThenAuthenticateSetFalseToAuthenticated() { // Verify that super.authenticate was not called 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"); + 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 664ad4d944..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 @@ -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,8 +31,10 @@ 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; 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 +53,9 @@ class MultiFactorTotpCheckProviderTests extends IamTotpMfaServiceTestSupport { @Mock private IamTotpMfaService totpMfaService; + @Mock + private LoginLockoutService lockoutService; + @Mock private ExtendedAuthenticationToken token; @@ -60,23 +68,25 @@ class MultiFactorTotpCheckProviderTests extends IamTotpMfaServiceTestSupport { @BeforeEach void setup() { MockitoAnnotations.openMocks(this); - multiFactorTotpCheckProvider = new MultiFactorTotpCheckProvider(accountRepo, totpMfaService); + multiFactorTotpCheckProvider = new MultiFactorTotpCheckProvider(accountRepo, totpMfaService, lockoutService); } @Test 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 @@ -102,10 +112,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"); @@ -113,10 +126,24 @@ 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 - void authenticateWithOidcTokenReturnsSuccessfulAuthenticationWhenTotpIsValid() { + void authenticateWithOidcTokenSucceeds() { IamAccount account = cloneAccount(TOTP_MFA_ACCOUNT); when(oidcToken.getName()).thenReturn("totp"); when(oidcToken.getTotp()).thenReturn("123456"); @@ -127,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"); 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;