Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Hide parts of the schema #782

Closed
stayallive opened this issue Feb 27, 2021 · 1 comment · Fixed by #1434
Closed

Hide parts of the schema #782

stayallive opened this issue Feb 27, 2021 · 1 comment · Fixed by #1434

Comments

@stayallive
Copy link
Contributor

So this already came up some times before: #649 & #405. But it never got anywhere

So I'm here to see if there is interest in getting this in even or that it's out of scope.

I'm really impressed by how GraphQL-Ruby has implemented this.

With GraphQL-Ruby, it’s possible to hide parts of your schema from some users. This isn’t exactly part of the GraphQL spec, but it’s roughly within the bounds of the spec.

They list some great arguments about why this is useful:

Here are some reasons you might want to hide parts of your schema:

  • You don’t want non-admin users to know about administration functions of the schema.
  • You’re developing a new feature and want to make a gradual release to only a few users first.

This is in part a security through obscurity functionality BUT it's also a really functional feature to be able to have feature switches and test out new parts of the schema in production without it being part of the public schema immediately.

That last part (feature flagging, incremental rollout) is what interests me most about this feature (see for example how GitHub is using it to hide their discussions API behind a feature flag.


I have been browsing around and I think a possible (from the user perspective) implementation could look something like this:

$userType = new ObjectType([
    'name' => 'User',
    'description' => 'Our blog visitor',
    'fields' => [
        'firstName' => [
            'type' => Type::string(),
            'description' => 'User first name'
        ],
        'email' => Type::string(),
        'internalId' => [
            'type' => Type::string(),
            'description' => 'Example field that could be hidden'
+           'visible' => function ($context): bool { /.../ },
        ],
    ],
+   'visible' => function ($context): bool { /.../ },
]);

GraphQL-Ruby implements the following rules for the visible callback result:

These methods are called with the query context, based on the hash you pass as context:. If the method returns false, then that member of the schema will be treated as though it doesn’t exist for the entirety of the query. That is:

  • In introspection, the member will not be included in the result
  • In normal queries, if a query references that member, it will return a validation error, since that member doesn’t exist

These rules would as far as I'm concerned also apply to graphql-php since they seem reasonable and go a little further than simply hiding them from the introspection, they basically remove them from the schema entirely so they also cannot be called.


For a bit more detail check out the excellent GraphQL-Ruby docs page: https://graphql-ruby.org/authorization/visibility.

I'm mainly here to see IF this is even something that would be considered for inclusion in graphql-php so please give me some feedback on the idea (which is a shameless copy of the GraphQL-Ruby implementation and not a original idea) so I know if I should be moving forward or that it won't be accepted at all 😄

@spawnia
Copy link
Collaborator

spawnia commented Feb 27, 2021

I think this is a useful addition and does not violate the specification.

It seems that only introspection will be affected at runtime and has to do a bit of extra work. Apart from that, there should be no negative effects from adding this.

Given the implementation is simple enough, i would be up for inclusion 👍

@vladar vladar closed this as completed Mar 25, 2021
@webonyx webonyx locked and limited conversation to collaborators Mar 25, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants