Merge branch 'master' into meta_where

.rvmrc

@@ -1,23 +1 @@
-#!/usr/bin/env bash
-
-# adapted from: http://rvm.beginrescueend.com/workflow/rvmrc/
-
-ruby_string="1.8.7"
-gemset_name="cancan"
-
-if rvm list strings | grep -q "${ruby_string}" ; then
-
-  # Load or create the specified environment
-  if [[ -d "${rvm_path:-$HOME/.rvm}/environments" \
-    && -s "${rvm_path:-$HOME/.rvm}/environments/${ruby_string}@${gemset_name}" ]] ; then
-    \. "${rvm_path:-$HOME/.rvm}/environments/${ruby_string}@${gemset_name}"
-  else
-    rvm --create  "${ruby_string}@${gemset_name}"
-  fi
-
-else
-
-  # Notify the user to install the desired interpreter before proceeding.
-  echo "${ruby_string} was not found, please run 'rvm install ${ruby_string}' and then cd back into the project directory."
-
-fi
+rvm use 1.8.7@cancan --create

Gemfile

@@ -2,7 +2,7 @@ source "http://rubygems.org"
 
 case ENV["MODEL_ADAPTER"]
 when nil, "active_record"
-  gem "sqlite3-ruby", :require => "sqlite3"
+  gem "sqlite3"
   gem "activerecord", :require => "active_record"
   gem "with_model"
   gem "meta_where"

lib/cancan/ability.rb

@@ -206,7 +206,7 @@ def authorize!(action, subject, *args)
     def unauthorized_message(action, subject)
       keys = unauthorized_message_keys(action, subject)
       variables = {:action => action.to_s}
-      variables[:subject] = (subject.class == Class ? subject : subject.class).to_s.downcase
+      variables[:subject] = (subject.class == Class ? subject : subject.class).to_s.underscore.humanize.downcase
       message = I18n.translate(nil, variables.merge(:scope => :unauthorized, :default => keys + [""]))
       message.blank? ? nil : message
     end

lib/cancan/controller_additions.rb

@@ -109,6 +109,9 @@ def load_and_authorize_resource(*args)
       #
       #     load_resource :new => :build
       #
+      # [:+prepend+]
+      #   Passing +true+ will use prepend_before_filter instead of a normal before_filter.
+      #
       def load_resource(*args)
         cancan_resource_class.add_before_filter(self, :load_resource, *args)
       end
@@ -162,6 +165,9 @@ def load_resource(*args)
       # [:+through+]
       #   Authorize conditions on this parent resource when instance isn't available.
       #
+      # [:+prepend+]
+      #   Passing +true+ will use prepend_before_filter instead of a normal before_filter.
+      #
       def authorize_resource(*args)
         cancan_resource_class.add_before_filter(self, :authorize_resource, *args)
       end
@@ -220,14 +226,31 @@ def skip_authorize_resource(*args)
       #     check_authorization
       #   end
       #
-      # Any arguments are passed to the +after_filter+ it triggers.
-      #
       # See skip_authorization_check to bypass this check on specific controller actions.
-      def check_authorization(*args)
-        self.after_filter(*args) do |controller|
-          unless controller.instance_variable_defined?(:@_authorized)
-            raise AuthorizationNotPerformed, "This action failed the check_authorization because it does not authorize_resource. Add skip_authorization_check to bypass this check."
-          end
+      #
+      # Options:
+      # [:+only+]
+      #   Only applies to given actions.
+      #
+      # [:+except+]
+      #   Does not apply to given actions.
+      #
+      # [:+if+]
+      #   Supply the name of a controller method to be called. The authorization check only takes place if this returns true.
+      #
+      #     check_authorization :if => :admin_controller?
+      #
+      # [:+unless+]
+      #   Supply the name of a controller method to be called. The authorization check only takes place if this returns false.
+      #
+      #     check_authorization :unless => :devise_controller?
+      #
+      def check_authorization(options = {})
+        self.after_filter(options.slice(:only, :except)) do |controller|
+          return if controller.instance_variable_defined?(:@_authorized)
+          return if options[:if] && !controller.send(options[:if])
+          return if options[:unless] && controller.send(options[:unless])
+          raise AuthorizationNotPerformed, "This action failed the check_authorization because it does not authorize_resource. Add skip_authorization_check to bypass this check."
         end
       end
 

