diff --git a/lib/cancan/ability.rb b/lib/cancan/ability.rb index 46bbc6a..6bd7cee 100644 --- a/lib/cancan/ability.rb +++ b/lib/cancan/ability.rb @@ -225,14 +225,6 @@ module CanCan end end - def sufficient_attribute_check?(action, subject, attribute) - !(%w[create update].include?(action.to_s) && attribute.nil? && has_attributes?(action, subject)) - end - - def sufficient_condition_check?(action, subject) - !((subject.kind_of?(Symbol) || subject.kind_of?(String)) && has_instance_conditions?(action, subject)) - end - def unauthorized_message(action, subject) keys = unauthorized_message_keys(action, subject) variables = {:action => action.to_s} @@ -286,6 +278,14 @@ module CanCan end.flatten end + def sufficient_attribute_check?(action, subject, attribute) + !(%w[create update].include?(action.to_s) && attribute.nil? && has_attributes?(action, subject)) + end + + def sufficient_condition_check?(action, subject) + !((subject.kind_of?(Symbol) || subject.kind_of?(String)) && has_instance_conditions?(action, subject)) + end + # Accepts an array of actions and returns an array of actions which match. # This should be called before "matches?" and other checking methods since they # rely on the actions to be expanded. diff --git a/lib/cancan/controller_additions.rb b/lib/cancan/controller_additions.rb index 66d476a..f8e964d 100644 --- a/lib/cancan/controller_additions.rb +++ b/lib/cancan/controller_additions.rb @@ -253,9 +253,16 @@ module CanCan # def enable_authorization(options = {}) self.before_filter(options.slice(:only, :except)) do |controller| - return if options[:if] && !controller.send(options[:if]) - return if options[:unless] && controller.send(options[:unless]) - authorize! controller.params[:action], controller.params[:controller] + break if options[:if] && !controller.send(options[:if]) + break if options[:unless] && controller.send(options[:unless]) + controller.authorize! controller.params[:action], controller.params[:controller] + end + self.after_filter(options.slice(:only, :except)) do |controller| + break if options[:if] && !controller.send(options[:if]) + break if options[:unless] && controller.send(options[:unless]) + unless controller.current_ability.fully_authorized? controller.params[:action], controller.params[:controller] + raise CanCan::InsufficientAuthorizationCheck, "Authorization check is not sufficient for this action. This is probably because you have a conditions or attributes defined in Ability and are not checking for them in the action." + end end end diff --git a/lib/cancan/exceptions.rb b/lib/cancan/exceptions.rb index fa676fc..54d693b 100644 --- a/lib/cancan/exceptions.rb +++ b/lib/cancan/exceptions.rb @@ -11,6 +11,9 @@ module CanCan # Raised when using check_authorization without calling authorized! class AuthorizationNotPerformed < Error; end + # Raised when enable_authorization is used and not fully authorized by the end of the action + class InsufficientAuthorizationCheck < Error; end + # This error is raised when a user isn't allowed to access a given controller action. # This usually happens within a call to ControllerAdditions#authorize! but can be # raised manually. diff --git a/spec/cancan/controller_additions_spec.rb b/spec/cancan/controller_additions_spec.rb index 613301a..a2f680c 100644 --- a/spec/cancan/controller_additions_spec.rb +++ b/spec/cancan/controller_additions_spec.rb @@ -60,25 +60,38 @@ describe CanCan::ControllerAdditions do it "enable_authorization should call authorize! with controller and action name" do @params.merge!(:controller => "projects", :action => "create") - mock(@controller_class).authorize!("create", "projects") + mock(@controller).authorize!("create", "projects") stub(@controller_class).before_filter(:only => :foo, :except => :bar) { |options, block| block.call(@controller) } + stub(@controller_class).after_filter(:only => :foo, :except => :bar) @controller_class.enable_authorization(:only => :foo, :except => :bar) end - it "enable_authorization should not not call authorize! when :if is false" do + it "enable_authorization should raise InsufficientAuthorizationCheck when not fully authoried" do + @params.merge!(:controller => "projects", :action => "create") + stub(@ability).fully_authorized? { false } + stub(@controller_class).before_filter(:only => :foo, :except => :bar) + stub(@controller_class).after_filter(:only => :foo, :except => :bar) { |options, block| block.call(@controller) } + lambda { + @controller_class.enable_authorization(:only => :foo, :except => :bar) + }.should raise_error(CanCan::InsufficientAuthorizationCheck) + end + + it "enable_authorization should not call authorize! when :if is false" do @authorize_called = false stub(@controller).authorize? { false } - stub(@controller_class).authorize! { @authorize_called = true } + stub(@controller).authorize! { @authorize_called = true } mock(@controller_class).before_filter({}) { |options, block| block.call(@controller) } + mock(@controller_class).after_filter({}) { |options, block| block.call(@controller) } @controller_class.enable_authorization(:if => :authorize?) @authorize_called.should be_false end - it "enable_authorization should not trigger AuthorizationNotPerformed when :unless is true" do + it "enable_authorization should not call authorize! when :unless is true" do @authorize_called = false stub(@controller).engine_controller? { true } - stub(@controller_class).authorize! { @authorize_called = true } + stub(@controller).authorize! { @authorize_called = true } mock(@controller_class).before_filter({}) { |options, block| block.call(@controller) } + mock(@controller_class).after_filter({}) { |options, block| block.call(@controller) } @controller_class.enable_authorization(:unless => :engine_controller?) @authorize_called.should be_false end