]> git.openstreetmap.org Git - rails.git/commitdiff
Merge remote-tracking branch 'upstream/pull/2075'
authorTom Hughes <tom@compton.nu>
Wed, 28 Nov 2018 18:09:20 +0000 (18:09 +0000)
committerTom Hughes <tom@compton.nu>
Wed, 28 Nov 2018 18:09:20 +0000 (18:09 +0000)
app/abilities/ability.rb
app/abilities/capability.rb
app/controllers/application_controller.rb
app/controllers/notes_controller.rb
config/locales/en.yml
test/abilities/abilities_test.rb
test/abilities/capability_test.rb

index 2db2746b6213d4a85114c432f345146372a66968..f2486d2b4307b3551c22a5899e5844eecb643e29 100644 (file)
@@ -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
index 89d88a7605353578d80ad3eedc7bca1e71460053..8ede9bb51bafedf8c40fa1d86bfd146d8ea79ba9 100644 (file)
@@ -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
 
index b9cf449ea2f3ffce06bd9d219b33f8f31b6b11e5..6c6a087b7d1b9dadd75a775ff8a688ad05da966f 100644 (file)
@@ -118,24 +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.
-  def require_moderator
-    unless current_user.moderator?
-      if request.get?
-        flash[:error] = t("application.require_moderator.not_a_moderator")
-        redirect_to :action => "index"
-      else
-        head :forbidden
-      end
-    end
-  end
-
   ##
   # sets up the current_user for use by other methods. this is mostly called
   # from the authorize method, but can be called elsewhere if authorisation
@@ -193,11 +175,6 @@ class ApplicationController < ActionController::Base
   ##
   # to be used as a before_filter *after* authorize. this checks that
   # the user is a moderator and, if not, returns a forbidden error.
-  #
-  # NOTE: this isn't a very good way of doing it - it duplicates logic
-  # from require_moderator - but what we really need to do is a fairly
-  # drastic refactoring based on :format and respond_to? but not a
-  # good idea to do that in this branch.
   def authorize_moderator(errormessage = "Access restricted to moderators")
     # check user is a moderator
     unless current_user.moderator?
index 9cdc38446ca5df7335528d34aeed3bbe5bbae4ad..62fd9d07830a322e1c70e40c125fbc19a2bd3a6a 100644 (file)
@@ -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
 
index ad825ef67162a42665620139595c7e7f8a9862c0..2777c1df30fba99b00fd8d3bfa5f6f67e7916586 100644 (file)
@@ -1811,10 +1811,6 @@ en:
       cookies_needed: "You appear to have cookies disabled - please enable cookies in your browser before continuing."
     require_admin:
       not_an_admin: You need to be an admin to perform that action.
-    require_moderator:
-      not_a_moderator: "You need to be a moderator to perform that action."
-    require_moderator_or_admin:
-      not_a_moderator_or_admin: You need to be a moderator or an admin to perform that action
     setup_user_auth:
       blocked_zero_hour: "You have an urgent message on the OpenStreetMap web site. You need to read the message before you will be able to save your edits."
       blocked: "Your access to the API has been blocked. Please log-in to the web interface to find out more."
index fc37b0e7df9ab034f731057d298a339f408e716a..9444a45f5ecee1f71eaecad118ea44670f970610 100644 (file)
@@ -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
index 8e77537a45c2f4eca02d22895d7cfa6b906f6cbc..ed42ef01a029937339a0042b24c8d69c2f03bee8 100644 (file)
@@ -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