Adding sendVariableValues and sendHeaders parameters and specificatio…

…ns to engine (#2847) * added new engine reporting options: `sendVariableValues` and `sendHeaders` to replace `privateVariables` and `privateHeaders`, which will be deprecated * the new options change the previous default (which was to send all): now all values will be hidden * but if users previously specified `privateVariables` or `privateHeaders`, this change will not affect them
apollographql · Jun 26, 2019 · ec03745 · ec03745
1 parent 1ef0c66
commit ec03745
Show file tree

Hide file tree

Showing 6 changed files with 672 additions and 87 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,11 +1,24 @@
 # Changelog
 
-The version headers in this history reflect the versions of Apollo Server itself.  Versions of other packages (e.g. which are not actual HTTP integrations; packages not prefixed with `apollo-server`) may use different versions.  For more details, check the publish commit for that version in the Git history.
+### vNext
 
-### vNEXT
+The version headers in this history reflect the versions of Apollo Server itself.  Versions of other packages (e.g. which are not actual HTTP integrations; packages not prefixed with `apollo-server`) may use different versions.  For more details, check the publish commit for that version in the Git history.
 
 > The changes noted within this `vNEXT` section have not been released yet.  New PRs and commits which introduce changes should include an entry in this `vNEXT` section as part of their development.  When a release is being prepared, a new header will be (manually) created below and the the appropriate changes within that release will be moved into the new section.
 
+- `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)
+
+  Note: This is only a breaking change if the option `privateVariables` was not previously specified. In order to preserve the old default, pass in the option:
+  `new ApolloServer({engine: {sendVariableValues: {all: true}}})`.
+
+- `apollo-engine-reporting`: BREAKING CHANGE: By default, send no GraphQL request headers to Apollo's servers instead of sending all. Use the new EngineReportingOption `sendHeaders` to send some or all headers and their values. 
+This replaces the `privateHeaders` option, which is now deprecated. [PR #2847](https://github.com/apollographql/apollo-server/pull/2847)
+
+  Note: This is only a breaking change if the option `privateHeaders` was not previously specified. In order to preserve the old default, pass in the option:
+  `new ApolloServer({engine: {sendHeaders: {all: true}}})`.
+
 ### v2.6.7
 
 > [See complete versioning details.](https://github.com/apollographql/apollo-server/commit/183de5f112324def375a45c239955e1bf1608fae)

diff --git a/docs/source/api/apollo-server.md b/docs/source/api/apollo-server.md
@@ -119,7 +119,7 @@ new ApolloServer({
 
 * `engine`: <`EngineReportingOptions`> | boolean
 
-  Provided the `ENGINE_API_KEY` environment variable is set, the engine reporting agent will be started automatically. The API key can also be provided as the `apiKey` field in an object passed as the `engine` field. See the [EngineReportingOptions](#enginereportingoptions) section for a full description of how to configure the reporting agent, including how to blacklist variables. When using the Engine proxy, this option should be set to false.
+  Provided the `ENGINE_API_KEY` environment variable is set, the engine reporting agent will be started automatically. The API key can also be provided as the `apiKey` field in an object passed as the `engine` field. See the [EngineReportingOptions](#enginereportingoptions) section for a full description of how to configure the reporting agent, including how to include variable values and HTTP headers. When using the Engine proxy, this option should be set to false.
 
 * `persistedQueries`: <`Object`> | false
 
@@ -342,20 +342,62 @@ addMockFunctionsToSchema({
    By default, errors sending reports to Engine servers will be logged
    to standard error. Specify this function to process errors in a different
    way.
+
+* `sendVariableValues`: { transform: (options: { variables: Record<string, any>, operationString?: string } ) => Record<string, any> }
+                     | { exceptNames: Array&lt;String&gt; }
+                     | { onlyNames: Array&lt;String&gt; }
+                     | { none: true }
+                     | { all: true }
+
+    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:
+    
+    - { none: true }: don't send any variable values (DEFAULT)
+    - { all: true }: send all variable values
+    - { transform: ... }: a custom function for modifying variable values. Keys added by the custom function will be removed, and keys removed will be added back with an empty value. 
+    - { exceptNames: ... }: a case-sensitive list of names of variables whose values should not be sent to Apollo servers
+    - { onlyNames: ... }: a case-sensitive list of names of variables whose values will be sent to Apollo Servers
+
+   Defaults to not sending any variable values if both this parameter and the deprecated `privateVariables` are not set. 
+   The report will indicate each private variable key whose value was redacted by { none: true } or { exceptNames: [...] }.
+
+*  `privateVariables`: Array&lt;String&gt; | boolean
 
-*  `privateVariables`: Array<String> | boolean
-
-   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.
-
-*  `privateHeaders`: Array<String> | boolean
-
-   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.
+   <!--- TODO(helen): update the version number here --->
+   DEPRECATING IN VERSION XX.XX.XX, to be replaced by the option `sendVariableValues`, which supports the same
+   functionalities but allows for more flexibility. Passing an array into `privateVariables` is equivalent to
+   passing in `{ exceptNames: array } ` to `sendVariableValues`, and passing in `true` or `false` is equivalent
+   to passing ` { none: true } ` or ` { all: true }`, respectively.
+   
+   NOTE: An error will be thrown if both this deprecated option and its replacement, `sendVariableValues` are defined.
+   In order to preserve the old default of `privateVariables`, which sends all variables and their values, pass in the `sendVariableValues` option:
+     `new ApolloServer({engine: {sendVariableValues: {all: true}}})`.
+    
+* `sendHeaders`: { exceptNames: Array&lt;String&gt; } | { onlyNames: Array&lt;String&gt; } | { all: boolean } | { none: boolean }
+   By default, Apollo Server does not send the list of HTTP request 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 several forms:
+   
+   - { none: true }: drop all HTTP request headers (DEFAULT)
+   - { all: true }: send the values of all HTTP request headers
+   - { exceptNames: ... }: A case-insensitive list of names of HTTP headers whose values should not be
+   sent to Apollo servers
+   - { onlyNames: ... }: A case-insensitive list of names of HTTP headers whose values will be sent to Apollo servers
+   
+   Defaults to not sending any request header names and values if both this parameter and the deprecated `privateHeaders` are not set.
+   Unlike with `sendVariableValues`, names of dropped headers are not reported.
+   The headers 'authorization', 'cookie', and 'set-cookie' are never reported.
+   
+*  `privateHeaders`: Array&lt;String&gt; | boolean
 
+   <!--- TODO(helen): update the version number here --->
+   DEPRECATING IN VERSION XX.XX.XX, use `sendHeaders` instead.
+   Passing an array into `privateHeaders` is equivalent to passing ` { exceptNames: array } ` into `sendHeaders`, and
+   passing `true` or `false` is equivalent to passing in ` { none: true } ` and ` { all: true }`, respectively.
+   
+   NOTE: An error will be thrown if both this deprecated option and its replacement, `sendHeaders` are defined.
+   In order to preserve the old default of `privateHeaders`, which sends all request headers and their values, pass in the `sendHeaders` option:
+      `new ApolloServer({engine: {sendHeaders: {all: true}}})`.
+      
 *  `handleSignals`: boolean
 
    By default, EngineReportingAgent listens for the 'SIGINT' and 'SIGTERM'

diff --git a/packages/apollo-engine-reporting/src/__tests__/agent.test.ts b/packages/apollo-engine-reporting/src/__tests__/agent.test.ts
@@ -1,4 +1,8 @@
-import { signatureCacheKey } from '../agent';
+import {
+  signatureCacheKey,
+  handleLegacyOptions,
+  EngineReportingOptions,
+} from '../agent';
 
 describe('signature cache key', () => {
   it('generates without the operationName', () => {
@@ -11,3 +15,80 @@ describe('signature cache key', () => {
     );
   });
 });
+
+describe("test handleLegacyOptions(), which converts the deprecated privateVariable and privateHeaders options to the new options' formats", () => {
+  it('Case 1: privateVariables/privateHeaders == False; same as all', () => {
+    const optionsPrivateFalse: EngineReportingOptions<any> = {
+      privateVariables: false,
+      privateHeaders: false,
+    };
+    handleLegacyOptions(optionsPrivateFalse);
+    expect(optionsPrivateFalse.privateVariables).toBe(undefined);
+    expect(optionsPrivateFalse.sendVariableValues).toEqual({ all: true });
+    expect(optionsPrivateFalse.privateHeaders).toBe(undefined);
+    expect(optionsPrivateFalse.sendHeaders).toEqual({ all: true });
+  });
+
+  it('Case 2: privateVariables/privateHeaders == True; same as none', () => {
+    const optionsPrivateTrue: EngineReportingOptions<any> = {
+      privateVariables: true,
+      privateHeaders: true,
+    };
+    handleLegacyOptions(optionsPrivateTrue);
+    expect(optionsPrivateTrue.privateVariables).toBe(undefined);
+    expect(optionsPrivateTrue.sendVariableValues).toEqual({ none: true });
+    expect(optionsPrivateTrue.privateHeaders).toBe(undefined);
+    expect(optionsPrivateTrue.sendHeaders).toEqual({ none: true });
+  });
+
+  it('Case 3: privateVariables/privateHeaders set to an array', () => {
+    const privateArray: Array<String> = ['t1', 't2'];
+    const optionsPrivateArray: EngineReportingOptions<any> = {
+      privateVariables: privateArray,
+      privateHeaders: privateArray,
+    };
+    handleLegacyOptions(optionsPrivateArray);
+    expect(optionsPrivateArray.privateVariables).toBe(undefined);
+    expect(optionsPrivateArray.sendVariableValues).toEqual({
+      exceptNames: privateArray,
+    });
+    expect(optionsPrivateArray.privateHeaders).toBe(undefined);
+    expect(optionsPrivateArray.sendHeaders).toEqual({
+      exceptNames: privateArray,
+    });
+  });
+
+  it('Case 4: throws error when both the new and old options are set', () => {
+    const optionsBothVariables: EngineReportingOptions<any> = {
+      privateVariables: true,
+      sendVariableValues: { none: true },
+    };
+    expect(() => {
+      handleLegacyOptions(optionsBothVariables);
+    }).toThrow();
+    const optionsBothHeaders: EngineReportingOptions<any> = {
+      privateHeaders: true,
+      sendHeaders: { none: true },
+    };
+    expect(() => {
+      handleLegacyOptions(optionsBothHeaders);
+    }).toThrow();
+  });
+
+  it('Case 5: the passed in options are not modified if deprecated fields were not set', () => {
+    const optionsNotDeprecated: EngineReportingOptions<any> = {
+      sendVariableValues: { exceptNames: ['test'] },
+      sendHeaders: true,
+    };
+    const output: EngineReportingOptions<any> = {
+      sendVariableValues: { exceptNames: ['test'] },
+      sendHeaders: true,
+    };
+    handleLegacyOptions(optionsNotDeprecated);
+    expect(optionsNotDeprecated).toEqual(output);
+
+    const emptyInput: EngineReportingOptions<any> = {};
+    handleLegacyOptions(emptyInput);
+    expect(emptyInput).toEqual({});
+  });
+});