2FA: validation and error handling refactoring

This commit is contained in:
Viacheslav Klimov 2022-03-18 11:05:17 +02:00
parent 9eb03950fa
commit b5afb32f56
14 changed files with 163 additions and 44 deletions

View File

@ -23,9 +23,11 @@ import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.context.support.DefaultMessageSourceResolvable;
import org.springframework.http.MediaType;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.web.bind.MethodArgumentNotValidException;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.thingsboard.server.cluster.TbClusterService;
import org.thingsboard.server.common.data.Customer;
@ -145,6 +147,7 @@ import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;
import static org.thingsboard.server.controller.ControllerConstants.DEFAULT_PAGE_SIZE;
import static org.thingsboard.server.controller.ControllerConstants.INCORRECT_TENANT_ID;
@ -334,6 +337,18 @@ public abstract class BaseController {
}
}
/**
* Handles validation error for controller method arguments annotated with @{@link javax.validation.Valid}
* */
@ExceptionHandler(MethodArgumentNotValidException.class)
public void handleValidationError(MethodArgumentNotValidException e, HttpServletResponse response) {
String errorMessage = "Validation error: " + e.getBindingResult().getAllErrors().stream()
.map(DefaultMessageSourceResolvable::getDefaultMessage)
.collect(Collectors.joining(", "));
ThingsboardException thingsboardException = new ThingsboardException(errorMessage, ThingsboardErrorCode.BAD_REQUEST_PARAMS);
handleThingsboardException(thingsboardException, response);
}
<T> T checkNotNull(T reference) throws ThingsboardException {
return checkNotNull(reference, "Requested item wasn't found!");
}

View File

