Accessing arguments in Pundit authorization

[Amnesthesia]

Is your feature request related to a problem? Please describe.

When authorizing resolvers/mutations with pundit on the resolver level, e.g by declaring a Policy class on the resolver, the object passed to the policy is the mutation/resolver instance.

If we want to authorize based on specific inputs — such as, is the user allowed to create a Comment on the Post in their arguments, it seems we have to resort to hacky ways of accessing arguments.

If we call arguments, we will get:

RuntimeError: Arguments have not been prepared yet, still waiting for #load_arguments to resolve. (Call .arguments later in the code.) (RuntimeError)
from /Users/amnesthesia/.asdf/installs/ruby/3.4.5/lib/ruby/gems/3.4.0/gems/graphql-2.5.14/lib/graphql/schema/resolver.rb:58:in 'GraphQL::Schema::Resolver#arguments'

However, we can access arguments like this: instance.context[:current_arguments][:input]. Here, the arguments are usually available, and will give us access even to the prepared values, but with one important caveat .... They haven't finished resolving yet, so dataloaded values (e.g if a dataloaded value is returned in prepare) aren't always accessible yet if they are inside an InputObject, more than 1 level down

Describe the solution you'd like

This seems to be an implementation detail where pundit on the resolver/mutation level is only intended to check a permission and not depend on any input variables, in which case we could authorize those specific arguments, but what we want to do is something like:

# app/policies/mutation_policy.rb
class MutationPolicy < ApplicationPolicy
  def initialize(user, instance)
    self.user = user
    self.instance = instance
  end

  protected

  def arguments
    # What we would like:
    #  instance.arguments
    # What we have to do instead:
    instance.context[:current_arguments][:input]
  end
end

# app/graphql/mutations/create_comment.rb
class Mutations::CreateComment < BaseMutation
  class Policy < MutationPolicy
    def create?
      self.arguments[:post].organization == self.user.organization
    end
  end

   argument :post_id, as: :post, loads: Types::Post

   def resolve(...)
   end
end

It would also be great if we could scope in the dataloader, rather than apply the scope after the dataloader returns results — we don't want to return a huge result set and then fire a second query to intersect with that result set, we want to apply the scope in the fetching. Since we can't access context[:current_user] in the dataloader, we have resorted to passing current_user as an argument when we initialize it, and then overridden the fetch method to apply policy scoping, but I feel like this is something that should naturally be supported by the integration

Describe alternatives you've considered

What we're currently doing is hacky and depends on specific internal implementation details where we're accessing the arguments through the context, but this only works on the top level and becomes flaky when we get into nested input objects

When authorizing with def authorized?(...), we have access to all input arguments. It doesn't seem fair that if we use pundit, we should not have access to the input arguments.

In our case, it seems like the pundit integration is actually making something that should be pretty easy much more complex. It would be simpler for us to just do:

def authorize?(post:)
  Pundit.policy!(context[:current_user], post).create?
end

But then again, I feel like this should be supported out of the box with the current integration on the resolver?

[rmosolgo]

Hey, thanks for asking about this and sorry for the trouble. The closest thing, out of the box, is to hang off the argument ..., adding pundit_policy_class: ... to manually name the resolver's policy. I sketched it out in a gist here, too: https://gist.github.com/rmosolgo/23626b6092894e39f04549a7dc47faea.

(By default, as I'm sure you found, it uses the loaded object's policy, not the resolver's policy.)

If you want to use the resolver's policy by default, you could add it by overriding def self.argument, for example:

class BaseResolver 
  def self.argument(*args, pundit_policy_class: nil, **kwargs, &block)
    # Add this resolver's policy class as the default if an override wasn't provided:
    if pundit_policy_class.nil? && defined?(self::Policy)
      pundit_policy_class = self::Policy
    end 
    super(*args, pundit_policy_class: pundit_policy_class, **kwargs, &block)
  end 
end

Dos that look like what you had in mind?

[Amnesthesia]

Hey @rmosolgo thanks for the quick reply!

I've thought about that too and we did try this at first, but it's not really a good solution for us, because mutations sometimes have multiple arguments, for example:

class Mutations::UpdateComment < BaseMutation
  argument :comment_id, loads: Types::Comment, as: :comment
  argument :attributes, Types::Comments::CommentInput
end

We have a lot of usecases where we want to do the authorization once, at the top level, and not have to do the authorization on each individual argument. Sometimes we need to access more than one argument for the authorization

I think the closest solution I can think of is not to use the pundit integration for resolvers and override the pundit_role and pundit_policy_class to create an authorized?(..) method

But I don't quite understand why pundit authorizes before arguments are resolved, but authorization with def authorized?(..) authorizes after arguments are resolved, this seems inconsistent?

[rmosolgo]

authorizes before arguments are resolved

Good question -- looking at the code, it seems like it was intentional:

   # ...
   # Check the policy defined on the mutation class before loading arguments

It uses def ready? instead of def authorized?, but I couldn't tell you why anymore 😖 ... (In general, you'd use ready? instead of authorized? if you know you want to quit running the mutation before loading the arguments.)

