SQL in ability definition raises activerecord exception - undefined method '%' #742

Open
decasia opened this Issue Sep 13, 2012 · 4 comments

Comments

Projects
None yet
5 participants

decasia commented Sep 13, 2012

I was trying to define permissions for an administrator role like this:

can :manage, :all
cannot :propose_with_code, SubjectCode, ["subject_codes.department_id NOT IN (select id from departments where departments.hidden = ?)", true] do |subject_code|
  subject_code.department.hidden == true
end

The idea here is that administrators should be able to do anything except use subject codes that belong to hidden departments. The reason, incidentally, why I'm passing the 'true' argument to the SQL fragment is that the SQL representation of 'true' varies depending on the database we're using ("t" on sqlite, 1 on mysql).

Anyway, this SQL condition string turns out to raise a fairly low-level AR exception:

  undefined method `%' for #<Array:0x0000010261c850>
  vendor/bundle/ruby/1.9.1/gems/activerecord-3.2.8/lib/active_record/sanitization.rb:120:in `sanitize_sql_array'

Stack trace looks like this:

  vendor/bundle/ruby/1.9.1/gems/activerecord-3.2.8/lib/active_record/sanitization.rb:120:in `sanitize_sql_array'
  vendor/bundle/ruby/1.9.1/gems/activerecord-3.2.8/lib/active_record/sanitization.rb:27:in `sanitize_sql_for_conditions'
  vendor/bundle/ruby/1.9.1/gems/activerecord-3.2.8/lib/active_record/relation/query_methods.rb:324:in `build_where'
  vendor/bundle/ruby/1.9.1/gems/activerecord-3.2.8/lib/active_record/relation/query_methods.rb:136:in `where'
  vendor/bundle/ruby/1.9.1/gems/activerecord-3.2.8/lib/active_record/querying.rb:9:in `where'
  vendor/bundle/ruby/1.9.1/gems/cancan-1.6.8/lib/cancan/model_adapters/active_record_adapter.rb:96:in `database_records'
  vendor/bundle/ruby/1.9.1/gems/cancan-1.6.8/lib/cancan/model_additions.rb:23:in `accessible_by'
  vendor/bundle/ruby/1.9.1/gems/activerecord-3.2.8/lib/active_record/relation/delegation.rb:37:in `block in method_missing'
  vendor/bundle/ruby/1.9.1/gems/activerecord-3.2.8/lib/active_record/relation.rb:241:in `block in scoping'
  vendor/bundle/ruby/1.9.1/gems/activerecord-3.2.8/lib/active_record/scoping.rb:98:in `with_scope'
  vendor/bundle/ruby/1.9.1/gems/activerecord-3.2.8/lib/active_record/relation.rb:241:in `scoping'
  vendor/bundle/ruby/1.9.1/gems/activerecord-3.2.8/lib/active_record/relation/delegation.rb:37:in `method_missing'
  app/views/schedule_proposal/_schedule_form.html.haml:6:in `block (2 levels) in _app_views_schedule_proposal__schedule_form_html_haml__467923406301105224_2174930480'

It looks to me like the problem comes from the database_records method in cancan/model_adapters/active_record_adapter.rb. The database_records method looks like this:

      def database_records
        if override_scope
          @model_class.scoped.merge(override_scope)
        elsif @model_class.respond_to?(:where) && @model_class.respond_to?(:joins)
          mergeable_conditions = @rules.select {|rule| rule.unmergeable? }.blank?
          if mergeable_conditions
            @model_class.where(conditions).joins(joins)
          else
            @model_class.where(*(@rules.map(&:conditions))).joins(joins)
          end
        else
          @model_class.scoped(:conditions => conditions, :joins => joins)
        end
      end

and line 96, the one that causes the exception, is this:
@model_class.where(*(@rules.map(&:conditions))).joins(joins)
Basically, it's trying to concatenate all the conditions on this model together and send them to where.

Now, when I inspect what's getting sent to where — i.e., the result of (@rules.map(&:conditions)) — I get this:

[["subject_codes.department_id NOT IN (select id from departments where department.hidden = ?)", true], {}]

In other words, we get an array with two elements: the first one is an array containing the SQL fragment intended to limit access to subject codes; the second is an empty hash corresponding to the :manage, :all permission.

Unfortunately, this seems to be problematic input to the ActiveRecord where method. I could go into more details about how this input gets processed and where the exception is eventually raised, but suffice it to say for now that AR can't sanitize this input correctly, since it's expecting something like a SQL string with ? characters that it can interpolate subsequent variables into, and instead it receives an array that it fails to handle (see sanitize_sql_array).

I don't know the cancan code well enough to know if I'm making some mistake in my own code, or if not, what the best fix would be. I can say, however, that it seems to work if we replace line 96 in active_record_adapter.rb with a loop that creates a chain of where conditions -- something like this:

- @model_class.where(*(@rules.map(&:conditions))).joins(joins)
+ ar_relation = @model_class
+ @rules.map(&:conditions).each do |condition|
+   ar_relation = ar_relation.where(condition)
+ end
+ ar_relation.joins(joins)

When I do this, the code appears to work as intended, with no further errors. Can someone advise me how to proceed further with this issue?

zilkey commented Oct 4, 2012

I have the same problem. Looks like this commit broke it:

c27ead5

Anyone know how to work around this issue, or do we need to patch CanCan?

+1 I have this problem as well. @zilkey, thanks for posting the offending commit hash.

I rolled back to 1.6.7 and I'm back in business.

Also, when I tried to use the Model.find_all_by_* methods, both 1.6.7 and 1.6.8 complain about undefined method `include?'

Hey! Just found this issue. Is it still a problem or can it be closed?

xhoy commented Apr 10, 2014

Dear submitter, Since cancan/raynB hasn't been active for more than 6 months and no body else then ryam himself has commit permissions the cancan project is on a stand still.
Since cancan has several issues including missing support for rails 4 cancan is moving forward to cancancan. More details on: #994

If your feel that your pull request or bug is still applicable (and hasn't been merged in to cancan) it would be really appreciated if you would resubmit it to cancancan (https://github.com/cancancommunity/cancancan)

We hope to see you on the other side!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment