nathanl / authority

*CURRENTLY UNMAINTAINED*. Authority helps you authorize actions in your Rails app. It's ORM-neutral and has very little fancy syntax; just group your models under one or more Authorizer classes and write plain Ruby methods on them.
MIT License
1.21k stars 67 forks source link

authorize_action_for: wrong number of arguments when using Sequel gem #100

Closed sporto closed 9 years ago

sporto commented 9 years ago

I am trying Authority with the sequel gem, this is my setup:

models/Enquiry.rb

class Enquiry < Sequel::Model
    include Authority::Abilities

    ...
end

controllers/enquiries_controller.rb

class EnquiriesController < BaseController
    authorize_actions_for Enquiry

    def index
        render json: ...
    end
end

I get this error:

ArgumentError (wrong number of arguments (10 for 3..4)):
  authority (3.0.0) lib/authority.rb:36:in `enforce'
  authority (3.0.0) lib/authority/controller.rb:119:in `authorize_action_for'
  authority (3.0.0) lib/authority/controller.rb:136:in `run_authorization_check'

Any ideas? Thanks

nathanl commented 9 years ago

Hmmm. I wonder what *Enquiry produces on this line. I'd expect it to produce a one-item array containing the class.

Try this: bundle open authority and puts all the arguments about to be passed to enforce here

sporto commented 9 years ago

This is what I found

    def run_authorization_check
      Rails.logger.debug '- instance_authority_resource'
      Rails.logger.debug instance_authority_resource.inspect # Enquiry
      authorize_action_for(*instance_authority_resource)
    end

In run_authorization_check instance_authority_resource == Enquiry sequel model

    def authorize_action_for(authority_resource, *options)
      Rails.logger.debug 'authorize_action_for'
      Rails.logger.debug authority_resource.inspect # <Enquiry @values={:id=>1, :state=>nil, :suburb=>nil, :postcode=>nil, :person_1_id=>1, :person_2_id=>2, :created_at=>2015-05-29 14:23:54 +1000, :updated_at=>2015-05-29 14:24:31 +1000}>

     ...
    end

But at this point authority_resource has become the first record in the db. So then it just passes all the record values as options.

I don't really understand how the *instance_authority_resource is making sequel grab the first record.

sporto commented 9 years ago

This snippet shows the same behaviour:

    task :sequel => :environment do
        e = Enquiry

        def foo(res, *options)
            puts res.inspect #<Enquiry:0x007fd16b3bb9d0>
        end

        puts e.inspect # Enquiry
        foo(*e)
    end
sporto commented 9 years ago

So *Enquiry tells sequel to try to expand this as an array will do, we end up passing all records in the DB

nathanl commented 9 years ago

Sebastian - I definitely consider this a bug. Thanks for reporting it. :smile: One of us will get to fixing it sooner or later, unless you decide to submit a PR yourself.

nathanl commented 9 years ago

Your little Rake task example is great, by the way, and since you said "*Enquiry tells sequel to try to expand this as an array will do", I thought, "I wonder if *thing is syntactic sugar for thing.to_a." Maybe you already knew this, but it is.

class Example
  def to_a
    ["woo a string in an array"]
  end
end

e = Example.new
o = *e # => ["woo a string in an array"]

I was wondering how Sequel managed to magically define an asterisk method for itself or something, but I guess they just define to_a. :)

sporto commented 9 years ago

Right, I see, they do define to to_a, so it behaves like an array. I didn't make the connection between * and implementing to_a.

nathanl commented 9 years ago

@sporto I think I have this fixed. Would you try the fix_sequel_bug branch and verify that it works for you? https://github.com/nathanl/authority/pull/105

sporto commented 9 years ago

@nathanl Yes, this works!

nathanl commented 9 years ago

Awesome! Merging.

nathanl commented 9 years ago

Fix released in 3.1.0