separate ability and capability

These are asking fundamentally different questions;

Abilities are asking the application if the user has a role that allows
the user to take a certain action
Capabilities are asking if the user has granted the application to
perform a certain type of action

CanCanCan makes no distinction, however, so the `granted_capabilities`
method is provided as a point that can be checked in rescue methods, so
that one can _attempt_ to continue to provide the more informative error
messages around permission refusals

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index b6a2467a4149d01bbfebe932d128f0331ed2c393..2be8c16371546e9fe39e7491b8e11a66536e54a4 100644 (file)
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -471,7 +471,11 @@ class ApplicationController < ActionController::Base
   end
 
   def current_ability
-    Ability.new(current_user, current_token)
+    Ability.new(current_user).merge(granted_capabily)
+  end
+
+  def granted_capabily
+    Capability.new(current_user, current_token)
   end
 
   def deny_access(exception)

diff --git a/app/models/ability.rb b/app/models/ability.rb
index d33430fb4fbc285148d216863f7e242c2c955a9e..2f86ea4120df3fc3de02b4351055aea79530def0 100644 (file)
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -3,7 +3,7 @@
 class Ability
   include CanCan::Ability
 
-  def initialize(user, token)
+  def initialize(user)
     can :index, :site
     can [:permalink, :edit, :help, :fixthemap, :offline, :export, :about, :preview, :copyright, :key, :id, :welcome], :site
 
@@ -17,9 +17,6 @@ class Ability
 
       can [:create, :edit, :comment, :subscribe, :unsubscribe], DiaryEntry
 
-      can [:read, :read_one], UserPreference if has_capability?(token, :allow_read_prefs)
-      can [:update, :update_one, :delete_one], UserPreference if has_capability?(token, :allow_write_prefs)
-
       if user.administrator?
         can [:hide, :hidecomment], [DiaryEntry, DiaryComment]
       end
@@ -51,10 +48,4 @@ class Ability
     # See the wiki for details:
     # https://github.com/CanCanCommunity/cancancan/wiki/Defining-Abilities
   end
-
-  # If a user provides no tokens, they've authenticated via a non-oauth method
-  # and permission to access to all capabilities is assumed.
-  def has_capability?(token, cap)
-    token.nil? || token.read_attribute(cap)
-  end
 end

diff --git a/app/models/capability.rb b/app/models/capability.rb
new file mode 100644 (file)
index 0000000..1746875
--- /dev/null
+++ b/app/models/capability.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class Capability
+  include CanCan::Ability
+
+  def initialize(user, token)
+    if user
+      can [:read, :read_one], UserPreference if has_capability?(token, :allow_read_prefs)
+      can [:update, :update_one, :delete_one], UserPreference if has_capability?(token, :allow_write_prefs)
+
+    end
+  end
+
+  # If a user provides no tokens, they've authenticated via a non-oauth method
+  # and permission to access to all capabilities is assumed.
+  def has_capability?(token, cap)
+    token.nil? || token.read_attribute(cap)
+  end
+end

diff --git a/test/models/abilities_test.rb b/test/models/abilities_test.rb
index 298e8299b4dd0988f916759baf11b7f5a1592fdc..77c14f40f5fb30498a01adf78ea95f973c54ee7c 100644 (file)
--- a/test/models/abilities_test.rb
+++ b/test/models/abilities_test.rb
@@ -3,21 +3,12 @@
 require "test_helper"
 
 class AbilityTest < ActiveSupport::TestCase
-
-  def tokens(*toks)
-    AccessToken.new do |token|
-      toks.each do |t|
-        token.public_send("#{t}=", true)
-      end
-    end
-  end
-
 end
 
 class GuestAbilityTest < AbilityTest
 
   test "geocoder permission for a guest" do
-    ability = Ability.new nil, tokens
+    ability = Ability.new nil
 
     [:search, :search_latlon, :search_ca_postcode, :search_osm_nominatim,
      :search_geonames, :search_osm_nominatim_reverse, :search_geonames_reverse].each do |action|
@@ -26,7 +17,7 @@ class GuestAbilityTest < AbilityTest
   end
 
   test "diary permissions for a guest" do
