From c00251518afd4621b0a40a4638d7284341b81a02 Mon Sep 17 00:00:00 2001 From: Dwayne Jeng Date: Mon, 17 Jul 2023 03:34:25 -0700 Subject: [PATCH] DHP-968 Rate Limiting on Create Participant --- .../bridge/services/ParticipantService.java | 40 ++++++++++++++-- src/main/resources/BridgeServer2.conf | 12 +++++ .../services/ParticipantServiceTest.java | 48 +++++++++++++++++-- 3 files changed, 92 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/sagebionetworks/bridge/services/ParticipantService.java b/src/main/java/org/sagebionetworks/bridge/services/ParticipantService.java index 4089e2523..f6e78d5da 100644 --- a/src/main/java/org/sagebionetworks/bridge/services/ParticipantService.java +++ b/src/main/java/org/sagebionetworks/bridge/services/ParticipantService.java @@ -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; @@ -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; @@ -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"; @@ -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 createParticipantRateLimiters = new ConcurrentHashMap<>(); + // Accessor so we can mock the value protected DateTime getInstallDateTime() { return new DateTime(); @@ -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); } @@ -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(); diff --git a/src/main/resources/BridgeServer2.conf b/src/main/resources/BridgeServer2.conf index 8120d1b52..2bfd6551d 100644 --- a/src/main/resources/BridgeServer2.conf +++ b/src/main/resources/BridgeServer2.conf @@ -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 diff --git a/src/test/java/org/sagebionetworks/bridge/services/ParticipantServiceTest.java b/src/test/java/org/sagebionetworks/bridge/services/ParticipantServiceTest.java index ef67e4fad..b8acd157a 100644 --- a/src/test/java/org/sagebionetworks/bridge/services/ParticipantServiceTest.java +++ b/src/test/java/org/sagebionetworks/bridge/services/ParticipantServiceTest.java @@ -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; @@ -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"); @@ -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()); } @@ -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();