This is an automated email from the ASF dual-hosted git repository. ilgrosso pushed a commit to branch 4_1_X in repository https://gitbox.apache.org/repos/asf/syncope.git
commit 7590e00ce341b3c80437e87a3263d6edd3c91dcd Author: Francesco Chicchiriccò <[email protected]> AuthorDate: Thu Jun 11 12:46:53 2026 +0200 Cleanup --- .../syncope/core/starter/SelfKeymasterContext.java | 2 +- .../security/AuthenticationAttemptThrottler.java | 55 +++++-------------- .../security/RateLimitAuthenticationException.java | 4 +- .../core/spring/security/WebSecurityContext.java | 21 +++++--- .../AuthenticationAttemptThrottlerTest.java | 63 +++++++++++++--------- .../SyncopeBasicAuthenticationEntryPointTest.java | 6 +-- 6 files changed, 71 insertions(+), 80 deletions(-) diff --git a/core/self-keymaster-starter/src/main/java/org/apache/syncope/core/starter/SelfKeymasterContext.java b/core/self-keymaster-starter/src/main/java/org/apache/syncope/core/starter/SelfKeymasterContext.java index 135547e75d..6e45361604 100644 --- a/core/self-keymaster-starter/src/main/java/org/apache/syncope/core/starter/SelfKeymasterContext.java +++ b/core/self-keymaster-starter/src/main/java/org/apache/syncope/core/starter/SelfKeymasterContext.java @@ -225,7 +225,7 @@ public class SelfKeymasterContext { final UserProvisioningManager provisioningManager, final SecurityProperties securityProperties, final EncryptorManager encryptorManager, - @Qualifier(AuthenticationAttemptThrottler.CACHE_NAME) + @Qualifier(AuthenticationAttemptThrottler.CACHE) final Cache<String, AuthenticationAttemptThrottler.Attempts> authenticationAttemptCache, final KeymasterProperties keymasterProperties) { diff --git a/core/spring/src/main/java/org/apache/syncope/core/spring/security/AuthenticationAttemptThrottler.java b/core/spring/src/main/java/org/apache/syncope/core/spring/security/AuthenticationAttemptThrottler.java index 65f4d7c086..e552a70c3f 100644 --- a/core/spring/src/main/java/org/apache/syncope/core/spring/security/AuthenticationAttemptThrottler.java +++ b/core/spring/src/main/java/org/apache/syncope/core/spring/security/AuthenticationAttemptThrottler.java @@ -24,15 +24,10 @@ import java.util.Deque; import java.util.concurrent.TimeUnit; import java.util.function.LongSupplier; import javax.cache.Cache; -import javax.cache.configuration.MutableConfiguration; -import javax.cache.expiry.TouchedExpiryPolicy; import org.apache.commons.lang3.StringUtils; public class AuthenticationAttemptThrottler { - public static final String CACHE_NAME = - "org.apache.syncope.core.spring.security.AuthenticationAttemptThrottler"; - public record Attempts(Deque<Long> failures, long blockedUntil) implements Serializable { private static final long serialVersionUID = 8023582605543650484L; @@ -46,9 +41,19 @@ public class AuthenticationAttemptThrottler { } } + public static final String CACHE = "AuthenticationAttemptCache"; + + protected static String key(final String domain, final String username) { + return StringUtils.defaultString(domain) + ':' + StringUtils.defaultString(username); + } + + protected static long retryAfterSeconds(final long blockedUntil, final long now) { + return Math.max(1, TimeUnit.MILLISECONDS.toSeconds(blockedUntil - now)); + } + protected final SecurityProperties.AuthenticationThrottleProperties throttle; - protected final LongSupplier clock; + protected final LongSupplier clock = System::currentTimeMillis; protected final Cache<String, Attempts> attempts; @@ -56,36 +61,10 @@ public class AuthenticationAttemptThrottler { final SecurityProperties securityProperties, final Cache<String, Attempts> attempts) { - this(securityProperties, System::currentTimeMillis, attempts); - } - - protected AuthenticationAttemptThrottler( - final SecurityProperties securityProperties, - final LongSupplier clock, - final Cache<String, Attempts> attempts) { - this.throttle = securityProperties.getAuthenticationThrottle(); - this.clock = clock; this.attempts = attempts; } - public static MutableConfiguration<String, Attempts> cacheConfiguration( - final SecurityProperties.AuthenticationThrottleProperties throttle) { - - return new MutableConfiguration<String, Attempts>(). - setTypes(String.class, Attempts.class). - setExpiryPolicyFactory(TouchedExpiryPolicy.factoryOf( - new javax.cache.expiry.Duration(TimeUnit.SECONDS, cacheExpirySeconds(throttle)))); - } - - protected static long cacheExpirySeconds(final SecurityProperties.AuthenticationThrottleProperties throttle) { - return Math.max(1, Math.max(throttle.getWindowSeconds(), throttle.getLockSeconds())); - } - - protected static String key(final String domain, final String username) { - return StringUtils.defaultString(domain) + ':' + StringUtils.defaultString(username); - } - public void checkAllowed(final String domain, final String username) { if (!isEnabled()) { return; @@ -112,7 +91,7 @@ public class AuthenticationAttemptThrottler { return null; }); if (retryAfter != null) { - throw blocked(retryAfter); + throw new RateLimitAuthenticationException(retryAfter); } } @@ -150,7 +129,7 @@ public class AuthenticationAttemptThrottler { return null; }); if (retryAfter != null) { - throw blocked(retryAfter); + throw new RateLimitAuthenticationException(retryAfter); } } @@ -162,12 +141,4 @@ public class AuthenticationAttemptThrottler { } return failures; } - - protected static long retryAfterSeconds(final long blockedUntil, final long now) { - return Math.max(1, TimeUnit.MILLISECONDS.toSeconds(blockedUntil - now)); - } - - protected static RateLimitAuthenticationException blocked(final long retryAfter) { - return new RateLimitAuthenticationException("Too many authentication failures", retryAfter); - } } diff --git a/core/spring/src/main/java/org/apache/syncope/core/spring/security/RateLimitAuthenticationException.java b/core/spring/src/main/java/org/apache/syncope/core/spring/security/RateLimitAuthenticationException.java index a78888b4a2..1a69f9b0af 100644 --- a/core/spring/src/main/java/org/apache/syncope/core/spring/security/RateLimitAuthenticationException.java +++ b/core/spring/src/main/java/org/apache/syncope/core/spring/security/RateLimitAuthenticationException.java @@ -26,8 +26,8 @@ public class RateLimitAuthenticationException extends AuthenticationException { private final long retryAfterSeconds; - public RateLimitAuthenticationException(final String msg, final long retryAfterSeconds) { - super(msg); + public RateLimitAuthenticationException(final long retryAfterSeconds) { + super("Too many authentication failures"); this.retryAfterSeconds = retryAfterSeconds; } diff --git a/core/spring/src/main/java/org/apache/syncope/core/spring/security/WebSecurityContext.java b/core/spring/src/main/java/org/apache/syncope/core/spring/security/WebSecurityContext.java index 77bfdc8b66..bb948d0961 100644 --- a/core/spring/src/main/java/org/apache/syncope/core/spring/security/WebSecurityContext.java +++ b/core/spring/src/main/java/org/apache/syncope/core/spring/security/WebSecurityContext.java @@ -20,8 +20,12 @@ package org.apache.syncope.core.spring.security; import dev.samstevens.totp.code.CodeVerifier; import java.util.List; +import java.util.concurrent.TimeUnit; import javax.cache.Cache; import javax.cache.CacheManager; +import javax.cache.configuration.MutableConfiguration; +import javax.cache.expiry.Duration; +import javax.cache.expiry.TouchedExpiryPolicy; import org.apache.syncope.common.keymaster.client.api.ConfParamOps; import org.apache.syncope.common.keymaster.client.api.DomainOps; import org.apache.syncope.common.lib.types.IdRepoEntitlement; @@ -130,7 +134,7 @@ public class WebSecurityContext { final UserProvisioningManager provisioningManager, final SecurityProperties securityProperties, final EncryptorManager encryptorManager, - @Qualifier(AuthenticationAttemptThrottler.CACHE_NAME) + @Qualifier(AuthenticationAttemptThrottler.CACHE) final Cache<String, AuthenticationAttemptThrottler.Attempts> authenticationAttemptCache) { return new UsernamePasswordAuthenticationProvider( @@ -142,15 +146,20 @@ public class WebSecurityContext { authenticationAttemptCache); } - @ConditionalOnMissingBean(name = AuthenticationAttemptThrottler.CACHE_NAME) - @Bean(name = AuthenticationAttemptThrottler.CACHE_NAME) + @ConditionalOnMissingBean(name = AuthenticationAttemptThrottler.CACHE) + @Bean(name = AuthenticationAttemptThrottler.CACHE) public Cache<String, AuthenticationAttemptThrottler.Attempts> authenticationAttemptCache( final CacheManager cacheManager, final SecurityProperties securityProperties) { - return cacheManager.createCache( - AuthenticationAttemptThrottler.CACHE_NAME, - AuthenticationAttemptThrottler.cacheConfiguration(securityProperties.getAuthenticationThrottle())); + return cacheManager.createCache(AuthenticationAttemptThrottler.CACHE, + new MutableConfiguration<String, AuthenticationAttemptThrottler.Attempts>(). + setTypes(String.class, AuthenticationAttemptThrottler.Attempts.class). + setExpiryPolicyFactory(TouchedExpiryPolicy.factoryOf(new Duration( + TimeUnit.SECONDS, + Math.max(1, Math.max( + securityProperties.getAuthenticationThrottle().getWindowSeconds(), + securityProperties.getAuthenticationThrottle().getLockSeconds())))))); } @Bean diff --git a/core/spring/src/test/java/org/apache/syncope/core/spring/security/AuthenticationAttemptThrottlerTest.java b/core/spring/src/test/java/org/apache/syncope/core/spring/security/AuthenticationAttemptThrottlerTest.java index 3db75e9671..97eac45dd1 100644 --- a/core/spring/src/test/java/org/apache/syncope/core/spring/security/AuthenticationAttemptThrottlerTest.java +++ b/core/spring/src/test/java/org/apache/syncope/core/spring/security/AuthenticationAttemptThrottlerTest.java @@ -21,14 +21,19 @@ package org.apache.syncope.core.spring.security; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertThrows; -import java.util.UUID; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import java.util.function.LongSupplier; import javax.cache.Cache; import javax.cache.Caching; +import javax.cache.configuration.MutableConfiguration; +import javax.cache.expiry.Duration; +import javax.cache.expiry.TouchedExpiryPolicy; +import org.apache.syncope.common.lib.SyncopeConstants; import org.junit.jupiter.api.Test; +import org.springframework.test.util.ReflectionTestUtils; -public class AuthenticationAttemptThrottlerTest { +class AuthenticationAttemptThrottlerTest { private static SecurityProperties securityProperties() { SecurityProperties securityProperties = new SecurityProperties(); @@ -43,55 +48,61 @@ public class AuthenticationAttemptThrottlerTest { final SecurityProperties securityProperties, final LongSupplier clock) { - Cache<String, AuthenticationAttemptThrottler.Attempts> authenticationAttemptCache = - Caching.getCachingProvider().getCacheManager().createCache( - AuthenticationAttemptThrottlerTest.class.getName() + '-' + UUID.randomUUID(), - AuthenticationAttemptThrottler.cacheConfiguration( - securityProperties.getAuthenticationThrottle())); + Cache<String, AuthenticationAttemptThrottler.Attempts> cache = + Caching.getCachingProvider().getCacheManager().getCache(AuthenticationAttemptThrottler.CACHE); + if (cache == null) { + cache = Caching.getCachingProvider().getCacheManager().createCache( + AuthenticationAttemptThrottler.CACHE, + new MutableConfiguration<String, AuthenticationAttemptThrottler.Attempts>(). + setTypes(String.class, AuthenticationAttemptThrottler.Attempts.class). + setExpiryPolicyFactory( + TouchedExpiryPolicy.factoryOf(new Duration(TimeUnit.SECONDS, 30)))); + } else { + cache.clear(); + } - return new AuthenticationAttemptThrottler(securityProperties, clock, authenticationAttemptCache); + AuthenticationAttemptThrottler throttler = new AuthenticationAttemptThrottler(securityProperties, cache); + ReflectionTestUtils.setField(throttler, "clock", clock); + return throttler; } @Test - public void blocksAfterConfiguredFailures() { + void blocksAfterConfiguredFailures() { AtomicLong now = new AtomicLong(); SecurityProperties securityProperties = securityProperties(); - AuthenticationAttemptThrottler throttler = - throttler(securityProperties, now::get); + AuthenticationAttemptThrottler throttler = throttler(securityProperties, now::get); for (int i = 1; i < securityProperties.getAuthenticationThrottle().getMaxAttempts(); i++) { - assertDoesNotThrow(() -> throttler.recordFailure("Master", "rossini")); + assertDoesNotThrow(() -> throttler.recordFailure(SyncopeConstants.MASTER_DOMAIN, "rossini")); } assertThrows(RateLimitAuthenticationException.class, - () -> throttler.recordFailure("Master", "rossini")); + () -> throttler.recordFailure(SyncopeConstants.MASTER_DOMAIN, "rossini")); assertThrows(RateLimitAuthenticationException.class, - () -> throttler.checkAllowed("Master", "rossini")); + () -> throttler.checkAllowed(SyncopeConstants.MASTER_DOMAIN, "rossini")); now.addAndGet(30_000); - assertDoesNotThrow(() -> throttler.checkAllowed("Master", "rossini")); + assertDoesNotThrow(() -> throttler.checkAllowed(SyncopeConstants.MASTER_DOMAIN, "rossini")); } @Test - public void successResetsFailures() { + void successResetsFailures() { AtomicLong now = new AtomicLong(); - AuthenticationAttemptThrottler throttler = - throttler(securityProperties(), now::get); + AuthenticationAttemptThrottler throttler = throttler(securityProperties(), now::get); - throttler.recordFailure("Master", "rossini"); - throttler.clearFailures("Master", "rossini"); + throttler.recordFailure(SyncopeConstants.MASTER_DOMAIN, "rossini"); + throttler.clearFailures(SyncopeConstants.MASTER_DOMAIN, "rossini"); - assertDoesNotThrow(() -> throttler.recordFailure("Master", "rossini")); + assertDoesNotThrow(() -> throttler.recordFailure(SyncopeConstants.MASTER_DOMAIN, "rossini")); } @Test - public void expiredFailuresAreIgnored() { + void expiredFailuresAreIgnored() { AtomicLong now = new AtomicLong(); - AuthenticationAttemptThrottler throttler = - throttler(securityProperties(), now::get); + AuthenticationAttemptThrottler throttler = throttler(securityProperties(), now::get); - throttler.recordFailure("Master", "rossini"); + throttler.recordFailure(SyncopeConstants.MASTER_DOMAIN, "rossini"); now.addAndGet(61_000); - assertDoesNotThrow(() -> throttler.recordFailure("Master", "rossini")); + assertDoesNotThrow(() -> throttler.recordFailure(SyncopeConstants.MASTER_DOMAIN, "rossini")); } } diff --git a/core/spring/src/test/java/org/apache/syncope/core/spring/security/SyncopeBasicAuthenticationEntryPointTest.java b/core/spring/src/test/java/org/apache/syncope/core/spring/security/SyncopeBasicAuthenticationEntryPointTest.java index c7e6022069..7f34017eb8 100644 --- a/core/spring/src/test/java/org/apache/syncope/core/spring/security/SyncopeBasicAuthenticationEntryPointTest.java +++ b/core/spring/src/test/java/org/apache/syncope/core/spring/security/SyncopeBasicAuthenticationEntryPointTest.java @@ -27,17 +27,17 @@ import org.springframework.http.HttpStatus; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; -public class SyncopeBasicAuthenticationEntryPointTest { +class SyncopeBasicAuthenticationEntryPointTest { @Test - public void rateLimitAuthenticationExceptionReturnsTooManyRequests() throws Exception { + void rateLimitAuthenticationExceptionReturnsTooManyRequests() throws Exception { SyncopeBasicAuthenticationEntryPoint entryPoint = new SyncopeBasicAuthenticationEntryPoint(); MockHttpServletResponse response = new MockHttpServletResponse(); entryPoint.commence( new MockHttpServletRequest(), response, - new RateLimitAuthenticationException("Too many authentication failures", 30)); + new RateLimitAuthenticationException(30)); assertEquals(HttpStatus.TOO_MANY_REQUESTS.value(), response.getStatus()); assertEquals("30", response.getHeader(HttpHeaders.RETRY_AFTER));
