diff --git a/application/src/main/java/org/thingsboard/server/controller/TwoFactorAuthController.java b/application/src/main/java/org/thingsboard/server/controller/TwoFactorAuthController.java index 125febeb6a..1b1983cdf8 100644 --- a/application/src/main/java/org/thingsboard/server/controller/TwoFactorAuthController.java +++ b/application/src/main/java/org/thingsboard/server/controller/TwoFactorAuthController.java @@ -26,11 +26,14 @@ import org.thingsboard.server.common.data.audit.ActionType; import org.thingsboard.server.common.data.exception.ThingsboardErrorCode; import org.thingsboard.server.common.data.exception.ThingsboardException; import org.thingsboard.server.service.security.auth.mfa.TwoFactorAuthService; +import org.thingsboard.server.service.security.auth.rest.RestAuthenticationDetails; import org.thingsboard.server.service.security.model.JwtTokenPair; import org.thingsboard.server.service.security.model.SecurityUser; import org.thingsboard.server.service.security.model.token.JwtTokenFactory; import org.thingsboard.server.service.security.system.SystemSecurityService; +import javax.servlet.http.HttpServletRequest; + /* * TODO [viacheslav]: * - Tests for 2FA @@ -64,16 +67,15 @@ public class TwoFactorAuthController extends BaseController { @PostMapping("/verification/check") @PreAuthorize("hasAuthority('PRE_VERIFICATION_TOKEN')") - public JwtTokenPair checkTwoFaVerificationCode(@RequestParam String verificationCode, Authentication authentication) throws Exception { + public JwtTokenPair checkTwoFaVerificationCode(@RequestParam String verificationCode, HttpServletRequest servletRequest) throws Exception { SecurityUser user = getCurrentUser(); boolean verificationSuccess = twoFactorAuthService.checkVerificationCode(user, verificationCode, true); if (verificationSuccess) { - systemSecurityService.logLoginAction(user, authentication, ActionType.LOGIN, null); + systemSecurityService.logLoginAction(user, new RestAuthenticationDetails(servletRequest), ActionType.LOGIN, null); return tokenFactory.createTokenPair(user); } else { ThingsboardException error = new ThingsboardException("Verification code is incorrect", ThingsboardErrorCode.AUTHENTICATION); - // FIXME [viacheslav]: log login action: no authentication details - systemSecurityService.logLoginAction(user, authentication, ActionType.LOGIN, error); + systemSecurityService.logLoginAction(user, new RestAuthenticationDetails(servletRequest), ActionType.LOGIN, error); throw error; } } diff --git a/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/config/TwoFactorAuthSettings.java b/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/config/TwoFactorAuthSettings.java index 0866aafbe1..ae39065e74 100644 --- a/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/config/TwoFactorAuthSettings.java +++ b/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/config/TwoFactorAuthSettings.java @@ -41,7 +41,7 @@ public class TwoFactorAuthSettings { private String verificationCodeCheckRateLimit; @ApiModelProperty(example = "10") @Min(0) - private int maxCodeVerificationFailuresBeforeUserLockout; + private int maxVerificationFailuresBeforeUserLockout; @ApiModelProperty(value = "in minutes", example = "60") @Min(1) private int totalAllowedTimeForVerification; diff --git a/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/config/account/TotpTwoFactorAuthAccountConfig.java b/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/config/account/TotpTwoFactorAuthAccountConfig.java index 78ab4cf80b..93975d49ad 100644 --- a/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/config/account/TotpTwoFactorAuthAccountConfig.java +++ b/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/config/account/TotpTwoFactorAuthAccountConfig.java @@ -25,7 +25,7 @@ import javax.validation.constraints.Pattern; public class TotpTwoFactorAuthAccountConfig implements TwoFactorAuthAccountConfig { @NotBlank -// @Pattern(regexp = ) // TODO [viacheslav]: validate otp auth url by pattern +// @Pattern(regexp = "otpauth://totp/") // FIXME [viacheslav]: validate otp auth url by pattern private String authUrl; @Override diff --git a/application/src/main/java/org/thingsboard/server/service/security/auth/rest/RestAuthenticationProvider.java b/application/src/main/java/org/thingsboard/server/service/security/auth/rest/RestAuthenticationProvider.java index 8429c70506..71d16d46d0 100644 --- a/application/src/main/java/org/thingsboard/server/service/security/auth/rest/RestAuthenticationProvider.java +++ b/application/src/main/java/org/thingsboard/server/service/security/auth/rest/RestAuthenticationProvider.java @@ -85,7 +85,7 @@ public class RestAuthenticationProvider implements AuthenticationProvider { if (twoFactorAuthConfigManager.isTwoFaEnabled(securityUser)) { return new MfaAuthenticationToken(securityUser); } else { - systemSecurityService.logLoginAction((User) authentication.getPrincipal(), authentication, ActionType.LOGIN, null); + systemSecurityService.logLoginAction((User) authentication.getPrincipal(), authentication.getDetails(), ActionType.LOGIN, null); } } else { String publicId = userPrincipal.getValue(); @@ -111,7 +111,7 @@ public class RestAuthenticationProvider implements AuthenticationProvider { try { systemSecurityService.validateUserCredentials(user.getTenantId(), userCredentials, username, password); } catch (LockedException e) { - systemSecurityService.logLoginAction(user, authentication, ActionType.LOCKOUT, null); + systemSecurityService.logLoginAction(user, authentication.getDetails(), ActionType.LOCKOUT, null); throw e; } @@ -120,7 +120,7 @@ public class RestAuthenticationProvider implements AuthenticationProvider { return new SecurityUser(user, userCredentials.isEnabled(), userPrincipal); } catch (Exception e) { - systemSecurityService.logLoginAction(user, authentication, ActionType.LOGIN, e); + systemSecurityService.logLoginAction(user, authentication.getDetails(), ActionType.LOGIN, e); throw e; } } diff --git a/application/src/main/java/org/thingsboard/server/service/security/system/DefaultSystemSecurityService.java b/application/src/main/java/org/thingsboard/server/service/security/system/DefaultSystemSecurityService.java index 6e6815ff5e..1f19b9c19b 100644 --- a/application/src/main/java/org/thingsboard/server/service/security/system/DefaultSystemSecurityService.java +++ b/application/src/main/java/org/thingsboard/server/service/security/system/DefaultSystemSecurityService.java @@ -34,7 +34,6 @@ import org.springframework.cache.annotation.Cacheable; import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.authentication.DisabledException; import org.springframework.security.authentication.LockedException; -import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; import org.springframework.stereotype.Service; @@ -136,7 +135,7 @@ public class DefaultSystemSecurityService implements SystemSecurityService { SecuritySettings securitySettings = self.getSecuritySettings(tenantId); if (securitySettings.getMaxFailedLoginAttempts() != null && securitySettings.getMaxFailedLoginAttempts() > 0) { if (failedLoginAttempts > securitySettings.getMaxFailedLoginAttempts() && userCredentials.isEnabled()) { - lockAccount(userCredentials.getUserId(), username, securitySettings); + lockAccount(userCredentials.getUserId(), username, securitySettings.getUserLockoutNotificationEmail(), securitySettings.getMaxFailedLoginAttempts()); throw new LockedException("Authentication Failed. Username was locked due to security policy."); } } @@ -172,21 +171,22 @@ public class DefaultSystemSecurityService implements SystemSecurityService { userService.resetFailedLoginAttempts(tenantId, userId); } - if (twoFaSettings.getMaxCodeVerificationFailuresBeforeUserLockout() > 0 - && failedVerificationAttempts >= twoFaSettings.getMaxCodeVerificationFailuresBeforeUserLockout()) { + if (twoFaSettings.getMaxVerificationFailuresBeforeUserLockout() > 0 + && failedVerificationAttempts >= twoFaSettings.getMaxVerificationFailuresBeforeUserLockout()) { userService.setUserCredentialsEnabled(TenantId.SYS_TENANT_ID, userId, false); - lockAccount(userId, securityUser.getEmail(), self.getSecuritySettings(tenantId)); + SecuritySettings securitySettings = self.getSecuritySettings(tenantId); + lockAccount(userId, securityUser.getEmail(), securitySettings.getUserLockoutNotificationEmail(), twoFaSettings.getMaxVerificationFailuresBeforeUserLockout()); throw new LockedException("User account was locked due to exceeded 2FA verification attempts"); } } - private void lockAccount(UserId userId, String username, SecuritySettings securitySettings) { + private void lockAccount(UserId userId, String username, String userLockoutNotificationEmail, Integer maxFailedLoginAttempts) { userService.setUserCredentialsEnabled(TenantId.SYS_TENANT_ID, userId, false); - if (StringUtils.isNoneBlank(securitySettings.getUserLockoutNotificationEmail())) { + if (StringUtils.isNotBlank(userLockoutNotificationEmail)) { try { - mailService.sendAccountLockoutEmail(username, securitySettings.getUserLockoutNotificationEmail(), securitySettings.getMaxFailedLoginAttempts()); + mailService.sendAccountLockoutEmail(username, userLockoutNotificationEmail, maxFailedLoginAttempts); } catch (ThingsboardException e) { - log.warn("Can't send email regarding user account [{}] lockout to provided email [{}]", username, securitySettings.getUserLockoutNotificationEmail(), e); + log.warn("Can't send email regarding user account [{}] lockout to provided email [{}]", username, userLockoutNotificationEmail, e); } } } @@ -257,23 +257,22 @@ public class DefaultSystemSecurityService implements SystemSecurityService { } @Override - public void logLoginAction(User user, Authentication authentication, ActionType actionType, Exception e) { + public void logLoginAction(User user, Object authenticationDetails, ActionType actionType, Exception e) { String clientAddress = "Unknown"; String browser = "Unknown"; String os = "Unknown"; String device = "Unknown"; - if (authentication != null && authentication.getDetails() != null) { - if (authentication.getDetails() instanceof RestAuthenticationDetails) { - RestAuthenticationDetails details = (RestAuthenticationDetails) authentication.getDetails(); - clientAddress = details.getClientAddress(); - if (details.getUserAgent() != null) { - Client userAgent = details.getUserAgent(); - if (userAgent.userAgent != null) { - browser = userAgent.userAgent.family; - if (userAgent.userAgent.major != null) { - browser += " " + userAgent.userAgent.major; - if (userAgent.userAgent.minor != null) { - browser += "." + userAgent.userAgent.minor; + if (authenticationDetails instanceof RestAuthenticationDetails) { + RestAuthenticationDetails details = (RestAuthenticationDetails) authenticationDetails; + clientAddress = details.getClientAddress(); + if (details.getUserAgent() != null) { + Client userAgent = details.getUserAgent(); + if (userAgent.userAgent != null) { + browser = userAgent.userAgent.family; + if (userAgent.userAgent.major != null) { + browser += " " + userAgent.userAgent.major; + if (userAgent.userAgent.minor != null) { + browser += "." + userAgent.userAgent.minor; if (userAgent.userAgent.patch != null) { browser += "." + userAgent.userAgent.patch; } @@ -300,7 +299,6 @@ public class DefaultSystemSecurityService implements SystemSecurityService { } } } - } if (actionType == ActionType.LOGIN && e == null) { userService.setLastLoginTs(user.getTenantId(), user.getId()); } diff --git a/application/src/main/java/org/thingsboard/server/service/security/system/SystemSecurityService.java b/application/src/main/java/org/thingsboard/server/service/security/system/SystemSecurityService.java index f6b401da4e..4c14e45ae6 100644 --- a/application/src/main/java/org/thingsboard/server/service/security/system/SystemSecurityService.java +++ b/application/src/main/java/org/thingsboard/server/service/security/system/SystemSecurityService.java @@ -15,7 +15,6 @@ */ package org.thingsboard.server.service.security.system; -import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; import org.thingsboard.server.common.data.User; import org.thingsboard.server.common.data.audit.ActionType; @@ -43,6 +42,6 @@ public interface SystemSecurityService { String getBaseUrl(TenantId tenantId, CustomerId customerId, HttpServletRequest httpServletRequest); - void logLoginAction(User user, Authentication authentication, ActionType actionType, Exception e); + void logLoginAction(User user, Object authenticationDetails, ActionType actionType, Exception e); } diff --git a/application/src/main/resources/templates/account.lockout.ftl b/application/src/main/resources/templates/account.lockout.ftl index 9832f8323d..fa21653cb9 100644 --- a/application/src/main/resources/templates/account.lockout.ftl +++ b/application/src/main/resources/templates/account.lockout.ftl @@ -88,7 +88,7 @@ background-color: #f6f6f6; - Thingsboard user account ${lockoutAccount} has been lockout due to failed credentials were provided more than ${maxFailedLoginAttempts} times. + Thingsboard user account ${lockoutAccount} has been locked out due to multiple authentication failures (more than ${maxFailedLoginAttempts}). diff --git a/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthTest.java b/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthTest.java index b338d0d99d..69bd49f6f5 100644 --- a/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthTest.java @@ -44,8 +44,8 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.verify; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; -// TODO [viacheslav]: test sessionId // TODO [viacheslav]: test validation for all account configs, provider configs and two factor auth settings +// TODO [viacheslav]: test authentication details, log login action, last login ts, rate limiting, user blocking, etc public abstract class TwoFactorAuthTest extends AbstractControllerTest { @SpyBean