Apply privateVariables to properties of custom input variables.

[erikkrieg]

Feature suggestion

Expand the privateVariables functionality to omit properties of custom input variables in Optics traces. Currently variables can be hidden, but properties cannot be targeted specifically.

Use case

Given the following mutation, we want to hide input.password from Optics, while still supporting a flexible mutation interface using inputs.

UpdateUserInput schema:

input UpdateUserInput {
  username: String
  email: String
  password: String
}

Query:

mutation UpdateUser($input: UpdateUserInput!) {
  updateUser(input: $input) {
    updatedAt
  }
}

Variables:

{
  "input": {
    "password": "i<3nsync"
  }
}

It is also possible that in the future we learn we need to start hiding input.email or another property in order to comply with some new regulations.

Workarounds

Right now the workarounds we considered are:

1. Change the schema so that any variables needing to be private are not properties of inputs. This can be challenging depending on how the API is being consumed and may require the mutation to be deprecated.
2. Don't use custom inputs to contain data that might need to be private. This is hard to predict, so the more pragmatic approach would be to never use inputs, but then the API is inflexible and might encounter more breaking changes based on product changes.
3. Make all variables private. This is probably this simplest option, but takes away a lot of value from traces.

For an issue we are encountering we are going with the first workaround, but are not very satisfied with any of the present options.

Interface suggestions

Suggestion A:

By default (or based on a flag) compare variables and their properties against the array of private variables. There are no variables named "password" in the updateUser mutation, but the "input" variable does have a "password" property, so omit it!

Example:

const server = new ApolloServer({
  typeDefs,
  resolvers,
  engine: {
    apiKey: process.env.API_KEY,
    privateVariables: ['password'],
    privateVariablesDeep: true,
  }
});

Suggestion B:

Use an interface like Lodash's get method that can use a string to match a specific variable's properties. This is my personal favorite :P

Example:

const server = new ApolloServer({
  typeDefs,
  resolvers,
  engine: {
    apiKey: process.env.API_KEY,
    privateVariables: ['password', 'input.password'],
  }
});

Suggestion C:

Accept a method to map over the variables and return the values that will be sent to Optics.

Example:

const {
  isPrivateVariable,
  hasPrivateProperty,
  replacePrivateProperties 
} = require('./my-custom-codez');

const server = new ApolloServer({
  typeDefs,
  resolvers,
  engine: {
    apiKey: process.env.API_KEY,
    privateVariables: (variable) => {
      if (isPrivateVariable(variable.name)) return '';
      if (hasPrivateProperty(variable.value)) {
        // find the private property and obscure its value
        const modifiedValue = replacePrivateProperties(variable.value);
        return modifiedValue;
      }
      return variable.value;
    }
  }
});

[danilobuerger]

@martijnwalraven @abernix Isn't that actually a pretty big security risk exposing secret data like this?

[Shahor]

Just realised today that my users' passwords are being sent to apollo engine because of that exactly 😨

I have privateVariables: ['password'] but of course this doesn't work in nested inputs :|

Is there something available as of today to solve this? (couldn't find it)

[erikkrieg]

@Shahor Until this functionality is expanded the easiest thing to do is hotfix your server and make all variables private, if you haven't already.

Then, if it is worth it to have variables in traces, you could change your API to accept passwords only as variables instead of properties on inputs so they can be made private.

[helenwh]

Hi @erikkrieg, @Shahor --

We just published a release of Apollo server (2.7.0-alpha.1) that includes a fix for this issue, using something along the lines of Suggestion C mentioned above: accepting a custom method for modifying variable values as input to an EngineReportingOption. We’ve included that functionality with a new option, sendVariableValues and will be deprecating privateVariables in the future.

Here’s an example custom function input to sendVariableValues. The Apollo server docs here have been updated to describe the option more in detail, and this is the PR making these changes: #2931

Also, we've changed the default reporting settings so that, unless specified, no variable values will be included in the traces.

We’re still looking for feedback on this, so please let us know if you have any ideas on how we might be able to make this better!