Skip to content

Commit d87303e

Browse files
nsgupta1nsgupta1
committed
Addressing minor comments
Signed-off-by: ngsupta1 <guptaneha.e@gmail.com>
1 parent cf2f975 commit d87303e

9 files changed

Lines changed: 79 additions & 64 deletions

File tree

‎data-prepper-plugins/saas-source-plugins/crowdstrike-source/src/main/java/org/opensearch/dataprepper/plugins/source/crowdstrike/CrowdStrikeService.java‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package org.opensearch.dataprepper.plugins.source.crowdstrike;
22
import io.micrometer.core.instrument.Timer;
33
import org.opensearch.dataprepper.metrics.PluginMetrics;
4-
import org.opensearch.dataprepper.plugins.source.crowdstrike.models.CrowdStrikeApiResponse;
4+
import org.opensearch.dataprepper.plugins.source.crowdstrike.models.CrowdStrikeThreatIntelApiResponse;
55
import org.opensearch.dataprepper.plugins.source.crowdstrike.models.CrowdStrikeIndicatorResult;
66
import org.opensearch.dataprepper.plugins.source.crowdstrike.rest.CrowdStrikeRestClient;
77
import org.opensearch.dataprepper.plugins.source.crowdstrike.utils.CrowdStrikeNextLinkValidator;
@@ -33,6 +33,9 @@ public class CrowdStrikeService {
3333
private static final String BASE_URL = "https://api.crowdstrike.com/";
3434
private static final String COMBINED_URL = "https://api.crowdstrike.com/intel/combined/indicators/v1";
3535
private final Timer searchCallLatencyTimer;
36+
private static final String GREATER_THAN_EQUALS = ">=";
37+
private static final String LESS_THAN = "<";
38+
private static final String ENCODED_PLUS_SIGN = "%2B";
3639

3740

3841
public CrowdStrikeService(CrowdStrikeRestClient crowdStrikeRestClient, PluginMetrics pluginMetrics) {
@@ -45,9 +48,9 @@ public CrowdStrikeService(CrowdStrikeRestClient crowdStrikeRestClient, PluginMet
4548
* @param startTime The start timestamp (inclusive) for filtering indicators.
4649
* @param endTime The end timestamp (exclusive) for filtering indicators.
4750
* @param paginationLink An optional pagination URL suffix (used when fetching next pages).
48-
* @return A {@link CrowdStrikeApiResponse} containing response body and headers.
51+
* @return A {@link CrowdStrikeThreatIntelApiResponse} containing response body and headers.
4952
*/
50-
public CrowdStrikeApiResponse getThreatIndicators(Instant startTime, Instant endTime, Optional<String> paginationLink) {
53+
public CrowdStrikeThreatIntelApiResponse getThreatIndicators(Instant startTime, Instant endTime, Optional<String> paginationLink) {
5154
if (startTime == null || endTime == null) {
5255
throw new IllegalArgumentException("startTime and endTime must not be null");
5356
}
@@ -57,7 +60,7 @@ public CrowdStrikeApiResponse getThreatIndicators(Instant startTime, Instant end
5760
log.debug("Calling CrowdStrike API with URI: {}", uri);
5861
ResponseEntity<CrowdStrikeIndicatorResult> responseEntity = crowdStrikeRestClient.invokeGetApi(uri, CrowdStrikeIndicatorResult.class);
5962

60-
return new CrowdStrikeApiResponse(responseEntity.getBody(), responseEntity.getHeaders());
63+
return new CrowdStrikeThreatIntelApiResponse(responseEntity.getBody(), responseEntity.getHeaders());
6164
});
6265
}
6366

@@ -69,9 +72,9 @@ protected URI buildCrowdStrikeUri(Instant startTime, Instant endTime, Optional<
6972
return new URI(urlString);
7073
} else {
7174
// Manually construct and encode the query string
72-
String filter1 = URLEncoder.encode(LAST_UPDATED + ":>=" + startTime.getEpochSecond(), StandardCharsets.UTF_8);
73-
String filter2 = URLEncoder.encode(LAST_UPDATED + ":<" + endTime.getEpochSecond(), StandardCharsets.UTF_8);
74-
String encodedFilter = filter1 + "%2B" + filter2; // Use literal '+' // ensure literal '+'
75+
String startTimeFilter = URLEncoder.encode(LAST_UPDATED + ":" + GREATER_THAN_EQUALS + startTime.getEpochSecond(), StandardCharsets.UTF_8);
76+
String endTimeFilter = URLEncoder.encode(LAST_UPDATED + ":" + LESS_THAN + endTime.getEpochSecond(), StandardCharsets.UTF_8);
77+
String encodedFilter = startTimeFilter + ENCODED_PLUS_SIGN + endTimeFilter; // ensure literal '+' is encoded
7578

7679
UriComponentsBuilder builder = UriComponentsBuilder
7780
.fromHttpUrl(COMBINED_URL)

‎data-prepper-plugins/saas-source-plugins/crowdstrike-source/src/main/java/org/opensearch/dataprepper/plugins/source/crowdstrike/models/CrowdStrikeIndicatorResult.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,16 @@
22

33
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
44
import com.fasterxml.jackson.annotation.JsonProperty;
5+
import lombok.Data;
56
import lombok.Getter;
67
import lombok.Setter;
78

89
import java.util.List;
910

1011
/**
11-
* The result of Falcon query search.
12+
* Represents the response from a CrowdStrike Falcon Query Language (FQL) search for threat indicators.
1213
*/
13-
@Getter
14-
@Setter
14+
@Data
1515
@JsonIgnoreProperties(ignoreUnknown = true)
1616
public class CrowdStrikeIndicatorResult {
1717

data-prepper-plugins/saas-source-plugins/crowdstrike-source/src/main/java/org/opensearch/dataprepper/plugins/source/crowdstrike/models/CrowdStrikeApiResponse.java renamed to data-prepper-plugins/saas-source-plugins/crowdstrike-source/src/main/java/org/opensearch/dataprepper/plugins/source/crowdstrike/models/CrowdStrikeThreatIntelApiResponse.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package org.opensearch.dataprepper.plugins.source.crowdstrike.models;
22

3-
import lombok.Getter;
4-
import lombok.Setter;
3+
import lombok.Data;
54
import org.springframework.util.CollectionUtils;
65
import java.util.Collections;
76
import java.util.List;
@@ -10,13 +9,13 @@
109
/**
1110
* Represents the response returned from a CrowdStrike API call.
1211
*/
13-
@Getter @Setter
14-
public class CrowdStrikeApiResponse {
12+
@Data
13+
public class CrowdStrikeThreatIntelApiResponse {
1514

1615
private CrowdStrikeIndicatorResult body;
1716
private Map<String, List<String>> headers;
1817

19-
public CrowdStrikeApiResponse(CrowdStrikeIndicatorResult body, Map<String, List<String>> headers) {
18+
public CrowdStrikeThreatIntelApiResponse(CrowdStrikeIndicatorResult body, Map<String, List<String>> headers) {
2019
this.body = body;
2120
this.headers = headers;
2221
}

‎data-prepper-plugins/saas-source-plugins/crowdstrike-source/src/main/java/org/opensearch/dataprepper/plugins/source/crowdstrike/models/ThreatIndicator.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
44
import com.fasterxml.jackson.annotation.JsonProperty;
5+
import lombok.Data;
56
import lombok.Getter;
67
import lombok.Setter;
78

@@ -10,8 +11,7 @@
1011
* This class encapsulates information about potential security threats,
1112
* including indicators of compromise (IoCs) and associated metadata.
1213
*/
13-
@Setter
14-
@Getter
14+
@Data
1515
@JsonIgnoreProperties(ignoreUnknown = true)
1616
public class ThreatIndicator {
1717
/**

‎data-prepper-plugins/saas-source-plugins/crowdstrike-source/src/main/java/org/opensearch/dataprepper/plugins/source/crowdstrike/rest/CrowdStrikeAuthClient.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public void initCredentials() {
6666
*
6767
* @throws UnauthorizedException Runtime exception if the token cannot be retrieved.
6868
*/
69-
protected void getAuthToken() {
69+
private void getAuthToken() {
7070
synchronized (tokenRenewLock) {
7171
if (isTokenValid()) {
7272
//Someone else must have already renewed it

‎data-prepper-plugins/saas-source-plugins/crowdstrike-source/src/test/java/org/opensearch/dataprepper/plugins/source/crowdstrike/CrowdStrikeServiceTest.java‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import org.junit.jupiter.api.Test;
55
import org.mockito.Mockito;
66
import org.opensearch.dataprepper.metrics.PluginMetrics;
7-
import org.opensearch.dataprepper.plugins.source.crowdstrike.models.CrowdStrikeApiResponse;
7+
import org.opensearch.dataprepper.plugins.source.crowdstrike.models.CrowdStrikeThreatIntelApiResponse;
88
import org.opensearch.dataprepper.plugins.source.crowdstrike.models.CrowdStrikeIndicatorResult;
99
import org.opensearch.dataprepper.plugins.source.crowdstrike.rest.CrowdStrikeRestClient;
1010
import io.micrometer.core.instrument.Timer;
@@ -21,7 +21,9 @@
2121
import static org.junit.jupiter.api.Assertions.assertTrue;
2222
import static org.mockito.ArgumentMatchers.any;
2323
import static org.mockito.ArgumentMatchers.eq;
24+
import static org.mockito.Mockito.doThrow;
2425
import static org.mockito.Mockito.mock;
26+
import static org.mockito.Mockito.spy;
2527
import static org.mockito.Mockito.times;
2628
import static org.mockito.Mockito.verify;
2729
import static org.mockito.Mockito.when;
@@ -65,7 +67,7 @@ void testGetThreatIndicatorsWithValidTimeRange() {
6567
when(restClient.invokeGetApi(any(), eq(CrowdStrikeIndicatorResult.class)))
6668
.thenReturn(responseEntity);
6769

68-
CrowdStrikeApiResponse response = service.getThreatIndicators(startTime, endTime, Optional.empty());
70+
CrowdStrikeThreatIntelApiResponse response = service.getThreatIndicators(startTime, endTime, Optional.empty());
6971

7072
assertNotNull(response.getBody());
7173
assertNotNull(response.getHeaders());
@@ -85,7 +87,7 @@ void testGetThreatIndicatorsWithPaginationLink() throws Exception {
8587
when(restClient.invokeGetApi(eq(sanitizedUri), eq(CrowdStrikeIndicatorResult.class)))
8688
.thenReturn(responseEntity);
8789

88-
CrowdStrikeApiResponse response = service.getThreatIndicators(startTime, endTime, Optional.of(paginationLink));
90+
CrowdStrikeThreatIntelApiResponse response = service.getThreatIndicators(startTime, endTime, Optional.of(paginationLink));
8991
assertNotNull(response.getBody());
9092
verify(restClient).invokeGetApi(eq(sanitizedUri), eq(CrowdStrikeIndicatorResult.class));
9193
}
@@ -103,15 +105,14 @@ void testRestClientThrowsException() {
103105

104106
@Test
105107
void testBuildUriFailsGracefully() {
106-
CrowdStrikeService faultyService = new CrowdStrikeService(restClient, pluginMetrics) {
107-
@Override
108-
protected URI buildCrowdStrikeUri(Instant startTime, Instant endTime, Optional<String> paginationLink) {
109-
throw new RuntimeException("URI construction failed");
110-
}
111-
};
108+
CrowdStrikeService service = new CrowdStrikeService(restClient, pluginMetrics);
109+
110+
CrowdStrikeService spyService = spy(service);
111+
doThrow(new RuntimeException("URI construction failed"))
112+
.when(spyService).buildCrowdStrikeUri(any(), any(), any());
112113

113114
RuntimeException ex = assertThrows(RuntimeException.class, () ->
114-
faultyService.getThreatIndicators(startTime, endTime, Optional.empty()));
115+
spyService.getThreatIndicators(startTime, endTime, Optional.empty()));
115116

116117
assertEquals("URI construction failed", ex.getMessage());
117118
}

data-prepper-plugins/saas-source-plugins/crowdstrike-source/src/test/java/org/opensearch/dataprepper/plugins/source/crowdstrike/models/CrowdStrikeApiResponseTest.java renamed to data-prepper-plugins/saas-source-plugins/crowdstrike-source/src/test/java/org/opensearch/dataprepper/plugins/source/crowdstrike/models/CrowdStrikeThreatIntelApiResponseTest.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
import static org.junit.jupiter.api.Assertions.assertNull;
1212
import static org.junit.jupiter.api.Assertions.assertTrue;
1313

14-
class CrowdStrikeApiResponseTest {
14+
class CrowdStrikeThreatIntelApiResponseTest {
1515
@Mock
1616
CrowdStrikeIndicatorResult mockResult;
1717
Map<String, List<String>> headers = Map.of(
@@ -40,7 +40,7 @@ void testSetAndGetBody() {
4040
result.setResults(List.of(indicator1, indicator2));
4141

4242
// Create response
43-
CrowdStrikeApiResponse response = new CrowdStrikeApiResponse(result, headers);
43+
CrowdStrikeThreatIntelApiResponse response = new CrowdStrikeThreatIntelApiResponse(result, headers);
4444
response.setBody(result);
4545

4646
// Verify body content
@@ -52,7 +52,7 @@ void testSetAndGetBody() {
5252

5353
@Test
5454
void testGetHeader_existingHeader() {
55-
CrowdStrikeApiResponse response = new CrowdStrikeApiResponse(mockResult, headers);
55+
CrowdStrikeThreatIntelApiResponse response = new CrowdStrikeThreatIntelApiResponse(mockResult, headers);
5656
response.setHeaders(headers);
5757

5858
List<String> values = response.getHeader("X-Rate-Limit");
@@ -61,7 +61,7 @@ void testGetHeader_existingHeader() {
6161

6262
@Test
6363
void testGetHeader_nonExistingHeaderReturnsEmptyList() {
64-
CrowdStrikeApiResponse response = new CrowdStrikeApiResponse(mockResult, Collections.emptyMap());
64+
CrowdStrikeThreatIntelApiResponse response = new CrowdStrikeThreatIntelApiResponse(mockResult, Collections.emptyMap());
6565
List<String> result = response.getHeader("Missing");
6666
assertNotNull(result);
6767
assertTrue(result.isEmpty());
@@ -73,7 +73,7 @@ void testGetFirstHeaderValue_existing() {
7373
"Content-Type", List.of("application/json", "charset=utf-8")
7474
);
7575

76-
CrowdStrikeApiResponse response = new CrowdStrikeApiResponse(mockResult, headers);
76+
CrowdStrikeThreatIntelApiResponse response = new CrowdStrikeThreatIntelApiResponse(mockResult, headers);
7777
response.setHeaders(headers);
7878

7979
String first = response.getFirstHeaderValue("Content-Type");
@@ -86,7 +86,7 @@ void testGetFirstHeaderValue_emptyList() {
8686
"X-Empty", List.of()
8787
);
8888

89-
CrowdStrikeApiResponse response = new CrowdStrikeApiResponse(mockResult, headers);
89+
CrowdStrikeThreatIntelApiResponse response = new CrowdStrikeThreatIntelApiResponse(mockResult, headers);
9090
response.setHeaders(headers);
9191

9292
assertNull(response.getFirstHeaderValue("X-Empty"));

‎data-prepper-plugins/saas-source-plugins/crowdstrike-source/src/test/java/org/opensearch/dataprepper/plugins/source/crowdstrike/rest/CrowdStrikeAuthClientTest.java‎

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,24 @@
1313
import org.springframework.http.ResponseEntity;
1414
import org.springframework.web.client.HttpClientErrorException;
1515
import org.springframework.web.client.RestTemplate;
16+
import java.lang.reflect.Field;
1617
import java.time.Instant;
1718
import java.util.HashMap;
1819
import java.util.Map;
20+
import java.util.concurrent.ExecutorService;
21+
import java.util.concurrent.Executors;
22+
import java.util.concurrent.Future;
23+
import static java.util.concurrent.TimeUnit.MILLISECONDS;
24+
import static java.util.concurrent.TimeUnit.SECONDS;
25+
import static org.awaitility.Awaitility.await;
1926
import static org.junit.jupiter.api.Assertions.assertEquals;
2027
import static org.junit.jupiter.api.Assertions.assertNotNull;
2128
import static org.junit.jupiter.api.Assertions.assertThrows;
2229
import static org.junit.jupiter.api.Assertions.assertTrue;
23-
import static org.junit.jupiter.api.Assertions.fail;
2430
import static org.mockito.ArgumentMatchers.any;
2531
import static org.mockito.ArgumentMatchers.anyString;
2632
import static org.mockito.ArgumentMatchers.eq;
2733
import static org.mockito.Mockito.mock;
28-
import static org.mockito.Mockito.never;
2934
import static org.mockito.Mockito.spy;
3035
import static org.mockito.Mockito.times;
3136
import static org.mockito.Mockito.verify;
@@ -97,22 +102,37 @@ void testTooManyRequestsBacksOffAndRetries() {
97102
assertTrue(authClient.getExpireTime().isAfter(Instant.now()));
98103
}
99104

105+
100106
@Test
101-
void testRefreshToken_skipsIfTokenIsValidInitially() {
102-
CrowdStrikeAuthClient client = spy(new CrowdStrikeAuthClient(mockSourceConfig) {
103-
@Override
104-
protected boolean isTokenValid() {
105-
return true;
106-
}
107-
108-
@Override
109-
protected void getAuthToken() {
110-
fail("getAuthToken should not be called when token is already valid.");
111-
}
112-
});
113-
114-
client.refreshToken();
115-
verify(client, times(1)).isTokenValid();
116-
verify(client, never()).getAuthToken();
107+
void testConcurrentRefreshToken_onlyOneApiCall() throws Exception {
108+
CrowdStrikeAuthClient client = spy(new CrowdStrikeAuthClient(mockSourceConfig));
109+
Field restTemplateField = CrowdStrikeAuthClient.class.getDeclaredField("restTemplate");
110+
restTemplateField.setAccessible(true);
111+
restTemplateField.set(client, restTemplateMock);
112+
when(restTemplateMock.postForEntity(anyString(), any(HttpEntity.class), eq(Map.class)))
113+
.thenReturn(ResponseEntity.ok(Map.of(
114+
"access_token", "mock_access_token",
115+
"expires_in", 3600
116+
)));
117+
118+
// Launch two parallel refreshToken() calls
119+
ExecutorService executor = Executors.newFixedThreadPool(2);
120+
Future<?> firstCall = executor.submit(client::refreshToken);
121+
Future<?> secondCall = executor.submit(client::refreshToken);
122+
123+
await()
124+
.atMost(10, SECONDS)
125+
.pollInterval(10, MILLISECONDS)
126+
.until(() -> firstCall.isDone() && secondCall.isDone());
127+
128+
executor.shutdown();
129+
130+
// Validate only 1 token request is made
131+
assertNotNull(client.getBearerToken());
132+
assertEquals("mock_access_token", client.getBearerToken());
133+
assertNotNull(client.getExpireTime());
134+
assertTrue(client.getExpireTime().isAfter(Instant.now().minusSeconds(3500)));
135+
136+
verify(restTemplateMock, times(1)).postForEntity(anyString(), any(HttpEntity.class), eq(Map.class));
117137
}
118138
}

‎data-prepper-plugins/saas-source-plugins/crowdstrike-source/src/test/java/org/opensearch/dataprepper/plugins/source/crowdstrike/rest/CrowdStrikeRestClientTest.java‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,10 @@ void setup() throws Exception {
4242
restTemplate = mock(RestTemplate.class);
4343
uri = new URI("https://api.crowdstrike.com/intel/combined/indicators/v1");
4444

45-
restClient = new CrowdStrikeRestClient(pluginMetrics, authClient) {
46-
{
47-
// inject mock RestTemplate
48-
try {
49-
var restTemplateField = CrowdStrikeRestClient.class.getDeclaredField("restTemplate");
50-
restTemplateField.setAccessible(true);
51-
restTemplateField.set(this, restTemplate);
52-
} catch (Exception e) {
53-
throw new RuntimeException(e);
54-
}
55-
}
56-
};
45+
restClient = new CrowdStrikeRestClient(pluginMetrics, authClient);
46+
var restTemplateField = CrowdStrikeRestClient.class.getDeclaredField("restTemplate");
47+
restTemplateField.setAccessible(true);
48+
restTemplateField.set(restClient, restTemplate);
5749
}
5850

5951
@Test

0 commit comments

Comments
 (0)