From 509962932c4535013f14a583f93be7364d4c14e6 Mon Sep 17 00:00:00 2001 From: Viacheslav Klimov Date: Fri, 20 May 2022 14:55:37 +0300 Subject: [PATCH] Change verificationCodeSendRateLimit to minVerificationCodeSendPeriod --- .../server/controller/TwoFaConfigController.java | 4 ++-- .../security/auth/mfa/DefaultTwoFactorAuthService.java | 7 ++++++- .../server/controller/TwoFactorAuthConfigTest.java | 3 --- .../thingsboard/server/controller/TwoFactorAuthTest.java | 8 +++----- .../data/security/model/mfa/PlatformTwoFaSettings.java | 3 +-- 5 files changed, 12 insertions(+), 13 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 f683c03187..87e54ec196 100644 --- a/application/src/main/java/org/thingsboard/server/controller/TwoFaConfigController.java +++ b/application/src/main/java/org/thingsboard/server/controller/TwoFaConfigController.java @@ -214,7 +214,7 @@ public class TwoFaConfigController extends BaseController { @ApiOperation(value = "Save platform 2FA settings (savePlatformTwoFaSettings)", notes = "Save 2FA settings for platform. The settings have following properties:\n" + "- `providers` - the list of 2FA providers' configs. Users will only be allowed to use 2FA providers from this list. \n\n" + - "- `verificationCodeSendRateLimit` - rate limit configuration for verification code sending. " + + "- `minVerificationCodeSendPeriod` - minimal period in seconds to wait after verification code send request to send next request. " + "The format is standard: 'amountOfRequests:periodInSeconds'. The value of '1:60' would limit verification " + "code sending requests to one per minute.\n" + "- `verificationCodeCheckRateLimit` - rate limit configuration for verification code checking.\n" + @@ -246,7 +246,7 @@ public class TwoFaConfigController extends BaseController { " \"smsVerificationMessageTemplate\": \"Here is your verification code: ${code}\"\n" + " }\n" + " ],\n" + - " \"verificationCodeSendRateLimit\": \"1:60\",\n" + + " \"minVerificationCodeSendPeriod\": 60,\n" + " \"verificationCodeCheckRateLimit\": \"3:900\",\n" + " \"maxVerificationFailuresBeforeUserLockout\": 10,\n" + " \"totalAllowedTimeForVerification\": 600\n" + diff --git a/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/DefaultTwoFactorAuthService.java b/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/DefaultTwoFactorAuthService.java index c3ab2c6650..d38483d0de 100644 --- a/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/DefaultTwoFactorAuthService.java +++ b/application/src/main/java/org/thingsboard/server/service/security/auth/mfa/DefaultTwoFactorAuthService.java @@ -86,7 +86,12 @@ public class DefaultTwoFactorAuthService implements TwoFactorAuthService { PlatformTwoFaSettings twoFaSettings = configManager.getPlatformTwoFaSettings(user.getTenantId(), true) .orElseThrow(() -> PROVIDER_NOT_CONFIGURED_ERROR); if (checkLimits) { - checkRateLimits(user.getId(), accountConfig.getProviderType(), twoFaSettings.getVerificationCodeSendRateLimit(), verificationCodeSendingRateLimits); + Integer minVerificationCodeSendPeriod = twoFaSettings.getMinVerificationCodeSendPeriod(); + String rateLimit = null; + if (minVerificationCodeSendPeriod != null && minVerificationCodeSendPeriod > 0) { + rateLimit = "1:" + minVerificationCodeSendPeriod; + } + checkRateLimits(user.getId(), accountConfig.getProviderType(), rateLimit, verificationCodeSendingRateLimits); } TwoFaProviderConfig providerConfig = twoFaSettings.getProviderConfig(accountConfig.getProviderType()) 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 c0b2e48c35..ec5b128efd 100644 --- a/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthConfigTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthConfigTest.java @@ -96,7 +96,6 @@ public abstract class TwoFactorAuthConfigTest extends AbstractControllerTest { PlatformTwoFaSettings twoFaSettings = new PlatformTwoFaSettings(); twoFaSettings.setProviders(List.of(totpTwoFaProviderConfig, smsTwoFaProviderConfig)); - twoFaSettings.setVerificationCodeSendRateLimit("1:60"); twoFaSettings.setVerificationCodeCheckRateLimit("3:900"); twoFaSettings.setMaxVerificationFailuresBeforeUserLockout(10); twoFaSettings.setTotalAllowedTimeForVerification(3600); @@ -115,7 +114,6 @@ public abstract class TwoFactorAuthConfigTest extends AbstractControllerTest { PlatformTwoFaSettings twoFaSettings = new PlatformTwoFaSettings(); twoFaSettings.setProviders(Collections.emptyList()); - twoFaSettings.setVerificationCodeSendRateLimit("ab:aba"); twoFaSettings.setVerificationCodeCheckRateLimit("0:12"); twoFaSettings.setMaxVerificationFailuresBeforeUserLockout(-1); twoFaSettings.setTotalAllowedTimeForVerification(0); @@ -124,7 +122,6 @@ public abstract class TwoFactorAuthConfigTest extends AbstractControllerTest { .andExpect(status().isBadRequest())); assertThat(errorMessage).contains( - "verification code send rate limit configuration is invalid", "verification code check rate limit configuration is invalid", "maximum number of verification failure before user lockout must be positive", "total amount of time allotted for verification must be greater than 0" 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 2d05974325..2f7ee93cee 100644 --- a/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthTest.java @@ -202,15 +202,13 @@ public abstract class TwoFactorAuthTest extends AbstractControllerTest { @Test public void testSendVerificationCode_rateLimit() throws Exception { configureTotpTwoFa(twoFaSettings -> { - twoFaSettings.setVerificationCodeSendRateLimit("3:10"); + twoFaSettings.setMinVerificationCodeSendPeriod(10); }); logInWithPreVerificationToken(username, password); - for (int i = 0; i < 3; i++) { - doPost("/api/auth/2fa/verification/send?providerType=TOTP") - .andExpect(status().isOk()); - } + doPost("/api/auth/2fa/verification/send?providerType=TOTP") + .andExpect(status().isOk()); String rateLimitExceededError = getErrorMessage(doPost("/api/auth/2fa/verification/send?providerType=TOTP") .andExpect(status().isTooManyRequests())); 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 87d6403b04..9d581c0867 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 @@ -33,8 +33,7 @@ public class PlatformTwoFaSettings { @Valid private List providers; - @Pattern(regexp = "[1-9]\\d*:[1-9]\\d*", message = "verification code send rate limit configuration is invalid") - private String verificationCodeSendRateLimit; + private Integer minVerificationCodeSendPeriod; @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")