Skip to content

Commit

Permalink
Include failed attempts when calculating last attempt time
Browse files Browse the repository at this point in the history
  • Loading branch information
@katherine-signal
katherine-signal authored Jul 12, 2024
1 parent 051283a commit 2896d8f
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 11 deletions.
20 changes: 14 additions & 6 deletions src/main/java/org/signal/registration/ratelimit/SendSmsVerificationCodeRateLimiter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
import java.util.stream.Stream;

import org.signal.registration.rpc.MessageTransport;
import org.signal.registration.session.FailedSendReason;
import org.signal.registration.session.RegistrationSession;
Expand Down Expand Up @@ -41,16 +43,22 @@ protected int getPriorAttemptCount(final RegistrationSession session) {
.filter(attempt -> attempt.getMessageTransport() == MessageTransport.MESSAGE_TRANSPORT_SMS)
.count() +
(int) session.getFailedAttemptsList().stream()
.filter(attempt -> attempt.getMessageTransport() == MessageTransport.MESSAGE_TRANSPORT_SMS
&& attempt.getFailedSendReason() != FailedSendReason.FAILED_SEND_REASON_UNAVAILABLE)
.filter(attempt -> attempt.getMessageTransport() == MessageTransport.MESSAGE_TRANSPORT_SMS)
.filter(attempt -> attempt.getFailedSendReason() != FailedSendReason.FAILED_SEND_REASON_UNAVAILABLE)
.count();
}

@Override
protected Optional<Instant> getLastAttemptTime(final RegistrationSession session) {
return session.getRegistrationAttemptsList().stream()
.filter(attempt -> attempt.getMessageTransport() == MessageTransport.MESSAGE_TRANSPORT_SMS)
.map(attempt -> Instant.ofEpochMilli(attempt.getTimestampEpochMillis()))
.max(Comparator.naturalOrder());
return Stream.concat(
session.getRegistrationAttemptsList().stream()
.filter(attempt -> attempt.getMessageTransport() == MessageTransport.MESSAGE_TRANSPORT_SMS)
.map(attempt -> Instant.ofEpochMilli(attempt.getTimestampEpochMillis())),
session.getFailedAttemptsList().stream()
.filter(attempt -> attempt.getMessageTransport() == MessageTransport.MESSAGE_TRANSPORT_SMS)
.filter(attempt -> attempt.getFailedSendReason() != FailedSendReason.FAILED_SEND_REASON_UNAVAILABLE)
.map(attempt -> Instant.ofEpochMilli(attempt.getTimestampEpochMillis()))
)
.max(Comparator.naturalOrder());
}
}
16 changes: 11 additions & 5 deletions src/main/java/org/signal/registration/ratelimit/SendVoiceVerificationCodeRateLimiter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.List;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.stream.Stream;

@Singleton
@Named("send-voice-verification-code")
Expand Down Expand Up @@ -76,16 +77,21 @@ protected int getPriorAttemptCount(final RegistrationSession session) {
.filter(attempt -> attempt.getMessageTransport() == MessageTransport.MESSAGE_TRANSPORT_VOICE)
.count() +
(int) session.getFailedAttemptsList().stream()
.filter(attempt -> attempt.getMessageTransport() == MessageTransport.MESSAGE_TRANSPORT_VOICE
&& attempt.getFailedSendReason() != FailedSendReason.FAILED_SEND_REASON_UNAVAILABLE)
.filter(attempt -> attempt.getMessageTransport() == MessageTransport.MESSAGE_TRANSPORT_VOICE)
.filter(attempt -> attempt.getFailedSendReason() != FailedSendReason.FAILED_SEND_REASON_UNAVAILABLE)
.count();
}

@Override
protected Optional<Instant> getLastAttemptTime(final RegistrationSession session) {
return session.getRegistrationAttemptsList().stream()
.filter(attempt -> attempt.getMessageTransport() == MessageTransport.MESSAGE_TRANSPORT_VOICE)
.map(attempt -> Instant.ofEpochMilli(attempt.getTimestampEpochMillis()))
return Stream.concat(
session.getRegistrationAttemptsList().stream()
.filter(attempt -> attempt.getMessageTransport() == MessageTransport.MESSAGE_TRANSPORT_VOICE)
.map(attempt -> Instant.ofEpochMilli(attempt.getTimestampEpochMillis())),
session.getFailedAttemptsList().stream()
.filter(attempt -> attempt.getMessageTransport() == MessageTransport.MESSAGE_TRANSPORT_VOICE)
.filter(attempt -> attempt.getFailedSendReason() != FailedSendReason.FAILED_SEND_REASON_UNAVAILABLE)
.map(attempt -> Instant.ofEpochMilli(attempt.getTimestampEpochMillis())))
.max(Comparator.naturalOrder());
}
}
55 changes: 55 additions & 0 deletions src/test/java/org/signal/registration/ratelimit/SendSmsVerificationCodeRateLimiterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,16 @@ void getLastAttemptTime() {
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_UNAVAILABLE)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_REJECTED)
.setTimestampEpochMillis(System.currentTimeMillis())
.build())
.build()));

