Skip to content

Commit

Permalink
adding enforcePrivateVariable parameter and specifications to engine …
Browse files Browse the repository at this point in the history 
…reporting options
  • Loading branch information
Helen Ho authored and Helen Ho committed Jun 14, 2019
1 parent c8c99e4 commit dbce299
Show file tree
Hide file tree
Showing 4 changed files with 214 additions and 37 deletions.
9 changes: 9 additions & 0 deletions packages/apollo-engine-reporting-protobuf/src/reports.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,15 @@ message Trace {
// enclosed in double quotes, etc. The value of a private variable is
// the empty string.
map<string, string> variables_json = 4;
// A flag so that it is possible to indicate in the Engine frontend whether/how
// variables were modified in the trace.
enum PrivateVariableEnforcerType {
NONE = 0;
BOOLEAN = 1;
ARRAY = 2;
FUNCTION = 3;
}
PrivateVariableEnforcerType privacy_enforcer_type = 5;
// Deprecated. Engineproxy did not encode variable values as JSON, so you
// couldn't tell numbers from numeric strings. Send variables_json instead.
map<string, bytes> variables = 1;
Expand Down
77 changes: 76 additions & 1 deletion packages/apollo-engine-reporting/src/__tests__/extension.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
import { Trace } from 'apollo-engine-reporting-protobuf';
import { graphql } from 'graphql';
import { Request } from 'node-fetch';
import { EngineReportingExtension } from '../extension';
import { EngineReportingExtension, makeTraceDetails } from '../extension';
import { InMemoryLRUCache } from 'apollo-server-caching';

test('trace construction', async () => {
Expand Down Expand Up @@ -83,3 +83,78 @@ test('trace construction', async () => {
requestDidEnd();
// XXX actually write some tests
});

const variables: Record<string, any> = {
testing: 'testing',
t2: 2,
};

test('check variableJson output for privacyEnforcer boolean type', () => {
// Case 1: No keys/values in variables to be filtered/not filtered
const emptyOutput = new Trace.Details();
emptyOutput.privacyEnforcerType =
Trace.Details.PrivateVariableEnforcerType.BOOLEAN;
expect(makeTraceDetails({}, true)).toEqual(emptyOutput);
expect(makeTraceDetails({}, false)).toEqual(emptyOutput);

// Case 2: Filter all variables (enforce == True)
const filteredOutput = new Trace.Details();
filteredOutput.privacyEnforcerType =
Trace.Details.PrivateVariableEnforcerType.BOOLEAN;
Object.keys(variables).forEach(name => {
filteredOutput.variablesJson[name] = '';
});
expect(makeTraceDetails(variables, true)).toEqual(filteredOutput);

// Case 3: Do not filter variables (enforce == False)
const nonFilteredOutput = new Trace.Details();
nonFilteredOutput.privacyEnforcerType =
Trace.Details.PrivateVariableEnforcerType.BOOLEAN;
Object.keys(variables).forEach(name => {
nonFilteredOutput.variablesJson[name] = JSON.stringify(variables[name]);
});
expect(makeTraceDetails(variables, false)).toEqual(nonFilteredOutput);
});

test('variableJson output for privacyEnforcer Array type', () => {
const privacyEnforcerArray: string[] = ['testing', 'notInVariables'];
const expectedVariablesJson = {
testing: '',
t2: JSON.stringify(2),
};
expect(
makeTraceDetails(variables, privacyEnforcerArray).variablesJson,
).toEqual(expectedVariablesJson);
expect(
makeTraceDetails(variables, privacyEnforcerArray).privacyEnforcerType,
).toEqual(Trace.Details.PrivateVariableEnforcerType.ARRAY);
});

test('variableJson output for privacyEnforcer custom function', () => {
// Custom function that redacts every variable to 100;
const modifiedValue = 100;
const customEnforcer = (input: Record<string, any>): Record<string, any> => {
let out: Record<string, any> = {};
Object.keys(input).map((name: string) => {
out[name] = modifiedValue;
});
return out;
};

// Expected output
const output = new Trace.Details();
output.privacyEnforcerType =
Trace.Details.PrivateVariableEnforcerType.FUNCTION;
Object.keys(variables).forEach(name => {
output.variablesJson[name] = JSON.stringify(modifiedValue);
});

expect(makeTraceDetails(variables, customEnforcer)).toEqual(output);
});

test('privacyEnforcer=True equivalent to privacyEnforcer=Array(all variables)', () => {
let privateVariablesArray: string[] = ['testing', 't2'];
expect(makeTraceDetails(variables, true).variablesJson).toEqual(
makeTraceDetails(variables, privateVariablesArray).variablesJson,
);
});
18 changes: 16 additions & 2 deletions packages/apollo-engine-reporting/src/agent.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,26 @@ export interface EngineReportingOptions<TContext> {
*/
reportErrorFunction?: (err: Error) => void;
/**
* 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
* [TO-BE-DEPRECATED] 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.
*/
privateVariables?: Array<String> | boolean;
/**
* A case-sensitive list of names of variables whose values should
* not be sent to Apollo servers, a custom function for modifying variable values,
* or 'true' v. 'false' to blacklist v. whitelist all variables, respectively. In the first
* case, the report will indicate each private variable redacted; in the second,
* the report will indicate that variable values were modified by a custom function;
* in the final case, the report will indicate whether the variables were all
* blacklisted or whitelisted. Will default to 'true' if not specified, and
* the report will indicate that no privacy enforcer was provided.
*/
enforcePrivateVariables?:
| ((v: Record<string, any>) => Record<string, any>)
| 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
Expand Down
147 changes: 113 additions & 34 deletions packages/apollo-engine-reporting/src/extension.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,41 +146,52 @@ export class EngineReportingExtension<TContext = any>
}
}

if (this.options.privateVariables !== true && o.variables) {
// Note: we explicitly do *not* include the details.rawQuery field. The
// Engine web app currently does nothing with this other than store it in
// the database and offer it up via its GraphQL API, and sending it means
// that using calculateSignature to hide sensitive data in the query
// string is ineffective.
this.trace.details = new Trace.Details();
Object.keys(o.variables).forEach(name => {
if (
this.options.privateVariables &&
Array.isArray(this.options.privateVariables) &&
// We assume that most users will have only a few private variables,
// or will just set privateVariables to true; we can change this
// linear-time operation if it causes real performance issues.
this.options.privateVariables.includes(name)
) {
// Special case for private variables. Note that this is a different
// representation from a variable containing the empty string, as that
// will be sent as '""'.
this.trace.details!.variablesJson![name] = '';
} else {
try {
this.trace.details!.variablesJson![name] = JSON.stringify(
o.variables![name],
);
} catch (e) {
// This probably means that the value contains a circular reference,
// causing `JSON.stringify()` to throw a TypeError:
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#Issue_with_JSON.stringify()_when_serializing_circular_references
this.trace.details!.variablesJson![name] = JSON.stringify(
'[Unable to convert value to JSON]',
);
if ((this.options.enforcePrivateVariables || this.options.privateVariables === null) && o.variables) {
// Default to the enforcePrivateVariables specs if privateVariables was not set.
this.trace.details = makeTraceDetails(
o.variables,
this.options.enforcePrivateVariables,
);
} else {
// [TO BE DEPRECATED] to use the parameter enforcePrivateVariables over privateVariables.
// If privateVariables was previously specified (non-null), trace.details.variablesJson will
// look the same as before.
if (this.options.privateVariables !== true && o.variables) {
// Note: we explicitly do *not* include the details.rawQuery field. The
// Engine web app currently does nothing with this other than store it in
// the database and offer it up via its GraphQL API, and sending it means
// that using calculateSignature to hide sensitive data in the query
// string is ineffective.
this.trace.details = new Trace.Details();
Object.keys(o.variables).forEach(name => {
if (
this.options.privateVariables &&
Array.isArray(this.options.privateVariables) &&
// We assume that most users will have only a few private variables,
// or will just set privateVariables to true; we can change this
// linear-time operation if it causes real performance issues.
this.options.privateVariables.includes(name)
) {
// Special case for private variables. Note that this is a different
// representation from a variable containing the empty string, as that
// will be sent as '""'.
this.trace.details!.variablesJson![name] = '';
} else {
try {
this.trace.details!.variablesJson![name] = JSON.stringify(
o.variables![name],
);
} catch (e) {
// This probably means that the value contains a circular reference,
// causing `JSON.stringify()` to throw a TypeError:
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#Issue_with_JSON.stringify()_when_serializing_circular_references
this.trace.details!.variablesJson![name] = JSON.stringify(
'[Unable to convert value to JSON]',
);
}
}
}
});
});
}
}

const clientInfo = this.generateClientInfo(o.requestContext);
Expand Down Expand Up @@ -455,3 +466,71 @@ function defaultGenerateClientInfo({ request }: GraphQLRequestContext) {
return {};
}
}

// Creates trace details from request variables, given a policy for modifying
// values of private or sensitive variables.
// The details include the variables in the request, and the privacy enforcer type.
// If enforcePrivateVariables is a bool or Array, it will act similarly to
// to the to-be-deprecated options.privateVariables, except that the redacted variable
// names will still be visible in the UI when 'true.'
// If enforcePrivateVariables is null, the policy will default to the 'true' case.
export function makeTraceDetails(
variables: Record<string, any>,
enforcePrivateVariables?:
| ((v: Record<string, any>) => Record<string, any>)
| Array<String>
| boolean,
): Trace.Details {
const details = new Trace.Details();
let variablesToRecord = variables;
if (typeof enforcePrivateVariables === 'function') {
// Custom function to allow user to specify what variablesJson will look like
variablesToRecord = enforcePrivateVariables(variables);
details.privacyEnforcerType =
Trace.Details.PrivateVariableEnforcerType.FUNCTION;
} else if (Array.isArray(enforcePrivateVariables)) {
details.privacyEnforcerType =
Trace.Details.PrivateVariableEnforcerType.ARRAY;
} else if (typeof enforcePrivateVariables === 'boolean') {
details.privacyEnforcerType =
Trace.Details.PrivateVariableEnforcerType.BOOLEAN;
} else {
details.privacyEnforcerType =
Trace.Details.PrivateVariableEnforcerType.NONE;
}

// Note: we explicitly do *not* include the details.rawQuery field. The
// Engine web app currently does nothing with this other than store it in
// the database and offer it up via its GraphQL API, and sending it means
// that using calculateSignature to hide sensitive data in the query
// string is ineffective.
Object.keys(variablesToRecord).forEach(name => {
if (
enforcePrivateVariables === true ||
enforcePrivateVariables === null ||
(Array.isArray(enforcePrivateVariables) &&
// We assume that most users will have only a few private variables,
// or will just set privateVariables to true; we can change this
// linear-time operation if it causes real performance issues.
enforcePrivateVariables.includes(name))
) {
// Special case for private variables. Note that this is a different
// representation from a variable containing the empty string, as that
// will be sent as '""'.
details.variablesJson![name] = '';
} else {
try {
details.variablesJson![name] = JSON.stringify(variablesToRecord[name]);
} catch (e) {
// This probably means that the value contains a circular reference,
// causing `JSON.stringify()` to throw a TypeError:
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#Issue_with_JSON.stringify()_when_serializing_circular_references
details.variablesJson![name] = JSON.stringify(
'[Unable to convert value to JSON]',
);
}
}
});

return details;
}

0 comments on commit dbce299

Please sign in to comment.