From 7ee942c3346c4187c40cb3caae055e092a177f96 Mon Sep 17 00:00:00 2001 From: Ryan Bates Date: Thu, 24 Mar 2011 11:22:32 -0700 Subject: [PATCH] adding enable_authorization method and deprecating some other controller methods --- lib/cancan/controller_additions.rb | 62 ++++++------- spec/cancan/controller_additions_spec.rb | 112 ++++++----------------- 2 files changed, 57 insertions(+), 117 deletions(-) diff --git a/lib/cancan/controller_additions.rb b/lib/cancan/controller_additions.rb index b26fe20..66d476a 100644 --- a/lib/cancan/controller_additions.rb +++ b/lib/cancan/controller_additions.rb @@ -113,6 +113,7 @@ module CanCan # Passing +true+ will use prepend_before_filter instead of a normal before_filter. # def load_resource(*args) + raise ImplementationRemoved, "The load_resource method has been removed, use load_and_authorize_resource instead." cancan_resource_class.add_before_filter(self, :load_resource, *args) end @@ -169,6 +170,7 @@ module CanCan # Passing +true+ will use prepend_before_filter instead of a normal before_filter. # def authorize_resource(*args) + raise ImplementationRemoved, "The authorize_resource method has been removed, use load_and_authorize_resource instead." cancan_resource_class.add_before_filter(self, :authorize_resource, *args) end @@ -197,6 +199,7 @@ module CanCan # # You can also pass the resource name as the first argument to skip that resource. def skip_load_resource(*args) + raise ImplementationRemoved, "The skip_load_resource method has been removed, use skip_load_and_authorize_resource instead." options = args.extract_options! name = args.first cancan_skipper[:load][name] = options @@ -213,20 +216,23 @@ module CanCan # # You can also pass the resource name as the first argument to skip that resource. def skip_authorize_resource(*args) + raise ImplementationRemoved, "The skip_authorize_resource method has been removed, use skip_load_and_authorize_resource instead." options = args.extract_options! name = args.first cancan_skipper[:authorize][name] = options end - # Add this to a controller to ensure it performs authorization through +authorized+! or +authorize_resource+ call. - # If neither of these authorization methods are called, a CanCan::AuthorizationNotPerformed exception will be raised. - # This is normally added to the ApplicationController to ensure all controller actions do authorization. + # Add this to a controller to automatically perform authorization on every action. # # class ApplicationController < ActionController::Base - # check_authorization + # enable_authorization # end # - # See skip_authorization_check to bypass this check on specific controller actions. + # Internally it does this in a before_filter for every action. + # + # authorize! params[:action], params[:controller] + # + # If you need to "skip" authorization in a given controller, it is best to enable :+access+ to it in the +Ability+. # # Options: # [:+only+] @@ -236,41 +242,23 @@ module CanCan # 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. + # Supply the name of a controller method to be called. The authorization only takes place if this returns true. # - # check_authorization :if => :admin_controller? + # enable_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. + # Supply the name of a controller method to be called. The authorization only takes place if this returns false. # - # check_authorization :unless => :devise_controller? + # enable_authorization :unless => :devise_controller? # - def check_authorization(options = {}) - self.after_filter(options.slice(:only, :except)) do |controller| - return if controller.instance_variable_defined?(:@_authorized) + 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]) - raise AuthorizationNotPerformed, "This action failed the check_authorization because it does not authorize_resource. Add skip_authorization_check to bypass this check." + authorize! controller.params[:action], controller.params[:controller] end end - # Call this in the class of a controller to skip the check_authorization behavior on the actions. - # - # class HomeController < ApplicationController - # skip_authorization_check :only => :index - # end - # - # Any arguments are passed to the +before_filter+ it triggers. - def skip_authorization_check(*args) - self.before_filter(*args) do |controller| - controller.instance_variable_set(:@_authorized, true) - end - end - - def skip_authorization(*args) - raise ImplementationRemoved, "The CanCan skip_authorization method has been renamed to skip_authorization_check. Please update your code." - end - def cancan_resource_class if ancestors.map(&:to_s).include? "InheritedResources::Actions" InheritedResource @@ -279,8 +267,16 @@ module CanCan end end + def check_authorization(options = {}) + raise ImplementationRemoved, "The check_authorization method has been removed, use enable_authorization instead." + end + + def skip_authorization_check(*args) + raise ImplementationRemoved, "The skip_authorization_check method has been removed, instead authorize access to controller in Ability to 'skip'." + end + def cancan_skipper - @_cancan_skipper ||= {:authorize => {}, :load => {}} + raise ImplementationRemoved, "The skip_authorization_check method has been removed, instead authorize access to controller in Ability to 'skip'." end end @@ -330,10 +326,6 @@ module CanCan current_ability.authorize!(*args) end - def unauthorized!(message = nil) - raise ImplementationRemoved, "The unauthorized! method has been removed from CanCan, use authorize! instead." - end - # Creates and returns the current user's ability and caches it. If you # want to override how the Ability is defined then this is the place. # Just define the method in the controller to change behavior. diff --git a/spec/cancan/controller_additions_spec.rb b/spec/cancan/controller_additions_spec.rb index 88b8be7..613301a 100644 --- a/spec/cancan/controller_additions_spec.rb +++ b/spec/cancan/controller_additions_spec.rb @@ -2,26 +2,28 @@ require "spec_helper" describe CanCan::ControllerAdditions do before(:each) do + @params = HashWithIndifferentAccess.new @controller_class = Class.new @controller = @controller_class.new - stub(@controller).params { {} } + stub(@controller).params { @params } stub(@controller).current_user { :current_user } mock(@controller_class).helper_method(:can?, :cannot?) @controller_class.send(:include, CanCan::ControllerAdditions) end - it "should raise ImplementationRemoved when attempting to call 'unauthorized!' on a controller" do - lambda { @controller.unauthorized! }.should raise_error(CanCan::ImplementationRemoved) + it "should raise ImplementationRemoved when attempting to call load/authorize/skip/check calls on a controller" do + lambda { @controller_class.load_resource }.should raise_error(CanCan::ImplementationRemoved) + lambda { @controller_class.authorize_resource }.should raise_error(CanCan::ImplementationRemoved) + lambda { @controller_class.skip_load_resource }.should raise_error(CanCan::ImplementationRemoved) + lambda { @controller_class.skip_authorize_resource }.should raise_error(CanCan::ImplementationRemoved) + lambda { @controller_class.check_authorization }.should raise_error(CanCan::ImplementationRemoved) + lambda { @controller_class.skip_authorization_check }.should raise_error(CanCan::ImplementationRemoved) + lambda { @controller_class.cancan_skipper }.should raise_error(CanCan::ImplementationRemoved) end - it "authorize! should assign @_authorized instance variable and pass args to current ability" do + it "authorize! should pass args to current ability" do mock(@controller.current_ability).authorize!(:foo, :bar) @controller.authorize!(:foo, :bar) - @controller.instance_variable_get(:@_authorized).should be_true - end - - it "should have a current_ability method which generates an ability for the current user" do - @controller.current_ability.should be_kind_of(Ability) end it "should provide a can? and cannot? methods which go through the current ability" do @@ -47,55 +49,6 @@ describe CanCan::ControllerAdditions do @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) } - @controller_class.authorize_resource :foo => :bar, :except => :show - end - - it "load_resource should setup a before filter which passes call to ControllerResource" do - stub(CanCan::ControllerResource).new(@controller, nil, :foo => :bar).mock!.load_resource - mock(@controller_class).before_filter(:only => [:show, :index]) { |options, block| block.call(@controller) } - @controller_class.load_resource :foo => :bar, :only => [:show, :index] - end - - it "skip_authorization_check should set up a before filter which sets @_authorized to true" do - mock(@controller_class).before_filter(:filter_options) { |options, block| block.call(@controller) } - @controller_class.skip_authorization_check(:filter_options) - @controller.instance_variable_get(:@_authorized).should be_true - end - - it "check_authorization should trigger AuthorizationNotPerformed in after filter" do - mock(@controller_class).after_filter(:only => [:test]) { |options, block| block.call(@controller) } - lambda { - @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(:only => [:test]) { |options, block| block.call(@controller) } - lambda { - @controller_class.check_authorization(:only => [:test]) - }.should_not raise_error(CanCan::AuthorizationNotPerformed) - end - it "cancan_resource_class should be ControllerResource by default" do @controller.class.cancan_resource_class.should == CanCan::ControllerResource end @@ -105,33 +58,28 @@ describe CanCan::ControllerAdditions do @controller.class.cancan_resource_class.should == CanCan::InheritedResource end - it "cancan_skipper should be an empty hash with :authorize and :load options and remember changes" do - @controller_class.cancan_skipper.should == {:authorize => {}, :load => {}} - @controller_class.cancan_skipper[:load] = true - @controller_class.cancan_skipper[:load].should == true + it "enable_authorization should call authorize! with controller and action name" do + @params.merge!(:controller => "projects", :action => "create") + mock(@controller_class).authorize!("create", "projects") + stub(@controller_class).before_filter(:only => :foo, :except => :bar) { |options, block| block.call(@controller) } + @controller_class.enable_authorization(:only => :foo, :except => :bar) end - it "skip_authorize_resource should add itself to the cancan skipper with given model name and options" do - @controller_class.skip_authorize_resource(:project, :only => [:index, :show]) - @controller_class.cancan_skipper[:authorize][:project].should == {:only => [:index, :show]} - @controller_class.skip_authorize_resource(:only => [:index, :show]) - @controller_class.cancan_skipper[:authorize][nil].should == {:only => [:index, :show]} - @controller_class.skip_authorize_resource(:article) - @controller_class.cancan_skipper[:authorize][:article].should == {} + it "enable_authorization should not not call authorize! when :if is false" do + @authorize_called = false + stub(@controller).authorize? { false } + stub(@controller_class).authorize! { @authorize_called = true } + mock(@controller_class).before_filter({}) { |options, block| block.call(@controller) } + @controller_class.enable_authorization(:if => :authorize?) + @authorize_called.should be_false end - it "skip_load_resource should add itself to the cancan skipper with given model name and options" do - @controller_class.skip_load_resource(:project, :only => [:index, :show]) - @controller_class.cancan_skipper[:load][:project].should == {:only => [:index, :show]} - @controller_class.skip_load_resource(:only => [:index, :show]) - @controller_class.cancan_skipper[:load][nil].should == {:only => [:index, :show]} - @controller_class.skip_load_resource(:article) - @controller_class.cancan_skipper[:load][:article].should == {} - end - - it "skip_load_and_authore_resource should add itself to the cancan skipper with given model name and options" do - @controller_class.skip_load_and_authorize_resource(:project, :only => [:index, :show]) - @controller_class.cancan_skipper[:load][:project].should == {:only => [:index, :show]} - @controller_class.cancan_skipper[:authorize][:project].should == {:only => [:index, :show]} + it "enable_authorization should not trigger AuthorizationNotPerformed when :unless is true" do + @authorize_called = false + stub(@controller).engine_controller? { true } + stub(@controller_class).authorize! { @authorize_called = true } + mock(@controller_class).before_filter({}) { |options, block| block.call(@controller) } + @controller_class.enable_authorization(:unless => :engine_controller?) + @authorize_called.should be_false end end