From c225c367e1ee31f68482ad43cd4cd630d9793071 Mon Sep 17 00:00:00 2001 From: ammarghaus Date: Thu, 12 May 2022 12:16:57 +0200 Subject: [PATCH] feat: allow if and unless options for skipping authorize and load --- CHANGELOG.md | 1 + docs/changing_defaults.md | 1 + lib/cancan/controller_additions.rb | 46 +++++++++++++++++-- lib/cancan/controller_resource.rb | 16 +++++-- spec/cancan/controller_additions_spec.rb | 8 ++++ spec/cancan/controller_resource_spec.rb | 56 ++++++++++++++++++++++++ 6 files changed, 121 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dece0e0..e4c07266 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ * [#653](https://github.com/CanCanCommunity/cancancan/pull/653): Add support for using an nil relation as a condition. ([@ghiculescu][]) * [#702](https://github.com/CanCanCommunity/cancancan/pull/702): Support scopes of STI classes as ability conditions. ([@honigc][]) * [#798](https://github.com/CanCanCommunity/cancancan/pull/798): Allow disabling of rules compressor via `CanCan.rules_compressor_enabled = false`. ([@coorasse][]) +* [#808](https://github.com/CanCanCommunity/cancancan/pull/808): Add `if` and `unless` options controller helpers for skipping. ([@ammarghaus][]) ## 3.4.0 diff --git a/docs/changing_defaults.md b/docs/changing_defaults.md index 5420b4c9..7e9cee24 100644 --- a/docs/changing_defaults.md +++ b/docs/changing_defaults.md @@ -146,6 +146,7 @@ class ProductsController < ActionController::Base skip_authorize_resource only: :new end ``` +Both `skip_authorize_resource` and `skip_load_resource` support `:if` and `:unless` options. Either one takes a method name as a symbol or a lambda/proc. This option will be called to determine if the authorization/loading will be performed. ### Custom class name diff --git a/lib/cancan/controller_additions.rb b/lib/cancan/controller_additions.rb index 0c84f83e..b1bfc43d 100644 --- a/lib/cancan/controller_additions.rb +++ b/lib/cancan/controller_additions.rb @@ -196,8 +196,7 @@ def skip_load_and_authorize_resource(*args) end # Skip the loading behavior of CanCan. This is useful when using +load_and_authorize_resource+ but want to - # only do authorization on certain actions. You can pass :only and :except options to specify which actions to - # skip the effects on. It will apply to all actions by default. + # only do authorization on certain actions. It will apply to all actions by default. # # class ProjectsController < ApplicationController # load_and_authorize_resource @@ -205,6 +204,26 @@ def skip_load_and_authorize_resource(*args) # end # # You can also pass the resource name as the first argument to skip that resource. + # + # Options: + # [:+only+] + # Only applies to given actions. + # + # [:+except+] + # Does not apply to given actions. + # + # [:+if+] + # Supply the name of a controller method or a lambda/proc to be called. + # The loading is only skipped if this returns true. + # + # skip_load_resource :if => :admin_controller? + # + # [:+unless+] + # Supply the name of a controller method to be called. + # The loading is skipped if this returns false. + # + # skip_load_resource :unless => :devise_controller? + # def skip_load_resource(*args) options = args.extract_options! name = args.first @@ -212,8 +231,7 @@ def skip_load_resource(*args) end # Skip the authorization behavior of CanCan. This is useful when using +load_and_authorize_resource+ but want to - # only do loading on certain actions. You can pass :only and :except options to specify which actions to - # skip the effects on. It will apply to all actions by default. + # only do loading on certain actions. It will apply to all actions by default. # # class ProjectsController < ApplicationController # load_and_authorize_resource @@ -221,6 +239,26 @@ def skip_load_resource(*args) # end # # You can also pass the resource name as the first argument to skip that resource. + # + # Options: + # [:+only+] + # Only applies to given actions. + # + # [:+except+] + # Does not apply to given actions. + # + # [:+if+] + # Supply the name of a controller method or a lambda/proc to be called. + # The authorization is only skipped if this returns true. + # + # skip_authorize_resource :if => :admin_controller? + # + # [:+unless+] + # Supply the name of a controller method to be called. + # The authorization is skipped if this returns false. + # + # skip_authorize_resource :unless => :devise_controller? + # def skip_authorize_resource(*args) options = args.extract_options! name = args.first diff --git a/lib/cancan/controller_resource.rb b/lib/cancan/controller_resource.rb index c99fd2dc..241a65b8 100644 --- a/lib/cancan/controller_resource.rb +++ b/lib/cancan/controller_resource.rb @@ -47,9 +47,7 @@ def parent? def skip?(behavior) return false unless (options = @controller.class.cancan_skipper[behavior][@name]) - options == {} || - options[:except] && !action_exists_in?(options[:except]) || - action_exists_in?(options[:only]) + options == {} || evaluate_options(options) end protected @@ -130,6 +128,18 @@ def action_exists_in?(options) Array(options).include?(@params[:action].to_sym) end + def evaluate_callable(option) + return option.call if option.respond_to?(:call) + return @controller.send(option) if option && defined?(option) + end + + def evaluate_options(options) + options[:except] && !action_exists_in?(options[:except]) || + options[:unless] && !evaluate_callable(options[:unless]) || + action_exists_in?(options[:only]) || + evaluate_callable(options[:if]) + end + def adapter ModelAdapters::AbstractAdapter.adapter_class(resource_class) end diff --git a/spec/cancan/controller_additions_spec.rb b/spec/cancan/controller_additions_spec.rb index b16c0c7f..1580a5d0 100644 --- a/spec/cancan/controller_additions_spec.rb +++ b/spec/cancan/controller_additions_spec.rb @@ -130,6 +130,10 @@ expect(@controller_class.cancan_skipper[:authorize][nil]).to eq(only: %i[index show]) @controller_class.skip_authorize_resource(:article) expect(@controller_class.cancan_skipper[:authorize][:article]).to eq({}) + @controller_class.skip_authorize_resource(:article, if: -> {}) + expect(@controller_class.cancan_skipper[:authorize][:article]).to have_key(:if) + @controller_class.skip_authorize_resource(:article, unless: -> {}) + expect(@controller_class.cancan_skipper[:authorize][:article]).to have_key(:unless) end it 'skip_load_resource adds itself to the cancan skipper with given model name and options' do @@ -139,6 +143,10 @@ expect(@controller_class.cancan_skipper[:load][nil]).to eq(only: %i[index show]) @controller_class.skip_load_resource(:article) expect(@controller_class.cancan_skipper[:load][:article]).to eq({}) + @controller_class.skip_load_resource(:article, if: -> {}) + expect(@controller_class.cancan_skipper[:load][:article]).to have_key(:if) + @controller_class.skip_load_resource(:article, unless: -> {}) + expect(@controller_class.cancan_skipper[:load][:article]).to have_key(:unless) end it 'skip_load_and_authorize_resource adds itself to the cancan skipper with given model name and options' do diff --git a/spec/cancan/controller_resource_spec.rb b/spec/cancan/controller_resource_spec.rb index f66eb046..2778485d 100644 --- a/spec/cancan/controller_resource_spec.rb +++ b/spec/cancan/controller_resource_spec.rb @@ -645,6 +645,62 @@ class Section; end expect(CanCan::ControllerResource.new(controller, :model).skip?(:authorize)).to be(true) end + it 'skips resource behavior :if method returns true' do + allow(controller).to receive(:should_skip?) { params[:action] == 'index' } + allow(controller_class).to receive(:cancan_skipper) { { authorize: { model: { if: :should_skip? } } } } + + params[:action] = 'index' + expect(CanCan::ControllerResource.new(controller, :model).skip?(:authorize)).to be_truthy + params[:action] = 'other_action' + expect(CanCan::ControllerResource.new(controller).skip?(:authorize)).to be(false) + expect(CanCan::ControllerResource.new(controller, :model).skip?(:authorize)).to be(false) + end + + it 'skips resource behavior :if lambda returns true' do + allow(controller_class).to receive(:cancan_skipper) { + { + authorize: { + model: { + if: -> { params[:action] == 'index' } + } + } + } + } + params[:action] = 'index' + expect(CanCan::ControllerResource.new(controller, :model).skip?(:authorize)).to be_truthy + params[:action] = 'other_action' + expect(CanCan::ControllerResource.new(controller).skip?(:authorize)).to be(false) + expect(CanCan::ControllerResource.new(controller, :model).skip?(:authorize)).to be(false) + end + + it 'skips resource behavior :unless method returns true' do + allow(controller).to receive(:should_not_skip?) { params[:action] == 'index' } + allow(controller_class).to receive(:cancan_skipper) { { authorize: { model: { unless: :should_not_skip? } } } } + + params[:action] = 'index' + expect(CanCan::ControllerResource.new(controller, :model).skip?(:authorize)).to be_falsey + params[:action] = 'other_action' + expect(CanCan::ControllerResource.new(controller).skip?(:authorize)).to be(false) + expect(CanCan::ControllerResource.new(controller, :model).skip?(:authorize)).to be(true) + end + + it 'skips resource behavior :unless lambda returns true' do + allow(controller_class).to receive(:cancan_skipper) { + { + authorize: { + model: { + unless: -> { params[:action] == 'index' } + } + } + } + } + params[:action] = 'index' + expect(CanCan::ControllerResource.new(controller, :model).skip?(:authorize)).to be_falsey + params[:action] = 'other_action' + expect(CanCan::ControllerResource.new(controller).skip?(:authorize)).to be(false) + expect(CanCan::ControllerResource.new(controller, :model).skip?(:authorize)).to be(true) + end + it 'skips loading and authorization' do allow(controller_class).to receive(:cancan_skipper) { { authorize: { nil => {} }, load: { nil => {} } } } params[:action] = 'new'