Query-level schema restrictions

[rmosolgo]

Here's a take on #269

For a given query, make a schema appear smaller than it really is. This way, you can restrict access (even visibility) to certain types, fields, arguments or enum values within a schema.

# Objects are hidden when `.call(schema_member)` returns truthy
top_secret = -> (schema_member) { schema_member.metadata[:top_secret] }
Schema.execute(query_string, except: top_secret)

In fact, the only requirement of a "mask" is that it responds to #call(member), so you can write your own, too!

[cjoudrey]

One reason I'm trying a dynamic approach is because it could lay the groundwork for query-level masks. Once the filtering is in place, you can add the query as an input, too.

Love that you're thinking about this query-based use-case.

@xuorig and I were talking about this last week for feature-flagged fields/types. Imagine you want to give a specific API client early-access to some feature being built. There is far too many possible permutations to pre-calculate all possible schemas in advance.

Instead you might be able to have a query-analyzer (or a static validator) to prevent unauthorized clients from resolving the feature-flagged fields. You'd also want some way to hook into the introspection schema to omit the feature-flagged fields/types when unauthorized client requests schema.

[kdaigle]

Instead you might be able to have a query-analyzer (or a static validator) to prevent unauthorized clients from resolving the feature-flagged fields. You'd also want some way to hook into the introspection schema to omit the feature-flagged fields/types when unauthorized client requests schema.

We've discussed this approach internally as well and become concerned that you'd need resolved objects to possibly check feature flags against which you couldn't do in a query analyzer. I like the idea of the masks quite a bit but, as @cjoudrey pointed out, eventually there would be some crazy multiple of permutations. Hmm.

[cjoudrey]

@kdaigle I think you could achieve this with a query analyzer since you have access to the GraphQL::Query, the query's context and the type/field definitions for the query.

Suppose you added a GraphQL::Field.accepts_definitions(required_feature_flag: ...) and have context[:api_client] you should be able to write a query analyzer that does something like:

def call(memo, visit_type, irep_node)
  if irep_node.ast_node.is_a?(GraphQL::Language::Nodes::Field) && visit_type == :enter
    irep_node.definitions.each do |type_defn, field_defn|
      if field_defn.required_feature_flag && memo[:context].has_feature_flag?(field_defn.required_feature_flag)
        # raise GraphQL::ExecutionError or AnalysisError or context.errors.add
      end
    end
  end
end

Edit: You'd have to check for fragments too.

[kdaigle]

Yeah, that'd work assuming you're keying off of something that's always in context. Sometimes, we key off of objects that aren't in context (for example, a repository and not the current user). We'd need that repository before we could flag it. It's not a deal breaker at all but I figured I'd mention that, for us, we'd likely need to rely heavily on the masks.

[rmosolgo]

An analyzer might work, depending on your privacy requirements. For example, if a certain enum value is only visible to some users, you couldn't remove it from an introspection result with an analyzer alone.

That would tell GraphiQL users that a certain value is valid for them ... even though it's not 😿

Even if an analyzer-based solution did work, it would be bummer to basically reimplement some gem internals (validation, introspection at least) in user code.

That's why I want to provide this in the library. It's a really nice feature for large-scale GraphQL schemas and the best implementation is ... right at the heart of the library :)

I pushed a hot take on query-level masking: 2260b7e It seems promising!

[cjoudrey]

For example, if a certain enum value is only visible to some users, you couldn't remove it from an introspection result with an analyzer alone.

Correct, you need both. The analyzer was simply to prevent people from using the hidden things. The introspection resolvers still need to be modified in order to hide the things in the first place.

It's a really nice feature for large-scale GraphQL schemas and the best implementation is ... right at the heart of the library :)

💯

I pushed a hot take on query-level masking: 2260b7e It seems promising!

👍

[cjoudrey]

Could you think of other use cases where metadata can be helpful in the future?

[rmosolgo]

It's definitely useful -- it's the basis for things like field_defn.required_feature_flag in your example above (that is, arbitrary data attached to schema members).

Right now it's only supported via

GraphQL::BaseType.accepts_definitions(
  required_feature_flag: GraphQL::Define.assign_metadata_key(:required_feature_flag)
)

and then

MyType = GraphQL::ObjectType.define do 
  required_feature_flag :top_secret_access 
end 

But with this you could "just" do

MyType = GraphQL::ObjectType.define do 
  metadata :required_feature_flag, :top_secret_access
end 

I added it because I wanted to mess with metadata without extending every schema in the test suite! Maybe it would be useful IRL to have a helper like that too.

[rmosolgo]

But, it's more prone to typos, that's a bit sad

[cjoudrey]

I asked because I'm always a bit wary with exposing key/value stores like that. lol

it's more prone to typos, that's a bit sad

Right!

MyType = GraphQL::ObjectType.define do 
  required_feature_flag :top_secret_access 
end

Letting people expose this on their own via accepts_definitions felt less prone to errors (and typos). It also means we don't need to expose metadata in the define block.

I could be wrong though. :)

[rmosolgo]

👍 I moved it to spec_helper.rb so it's there for testing but not default for the gem :)

[rmosolgo]

You can see the API I sketched out here:

graphql-ruby/lib/graphql/schema/mask.rb

Line 3 in bf75ccb

# This object can restrict access to schema members on a query-by-query basis.

How does that look to you?

I was considering between these two:

# 1) Yield the schema member 
Schema::Mask.new { |member| ... } 
# 2) Yield the schema member and the query context 
Schema::Mask.new { |member, ctx| ... }

I opted for 1) so that a mask could be independent of a GraphQL::Query instance. That way, in the future, you could apply a mask to a schema dump (Schema::Printer.print_schema), too.

