Skip to content

Commit 0613d2b

Browse files
DavidMcClatcheyDavidMcClatchey
committed
fixing recursion of SR objects and more testing
1 parent 88982f7 commit 0613d2b

2 files changed

Lines changed: 66 additions & 12 deletions

File tree

‎backend/src/main/java/gov/cdc/usds/simplereport/db/model/ConsoleApiAuditEvent.java‎

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import gov.cdc.usds.simplereport.db.model.auxiliary.HttpRequestDetails;
1010
import java.lang.reflect.Array;
1111
import java.lang.reflect.Field;
12+
import java.lang.reflect.Modifier;
1213
import java.time.temporal.Temporal;
1314
import java.util.ArrayList;
1415
import java.util.Date;
@@ -127,7 +128,7 @@ private Object depthFirstSearchRedact(
127128
visited.add(variableValue);
128129

129130
// early return if match found, ignoring variableValue's object type
130-
if (piiJsonVariableNames.contains(variableName)) {
131+
if (piiVariableNames.contains(variableName)) {
131132
redacted.add(variableValue);
132133
return replacement;
133134
}
@@ -166,11 +167,13 @@ private Object depthFirstSearchRedact(
166167
int length = Array.getLength(variableValue);
167168
for (int i = 0; i < length; i++) {
168169
Object element = Array.get(variableValue, i);
169-
Object redactedElement =
170-
depthFirstSearchRedact(
171-
variableName + "[" + i + "]", element, replacement, visited, redacted);
170+
if (element != null) {
171+
Object redactedElement =
172+
depthFirstSearchRedact(
173+
element.getClass().getName(), element, replacement, visited, redacted);
172174

173-
Array.set(variableValue, i, redactedElement);
175+
Array.set(variableValue, i, redactedElement);
176+
}
174177
}
175178

176179
return variableValue;
@@ -181,11 +184,30 @@ private Object depthFirstSearchRedact(
181184
return variableValue;
182185
} else {
183186
Class<?> cls = variableValue.getClass();
184-
for (Field f : cls.getDeclaredFields()) {
185-
f.setAccessible(true);
187+
for (Field field : cls.getDeclaredFields()) {
188+
if (Modifier.isStatic(field.getModifiers())) {
189+
continue;
190+
}
191+
field.setAccessible(true);
186192
try {
187-
Object child = f.get(variableValue);
188-
depthFirstSearchRedact(child.getClass().getName(), child, replacement, visited, redacted);
193+
String childVariableName = field.getName();
194+
Object childVariableValue = field.get(variableValue);
195+
Object depthFirstSearchReplacementValue =
196+
depthFirstSearchRedact(
197+
childVariableName, childVariableValue, replacement, visited, redacted);
198+
199+
if (depthFirstSearchReplacementValue.equals(replacement)) {
200+
if (field.getType() == String.class) {
201+
field.set(variableValue, replacement);
202+
} else {
203+
field.set(
204+
variableValue,
205+
null); // cannot assign the string value "redacted" to a non-string field, so
206+
// setting to null instead
207+
}
208+
} else {
209+
field.set(variableValue, depthFirstSearchReplacementValue);
210+
}
189211
} catch (IllegalAccessException e) {
190212
log.info(e.getMessage());
191213
}
@@ -214,7 +236,7 @@ private static boolean isLeaf(Object o) {
214236
}
215237

216238
@JsonIgnore
217-
private final List<String> piiJsonVariableNames =
239+
private final List<String> piiVariableNames =
218240
// these are taken from pii-containing field names in main.graphqls
219241
new ArrayList<>(
220242
List.of(
@@ -231,7 +253,9 @@ private static boolean isLeaf(Object o) {
231253
"state",
232254
"zipCode",
233255
"telephone",
256+
"number",
234257
"phoneNumbers",
258+
"primaryPhone",
235259
"role",
236260
"lookupId",
237261
"email",

‎backend/src/test/java/gov/cdc/usds/simplereport/service/AuditServiceTest.java‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,17 @@
77
import com.fasterxml.jackson.core.JsonProcessingException;
88
import gov.cdc.usds.simplereport.config.authorization.UserPermission;
99
import gov.cdc.usds.simplereport.db.model.ConsoleApiAuditEvent;
10+
import gov.cdc.usds.simplereport.db.model.Facility;
11+
import gov.cdc.usds.simplereport.db.model.Organization;
12+
import gov.cdc.usds.simplereport.db.model.Person;
13+
import gov.cdc.usds.simplereport.db.model.TestEvent;
14+
import gov.cdc.usds.simplereport.db.model.TestOrder;
1015
import gov.cdc.usds.simplereport.db.model.auxiliary.GraphQlInputs;
1116
import gov.cdc.usds.simplereport.db.model.auxiliary.HttpRequestDetails;
1217
import gov.cdc.usds.simplereport.logging.GraphqlQueryState;
1318
import gov.cdc.usds.simplereport.service.model.UserInfo;
1419
import gov.cdc.usds.simplereport.test_util.SliceTestConfiguration.WithSimpleReportEntryOnlyUser;
20+
import gov.cdc.usds.simplereport.test_util.TestDataFactory;
1521
import java.util.LinkedHashMap;
1622
import java.util.List;
1723
import java.util.Map;
@@ -27,6 +33,7 @@
2733
class AuditServiceTest extends BaseServiceTest<AuditService> {
2834

2935
@Autowired private ApiUserService _userService;
36+
@Autowired private TestDataFactory testDataFactory;
3037
@MockitoBean AuditLoggerService auditLoggerServiceSpy;
3138
@Captor private ArgumentCaptor<ConsoleApiAuditEvent> auditLogCaptor;
3239

@@ -62,6 +69,14 @@ void smokeTestGraphqlAudit() throws JsonProcessingException {
6269
List<LinkedHashMap<String, String>> redactedMultiplexResults =
6370
List.of(covid19RedactedResult, fluARedactedResult, fluBRedactedResult);
6471

72+
int[] unredactablePrimitiveArray = new int[3];
73+
TestEvent[] redactableTestEventArray = new TestEvent[3];
74+
Organization org = testDataFactory.saveValidOrganization();
75+
Facility facility = testDataFactory.createValidFacility(org);
76+
Person patient = testDataFactory.createFullPerson(org);
77+
TestOrder testOrder = testDataFactory.createTestOrder(patient, facility);
78+
redactableTestEventArray[0] = new TestEvent(testOrder);
79+
6580
state.setGraphqlDetails(
6681
new GraphQlInputs(
6782
"A",
@@ -72,7 +87,11 @@ void smokeTestGraphqlAudit() throws JsonProcessingException {
7287
"non-pii key",
7388
"non-pii value",
7489
"results",
75-
multiplexResults)));
90+
multiplexResults,
91+
"unredactablePrimitiveArray",
92+
unredactablePrimitiveArray,
93+
"redactableTestEventArray",
94+
redactableTestEventArray)));
7695
state.setHttpDetails(
7796
new HttpRequestDetails(
7897
"foo.com",
@@ -91,6 +110,7 @@ void smokeTestGraphqlAudit() throws JsonProcessingException {
91110

92111
verify(auditLoggerServiceSpy, atLeastOnce()).logEvent(auditLogCaptor.capture());
93112
ConsoleApiAuditEvent saved = auditLogCaptor.getValue();
113+
Map<String, Object> graphQlVariables = saved.getGraphqlQueryDetails().getVariables();
94114

95115
assertEquals(1L, auditLogCaptor.getAllValues().size());
96116
assertEquals("ABCDE", saved.getRequestId());
@@ -102,9 +122,19 @@ void smokeTestGraphqlAudit() throws JsonProcessingException {
102122
"redacted",
103123
"results",
104124
redactedMultiplexResults,
125+
"unredactablePrimitiveArray",
126+
unredactablePrimitiveArray,
127+
"redactableTestEventArray",
128+
redactableTestEventArray,
105129
"non-pii key",
106130
"non-pii value"),
107-
saved.getGraphqlQueryDetails().getVariables());
131+
graphQlVariables);
132+
133+
TestEvent[] redactableTestEventArrayResult =
134+
(TestEvent[]) graphQlVariables.get("redactableTestEventArray");
135+
assertEquals("redacted", redactableTestEventArrayResult[0].getPatientData().getCountry());
136+
assertEquals("redacted", redactableTestEventArrayResult[0].getPatientData().getRace());
137+
assertEquals("redacted", redactableTestEventArrayResult[0].getPatientData().getLookupId());
108138
assertEquals(
109139
List.of(
110140
UserPermission.SEARCH_PATIENTS,

0 commit comments

Comments
 (0)