diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb index 4ed9708b40..cf84c24c50 100644 --- a/app/abilities/api_ability.rb +++ b/app/abilities/api_ability.rb @@ -37,10 +37,10 @@ def initialize(token) can [:inbox, :outbox, :read, :update, :destroy], Message if scope?(token, :consume_messages) can :create, Message if scope?(token, :send_messages) - if user.terms_agreed? - can [:create, :update, :upload, :close, :subscribe, :unsubscribe], Changeset if scope?(token, :write_api) - can :create, ChangesetComment if scope?(token, :write_api) - can [:create, :update, :delete], [Node, Way, Relation] if scope?(token, :write_api) + if user.terms_agreed? && scope?(token, :write_api) + can [:create, :update, :upload, :close, :subscribe, :unsubscribe], Changeset + can :create, ChangesetComment + can [:create, :update, :delete], [Node, Way, Relation] end if user.moderator? @@ -48,7 +48,9 @@ def initialize(token) can :destroy, Note if scope?(token, :write_notes) - can :redact, [OldNode, OldWay, OldRelation] if user&.terms_agreed? && scope?(token, :write_redactions) + can :redact, [OldNode, OldWay, OldRelation] if user.terms_agreed? && scope?(token, :write_redactions) + + can :create, UserBlock if scope?(token, :write_blocks) end end end diff --git a/app/controllers/api/user_blocks_controller.rb b/app/controllers/api/user_blocks_controller.rb index 51f0d26d3e..e1fb70a659 100644 --- a/app/controllers/api/user_blocks_controller.rb +++ b/app/controllers/api/user_blocks_controller.rb @@ -1,5 +1,8 @@ module Api class UserBlocksController < ApiController + before_action :check_api_writable, :only => :create + before_action :authorize, :only => :create + authorize_resource before_action :set_request_formats @@ -11,5 +14,33 @@ def show rescue ActiveRecord::RecordNotFound raise OSM::APINotFoundError end + + def create + raise OSM::APIBadUserInput, "No user was given" unless params[:user] + + user = User.visible.find_by(:id => params[:user]) + raise OSM::APINotFoundError unless user + raise OSM::APIBadUserInput, "No reason was given" unless params[:reason] + raise OSM::APIBadUserInput, "No period was given" unless params[:period] + + period = Integer(params[:period], :exception => false) + raise OSM::APIBadUserInput, "Period should be a number of hours" unless period + + max_period = UserBlock::PERIODS.max + raise OSM::APIBadUserInput, "Period must be between 0 and #{max_period}" if period.negative? || period > max_period + raise OSM::APIBadUserInput, "Needs_view must be true if provided" unless params[:needs_view].nil? || params[:needs_view] == "true" + + ends_at = Time.now.utc + period.hours + needs_view = params[:needs_view] == "true" + @user_block = UserBlock.create( + :user => user, + :creator => current_user, + :reason => params[:reason], + :ends_at => ends_at, + :deactivates_at => (ends_at unless needs_view), + :needs_view => needs_view + ) + render :show + end end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 7faf3bebcf..1c26a574f5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2663,6 +2663,7 @@ en: write_gpx: Upload GPS traces write_notes: Modify notes write_redactions: Redact map data + write_blocks: Create and revoke user blocks read_email: Read user email address consume_messages: Read, update status and delete user messages send_messages: Send private messages to other users diff --git a/config/routes.rb b/config/routes.rb index aae32527a4..2ce57404e4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -112,7 +112,7 @@ resource :subscription, :only => [:create, :destroy], :controller => "note_subscriptions" end - resources :user_blocks, :only => :show, :id => /\d+/, :controller => "user_blocks" + resources :user_blocks, :only => [:show, :create], :id => /\d+/, :controller => "user_blocks" end # Data browsing diff --git a/lib/oauth.rb b/lib/oauth.rb index 679c564a63..37393fbf07 100644 --- a/lib/oauth.rb +++ b/lib/oauth.rb @@ -1,11 +1,11 @@ module Oauth SCOPES = %w[ read_prefs write_prefs write_diary - write_api read_gpx write_gpx write_notes write_redactions + write_api read_gpx write_gpx write_notes write_redactions write_blocks consume_messages send_messages openid ].freeze PRIVILEGED_SCOPES = %w[read_email skip_authorization].freeze - MODERATOR_SCOPES = %w[write_redactions].freeze + MODERATOR_SCOPES = %w[write_redactions write_blocks].freeze class Scope attr_reader :name diff --git a/test/controllers/api/user_blocks_controller_test.rb b/test/controllers/api/user_blocks_controller_test.rb index 169338811d..2705e332d5 100644 --- a/test/controllers/api/user_blocks_controller_test.rb +++ b/test/controllers/api/user_blocks_controller_test.rb @@ -3,6 +3,10 @@ module Api class UserBlocksControllerTest < ActionDispatch::IntegrationTest def test_routes + assert_routing( + { :path => "/api/0.6/user_blocks", :method => :post }, + { :controller => "api/user_blocks", :action => "create" } + ) assert_routing( { :path => "/api/0.6/user_blocks/1", :method => :get }, { :controller => "api/user_blocks", :action => "show", :id => "1" } @@ -14,11 +18,22 @@ def test_routes end def test_show - block = create(:user_block) + blocked_user = create(:user) + creator_user = create(:moderator_user) + block = create(:user_block, :user => blocked_user, :creator => creator_user, :reason => "because running tests") get api_user_block_path(block) assert_response :success - assert_select "user_block[id='#{block.id}']", 1 + assert_select "osm>user_block", 1 do + assert_select ">@id", block.id.to_s + assert_select ">user", 1 + assert_select ">user>@uid", blocked_user.id.to_s + assert_select ">creator", 1 + assert_select ">creator>@uid", creator_user.id.to_s + assert_select ">revoker", 0 + assert_select ">reason", 1 + assert_select ">reason", "because running tests" + end get api_user_block_path(block, :format => "json") assert_response :success @@ -32,5 +47,165 @@ def test_show_not_found assert_response :not_found assert_equal "text/plain", @response.media_type end + + def test_create_no_permission + blocked_user = create(:user) + assert_empty blocked_user.blocks + + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => 1) + assert_response :unauthorized + assert_empty blocked_user.blocks + + regular_creator_user = create(:user) + auth_header = bearer_authorization_header(regular_creator_user, :scopes => %w[read_prefs]) + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => 1), :headers => auth_header + assert_response :forbidden + assert_empty blocked_user.blocks + + auth_header = bearer_authorization_header(regular_creator_user, :scopes => %w[read_prefs write_blocks]) + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => 1), :headers => auth_header + assert_response :forbidden + assert_empty blocked_user.blocks + + moderator_creator_user = create(:moderator_user) + auth_header = bearer_authorization_header(moderator_creator_user, :scopes => %w[read_prefs]) + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => 1), :headers => auth_header + assert_response :forbidden + assert_empty blocked_user.blocks + end + + def test_create_invalid_because_no_user + blocked_user = create(:user, :deleted) + assert_empty blocked_user.blocks + + creator_user = create(:moderator_user) + auth_header = bearer_authorization_header(creator_user, :scopes => %w[read_prefs write_blocks]) + post api_user_blocks_path(:reason => "because", :period => 1), :headers => auth_header + assert_response :bad_request + assert_equal "text/plain", @response.media_type + assert_equal "No user was given", @response.body + + assert_empty blocked_user.blocks + end + + def test_create_invalid_because_user_is_unknown + creator_user = create(:moderator_user) + auth_header = bearer_authorization_header(creator_user, :scopes => %w[read_prefs write_blocks]) + post api_user_blocks_path(:user => 0, :reason => "because", :period => 1), :headers => auth_header + assert_response :not_found + assert_equal "text/plain", @response.media_type + end + + def test_create_invalid_because_user_is_deleted + blocked_user = create(:user, :deleted) + assert_empty blocked_user.blocks + + creator_user = create(:moderator_user) + auth_header = bearer_authorization_header(creator_user, :scopes => %w[read_prefs write_blocks]) + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => 1), :headers => auth_header + assert_response :not_found + assert_equal "text/plain", @response.media_type + + assert_empty blocked_user.blocks + end + + def test_create_invalid_because_missing_reason + create_with_params_and_assert_bad_request("No reason was given", :period => "10") + end + + def test_create_invalid_because_missing_period + create_with_params_and_assert_bad_request("No period was given", :reason => "because") + end + + def test_create_invalid_because_non_numeric_period + create_with_params_and_assert_bad_request("Period should be a number of hours", :reason => "because", :period => "one hour") + end + + def test_create_invalid_because_negative_period + create_with_params_and_assert_bad_request("Period must be between 0 and #{UserBlock::PERIODS.max}", :reason => "go away", :period => "-1") + end + + def test_create_invalid_because_excessive_period + create_with_params_and_assert_bad_request("Period must be between 0 and #{UserBlock::PERIODS.max}", :reason => "go away", :period => "10000000") + end + + def test_create_invalid_because_unknown_needs_view + create_with_params_and_assert_bad_request("Needs_view must be true if provided", :reason => "because", :period => "1", :needs_view => "maybe") + end + + def test_create_success + blocked_user = create(:user) + creator_user = create(:moderator_user) + + assert_empty blocked_user.blocks + auth_header = bearer_authorization_header(creator_user, :scopes => %w[read_prefs write_blocks]) + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => 1), :headers => auth_header + assert_response :success + assert_equal 1, blocked_user.blocks.length + + block = blocked_user.blocks.take + assert_predicate block, :active? + assert_equal "because", block.reason + assert_equal creator_user, block.creator + + assert_equal "application/xml", @response.media_type + assert_select "osm>user_block", 1 do + assert_select ">@id", block.id.to_s + assert_select ">@needs_view", "false" + assert_select ">user", 1 + assert_select ">user>@uid", blocked_user.id.to_s + assert_select ">creator", 1 + assert_select ">creator>@uid", creator_user.id.to_s + assert_select ">revoker", 0 + assert_select ">reason", 1 + assert_select ">reason", "because" + end + end + + def test_create_success_with_needs_view + blocked_user = create(:user) + creator_user = create(:moderator_user) + + assert_empty blocked_user.blocks + auth_header = bearer_authorization_header(creator_user, :scopes => %w[read_prefs write_blocks]) + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => "1", :needs_view => "true"), :headers => auth_header + assert_response :success + assert_equal 1, blocked_user.blocks.length + + block = blocked_user.blocks.take + assert_predicate block, :active? + assert_equal "because", block.reason + assert_equal creator_user, block.creator + + assert_equal "application/xml", @response.media_type + assert_select "osm>user_block", 1 do + assert_select ">@id", block.id.to_s + assert_select ">@needs_view", "true" + assert_select ">user", 1 + assert_select ">user>@uid", blocked_user.id.to_s + assert_select ">creator", 1 + assert_select ">creator>@uid", creator_user.id.to_s + assert_select ">revoker", 0 + assert_select ">reason", 1 + assert_select ">reason", "because" + end + end + + private + + def create_with_params_and_assert_bad_request(message, **params) + blocked_user = create(:user) + assert_empty blocked_user.blocks + + moderator_creator_user = create(:moderator_user) + auth_header = bearer_authorization_header(moderator_creator_user, :scopes => %w[read_prefs write_blocks]) + + post api_user_blocks_path({ :user => blocked_user.id }.merge(params)), :headers => auth_header + assert_response :bad_request + assert_equal "text/plain", @response.media_type + assert_equal message, @response.body + + assert_empty blocked_user.blocks + end end end