Skip to content

Commit

Permalink
Merge pull request #665 from DwayneJengSage/dev3
Browse files Browse the repository at this point in the history
DHP-968 Rate Limiting on Create Participant
  • Loading branch information
DwayneJengSage authored Jul 17, 2023
2 parents 17a9258 + c002515 commit a0b93ea
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;

import com.amazonaws.services.sqs.AmazonSQS;
import com.amazonaws.services.sqs.model.SendMessageResult;
Expand All @@ -48,6 +49,7 @@
import org.sagebionetworks.bridge.config.BridgeConfig;
import org.sagebionetworks.bridge.json.BridgeObjectMapper;
import org.sagebionetworks.bridge.models.ParticipantRosterRequest;
import org.sagebionetworks.bridge.util.ByteRateLimiter;
import org.sagebionetworks.bridge.validators.ParticipantRosterRequestValidator;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -113,6 +115,17 @@ public class ParticipantService {
static final String NO_INSTALL_LINKS_ERROR = "No install links configured for app.";
static final String ACCOUNT_UNABLE_TO_BE_CONTACTED_ERROR = "Account unable to be contacted via phone or email";
static final String CONFIG_KEY_DOWNLOAD_ROSTER_SQS_URL = "workerPlatform.request.sqs.queue.url";
static final String CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_INITIAL_COUNT =
"create-participant.rate-limiter.initial-count";
static final String CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_MAXIMUM_COUNT =
"create-participant.rate-limiter.maximum-count";
static final String CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_REFILL_INTERVAL =
"create-participant.rate-limiter.refill-interval-seconds";
static final String CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_REFILL_COUNT =
"create-participant.rate-limiter.refill-count";

private static final String CREATE_PARTICIPANT_RATE_LIMIT_ERROR =
"You cannot create more than 3 accounts per 5 minutes";
static final String REQUEST_KEY_BODY = "body";
static final String REQUEST_KEY_SERVICE = "service";
static final String REQUEST_KEY_APP_ID = "appId";
Expand Down Expand Up @@ -159,6 +172,9 @@ public class ParticipantService {
@Autowired
private SendMailService sendMailService;

// These are byte rate limiters, but we can use them as count limiters. Key is the user ID of the caller.
private final Map<String, ByteRateLimiter> createParticipantRateLimiters = new ConcurrentHashMap<>();

// Accessor so we can mock the value
protected DateTime getInstallDateTime() {
return new DateTime();
Expand Down Expand Up @@ -378,12 +394,18 @@ public IdentifierHolder createParticipant(App app, StudyParticipant participant,
checkNotNull(app);
checkNotNull(participant);

// https://sagebionetworks.jira.com/browse/DHP-979
// Temporarily disable creating participants for mobile-toolbox.
if (app.getIdentifier().equals("mobile-toolbox")) {
throw new LimitExceededException("You are creating accounts too quickly. Please wait a while and try again later.");
// https://sagebionetworks.jira.com/browse/DHP-968 - Rate limiting.
// Note that it's possible for the RequestContext to not have a User ID. This is common for sign-up calls.
// In that case, we don't rate limit.
String userId = RequestContext.get().getCallerUserId();
if (userId != null) {
ByteRateLimiter rateLimiter = createParticipantRateLimiters.computeIfAbsent(userId,
(u) -> createParticipantRateLimiter());
if (!rateLimiter.tryConsumeBytes(1)) {
throw new LimitExceededException(CREATE_PARTICIPANT_RATE_LIMIT_ERROR);
}
}

if (app.getAccountLimit() > 0) {
throwExceptionIfLimitMetOrExceeded(app);
}
Expand Down Expand Up @@ -463,6 +485,14 @@ public IdentifierHolder createParticipant(App app, StudyParticipant participant,
return new IdentifierHolder(account.getId());
}

// Creates and returns a RateLimiter with different settings depending on the environment.
private ByteRateLimiter createParticipantRateLimiter() {
return new ByteRateLimiter(bridgeConfig.getInt(CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_INITIAL_COUNT),
bridgeConfig.getInt(CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_MAXIMUM_COUNT),
bridgeConfig.getInt(CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_REFILL_INTERVAL),
bridgeConfig.getInt(CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_REFILL_COUNT));
}

// Provided to override in tests
protected Account getAccount() {
return Account.create();
Expand Down
12 changes: 12 additions & 0 deletions src/main/resources/BridgeServer2.conf
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,18 @@ query.param.allowlist = type,appId,studyId,IdFilter,assignmentFilter,externalId,
# Participant File S3 bucket name
participant-file.bucket = org-sagebridge-participantfile-${bucket.suffix}

# Create Participant rate limiting production constants (3 every 5 minutes)
prod.create-participant.rate-limiter.initial-count = 3
prod.create-participant.rate-limiter.maximum-count = 3
prod.create-participant.rate-limiter.refill-interval-seconds = 100
prod.create-participant.rate-limiter.refill-count = 1

# Create Participant rate limiting non-production constants (10 per second with a max of 100)
create-participant.rate-limiter.initial-count = 100
create-participant.rate-limiter.maximum-count = 100
create-participant.rate-limiter.refill-interval-seconds = 1
create-participant.rate-limiter.refill-count = 10

# Participant file rate limiting production constants
# 1 MB
prod.participant-file.rate-limiter.initial-bytes = 1000000
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@
import static org.sagebionetworks.bridge.models.templates.TemplateType.SMS_APP_INSTALL_LINK;
import static org.sagebionetworks.bridge.services.ParticipantService.ACCOUNT_UNABLE_TO_BE_CONTACTED_ERROR;
import static org.sagebionetworks.bridge.services.ParticipantService.APP_INSTALL_URL_KEY;
import static org.sagebionetworks.bridge.services.ParticipantService.CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_INITIAL_COUNT;
import static org.sagebionetworks.bridge.services.ParticipantService.CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_MAXIMUM_COUNT;
import static org.sagebionetworks.bridge.services.ParticipantService.CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_REFILL_COUNT;
import static org.sagebionetworks.bridge.services.ParticipantService.CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_REFILL_INTERVAL;
import static org.sagebionetworks.bridge.services.ParticipantService.NO_INSTALL_LINKS_ERROR;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
Expand Down Expand Up @@ -121,6 +125,7 @@

public class ParticipantServiceTest extends Mockito {
private static final DateTime ACTIVITIES_RETRIEVED_DATETIME = DateTime.parse("2019-08-01T18:32:36.487-0700");
private static final String ADMIN_USER_ID = "admin-user-id";
private static final ClientInfo CLIENT_INFO = new ClientInfo.Builder().withAppName("unit test")
.withAppVersion(4).build();
private static final DateTime CREATED_ON_DATETIME = DateTime.parse("2019-07-30T12:09:28.184-0700");
Expand Down Expand Up @@ -296,9 +301,15 @@ public void before() {
account = Account.create();
account.setAppId(TEST_APP_ID);
account.setHealthCode(HEALTH_CODE);


// Set rate limiting to something that won't impact tests.
when(bridgeConfig.getInt(CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_INITIAL_COUNT)).thenReturn(100);
when(bridgeConfig.getInt(CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_MAXIMUM_COUNT)).thenReturn(100);
when(bridgeConfig.getInt(CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_REFILL_INTERVAL)).thenReturn(1);
when(bridgeConfig.getInt(CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_REFILL_COUNT)).thenReturn(100);

RequestContext.set(new RequestContext.Builder()
.withCallerUserId("id").withCallerAppId(TEST_APP_ID)
.withCallerUserId(ADMIN_USER_ID).withCallerAppId(TEST_APP_ID)
.withCallerRoles(ImmutableSet.of(Roles.RESEARCHER))
.withOrgSponsoredStudies(CALLER_SUBS).build());
}
Expand Down Expand Up @@ -390,7 +401,38 @@ public void createParticipant() {
// don't update cache
Mockito.verifyNoMoreInteractions(cacheProvider);
}


@Test
public void createParticipant_RateLimiting() {
// Set rate limit to 1 request for all time.
when(bridgeConfig.getInt(CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_INITIAL_COUNT)).thenReturn(1);
when(bridgeConfig.getInt(CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_MAXIMUM_COUNT)).thenReturn(1);
when(bridgeConfig.getInt(CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_REFILL_INTERVAL)).thenReturn(1000);
when(bridgeConfig.getInt(CONFIG_KEY_CREATE_PARTICIPANT_RATE_LIMIT_REFILL_COUNT)).thenReturn(1);

// Set a unique caller ID, so we don't conflict with other tests.
RequestContext.set(new RequestContext.Builder()
.withCallerUserId("rate-limiting-user").withCallerAppId(TEST_APP_ID)
.withCallerRoles(ImmutableSet.of(Roles.RESEARCHER)).build());

// Mock dependencies.
when(studyService.getStudy(TEST_APP_ID, STUDY_ID, false)).thenReturn(Study.create());

// First call succeeds.
StudyParticipant participant = withParticipant().build();
participantService.createParticipant(APP, participant, false);

// Second call throws.
try {
participantService.createParticipant(APP, participant, false);
fail("expected exception");
} catch (LimitExceededException ex) {
// expected exception
}

// Don't need to test restocking the Rate Limiter. This is tested in ByteRateLimiterTest.
}

@Test(expectedExceptions = InvalidEntityException.class)
public void createParticipantDoesNotAlreadyExistThrowsInvalidEntity() {
mockHealthCodeAndAccountRetrieval();
Expand Down

0 comments on commit a0b93ea

Please sign in to comment.