From f29f75d83b599274fd4565a470aa8521eabfea1e Mon Sep 17 00:00:00 2001 From: Nicolae Claudius Date: Thu, 2 Feb 2012 13:36:19 -0800 Subject: [PATCH] ACL things --- app/controllers/application_controller.rb | 8 +---- app/models/ability.rb | 14 ++++---- app/models/domain.rb | 3 +- app/models/user.rb | 12 +++---- config/initializers/cancan.rb | 9 ++++- spec/models/ability_spec.rb | 43 ++++++++++++----------- spec/support/matchers/be_able_to.rb | 23 ++++++++++++ 7 files changed, 69 insertions(+), 43 deletions(-) create mode 100644 spec/support/matchers/be_able_to.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 09f9d75..1237d41 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -3,6 +3,7 @@ class ApplicationController < ActionController::Base include SentientController protect_from_forgery before_filter :check_honeypot + helper_method :client_remote_ip, :respond_to rescue_from CanCan::AccessDenied, ActiveScaffold::ActionNotAllowed do |exception| flash.now[:error] = exception.message @@ -29,13 +30,6 @@ class ApplicationController < ActionController::Base @client_remote_ip ||= request.env["HTTP_X_FORWARDED_FOR"] end - def current_ability - @current_ability ||= ::Ability.new(:user => current_user) - end - - helper_method :client_remote_ip - helper_method :respond_to - def check_honeypot render :nothing => true if params[Settings.honeypot].present? end diff --git a/app/models/ability.rb b/app/models/ability.rb index 7daaccd..e74c0b8 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -6,12 +6,12 @@ class Ability attr_accessor :context # See the wiki for details: https://github.com/ryanb/cancan/wiki/Defining-Abilities - def initialize(options) - @user = options[:user] || User.new + def initialize(user, options = {}) + @user = user || User.new @context = options[:context] || :application action_aliases - if user.persisted? + if @user.persisted? owner_abilities sharing_abilities end @@ -37,18 +37,18 @@ class Ability # can manage shared domains and records can CRUD, Domain, :permissions.outer => {:user_id => user.id} can CRUD, Record, :domain => {:permissions.outer => {:user_id => user.id}} - + # can manage shared domains and records descendants for domain in user.permitted_domains can CRUD, Domain, :name_reversed.matches => "#{domain.name_reversed}.%" # descendants - can CRUD, Record, :domain => {:name_reversed.matches => "#{domain.name_reversed}.%"} # descendant's + can CRUD, Record, :domain => {:name_reversed.matches => "#{domain.name_reversed}.%"} # descendants end end def action_aliases - alias_action :row, :show_search, :render_field, :to => :read + alias_action :list, :row, :show_search, :render_field, :to => :read alias_action :update_column, :add_association, :edit_associated, - :edit_associated, :new_existing, :add_existing, :to => :update + :edit_associated, :new_existing, :add_existing, :new_token, :to => :update alias_action :delete, :destroy_existing, :to => :destroy end diff --git a/app/models/domain.rb b/app/models/domain.rb index a32d220..f237ef2 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -76,7 +76,8 @@ class Domain < ActiveRecord::Base # If current user present authorize it # If current user not present, just allow (rake tasks etc) def can_be_managed_by_current_user? - User.current.present? ? User.current.can?(:manage, self) : true + return true if User.current.nil? + Ability::CRUD.all?{|operation| User.current.can?(operation, self)} end def slave?; self.type == 'SLAVE' end diff --git a/app/models/user.rb b/app/models/user.rb index dd37954..9002f88 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -31,21 +31,21 @@ class User < ActiveRecord::Base "#{first_name} #{last_name}" end + # domains per user limit for DOS protection + def domains_exceeding? + domains.count >= Settings.max_domains_per_user.to_i + end + delegate :can?, :cannot?, :to => :ability def ability(options = {:reload => false}) options[:reload] ? _ability : (@ability ||= _ability) end - # domains per user limit for DOS protection - def domains_exceeding? - domains.count >= Settings.max_domains_per_user.to_i - end - protected def _ability - Ability.new(:user => self) + Ability.new(self) end end \ No newline at end of file diff --git a/config/initializers/cancan.rb b/config/initializers/cancan.rb index 877526b..c89788c 100644 --- a/config/initializers/cancan.rb +++ b/config/initializers/cancan.rb @@ -57,7 +57,14 @@ module CanCan # Return the conditions directly if there's just one definition @rules.first.conditions.dup else - @rules.reverse.inject(false_sql) {|acc, rule| acc | rule.conditions.dup} + @rules.reverse.inject(false_sql) do |accumulator, rule| + conditions = rule.conditions.dup + if conditions.blank? + rule.base_behavior ? (accumulator | true_sql) : (accumulator & false_sql) + else + rule.base_behavior ? (accumulator | conditions) : (accumulator & -conditions) + end + end end end diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 9d29cb2..ed224ae 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -4,25 +4,26 @@ require 'spec_helper' describe Ability do include_context "data" + let(:crud){Ability::CRUD} context "basic" do it "allows me to manage my domains and their records, and my hosts" do - user.should be_able_to(:manage, domain) - user.should be_able_to(:manage, a_record) - user.should be_able_to(:manage, soa_record) + user.should be_able_to(crud, domain) + user.should be_able_to(crud, a_record) + user.should be_able_to(crud, soa_record) user.should_not be_able_to(:delete, soa_record) # SOA deleted only via parent - user.should be_able_to(:manage, host_a_record) + user.should be_able_to(crud, host_a_record) end it "denies other user to manage my domains and their records, and my hosts" do - user2.should_not be_able_to(:manage, domain) - user2.should_not be_able_to(:manage, a_record) - user2.should_not be_able_to(:manage, soa_record) - user2.should_not be_able_to(:manage, host_a_record) + user2.should_not be_able_to(crud, domain) + user2.should_not be_able_to(crud, a_record) + user2.should_not be_able_to(crud, soa_record) + user2.should_not be_able_to(crud, host_a_record) end it "allows admin to manage other user's hosts" do - admin.should be_able_to(:manage, host_a_record) + admin.should be_able_to(crud, host_a_record) end end @@ -32,35 +33,35 @@ describe Ability do end it "allows other user to manage user's domains and records, if permitted" do - user2.should be_able_to(:manage, domain) - user2.should be_able_to(:manage, a_record) - user2.should be_able_to(:manage, soa_record) + user2.should be_able_to(crud, domain) + user2.should be_able_to(crud, a_record) + user2.should be_able_to(crud, soa_record) end it "denies third user to manage user's permitted domains and records" do - user3.should_not be_able_to(:manage, domain) - user3.should_not be_able_to(:manage, a_record) - user3.should_not be_able_to(:manage, soa_record) + user3.should_not be_able_to(crud, domain) + user3.should_not be_able_to(crud, a_record) + user3.should_not be_able_to(crud, soa_record) end it "allows other user to manage user's permitted subdomains" do - user2.should be_able_to(:manage, subdomain) - user2.should be_able_to(:manage, subsubdomain) + user2.should be_able_to(crud, subdomain) + user2.should be_able_to(crud, subsubdomain) end it "denies third user to manage other user's permitted subdomains" do - user3.should_not be_able_to(:manage, subdomain) - user3.should_not be_able_to(:manage, subsubdomain) + user3.should_not be_able_to(crud, subdomain) + user3.should_not be_able_to(crud, subsubdomain) end end context "permission" do it "allows me to manage my domain's permissions" do - user.should be_able_to(:manage, permission) + user.should be_able_to(crud, permission) end it "denies other user to manage my domain's permissions" do - user2.should_not be_able_to(:manage, permission) + user2.should_not be_able_to(crud, permission) end end diff --git a/spec/support/matchers/be_able_to.rb b/spec/support/matchers/be_able_to.rb new file mode 100644 index 0000000..c30ad02 --- /dev/null +++ b/spec/support/matchers/be_able_to.rb @@ -0,0 +1,23 @@ +# Allow matching arrays in "ba_able_to" +# +# user.should be_able_to [:read, :write], post +# +RSpec::Matchers.define :be_able_to do |*args| + match do |ability| + arguments = args.dup + first = arguments.delete_at(0) + if first.is_a?(Array) + first.all?{|operation| ability.can?(operation, *arguments)} + else + ability.can?(*args) + end + end + + failure_message_for_should do |ability| + "expected to be able to #{args.map(&:inspect).join(" ")}" + end + + failure_message_for_should_not do |ability| + "expected not to be able to #{args.map(&:inspect).join(" ")}" + end +end