From 03dcfd251a37149723a712f6760df053219525d8 Mon Sep 17 00:00:00 2001 From: Jacob Date: Sun, 28 Jun 2020 07:34:16 -0400 Subject: [PATCH] Support strict with_role queries. Issue #362 (#543) * Support strict with_role queries Why: * There is an outstanding issues #362 * strict mode should be enforced on role queries and resource queries How: * Pass a strict flag to the scope function * call where or where_strict depending on strict flag in active_record adapter * Add tests for strict mode resource queries Note: * This PR does not address strict mode for mongoid since I am unfamiliar with that ORM * Update where_strict for mongoid Why: * The tests will fail until mongo works with strict resource querying This change addresses the need by: * Copy the where_strict implementation from the active record adapter * Make sure to call strict where * Refactor to reduce mutations Co-authored-by: Thomas McDonald --- .../adapters/active_record/role_adapter.rb | 26 +++--- lib/rolify/adapters/mongoid/role_adapter.rb | 27 +++--- lib/rolify/finders.rb | 7 +- .../shared_examples_for_finders.rb | 82 +++++++++++-------- 4 files changed, 89 insertions(+), 53 deletions(-) diff --git a/lib/rolify/adapters/active_record/role_adapter.rb b/lib/rolify/adapters/active_record/role_adapter.rb index 011a0411..a1f01fb3 100644 --- a/lib/rolify/adapters/active_record/role_adapter.rb +++ b/lib/rolify/adapters/active_record/role_adapter.rb @@ -9,14 +9,20 @@ def where(relation, *args) end def where_strict(relation, args) - return relation.where(:name => args[:name]) if args[:resource].blank? - resource = if args[:resource].is_a?(Class) - {class: args[:resource].to_s, id: nil} - else - {class: args[:resource].class.name, id: args[:resource].id} - end - - relation.where(:name => args[:name], :resource_type => resource[:class], :resource_id => resource[:id]) + wrap_conditions = relation.name != role_class.name + + conditions = if args[:resource].is_a?(Class) + {:resource_type => args[:resource].to_s, :resource_id => nil } + elsif args[:resource].present? + {:resource_type => args[:resource].class.name, :resource_id => args[:resource].id} + else + {} + end + + conditions.merge!(:name => args[:name]) + conditions = wrap_conditions ? { role_table => conditions } : conditions + + relation.where(conditions) end def find_cached(relation, args) @@ -67,9 +73,9 @@ def exists?(relation, column) relation.where("#{column} IS NOT NULL") end - def scope(relation, conditions) + def scope(relation, conditions, strict) query = relation.joins(:roles) - query = where(query, conditions) + query = strict ? where_strict(query, conditions) : where(query, conditions) query end diff --git a/lib/rolify/adapters/mongoid/role_adapter.rb b/lib/rolify/adapters/mongoid/role_adapter.rb index 042a8b73..a3fb0486 100644 --- a/lib/rolify/adapters/mongoid/role_adapter.rb +++ b/lib/rolify/adapters/mongoid/role_adapter.rb @@ -9,14 +9,20 @@ def where(relation, *args) end def where_strict(relation, args) - return relation.where(:name => args[:name]) if args[:resource].blank? - resource = if args[:resource].is_a?(Class) - {class: args[:resource].to_s, id: nil} - else - {class: args[:resource].class.name, id: args[:resource].id} - end - - relation.where(:name => args[:name], :resource_type => resource[:class], :resource_id => resource[:id]) + wrap_conditions = relation.name != role_class.name + + conditions = if args[:resource].is_a?(Class) + {:resource_type => args[:resource].to_s, :resource_id => nil } + elsif args[:resource].present? + {:resource_type => args[:resource].class.name, :resource_id => args[:resource].id} + else + {} + end + + conditions.merge!(:name => args[:name]) + conditions = wrap_conditions ? { role_table => conditions } : conditions + + relation.where(conditions) end def find_cached(relation, args) @@ -84,8 +90,9 @@ def exists?(relation, column) relation.where(column.to_sym.ne => nil) end - def scope(relation, conditions) - roles = where(role_class, conditions).map { |role| role.id } + def scope(relation, conditions, strict) + query = strict ? where_strict(role_class, conditions) : where(role_class, conditions) + roles = query.map { |role| role.id } return [] if roles.size.zero? query = relation.any_in(:role_ids => roles) query diff --git a/lib/rolify/finders.rb b/lib/rolify/finders.rb index b17557d5..c7306ac4 100644 --- a/lib/rolify/finders.rb +++ b/lib/rolify/finders.rb @@ -1,7 +1,12 @@ module Rolify module Finders def with_role(role_name, resource = nil) - self.adapter.scope(self, :name => role_name, :resource => resource) + strict = self.strict_rolify and resource and resource != :any + self.adapter.scope( + self, + { :name => role_name, :resource => resource }, + strict + ) end def without_role(role_name, resource = nil) diff --git a/spec/rolify/shared_examples/shared_examples_for_finders.rb b/spec/rolify/shared_examples/shared_examples_for_finders.rb index 0c9194c7..216d12eb 100644 --- a/spec/rolify/shared_examples/shared_examples_for_finders.rb +++ b/spec/rolify/shared_examples/shared_examples_for_finders.rb @@ -4,47 +4,65 @@ it { should respond_to(:with_role).with(1).argument } it { should respond_to(:with_role).with(2).arguments } - context "with a global role" do - it { subject.with_role("admin".send(param_method)).should eq([ root ]) } - it { subject.with_role("moderator".send(param_method)).should be_empty } - it { subject.with_role("visitor".send(param_method)).should be_empty } - end - - context "with a class scoped role" do - context "on Forum class" do - it { subject.with_role("admin".send(param_method), Forum).should eq([ root ]) } - it { subject.with_role("moderator".send(param_method), Forum).should eq([ modo ]) } - it { subject.with_role("visitor".send(param_method), Forum).should be_empty } + context "when resource setting: strict is set to false" do + context "with a global role" do + it { subject.with_role("admin".send(param_method)).should eq([ root ]) } + it { subject.with_role("moderator".send(param_method)).should be_empty } + it { subject.with_role("visitor".send(param_method)).should be_empty } end - context "on Group class" do - it { subject.with_role("admin".send(param_method), Group).should eq([ root ]) } - it { subject.with_role("moderator".send(param_method), Group).should eq([ root ]) } - it { subject.with_role("visitor".send(param_method), Group).should be_empty } + context "with a class scoped role" do + context "on Forum class" do + it { subject.with_role("admin".send(param_method), Forum).should eq([ root ]) } + it { subject.with_role("moderator".send(param_method), Forum).should eq([ modo ]) } + it { subject.with_role("visitor".send(param_method), Forum).should be_empty } + end + + context "on Group class" do + it { subject.with_role("admin".send(param_method), Group).should eq([ root ]) } + it { subject.with_role("moderator".send(param_method), Group).should eq([ root ]) } + it { subject.with_role("visitor".send(param_method), Group).should be_empty } + end end - end - context "with an instance scoped role" do - context "on Forum.first instance" do - it { subject.with_role("admin".send(param_method), Forum.first).should eq([ root ]) } - it { subject.with_role("moderator".send(param_method), Forum.first).should eq([ modo ]) } - it { subject.with_role("visitor".send(param_method), Forum.first).should be_empty } + context "with an instance scoped role" do + context "on Forum.first instance" do + it { subject.with_role("admin".send(param_method), Forum.first).should eq([ root ]) } + it { subject.with_role("moderator".send(param_method), Forum.first).should eq([ modo ]) } + it { subject.with_role("visitor".send(param_method), Forum.first).should be_empty } + end + + context "on Forum.last instance" do + it { subject.with_role("admin".send(param_method), Forum.last).should eq([ root ]) } + it { subject.with_role("moderator".send(param_method), Forum.last).should eq([ modo ]) } + it { subject.with_role("visitor".send(param_method), Forum.last).should include(root, visitor) } # =~ doesn't pass using mongoid, don't know why... + end + + context "on Group.first instance" do + it { subject.with_role("admin".send(param_method), Group.first).should eq([ root ]) } + it { subject.with_role("moderator".send(param_method), Group.first).should eq([ root ]) } + it { subject.with_role("visitor".send(param_method), Group.first).should eq([ modo ]) } + end + + context "on Company.first_instance" do + it { subject.with_role("owner".send(param_method), Company.first).should eq([ owner ]) } + end end + end - context "on Forum.last instance" do - it { subject.with_role("admin".send(param_method), Forum.last).should eq([ root ]) } - it { subject.with_role("moderator".send(param_method), Forum.last).should eq([ modo ]) } - it { subject.with_role("visitor".send(param_method), Forum.last).should include(root, visitor) } # =~ doesn't pass using mongoid, don't know why... + context "when resource setting: strict is set to true" do + before(:context) do + user_class.strict_rolify = true end - - context "on Group.first instance" do - it { subject.with_role("admin".send(param_method), Group.first).should eq([ root ]) } - it { subject.with_role("moderator".send(param_method), Group.first).should eq([ root ]) } - it { subject.with_role("visitor".send(param_method), Group.first).should eq([ modo ]) } + after(:context) do + user_class.strict_rolify = false end - context "on Company.first_instance" do - it { subject.with_role("owner".send(param_method), Company.first).should eq([ owner ]) } + context "with an instance scoped role" do + context "on Forum.first instance" do + it { subject.with_role("admin".send(param_method), Forum.first).should be_empty } + it { subject.with_role("moderator".send(param_method), Forum.first).should be_empty } + end end end end