From ea766ec57dc4c1be796d0f393adc62edb6862436 Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 28 Nov 2018 15:33:43 +0100 Subject: [PATCH 1/1] Use CanCanCan for notes authorization --- app/abilities/ability.rb | 3 ++ app/abilities/capability.rb | 2 ++ app/controllers/application_controller.rb | 4 --- app/controllers/notes_controller.rb | 6 ++-- test/abilities/abilities_test.rb | 32 +++++++++++++++++ test/abilities/capability_test.rb | 42 +++++++++++++++++++++++ 6 files changed, 83 insertions(+), 6 deletions(-) diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index 2db2746b6..f2486d2b4 100644 --- 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, :create, :comment, :feed, :show, :search, :mine], Note 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 [:close, :reopen], Note 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, Note 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 89d88a760..8ede9bb51 100644 --- 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) + 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 :destroy, Note if capability?(token, :allow_write_notes) end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index b9cf449ea..8fa279367 100644 --- 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 - 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. diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index 9cdc38446..62fd9d078 100644 --- 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 :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 :require_allow_write_notes, :only => [:create, :comment, :close, :reopen, :destroy] 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 fc37b0e7d..9444a45f5 100644 --- 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 + + 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 @@ -45,6 +57,18 @@ class UserAbilityTest < AbilityTest 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 @@ -55,6 +79,14 @@ class ModeratorAbilityTest < AbilityTest 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 diff --git a/test/abilities/capability_test.rb b/test/abilities/capability_test.rb index 8e77537a4..ed42ef01a 100644 --- a/test/abilities/capability_test.rb +++ b/test/abilities/capability_test.rb @@ -54,6 +54,48 @@ class ChangesetCommentCapabilityTest < CapabilityTest 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 -- 2.43.2