2FA: log login action, fix user lockout

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, new RestAuthenticationDetails(servletRequest), ActionType.LOGIN, error);
-            systemSecurityService.logLoginAction(user, authentication, ActionType.LOGIN, error);
             throw error;
         }
     }

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;

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

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;
         }
     }

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
+        if (twoFaSettings.getMaxVerificationFailuresBeforeUserLockout() > 0
-                && failedVerificationAttempts >= twoFaSettings.getMaxCodeVerificationFailuresBeforeUserLockout()) {
+                && 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 (authenticationDetails instanceof RestAuthenticationDetails) {
-            if (authentication.getDetails() instanceof RestAuthenticationDetails) {
+            RestAuthenticationDetails details = (RestAuthenticationDetails) authenticationDetails;
-                RestAuthenticationDetails details = (RestAuthenticationDetails) authentication.getDetails();
+            clientAddress = details.getClientAddress();
-                clientAddress = details.getClientAddress();
+            if (details.getUserAgent() != null) {
-                if (details.getUserAgent() != null) {
+                Client userAgent = details.getUserAgent();
-                    Client userAgent = details.getUserAgent();
+                if (userAgent.userAgent != null) {
-                    if (userAgent.userAgent != null) {
+                    browser = userAgent.userAgent.family;
-                        browser = userAgent.userAgent.family;
+                    if (userAgent.userAgent.major != null) {
-                        if (userAgent.userAgent.major != null) {
+                        browser += " " + userAgent.userAgent.major;
-                            browser += " " + userAgent.userAgent.major;
+                        if (userAgent.userAgent.minor != null) {
-                            if (userAgent.userAgent.minor != null) {
+                            browser += "." + userAgent.userAgent.minor;
-                                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());
         }

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);
 
 }

application/src/main/resources/templates/account.lockout.ftl

@@ -88,7 +88,7 @@ background-color: #f6f6f6;
 								</tr>
 								<tr style="font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif; box-sizing: border-box; font-size: 14px; margin: 0;">
 									<td class="content-block" style="font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif; box-sizing: border-box; font-size: 14px; vertical-align: top; margin: 0; padding: 0 0 20px;" valign="top">
-										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}).
 									</td>
 								</tr>
 								<tr style="font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif; box-sizing: border-box; font-size: 14px; margin: 0;">

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