-    ability = Ability.new nil, tokens
+    ability = Ability.new nil
     [:list, :rss, :view, :comments].each do |action|
       assert ability.can?(action, DiaryEntry), "should be able to #{action} DiaryEntries"
     end
@@ -42,7 +33,7 @@ end
 class UserAbilityTest < AbilityTest
 
   test "Diary permissions" do
-    ability = Ability.new create(:user), tokens
+    ability = Ability.new create(:user)
 
     [:list, :rss, :view, :comments, :create, :edit, :comment, :subscribe, :unsubscribe].each do |action|
       assert ability.can?(action, DiaryEntry), "should be able to #{action} DiaryEntries"
@@ -53,48 +44,12 @@ class UserAbilityTest < AbilityTest
       assert ability.cannot?(action, DiaryComment), "should be able to #{action} DiaryEntries"
     end
   end
-
-  test "user preferences" do
-    user = create(:user)
-
-    # a user with no tokens
-    ability = Ability.new create(:user), nil
-    [:read, :read_one, :update, :update_one, :delete_one].each do |act|
-      assert ability.can? act, UserPreference
-    end
-
-    # A user with empty tokens
-    ability = Ability.new create(:user), tokens
-
-    [:read, :read_one, :update, :update_one, :delete_one].each do |act|
-      assert ability.cannot? act, UserPreference
-    end
-
-    ability = Ability.new user, tokens(:allow_read_prefs)
-
-    [:update, :update_one, :delete_one].each do |act|
-      assert ability.cannot? act, UserPreference
-    end
-
-    [:read, :read_one].each do |act|
-      assert ability.can? act, UserPreference
-    end
-
-    ability = Ability.new user, tokens(:allow_write_prefs)
-    [:read, :read_one].each do |act|
-      assert ability.cannot? act, UserPreference
-    end
-
-    [:update, :update_one, :delete_one].each do |act|
-      assert ability.can? act, UserPreference
-    end
-  end
 end
 
 class AdministratorAbilityTest < AbilityTest
 
   test "Diary for an administrator" do
-    ability = Ability.new create(:administrator_user), tokens
+    ability = Ability.new create(:administrator_user)
     [:list, :rss, :view, :comments, :create, :edit, :comment, :subscribe, :unsubscribe, :hide, :hidecomment].each do |action|
       assert ability.can?(action, DiaryEntry), "should be able to #{action} DiaryEntries"
     end
@@ -105,7 +60,7 @@ class AdministratorAbilityTest < AbilityTest
   end
 
   test "administrator does not auto-grant user preferences" do
-    ability = Ability.new create(:administrator_user), tokens
+    ability = Ability.new create(:administrator_user)
 
     [:read, :read_one, :update, :update_one, :delete_one].each do |act|
       assert ability.cannot? act, UserPreference

diff --git a/test/models/capability_test.rb b/test/models/capability_test.rb
new file mode 100644 (file)
index 0000000..2d5d650
--- /dev/null
+++ b/test/models/capability_test.rb
@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+class CapabilityTest < ActiveSupport::TestCase
+  def tokens(*toks)
+    AccessToken.new do |token|
+      toks.each do |t|
+        token.public_send("#{t}=", true)
+      end
+    end
+  end
+end
+
+class UserCapabilityTest < CapabilityTest
+  test "user preferences" do
+    user = create(:user)
+
+    # a user with no tokens
+    capability = Capability.new create(:user), nil
+    [:read, :read_one, :update, :update_one, :delete_one].each do |act|
+      assert capability.can? act, UserPreference
+    end
+
+    # A user with empty tokens
+    capability = Capability.new create(:user), tokens
+
+    [:read, :read_one, :update, :update_one, :delete_one].each do |act|
+      assert capability.cannot? act, UserPreference
+    end
+
+    capability = Capability.new user, tokens(:allow_read_prefs)
+
+    [:update, :update_one, :delete_one].each do |act|
+      assert capability.cannot? act, UserPreference
+    end
+
+    [:read, :read_one].each do |act|
+      assert capability.can? act, UserPreference
+    end
+
+    capability = Capability.new user, tokens(:allow_write_prefs)
+    [:read, :read_one].each do |act|
+      assert capability.cannot? act, UserPreference
+    end
+
+    [:update, :update_one, :delete_one].each do |act|
+      assert capability.can? act, UserPreference
+    end
+  end
+end