From eeb7dc23382770017847158ea76b5925695ac4fb Mon Sep 17 00:00:00 2001 From: Viacheslav Klimov Date: Thu, 26 May 2022 16:13:24 +0300 Subject: [PATCH] Improvements for 2FA --- .../server/controller/TwoFaConfigController.java | 4 ++-- .../auth/mfa/config/DefaultTwoFaConfigManager.java | 12 ++++++++++-- .../security/auth/mfa/config/TwoFaConfigManager.java | 2 +- .../rest/RestAwareAuthenticationSuccessHandler.java | 4 +++- .../system/DefaultSystemSecurityService.java | 7 ++++--- .../security/model/mfa/PlatformTwoFaSettings.java | 2 +- .../mfa/provider/BackupCodeTwoFaProviderConfig.java | 2 +- 7 files changed, 22 insertions(+), 11 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/controller/TwoFaConfigController.java b/application/src/main/java/org/thingsboard/server/controller/TwoFaConfigController.java index 138b20b160..60239f3603 100644 --- a/application/src/main/java/org/thingsboard/server/controller/TwoFaConfigController.java +++ b/application/src/main/java/org/thingsboard/server/controller/TwoFaConfigController.java @@ -258,9 +258,9 @@ public class TwoFaConfigController extends BaseController { ControllerConstants.SYSTEM_OR_TENANT_AUTHORITY_PARAGRAPH) @PostMapping("/settings") @PreAuthorize("hasAnyAuthority('SYS_ADMIN')") - public void savePlatformTwoFaSettings(@ApiParam(value = "Settings value", required = true) + public PlatformTwoFaSettings savePlatformTwoFaSettings(@ApiParam(value = "Settings value", required = true) @RequestBody PlatformTwoFaSettings twoFaSettings) throws ThingsboardException { - twoFaConfigManager.savePlatformTwoFaSettings(getTenantId(), twoFaSettings); + return twoFaConfigManager.savePlatformTwoFaSettings(getTenantId(), twoFaSettings); } diff --git a/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/config/DefaultTwoFaConfigManager.java b/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/config/DefaultTwoFaConfigManager.java index bfe426d532..23e6dc065f 100644 --- a/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/config/DefaultTwoFaConfigManager.java +++ b/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/config/DefaultTwoFaConfigManager.java @@ -99,6 +99,9 @@ public class DefaultTwoFaConfigManager implements TwoFaConfigManager { return newSettings; }); Map configs = settings.getConfigs(); + if (configs.isEmpty() && accountConfig.getProviderType() == TwoFaProviderType.BACKUP_CODE) { + throw new IllegalArgumentException("To use 2FA backup codes you first need to configure at least one provider"); + } if (accountConfig.isUseByDefault()) { configs.values().forEach(config -> config.setUseByDefault(false)); } @@ -114,7 +117,11 @@ public class DefaultTwoFaConfigManager implements TwoFaConfigManager { AccountTwoFaSettings settings = getAccountTwoFaSettings(tenantId, userId) .orElseThrow(() -> new IllegalArgumentException("2FA not configured")); settings.getConfigs().remove(providerType); - if (!settings.getConfigs().isEmpty() && settings.getConfigs().values().stream().noneMatch(TwoFaAccountConfig::isUseByDefault)) { + if (settings.getConfigs().size() == 1) { + settings.getConfigs().remove(TwoFaProviderType.BACKUP_CODE); + } + if (!settings.getConfigs().isEmpty() && settings.getConfigs().values().stream() + .noneMatch(TwoFaAccountConfig::isUseByDefault)) { settings.getConfigs().values().stream() .min(Comparator.comparing(TwoFaAccountConfig::getProviderType)) .ifPresent(config -> config.setUseByDefault(true)); @@ -135,7 +142,7 @@ public class DefaultTwoFaConfigManager implements TwoFaConfigManager { } @Override - public void savePlatformTwoFaSettings(TenantId tenantId, PlatformTwoFaSettings twoFactorAuthSettings) throws ThingsboardException { + public PlatformTwoFaSettings savePlatformTwoFaSettings(TenantId tenantId, PlatformTwoFaSettings twoFactorAuthSettings) throws ThingsboardException { ConstraintValidator.validateFields(twoFactorAuthSettings); for (TwoFaProviderConfig providerConfig : twoFactorAuthSettings.getProviders()) { twoFactorAuthService.checkProvider(tenantId, providerConfig.getProviderType()); @@ -149,6 +156,7 @@ public class DefaultTwoFaConfigManager implements TwoFaConfigManager { }); settings.setJsonValue(JacksonUtil.valueToTree(twoFactorAuthSettings)); adminSettingsService.saveAdminSettings(tenantId, settings); + return twoFactorAuthSettings; } @Override diff --git a/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/config/TwoFaConfigManager.java b/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/config/TwoFaConfigManager.java index 0fe33b7757..c0e3200a4d 100644 --- a/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/config/TwoFaConfigManager.java +++ b/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/config/TwoFaConfigManager.java @@ -39,7 +39,7 @@ public interface TwoFaConfigManager { Optional getPlatformTwoFaSettings(TenantId tenantId, boolean sysadminSettingsAsDefault); - void savePlatformTwoFaSettings(TenantId tenantId, PlatformTwoFaSettings twoFactorAuthSettings) throws ThingsboardException; + PlatformTwoFaSettings savePlatformTwoFaSettings(TenantId tenantId, PlatformTwoFaSettings twoFactorAuthSettings) throws ThingsboardException; void deletePlatformTwoFaSettings(TenantId tenantId); diff --git a/application/src/main/java/org/thingsboard/server/service/security/auth/rest/RestAwareAuthenticationSuccessHandler.java b/application/src/main/java/org/thingsboard/server/service/security/auth/rest/RestAwareAuthenticationSuccessHandler.java index 4ab4069b61..b4f0b293d3 100644 --- a/application/src/main/java/org/thingsboard/server/service/security/auth/rest/RestAwareAuthenticationSuccessHandler.java +++ b/application/src/main/java/org/thingsboard/server/service/security/auth/rest/RestAwareAuthenticationSuccessHandler.java @@ -55,7 +55,9 @@ public class RestAwareAuthenticationSuccessHandler implements AuthenticationSucc if (authentication instanceof MfaAuthenticationToken) { int preVerificationTokenLifetime = twoFaConfigManager.getPlatformTwoFaSettings(securityUser.getTenantId(), true) - .flatMap(settings -> Optional.ofNullable(settings.getTotalAllowedTimeForVerification())).orElse((int) TimeUnit.MINUTES.toSeconds(30)); + .flatMap(settings -> Optional.ofNullable(settings.getTotalAllowedTimeForVerification()) + .filter(time -> time > 0)) + .orElse((int) TimeUnit.MINUTES.toSeconds(30)); tokenPair.setToken(tokenFactory.createPreVerificationToken(securityUser, preVerificationTokenLifetime).getToken()); tokenPair.setRefreshToken(null); tokenPair.setScope(Authority.PRE_VERIFICATION_TOKEN); 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 57d009070c..65b89c85b8 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 @@ -172,11 +172,12 @@ public class DefaultSystemSecurityService implements SystemSecurityService { return; } - if (twoFaSettings.getMaxVerificationFailuresBeforeUserLockout() > 0 - && failedVerificationAttempts >= twoFaSettings.getMaxVerificationFailuresBeforeUserLockout()) { + Integer maxVerificationFailures = twoFaSettings.getMaxVerificationFailuresBeforeUserLockout(); + if (maxVerificationFailures != null && maxVerificationFailures > 0 + && failedVerificationAttempts >= maxVerificationFailures) { userService.setUserCredentialsEnabled(TenantId.SYS_TENANT_ID, userId, false); SecuritySettings securitySettings = self.getSecuritySettings(tenantId); - lockAccount(userId, securityUser.getEmail(), securitySettings.getUserLockoutNotificationEmail(), twoFaSettings.getMaxVerificationFailuresBeforeUserLockout()); + lockAccount(userId, securityUser.getEmail(), securitySettings.getUserLockoutNotificationEmail(), maxVerificationFailures); throw new LockedException("User account was locked due to exceeded 2FA verification attempts"); } } diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/security/model/mfa/PlatformTwoFaSettings.java b/common/data/src/main/java/org/thingsboard/server/common/data/security/model/mfa/PlatformTwoFaSettings.java index 9d581c0867..930b858316 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/security/model/mfa/PlatformTwoFaSettings.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/security/model/mfa/PlatformTwoFaSettings.java @@ -37,7 +37,7 @@ public class PlatformTwoFaSettings { @Pattern(regexp = "[1-9]\\d*:[1-9]\\d*", message = "verification code check rate limit configuration is invalid") private String verificationCodeCheckRateLimit; @Min(value = 0, message = "maximum number of verification failure before user lockout must be positive") - private int maxVerificationFailuresBeforeUserLockout; + private Integer maxVerificationFailuresBeforeUserLockout; @Min(value = 1, message = "total amount of time allotted for verification must be greater than 0") private Integer totalAllowedTimeForVerification; diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/security/model/mfa/provider/BackupCodeTwoFaProviderConfig.java b/common/data/src/main/java/org/thingsboard/server/common/data/security/model/mfa/provider/BackupCodeTwoFaProviderConfig.java index 9dc2a00b23..92def57ee4 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/security/model/mfa/provider/BackupCodeTwoFaProviderConfig.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/security/model/mfa/provider/BackupCodeTwoFaProviderConfig.java @@ -22,7 +22,7 @@ import javax.validation.constraints.Min; @Data public class BackupCodeTwoFaProviderConfig implements TwoFaProviderConfig { - @Min(0) + @Min(1) private int codesQuantity; @Override