Use CanCanCan for notes authorization

diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb
index 2db2746b6213d4a85114c432f345146372a66968..f2486d2b4307b3551c22a5899e5844eecb643e29 100644 (file)
--- a/app/abilities/ability.rb
+++ b/app/abilities/ability.rb
@@ -9,6 +9,7 @@ class Ability
     can [:index, :rss, :show, :comments], DiaryEntry
     can [:search, :search_latlon, :search_ca_postcode, :search_osm_nominatim,
          :search_geonames, :search_osm_nominatim_reverse, :search_geonames_reverse], :geocoder
     can [:index, :rss, :show, :comments], DiaryEntry
     can [:search, :search_latlon, :search_ca_postcode, :search_osm_nominatim,
          :search_geonames, :search_osm_nominatim_reverse, :search_geonames_reverse], :geocoder

+    can [:index, :create, :comment, :feed, :show, :search, :mine], Note

     can [:index, :show], Redaction
     can [:index, :show, :blocks_on, :blocks_by], UserBlock
 
     can [:index, :show], Redaction
     can [:index, :show, :blocks_on, :blocks_by], UserBlock
 


@@ -16,6 +17,7 @@ class Ability
       can :welcome, :site
       can :create, ChangesetComment
       can [:create, :edit, :comment, :subscribe, :unsubscribe], DiaryEntry
       can :welcome, :site
       can :create, ChangesetComment
       can [:create, :edit, :comment, :subscribe, :unsubscribe], DiaryEntry

+      can [:close, :reopen], Note

       can [:new, :create], Report
       can [:read, :read_one, :update, :update_one, :delete_one], UserPreference
 
       can [:new, :create], Report
       can [:read, :read_one, :update, :update_one, :delete_one], UserPreference
 


@@ -23,6 +25,7 @@ class Ability
         can [:destroy, :restore], ChangesetComment
         can [:index, :show, :resolve, :ignore, :reopen], Issue
         can :create, IssueComment
         can [:destroy, :restore], ChangesetComment
         can [:index, :show, :resolve, :ignore, :reopen], Issue
         can :create, IssueComment

+        can :destroy, Note

         can [:new, :create, :edit, :update, :destroy], Redaction
         can [:new, :edit, :create, :update, :revoke], UserBlock
       end
         can [:new, :create, :edit, :update, :destroy], Redaction
         can [:new, :edit, :create, :update, :revoke], UserBlock
       end

diff --git a/app/abilities/capability.rb b/app/abilities/capability.rb
index 89d88a7605353578d80ad3eedc7bca1e71460053..8ede9bb51bafedf8c40fa1d86bfd146d8ea79ba9 100644 (file)
--- a/app/abilities/capability.rb
+++ b/app/abilities/capability.rb
@@ -5,11 +5,13 @@ class Capability
 
   def initialize(token)
     can :create, ChangesetComment if capability?(token, :allow_write_api)
 
   def initialize(token)
     can :create, ChangesetComment if capability?(token, :allow_write_api)

+    can [:create, :comment, :close, :reopen], Note if capability?(token, :allow_write_notes)

     can [:read, :read_one], UserPreference if capability?(token, :allow_read_prefs)
     can [:update, :update_one, :delete_one], UserPreference if capability?(token, :allow_write_prefs)
 
     if token&.user&.moderator?
       can [:destroy, :restore], ChangesetComment if capability?(token, :allow_write_api)
     can [:read, :read_one], UserPreference if capability?(token, :allow_read_prefs)
     can [:update, :update_one, :delete_one], UserPreference if capability?(token, :allow_write_prefs)
 
     if token&.user&.moderator?
       can [:destroy, :restore], ChangesetComment if capability?(token, :allow_write_api)

+      can :destroy, Note if capability?(token, :allow_write_notes)

     end
   end
 
     end
   end
 

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index b9cf449ea2f3ffce06bd9d219b33f8f31b6b11e5..8fa279367efd3793150f02f2a582a3da01134415 100644 (file)
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -118,10 +118,6 @@ class ApplicationController < ActionController::Base
     require_capability(:allow_write_gpx)
   end
 
     require_capability(:allow_write_gpx)
   end
 

-  def require_allow_write_notes
-    require_capability(:allow_write_notes)
-  end
-

   ##
   # require that the user is a moderator, or fill out a helpful error message
   # and return them to the index for the controller this is wrapped from.
   ##
   # require that the user is a moderator, or fill out a helpful error message
   # and return them to the index for the controller this is wrapped from.

diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb
index 9cdc38446ca5df7335528d34aeed3bbe5bbae4ad..62fd9d07830a322e1c70e40c125fbc19a2bd3a6a 100644 (file)
--- a/app/controllers/notes_controller.rb
+++ b/app/controllers/notes_controller.rb
@@ -6,9 +6,11 @@ class NotesController < ApplicationController
   before_action :authorize_web, :only => [:mine]
   before_action :setup_user_auth, :only => [:create, :comment, :show]
   before_action :authorize, :only => [:close, :reopen, :destroy]
   before_action :authorize_web, :only => [:mine]
   before_action :setup_user_auth, :only => [:create, :comment, :show]
   before_action :authorize, :only => [:close, :reopen, :destroy]

