From: Tom Hughes Date: Wed, 27 Mar 2019 18:19:08 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/2192' X-Git-Tag: live~3758 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/dbbbd62ef1982e905ceb23f1278e9b4bfdb49d5b?hp=69b952ae78b0e7d7ded7d121070c24093e0dd322 Merge remote-tracking branch 'upstream/pull/2192' --- diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index 7e8e921a2..d2864e452 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -6,28 +6,21 @@ class Ability def initialize(user) can [:relation, :relation_history, :way, :way_history, :node, :node_history, :changeset, :note, :new_note, :query], :browse - can :show, :capability - can :index, :change can :search, :direction can [:index, :permalink, :edit, :help, :fixthemap, :offline, :export, :about, :preview, :copyright, :key, :id], :site can [:finish, :embed], :export can [:search, :search_latlon, :search_ca_postcode, :search_osm_nominatim, :search_geonames, :search_osm_nominatim_reverse, :search_geonames_reverse], :geocoder - can :index, :map can [:token, :request_token, :access_token, :test_request], :oauth - can :show, :permission - can [:search_all, :search_nodes, :search_ways, :search_relations], :search - can [:trackpoints], :swf if Settings.status != "database_offline" - can [:index, :feed, :show, :download, :query], Changeset + can [:index, :feed], Changeset can :index, ChangesetComment can [:index, :rss, :show, :comments], DiaryEntry - can [:index, :create, :comment, :feed, :show, :search, :mine], Note + can [:mine], Note can [:index, :show], Redaction can [:index, :show, :data, :georss, :picture, :icon], Trace - can :index, Tracepoint - can [:terms, :api_users, :login, :logout, :new, :create, :save, :confirm, :confirm_resend, :confirm_email, :lost_password, :reset_password, :show, :api_read, :auth_success, :auth_failure], User + can [:terms, :login, :logout, :new, :create, :save, :confirm, :confirm_resend, :confirm_email, :lost_password, :reset_password, :show, :auth_success, :auth_failure], User can [:index, :show, :blocks_on, :blocks_by], UserBlock can [:index, :show], Node can [:index, :show, :full, :ways_for_node], Way @@ -47,31 +40,14 @@ class Ability can [:new, :create, :reply, :show, :inbox, :outbox, :mark, :destroy], Message can [:close, :reopen], Note can [:new, :create], Report - can [:mine, :new, :create, :edit, :update, :delete, :api_create, :api_read, :api_update, :api_delete, :api_data], Trace - can [:account, :go_public, :make_friend, :remove_friend, :api_details, :api_gpx_files], User - can [:read, :read_one, :update, :update_one, :delete_one], UserPreference - - if user.terms_agreed? - can [:create, :update, :upload, :close, :subscribe, :unsubscribe, :expand_bbox], Changeset - can :create, ChangesetComment - can [:create, :update, :delete], Node - can [:create, :update, :delete], Way - can [:create, :update, :delete], Relation - end + can [:mine, :new, :create, :edit, :update, :delete], Trace + can [:account, :go_public, :make_friend, :remove_friend], User if user.moderator? - 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 - - if user.terms_agreed? - can :redact, OldNode - can :redact, OldWay - can :redact, OldRelation - end end if user.administrator? diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb new file mode 100644 index 000000000..54bc8fb4c --- /dev/null +++ b/app/abilities/api_ability.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +class ApiAbility + include CanCan::Ability + + def initialize(user) + can :show, :capability + can :index, :change + can :index, :map + can :show, :permission + can [:search_all, :search_nodes, :search_ways, :search_relations], :search + can [:trackpoints], :swf + + if Settings.status != "database_offline" + can [:show, :download, :query], Changeset + can [:index, :create, :comment, :feed, :show, :search], Note + can :index, Tracepoint + can [:api_users, :api_read], User + can [:index, :show], Node + can [:index, :show, :full, :ways_for_node], Way + can [:index, :show, :full, :relations_for_node, :relations_for_way, :relations_for_relation], Relation + can [:history, :version], OldNode + can [:history, :version], OldWay + can [:history, :version], OldRelation + end + + if user + can :welcome, :site + can [:revoke, :authorize], :oauth + + if Settings.status != "database_offline" + can [:index, :new, :create, :show, :edit, :update, :destroy], ClientApplication + can [:new, :create, :reply, :show, :inbox, :outbox, :mark, :destroy], Message + can [:close, :reopen], Note + can [:new, :create], Report + can [:api_create, :api_read, :api_update, :api_delete, :api_data], Trace + can [:api_details, :api_gpx_files], User + can [:read, :read_one, :update, :update_one, :delete_one], UserPreference + + if user.terms_agreed? + can [:create, :update, :upload, :close, :subscribe, :unsubscribe, :expand_bbox], Changeset + can :create, ChangesetComment + can [:create, :update, :delete], Node + can [:create, :update, :delete], Way + can [:create, :update, :delete], Relation + end + + if user.moderator? + can [:destroy, :restore], ChangesetComment + can :destroy, Note + + if user.terms_agreed? + can :redact, OldNode + can :redact, OldWay + can :redact, OldRelation + end + end + end + end + + # Define abilities for the passed in user here. For example: + # + # user ||= User.new # guest user (not logged in) + # if user.admin? + # can :manage, :all + # else + # can :read, :all + # end + # + # The first argument to `can` is the action you are giving the user + # permission to do. + # If you pass :manage it will apply to every action. Other common actions + # here are :read, :create, :update and :destroy. + # + # The second argument is the resource the user can perform the action on. + # If you pass :all it will apply to every resource. Otherwise pass a Ruby + # class of the resource. + # + # The third argument is an optional hash of conditions to further filter the + # objects. + # For example, here the user can only update published articles. + # + # can :update, Article, :published => true + # + # See the wiki for details: + # https://github.com/CanCanCommunity/cancancan/wiki/Defining-Abilities + end +end diff --git a/app/abilities/capability.rb b/app/abilities/api_capability.rb similarity index 98% rename from app/abilities/capability.rb rename to app/abilities/api_capability.rb index f4c24e97d..9f59b1f24 100644 --- a/app/abilities/capability.rb +++ b/app/abilities/api_capability.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class Capability +class ApiCapability include CanCan::Ability def initialize(token) diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 579af27cf..df7cfe93b 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -16,6 +16,15 @@ class ApiController < ApplicationController end end + def current_ability + # Use capabilities from the oauth token if it exists and is a valid access token + if Authenticator.new(self, [:token]).allow? + ApiAbility.new(nil).merge(ApiCapability.new(current_token)) + else + ApiAbility.new(current_user) + end + end + def deny_access(_exception) if current_token set_locale diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3ab09b63d..c880e6add 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -329,12 +329,7 @@ class ApplicationController < ActionController::Base end def current_ability - # Use capabilities from the oauth token if it exists and is a valid access token - if Authenticator.new(self, [:token]).allow? - Ability.new(nil).merge(Capability.new(current_token)) - else - Ability.new(current_user) - end + Ability.new(current_user) end def deny_access(_exception) diff --git a/test/abilities/abilities_test.rb b/test/abilities/abilities_test.rb index b951e23e5..f43b6bf50 100644 --- a/test/abilities/abilities_test.rb +++ b/test/abilities/abilities_test.rb @@ -30,13 +30,9 @@ class GuestAbilityTest < AbilityTest test "note permissions for a guest" do ability = Ability.new nil - [:index, :create, :comment, :feed, :show, :search, :mine].each do |action| + [: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 test "user roles permissions for a guest" do @@ -65,18 +61,6 @@ 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 @@ -88,14 +72,6 @@ class ModeratorAbilityTest < AbilityTest 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 - test "User Roles permissions" do ability = Ability.new create(:moderator_user) diff --git a/test/abilities/api_abilities_test.rb b/test/abilities/api_abilities_test.rb new file mode 100644 index 000000000..7734ce996 --- /dev/null +++ b/test/abilities/api_abilities_test.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require "test_helper" + +class ApiAbilityTest < ActiveSupport::TestCase +end + +class GuestApiAbilityTest < ApiAbilityTest + test "note permissions for a guest" do + ability = ApiAbility.new nil + + [:index, :create, :comment, :feed, :show, :search].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 UserApiAbilityTest < ApiAbilityTest + test "Note permissions" do + ability = ApiAbility.new create(:user) + + [:index, :create, :comment, :feed, :show, :search, :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 ModeratorApiAbilityTest < ApiAbilityTest + test "Note permissions" do + ability = ApiAbility.new create(:moderator_user) + + [:index, :create, :comment, :feed, :show, :search, :close, :reopen, :destroy].each do |action| + assert ability.can?(action, Note), "should be able to #{action} Notes" + end + end +end diff --git a/test/abilities/capability_test.rb b/test/abilities/api_capability_test.rb similarity index 80% rename from test/abilities/capability_test.rb rename to test/abilities/api_capability_test.rb index ed42ef01a..8d0e682f6 100644 --- a/test/abilities/capability_test.rb +++ b/test/abilities/api_capability_test.rb @@ -2,7 +2,7 @@ require "test_helper" -class CapabilityTest < ActiveSupport::TestCase +class ApiCapabilityTest < ActiveSupport::TestCase def tokens(*toks) AccessToken.new do |token| toks.each do |t| @@ -12,10 +12,10 @@ class CapabilityTest < ActiveSupport::TestCase end end -class ChangesetCommentCapabilityTest < CapabilityTest +class ChangesetCommentApiCapabilityTest < ApiCapabilityTest test "as a normal user with permissionless token" do token = create(:access_token) - capability = Capability.new token + capability = ApiCapability.new token [:create, :destroy, :restore].each do |action| assert capability.cannot? action, ChangesetComment @@ -24,7 +24,7 @@ class ChangesetCommentCapabilityTest < CapabilityTest test "as a normal user with allow_write_api token" do token = create(:access_token, :allow_write_api => true) - capability = Capability.new token + capability = ApiCapability.new token [:destroy, :restore].each do |action| assert capability.cannot? action, ChangesetComment @@ -37,7 +37,7 @@ class ChangesetCommentCapabilityTest < CapabilityTest test "as a moderator with permissionless token" do token = create(:access_token, :user => create(:moderator_user)) - capability = Capability.new token + capability = ApiCapability.new token [:create, :destroy, :restore].each do |action| assert capability.cannot? action, ChangesetComment @@ -46,7 +46,7 @@ class ChangesetCommentCapabilityTest < CapabilityTest test "as a moderator with allow_write_api token" do token = create(:access_token, :user => create(:moderator_user), :allow_write_api => true) - capability = Capability.new token + capability = ApiCapability.new token [:create, :destroy, :restore].each do |action| assert capability.can? action, ChangesetComment @@ -54,10 +54,10 @@ class ChangesetCommentCapabilityTest < CapabilityTest end end -class NoteCapabilityTest < CapabilityTest +class NoteApiCapabilityTest < ApiCapabilityTest test "as a normal user with permissionless token" do token = create(:access_token) - capability = Capability.new token + capability = ApiCapability.new token [:create, :comment, :close, :reopen, :destroy].each do |action| assert capability.cannot? action, Note @@ -66,7 +66,7 @@ class NoteCapabilityTest < CapabilityTest test "as a normal user with allow_write_notes token" do token = create(:access_token, :allow_write_notes => true) - capability = Capability.new token + capability = ApiCapability.new token [:destroy].each do |action| assert capability.cannot? action, Note @@ -79,7 +79,7 @@ class NoteCapabilityTest < CapabilityTest test "as a moderator with permissionless token" do token = create(:access_token, :user => create(:moderator_user)) - capability = Capability.new token + capability = ApiCapability.new token [:destroy].each do |action| assert capability.cannot? action, Note @@ -88,7 +88,7 @@ class NoteCapabilityTest < CapabilityTest 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 + capability = ApiCapability.new token [:destroy].each do |action| assert capability.can? action, Note @@ -96,22 +96,22 @@ class NoteCapabilityTest < CapabilityTest end end -class UserCapabilityTest < CapabilityTest +class UserApiCapabilityTest < ApiCapabilityTest test "user preferences" do # a user with no tokens - capability = Capability.new nil + capability = ApiCapability.new nil [:read, :read_one, :update, :update_one, :delete_one].each do |act| assert capability.cannot? act, UserPreference end # A user with empty tokens - capability = Capability.new tokens + capability = ApiCapability.new tokens [:read, :read_one, :update, :update_one, :delete_one].each do |act| assert capability.cannot? act, UserPreference end - capability = Capability.new tokens(:allow_read_prefs) + capability = ApiCapability.new tokens(:allow_read_prefs) [:update, :update_one, :delete_one].each do |act| assert capability.cannot? act, UserPreference @@ -121,7 +121,7 @@ class UserCapabilityTest < CapabilityTest assert capability.can? act, UserPreference end - capability = Capability.new tokens(:allow_write_prefs) + capability = ApiCapability.new tokens(:allow_write_prefs) [:read, :read_one].each do |act| assert capability.cannot? act, UserPreference end