diff --git a/app/controllers/messages/by_bots_controller.rb b/app/controllers/messages/by_bots_controller.rb index f225e51..503a6a7 100644 --- a/app/controllers/messages/by_bots_controller.rb +++ b/app/controllers/messages/by_bots_controller.rb @@ -1,9 +1,11 @@ class Messages::ByBotsController < MessagesController include RawRequestBody - allow_bot_access only: %i[ index create ] + allow_bot_access only: %i[ index create update destroy ] before_action :set_room + before_action :set_message, only: %i[ update destroy ] + before_action :ensure_can_administer, only: %i[ update destroy ] before_action :ensure_body_or_attachment_present, only: :create def index @@ -16,6 +18,18 @@ class Messages::ByBotsController < MessagesController head :created, location: message_url(@message) end + # ensure_can_administer still applies, and can_administer? only grants access to + # a record the user created, so a bot key reaches that bot's own messages and no others. + def update + update_message + head :ok + end + + def destroy + super + head :no_content + end + private def set_room @room = Current.user.rooms.find_by(id: params[:room_id]) diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb index 959b652..9388a00 100644 --- a/app/controllers/messages_controller.rb +++ b/app/controllers/messages_controller.rb @@ -34,9 +34,7 @@ class MessagesController < ApplicationController end def update - @message.update!(message_params) - - @message.broadcast_replace_to @room, :messages, target: [ @message, :presentation ], partial: "messages/presentation", attributes: { maintain_scroll: true } + update_message redirect_to room_message_url(@room, @message) end @@ -50,6 +48,13 @@ class MessagesController < ApplicationController @message = @room.messages.find(params[:id]) end + # Extracted so bots can reuse the update and its broadcast while answering with + # a status code instead of a redirect. + def update_message + @message.update!(message_params) + @message.broadcast_replace_to @room, :messages, target: [ @message, :presentation ], partial: "messages/presentation", attributes: { maintain_scroll: true } + end + def ensure_can_administer head :forbidden unless Current.user.can_administer?(@message) end diff --git a/config/routes.rb b/config/routes.rb index 03466a1..da01b4f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -64,7 +64,7 @@ Rails.application.routes.draw do nested do scope path: ":bot_key", as: :bot, defaults: { format: :json } do - resources :messages, controller: "messages/by_bots", only: %i[ index create ] do + resources :messages, controller: "messages/by_bots", only: %i[ index create update destroy ] do resources :boosts, controller: "messages/boosts/by_bots", only: :create end end diff --git a/test/controllers/messages/by_bots_controller_test.rb b/test/controllers/messages/by_bots_controller_test.rb index 0c145e7..d535656 100644 --- a/test/controllers/messages/by_bots_controller_test.rb +++ b/test/controllers/messages/by_bots_controller_test.rb @@ -148,4 +148,81 @@ class Messages::ByBotsControllerTest < ActionDispatch::IntegrationTest get room_messages_url(@room, bot_key: users(:bender).bot_key) assert_response :forbidden end + + test "update" do + message = post_bot_message "Deploying..." + + assert_no_difference -> { Message.count } do + patch room_bot_message_url(@room, users(:bender).bot_key, message), params: +"Deployed." + end + + assert_response :ok + assert_equal "Deployed.", message.reload.plain_text_body + end + + test "update with UTF-8 content" do + message = post_bot_message "Deploying..." + + patch room_bot_message_url(@room, users(:bender).bot_key, message), params: +"Deployed 🚀!" + + assert_response :ok + assert_equal "Deployed 🚀!", message.reload.plain_text_body + end + + test "update can't touch a message the bot did not create" do + message = messages(:fourth) + original = message.plain_text_body + + patch room_bot_message_url(@room, users(:bender).bot_key, message), params: +"Hijacked!" + + assert_response :forbidden + assert_equal original, message.reload.plain_text_body + end + + test "update is not found for a room the bot is not a member of" do + message = messages(:first) + original = message.plain_text_body + + patch room_bot_message_url(rooms(:designers), users(:bender).bot_key, message), params: +"Hijacked!" + + assert_response :not_found + assert_equal original, message.reload.plain_text_body + end + + test "update can't be abused to edit messages as any user" do + message = messages(:fourth) + bot_key = "#{users(:jz).id}-" + original = message.plain_text_body + + patch room_bot_message_url(@room, bot_key, message), params: +"Hijacked!" + + assert_response :redirect + assert_equal original, message.reload.plain_text_body + end + + test "destroy" do + message = post_bot_message "Deploying..." + + assert_difference -> { Message.count }, -1 do + delete room_bot_message_url(@room, users(:bender).bot_key, message) + end + + assert_response :no_content + end + + test "destroy can't touch a message the bot did not create" do + message = messages(:fourth) + + assert_no_difference -> { Message.count } do + delete room_bot_message_url(@room, users(:bender).bot_key, message) + end + + assert_response :forbidden + end + + private + def post_bot_message(body) + post room_bot_messages_url(@room, users(:bender).bot_key), params: +body + Message.last + end end