Giter Site home page Giter Site logo

Comments (8)

stanishev avatar stanishev commented on June 6, 2024 1

If you want, feel free to open a PR to mention it. Perhaps, we could add a separate section for it, something like Introspection query authorization after the Error handling with a simple example of the policy object?

👍
will do

from graphql-guard.

exAspArk avatar exAspArk commented on June 6, 2024

Hey Vlad!

Thanks for opening the issue!

Do I understand correctly that you don't want to perform authentication to the fields like the following?

query {
  __schema {
    types {
      name
    }
  }
}

By default graphql-guard is disabled. So, it basically performs an authentication only if you specified guard on your field:

field :posts, !types[!PostType] do
  guard ->(obj, args, ctx) { false }
end

Or your policy object with the guard method doesn't return nil:

class GraphqlPolicy
  def self.guard(type, field)
    ->(obj, args, ctx) { false }
  end
end

If you use '*' with the policy objects, then you can either whitelist the fields:

class GraphqlPolicy
  RULES = {
    QueryType => {
      '*': ->(obj, args, ctx) { false },
      __schema: ->(obj, args, ctx) { true } # always accessible
    }
  }

  def self.guard(type, field)
    RULES.dig(type, field)
  end
end

Or don't use '*' and blacklist the fields:

class GraphqlPolicy
  RULES = {
    QueryType => {
      posts: ->(obj, args, ctx) { false }
    }
  }
end

Please let me know whether the solutions work for you.

from graphql-guard.

stanishev avatar stanishev commented on June 6, 2024

Hi exAspArk,

yes that's exactly what I was referring to, thank you for the thorough walkthrough.

We opted for a whitelist policy, using a policy object very similar to the one in your example.

The reason I thought it might make sense to bake an option into the the gem to allow someone to whitelist all introspection types in one go, is that following the sample code in the usage guidelines we started enumerating all of them in the RULES hash and it quickly started feeling very hacky. Listing GraphQL(-ruby) types in the whitelist rules felt very out of place. Also as new types are added to the graphql-ruby gem in the future it implies things will break at some point.

We worked around this by using: type.introspection? in the guard method, which is simple enough and catches all introspection types (__Schema, __Field, _InputType, etc).

if type.introspection? || WHITELIST.dig(type,field)

So that works fine and doesn't feel too hacky. It should be future proof too.

Also I realize from reading your response above that if someone opts for the WHITELIST approach then they should be prepared to whitelist everything - including having to deal with introspection fields - even if they come as a surprise :). After all that's the definition of a whitelist.

It may still make sense to give introspection fields some special treatment, only because I can't imagine anyone wanting to whitelist just a few of them. They'll either be whitelisted as a group, or not at all.

But I am just thinking out loud at this point. :)

from graphql-guard.

exAspArk avatar exAspArk commented on June 6, 2024

Hey, @stanishev!

It may still make sense to give introspection fields some special treatment, only because I can't imagine anyone wanting to whitelist just a few of them.

Your point makes perfect sense 👍

However, seems like I was wrong, the following code doesn't affect introspection queries (at least in my case):

QueryType => {
  __schema: ->(obj, args, ctx) { false }
}

I'm still able to send an introspection query succesfully. Seems like graphql-ruby itself treats them differently:

query IntrospectionQuery {
  __schema {
    queryType { name }
    mutationType { name }
    subscriptionType { name }
    ...
  }
}

We worked around this by using: type.introspection? in the guard method, which is simple enough and catches all introspection types (__Schema, __Field, _InputType, etc).

Could you please describe what was the problem in your case? What do you have in the schema and which queries do you send? Which versions of graphql and graphql-guard do you use? If you remove type.introspection?, how does it affect the response?

Happy to submit a PR if you think this is a sensible idea.

Will be glad to accept your PR if we're able to identify and reproduce the problem 🙌

from graphql-guard.

stanishev avatar stanishev commented on June 6, 2024

hi @exAspArk

However, seems like I was wrong, the following code doesn't affect introspection queries (at least in my case):
QueryType => {
__schema: ->(obj, args, ctx) { false }
}

yes, having that snippet in the whitelist rules did not work for us either as written. For this to work we'd have to use fully qualified graphql-ruby types (some pseudo code below, won't compile):

class GraphqlWhitelistPolicy
  @no_login_required = ->(_, _, _) { true }
  WHITELIST = {
      GraphQL::Introspection::FieldType => Set[:description, etc],
      GraphQL::Introspection::TypeType => Set[:kind, etc.]
      # etc
  }.freeze

  def self.guard(type, field)
    return @no_login_required if(WHITELIST[type] && WHITELIST[type].include?(field))
    ->(_, _, ctx) { // some login logic }
  end
end

but in any case, we moved away from enumerating these in the WHITELIST hash, and instead used the type.introspection? method as mentioned.

so the first line in the self.guard method above becomes:
return @no_login_required if type.introspection? || (WHITELIST[type] && WHITELIST[type].include?(field))

Could you please describe what was the problem in your case?

I would no longer call it a problem because the graphql-guard gem's whitelist policy does work as advertised and because handling the introspection fields turned out to be so simple. But it did feel as a bit of a surprise to have to look into. Again, in any case, we ran into it because:

  • we use the graphiql app to test.
  • the right most pane of the graphiql app - called Documentation Explorer displays the schema being served by the client. graphiql populates that pane after querying the server with an introspection query.
  • you can manually request a reload of the schema from graphiql if hit Ctl+R (and graphiql will send an introspection query).
  • we do this all the time during development because as we're building our endpoints we frequently change the schema and retest.
  • and when we switched to a WHITELIST approach for the guard gem, we started getting errors like "message": "Not authorized to access __Schema.queryType",
  • this is where we realized that if we want to use a whitelist approach and if we wanted introspections initiated by graphiql (or any client requesting the schema from our server) to work without authorization, then we'd have to do something about it.

I suspect that even teams that don't use the graphiql app would run into a similar situation if they attempted a whitelist policy with graphql-guard, because most teams using graphql seem to be making their schema publicly available (github's schema for example).

As far as addressing the "problem" :) I would have suggested adding a new method to guard.rb

def whitelist_introspection?(type)
  type.introspection? && introspection_whitelisted 
  #introspection_whitelisted is a private field on the Guard class, true by default, 
  # but settable to false by the user on calling GraphQL::Guard.new
end

and a call to this method from the guard_proc method in guard.rb

or alternatively, just mentioning the type.introspection? call somewhere in the examples you provide in the readme file would be sufficient to point people in the right direction.

from graphql-guard.

exAspArk avatar exAspArk commented on June 6, 2024

@stanishev thanks a lot for explaining!!

I was confused by the fact that guard wasn't triggered for __Schema but it works for __Schema.queryType.

I think that for now, we can mention your type.introspection? solution in the README file 👍 Don't want to make the gem complicated with multiple configuration options unless it's being used very often :)

If you want, feel free to open a PR to mention it. Perhaps, we could add a separate section for it, something like Introspection query authorization after the Error handling with a simple example of the policy object?

from graphql-guard.

 avatar commented on June 6, 2024

Can you release this in 1.0.1 please?

from graphql-guard.

 avatar commented on June 6, 2024

I tried with the master branch in my Gemfile but it still doesn't work: #7

from graphql-guard.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.