It wouldn't be too hard to reimplement this integration to use authorized? instead -- then you could use arguments as you were expecting. For example:

# app/graphql/mutations/custom_pundit_integration.rb
# The integration in GraphQL-Pro runs the pundit check in `.ready?` 
# which means that arguments aren't actually available yet. 
# This module runs the check in `.authorized?` instead, so arguments have all been loaded. 
#
# @example Adding pundit integration to mutations 
#   class Mutations::BaseMutation < GraphQL::Schema::Mutation
#     include Mutations::CustomPunditIntegration 
#   end 
module Mutations::CustomPunditIntegration
  def self.included(mutation_class)
    mutation_class.include(GraphQL::Pro::PunditIntegration::HasPunditRole)
    mutation_class.extend(GraphQL::Pro::PunditIntegration::HasPunditRole)
  end 

  def authorized?(**inputs)
    super && if authorized_by_policy?(self, context)
      true
    else
      return false, unauthorized_by_pundit(self.class, self)
    end
  end

  def unauthorized_by_pundit(owner, value)
    raise GraphQL::UnauthorizedError.new(object: value, type: owner, context: context)
  end
end

I didn't update the gist with this code but here's the same example, updated to use this customized integration. It uses @object.arguments[:post] to use the loaded post and check its id:

Using a custom pundit integration to use `.authorized?` instead of `.ready?`

require "bundler/inline"

gemfile do
  gem "graphql"
  gem "graphql-pro"
  gem "pundit"
  gem "ostruct"
end

Post = Data.define(:id)


# app/graphql/mutations/custom_pundit_integration.rb
# The integration in GraphQL-Pro runs the pundit check in `.ready?`
# which means that arguments aren't actually available yet.
# This module runs the check in `.authorized?` instead, so arguments have all been loaded.
#
# @example Adding pundit integration to mutations
#   class Mutations::BaseMutation < GraphQL::Schema::Mutation
#     include Mutations::CustomPunditIntegration
#   end
module CustomPunditIntegration
  def self.included(mutation_class)
    mutation_class.include(GraphQL::Pro::PunditIntegration::HasPunditRole)
    mutation_class.extend(GraphQL::Pro::PunditIntegration::HasPunditRole)
  end

  def authorized?(**inputs)
    super && if authorized_by_policy?(self, context)
      true
    else
      return false, unauthorized_by_pundit(self.class, self)
    end
  end

  # This hook is called when a check fails
  def unauthorized_by_pundit(owner, value)
    raise GraphQL::UnauthorizedError.new(object: value, type: owner, context: context)
  end
end


class CreateCommentPolicy
  def initialize(user, object)
    @user = user
    @object = object
  end

  def create_comment?
    @user.post_ids.include?(@object.arguments[:post].id)
  end
end

class PunditExampleSchema < GraphQL::Schema
  class Post < GraphQL::Schema::Object
    field :id, ID
  end

  class Comment < GraphQL::Schema::Object
    field :message, String
  end

  class CreateComment < GraphQL::Schema::Mutation
    include CustomPunditIntegration
    pundit_role :create_comment
    pundit_policy_class CreateCommentPolicy

    argument :post_id, ID, loads: Post
    argument :message, String
    field :comment, Comment

    def resolve(post:, message:)
      { comment: { message: message } }
    end
  end

  class Mutation < GraphQL::Schema::Object
    field :create_comment, mutation: CreateComment
  end

  def self.object_from_id(id, ctx)
    ::Post.new(id: id)
  end

  def self.resolve_type(abs_t, obj, ctx)
    Post
  end

  mutation(Mutation)

  def self.unauthorized_object(*)
    raise GraphQL::ExecutionError, "You don't have permission for that"
  end
end

query_str = "mutation($id: ID!) { createComment(postId: $id, message: \"Great job\") { comment { message } } }"
# Permitted
pp PunditExampleSchema.execute(query_str, variables: { id: "1" }, context: { current_user: OpenStruct.new(post_ids: ["1", "2", "3"] )}).to_h
# {"data" => {"createComment" => {"comment" => {"message" => "Great job"}}}}

# Not permitted
pp PunditExampleSchema.execute(query_str, variables: { id: "1" }, context: { current_user: OpenStruct.new(post_ids: ["4", "5", "6"] )}).to_h
# {"errors" => [{"message" => "You don't have permission for that", "locations" => [{"line" => 1, "column" => 22}], "path" => ["createComment"]}], "data" => {"createComment" => nil}}

How about adding a custom integration like that to your app?

[Amnesthesia]

@rmosolgo Yeah we already have a concern that streamlines the authorization, will probably need to add something like this to it instead of what we're currently doing. I was trying to avoid rolling a custom integration and stick to the built-in integration as much as possible