Apollo-client: cache leaks secrets

[danilobuerger]

Intended outcome:

Use a mutation with a secret parameter without it leaking.

Actual outcome:

Mutation with secret parameter is attached to cache.ROOT_MUTATION and is therefor leaking.

How to reproduce the issue:

const LOGIN = gql`
  mutation login($email: String, $password: String) {
    login(email: $email, password: $password)
  }
`;

performing with

login({ variables: { email: "my_email", password: "superSafe" } })

leads to

data.ROOT_MUTATION.login({"email":"my_email","password":"superSafe"}): "some return"

Versions

apollo-cache-inmemory: ^1.2.4 => 1.2.4 
apollo-client: ^2.3.4 => 2.3.4 
apollo-link: ^1.2.2 => 1.2.2 
apollo-link-context: ^1.0.8 => 1.0.8 
apollo-link-error: ^1.1.0 => 1.1.0 
apollo-link-http: ^1.5.4 => 1.5.4 
apollo-link-state: ^0.4.1 => 0.4.1 
react-apollo: ^2.1.5 => 2.1.5

More

This can be circumvented by using the @connection directive:

const LOGIN = gql`
  mutation login($email: String, $password: String) {
    login(email: $email, password: $password) @connection(key: "login", filter: ["email"]) 
  }
`;

leads to:

data.ROOT_MUTATION.login({"email":"my_email"}): "some return"

However, this is not obvious. I would suggest updating the docs with either:
1) Mention that you should never use secret parameters as they will leak
2) Use the @connection directive to filter them out
3) ...

[stubailo]

What type of attack are you concerned about? The browser is already aware of the password since that's where the user typed it in, and your cache will definitely have secret user data in it because that's what it's for. We recommend clearing the entire cache on logout to remove any private data. In general, this is about as safe as storing the password in a JavaScript variable.

[danilobuerger]

A few things first:

• I am not talking about passwords but secrets in general, e.g. credit card, cvv, social security. The example was just chosen for ease of talking about it.
• I am not talking about the browser as apollo-client can be used in a variety of environments, e.g. mobile (via react-native), server (via nodejs). In a server-to-server example one could imagine a credit card fraud check that transfers a credit card number from one system to another. This would also leak to the cache.

Now its never wise to store secrets longer than we have to, because it increases the potential attack surface. Thats why the best practice is to not keep a user password around on a client. Even if the client has seen the password for a very short period. Its not the same thing from a security standpoint.

In any case, if we had a mutation to change a credit card along with the cvv, we wouldn't be pci compliant as the data lingers around. Or in the case of a password, we wouldn't meet for example the BSI IT-Grundschutz:

Passwörter dürfen nicht im Cache des Clients vorgehalten werden, dieses ist server- bzw. anwendungsseitig zu verhindern. Die Autocomplete-Funktion muss für Passwörter ebenfalls deaktiviert sein.

(translates to: don't keep passwords in the cache of a client)-

https://www.bsi.bund.de/DE/Themen/ITGrundschutz/ITGrundschutzKataloge/Inhalt/_content/m/m05/m05174.html

The apollo-client documentation suggest (very hidden!) how to handle sensitive data, but only for queries:

Sometimes it makes sense to not use the cache for a specfic operation. This can be done using either the network-only or no-cache fetchPolicy. The key difference between these two policies is that network-only still saves the response to the cache for later use, bypassing the reading and forcing a network request. The no-cache policy does not read, nor does it write to the cache with the response. This may be useful for sensitive data like passwords that you don’t want to keep in the cache.

https://www.apollographql.com/docs/react/advanced/caching.html#ignore

What I am suggesting is informing the developer that mutations are being cached along with parameters in plaintext and how that can be avoided. Then the developer can make a conscious choice on how to handle the situation. As of now it is not clear from the documentation that this is happening.

[hwillson]

Thanks for your detailed thoughts/comments on this @danilobuerger. I agree, let's get this into the docs. help-wanted here for sure if anyone is interested in submitting a PR. Thanks!

[AudyOdi]

@danilobuerger Hi, I am facing the same issue right now. I would like to ask, you mentioned that "What I am suggesting is informing the developer that mutations are being cached along with parameters in plaintext and how that can be avoided", so is there a way to avoid this? If yes, could you provide some ways in your opinion? That would be really helpful!

[sirugh]

As mentioned by OP the "solution" was to use @connection directives to force ApolloClient to use a combination of string and arguments as a cache key.

In v3 we now can use typePolicies to do the same thing:

```js
const TYPE_POLICIES = {
Mutation: {
fields: {
login: {
keyArgs: () => 'login' // same as @connection(key: "login")
}
}
}
}