Lazily get captcha clients to avoid initialization issues
This commit is contained in:
parent
39b1935350
commit
997129871c
|
@ -57,6 +57,7 @@ import java.util.concurrent.LinkedBlockingQueue;
|
|||
import java.util.concurrent.ScheduledExecutorService;
|
||||
import java.util.concurrent.SynchronousQueue;
|
||||
import java.util.concurrent.ThreadPoolExecutor;
|
||||
import java.util.function.Function;
|
||||
import java.util.stream.Stream;
|
||||
import javax.annotation.Nullable;
|
||||
import javax.servlet.DispatcherType;
|
||||
|
@ -1057,11 +1058,11 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
|
|||
log.warn("No registration-recovery-checkers found; using default (no-op) provider as a default");
|
||||
return RegistrationRecoveryChecker.noop();
|
||||
});
|
||||
final List<CaptchaClient> captchaClients = spamFilter
|
||||
.map(SpamFilter::getCaptchaClients)
|
||||
final Function<String, CaptchaClient> captchaClientSupplier = spamFilter
|
||||
.map(SpamFilter::getCaptchaClientSupplier)
|
||||
.orElseGet(() -> {
|
||||
log.warn("No captcha clients found; using default (no-op) client as default");
|
||||
return List.of(CaptchaClient.noop());
|
||||
return ignored -> CaptchaClient.noop();
|
||||
});
|
||||
|
||||
spamFilter.map(SpamFilter::getReportedMessageListener).ifPresent(reportMessageManager::addListener);
|
||||
|
@ -1069,7 +1070,7 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
|
|||
final HttpClient shortCodeRetrieverHttpClient = HttpClient.newBuilder().version(HttpClient.Version.HTTP_2)
|
||||
.connectTimeout(Duration.ofSeconds(10)).build();
|
||||
final ShortCodeExpander shortCodeRetriever = new ShortCodeExpander(shortCodeRetrieverHttpClient, config.getShortCodeRetrieverConfiguration().baseUrl());
|
||||
final CaptchaChecker captchaChecker = new CaptchaChecker(shortCodeRetriever, captchaClients);
|
||||
final CaptchaChecker captchaChecker = new CaptchaChecker(shortCodeRetriever, captchaClientSupplier);
|
||||
|
||||
final RegistrationCaptchaManager registrationCaptchaManager = new RegistrationCaptchaManager(captchaChecker);
|
||||
|
||||
|
|
|
@ -10,12 +10,9 @@ import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name;
|
|||
import com.google.common.annotations.VisibleForTesting;
|
||||
import io.micrometer.core.instrument.Metrics;
|
||||
import java.io.IOException;
|
||||
import java.util.List;
|
||||
import java.util.Locale;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
import java.util.function.Function;
|
||||
import java.util.stream.Collectors;
|
||||
import javax.ws.rs.BadRequestException;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
|
@ -32,14 +29,13 @@ public class CaptchaChecker {
|
|||
private static final String SHORT_SUFFIX = "-short";
|
||||
|
||||
private final ShortCodeExpander shortCodeExpander;
|
||||
private final Map<String, CaptchaClient> captchaClientMap;
|
||||
private final Function<String, CaptchaClient> captchaClientSupplier;
|
||||
|
||||
public CaptchaChecker(
|
||||
final ShortCodeExpander shortCodeRetriever,
|
||||
final List<CaptchaClient> captchaClients) {
|
||||
final Function<String, CaptchaClient> captchaClientSupplier) {
|
||||
this.shortCodeExpander = shortCodeRetriever;
|
||||
this.captchaClientMap = captchaClients.stream()
|
||||
.collect(Collectors.toMap(CaptchaClient::scheme, Function.identity()));
|
||||
this.captchaClientSupplier = captchaClientSupplier;
|
||||
}
|
||||
|
||||
|
||||
|
@ -81,7 +77,7 @@ public class CaptchaChecker {
|
|||
token = shortCodeExpander.retrieve(token).orElseThrow(() -> new BadRequestException("invalid shortcode"));
|
||||
}
|
||||
|
||||
final CaptchaClient client = this.captchaClientMap.get(provider);
|
||||
final CaptchaClient client = this.captchaClientSupplier.apply(provider);
|
||||
if (client == null) {
|
||||
throw new BadRequestException("invalid captcha scheme");
|
||||
}
|
||||
|
|
|
@ -9,6 +9,7 @@ import io.dropwizard.configuration.ConfigurationValidationException;
|
|||
import io.dropwizard.lifecycle.Managed;
|
||||
import java.io.IOException;
|
||||
import java.util.List;
|
||||
import java.util.function.Function;
|
||||
import javax.validation.Validator;
|
||||
import org.whispersystems.textsecuregcm.captcha.CaptchaChecker;
|
||||
import org.whispersystems.textsecuregcm.captcha.CaptchaClient;
|
||||
|
@ -84,5 +85,12 @@ public interface SpamFilter extends Managed {
|
|||
*/
|
||||
RegistrationRecoveryChecker getRegistrationRecoveryChecker();
|
||||
|
||||
List<CaptchaClient> getCaptchaClients();
|
||||
/**
|
||||
* Return a function that will be used to lazily fetch the captcha client for a specified scheme. This is to avoid
|
||||
* initialization issues with the spam filter if eagerly fetched.
|
||||
*
|
||||
* @return a {@link Function} that takes the scheme and returns a {@link CaptchaClient}. Returns null if no captcha
|
||||
* client for the scheme exists
|
||||
*/
|
||||
Function<String, CaptchaClient> getCaptchaClientSupplier();
|
||||
}
|
||||
|
|
|
@ -18,6 +18,7 @@ import static org.whispersystems.textsecuregcm.captcha.CaptchaChecker.SEPARATOR;
|
|||
import java.io.IOException;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Optional;
|
||||
import java.util.stream.Stream;
|
||||
import javax.ws.rs.BadRequestException;
|
||||
|
@ -79,7 +80,7 @@ public class CaptchaCheckerTest {
|
|||
final String siteKey,
|
||||
final Action expectedAction) throws IOException {
|
||||
final CaptchaClient captchaClient = mockClient(PREFIX);
|
||||
new CaptchaChecker(null, List.of(captchaClient)).verify(expectedAction, input, null, USER_AGENT);
|
||||
new CaptchaChecker(null, PREFIX -> captchaClient).verify(expectedAction, input, null, USER_AGENT);
|
||||
verify(captchaClient, times(1)).verify(eq(siteKey), eq(expectedAction), eq(expectedToken), any(), eq(USER_AGENT));
|
||||
}
|
||||
|
||||
|
@ -106,11 +107,12 @@ public class CaptchaCheckerTest {
|
|||
String binput = String.join(SEPARATOR, PREFIX_B, CHALLENGE_SITE_KEY, "challenge", TOKEN);
|
||||
final CaptchaClient a = mockClient(PREFIX_A);
|
||||
final CaptchaClient b = mockClient(PREFIX_B);
|
||||
final Map<String, CaptchaClient> captchaClientMap = Map.of(PREFIX_A, a, PREFIX_B, b);
|
||||
|
||||
new CaptchaChecker(null, List.of(a, b)).verify(Action.CHALLENGE, ainput, null, USER_AGENT);
|
||||
new CaptchaChecker(null, captchaClientMap::get).verify(Action.CHALLENGE, ainput, null, USER_AGENT);
|
||||
verify(a, times(1)).verify(any(), any(), any(), any(), any());
|
||||
|
||||
new CaptchaChecker(null, List.of(a, b)).verify(Action.CHALLENGE, binput, null, USER_AGENT);
|
||||
new CaptchaChecker(null, captchaClientMap::get).verify(Action.CHALLENGE, binput, null, USER_AGENT);
|
||||
verify(b, times(1)).verify(any(), any(), any(), any(), any());
|
||||
}
|
||||
|
||||
|
@ -132,7 +134,7 @@ public class CaptchaCheckerTest {
|
|||
public void badArgs(final String input) throws IOException {
|
||||
final CaptchaClient cc = mockClient(PREFIX);
|
||||
assertThrows(BadRequestException.class,
|
||||
() -> new CaptchaChecker(null, List.of(cc)).verify(Action.CHALLENGE, input, null, USER_AGENT));
|
||||
() -> new CaptchaChecker(null, prefix -> PREFIX.equals(prefix) ? cc : null).verify(Action.CHALLENGE, input, null, USER_AGENT));
|
||||
|
||||
}
|
||||
|
||||
|
@ -142,7 +144,7 @@ public class CaptchaCheckerTest {
|
|||
final ShortCodeExpander retriever = mock(ShortCodeExpander.class);
|
||||
when(retriever.retrieve("abc")).thenReturn(Optional.of(TOKEN));
|
||||
final String input = String.join(SEPARATOR, PREFIX + "-short", REG_SITE_KEY, "registration", "abc");
|
||||
new CaptchaChecker(retriever, List.of(captchaClient)).verify(Action.REGISTRATION, input, null, USER_AGENT);
|
||||
new CaptchaChecker(retriever, ignored -> captchaClient).verify(Action.REGISTRATION, input, null, USER_AGENT);
|
||||
verify(captchaClient, times(1)).verify(eq(REG_SITE_KEY), eq(Action.REGISTRATION), eq(TOKEN), any(), any());
|
||||
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue