jsonapi-authorization icon indicating copy to clipboard operation
jsonapi-authorization copied to clipboard

Add a friendly error when Authorizer class is called with a `nil` record

Open jsmestad opened this issue 7 years ago • 2 comments

I am trying to use Pundit scopes and I am unable to get jsonapi-authorization to return a 404 when the user does not have a record.

Am I doing something wrong? I've tried changing the code to have the scope return scope.where(owner: user) without the #all call, but that still does not work.

Example Code

class AccountPolicy < ApplicationPolicy
  class Scope < Scope
    def resolve
      scope.where(owner: user).all
    end
  end

  def show?
    record.exists?
  end
end

Stacktrace

Internal Server Error: unable to find policy of nil /Users/justinsmestad/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/pundit-1.1.0/lib/pundit/policy_finder.rb:58:in `policy!'
/Users/justinsmestad/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/pundit-1.1.0/lib/pundit.rb:112:in `policy!'
/Users/justinsmestad/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/pundit-1.1.0/lib/pundit.rb:62:in `authorize'
/Users/justinsmestad/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/jsonapi-authorization-1.0.0.alpha6/lib/jsonapi/authorization/default_pundit_authorizer.rb:40:in `show'
/Users/justinsmestad/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/jsonapi-authorization-1.0.0.alpha6/lib/jsonapi/authorization/authorizing_processor.rb:61:in `authorize_show'
# ....

jsmestad avatar Jan 23 '18 17:01 jsmestad

TL;DR

This was a user error. It would be great for this gem to have a wiki entry on how to handle pundit scopes. For example if I want to scope lookups to current_user and return a 404 (like described in #76)

Long version

It appears the issue was with JSONAPI::Resources and customizing find_by_key. Removing that made things work. Side note, this was the advised approach from the JR wiki

What I had done in there was:

  def self.find_by_key(_key, options={})
    context = options[:context]
    self.new(context[:user].account, context)
end

What I need to change it to:

  def self.find_by_key(key, options={})
    context = options[:context]
    fail JSONAPI::Exceptions::RecordNotFound.new(key) if context[:user].billing_account.nil?
    self.new(context[:user].billing_account, context)
  end

jsmestad avatar Jan 23 '18 17:01 jsmestad

Oh so due to your customization, a nil record was passed into the authorization code and then we tried to find a policy class for that nil?

That sounds like something that is nasty to debug. We could add a friendlier error in case that happens. Would you be open to create a PR to error in a friendly way if a nil record gets passed in to the authorizer class?

valscion avatar Jan 24 '18 06:01 valscion