adding new reporting option sendHeaders, fixing some names

CHANGELOG.md

@@ -3,7 +3,8 @@
 ### vNext
 - `apollo-engine-reporting`: BREAKING CHANGE: By default, send no GraphQL variable values to Apollo's servers instead of sending all variable values. Use the new EngineReportingOption `sendVariableValues` to send some or all variable values, possibly after transforming them. 
 This replaces the `privateVariables` option, which is now deprecated. [PR #2847](https://github.com/apollographql/apollo-server/pull/2847) [PR #2472](https://github.com/apollographql/apollo-server/pull/2472)
-
+- `apollo-engine-reporting`: BREAKING CHANGE: By default, send no GraphQL headers to Apollo's servers instead of sending all. Use the new EngineReportingOption `sendHeaders` to send some or all headers and their values. 
+A replacement for the 'privateHeaders' option, which is now deprecated. [PR #2847](https://github.com/apollographql/apollo-server/pull/2847)
 ### v2.6.2
 
 - `apollo-engine-reporting-protobuf`: Update protobuff to include `forbiddenOperations` and `registeredOperations` [PR #2768](https://github.com/apollographql/apollo-server/pull/2768)

docs/source/api/apollo-server.md

@@ -355,20 +355,30 @@ addMockFunctionsToSchema({
 
 * `sendVariableValues`: { valueModifier: (options: { variables: Record<string, any>, operationString?: string } ) => Record<string, any> }
                      | { exceptVariableNames: Array<String\> }
-                     | { whitelistAll: boolean }
+                     | { safelistAll: boolean }
 
     By default, Apollo Server does not send the values of any GraphQL variables to Apollo's servers, because variable values often contain the private data of your app's users. If you'd like variable values to be included in traces, set this option. This option can take several forms:
 
-    - { whitelistAll: ... }: false to blocklist, or true to whitelist all variable values
+    - { safelistAll: ... }: false to blocklist, or true to safelist all variable values
     - { valueModifier: ... }: a custom function for modifying variable values
     - { exceptVariableNames: ... }: a case-sensitive list of names of variables whose values should not be sent to Apollo servers
 
     Defaults to blocklisting all variable values if both this parameter and
     the to-be-deprecated `privateVariables` are not set. The report will also
-    indicate each private variable key redacted by { whitelistAll: false } or { exceptVariableNames: [...] }.
-
+    indicate each private variable key redacted by { safelistAll: false } or { exceptVariableNames: [...] }.
+* `sendHeaders`: { except: Array<String\> } | { safelistAll: boolean }
+   By default, Apollo Server does not send the list of HTTP headers and values to
+   Apollo's servers, to protect private data of your app's users. If you'd like this information included in traces,
+   set this option. This option can take two forms:
+
+   - {except: Array<String\>} A case-insensitive list of names of HTTP headers whose values should not be
+   sent to Apollo servers
+   - {safelistAll: true/false} to include or leave out all HTTP headers.
+
+   Unlike with sendVariableValues, names of dropped headers are not reported.
 *  `privateHeaders`: Array<String\> | boolean
 
+   DEPRECATING IN VERSION XX.XX.XX, use 'sendHeaders' instead.
    A case-insensitive list of names of HTTP headers whose values should not be
    sent to Apollo servers, or 'true' to leave out all HTTP headers. Unlike
    with privateVariables, names of dropped headers are not reported.

packages/apollo-engine-reporting/src/__tests__/extension.test.ts

@@ -10,7 +10,10 @@ import {
   EngineReportingExtension,
   makeTraceDetails,
   makeTraceDetailsLegacy,
+  makeHTTPRequestHeaders,
+  makeHTTPRequestHeadersLegacy,
 } from '../extension';
+import { Headers } from 'apollo-server-env';
 import { InMemoryLRUCache } from 'apollo-server-caching';
 
 test('trace construction', async () => {
@@ -88,6 +91,9 @@ test('trace construction', async () => {
   // XXX actually write some tests
 });
 
+/**
+ * TESTS FOR sendVariableValues REPORTING OPTION
+ */
 const variables: Record<string, any> = {
   testing: 'testing',
   t2: 2,
@@ -110,18 +116,18 @@ test('check variableJson output for sendVariableValues null or undefined (defaul
   expect(makeTraceDetails(variables, undefined)).toEqual(filteredOutput);
 });
 
-test('check variableJson output for sendVariableValues whitelist type', () => {
+test('check variableJson output for sendVariableValues safelist type', () => {
   // Case 1: No keys/values in variables to be filtered/not filtered
   const emptyOutput = new Trace.Details();
-  expect(makeTraceDetails({}, { always: true })).toEqual(emptyOutput);
-  expect(makeTraceDetails({}, { always: false })).toEqual(emptyOutput);
+  expect(makeTraceDetails({}, { safelistAll: true })).toEqual(emptyOutput);
+  expect(makeTraceDetails({}, { safelistAll: false })).toEqual(emptyOutput);
 
   // Case 2: Filter all variables
   const filteredOutput = new Trace.Details();
   Object.keys(variables).forEach(name => {
     filteredOutput.variablesJson[name] = '';
   });
-  expect(makeTraceDetails(variables, { whitelistAll: false })).toEqual(
+  expect(makeTraceDetails(variables, { safelistAll: false })).toEqual(
     filteredOutput,
   );
 
@@ -130,7 +136,7 @@ test('check variableJson output for sendVariableValues whitelist type', () => {
   Object.keys(variables).forEach(name => {
     nonFilteredOutput.variablesJson[name] = JSON.stringify(variables[name]);
   });
-  expect(makeTraceDetails(variables, { whitelistAll: true })).toEqual(
+  expect(makeTraceDetails(variables, { safelistAll: true })).toEqual(
     nonFilteredOutput,
   );
 });
@@ -171,10 +177,10 @@ test('variableJson output for privacyEnforcer custom function', () => {
   ).toEqual(output);
 });
 
-test('whitelist=False equivalent to Array(all variables)', () => {
+test('safelist=False equivalent to Array(all variables)', () => {
   let privateVariablesArray: string[] = ['testing', 't2'];
   expect(
-    makeTraceDetails(variables, { whitelistAll: false }).variablesJson,
+    makeTraceDetails(variables, { safelistAll: false }).variablesJson,
   ).toEqual(
     makeTraceDetails(variables, { exceptVariableNames: privateVariablesArray })
       .variablesJson,
@@ -214,15 +220,15 @@ test('original keys in variables match the modified keys', () => {
 });
 
 /**
- * Tests for old privateVariables reporting option
+ * Tests to ensure support of the old privateVariables reporting option
  */
 test('test variableJson output for to-be-deprecated privateVariable option', () => {
-  // Case 1: privateVariables == False; same as whitelist all
+  // Case 1: privateVariables == False; same as safelist all
   expect(makeTraceDetailsLegacy({}, false)).toEqual(
-    makeTraceDetails({}, { whitelistAll: false }),
+    makeTraceDetails({}, { safelistAll: false }),
   );
   expect(makeTraceDetailsLegacy(variables, false)).toEqual(
-    makeTraceDetails(variables, { whitelistAll: true }),
+    makeTraceDetails(variables, { safelistAll: true }),
   );
 
   // Case 2: privateVariables is an Array; same as makeTraceDetails
@@ -234,3 +240,80 @@ test('test variableJson output for to-be-deprecated privateVariable option', ()
       .variablesJson,
   );
 });
+
+function makeTestHTTP(): Trace.HTTP {
+  return new Trace.HTTP({
+    method: Trace.HTTP.Method.UNKNOWN,
+    host: null,
+    path: null,
+  });
+}
+
+const headers = new Headers();
+headers.append('name', 'value');
+headers.append('authorization', 'blahblah'); // THIS SHOULD NEVER BE SENT
+
+const headersOutput = { name: new Trace.HTTP.Values({ value: ['value'] }) };
+
+/**
+ * TESTS FOR sendHeaders REPORTING OPTION
+ */
+test('test that sendHeaders defaults to hiding all', () => {
+  const http = makeTestHTTP();
+  makeHTTPRequestHeaders(http, headers, null);
+  expect(http.requestHeaders).toEqual({});
+  makeHTTPRequestHeaders(http, headers, undefined);
+  expect(http.requestHeaders).toEqual({});
+  makeHTTPRequestHeaders(http, headers);
+  expect(http.requestHeaders).toEqual({});
+});
+
+test('test sendHeaders.safelistAll', () => {
+  const httpSafelist = makeTestHTTP();
+  makeHTTPRequestHeaders(httpSafelist, headers, { safelistAll: true });
+  expect(httpSafelist.requestHeaders).toEqual(headersOutput);
+
+  const httpBlocklist = makeTestHTTP();
+  makeHTTPRequestHeaders(httpBlocklist, headers, { safelistAll: false });
+  expect(httpBlocklist.requestHeaders).toEqual({});
+});
+
+test('test sendHeaders.except', () => {
+  const except: String[] = ['name', 'notinheaders'];
+  const http = makeTestHTTP();
+  makeHTTPRequestHeaders(http, headers, { except: except });
+  expect(http.requestHeaders).toEqual({});
+});
+
+/**
+ * And test to ensure support of old privateHeaders ooption
+ */
+test('test legacy privateHeaders boolean / Array ', () => {
+  // Test Array input
+  const except: String[] = ['name', 'notinheaders'];
+  const httpExcept = makeTestHTTP();
+  makeHTTPRequestHeaders(httpExcept, headers, { except: except });
+  const httpPrivateHeadersArray = makeTestHTTP();
+  makeHTTPRequestHeadersLegacy(httpPrivateHeadersArray, headers, except);
+  expect(httpExcept.requestHeaders).toEqual(
+    httpPrivateHeadersArray.requestHeaders,
+  );
+
+  // Test boolean input safelist vs. privateHeaders false
+  const httpSafelist = makeTestHTTP();
+  makeHTTPRequestHeaders(httpSafelist, headers, { safelistAll: true });
+  const httpPrivateHeadersFalse = makeTestHTTP();
+  makeHTTPRequestHeadersLegacy(httpPrivateHeadersFalse, headers, false);
+  expect(httpSafelist.requestHeaders).toEqual(
+    httpPrivateHeadersFalse.requestHeaders,
+  );
+
+  // Test boolean input blocklist vs. privateHeaders true
+  const httpBlocklist = makeTestHTTP();
+  makeHTTPRequestHeaders(httpBlocklist, headers, { safelistAll: false });
+  const httpPrivateHeadersTrue = makeTestHTTP();
+  makeHTTPRequestHeadersLegacy(httpPrivateHeadersTrue, headers, true);
+  expect(httpBlocklist.requestHeaders).toEqual(
+    httpPrivateHeadersTrue.requestHeaders,
+  );
+});

packages/apollo-engine-reporting/src/agent.ts

@@ -37,7 +37,7 @@ export type VariableValueOptions =
       ) => Record<string, any>;
     }
   | { exceptVariableNames: Array<String> }
-  | { whitelistAll: boolean };
+  | { safelistAll: boolean };
 
 export type GenerateClientInfo<TContext> = (
   requestContext: GraphQLRequestContext<TContext>,
@@ -96,7 +96,8 @@ export interface EngineReportingOptions<TContext> {
    */
   reportErrorFunction?: (err: Error) => void;
   /**
-   * [TO-BE-DEPRECATED] A case-sensitive list of names of variables whose values should
+   * [TO-BE-DEPRECATED] Use sendVariableValues
+   * A case-sensitive list of names of variables whose values should
    * not be sent to Apollo servers, or 'true' to leave out all variables. In the former
    * case, the report will indicate that each private variable was redacted; in
    * the latter case, no variables are sent at all.
@@ -106,20 +107,33 @@ export interface EngineReportingOptions<TContext> {
    * By default, Apollo Server does not send the values of any GraphQL variables to Apollo's servers, because variable
    * values often contain the private data of your app's users. If you'd like variable values to be included in traces, set this option.
    * This option can take several forms:
-   * - { whitelistAll: ... }: false to blocklist, or true to whitelist all variable values
+   * - { safelistAll: ... }: false to blocklist, or true to safelist all variable values
    * - { valueModifier: ... }: a custom function for modifying variable values
    * - { exceptVariableNames: ... }: a case-sensitive list of names of variables whose values should not be sent to Apollo servers
    *
    * Defaults to blocklisting all variable values if both this parameter and
    * the to-be-deprecated `privateVariables` are not set. The report will also
-   * indicate each private variable key redacted by { whitelistAll: false } or { exceptVariableNames: [...] }.
+   * indicate each private variable key redacted by { safelistAll: false } or { exceptVariableNames: [...] }.
    *
    * TODO(helen): Add new flag to the trace details (and modify the protobuf message structure) to indicate the type of modification. Then, add the following description to the docs:
    * "The report will indicate that variable values were modified by a custom function, or will list all private variables redacted."
    * TODO(helen): LINK TO EXAMPLE FUNCTION? e.g. a function recursively search for keys to be blocklisted
    */
   sendVariableValues?: VariableValueOptions;
   /**
+   * By default, Apollo Server does not send the list of HTTP headers and values to
+   * Apollo's servers, to protect private data of your app's users. If you'd like this information included in traces,
+   * set this option. This option can take two forms:
+   *
+   * - {except: Array<String>} A case-insensitive list of names of HTTP headers whose values should not be
+   * sent to Apollo servers
+   * - {safelistAll: 'true'/'false'} to include or leave out all HTTP headers.
+   *
+   * Unlike with sendVariableValues, names of dropped headers are not reported.
+   */
+  sendHeaders?: { except: Array<String> } | { safelistAll: boolean };
+  /**
+   * [TO-BE-DEPRECATED] Use sendHeaders
    * A case-insensitive list of names of HTTP headers whose values should not be
    * sent to Apollo servers, or 'true' to leave out all HTTP headers. Unlike
    * with privateVariables, names of dropped headers are not reported.