From 38a0737afbe1b71130e63009ca7da659aef8d2af Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Mon, 30 Jan 2023 11:48:32 -0500 Subject: [PATCH] Retire `ReportSpamTokenHandler` interface in favor of `ReportedMessageListener` --- .../textsecuregcm/WhisperServerService.java | 23 +++------ .../controllers/MessageController.java | 16 ++----- .../spam/ReportSpamTokenHandler.java | 47 ------------------- .../textsecuregcm/spam/SpamFilter.java | 10 ++-- .../controllers/MessageControllerTest.java | 23 ++------- 5 files changed, 18 insertions(+), 101 deletions(-) delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/spam/ReportSpamTokenHandler.java diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 3f7409394..79141fae2 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -70,11 +70,6 @@ import org.signal.libsignal.zkgroup.receipts.ServerZkReceiptOperations; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.whispersystems.dispatch.DispatchManager; -import org.whispersystems.textsecuregcm.spam.SpamFilter; -import org.whispersystems.textsecuregcm.spam.FilterSpam; -import org.whispersystems.textsecuregcm.spam.RateLimitChallengeListener; -import org.whispersystems.textsecuregcm.spam.ReportSpamTokenHandler; -import org.whispersystems.textsecuregcm.spam.ReportSpamTokenProvider; import org.whispersystems.textsecuregcm.auth.AccountAuthenticator; import org.whispersystems.textsecuregcm.auth.AuthenticatedAccount; import org.whispersystems.textsecuregcm.auth.CertificateGenerator; @@ -168,6 +163,10 @@ import org.whispersystems.textsecuregcm.s3.PolicySigner; import org.whispersystems.textsecuregcm.s3.PostPolicyGenerator; import org.whispersystems.textsecuregcm.securebackup.SecureBackupClient; import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient; +import org.whispersystems.textsecuregcm.spam.FilterSpam; +import org.whispersystems.textsecuregcm.spam.RateLimitChallengeListener; +import org.whispersystems.textsecuregcm.spam.ReportSpamTokenProvider; +import org.whispersystems.textsecuregcm.spam.SpamFilter; import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; import org.whispersystems.textsecuregcm.storage.AccountCleaner; import org.whispersystems.textsecuregcm.storage.AccountDatabaseCrawler; @@ -679,7 +678,6 @@ public class WhisperServerService extends Application commonControllers = Lists.newArrayList( new ArtController(rateLimiters, artCredentialsGenerator), new AttachmentControllerV2(rateLimiters, config.getAwsAttachmentsConfiguration().getAccessKey(), config.getAwsAttachmentsConfiguration().getAccessSecret(), config.getAwsAttachmentsConfiguration().getRegion(), config.getAwsAttachmentsConfiguration().getBucket()), @@ -744,7 +733,7 @@ public class WhisperServerService extends Application maybeSpamReportToken = spamReport != null ? Optional.of(spamReport.token()) : Optional.empty(); - // fire-and-forget: we don't want to block the response on this action. - CompletableFuture ignored = - reportSpamTokenHandler.handle(sourceNumber, sourceAci, sourcePni, messageGuid, spamReporterUuid, maybeSpamReportToken.orElse(null)); - reportMessageManager.report(sourceNumber, sourceAci, sourcePni, messageGuid, spamReporterUuid, maybeSpamReportToken); return Response.status(Status.ACCEPTED) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/spam/ReportSpamTokenHandler.java b/service/src/main/java/org/whispersystems/textsecuregcm/spam/ReportSpamTokenHandler.java deleted file mode 100644 index 1b53aa57d..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/spam/ReportSpamTokenHandler.java +++ /dev/null @@ -1,47 +0,0 @@ -package org.whispersystems.textsecuregcm.spam; - -import java.util.Optional; -import java.util.UUID; -import java.util.concurrent.CompletableFuture; - -/** - * Handles ReportSpamTokens during spam reports. - */ -public interface ReportSpamTokenHandler { - - /** - * Handle spam reports using the given ReportSpamToken and other provided parameters. - * - * @param reportSpamToken binary data representing a spam report token. - * @return true if the token could be handled (and was), false otherwise. - */ - CompletableFuture handle( - Optional sourceNumber, - Optional sourceAci, - Optional sourcePni, - UUID messageGuid, - UUID spamReporterUuid, - byte[] reportSpamToken); - - /** - * Handler which does nothing. - * - * @return the handler - */ - static ReportSpamTokenHandler noop() { - return new ReportSpamTokenHandler() { - @Override - public CompletableFuture handle( - final Optional sourceNumber, - final Optional sourceAci, - final Optional sourcePni, - final UUID messageGuid, - final UUID spamReporterUuid, - final byte[] reportSpamToken) { - return CompletableFuture.completedFuture(false); - } - }; - } - - -} 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 7c711fb05..2591f0ae3 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/spam/SpamFilter.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/spam/SpamFilter.java @@ -6,8 +6,10 @@ package org.whispersystems.textsecuregcm.spam; import io.dropwizard.lifecycle.Managed; +import org.whispersystems.textsecuregcm.storage.ReportedMessageListener; import javax.ws.rs.container.ContainerRequestFilter; import java.io.IOException; +import java.util.List; /** * A spam filter is a {@link ContainerRequestFilter} that filters requests to message-sending endpoints to @@ -39,10 +41,10 @@ public interface SpamFilter extends ContainerRequestFilter, Managed { ReportSpamTokenProvider getReportSpamTokenProvider(); /** - * Builds a spam report token handler. This will handle tokens received by the spam reporting system. + * Return any and all reported message listeners controlled by the spam filter. Listeners will be registered with the + * {@link org.whispersystems.textsecuregcm.storage.ReportMessageManager}. * - * @return the configured spam report token handler + * @return a list of reported message listeners controlled by the spam filter */ - ReportSpamTokenHandler getReportSpamTokenHandler(); - + List getReportedMessageListeners(); } 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 55eeb9fcd..b02cc0ead 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java @@ -8,7 +8,6 @@ package org.whispersystems.textsecuregcm.controllers; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; @@ -19,7 +18,6 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.anyBoolean; import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.atLeastOnce; -import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; @@ -68,9 +66,6 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.mockito.ArgumentCaptor; -import org.mockito.ArgumentMatcher; -import org.whispersystems.textsecuregcm.spam.ReportSpamTokenHandler; -import org.whispersystems.textsecuregcm.spam.ReportSpamTokenProvider; import org.whispersystems.textsecuregcm.auth.AuthenticatedAccount; import org.whispersystems.textsecuregcm.auth.DisabledPermittedAuthenticatedAccount; import org.whispersystems.textsecuregcm.auth.OptionalAccess; @@ -94,6 +89,7 @@ import org.whispersystems.textsecuregcm.providers.MultiRecipientMessageProvider; import org.whispersystems.textsecuregcm.push.MessageSender; import org.whispersystems.textsecuregcm.push.PushNotificationManager; import org.whispersystems.textsecuregcm.push.ReceiptSender; +import org.whispersystems.textsecuregcm.spam.ReportSpamTokenProvider; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.DeletedAccountsManager; @@ -144,7 +140,6 @@ class MessageControllerTest { private static final PushNotificationManager pushNotificationManager = mock(PushNotificationManager.class); private static final ReportMessageManager reportMessageManager = mock(ReportMessageManager.class); private static final ExecutorService multiRecipientMessageExecutor = mock(ExecutorService.class); - private static final ReportSpamTokenHandler REPORT_SPAM_TOKEN_HANDLER = mock(ReportSpamTokenHandler.class); private static final ResourceExtension resources = ResourceExtension.builder() .addProperty(ServerProperties.UNWRAP_COMPLETION_STAGE_IN_WRITER_ENABLE, Boolean.TRUE) @@ -157,7 +152,7 @@ class MessageControllerTest { .addResource( new MessageController(rateLimiters, messageSender, receiptSender, accountsManager, deletedAccountsManager, messagesManager, pushNotificationManager, reportMessageManager, multiRecipientMessageExecutor, - ReportSpamTokenProvider.noop(), REPORT_SPAM_TOKEN_HANDLER)) + ReportSpamTokenProvider.noop())) .build(); @BeforeEach @@ -184,8 +179,6 @@ class MessageControllerTest { when(accountsManager.getByAccountIdentifier(INTERNATIONAL_UUID)).thenReturn(Optional.of(internationalAccount)); when(rateLimiters.getMessagesLimiter()).thenReturn(rateLimiter); - - when(REPORT_SPAM_TOKEN_HANDLER.handle(any(), any(), any(), any(), any(), any())).thenReturn(CompletableFuture.completedFuture(false)); } private static Device generateTestDevice(final long id, final int registrationId, final int pniRegistrationId, final SignedPreKey signedPreKey, final long createdAt, final long lastSeen) { @@ -214,8 +207,7 @@ class MessageControllerTest { rateLimiter, pushNotificationManager, reportMessageManager, - multiRecipientMessageExecutor, - REPORT_SPAM_TOKEN_HANDLER + multiRecipientMessageExecutor ); } @@ -720,9 +712,6 @@ class MessageControllerTest { when(accountsManager.getByAccountIdentifier(senderAci)).thenReturn(Optional.of(account)); when(deletedAccountsManager.findDeletedAccountE164(senderAci)).thenReturn(Optional.of(senderNumber)); when(accountsManager.getPhoneNumberIdentifier(senderNumber)).thenReturn(senderPni); - when(REPORT_SPAM_TOKEN_HANDLER.handle(any(), any(), any(), any(), any(), any())).thenReturn(CompletableFuture.completedFuture(true)); - - ArgumentCaptor captor = ArgumentCaptor.forClass(byte[].class); Entity entity = Entity.entity(new SpamReport(new byte[3]), "application/json"); Response response = @@ -733,8 +722,6 @@ class MessageControllerTest { .post(entity); assertThat(response.getStatus(), is(equalTo(202))); - verify(REPORT_SPAM_TOKEN_HANDLER).handle(any(), any(), any(), any(), any(), captor.capture()); - assertArrayEquals(new byte[3], captor.getValue()); verify(reportMessageManager).report(eq(Optional.of(senderNumber)), eq(Optional.of(senderAci)), eq(Optional.of(senderPni)), @@ -745,8 +732,6 @@ class MessageControllerTest { verify(accountsManager, never()).getPhoneNumberIdentifier(anyString()); when(accountsManager.getByAccountIdentifier(senderAci)).thenReturn(Optional.empty()); - clearInvocations(REPORT_SPAM_TOKEN_HANDLER); - messageGuid = UUID.randomUUID(); entity = Entity.entity(new SpamReport(new byte[5]), "application/json"); @@ -758,8 +743,6 @@ class MessageControllerTest { .post(entity); assertThat(response.getStatus(), is(equalTo(202))); - verify(REPORT_SPAM_TOKEN_HANDLER).handle(any(), any(), any(), any(), any(), captor.capture()); - assertArrayEquals(new byte[5], captor.getValue()); verify(reportMessageManager).report(eq(Optional.of(senderNumber)), eq(Optional.of(senderAci)), eq(Optional.of(senderPni)),