lib/cancan/controller_resource.rb

@@ -5,7 +5,8 @@ class ControllerResource # :nodoc:
     def self.add_before_filter(controller_class, method, *args)
       options = args.extract_options!
       resource_name = args.first
-      controller_class.before_filter(options.slice(:only, :except)) do |controller|
+      before_filter_method = options.delete(:prepend) ? :prepend_before_filter : :before_filter
+      controller_class.send(before_filter_method, options.slice(:only, :except)) do |controller|
         controller.class.cancan_resource_class.new(controller, resource_name, options.except(:only, :except)).send(method)
       end
     end
@@ -112,7 +113,7 @@ def id_param
     end
 
     def member_action?
-      !collection_actions.include? @params[:action].to_sym
+      new_actions.include?(@params[:action].to_sym) || (@params[:id] && !collection_actions.include?(@params[:action].to_sym))
     end
 
     # Returns the class used for this resource. This can be overriden by the :class option.

lib/cancan/inherited_resource.rb

@@ -3,7 +3,8 @@ module CanCan
   class InheritedResource < ControllerResource # :nodoc:
     def load_resource_instance
       if parent?
-        @controller.send :parent
+        @controller.send :association_chain
+        @controller.instance_variable_get("@#{instance_name}")
       elsif new_actions.include? @params[:action].to_sym
         @controller.send :build_resource
       else
@@ -12,7 +13,7 @@ def load_resource_instance
     end
 
     def resource_base
-      @controller.send :end_of_association_chain
+      @controller.send :collection
     end
   end
 end

lib/cancan/model_adapters/active_record_adapter.rb

@@ -68,7 +68,9 @@ def joins
       end
 
       def database_records
-        if @model_class.respond_to?(:where) && @model_class.respond_to?(:joins)
+        if override_scope
+          override_scope
+        elsif @model_class.respond_to?(:where) && @model_class.respond_to?(:joins)
           @model_class.where(conditions).joins(joins)
         else
           @model_class.scoped(:conditions => conditions, :joins => joins)
@@ -77,6 +79,18 @@ def database_records
 
       private
 
+      def override_scope
+        conditions = @rules.map(&:conditions).compact
+        if conditions.any? { |c| c.kind_of?(ActiveRecord::Relation) }
+          if conditions.size == 1
+            conditions.first
+          else
+            rule = @rules.detect { |rule| rule.conditions.kind_of?(ActiveRecord::Relation) }
+            raise Error, "Unable to merge an Active Record scope with other conditions. Instead use a hash or SQL for #{rule.actions.first} #{rule.subjects.first} ability."
+          end
+        end
+      end
+
       def merge_conditions(sql, conditions_hash, behavior)
         if conditions_hash.blank?
           behavior ? true_sql : false_sql

lib/cancan/rule.rb

@@ -3,14 +3,15 @@ module CanCan
   # it holds the information about a "can" call made on Ability and provides
   # helpful methods to determine permission checking and conditions hash generation.
   class Rule # :nodoc:
-    attr_reader :base_behavior, :actions, :conditions
+    attr_reader :base_behavior, :subjects, :actions, :conditions
     attr_writer :expanded_actions
 
     # The first argument when initializing is the base_behavior which is a true/false
     # value. True for "can" and false for "cannot". The next two arguments are the action
     # and subject respectively (such as :read, @project). The third argument is a hash
     # of conditions and the last one is the block passed to the "can" call.
     def initialize(base_behavior, action, subject, conditions, block)
+      raise Error, "You are not able to supply a block with a hash of conditions in #{action} #{subject} ability. Use either one." if conditions.kind_of?(Hash) && !block.nil?
       @match_all = action.nil? && subject.nil?
       @base_behavior = base_behavior
       @actions = [action].flatten

spec/cancan/ability_spec.rb

@@ -357,6 +357,14 @@ class Container < Hash; end
     @ability.model_adapter(model_class, :read).should == :adapter_instance
   end
 
