Consider Logger option to mask based on argument name (not variable name)

[dillonkearns]

Hello @benwilson512 @bruce! I'm the author of elm-graphql and I was wondering your opinion on something related to logging in Absinthe. In a nutshell, the problem that I'm running into is that Absinthe assumes that you use variables in order to mask sensitive data. But elm-graphql doesn't have GraphQL variables. Here's a little background as to why.

Why there are no variables in elm-graphql

elm-graphql has a philosophy to use elm constructs rather than GraphQL constructs wherever possible. GraphQL variables are one of the most interesting cases of this. There's no need for variables because elm has constants, functions, modules, variables... so you can do anything you would want to do with a variable in your elm-graphql code. Adding GraphQL variables would add significant complexity from the developer ergonomics and the complexity of the type annotations, and for little benefit.

Masking assumes variables in Absinthe

However, the Absinthe logger relies on variable names to decide which values to mask in the logs. The reasoning given in this issue is that it is more performant to filter based on the JSON that's passed in for variables. This is a totally reasonable decision to arrive at. I'd like to offer some reasons to consider supporting an option to filter based on argument name as well.

Reasons to consider an option to mask based on argument name (not variable name)

Single source of truth

My thinking is that it would be less error-prone to have the knowledge of what is masked in one place, instead of split between client and server.

Public-facing API instructions

Also, consider the case where there is a public-facing API. You wouldn't want to put a note in your docs that says "please be sure to pass in password args under a variable named password, otherwise it will be logged in our server logs."

Conclusion

So it seems like it would be useful, at least for some standard use cases, to support the ability to mask based on argument names (and parse the GraphQL query if that option is set). It appears that there is currently no way to configure this or use a custom logger.

Here's the issue in the elm-graphql repo if you are curious to see more context: dillonkearns/elm-graphql#27

Thanks for taking the time to read. I would love to hear any thoughts you have on this topic. Thank you!

[benwilson512]

Hi @dillonkearns. I do think this is a useful feature. However, I also find the stance taken in that elm-graphql channel quite puzzling, and not just for the implications it has for input security.

Fundamentally, the issue is that today the query string is logged before it's parsed. Excising data from the query string requires that it is parsed, iterated over, and then turned back into a string, and then logged. You need to account for the possibility that the client sent invalid GraphQL. None of this is impossible, but it's definitely work. With the work we have before the core team already, this feature would definitely be something that would need to be solved by an external contributor.

I'll elaborate on my thoughts WRT Elm + variables in the elm issue.

[benwilson512]

Handled via documentation.