From 0873b9e1581be217b757e23312c05fc8ceb2e933 Mon Sep 17 00:00:00 2001 From: Andrii Shvaika Date: Mon, 7 Nov 2022 18:12:28 +0200 Subject: [PATCH] Improvements to #7434 --- .../server/controller/AuthController.java | 1 - .../server/controller/UserController.java | 1 - .../auth/DefaultTokenOutdatingService.java | 14 +++-- .../security/auth/TokenOutdatingService.java | 3 +- .../src/main/resources/thingsboard.yml | 8 +-- .../security/auth/TokenOutdatingTest.java | 60 +++++++++++-------- .../server/cache/CacheSpecsMap.java | 14 +++++ .../cache/TbCaffeineCacheConfiguration.java | 2 + .../server/common/data/CacheConstants.java | 2 +- .../server/dao/user/UserServiceImpl.java | 1 - 10 files changed, 67 insertions(+), 39 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/controller/AuthController.java b/application/src/main/java/org/thingsboard/server/controller/AuthController.java index d098b1edec..f98de380ef 100644 --- a/application/src/main/java/org/thingsboard/server/controller/AuthController.java +++ b/application/src/main/java/org/thingsboard/server/controller/AuthController.java @@ -42,7 +42,6 @@ import org.thingsboard.server.common.data.exception.ThingsboardErrorCode; import org.thingsboard.server.common.data.exception.ThingsboardException; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.security.UserCredentials; -import org.thingsboard.server.common.data.security.event.UserAuthDataChangedEvent; import org.thingsboard.server.common.data.security.event.UserCredentialsInvalidationEvent; import org.thingsboard.server.common.data.security.event.UserSessionInvalidationEvent; import org.thingsboard.server.common.data.security.model.SecuritySettings; diff --git a/application/src/main/java/org/thingsboard/server/controller/UserController.java b/application/src/main/java/org/thingsboard/server/controller/UserController.java index fe07c4b49c..cea9d2e95a 100644 --- a/application/src/main/java/org/thingsboard/server/controller/UserController.java +++ b/application/src/main/java/org/thingsboard/server/controller/UserController.java @@ -43,7 +43,6 @@ import org.thingsboard.server.common.data.page.PageData; import org.thingsboard.server.common.data.page.PageLink; import org.thingsboard.server.common.data.security.Authority; import org.thingsboard.server.common.data.security.UserCredentials; -import org.thingsboard.server.common.data.security.event.UserAuthDataChangedEvent; import org.thingsboard.server.common.data.security.event.UserCredentialsInvalidationEvent; import org.thingsboard.server.queue.util.TbCoreComponent; import org.thingsboard.server.service.entitiy.user.TbUserService; diff --git a/application/src/main/java/org/thingsboard/server/service/security/auth/DefaultTokenOutdatingService.java b/application/src/main/java/org/thingsboard/server/service/security/auth/DefaultTokenOutdatingService.java index fecc17d33b..74230d93ec 100644 --- a/application/src/main/java/org/thingsboard/server/service/security/auth/DefaultTokenOutdatingService.java +++ b/application/src/main/java/org/thingsboard/server/service/security/auth/DefaultTokenOutdatingService.java @@ -17,6 +17,7 @@ package org.thingsboard.server.service.security.auth; import io.jsonwebtoken.Claims; import lombok.RequiredArgsConstructor; +import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.event.EventListener; import org.springframework.stereotype.Service; import org.thingsboard.server.cache.TbTransactionalCache; @@ -31,11 +32,16 @@ import java.util.Optional; import static java.util.concurrent.TimeUnit.MILLISECONDS; @Service -@RequiredArgsConstructor public class DefaultTokenOutdatingService implements TokenOutdatingService { + private final TbTransactionalCache cache; private final JwtTokenFactory tokenFactory; + public DefaultTokenOutdatingService(@Qualifier("UsersSessionInvalidation") TbTransactionalCache cache, JwtTokenFactory tokenFactory) { + this.cache = cache; + this.tokenFactory = tokenFactory; + } + @EventListener(classes = UserAuthDataChangedEvent.class) public void onUserAuthDataChanged(UserAuthDataChangedEvent event) { if (StringUtils.hasText(event.getId())) { @@ -48,10 +54,10 @@ public class DefaultTokenOutdatingService implements TokenOutdatingService { Claims claims = tokenFactory.parseTokenClaims(token).getBody(); long issueTime = claims.getIssuedAt().getTime(); String sessionId = claims.get("sessionId", String.class); - if (sessionId == null) { - return isTokenOutdated(issueTime, userId.toString()); + if (isTokenOutdated(issueTime, userId.toString())){ + return true; } else { - return isTokenOutdated(issueTime, userId.toString()) || isTokenOutdated(issueTime, sessionId); + return sessionId != null && isTokenOutdated(issueTime, sessionId); } } diff --git a/application/src/main/java/org/thingsboard/server/service/security/auth/TokenOutdatingService.java b/application/src/main/java/org/thingsboard/server/service/security/auth/TokenOutdatingService.java index 9013a85553..02c1bf685a 100644 --- a/application/src/main/java/org/thingsboard/server/service/security/auth/TokenOutdatingService.java +++ b/application/src/main/java/org/thingsboard/server/service/security/auth/TokenOutdatingService.java @@ -16,11 +16,10 @@ package org.thingsboard.server.service.security.auth; import org.thingsboard.server.common.data.id.UserId; -import org.thingsboard.server.common.data.security.event.UserAuthDataChangedEvent; import org.thingsboard.server.common.data.security.model.JwtToken; public interface TokenOutdatingService { - void onUserAuthDataChanged(UserAuthDataChangedEvent event); boolean isOutdated(JwtToken token, UserId userId); + } diff --git a/application/src/main/resources/thingsboard.yml b/application/src/main/resources/thingsboard.yml index ecb15eb03c..4fae947883 100644 --- a/application/src/main/resources/thingsboard.yml +++ b/application/src/main/resources/thingsboard.yml @@ -109,7 +109,7 @@ security: # JWT Token parameters jwt: tokenExpirationTime: "${JWT_TOKEN_EXPIRATION_TIME:9000}" # Number of seconds (2.5 hours) - refreshTokenExpTime: "${JWT_REFRESH_TOKEN_EXPIRATION_TIME:604800}" # Number of seconds (1 week) + refreshTokenExpTime: "${JWT_REFRESH_TOKEN_EXPIRATION_TIME:604800}" # Number of seconds (1 week). tokenIssuer: "${JWT_TOKEN_ISSUER:thingsboard.io}" tokenSigningKey: "${JWT_TOKEN_SIGNING_KEY:thingsboardDefaultSigningKey}" # Enable/disable access to Tenant Administrators JWT token by System Administrator or Customer Users JWT token by Tenant Administrator @@ -420,9 +420,9 @@ cache: attributes: timeToLiveInMinutes: "${CACHE_SPECS_ATTRIBUTES_TTL:1440}" maxSize: "${CACHE_SPECS_ATTRIBUTES_MAX_SIZE:100000}" - usersUpdateTime: - # MUST be the same as jwt refresh token expiration time, the value here represents 604800 seconds in minutes - timeToLiveInMinutes: "${CACHE_SPECS_USERS_UPDATE_TIME_TTL:10080}" + userSessionsInvalidation: + # The value of this TTL is ignored and replaced by JWT refresh token expiration time + timeToLiveInMinutes: "0" maxSize: "${CACHE_SPECS_USERS_UPDATE_TIME_MAX_SIZE:10000}" otaPackages: timeToLiveInMinutes: "${CACHE_SPECS_OTA_PACKAGES_TTL:60}" diff --git a/application/src/test/java/org/thingsboard/server/service/security/auth/TokenOutdatingTest.java b/application/src/test/java/org/thingsboard/server/service/security/auth/TokenOutdatingTest.java index dddd1f8a71..c84253b92c 100644 --- a/application/src/test/java/org/thingsboard/server/service/security/auth/TokenOutdatingTest.java +++ b/application/src/test/java/org/thingsboard/server/service/security/auth/TokenOutdatingTest.java @@ -21,6 +21,7 @@ import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootContextLoader; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.annotation.ComponentScan; import org.springframework.security.authentication.CredentialsExpiredException; import org.springframework.test.annotation.DirtiesContext; @@ -69,8 +70,9 @@ import static org.mockito.Mockito.when; "security.jwt.tokenIssuer=test.io", "security.jwt.tokenSigningKey=secret", "security.jwt.tokenExpirationTime=600", - "security.jwt.refreshTokenExpTime=60", - "cache.specs.usersUpdateTime.timeToLiveInMinutes=1" + "security.jwt.refreshTokenExpTime=15", + // explicitly set the wrong value to check that it is NOT used. + "cache.specs.userSessionsInvalidation.timeToLiveInMinutes=2" }) public class TokenOutdatingTest { private JwtAuthenticationProvider accessTokenAuthenticationProvider; @@ -79,6 +81,8 @@ public class TokenOutdatingTest { @Autowired private TokenOutdatingService tokenOutdatingService; @Autowired + private ApplicationEventPublisher eventPublisher; + @Autowired private JwtTokenFactory tokenFactory; private SecurityUser securityUser; @@ -107,8 +111,9 @@ public class TokenOutdatingTest { public void testOutdateOldUserTokens() throws Exception { JwtToken jwtToken = tokenFactory.createAccessJwtToken(securityUser); - SECONDS.sleep(1); // need to wait before outdating so that outdatage time is strictly after token issue time - tokenOutdatingService.onUserAuthDataChanged(new UserCredentialsInvalidationEvent(securityUser.getId())); + // Token outdatage time is rounded to 1 sec. Need to wait before outdating so that outdatage time is strictly after token issue time + SECONDS.sleep(1); + eventPublisher.publishEvent(new UserCredentialsInvalidationEvent(securityUser.getId())); assertTrue(tokenOutdatingService.isOutdated(jwtToken, securityUser.getId())); SECONDS.sleep(1); @@ -126,7 +131,7 @@ public class TokenOutdatingTest { }); SECONDS.sleep(1); - tokenOutdatingService.onUserAuthDataChanged(new UserCredentialsInvalidationEvent(securityUser.getId())); + eventPublisher.publishEvent(new UserCredentialsInvalidationEvent(securityUser.getId())); assertThrows(JwtExpiredTokenException.class, () -> { accessTokenAuthenticationProvider.authenticate(new JwtAuthenticationToken(accessJwtToken)); @@ -142,28 +147,33 @@ public class TokenOutdatingTest { }); SECONDS.sleep(1); - tokenOutdatingService.onUserAuthDataChanged(new UserCredentialsInvalidationEvent(securityUser.getId())); + eventPublisher.publishEvent(new UserCredentialsInvalidationEvent(securityUser.getId())); assertThrows(CredentialsExpiredException.class, () -> { refreshTokenAuthenticationProvider.authenticate(new RefreshAuthenticationToken(refreshJwtToken)); }); } - @Test - public void testTokensOutdatageTimeRemovalFromCache() throws Exception { - JwtToken jwtToken = tokenFactory.createAccessJwtToken(securityUser); - - SECONDS.sleep(1); - tokenOutdatingService.onUserAuthDataChanged(new UserCredentialsInvalidationEvent(securityUser.getId())); - - SECONDS.sleep(1); - - assertTrue(tokenOutdatingService.isOutdated(jwtToken, securityUser.getId())); - - SECONDS.sleep(60); - - assertFalse(tokenOutdatingService.isOutdated(jwtToken, securityUser.getId())); - } + // This test takes too long to run and is basically testing the cache logic +// @Test +// public void testTokensOutdatageTimeRemovalFromCache() throws Exception { +// JwtToken jwtToken = tokenFactory.createAccessJwtToken(securityUser); +// +// SECONDS.sleep(1); +// eventPublisher.publishEvent(new UserCredentialsInvalidationEvent(securityUser.getId())); +// +// SECONDS.sleep(1); +// +// assertTrue(tokenOutdatingService.isOutdated(jwtToken, securityUser.getId())); +// +// SECONDS.sleep(30); // refreshTokenExpTime/2 +// +// assertTrue(tokenOutdatingService.isOutdated(jwtToken, securityUser.getId())); +// +// SECONDS.sleep(30 + 1); // refreshTokenExpTime/2 + 1 +// +// assertFalse(tokenOutdatingService.isOutdated(jwtToken, securityUser.getId())); +// } @Test public void testOnlyOneTokenExpired() throws InterruptedException { @@ -178,7 +188,7 @@ public class TokenOutdatingTest { SECONDS.sleep(1); - tokenOutdatingService.onUserAuthDataChanged(new UserSessionInvalidationEvent(securityUser.getSessionId())); + eventPublisher.publishEvent(new UserSessionInvalidationEvent(securityUser.getSessionId())); assertThrows(JwtExpiredTokenException.class, () -> { accessTokenAuthenticationProvider.authenticate(new JwtAuthenticationToken(getRawJwtToken(jwtToken))); @@ -206,14 +216,14 @@ public class TokenOutdatingTest { SECONDS.sleep(1); - tokenOutdatingService.onUserAuthDataChanged(new UserCredentialsInvalidationEvent(securityUser.getId())); + eventPublisher.publishEvent(new UserCredentialsInvalidationEvent(securityUser.getId())); assertThrows(JwtExpiredTokenException.class, () -> { - accessTokenAuthenticationProvider.authenticate(new JwtAuthenticationToken(getRawJwtToken(jwtToken))); + accessTokenAuthenticationProvider.authenticate(new JwtAuthenticationToken(getRawJwtToken(jwtToken))); }); assertThrows(JwtExpiredTokenException.class, () -> { - accessTokenAuthenticationProvider.authenticate(new JwtAuthenticationToken(getRawJwtToken(anotherJwtToken))); + accessTokenAuthenticationProvider.authenticate(new JwtAuthenticationToken(getRawJwtToken(anotherJwtToken))); }); } diff --git a/common/cache/src/main/java/org/thingsboard/server/cache/CacheSpecsMap.java b/common/cache/src/main/java/org/thingsboard/server/cache/CacheSpecsMap.java index 594bfff146..caf1c1f109 100644 --- a/common/cache/src/main/java/org/thingsboard/server/cache/CacheSpecsMap.java +++ b/common/cache/src/main/java/org/thingsboard/server/cache/CacheSpecsMap.java @@ -17,9 +17,12 @@ package org.thingsboard.server.cache; import lombok.Data; import lombok.Getter; +import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.context.annotation.Configuration; +import org.thingsboard.server.common.data.CacheConstants; +import javax.annotation.PostConstruct; import java.util.Map; @Configuration @@ -27,7 +30,18 @@ import java.util.Map; @Data public class CacheSpecsMap { + @Value("${security.jwt.refreshTokenExpTime}") + private int refreshTokenExpTime; + @Getter private Map specs; + @PostConstruct + public void replaceTheJWTTokenRefreshExpTime() { + var cacheSpecs = specs.get(CacheConstants.USERS_SESSION_INVALIDATION_CACHE); + if (cacheSpecs != null) { + cacheSpecs.setTimeToLiveInMinutes((refreshTokenExpTime / 60) + 1); + } + } + } diff --git a/common/cache/src/main/java/org/thingsboard/server/cache/TbCaffeineCacheConfiguration.java b/common/cache/src/main/java/org/thingsboard/server/cache/TbCaffeineCacheConfiguration.java index 9cb139e26a..198404de2e 100644 --- a/common/cache/src/main/java/org/thingsboard/server/cache/TbCaffeineCacheConfiguration.java +++ b/common/cache/src/main/java/org/thingsboard/server/cache/TbCaffeineCacheConfiguration.java @@ -20,6 +20,7 @@ import com.github.benmanes.caffeine.cache.RemovalCause; import com.github.benmanes.caffeine.cache.Ticker; import com.github.benmanes.caffeine.cache.Weigher; import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cache.CacheManager; import org.springframework.cache.annotation.EnableCaching; @@ -71,6 +72,7 @@ public class TbCaffeineCacheConfiguration { } private CaffeineCache buildCache(String name, CacheSpecs cacheSpec) { + final Caffeine caffeineBuilder = Caffeine.newBuilder() .weigher(collectionSafeWeigher()) diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/CacheConstants.java b/common/data/src/main/java/org/thingsboard/server/common/data/CacheConstants.java index 3199b1c0cc..c6fbe3be5f 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/CacheConstants.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/CacheConstants.java @@ -32,7 +32,7 @@ public class CacheConstants { public static final String ASSET_PROFILE_CACHE = "assetProfiles"; public static final String ATTRIBUTES_CACHE = "attributes"; - public static final String USERS_SESSION_INVALIDATION_CACHE = "usersUpdateTime"; + public static final String USERS_SESSION_INVALIDATION_CACHE = "userSessionsInvalidation"; public static final String OTA_PACKAGE_CACHE = "otaPackages"; public static final String OTA_PACKAGE_DATA_CACHE = "otaPackagesData"; public static final String REPOSITORY_SETTINGS_CACHE = "repositorySettings"; diff --git a/dao/src/main/java/org/thingsboard/server/dao/user/UserServiceImpl.java b/dao/src/main/java/org/thingsboard/server/dao/user/UserServiceImpl.java index d4f0dde0a3..41d121b889 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/user/UserServiceImpl.java +++ b/dao/src/main/java/org/thingsboard/server/dao/user/UserServiceImpl.java @@ -37,7 +37,6 @@ import org.thingsboard.server.common.data.id.UserId; import org.thingsboard.server.common.data.page.PageData; import org.thingsboard.server.common.data.page.PageLink; import org.thingsboard.server.common.data.security.UserCredentials; -import org.thingsboard.server.common.data.security.event.UserAuthDataChangedEvent; import org.thingsboard.server.common.data.security.event.UserCredentialsInvalidationEvent; import org.thingsboard.server.dao.entity.AbstractEntityService; import org.thingsboard.server.dao.exception.IncorrectParameterException;