You can accomplish the equivalent of 2) by closing over query-level values with a function or an object:

@current_user = user_from_session
# Access outer scope in the block 
mask = Schema::Mask.new { |member| @current_user.can_see?(member) } 
# Or pass it into another object
mask = CustomUserBasedMask.new(@current_user)

Schema.execute(query_string, mask: mask)

[jamesmacaulay]

Personally I didn't notice at first that the result of the provided block is negated in Mask's implementation of visible?. The fact that the block result is the opposite of the method result was confusing to me, and something that I think I would always end up having to look up or figure out through trial and error.

One alternative would be to use bare Proc objects without any Mask wrapper, and use a keyword param that is more explicit about the interface. For example:

top_secret = ->(schema_member) { schema_member.metadata[:top_secret] }
Schema.execute(query_string, except: top_secret)

Just an idea.

[rmosolgo]

👍 for except: <#call(member)> API! That's much clearer, I'll make the refactor soon.

[rmosolgo]

Also, implementing only: would be a breeze from here if we ever need it!

[rmosolgo]

Oops, realized I missed a spot with variables (and fixed it in 7b91e3f), where it was still possible to pass in hidden enum values or input object fields. If anyone gets a chance to think through any other "leaks", I'd love to hear the outcome of that!

[rmosolgo]

I added some docs and lots of caching on Warden's result, but this feature still adds quite a bit of overhead, running the introspection schema is 10% slower

~/code/graphql $ git co master
Switched to branch 'master'
~/code/graphql $ ruby -Ilib:spec/support introspection_benchmark.rb
Warming up --------------------------------------
           run query     3.000  i/100ms
Calculating -------------------------------------
           run query     33.294  (± 3.0%) i/s -    168.000  in   5.050755s
~/code/graphql $ ruby -Ilib:spec/support introspection_benchmark.rb
Warming up --------------------------------------
           run query     3.000  i/100ms
Calculating -------------------------------------
           run query     33.386  (± 3.0%) i/s -    168.000  in   5.037097s
~/code/graphql $ ruby -Ilib:spec/support introspection_benchmark.rb
Warming up --------------------------------------
           run query     3.000  i/100ms
Calculating -------------------------------------
           run query     34.060  (± 2.9%) i/s -    171.000  in   5.026944s
~/code/graphql $ git co masks
Switched to branch 'masks'
~/code/graphql $ ruby -Ilib:spec/support introspection_benchmark.rb
Warming up --------------------------------------
           run query     2.000  i/100ms
Calculating -------------------------------------
           run query     29.056  (± 6.9%) i/s -    146.000  in   5.039352s
~/code/graphql $ ruby -Ilib:spec/support introspection_benchmark.rb
Warming up --------------------------------------
           run query     2.000  i/100ms
Calculating -------------------------------------
           run query     31.288  (± 6.4%) i/s -    158.000  in   5.064336s
~/code/graphql $ ruby -Ilib:spec/support introspection_benchmark.rb
Warming up --------------------------------------
           run query     2.000  i/100ms
Calculating -------------------------------------
           run query     29.425  (± 3.4%) i/s -    148.000  in   5.039897s
~/code/graphql $ ruby -Ilib:spec/support introspection_benchmark.rb
Warming up --------------------------------------
           run query     3.000  i/100ms
Calculating -------------------------------------
           run query     26.531  (±15.1%) i/s -    132.000  in   5.067148s

To be honest, it's not even clear that the caching improved the situation, so maybe I should remove it:

~/code/graphql $ git co 065e9bde5f12d336eb368fc1ed834fdf94fd11a7
HEAD is now at 065e9bd... refactor(BaseType#validate_input) thread the Warden through all input validation
~/code/graphql $ ruby -Ilib:spec/support introspection_benchmark.rb
Warming up --------------------------------------
           run query     3.000  i/100ms
Calculating -------------------------------------
           run query     30.811  (± 6.5%) i/s -    156.000  in   5.082216s
~/code/graphql $ ruby -Ilib:spec/support introspection_benchmark.rb
Warming up --------------------------------------
           run query     3.000  i/100ms
Calculating -------------------------------------
           run query     31.348  (± 3.2%) i/s -    159.000  in   5.076198s
~/code/graphql $ ruby -Ilib:spec/support introspection_benchmark.rb
Warming up --------------------------------------
           run query     3.000  i/100ms
Calculating -------------------------------------
           run query     32.714  (± 3.1%) i/s -    165.000  in   5.050744s
~/code/graphql $ git co 9de13412b5b16500263299f189db429f22f18434
Previous HEAD position was 065e9bd... refactor(BaseType#validate_input) thread the Warden through all input validation
HEAD is now at 9de1341... feat(Warden) cache generated results too
~/code/graphql $ ruby -Ilib:spec/support introspection_benchmark.rb
Warming up --------------------------------------
           run query     3.000  i/100ms
Calculating -------------------------------------
           run query     31.463  (± 3.2%) i/s -    159.000  in   5.060034s
~/code/graphql $ ruby -Ilib:spec/support introspection_benchmark.rb
Warming up --------------------------------------
           run query     2.000  i/100ms
Calculating -------------------------------------
           run query^[[A     31.937  (± 3.1%) i/s -    160.000  in   5.019291s
~/code/graphql $ ruby -Ilib:spec/support introspection_benchmark.rb
Warming up --------------------------------------
           run query     2.000  i/100ms
Calculating -------------------------------------
           run query     31.050  (± 3.2%) i/s -    156.000  in   5.032587s

[rmosolgo]

I'm gonna push without caching since it doesn't seem to be much of a win, and make up the time someplace else.

[cjoudrey]

🎉!