adding another parameter for specifying safelisted variable names

apollographql · Jun 24, 2019 · 2981485 · 2981485
1 parent 8f9bbaf
commit 2981485
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 14 deletions.
diff --git a/packages/apollo-engine-reporting/src/__tests__/extension.test.ts b/packages/apollo-engine-reporting/src/__tests__/extension.test.ts
@@ -169,6 +169,40 @@ describe('variableJson output for sendVariableValues exceptNames: Array type', (
   });
 });
 
+describe('variableJson output for sendVariableValues includeNames: Array type', () => {
+  it('array contains some values not in keys', () => {
+    const privateVariablesArray: string[] = ['t2', 'notInVariables'];
+    const expectedVariablesJson = {
+      testing: '',
+      t2: JSON.stringify(2),
+    };
+    expect(
+      makeTraceDetails(variables, { includeNames: privateVariablesArray })
+        .variablesJson,
+    ).toEqual(expectedVariablesJson);
+  });
+
+  it('sendAll=true equivalent to includeNames=[all variables]', () => {
+    let privateVariablesArray: string[] = ['testing', 't2'];
+    expect(
+      makeTraceDetails(variables, { sendAll: true }).variablesJson,
+    ).toEqual(
+      makeTraceDetails(variables, { includeNames: privateVariablesArray })
+        .variablesJson,
+    );
+  });
+
+  it('sendNone=true equivalent to includeNames=[]', () => {
+    let privateVariablesArray: string[] = [];
+    expect(
+      makeTraceDetails(variables, { sendNone: true }).variablesJson,
+    ).toEqual(
+      makeTraceDetails(variables, { includeNames: privateVariablesArray })
+        .variablesJson,
+    );
+  });
+});
+
 describe('variableJson output for sendVariableValues transform: custom function type', () => {
   it('test custom function that redacts every variable to some value', () => {
     const modifiedValue = 100;
@@ -280,6 +314,14 @@ describe('tests for the sendHeaders reporting option', () => {
     expect(http.requestHeaders).toEqual({});
   });
 
+  it('test sendHeaders.includeNames', () => {
+    // headers that should never be sent (such as "authorization") should still be removed if in includeHeaders
+    const include: String[] = ['name', 'authorization', 'notinheaders'];
+    const http = makeTestHTTP();
+    makeHTTPRequestHeaders(http, headers, { includeNames: include });
+    expect(http.requestHeaders).toEqual(headersOutput);
+  });
+
   it('authorization, cookie, and set-cookie headers should never be sent', () => {
     headers.append('cookie', 'blahblah');
     headers.append('set-cookie', 'blahblah');

diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts
@@ -25,7 +25,8 @@ export interface ClientInfo {
   clientReferenceId?: string;
 }
 
-export type BlocklistValuesBaseOptions =
+export type SendValuesBaseOptions =
+  | { includeNames: Array<String> }
   | { exceptNames: Array<String> }
   | { sendAll: true }
   | { sendNone: true };
@@ -41,7 +42,7 @@ export type VariableValueOptions =
         options: VariableValueTransformOptions,
       ) => Record<string, any>;
     }
-  | BlocklistValuesBaseOptions;
+  | SendValuesBaseOptions;
 
 export type GenerateClientInfo<TContext> = (
   requestContext: GraphQLRequestContext<TContext>,

diff --git a/packages/apollo-engine-reporting/src/extension.ts b/packages/apollo-engine-reporting/src/extension.ts
@@ -16,7 +16,7 @@ import {
   GenerateClientInfo,
   AddTraceArgs,
   VariableValueOptions,
-  BlocklistValuesBaseOptions,
+  SendValuesBaseOptions,
 } from './agent';
 import { GraphQLRequestContext } from 'apollo-server-core/dist/requestPipelineAPI';
 
@@ -449,7 +449,9 @@ export function makeTraceDetails(
         // We assume that most users will have only a few variables values to hide,
         // or will just set {sendNone: true}; we can change this
         // linear-time operation if it causes real performance issues.
-        sendVariableValues.exceptNames.includes(name))
+        sendVariableValues.exceptNames.includes(name)) ||
+      ('includeNames' in sendVariableValues &&
+        !sendVariableValues.includeNames.includes(name))
     ) {
       // Special case for private variables. Note that this is a different
       // representation from a variable containing the empty string, as that
@@ -490,22 +492,25 @@ function cleanModifiedVariables(
 export function makeHTTPRequestHeaders(
   http: Trace.IHTTP,
   headers: Headers,
-  sendHeaders?: BlocklistValuesBaseOptions,
+  sendHeaders?: SendValuesBaseOptions,
 ): void {
   if (!sendHeaders || 'sendNone' in sendHeaders) {
     return;
   }
   for (const [key, value] of headers) {
     if (
-      sendHeaders &&
-      'exceptNames' in sendHeaders &&
-      // We assume that most users only have a few headers to hide, or will
-      // just set {sendNone: true} ; we can change this linear-time
-      // operation if it causes real performance issues.
-      sendHeaders.exceptNames.some(exceptHeader => {
-        // Headers are case-insensitive, and should be compared as such.
-        return exceptHeader.toLowerCase() === key.toLowerCase();
-      })
+      ('exceptNames' in sendHeaders &&
+        // We assume that most users only have a few headers to hide, or will
+        // just set {sendNone: true} ; we can change this linear-time
+        // operation if it causes real performance issues.
+        sendHeaders.exceptNames.some(exceptHeader => {
+          // Headers are case-insensitive, and should be compared as such.
+          return exceptHeader.toLowerCase() === key.toLowerCase();
+        })) ||
+      ('includeNames' in sendHeaders &&
+        !sendHeaders.includeNames.some(header => {
+          return header.toLowerCase() === key.toLowerCase();
+        }))
     ) {
       continue;
     }