final long firstTimestamp = 37;
Expand All @@ -143,6 +153,15 @@ void getLastAttemptTime() {
.build())
.build()));

assertEquals(Optional.of(Instant.ofEpochMilli(firstTimestamp)),
rateLimiter.getLastAttemptTime(RegistrationSession.newBuilder()
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_REJECTED)
.setTimestampEpochMillis(firstTimestamp)
.build())
.build()));

assertEquals(Optional.of(Instant.ofEpochMilli(firstTimestamp)),
rateLimiter.getLastAttemptTime(RegistrationSession.newBuilder()
.addRegistrationAttempts(RegistrationAttempt.newBuilder()
Expand All @@ -153,6 +172,29 @@ void getLastAttemptTime() {
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setTimestampEpochMillis(secondTimestamp)
.build())
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_UNAVAILABLE)
.setTimestampEpochMillis(secondTimestamp)
.build())
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_REJECTED)
.setTimestampEpochMillis(secondTimestamp)
.build())
.build()));

assertEquals(Optional.of(Instant.ofEpochMilli(firstTimestamp)),
rateLimiter.getLastAttemptTime(RegistrationSession.newBuilder()
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_REJECTED)
.setTimestampEpochMillis(firstTimestamp)
.build())
.addRegistrationAttempts(RegistrationAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setTimestampEpochMillis(secondTimestamp)
.build())
.build()));

assertEquals(Optional.of(Instant.ofEpochMilli(secondTimestamp)),
Expand All @@ -166,6 +208,19 @@ void getLastAttemptTime() {
.setTimestampEpochMillis(secondTimestamp)
.build())
.build()));

assertEquals(Optional.of(Instant.ofEpochMilli(secondTimestamp)),
rateLimiter.getLastAttemptTime(RegistrationSession.newBuilder()
.addRegistrationAttempts(RegistrationAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setTimestampEpochMillis(firstTimestamp)
.build())
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_REJECTED)
.setTimestampEpochMillis(secondTimestamp)
.build())
.build()));
}

@Test
Expand Down
55 changes: 55 additions & 0 deletions ...test/java/org/signal/registration/ratelimit/SendVoiceVerificationCodeRateLimiterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,16 @@ void getLastAttemptTime() {
.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()));

final long firstTimestamp = 37;
Expand All @@ -234,6 +244,15 @@ void getLastAttemptTime() {
.build())
.build()));

assertEquals(Optional.of(Instant.ofEpochMilli(firstTimestamp)),
rateLimiter.getLastAttemptTime(RegistrationSession.newBuilder()
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_REJECTED)
.setTimestampEpochMillis(firstTimestamp)
.build())
.build()));

assertEquals(Optional.of(Instant.ofEpochMilli(firstTimestamp)),
rateLimiter.getLastAttemptTime(RegistrationSession.newBuilder()
.addRegistrationAttempts(RegistrationAttempt.newBuilder()
Expand All @@ -244,6 +263,29 @@ void getLastAttemptTime() {
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setTimestampEpochMillis(secondTimestamp)
.build())
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_UNAVAILABLE)
.setTimestampEpochMillis(secondTimestamp)
.build())
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_REJECTED)
.setTimestampEpochMillis(secondTimestamp)
.build())
.build()));

assertEquals(Optional.of(Instant.ofEpochMilli(firstTimestamp)),
rateLimiter.getLastAttemptTime(RegistrationSession.newBuilder()
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_REJECTED)
.setTimestampEpochMillis(firstTimestamp)
.build())
.addRegistrationAttempts(RegistrationAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_SMS)
.setTimestampEpochMillis(secondTimestamp)
.build())
.build()));

assertEquals(Optional.of(Instant.ofEpochMilli(secondTimestamp)),
Expand All @@ -257,5 +299,18 @@ void getLastAttemptTime() {
.setTimestampEpochMillis(secondTimestamp)
.build())
.build()));

assertEquals(Optional.of(Instant.ofEpochMilli(secondTimestamp)),
rateLimiter.getLastAttemptTime(RegistrationSession.newBuilder()
.addRegistrationAttempts(RegistrationAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setTimestampEpochMillis(firstTimestamp)
.build())
.addFailedAttempts(FailedSendAttempt.newBuilder()
.setMessageTransport(MessageTransport.MESSAGE_TRANSPORT_VOICE)
.setFailedSendReason(FailedSendReason.FAILED_SEND_REASON_REJECTED)
.setTimestampEpochMillis(secondTimestamp)
.build())
.build()));
}
}

0 comments on commit 2896d8f

Please sign in to comment.