@ -30,7 +30,7 @@ import org.springframework.web.bind.annotation.RestController;
import org.thingsboard.common.util.JacksonUtil;
import org.thingsboard.server.common.data.exception.ThingsboardErrorCode;
import org.thingsboard.server.common.data.exception.ThingsboardException;
import org.thingsboard.server.dao.service.ConstraintValidator;
import org.thingsboard.server.service.security.auth.TokenOutdatingService;
import org.thingsboard.server.service.security.auth.mfa.TwoFactorAuthService;
import org.thingsboard.server.service.security.auth.mfa.config.TwoFactorAuthSettings;
import org.thingsboard.server.service.security.auth.mfa.config.account.TotpTwoFactorAuthAccountConfig;
@ -42,10 +42,25 @@ import org.thingsboard.server.service.security.model.token.JwtTokenFactory;
import javax.servlet.ServletOutputStream;
import javax.servlet.http.HttpServletResponse;
import javax.validation.Valid;
// FIXME: Swagger documentation
// FIXME: tests for 2FA
/*
*
* TODO [viacheslav]:
* - 2FA should be mandatory when logging in and must be rolled out to all existing users when 2FA is activated.
* - Rate limits should be implemented to protect against brute force leaked accounts to prevent SMS cost explosion.
* - Configurable softlock after XX (3) attempts: XX (15) mins
* - Configurable hardlock (user blocking) after a total of XX (10) unsuccessful attempts.
* - The OTP token should only be valid for XX (5) minutes.
* - Disable 2FA only possible after successful 2FA auth - it is possible with simple password resest
* - 2FA entries should be secured against code injection by code validation.
* - Email 2FA provider
*
* FIXME [viacheslav]:
* - Tests for 2FA
* - Swagger documentation
*
* */
@RestController
@RequestMapping("/api")
@RequiredArgsConstructor
@ -65,7 +80,7 @@ public class TwoFactorAuthController extends BaseController {
@PostMapping("/2fa/account/config/generate")
@PreAuthorize("isAuthenticated()")
public TwoFactorAuthAccountConfig generateTwoFactorAuthAccountConfig(@RequestParam TwoFactorAuthProviderType providerType) throws ThingsboardException {
public TwoFactorAuthAccountConfig generateTwoFactorAuthAccountConfig(@RequestParam TwoFactorAuthProviderType providerType) throws Exception {
SecurityUser user = getCurrentUser();
return twoFactorAuthService.processByTwoFaProvider(user.getTenantId(), providerType,
@ -90,20 +105,19 @@ public class TwoFactorAuthController extends BaseController {
@PostMapping("/2fa/account/config/submit")
@PreAuthorize("isAuthenticated()")
public void submitTwoFactorAuthAccountConfig(@RequestBody TwoFactorAuthAccountConfig accountConfig) throws ThingsboardException {
public void submitTwoFactorAuthAccountConfig(@Valid @RequestBody TwoFactorAuthAccountConfig accountConfig) throws Exception {
SecurityUser user = getCurrentUser();
twoFactorAuthService.processByTwoFaProvider(user.getTenantId(), accountConfig.getProviderType(),
(provider, providerConfig) -> {
ConstraintValidator.validateFields(accountConfig);
provider.prepareVerificationCode(user, providerConfig, accountConfig);
});
}
@PostMapping("/2fa/account/config")
@PreAuthorize("isAuthenticated()")
public void verifyAndSaveTwoFactorAuthAccountConfig(@RequestBody TwoFactorAuthAccountConfig accountConfig,
@RequestParam String verificationCode) throws ThingsboardException {
public void verifyAndSaveTwoFactorAuthAccountConfig(@Valid @RequestBody TwoFactorAuthAccountConfig accountConfig,
@RequestParam String verificationCode) throws Exception {
SecurityUser user = getCurrentUser();
boolean verificationSuccess = twoFactorAuthService.processByTwoFaProvider(user.getTenantId(), accountConfig.getProviderType(),
@ -127,14 +141,14 @@ public class TwoFactorAuthController extends BaseController {
@PostMapping("/2fa/settings")
@PreAuthorize("hasAnyAuthority('SYS_ADMIN', 'TENANT_ADMIN')")
public void saveTwoFactorAuthSettings(@RequestBody TwoFactorAuthSettings twoFactorAuthSettings) throws ThingsboardException {
public void saveTwoFactorAuthSettings(@Valid @RequestBody TwoFactorAuthSettings twoFactorAuthSettings) throws ThingsboardException {
twoFactorAuthService.saveTwoFaSettings(getTenantId(), twoFactorAuthSettings);
}
@PostMapping("/auth/2fa/verification/check")
@PreAuthorize("hasAuthority('PRE_VERIFICATION_TOKEN')")
public JwtTokenPair checkTwoFaVerificationCode(@RequestParam String verificationCode) throws ThingsboardException {
public JwtTokenPair checkTwoFaVerificationCode(@RequestParam String verificationCode) throws Exception {
SecurityUser user = getCurrentUser();
boolean verificationSuccess = twoFactorAuthService.processByTwoFaProvider(user.getTenantId(), user.getId(),
@ -149,4 +163,15 @@ public class TwoFactorAuthController extends BaseController {
}
}
@PostMapping("/auth/2fa/verification/resend")
@PreAuthorize("hasAuthority('PRE_VERIFICATION_TOKEN')")
public void resendTwoFaVerificationCode() throws Exception {
SecurityUser user = getCurrentUser();
twoFactorAuthService.processByTwoFaProvider(user.getTenantId(), user.getId(),
(provider, providerConfig, accountConfig) -> {
provider.prepareVerificationCode(user, providerConfig, accountConfig);
});
}
}

View File

@ -21,7 +21,10 @@ import lombok.SneakyThrows;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;
import org.thingsboard.common.util.JacksonUtil;
import org.thingsboard.common.util.TripleFunction;
import org.thingsboard.common.util.ThrowingBiConsumer;
import org.thingsboard.common.util.ThrowingBiFunction;
import org.thingsboard.common.util.ThrowingTripleConsumer;
import org.thingsboard.common.util.ThrowingTripleFunction;
import org.thingsboard.server.common.data.AdminSettings;
import org.thingsboard.server.common.data.DataConstants;
import org.thingsboard.server.common.data.User;
@ -32,7 +35,6 @@ import org.thingsboard.server.common.data.id.UserId;
import org.thingsboard.server.common.data.kv.BaseAttributeKvEntry;
import org.thingsboard.server.common.data.kv.JsonDataEntry;
import org.thingsboard.server.dao.attributes.AttributesService;
import org.thingsboard.server.dao.service.ConstraintValidator;
import org.thingsboard.server.dao.settings.AdminSettingsService;
import org.thingsboard.server.dao.user.UserService;
import org.thingsboard.server.service.security.auth.mfa.config.TwoFactorAuthSettings;
@ -47,8 +49,6 @@ import java.util.EnumMap;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ExecutionException;
import java.util.function.BiConsumer;
import java.util.function.BiFunction;
@Service
@RequiredArgsConstructor
@ -81,7 +81,7 @@ public class TwoFactorAuthService {
}
public <R> R processByTwoFaProvider(TenantId tenantId, TwoFactorAuthProviderType providerType, BiFunction<TwoFactorAuthProvider<TwoFactorAuthProviderConfig, TwoFactorAuthAccountConfig>, TwoFactorAuthProviderConfig, R> function) throws ThingsboardException {
public <R> R processByTwoFaProvider(TenantId tenantId, TwoFactorAuthProviderType providerType, ThrowingBiFunction<TwoFactorAuthProvider<TwoFactorAuthProviderConfig, TwoFactorAuthAccountConfig>, TwoFactorAuthProviderConfig, R> function) throws Exception {
TwoFactorAuthProviderConfig providerConfig = getTwoFaProviderConfig(tenantId, providerType)
.orElseThrow(() -> new ThingsboardException("2FA provider is not configured", ThingsboardErrorCode.BAD_REQUEST_PARAMS));
TwoFactorAuthProvider<TwoFactorAuthProviderConfig, TwoFactorAuthAccountConfig> provider = getTwoFaProvider(providerType)
@ -90,14 +90,14 @@ public class TwoFactorAuthService {
return function.apply(provider, providerConfig);
}
public void processByTwoFaProvider(TenantId tenantId, TwoFactorAuthProviderType providerType, BiConsumer<TwoFactorAuthProvider<TwoFactorAuthProviderConfig, TwoFactorAuthAccountConfig>, TwoFactorAuthProviderConfig> function) throws ThingsboardException {
public void processByTwoFaProvider(TenantId tenantId, TwoFactorAuthProviderType providerType, ThrowingBiConsumer<TwoFactorAuthProvider<TwoFactorAuthProviderConfig, TwoFactorAuthAccountConfig>, TwoFactorAuthProviderConfig> function) throws Exception {
processByTwoFaProvider(tenantId, providerType, (provider, providerConfig) -> {
function.accept(provider, providerConfig);
return null;
});
}
public <R> R processByTwoFaProvider(TenantId tenantId, UserId userId, TripleFunction<TwoFactorAuthProvider<TwoFactorAuthProviderConfig, TwoFactorAuthAccountConfig>, TwoFactorAuthProviderConfig, TwoFactorAuthAccountConfig, R> function) throws ThingsboardException {
public <R> R processByTwoFaProvider(TenantId tenantId, UserId userId, ThrowingTripleFunction<TwoFactorAuthProvider<TwoFactorAuthProviderConfig, TwoFactorAuthAccountConfig>, TwoFactorAuthProviderConfig, TwoFactorAuthAccountConfig, R> function) throws Exception {
TwoFactorAuthAccountConfig accountConfig = getTwoFaAccountConfig(tenantId, userId)
.orElseThrow(() -> new ThingsboardException("2FA is not configured for user", ThingsboardErrorCode.BAD_REQUEST_PARAMS));
@ -109,6 +109,13 @@ public class TwoFactorAuthService {
return function.apply(provider, providerConfig, accountConfig);
}
public void processByTwoFaProvider(TenantId tenantId, UserId userId, ThrowingTripleConsumer<TwoFactorAuthProvider<TwoFactorAuthProviderConfig, TwoFactorAuthAccountConfig>, TwoFactorAuthProviderConfig, TwoFactorAuthAccountConfig> function) throws Exception {
processByTwoFaProvider(tenantId, userId, (provider, providerConfig, accountConfig) -> {
function.accept(provider, providerConfig, accountConfig);
return null;
});
}
public Optional<TwoFactorAuthAccountConfig> getTwoFaAccountConfig(TenantId tenantId, UserId userId) {
User user = userService.findUserById(tenantId, userId);
@ -121,7 +128,6 @@ public class TwoFactorAuthService {
}
public void saveTwoFaAccountConfig(TenantId tenantId, UserId userId, TwoFactorAuthAccountConfig accountConfig) throws ThingsboardException {
ConstraintValidator.validateFields(accountConfig);
getTwoFaProviderConfig(tenantId, accountConfig.getProviderType())
.orElseThrow(() -> new ThingsboardException("2FA provider is not configured", ThingsboardErrorCode.BAD_REQUEST_PARAMS));
@ -160,7 +166,6 @@ public class TwoFactorAuthService {
@SneakyThrows({InterruptedException.class, ExecutionException.class})
public void saveTwoFaSettings(TenantId tenantId, TwoFactorAuthSettings twoFactorAuthSettings) {
ConstraintValidator.validateFields(twoFactorAuthSettings);
if (tenantId.equals(TenantId.SYS_TENANT_ID)) {
AdminSettings settings = Optional.ofNullable(adminSettingsService.findAdminSettingsByKey(tenantId, TWO_FACTOR_AUTH_SETTINGS_KEY))
.orElseGet(() -> {

View File

@ -19,11 +19,13 @@ import lombok.Data;
import org.thingsboard.server.service.security.auth.mfa.provider.TwoFactorAuthProviderType;
import javax.validation.constraints.NotBlank;
import javax.validation.constraints.Pattern;
@Data
public class TotpTwoFactorAuthAccountConfig implements TwoFactorAuthAccountConfig {
@NotBlank
// @Pattern(regexp = ) // TODO [viacheslav]: validate otp auth url by pattern
private String authUrl;
@Override

View File

@ -25,7 +25,7 @@ import javax.validation.constraints.Pattern;
public class SmsTwoFactorAuthProviderConfig implements TwoFactorAuthProviderConfig {
@NotBlank
@Pattern(regexp = ".*\\$\\{verificationCode}.*", message = "Template must contain verification code")
@Pattern(regexp = ".*\\$\\{verificationCode}.*", message = "template must contain verification code")
private String smsVerificationMessageTemplate;
@Override

View File

@ -23,7 +23,7 @@ import javax.validation.constraints.NotBlank;
@Data
public class TotpTwoFactorAuthProviderConfig implements TwoFactorAuthProviderConfig {
@NotBlank(message = "Issuer name must not be blank")
@NotBlank(message = "issuer name must not be blank")
private String issuerName;
@Override

View File

@ -16,6 +16,7 @@
package org.thingsboard.server.service.security.auth.mfa.provider;
import org.thingsboard.server.common.data.User;
import org.thingsboard.server.common.data.exception.ThingsboardException;
import org.thingsboard.server.service.security.auth.mfa.config.account.TwoFactorAuthAccountConfig;
import org.thingsboard.server.service.security.auth.mfa.config.provider.TwoFactorAuthProviderConfig;
import org.thingsboard.server.service.security.model.SecurityUser;
@ -24,7 +25,7 @@ public interface TwoFactorAuthProvider<C extends TwoFactorAuthProviderConfig, A
A generateNewAccountConfig(User user, C providerConfig);
default void prepareVerificationCode(SecurityUser user, C providerConfig, A accountConfig) {}
default void prepareVerificationCode(SecurityUser user, C providerConfig, A accountConfig) throws ThingsboardException {}
boolean checkVerificationCode(SecurityUser user, String verificationCode, A accountConfig);

View File

@ -25,6 +25,7 @@ import org.thingsboard.rule.engine.api.SmsService;
import org.thingsboard.rule.engine.api.util.TbNodeUtils;
import org.thingsboard.server.common.data.CacheConstants;
import org.thingsboard.server.common.data.User;
import org.thingsboard.server.common.data.exception.ThingsboardException;
import org.thingsboard.server.queue.util.TbCoreComponent;
import org.thingsboard.server.service.security.auth.mfa.config.account.SmsTwoFactorAuthAccountConfig;
import org.thingsboard.server.service.security.auth.mfa.config.provider.SmsTwoFactorAuthProviderConfig;
@ -53,8 +54,7 @@ public class SmsTwoFactorAuthProvider implements TwoFactorAuthProvider<SmsTwoFac
}
@Override
@SneakyThrows // fixme
public void prepareVerificationCode(SecurityUser user, SmsTwoFactorAuthProviderConfig providerConfig, SmsTwoFactorAuthAccountConfig accountConfig) {
public void prepareVerificationCode(SecurityUser user, SmsTwoFactorAuthProviderConfig providerConfig, SmsTwoFactorAuthAccountConfig accountConfig) throws ThingsboardException {
String verificationCode = RandomStringUtils.randomNumeric(6);
verificationCodesCache.put(user.getSessionId(), new VerificationCode(System.currentTimeMillis(), verificationCode));

View File

@ -111,6 +111,7 @@ public class RestAuthenticationProvider implements AuthenticationProvider {
throw new InsufficientAuthenticationException("User has no authority assigned");
SecurityUser securityUser = new SecurityUser(user, userCredentials.isEnabled(), userPrincipal);
// FIXME [viacheslav]: must not yet log login action if 2FA is used !
logLoginAction(user, authentication, ActionType.LOGIN, null);
return new UsernamePasswordAuthenticationToken(securityUser, null, securityUser.getAuthorities());
} catch (Exception e) {

View File

@ -20,12 +20,12 @@ import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.core.Authentication;
import org.springframework.security.web.WebAttributes;
import org.springframework.security.web.authentication.AuthenticationSuccessHandler;
import org.springframework.stereotype.Component;
import org.thingsboard.server.common.data.security.Authority;
import org.thingsboard.server.service.security.auth.jwt.RefreshTokenRepository;
import org.thingsboard.server.service.security.auth.mfa.TwoFactorAuthService;
import org.thingsboard.server.service.security.auth.mfa.config.account.TwoFactorAuthAccountConfig;
import org.thingsboard.server.service.security.model.JwtTokenPair;
@ -53,22 +53,9 @@ public class RestAwareAuthenticationSuccessHandler implements AuthenticationSucc
SecurityUser securityUser = (SecurityUser) authentication.getPrincipal();
JwtTokenPair tokenPair;
// fixme: check if this handler is not called when token is refreshed
Optional<TwoFactorAuthAccountConfig> twoFaAccountConfig = twoFactorAuthService.getTwoFaAccountConfig(securityUser.getTenantId(), securityUser.getId());
if (twoFaAccountConfig.isPresent()) {
try {
twoFactorAuthService.processByTwoFaProvider(securityUser.getTenantId(), twoFaAccountConfig.get().getProviderType(),
(provider, providerConfig) -> {
provider.prepareVerificationCode(securityUser, providerConfig, twoFaAccountConfig.get());
});
tokenPair = new JwtTokenPair();
tokenPair.setToken(tokenFactory.createPreVerificationToken(securityUser).getToken());
tokenPair.setScope(Authority.PRE_VERIFICATION_TOKEN);
} catch (Exception e) {
log.error("Failed to process 2FA for user {}. Falling back to plain auth", securityUser.getId(), e);
tokenPair = tokenFactory.createTokenPair(securityUser);
}
if (authentication instanceof UsernamePasswordAuthenticationToken) {
// TODO [viacheslav]: or maybe create another AuthenticationProvider and put it after rest authentication provider ?
tokenPair = processTwoFa(securityUser);
} else {
tokenPair = tokenFactory.createTokenPair(securityUser);
}
@ -81,6 +68,26 @@ public class RestAwareAuthenticationSuccessHandler implements AuthenticationSucc
clearAuthenticationAttributes(request);
}
private JwtTokenPair processTwoFa(SecurityUser securityUser) {
Optional<TwoFactorAuthAccountConfig> twoFaAccountConfig = twoFactorAuthService.getTwoFaAccountConfig(securityUser.getTenantId(), securityUser.getId());
if (twoFaAccountConfig.isPresent()) {
try {
twoFactorAuthService.processByTwoFaProvider(securityUser.getTenantId(), twoFaAccountConfig.get().getProviderType(),
(provider, providerConfig) -> {
provider.prepareVerificationCode(securityUser, providerConfig, twoFaAccountConfig.get());
});
JwtTokenPair tokenPair = new JwtTokenPair();
tokenPair.setToken(tokenFactory.createPreVerificationToken(securityUser).getToken());
tokenPair.setScope(Authority.PRE_VERIFICATION_TOKEN);
return tokenPair;
} catch (Exception e) {
// TODO [viacheslav]: write audit log
log.error("Failed to process 2FA for user {}. Falling back to plain auth", securityUser.getId(), e);
}
}
return tokenFactory.createTokenPair(securityUser);
}
/**
* Removes temporary authentication-related data which may have been stored
* in the session during the authentication process..

View File

@ -0,0 +1,21 @@
/**
* Copyright © 2016-2022 The Thingsboard Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.thingsboard.common.util;
@FunctionalInterface
public interface ThrowingBiConsumer<A, B> {
void accept(A a, B b) throws Exception;
}

View File

@ -16,6 +16,6 @@
package org.thingsboard.common.util;
@FunctionalInterface
public interface TripleFunction<A, B, C, R> {
R apply(A a, B b, C c);
public interface ThrowingBiFunction<A, B, R> {
R apply(A a, B b) throws Exception;
}

View File

@ -0,0 +1,21 @@
/**
* Copyright © 2016-2022 The Thingsboard Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.thingsboard.common.util;
@FunctionalInterface
public interface ThrowingTripleConsumer<A, B, C> {
void accept(A a, B b, C c) throws Exception;
}

View File

@ -0,0 +1,21 @@
/**
* Copyright © 2016-2022 The Thingsboard Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.thingsboard.common.util;
@FunctionalInterface
public interface ThrowingTripleFunction<A, B, C, R> {
R apply(A a, B b, C c) throws Exception;
}