Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Variable Declaration Directives / Secret Hiding #1997

Closed
clayplumridge opened this issue May 26, 2020 · 2 comments · Fixed by #6825
Closed

Variable Declaration Directives / Secret Hiding #1997

clayplumridge opened this issue May 26, 2020 · 2 comments · Fixed by #6825
Assignees
Labels
🌶️ hot chocolate 🔍 investigate Indicates that an issue or pull request needs more information. 📌 pinned ⚖️ spec Implement or fix a GraphQL specification item.
Milestone

Comments

@clayplumridge
Copy link

Our Scenario

We're using a DiagnosticObserver to log query executions along with their payloads:

[DiagnosticName(DiagnosticNames.StartQuery)]
public void BeginQueryExecute(IQueryContext context)
{
    _logger.LogInformation("Beginning GQL Query Execution\nQuery: {Query}\nParams: {Params}", context.Request.Query.ToString(), String.Join(' ', context.Request.VariableValues.Select(x => $"[{x.Key}, {x.Value.ToJson()}]")));
}

However, we sometimes communicate secrets to HotChocolate, and don't want to store those in logs. What we'd like to do is allow consumers to identify input variables as Secrets, detect that directive in our logger, and instead use "****" when stringifying it within the logger.

Eg:

query myCustomQuery($id: String!, $mySecret: String! @secret) {
    /* Other stuff goes here */
}

For the variables, would produce:

[id, "SomeValue"] [mySecret, "****"]

However, when I attempt something like this, once HotChocolate parses the query, it outputs as (on this log line):

query myCustomQuery($id: String!, $mySecret: String!) {
    /* Other stuff goes here */
}

Which has the directive dropped, implying that it isn't getting parsed correctly / at all. In this scenario, I've not defined a DirectiveType that would correspond to this Directive, which I suppose could be a cause. Which DirectiveLocation should I use for this if that's the problem?

The GQL spec is... fuzzy as to whether this is a supported scenario or not. I did find PRs that are almost 2 years old attempting to test HotChocolate's ability to consume these kinds of directives, but it's unclear to me if this is still supported or not.

If this method of interaction isn't supported, do you have any other suggestions as to how to make this work? Something like a Query-level directive we've also considered, but it's not nearly as good:

query myCustomQuery($id: String!, $mySecret: String!) @secrets(variableNames: ["mySecret"]) {
    /* Query Body */
}

Thanks! And thank you for developing and maintaining this project; my team is using it to great effect and we really appreciate the time, effort, and care that's gone into it.

@michaelstaib
Copy link
Member

The 2018 GraphQL Spec is quite clear on that this is not supported:
http://spec.graphql.org/June2018/#sec-Type-System.Directives

However, the upcoming GraphQL Spec 2020 will add support for that. We also have implemented this already. It might just be that the query syntax printer has a bug and does not output it.

I will have a look at this.

@michaelstaib michaelstaib self-assigned this May 28, 2020
@michaelstaib michaelstaib added ⚖️ spec Implement or fix a GraphQL specification item. 🌶 hot chocolate 🔍 investigate Indicates that an issue or pull request needs more information. labels May 28, 2020
@michaelstaib michaelstaib added this to the HC-10.5.0 milestone May 28, 2020
@michaelstaib michaelstaib modified the milestones: HC-10.5.0, HC-10.6.0 Jul 30, 2020
@michaelstaib michaelstaib modified the milestones: HC-10.6.0, HC-11.0.0 Sep 18, 2020
@michaelstaib michaelstaib modified the milestones: HC-11.0.0, HC-11.1.0 Nov 23, 2020
@PascalSenn PascalSenn modified the milestones: HC-2021-03, HC-2021-06 Mar 22, 2021
@michaelstaib michaelstaib modified the milestones: HC-2021-08, HC-2021-10 Sep 15, 2021
@michaelstaib
Copy link
Member

This should be possible now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌶️ hot chocolate 🔍 investigate Indicates that an issue or pull request needs more information. 📌 pinned ⚖️ spec Implement or fix a GraphQL specification item.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants