From d6d015aa50afcccaafdcfd1f11451c6a24850feb Mon Sep 17 00:00:00 2001
From: Michael Noack <michael@noack.com.au>
Date: Tue, 10 Apr 2018 15:39:46 +0930
Subject: [PATCH 1/3] Add CanCanRight classes into RightOn

 * Now all dependencies exist in one place
   (Much better than intermingled dependencies)
---
 CHANGELOG.md                         |   1 +
 lib/right_on/ability.rb              |   9 ++
 lib/right_on/controller_additions.rb |  23 +++++
 lib/right_on/error.rb                |   3 +
 lib/right_on/rule.rb                 |  51 ++++++++++
 right_on.gemspec                     |   1 +
 spec/ability_spec.rb                 |  29 ++++++
 spec/controller_additions_spec.rb    | 137 +++++++++++++++++++++++++++
 spec/rule_spec.rb                    |  81 ++++++++++++++++
 spec/support/coverage_loader.rb      |   2 +-
 10 files changed, 336 insertions(+), 1 deletion(-)
 create mode 100644 lib/right_on/ability.rb
 create mode 100644 lib/right_on/controller_additions.rb
 create mode 100644 lib/right_on/error.rb
 create mode 100644 lib/right_on/rule.rb
 create mode 100644 spec/ability_spec.rb
 create mode 100644 spec/controller_additions_spec.rb
 create mode 100644 spec/rule_spec.rb

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2f451c2..194d662 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/lib/right_on/ability.rb b/lib/right_on/ability.rb
new file mode 100644
index 0000000..e326736
--- /dev/null
+++ b/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
diff --git a/lib/right_on/controller_additions.rb b/lib/right_on/controller_additions.rb
new file mode 100644
index 0000000..450c813
--- /dev/null
+++ b/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
diff --git a/lib/right_on/error.rb b/lib/right_on/error.rb
new file mode 100644
index 0000000..5019547
--- /dev/null
+++ b/lib/right_on/error.rb
@@ -0,0 +1,3 @@
+module RightOn
+  class Error < StandardError; end
+end
diff --git a/lib/right_on/rule.rb b/lib/right_on/rule.rb
new file mode 100644
index 0000000..77ceaf7
--- /dev/null
+++ b/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
diff --git a/right_on.gemspec b/right_on.gemspec
index 07d065a..04f98a4 100644
--- a/right_on.gemspec
+++ b/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'
diff --git a/spec/ability_spec.rb b/spec/ability_spec.rb
new file mode 100644
index 0000000..774475e
--- /dev/null
+++ b/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
diff --git a/spec/controller_additions_spec.rb b/spec/controller_additions_spec.rb
new file mode 100644
index 0000000..a3baa3b
--- /dev/null
+++ b/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
diff --git a/spec/rule_spec.rb b/spec/rule_spec.rb
new file mode 100644
index 0000000..a00f2f7
--- /dev/null
+++ b/spec/rule_spec.rb
@@ -0,0 +1,81 @@
+require 'active_record'
+require 'active_support/all'
+require 'cancan/rule'
+require 'right_on/error'
+require 'right_on/rule'
+require 'spec_helper'
+
+describe RightOn::Rule do
+  subject(:rule) { RightOn::Rule.rule_for(right) }
+
+  describe '#self.rule_for' do
+    let(:right) {
+      double(name: 'Do Something', can: true, action: 'action', subject: 'subject', conditions: {})
+    }
+
+    it 'should return a cancan rule' do
+      is_expected.to be_a(CanCan::Rule)
+    end
+  end
+
+  describe '#call' do
+    context 'when an action is not specified' do
+      let(:right) {
+        double(name: 'Do Something', can: true, action: nil, subject: 'subject', conditions: {})
+      }
+
+      it 'should fail with exception' do
+        expect{rule}.to raise_error(RightOn::Error, 'must specify an action')
+      end
+    end
+
+    context 'when the subject is not a model' do
+      let(:right) {
+        double(name: 'Do Something', can: true, action: 'action', subject: 'subject', conditions: {})
+      }
+
+      it 'should return a CanCan::Rule' do
+        is_expected.to be_a(CanCan::Rule)
+      end
+
+      it 'should convert the action to a symbol' do
+        expect(rule.actions).to eq([:action])
+      end
+
+      it 'should set the subject' do
+        expect(rule.subjects).to eq(['subject'])
+      end
+
+      it 'should not have any conditions' do
+        expect(rule.conditions).to eq({})
+      end
+    end
+
+    context 'when the subject is a model' do
+      let(:right) {
+        double(name: 'Do Something', can: true, action: 'action', subject: 'Model', conditions: {})
+      }
+
+      before do
+        class Model < ActiveRecord::Base
+        end
+      end
+
+      it 'should return a CanCan::Rule' do
+        is_expected.to be_a(CanCan::Rule)
+      end
+
+      it 'should convert the action to a symbol' do
+        expect(rule.actions).to eq([:action])
+      end
+
+      it 'should convert the subject to a model' do
+        expect(rule.subjects).to eq([Model])
+      end
+
+      it 'should not have any conditions' do
+        expect(rule.conditions).to eq({})
+      end
+    end
+  end
+end
diff --git a/spec/support/coverage_loader.rb b/spec/support/coverage_loader.rb
index b2b2993..7cb74bb 100644
--- a/spec/support/coverage_loader.rb
+++ b/spec/support/coverage_loader.rb
@@ -1,4 +1,4 @@
 require 'simplecov-rcov'
 require 'coveralls'
 require 'coverage/kit'
-Coverage::Kit.setup(minimum_coverage: 91.1)
+Coverage::Kit.setup(minimum_coverage: 91.8)

From b7f86bef1deb64918e849b03769e75bcf516c659 Mon Sep 17 00:00:00 2001
From: Michael Noack <michael@noack.com.au>
Date: Wed, 11 Apr 2018 11:39:43 +0930
Subject: [PATCH 2/3] Remove unused method and bump coverage

---
 lib/right_on/right_allowed.rb   | 4 ----
 spec/support/coverage_loader.rb | 2 +-
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/lib/right_on/right_allowed.rb b/lib/right_on/right_allowed.rb
index dcbd5db..e54440a 100644
--- a/lib/right_on/right_allowed.rb
+++ b/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
diff --git a/spec/support/coverage_loader.rb b/spec/support/coverage_loader.rb
index 7cb74bb..f3f74d0 100644
--- a/spec/support/coverage_loader.rb
+++ b/spec/support/coverage_loader.rb
@@ -1,4 +1,4 @@
 require 'simplecov-rcov'
 require 'coveralls'
 require 'coverage/kit'
-Coverage::Kit.setup(minimum_coverage: 91.8)
+Coverage::Kit.setup(minimum_coverage: 92.2)

From 6d7959ed6e9ce9e2e6af7b45719f728702edc803 Mon Sep 17 00:00:00 2001
From: Michael Noack <michael@noack.com.au>
Date: Wed, 11 Apr 2018 12:08:18 +0930
Subject: [PATCH 3/3] Make the hound happy for one file

---
 spec/right_allowed_spec.rb | 47 ++++++++++++++++++++++++++++----------
 spec/support/bootstrap.rb  |  8 +++++--
 2 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/spec/right_allowed_spec.rb b/spec/right_allowed_spec.rb
index 40b2b2c..a54f423 100644
--- a/spec/right_allowed_spec.rb
+++ b/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
diff --git a/spec/support/bootstrap.rb b/spec/support/bootstrap.rb
index d0e7a98..0b434cb 100644
--- a/spec/support/bootstrap.rb
+++ b/spec/support/bootstrap.rb
@@ -25,11 +25,15 @@ def self.various_rights_with_actions
   end
 
   def self.create_right(name)
+    RightOn::Right.create!(build_right_attrs(name))
+  end
+
+  def self.build_right_attrs(name)
     if name['#']
       controller, action = name.split('#')
-      RightOn::Right.create!(name: name, controller: controller, action: action)
+      { name: name, controller: controller, action: action }
     else
-      RightOn::Right.create!(name: name, controller: name)
+      { name: name, controller: name }
     end
   end
 end