+  it "should raise an error when attempting to use a block with a hash condition since it's not likely what they want" do
+    lambda {
+      @ability.can :read, Array, :published => true do
+        false
+      end
+    }.should raise_error(CanCan::Error, "You are not able to supply a block with a hash of conditions in read Array ability. Use either one.")
+  end
+
   describe "unauthorized message" do
     after(:each) do
       I18n.backend = nil
@@ -395,6 +403,7 @@ class Container < Hash; end
     it "should have variables for action and subject" do
       I18n.backend.store_translations :en, :unauthorized => {:manage => {:all => "%{action} %{subject}"}} # old syntax for now in case testing with old I18n
       @ability.unauthorized_message(:update, Array).should == "update array"
+      @ability.unauthorized_message(:update, ArgumentError).should == "update argument error"
       @ability.unauthorized_message(:edit, 1..3).should == "edit range"
     end
   end

spec/cancan/controller_additions_spec.rb

@@ -42,6 +42,11 @@
     @controller_class.load_and_authorize_resource :project, :foo => :bar
   end
 
+  it "load_and_authorize_resource with :prepend should prepend the before filter" do
+    mock(@controller_class).prepend_before_filter({})
+    @controller_class.load_and_authorize_resource :foo => :bar, :prepend => true
+  end
+
   it "authorize_resource should setup a before filter which passes call to ControllerResource" do
     stub(CanCan::ControllerResource).new(@controller, nil, :foo => :bar).mock!.authorize_resource
     mock(@controller_class).before_filter(:except => :show) { |options, block| block.call(@controller) }
@@ -61,17 +66,33 @@
   end
 
   it "check_authorization should trigger AuthorizationNotPerformed in after filter" do
-    mock(@controller_class).after_filter(:some_options) { |options, block| block.call(@controller) }
+    mock(@controller_class).after_filter(:only => [:test]) { |options, block| block.call(@controller) }
     lambda {
-      @controller_class.check_authorization(:some_options)
+      @controller_class.check_authorization(:only => [:test])
     }.should raise_error(CanCan::AuthorizationNotPerformed)
   end
 
+  it "check_authorization should not trigger AuthorizationNotPerformed when :if is false" do
+    stub(@controller).check_auth? { false }
+    mock(@controller_class).after_filter({}) { |options, block| block.call(@controller) }
+    lambda {
+      @controller_class.check_authorization(:if => :check_auth?)
+    }.should_not raise_error(CanCan::AuthorizationNotPerformed)
+  end
+
+  it "check_authorization should not trigger AuthorizationNotPerformed when :unless is true" do
+    stub(@controller).engine_controller? { true }
+    mock(@controller_class).after_filter({}) { |options, block| block.call(@controller) }
+    lambda {
+      @controller_class.check_authorization(:unless => :engine_controller?)
+    }.should_not raise_error(CanCan::AuthorizationNotPerformed)
+  end
+
   it "check_authorization should not raise error when @_authorized is set" do
     @controller.instance_variable_set(:@_authorized, true)
-    mock(@controller_class).after_filter(:some_options) { |options, block| block.call(@controller) }
+    mock(@controller_class).after_filter(:only => [:test]) { |options, block| block.call(@controller) }
     lambda {
-      @controller_class.check_authorization(:some_options)
+      @controller_class.check_authorization(:only => [:test])
     }.should_not raise_error(CanCan::AuthorizationNotPerformed)
   end
 

spec/cancan/controller_resource_spec.rb

@@ -110,35 +110,44 @@
   end
 
   it "should perform authorization using controller action and loaded model" do
-    @params[:action] = "show"
+    @params.merge!(:action => "show", :id => 123)
     @controller.instance_variable_set(:@project, :some_project)
     stub(@controller).authorize!(:show, :some_project) { raise CanCan::AccessDenied }
     resource = CanCan::ControllerResource.new(@controller)
     lambda { resource.authorize_resource }.should raise_error(CanCan::AccessDenied)
   end
 
   it "should perform authorization using controller action and non loaded model" do
-    @params[:action] = "show"
+    @params.merge!(:action => "show", :id => 123)
     stub(@controller).authorize!(:show, Project) { raise CanCan::AccessDenied }
     resource = CanCan::ControllerResource.new(@controller)
     lambda { resource.authorize_resource }.should raise_error(CanCan::AccessDenied)
   end
 
   it "should call load_resource and authorize_resource for load_and_authorize_resource" do
