Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Class based API: parent object passed to policy is instance of the object type class, not an instance of the model class. #53

Open
haizop opened this issue Jun 21, 2018 · 5 comments
Labels

Comments

@haizop
Copy link

haizop commented Jun 21, 2018

When calling authorize, the 'parent object' passed to the policy is not an instance of the relevant model, it is instead an instance of the object type.

# Gemfile
gem 'graphql-pundit', '~> 0.7.1'

...

class Types::BaseObject < GraphQL::Schema::Object
  field_class GraphQL::Pundit::Field
end

...

class AppSchema < GraphQL::Schema
  query Types::Query
end

...

class Types::Query < Types::BaseObject
  field :current_user, Types::User, null: true

  def current_user
    context[:current_user]
  end
end

...

class Types::User < Types::BaseObject
  field :test_pundit, String, null: true do
    authorize ->(obj, args, ctx) {
      binding.pry
      # obj.class => Types::User
      # obj.object.class => User
    }
  end

  # compare with class of obj as passed to resolver.
  field :test, String, null: true, resolve: ->(obj, args, ctx) { 
    binding.pry
    # obj.class => User
  }
end
@haizop
Copy link
Author

haizop commented Jun 22, 2018

@phyrog - Am I missing something here? That is not the intended behavior is it?

@phyrog
Copy link
Collaborator

phyrog commented Jun 22, 2018

No, you're right, that shouldn't happen. The fix for authorize should be pretty easy, but scope is also affected, and that is possibly a bit more complicated.

Edit: The reason for scope being more complicated is that the return values of the scopes need to be wrapped with a graphql type before calling the resolve method or returning; the easiest solution is probably just to take the class of the originally passed in parent object and wrap the return value in that, but I'm not sure yet if that already solves the problem.

Side note: the internals of graphql-ruby are currently really complicated with the new API building on top of the old API, but also mixing in new code into the old stuff (e.g. the new resolver stuff that wraps values in the type classes). I hope this will become cleaner in the future.

@phyrog phyrog added the bug label Jun 22, 2018
@haizop
Copy link
Author

haizop commented Jun 22, 2018

Thanks for the explanation.

@jejacks0n
Copy link

I came across this and fixed it by defining a policy_class instance method on my base type. For me this is Types::ApplicationType, but it could just as easily be BaseObject or whatever.

So my Types::UserType that manages the User model can share policies -- in this case the UserPolicy. I chose the instance method so if the Types::UserType needs to override it, it's cleaner.

  def policy_class
    self.class.name.gsub(/^Types::(\w+)Type$/, "\\1Policy")
  end

@phyrog
Copy link
Collaborator

phyrog commented Aug 1, 2018

Quick heads up from me: While I no longer officially work for Ontohub, I will still support this Gem in the future. However: I don't think there currently is a fix for this issue that doesn't involve rewriting large portions of the code; that, and the planned graphql-ruby support for authorization and scopes mean, that this issue will probably have to wait for a while until the situation on the graphql-ruby side has settled down a bit.

In the meantime, I would suggest either using the define-based API (at least for the types that need authorization) or the workaround by @jejacks0n.

On the other hand: If someone wants to take a shot at this, feel free to create a PR 😁

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants