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

Conditionally removing fields from introspection based on auth #380

Open
vektah opened this issue Oct 13, 2018 · 18 comments
Open

Conditionally removing fields from introspection based on auth #380

vektah opened this issue Oct 13, 2018 · 18 comments
Labels
accepted enhancement New feature or request

Comments

@vektah
Copy link
Collaborator

vektah commented Oct 13, 2018

Enable consuming apps with different application roles to generate clients with only the methods they have access to.

Note there is terminology overlap here between the end user role and the application role. Each end user would probably still see the same view of the graph.

@vektah vektah added the enhancement New feature or request label Oct 13, 2018
@saward
Copy link

saward commented Dec 1, 2018

I'm interested in hiding certain fields/queries/mutations from the schema introspection depending on whether the user is authenticated or not, and varying based on roles. Am I right in thinking that this issue relates to doing something like that?

@mathewbyrne
Copy link
Contributor

@saward correct.

@sdalezman
Copy link

@vektah @mathewbyrne was looking through the introspection code - would it make sense to add a directive that allows disabling of fields and inputs in the schema similar to how deprecated is currently handled?

If so, think this would be a quick win we'd be able to help out with. Let me know and someone on our team can allocate some time in the next few days to solve this

@mathewbyrne
Copy link
Contributor

I think a specific directive is probably not the right way to implement this; @deprecated is special in that it is a built-in directive from the spec and field deprecation is also a feature from the spec.

So I think this feature needs to be thought out a little more. Currently the only input into introspection is the parsed schema, so I imagine there might be some sort of additional input here that introspection exports so allow granular control of which types are exposed (maybe?). If you guys are keen to address this I'd love to see a proposal.

@sdalezman
Copy link

That's a good point regarding directives - reason I reached for adding a built in directive is that it would be embedded in the workflow of a developer.

I think the other option would be to introduce another key in gqlgen so that we can specify the fields/input to be excluded from introspection. I think this would be a happy medium and fit into the overall workflow of gqlgen. I don't think this should be something that's introduced in the options to GraphQL. To me the GraphQL instantiation is something that should change very infrequently and isn't part of my day to day workflow as a developer.

The nice part of introducing a new key to gqglen is it's already part of my existing workflow (whenever I define queries I always think about 1. updating schema 2. any updates that need to happen to gqlgen.yml).

I can understand not wanting to "pollute" gqlgen, but maybe it becomes a new key under schema called disableIntrospection and it would include a list of fields and inputs to be disabled? That would fit very cleanly into existing workflows I think.

If that works for you we can map out the code updates itself to facilitate that update.

@mathewbyrne
Copy link
Contributor

FWIW I still think this could be exposed as a directive to the end developer, probably through a plugin. But that means that introspection cannot look specifically for a named directive, so you'd need to expose this through configuration as you've described above.

I think this sounds like a reasonable approach, there would be a bit of work involved to inject this configuration into introspection but it doesn't seem too crazy. Keen to get some input from @vektah here as well.

@sdalezman
Copy link

That makes sense! Am curious to get that feedback as well - definitely something that's becoming a larger priority for our team. Thanks!

@stale
Copy link

stale bot commented Aug 28, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 28, 2019
@saward
Copy link

saward commented Aug 28, 2019

I'm still interested in this.

@vvakame
Copy link
Collaborator

vvakame commented Oct 8, 2019

yeah, I found this Issue now. I have some interest in.

@stale
Copy link

stale bot commented Dec 7, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 7, 2019
@vvakame vvakame added accepted and removed stale labels Dec 8, 2019
@vvakame
Copy link
Collaborator

vvakame commented Dec 8, 2019

👀

@jinxmcg
Copy link

jinxmcg commented Jan 3, 2020

interested in this too

@AienTech
Copy link

Any updates on this issue?

@sumanthakannantha
Copy link

@vektah Any updates on this? We are also interested with this feature.

@nsemikov
Copy link

👀

@degarajesh
Copy link

We are desperately waiting for this feature

@duckbrain
Copy link
Contributor

This can be implemented as a plugin.

https://github.com/ec2-software/gqlgen-introspect-filter/blob/v0.1.0/introspection_filter.go

It uses GQLGen middleware around all fields to check if the field is returning a list of introspection types, and filters them down based on a configurable function.

The package also implements a second plugin to sort results to work around #1497

It could be made smarter to automatically filter out fields that use types that are filtered out, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.