From d3c23c914d2a29a1b27fdfde8b7632aa3d1e312c Mon Sep 17 00:00:00 2001 From: Viacheslav Klimov Date: Mon, 9 May 2022 11:45:21 +0300 Subject: [PATCH] Ensure at least one 2FA account config is default --- .../controller/TwoFaConfigController.java | 17 +++-------------- .../mfa/config/DefaultTwoFaConfigManager.java | 17 +++++++++++++---- .../auth/mfa/config/TwoFaConfigManager.java | 1 - .../controller/TwoFactorAuthConfigTest.java | 3 +++ 4 files changed, 19 insertions(+), 19 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 60541b1ce4..d974189bab 100644 --- a/application/src/main/java/org/thingsboard/server/controller/TwoFaConfigController.java +++ b/application/src/main/java/org/thingsboard/server/controller/TwoFaConfigController.java @@ -50,7 +50,6 @@ import javax.servlet.http.HttpServletResponse; import javax.validation.Valid; import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.stream.Collectors; import static org.thingsboard.server.controller.ControllerConstants.NEW_LINE; @@ -191,20 +190,10 @@ public class TwoFaConfigController extends BaseController { @RequestBody TwoFaAccountConfigUpdateRequest updateRequest) throws ThingsboardException { SecurityUser user = getCurrentUser(); - AccountTwoFaSettings settings = twoFaConfigManager.getAccountTwoFaSettings(user.getTenantId(), user.getId()) - .orElseThrow(() -> new IllegalArgumentException("No 2FA config found")); - Map configs = settings.getConfigs(); - - TwoFaAccountConfig accountConfig; - if ((accountConfig = configs.get(providerType)) == null) { - throw new IllegalArgumentException("Config for " + providerType + " 2FA provider not found"); - } - if (updateRequest.isUseByDefault()) { - configs.values().forEach(config -> config.setUseByDefault(false)); - } + TwoFaAccountConfig accountConfig = twoFaConfigManager.getTwoFaAccountConfig(user.getTenantId(), user.getId(), providerType) + .orElseThrow(() -> new IllegalArgumentException("Config for " + providerType + " 2FA provider not found")); accountConfig.setUseByDefault(updateRequest.isUseByDefault()); - - return twoFaConfigManager.saveAccountTwoFaSettings(user.getTenantId(), user.getId(), settings); + return twoFaConfigManager.saveTwoFaAccountConfig(user.getTenantId(), user.getId(), accountConfig); } @ApiOperation(value = "Delete 2FA account config (deleteTwoFaAccountConfig)", notes = 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 75ca7b1e45..a1c56e4cd7 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 @@ -38,11 +38,10 @@ import org.thingsboard.server.dao.settings.AdminSettingsService; import org.thingsboard.server.dao.user.UserAuthSettingsDao; import java.util.Collections; +import java.util.Comparator; import java.util.LinkedHashMap; -import java.util.Map; import java.util.Optional; import java.util.concurrent.ExecutionException; -import java.util.function.Consumer; @Service @RequiredArgsConstructor @@ -68,8 +67,7 @@ public class DefaultTwoFaConfigManager implements TwoFaConfigManager { }); } - @Override - public AccountTwoFaSettings saveAccountTwoFaSettings(TenantId tenantId, UserId userId, AccountTwoFaSettings settings) { + protected AccountTwoFaSettings saveAccountTwoFaSettings(TenantId tenantId, UserId userId, AccountTwoFaSettings settings) { UserAuthSettings userAuthSettings = Optional.ofNullable(userAuthSettingsDao.findByUserId(userId)) .orElseGet(() -> { UserAuthSettings newUserAuthSettings = new UserAuthSettings(); @@ -99,7 +97,13 @@ public class DefaultTwoFaConfigManager implements TwoFaConfigManager { newSettings.setConfigs(new LinkedHashMap<>()); return newSettings; }); + if (accountConfig.isUseByDefault()) { + settings.getConfigs().values().forEach(config -> config.setUseByDefault(false)); + } settings.getConfigs().put(accountConfig.getProviderType(), accountConfig); + if (settings.getConfigs().values().stream().noneMatch(TwoFaAccountConfig::isUseByDefault)) { + settings.getConfigs().values().stream().findFirst().ifPresent(config -> config.setUseByDefault(true)); + } return saveAccountTwoFaSettings(tenantId, userId, settings); } @@ -108,6 +112,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)) { + settings.getConfigs().values().stream() + .min(Comparator.comparing(TwoFaAccountConfig::getProviderType)) + .ifPresent(config -> config.setUseByDefault(true)); + } return saveAccountTwoFaSettings(tenantId, userId, settings); } 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 f1a4590572..212c4697bf 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 @@ -28,7 +28,6 @@ public interface TwoFaConfigManager { Optional getAccountTwoFaSettings(TenantId tenantId, UserId userId); - AccountTwoFaSettings saveAccountTwoFaSettings(TenantId tenantId, UserId userId, AccountTwoFaSettings settings); Optional getTwoFaAccountConfig(TenantId tenantId, UserId userId, TwoFaProviderType providerType); diff --git a/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthConfigTest.java b/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthConfigTest.java index 3438ed19f8..f9dc02e8d3 100644 --- a/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthConfigTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthConfigTest.java @@ -303,6 +303,7 @@ public abstract class TwoFactorAuthConfigTest extends AbstractControllerTest { loginTenantAdmin(); TotpTwoFaAccountConfig generatedTotpTwoFaAccountConfig = generateTotpTwoFaAccountConfig(totpTwoFaProviderConfig); + generatedTotpTwoFaAccountConfig.setUseByDefault(true); String secret = UriComponentsBuilder.fromUriString(generatedTotpTwoFaAccountConfig.getAuthUrl()).build() .getQueryParams().getFirst("secret"); @@ -421,6 +422,7 @@ public abstract class TwoFactorAuthConfigTest extends AbstractControllerTest { SmsTwoFaAccountConfig smsTwoFaAccountConfig = new SmsTwoFaAccountConfig(); smsTwoFaAccountConfig.setPhoneNumber("+38051889445"); + smsTwoFaAccountConfig.setUseByDefault(true); ArgumentCaptor verificationCodeCaptor = ArgumentCaptor.forClass(String.class); doPost("/api/2fa/account/config/submit", smsTwoFaAccountConfig).andExpect(status().isOk()); @@ -459,6 +461,7 @@ public abstract class TwoFactorAuthConfigTest extends AbstractControllerTest { SmsTwoFaAccountConfig initialSmsTwoFaAccountConfig = new SmsTwoFaAccountConfig(); initialSmsTwoFaAccountConfig.setPhoneNumber("+38051889445"); + initialSmsTwoFaAccountConfig.setUseByDefault(true); ArgumentCaptor verificationCodeCaptor = ArgumentCaptor.forClass(String.class); doPost("/api/2fa/account/config/submit", initialSmsTwoFaAccountConfig).andExpect(status().isOk());