From e9b3e155561329f5d30b0ad727f9c3da1baec977 Mon Sep 17 00:00:00 2001 From: Ameya Lokare Date: Mon, 23 Sep 2024 15:34:48 -0700 Subject: [PATCH] Return report spam token from spam check instead of separate call --- .../textsecuregcm/WhisperServerService.java | 9 +----- .../controllers/MessageController.java | 29 +++++++---------- .../spam/ReportSpamTokenProvider.java | 32 ------------------- .../textsecuregcm/spam/SpamChecker.java | 28 +++++++++++++--- .../textsecuregcm/spam/SpamFilter.java | 7 ---- .../controllers/MessageControllerTest.java | 3 +- 6 files changed, 37 insertions(+), 71 deletions(-) delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/spam/ReportSpamTokenProvider.java diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index fdfa31d86..8c5d1a419 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -208,7 +208,6 @@ import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecovery2 import org.whispersystems.textsecuregcm.spam.ChallengeConstraintChecker; import org.whispersystems.textsecuregcm.spam.RegistrationFraudChecker; import org.whispersystems.textsecuregcm.spam.RegistrationRecoveryChecker; -import org.whispersystems.textsecuregcm.spam.ReportSpamTokenProvider; import org.whispersystems.textsecuregcm.spam.SpamChecker; import org.whispersystems.textsecuregcm.spam.SpamFilter; import org.whispersystems.textsecuregcm.storage.AccountLockManager; @@ -1049,12 +1048,6 @@ public class WhisperServerService extends Application { - log.warn("No spam-reporting token providers found; using default (no-op) provider as a default"); - return ReportSpamTokenProvider.noop(); - }); final SpamChecker spamChecker = spamFilter .map(SpamFilter::getSpamChecker) .orElseGet(() -> { @@ -1123,7 +1116,7 @@ public class WhisperServerService extends Application dynamicConfigurationManager; private final ServerSecretParams serverSecretParams; @@ -226,7 +224,6 @@ public class MessageController { ReportMessageManager reportMessageManager, @Nonnull ExecutorService multiRecipientMessageExecutor, Scheduler messageDeliveryScheduler, - @Nonnull ReportSpamTokenProvider reportSpamTokenProvider, final ClientReleaseManager clientReleaseManager, final DynamicConfigurationManager dynamicConfigurationManager, final ServerSecretParams serverSecretParams, @@ -245,7 +242,6 @@ public class MessageController { this.reportMessageManager = reportMessageManager; this.multiRecipientMessageExecutor = Objects.requireNonNull(multiRecipientMessageExecutor); this.messageDeliveryScheduler = messageDeliveryScheduler; - this.reportSpamTokenProvider = reportSpamTokenProvider; this.clientReleaseManager = clientReleaseManager; this.dynamicConfigurationManager = dynamicConfigurationManager; this.serverSecretParams = serverSecretParams; @@ -304,7 +300,7 @@ public class MessageController { final Sample sample = Timer.start(); try { if (source.isEmpty() && accessKey.isEmpty() && groupSendToken == null && !isStory) { - throw new WebApplicationException(Response.Status.UNAUTHORIZED); + throw new WebApplicationException(Status.UNAUTHORIZED); } if (groupSendToken != null) { @@ -333,17 +329,14 @@ public class MessageController { destination = source.map(AuthenticatedDevice::getAccount); } - final Optional spamCheck = spamChecker.checkForSpam( - context, source.map(AuthenticatedDevice::getAccount), destination); - if (spamCheck.isPresent()) { - return spamCheck.get(); + final SpamChecker.SpamCheckResult spamCheck = spamChecker.checkForSpam( + context, source, destination); + final Optional reportSpamToken; + switch (spamCheck) { + case final SpamChecker.Spam spam: return spam.response(); + case final SpamChecker.NotSpam notSpam: reportSpamToken = notSpam.token(); } - final Optional spamReportToken = switch (senderType) { - case SENDER_TYPE_IDENTIFIED -> reportSpamTokenProvider.makeReportSpamToken(context, source.get(), destination); - default -> Optional.empty(); - }; - int totalContentLength = 0; for (final IncomingMessage message : messages.messages()) { @@ -453,7 +446,7 @@ public class MessageController { messages.urgent(), incomingMessage, userAgent, - spamReportToken); + reportSpamToken); }); } @@ -555,9 +548,9 @@ public class MessageController { @Context ContainerRequestContext context) throws RateLimitExceededException { - final Optional spamCheck = spamChecker.checkForSpam(context, Optional.empty(), Optional.empty()); - if (spamCheck.isPresent()) { - return spamCheck.get(); + final SpamChecker.SpamCheckResult spamCheck = spamChecker.checkForSpam(context, Optional.empty(), Optional.empty()); + if (spamCheck instanceof final SpamChecker.Spam spam) { + return spam.response(); } if (groupSendToken == null && accessKeys == null && !isStory) { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/spam/ReportSpamTokenProvider.java b/service/src/main/java/org/whispersystems/textsecuregcm/spam/ReportSpamTokenProvider.java deleted file mode 100644 index de785c02b..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/spam/ReportSpamTokenProvider.java +++ /dev/null @@ -1,32 +0,0 @@ -package org.whispersystems.textsecuregcm.spam; - -import org.whispersystems.textsecuregcm.auth.AccountAndAuthenticatedDeviceHolder; -import org.whispersystems.textsecuregcm.storage.Account; -import javax.ws.rs.container.ContainerRequestContext; -import java.util.Optional; - -/** - * Generates ReportSpamTokens to be used for spam reports. - */ -public interface ReportSpamTokenProvider { - - /** - * Generate a new ReportSpamToken - * - * @param context the message request context - * @param sender the account that sent the unsealed sender message - * @param maybeDestination the intended recepient of the message if available - * @return either a generated token or nothing - */ - Optional makeReportSpamToken(ContainerRequestContext context, final AccountAndAuthenticatedDeviceHolder sender, - final Optional maybeDestination); - - /** - * Provider which generates nothing - * - * @return the provider - */ - static ReportSpamTokenProvider noop() { - return (ignoredContext, ignoredSender, ignoredDest) -> Optional.empty(); - } -} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/spam/SpamChecker.java b/service/src/main/java/org/whispersystems/textsecuregcm/spam/SpamChecker.java index 15983d191..dbd288999 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/spam/SpamChecker.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/spam/SpamChecker.java @@ -4,6 +4,7 @@ */ package org.whispersystems.textsecuregcm.spam; +import org.whispersystems.textsecuregcm.auth.AccountAndAuthenticatedDeviceHolder; import org.whispersystems.textsecuregcm.storage.Account; import javax.ws.rs.container.ContainerRequestContext; import javax.ws.rs.core.Response; @@ -11,6 +12,25 @@ import java.util.Optional; public interface SpamChecker { + /** + * A result from the spam checker that is one of: + *
    + *
  • + * Message is determined to be spam, and a response is returned + *
    • + *
  • + * Message is not spam, and an optional spam token is returned + *
    • + *
+ */ + sealed interface SpamCheckResult {} + + record Spam(Response response) implements SpamCheckResult {} + + record NotSpam(Optional token) implements SpamCheckResult { + public static final NotSpam EMPTY_TOKEN = new NotSpam(Optional.empty()); + } + /** * Determine if a message may be spam * @@ -18,14 +38,14 @@ public interface SpamChecker { * @param maybeSource The sender of the message, could be empty if this as message sent with sealed sender * @param maybeDestination The destination of the message, could be empty if the destination does not exist or could * not be retrieved - * @return A response to return if the request is determined to be spam, otherwise empty if the message should be sent + * @return A {@link SpamCheckResult} */ - Optional checkForSpam( + SpamCheckResult checkForSpam( final ContainerRequestContext requestContext, - final Optional maybeSource, + final Optional maybeSource, final Optional maybeDestination); static SpamChecker noop() { - return (ignoredContext, ignoredSource, ignoredDestination) -> Optional.empty(); + return (ignoredContext, ignoredSource, ignoredDestination) -> NotSpam.EMPTY_TOKEN; } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/spam/SpamFilter.java b/service/src/main/java/org/whispersystems/textsecuregcm/spam/SpamFilter.java index 89cba9e9f..6d05858b0 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/spam/SpamFilter.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/spam/SpamFilter.java @@ -33,13 +33,6 @@ public interface SpamFilter extends Managed { */ void configure(String environmentName, Validator validator) throws IOException, ConfigurationValidationException; - /** - * Builds a spam report token provider. This will generate tokens used by the spam reporting system. - * - * @return the configured spam report token provider. - */ - ReportSpamTokenProvider getReportSpamTokenProvider(); - /** * Return a reported message listener controlled by the spam filter. Listeners will be registered with the * {@link org.whispersystems.textsecuregcm.storage.ReportMessageManager}. diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java index fc9b3f92d..a2c54c905 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java @@ -114,7 +114,6 @@ import org.whispersystems.textsecuregcm.push.MessageSender; import org.whispersystems.textsecuregcm.push.PushNotificationManager; import org.whispersystems.textsecuregcm.push.PushNotificationScheduler; import org.whispersystems.textsecuregcm.push.ReceiptSender; -import org.whispersystems.textsecuregcm.spam.ReportSpamTokenProvider; import org.whispersystems.textsecuregcm.spam.SpamChecker; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; @@ -207,7 +206,7 @@ class MessageControllerTest { .addResource( new MessageController(rateLimiters, cardinalityEstimator, messageSender, receiptSender, accountsManager, messagesManager, pushNotificationManager, pushNotificationScheduler, reportMessageManager, multiRecipientMessageExecutor, - messageDeliveryScheduler, ReportSpamTokenProvider.noop(), mock(ClientReleaseManager.class), dynamicConfigurationManager, + messageDeliveryScheduler, mock(ClientReleaseManager.class), dynamicConfigurationManager, serverSecretParams, SpamChecker.noop(), new MessageMetrics(), mock(MessageDeliveryLoopMonitor.class), clock)) .build();