Skip to content

Commit

Permalink
Count some failed attempts in SMS and voice ratelimits
Browse files Browse the repository at this point in the history
  • Loading branch information
@katherine-signal
katherine-signal authored Jul 12, 2024
1 parent 01abf5c commit 051283a
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 4 deletions.
7 changes: 6 additions & 1 deletion src/main/java/org/signal/registration/ratelimit/SendSmsVerificationCodeRateLimiter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.util.List;
import java.util.Optional;
import org.signal.registration.rpc.MessageTransport;
import org.signal.registration.session.FailedSendReason;
import org.signal.registration.session.RegistrationSession;

@Singleton
Expand All @@ -38,7 +39,11 @@ protected int getPriorAttemptCount(final RegistrationSession session) {

return (int) session.getRegistrationAttemptsList().stream()
.filter(attempt -> attempt.getMessageTransport() == MessageTransport.MESSAGE_TRANSPORT_SMS)
.count();
.count() +
(int) session.getFailedAttemptsList().stream()
.filter(attempt -> attempt.getMessageTransport() == MessageTransport.MESSAGE_TRANSPORT_SMS
&& attempt.getFailedSendReason() != FailedSendReason.FAILED_SEND_REASON_UNAVAILABLE)
.count();
}

@Override
Expand Down
7 changes: 6 additions & 1 deletion src/main/java/org/signal/registration/ratelimit/SendVoiceVerificationCodeRateLimiter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import jakarta.inject.Named;
import jakarta.inject.Singleton;
import org.signal.registration.rpc.MessageTransport;
import org.signal.registration.session.FailedSendReason;
import org.signal.registration.session.RegistrationSession;
import java.time.Clock;
import java.time.Duration;
Expand Down Expand Up @@ -73,7 +74,11 @@ protected int getPriorAttemptCount(final RegistrationSession session) {

return (int) session.getRegistrationAttemptsList().stream()
.filter(attempt -> attempt.getMessageTransport() == MessageTransport.MESSAGE_TRANSPORT_VOICE)
.count();
.count() +
(int) session.getFailedAttemptsList().stream()
.filter(attempt -> attempt.getMessageTransport() == MessageTransport.MESSAGE_TRANSPORT_VOICE
&& attempt.getFailedSendReason() != FailedSendReason.FAILED_SEND_REASON_UNAVAILABLE)
.count();
}

@Override
Expand Down
56 changes: 55 additions & 1 deletion src/test/java/org/signal/registration/ratelimit/SendSmsVerificationCodeRateLimiterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

import org.junit.jupiter.api.Test;
import org.signal.registration.rpc.MessageTransport;
import org.signal.registration.session.FailedSendAttempt;
import org.signal.registration.session.FailedSendReason;
import org.signal.registration.session.RegistrationAttempt;
import org.signal.registration.session.RegistrationSession;

Expand Down Expand Up @@ -34,6 +36,38 @@ void getPriorAttemptCount() {
.build())
.build()));

assertEquals(0, rateLimiter.getPriorAttemptCount(RegistrationSession.newBuilder()
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_SUSPECTED_FRAUD)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.build()));

assertEquals(0, rateLimiter.getPriorAttemptCount(RegistrationSession.newBuilder()
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_UNAVAILABLE)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.build()));

assertEquals(1, rateLimiter.getPriorAttemptCount(RegistrationSession.newBuilder()
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_SUSPECTED_FRAUD)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.build()));

assertEquals(1, rateLimiter.getPriorAttemptCount(RegistrationSession.newBuilder()
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_REJECTED)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.build()));

assertEquals(1, rateLimiter.getPriorAttemptCount(RegistrationSession.newBuilder()
.addRegistrationAttempts(RegistrationAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
Expand All @@ -50,9 +84,19 @@ void getPriorAttemptCount() {
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_REJECTED)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_UNAVAILABLE)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.build()));

assertEquals(2, rateLimiter.getPriorAttemptCount(RegistrationSession.newBuilder()
assertEquals(4, rateLimiter.getPriorAttemptCount(RegistrationSession.newBuilder()
.addRegistrationAttempts(RegistrationAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setTimestampEpochMillis(System.currentTimeMillis())
Expand All @@ -61,6 +105,16 @@ void getPriorAttemptCount() {
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_REJECTED)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_SUSPECTED_FRAUD)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.build()));
}

Expand Down
56 changes: 55 additions & 1 deletion ...test/java/org/signal/registration/ratelimit/SendVoiceVerificationCodeRateLimiterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

import org.junit.jupiter.api.Test;
import org.signal.registration.rpc.MessageTransport;
import org.signal.registration.session.FailedSendAttempt;
import org.signal.registration.session.FailedSendReason;
import org.signal.registration.session.RegistrationAttempt;
import org.signal.registration.session.RegistrationSession;

Expand Down Expand Up @@ -125,6 +127,38 @@ void getPriorAttemptCount() {
.build())
.build()));

assertEquals(0, rateLimiter.getPriorAttemptCount(RegistrationSession.newBuilder()
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_SUSPECTED_FRAUD)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.build()));

assertEquals(0, rateLimiter.getPriorAttemptCount(RegistrationSession.newBuilder()
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_UNAVAILABLE)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.build()));

assertEquals(1, rateLimiter.getPriorAttemptCount(RegistrationSession.newBuilder()
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_SUSPECTED_FRAUD)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.build()));

assertEquals(1, rateLimiter.getPriorAttemptCount(RegistrationSession.newBuilder()
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_REJECTED)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.build()));

assertEquals(1, rateLimiter.getPriorAttemptCount(RegistrationSession.newBuilder()
.addRegistrationAttempts(RegistrationAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
Expand All @@ -141,9 +175,19 @@ void getPriorAttemptCount() {
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_REJECTED)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_UNAVAILABLE)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.build()));

assertEquals(2, rateLimiter.getPriorAttemptCount(RegistrationSession.newBuilder()
assertEquals(4, rateLimiter.getPriorAttemptCount(RegistrationSession.newBuilder()
.addRegistrationAttempts(RegistrationAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setTimestampEpochMillis(System.currentTimeMillis())
Expand All @@ -152,6 +196,16 @@ void getPriorAttemptCount() {
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_REJECTED)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_SUSPECTED_FRAUD)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.build()));
}

Expand Down

0 comments on commit 051283a

Please sign in to comment.