-  before_action :require_moderator, :only => [:destroy]
+  before_action :api_deny_access_handler, :except => [:mine]
+
+  authorize_resource
+

   before_action :check_api_writable, :only => [:create, :comment, :close, :reopen, :destroy]
   before_action :check_api_writable, :only => [:create, :comment, :close, :reopen, :destroy]

-  before_action :require_allow_write_notes, :only => [:create, :comment, :close, :reopen, :destroy]

   before_action :set_locale
   around_action :api_call_handle_error, :api_call_timeout
 
   before_action :set_locale
   around_action :api_call_handle_error, :api_call_timeout
 

diff --git a/test/abilities/abilities_test.rb b/test/abilities/abilities_test.rb
index fc37b0e7df9ab034f731057d298a339f408e716a..9444a45f5ecee1f71eaecad118ea44670f970610 100644 (file)
--- a/test/abilities/abilities_test.rb
+++ b/test/abilities/abilities_test.rb
@@ -26,6 +26,18 @@ class GuestAbilityTest < AbilityTest
       assert ability.cannot?(action, DiaryComment), "should not be able to #{action} DiaryEntries"
     end
   end
       assert ability.cannot?(action, DiaryComment), "should not be able to #{action} DiaryEntries"
     end
   end

+
+  test "note permissions for a guest" do
+    ability = Ability.new nil
+
+    [:index, :create, :comment, :feed, :show, :search, :mine].each do |action|
+      assert ability.can?(action, Note), "should be able to #{action} Notes"
+    end
+
+    [:close, :reopen, :destroy].each do |action|
+      assert ability.cannot?(action, Note), "should not be able to #{action} Notes"
+    end
+  end

 end
 
 class UserAbilityTest < AbilityTest
 end
 
 class UserAbilityTest < AbilityTest


@@ -45,6 +57,18 @@ class UserAbilityTest < AbilityTest
       assert ability.cannot?(action, Issue), "should not be able to #{action} Issues"
     end
   end
       assert ability.cannot?(action, Issue), "should not be able to #{action} Issues"
     end
   end

+
+  test "Note permissions" do
+    ability = Ability.new create(:user)
+
+    [:index, :create, :comment, :feed, :show, :search, :mine, :close, :reopen].each do |action|
+      assert ability.can?(action, Note), "should be able to #{action} Notes"
+    end
+
+    [:destroy].each do |action|
+      assert ability.cannot?(action, Note), "should not be able to #{action} Notes"
+    end
+  end

 end
 
 class ModeratorAbilityTest < AbilityTest
 end
 
 class ModeratorAbilityTest < AbilityTest


@@ -55,6 +79,14 @@ class ModeratorAbilityTest < AbilityTest
       assert ability.can?(action, Issue), "should be able to #{action} Issues"
     end
   end
       assert ability.can?(action, Issue), "should be able to #{action} Issues"
     end
   end

+
+  test "Note permissions" do
+    ability = Ability.new create(:moderator_user)
+
+    [:index, :create, :comment, :feed, :show, :search, :mine, :close, :reopen, :destroy].each do |action|
+      assert ability.can?(action, Note), "should be able to #{action} Notes"
+    end
+  end

 end
 
 class AdministratorAbilityTest < AbilityTest
 end
 
 class AdministratorAbilityTest < AbilityTest

diff --git a/test/abilities/capability_test.rb b/test/abilities/capability_test.rb
index 8e77537a45c2f4eca02d22895d7cfa6b906f6cbc..ed42ef01a029937339a0042b24c8d69c2f03bee8 100644 (file)
--- a/test/abilities/capability_test.rb
+++ b/test/abilities/capability_test.rb
@@ -54,6 +54,48 @@ class ChangesetCommentCapabilityTest < CapabilityTest
   end
 end
 
   end
 end
 

+class NoteCapabilityTest < CapabilityTest
+  test "as a normal user with permissionless token" do
+    token = create(:access_token)
+    capability = Capability.new token
+
+    [:create, :comment, :close, :reopen, :destroy].each do |action|
+      assert capability.cannot? action, Note
+    end
+  end
+
+  test "as a normal user with allow_write_notes token" do
+    token = create(:access_token, :allow_write_notes => true)
+    capability = Capability.new token
+
+    [:destroy].each do |action|
+      assert capability.cannot? action, Note
+    end
+
+    [:create, :comment, :close, :reopen].each do |action|
+      assert capability.can? action, Note
+    end
+  end
+
+  test "as a moderator with permissionless token" do
+    token = create(:access_token, :user => create(:moderator_user))
+    capability = Capability.new token
+
+    [:destroy].each do |action|
+      assert capability.cannot? action, Note
+    end
+  end
+
+  test "as a moderator with allow_write_notes token" do
+    token = create(:access_token, :user => create(:moderator_user), :allow_write_notes => true)
+    capability = Capability.new token
+
+    [:destroy].each do |action|
+      assert capability.can? action, Note
+    end
+  end
+end
+

 class UserCapabilityTest < CapabilityTest
   test "user preferences" do
     # a user with no tokens
 class UserCapabilityTest < CapabilityTest
   test "user preferences" do
     # a user with no tokens