Remove global lock in RateLimitChallengeManager

This commit is contained in:
Ravi Khadiwala 2024-01-16 19:24:45 -06:00 committed by ravi-signal
parent 47bfb25f2c
commit 360a4793ae
3 changed files with 13 additions and 16 deletions

View File

@ -29,6 +29,7 @@ import io.micrometer.core.instrument.binder.jvm.ExecutorServiceMetrics;
import java.net.http.HttpClient; import java.net.http.HttpClient;
import java.time.Clock; import java.time.Clock;
import java.time.Duration; import java.time.Duration;
import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.EnumSet; import java.util.EnumSet;
import java.util.List; import java.util.List;
@ -603,8 +604,6 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
PushChallengeManager pushChallengeManager = new PushChallengeManager(pushNotificationManager, PushChallengeManager pushChallengeManager = new PushChallengeManager(pushNotificationManager,
pushChallengeDynamoDb); pushChallengeDynamoDb);
RateLimitChallengeManager rateLimitChallengeManager = new RateLimitChallengeManager(pushChallengeManager,
captchaChecker, rateLimiters);
ChangeNumberManager changeNumberManager = new ChangeNumberManager(messageSender, accountsManager); ChangeNumberManager changeNumberManager = new ChangeNumberManager(messageSender, accountsManager);
@ -740,6 +739,7 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
boolean registeredSpamFilter = false; boolean registeredSpamFilter = false;
ReportSpamTokenProvider reportSpamTokenProvider = null; ReportSpamTokenProvider reportSpamTokenProvider = null;
List<RateLimitChallengeListener> rateLimitChallengeListeners = new ArrayList<>();
for (final SpamFilter filter : ServiceLoader.load(SpamFilter.class)) { for (final SpamFilter filter : ServiceLoader.load(SpamFilter.class)) {
if (filter.getClass().isAnnotationPresent(FilterSpam.class)) { if (filter.getClass().isAnnotationPresent(FilterSpam.class)) {
try { try {
@ -770,9 +770,12 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
if (filter instanceof RateLimitChallengeListener) { if (filter instanceof RateLimitChallengeListener) {
log.info("Registered rate limit challenge listener: {}", filter.getClass().getName()); log.info("Registered rate limit challenge listener: {}", filter.getClass().getName());
rateLimitChallengeManager.addListener((RateLimitChallengeListener) filter); rateLimitChallengeListeners.add((RateLimitChallengeListener) filter);
} }
} }
RateLimitChallengeManager rateLimitChallengeManager = new RateLimitChallengeManager(pushChallengeManager,
captchaChecker, rateLimiters, rateLimitChallengeListeners);
if (!registeredSpamFilter) { if (!registeredSpamFilter) {
log.warn("No spam filters installed"); log.warn("No spam filters installed");

View File

@ -11,11 +11,8 @@ import io.micrometer.core.instrument.Metrics;
import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.Tag;
import io.micrometer.core.instrument.Tags; import io.micrometer.core.instrument.Tags;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.Optional; import java.util.Optional;
import org.whispersystems.textsecuregcm.captcha.Action; import org.whispersystems.textsecuregcm.captcha.Action;
import org.whispersystems.textsecuregcm.captcha.CaptchaChecker; import org.whispersystems.textsecuregcm.captcha.CaptchaChecker;
import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException; import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException;
@ -32,8 +29,7 @@ public class RateLimitChallengeManager {
private final CaptchaChecker captchaChecker; private final CaptchaChecker captchaChecker;
private final RateLimiters rateLimiters; private final RateLimiters rateLimiters;
private final List<RateLimitChallengeListener> rateLimitChallengeListeners = private final List<RateLimitChallengeListener> rateLimitChallengeListeners;
Collections.synchronizedList(new ArrayList<>());
private static final String RECAPTCHA_ATTEMPT_COUNTER_NAME = name(RateLimitChallengeManager.class, "recaptcha", "attempt"); private static final String RECAPTCHA_ATTEMPT_COUNTER_NAME = name(RateLimitChallengeManager.class, "recaptcha", "attempt");
private static final String RESET_RATE_LIMIT_EXCEEDED_COUNTER_NAME = name(RateLimitChallengeManager.class, "resetRateLimitExceeded"); private static final String RESET_RATE_LIMIT_EXCEEDED_COUNTER_NAME = name(RateLimitChallengeManager.class, "resetRateLimitExceeded");
@ -44,15 +40,13 @@ public class RateLimitChallengeManager {
public RateLimitChallengeManager( public RateLimitChallengeManager(
final PushChallengeManager pushChallengeManager, final PushChallengeManager pushChallengeManager,
final CaptchaChecker captchaChecker, final CaptchaChecker captchaChecker,
final RateLimiters rateLimiters) { final RateLimiters rateLimiters,
final List<RateLimitChallengeListener> rateLimitChallengeListeners) {
this.pushChallengeManager = pushChallengeManager; this.pushChallengeManager = pushChallengeManager;
this.captchaChecker = captchaChecker; this.captchaChecker = captchaChecker;
this.rateLimiters = rateLimiters; this.rateLimiters = rateLimiters;
} this.rateLimitChallengeListeners = rateLimitChallengeListeners;
public void addListener(final RateLimitChallengeListener rateLimitChallengeListener) {
rateLimitChallengeListeners.add(rateLimitChallengeListener);
} }
public void answerPushChallenge(final Account account, final String challenge) throws RateLimitExceededException { public void answerPushChallenge(final Account account, final String challenge) throws RateLimitExceededException {

View File

@ -13,6 +13,7 @@ import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
import java.io.IOException; import java.io.IOException;
import java.util.List;
import java.util.Optional; import java.util.Optional;
import java.util.UUID; import java.util.UUID;
import java.util.stream.Stream; import java.util.stream.Stream;
@ -51,9 +52,8 @@ class RateLimitChallengeManagerTest {
rateLimitChallengeManager = new RateLimitChallengeManager( rateLimitChallengeManager = new RateLimitChallengeManager(
pushChallengeManager, pushChallengeManager,
captchaChecker, captchaChecker,
rateLimiters); rateLimiters,
List.of(rateLimitChallengeListener));
rateLimitChallengeManager.addListener(rateLimitChallengeListener);
} }
@ParameterizedTest @ParameterizedTest