-    @params[:action] = "show"
+    @params.merge!(:action => "show", :id => 123)
     resource = CanCan::ControllerResource.new(@controller)
     mock(resource).load_resource
     mock(resource).authorize_resource
     resource.load_and_authorize_resource
   end
 
-  it "should not build a resource when on custom collection action" do
-    @params[:action] = "sort"
+  it "should not build a single resource when on custom collection action even with id" do
+    @params.merge!(:action => "sort", :id => 123)
     resource = CanCan::ControllerResource.new(@controller, :collection => [:sort, :list])
     resource.load_resource
     @controller.instance_variable_get(:@project).should be_nil
   end
 
+  it "should load a collection resource when on custom action with no id param" do
+    stub(Project).accessible_by(@ability, :sort) { :found_projects }
+    @params[:action] = "sort"
+    resource = CanCan::ControllerResource.new(@controller)
+    resource.load_resource
+    @controller.instance_variable_get(:@project).should be_nil
+    @controller.instance_variable_get(:@projects).should == :found_projects
+  end
+
   it "should build a resource when on custom new action even when params[:id] exists" do
     @params.merge!(:action => "build", :id => 123)
     stub(Project).new { :some_project }
@@ -250,7 +259,7 @@
   end
 
   it "should find record through has_one association with :singleton option" do
-    @params.merge!(:action => "show")
+    @params.merge!(:action => "show", :id => 123)
     category = Object.new
     @controller.instance_variable_set(:@category, category)
     stub(category).project { :some_project }

spec/cancan/inherited_resource_spec.rb

@@ -12,7 +12,7 @@
   end
 
   it "show should load resource through @controller.resource" do
-    @params[:action] = "show"
+    @params.merge!(:action => "show", :id => 123)
     stub(@controller).resource { :project_resource }
     CanCan::InheritedResource.new(@controller).load_resource
     @controller.instance_variable_get(:@project).should == :project_resource
@@ -25,17 +25,17 @@
     @controller.instance_variable_get(:@project).should == :project_resource
   end
 
-  it "index should load through @controller.parent when parent" do
+  it "index should load through @controller.association_chain when parent" do
     @params[:action] = "index"
-    stub(@controller).parent { :project_resource }
+    stub(@controller).association_chain { @controller.instance_variable_set(:@project, :project_resource) }
     CanCan::InheritedResource.new(@controller, :parent => true).load_resource
     @controller.instance_variable_get(:@project).should == :project_resource
   end
 
-  it "index should load through @controller.end_of_association_chain" do
+  it "index should load through @controller.collection" do
     @params[:action] = "index"
     stub(Project).accessible_by(@ability, :index) { :projects }
-    stub(@controller).end_of_association_chain { Project }
+    stub(@controller).collection { Project }
     CanCan::InheritedResource.new(@controller).load_resource
     @controller.instance_variable_get(:@projects).should == :projects
   end

spec/cancan/model_adapters/active_record_adapter_spec.rb

@@ -111,10 +111,25 @@
       @ability.can :read, Article, :published => true
       @ability.can :read, Article, ["secret=?", true]
       article1 = Article.create!(:published => true, :secret => false)
+      article2 = Article.create!(:published => true, :secret => true)
+      article3 = Article.create!(:published => false, :secret => true)
       article4 = Article.create!(:published => false, :secret => false)
+      Article.accessible_by(@ability).should == [article1, article2, article3]
+    end
+
+    it "should allow a scope for conditions" do
+      @ability.can :read, Article, Article.where(:secret => true)
+      article1 = Article.create!(:secret => true)
+      article2 = Article.create!(:secret => false)
       Article.accessible_by(@ability).should == [article1]
     end
 
+    it "should raise an exception when trying to merge scope with other conditions" do
+      @ability.can :read, Article, :published => true
+      @ability.can :read, Article, Article.where(:secret => true)
+      lambda { Article.accessible_by(@ability) }.should raise_error(CanCan::Error, "Unable to merge an Active Record scope with other conditions. Instead use a hash or SQL for read Article ability.")
+    end
+
     it "should not allow to fetch records when ability with just block present" do
       @ability.can :read, Article do
         false