Merge pull request #668 from DwayneJengSage/dev3

patch: exempt api, api-2, and shared from create participant rate limiting
Sage-Bionetworks · Jul 18, 2023 · 131e454 · 131e454
2 parents 183fcf0 + e4c780a
commit 131e454
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 9 deletions.
diff --git a/src/main/java/org/sagebionetworks/bridge/services/ParticipantService.java b/src/main/java/org/sagebionetworks/bridge/services/ParticipantService.java
@@ -40,6 +40,7 @@
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
@@ -126,6 +127,8 @@ public class ParticipantService {
 
     private static final String CREATE_PARTICIPANT_RATE_LIMIT_ERROR =
             "You cannot create more than 3 accounts per 5 minutes";
+    private static final Set<String> CREATE_PARTICIPANT_RATE_LIMIT_EXEMPT_APP_IDS = ImmutableSet.of(
+            BridgeConstants.API_APP_ID, BridgeConstants.API_2_APP_ID, BridgeConstants.SHARED_APP_ID);
     static final String REQUEST_KEY_BODY = "body";
     static final String REQUEST_KEY_SERVICE = "service";
     static final String REQUEST_KEY_APP_ID = "appId";
@@ -397,10 +400,9 @@ public IdentifierHolder createParticipant(App app, StudyParticipant participant,
         // 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.
-        // Note: Don't rate limit for superadmin accounts.
         RequestContext requestContext = RequestContext.get();
         String userId = requestContext.getCallerUserId();
-        if (!requestContext.isInRole(Roles.SUPERADMIN) && userId != null) {
+        if (!CREATE_PARTICIPANT_RATE_LIMIT_EXEMPT_APP_IDS.contains(app.getIdentifier()) && userId != null) {
             ByteRateLimiter rateLimiter = createParticipantRateLimiters.computeIfAbsent(userId,
                     (u) -> createParticipantRateLimiter());
             if (!rateLimiter.tryConsumeBytes(1)) {

diff --git a/src/test/java/org/sagebionetworks/bridge/services/ParticipantServiceTest.java b/src/test/java/org/sagebionetworks/bridge/services/ParticipantServiceTest.java
@@ -59,6 +59,8 @@
 import org.mockito.Captor;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
+
+import org.sagebionetworks.bridge.BridgeConstants;
 import org.sagebionetworks.bridge.config.BridgeConfig;
 import org.sagebionetworks.bridge.models.ParticipantRosterRequest;
 import org.testng.annotations.AfterMethod;
@@ -430,16 +432,37 @@ public void createParticipant_RateLimiting() {
             // expected exception
         }
 
-        // Use a superadmin account with a different User ID. This should not be rate limited.
+        // Don't need to test restocking the Rate Limiter. This is tested in ByteRateLimiterTest.
+    }
+
+    @Test
+    public void createParticipant_RateLimiting_ExemptAppId() {
+        // 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("superadmin-user").withCallerAppId(TEST_APP_ID)
-                .withCallerRoles(ImmutableSet.of(Roles.SUPERADMIN)).build());
+                .withCallerUserId("api-2-user").withCallerAppId(BridgeConstants.API_2_APP_ID)
+                .withCallerRoles(ImmutableSet.of(Roles.RESEARCHER)).build());
 
-        // Can create multiple participants without rate limiting.
-        participantService.createParticipant(APP, participant, false);
-        participantService.createParticipant(APP, participant, false);
+        // Mock dependencies.
+        when(studyService.getStudy(BridgeConstants.API_2_APP_ID, STUDY_ID, false)).thenReturn(
+                Study.create());
 
-        // Don't need to test restocking the Rate Limiter. This is tested in ByteRateLimiterTest.
+        // Make an app in api-2.
+        App app = App.create();
+        app.setIdentifier(BridgeConstants.API_2_APP_ID);
+        app.setDataGroups(APP_DATA_GROUPS);
+        app.setPasswordPolicy(PasswordPolicy.DEFAULT_PASSWORD_POLICY);
+        app.getUserProfileAttributes().add("can_be_recontacted");
+
+        // Can create multiple participants without rate limiting.
+        StudyParticipant participant = withParticipant().build();
+        participantService.createParticipant(app, participant, false);
+        participantService.createParticipant(app, participant, false);
     }
 
     @Test(expectedExceptions = InvalidEntityException.class)