Merge 91f0c01 into 20c811c

CHANGELOG.md

@@ -9,6 +9,7 @@ This changelog adheres to [Keep a CHANGELOG](http://keepachangelog.com/).
 - Improve tests for RightOn::ByGroup
 - Internal improvement of RightOn::ByGroup
 - Internal extraction of 'allowed?' feature for failure message
+- CanCanRight functionality merged into RightOn
 
 ### Fixed
 - [TT-3352] Ensure roles currently in use cannot be deleted

lib/right_on/ability.rb

@@ -0,0 +1,9 @@
+module RightOn
+  module Ability
+    include CanCan::Ability
+
+    private def add_rule_for(right)
+      add_rule(RightOn::Rule.rule_for(right))
+    end
+  end
+end

lib/right_on/controller_additions.rb

@@ -0,0 +1,23 @@
+module RightOn
+  module ControllerAdditions
+    def authorize_action!
+      controller = (self.rights_from || params[:controller]).to_s
+      action = params[:action].to_s
+
+      return if can_access_controller_action?(controller, action)
+
+      fail CanCan::AccessDenied, "You are not authorized to access this page."
+    end
+
+    def can_access_controller_action?(controller, action)
+      (can?(:access, controller) && !Right.where(subject: controller + '#' + action).exists?) ||
+        can?(:access, controller + '#' + action)
+    end
+  end
+end
+
+if defined? ActionController::Base
+  ActionController::Base.class_eval do
+    include RightOn::ControllerAdditions
+  end
+end

lib/right_on/error.rb

@@ -0,0 +1,3 @@
+module RightOn
+  class Error < StandardError; end
+end

lib/right_on/right_allowed.rb

@@ -55,10 +55,6 @@ def self.clear_cache
       cache.delete('Right.all')
     end
 
-    def clear_cache
-      self.class.clear_cache
-    end
-
     attr_accessor :rights
     def self.[](name)
       @rights = cache.read('Right.all') || calculate_and_write_cache

lib/right_on/rule.rb

@@ -0,0 +1,51 @@
+module RightOn
+  class Rule
+    def self.rule_for(right)
+      self.new(right).call
+    end
+
+    def initialize(right)
+      @right = right
+    end
+
+    def call
+      validate!
+
+      CanCan::Rule.new(can?, action, subject, conditions, nil)
+    end
+
+    private
+
+    def validate!
+      fail RightOn::Error, 'must specify an action' unless @right.action.present?
+    end
+
+    def can?
+      @right.can
+    end
+
+    def action
+      @right.action.to_sym
+    end
+
+    def subject
+      model_class || @right.subject
+    end
+
+    def conditions
+      model_class ? @right.conditions : nil
+    end
+
+    def model_class
+      return nil unless @right.subject.present?
+
+      begin
+        model_class = self.class.const_get(@right.subject)
+      rescue NameError
+        model_class = Class
+      end
+
+      return model_class if model_class.ancestors.include?(ActiveRecord::Base)
+    end
+  end
+end

right_on.gemspec

@@ -18,6 +18,7 @@ Gem::Specification.new do |spec|
   spec.test_files    = spec.files.grep(%r{^(test|spec|features)/})
   spec.require_paths = ["lib"]
 
+  spec.add_dependency 'cancancan'
   spec.add_dependency 'activerecord', '>= 4.0.0'
   spec.add_dependency 'activesupport', '>= 4.0.0'
   spec.add_dependency 'input_reader', '~> 0.0'

spec/ability_spec.rb

@@ -0,0 +1,29 @@
+require 'active_support/all'
+require 'cancan/ability'
+require 'right_on/error'
+require 'right_on/rule'
+require 'right_on/ability'
+require 'spec_helper'
+
+describe RightOn::Ability do
+  describe 'private #add_rule_for' do
+    subject(:ability) {
+      class TestAbility
+        include RightOn::Ability
+      end
+
+      TestAbility.new
+    }
+    let(:right) {
+      double(name: 'Do Something', can: true, action: 'action', subject: 'subject', conditions: {})
+    }
+
+    before do
+      ability.send(:add_rule_for, right)
+    end
+
+    it 'should add a rule to the ability' do
+      expect(ability.send(:rules).count).to eq(1)
+    end
+  end
+end

spec/controller_additions_spec.rb

@@ -0,0 +1,137 @@
+require 'active_support/all'
+require 'action_controller'
+require 'cancan/ability'
+require 'cancan/controller_additions'
+require 'cancan/exceptions'
+require 'cancan/rule'
+require 'right_on/ability'
+require 'right_on/controller_additions'
+require 'right_on/error'
+require 'right_on/rule'
+require 'active_record'
+require 'spec_helper'
+
+# Mock this so we don't need to include active record
+module RightOn
+  class Right
+    def self.where(args)
+    end
+  end
+end
+
+describe RightOn::ControllerAdditions do
+  let(:rule_override) { false }
+  before do
+    rule_class = class_double('RightOn::ControllerAdditions::Model')
+    allow(RightOn::Right).to receive(:where).and_return(double(exists?: rule_override))
+  end
+
+  subject(:controller) {
+    class Ability
+      include RightOn::Ability
+
+      def initialize(user)
+
+      end
+    end
+
+    class Controller < ActionController::Base
+      def rights_from
+        nil
+      end
+
+      private
+
+      def params
+        { controller: 'controller', action: 'action' }
+      end
+
+      def current_user
+        nil
+      end
+    end
+
+    Controller.new
+  }
+
+  it 'should respond to authorize_action!' do
+    expect(controller.respond_to? :authorize_action!).to be_truthy
+  end
+
+  describe 'private #authorize_action!' do
+    context 'when the ability has a matching rule' do
+      let(:right) {
+        double(name: 'Do Something', can: true, action: 'access', subject: 'controller#action', conditions: {})
+      }
+
+      before do
+        controller.send(:current_ability).send(:add_rule_for, right)
+      end
+
+      it 'should grant access to controller#action' do
+        expect{controller.send(:authorize_action!)}.to_not(
+          raise_error(CanCan::AccessDenied, 'You are not authorized to access this page.'))
+      end
+    end
+
+    context 'when the ability does not have a matching rule' do
+      let(:right) {
+        double(name: 'Do Something', can: true, action: 'access', subject: 'controller#other_action', conditions: {})
+      }
+
+      before do
+        controller.send(:current_ability).send(:add_rule_for, right)
+      end
+
+      it 'should grant access to controller#action' do
+        expect{controller.send(:authorize_action!)}.to(
+          raise_error(CanCan::AccessDenied, 'You are not authorized to access this page.'))
+      end
+    end
+
+    context 'when the ability has a specific rule overriding the general rule' do
+      let(:rule_override) { true }
+      let(:right) {
+        double(name: 'Generic', can: true, action: 'access', subject: 'controller', conditions: {})
+      }
+
+      before do
+        controller.send(:current_ability).send(:add_rule_for, right)
+      end
+
+      it 'should not grant access to controller#action' do
+        expect{controller.send(:authorize_action!)}.to(
+          raise_error(CanCan::AccessDenied, 'You are not authorized to access this page.'))
+      end
+    end
+  end
+
+  describe 'private #authorize_action!' do
+    let(:controller) {
+      class Controller < ActionController::Base
+        def rights_from
+          :other_controller
+        end
+
+        private
+
+        def params
+          { controller: 'controller', action: 'action' }
+        end
+
+        def current_user
+          nil
+        end
+      end
+
+      Controller.new
+    }
+
+    context 'when rights from is a symbol' do
+      specify do
+        expect{controller.send(:authorize_action!)}.to(
+          raise_error(CanCan::AccessDenied, 'You are not authorized to access this page.'))
+      end
+    end
+  end
+end

spec/right_allowed_spec.rb

@@ -3,8 +3,14 @@
 require 'right_on/right'
 require 'right_on/right_allowed'
 require 'spec_helper'
+require 'support/bootstrap'
 
 describe RightOn::RightAllowed do
+  def right_double(name)
+    default_attrs = { id: rand(1_000_000), action: nil }
+    double default_attrs.merge(Bootstrap.build_right_attrs(name))
+  end
+
   let(:cache) { ActiveSupport::Cache::MemoryStore.new }
 
   before do
@@ -15,12 +21,23 @@
 
   context 'for simple case with one controller right' do
     let(:all) { [users] }
-    let(:users)  { double(id: 1, name: 'name', controller: 'users', action: nil) }
+    let(:users) { right_double('users') }
 
-    it 'should allow all actions' do
-      expect(RightOn::RightAllowed.new('users', 'index').allowed?(users)).to be true
-      expect(RightOn::RightAllowed.new('users', 'edit' ).allowed?(users)).to be true
-      expect(RightOn::RightAllowed.new('users', 'hello').allowed?(users)).to be true
+    subject { RightOn::RightAllowed.new('users', action).allowed?(users) }
+
+    context 'index action' do
+      let(:action) { 'index' }
+      it { is_expected.to be true }
+    end
+
+    context 'edit action' do
+      let(:action) { 'edit' }
+      it { is_expected.to be true }
+    end
+
+    context 'hello action' do
+      let(:action) { 'hello' }
+      it { is_expected.to be true }
     end
   end
 
@@ -31,14 +48,16 @@
     let(:edit_action)  { RightOn::RightAllowed.new('models', 'edit')  }
     let(:hello_action) { RightOn::RightAllowed.new('models', 'hello') }
 
-    let(:other)  { double(id: 2, name: 'models',        controller: 'models', action: nil)      }
-    let(:index)  { double(id: 3, name: 'models#index',  controller: 'models', action: 'index')  }
-    let(:change) { double(id: 4, name: 'models#change', controller: 'models', action: 'change') }
-    let(:view)   { double(id: 5, name: 'models#view',   controller: 'models', action: 'view')   }
+    let(:other)  { right_double('models') }
+    let(:index)  { right_double('models#index') }
+    let(:change) { right_double('models#change') }
+    let(:view)   { right_double('models#view') }
 
     context 'index action' do
       specify do
-        expect(index_action.allowed?(other)).to eq false # as specific action exists
+        # as specific action exists
+        expect(index_action.allowed?(other)).to eq false
+
         expect(index_action.allowed?(index)).to eq true
         expect(index_action.allowed?(view)).to eq true
         expect(index_action.allowed?(change)).to eq true
@@ -47,7 +66,9 @@
 
     context 'edit action' do
       specify do
-        expect(edit_action.allowed?(other)).to eq false # as specific action exists
+        # as specific action exists
+        expect(edit_action.allowed?(other)).to eq false
+
         expect(edit_action.allowed?(index)).to eq false
         expect(edit_action.allowed?(view)).to eq false
         expect(edit_action.allowed?(change)).to eq true
@@ -56,7 +77,9 @@
 
     context 'hello action' do
       specify do
-        expect(hello_action.allowed?(other)).to eq true # as hello isn't defined
+        # as hello isn't defined
+        expect(hello_action.allowed?(other)).to eq true
+
         expect(hello_action.allowed?(index)).to eq false
         expect(hello_action.allowed?(view)).to eq false
         expect(hello_action.allowed?(change)).to eq false