Skip to content
15 changes: 14 additions & 1 deletion app/controllers/concerns/authentication.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,23 @@ def require_authentication
restore_authentication || bot_authentication || request_authentication
end

def require_bot_authentication
bot_key = request.authorization.nil? ? request.path_parameters[:bot_key] : bearer_bot_key
authenticate_bot_key(bot_key) || head(:unauthorized)
end

def restore_authentication
if session = find_session_by_cookie
resume_session session
end
end

def bot_authentication
if params[:bot_key].present? && bot = User.authenticate_bot(params[:bot_key].strip)
authenticate_bot_key(request.path_parameters[:bot_key])
end

def authenticate_bot_key(bot_key)
if bot_key.present? && bot = User.authenticate_bot(bot_key.strip)
Current.user = bot
set_authenticated_by(:bot_key)
end
Expand Down Expand Up @@ -98,6 +107,10 @@ def remove_authentication_cookie
cookies.delete(:session_token)
end

def bearer_bot_key
authenticate_with_http_token(scheme: :bearer) { |token, _options| token }
end

def deny_bots
head :forbidden if authenticated_by.bot_key?
end
Expand Down
3 changes: 3 additions & 0 deletions app/controllers/messages/boosts/by_bots_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
class Messages::Boosts::ByBotsController < Messages::BoostsController
include RawRequestBody

skip_before_action :require_authentication
prepend_before_action :require_bot_authentication

allow_bot_access only: %i[ create destroy ]

before_action :ensure_content_present, only: :create
Expand Down
14 changes: 13 additions & 1 deletion app/controllers/messages/by_bots_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
class Messages::ByBotsController < MessagesController
include RawRequestBody

skip_before_action :require_authentication
prepend_before_action :require_bot_authentication

allow_bot_access only: %i[ index create update destroy ]

before_action :set_room
Expand Down Expand Up @@ -40,7 +43,16 @@ def set_pagination_headers
headers["X-Total-Count"] = @room.messages_count.to_s

if next_page = next_page_params
headers["Link"] = %(<#{room_bot_messages_url(@room, params[:bot_key], **next_page)}>; rel="next")
headers["Link"] = %(<#{next_page_url(next_page)}>; rel="next")
end
end

# Keep the key out of the link when it arrived in a header.
def next_page_url(page)
if bot_key = request.path_parameters[:bot_key]
room_bot_messages_url(@room, bot_key, **page)
else
room_bot_api_messages_url(@room, **page)
end
end

Expand Down
12 changes: 11 additions & 1 deletion app/models/webhook.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,13 @@ def uri
def payload(message)
{
user: { id: message.creator.id, name: message.creator.name },
room: { id: message.room.id, name: message.room.name, path: room_bot_messages_path(message) },
room: {
id: message.room.id,
name: message.room.name,
path: room_bot_messages_path(message),
api_path: room_bot_api_messages_path(message),
bot_key: user.bot_key
},
message: { id: message.id, body: { html: message.body.body, plain: without_recipient_mentions(message.plain_text_body) }, path: message_path(message) }
}.to_json
end
Expand All @@ -54,6 +60,10 @@ def room_bot_messages_path(message)
Rails.application.routes.url_helpers.room_bot_messages_path(message.room, user.bot_key)
end

def room_bot_api_messages_path(message)
Rails.application.routes.url_helpers.room_bot_api_messages_path(message.room)
end

def extract_text_from(response)
String.new(response.body).force_encoding("UTF-8") if response.code == "200" && response.content_type.in?(%w[ text/html text/plain ])
end
Expand Down
4 changes: 2 additions & 2 deletions app/views/accounts/bots/_bot.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
<strong class="overflow-ellipsis"><%= room_display_name(room) %></strong>
</legend>

<% curl_text_line = "curl -d 'Hello!' #{room_bot_messages_url(room, bot.bot_key)}" %>
<% curl_text_line = "curl -H 'Authorization: Bearer #{bot.bot_key}' -d 'Hello!' #{room_bot_api_messages_url(room)}" %>
<div class="flex align-center gap">
<%= image_tag "messages-outlined.svg", aria: { hidden: "true" }, size: 24, class: "colorize--black" %>

Expand All @@ -36,7 +36,7 @@
</div>
</div>

<% curl_upload_line = %[curl -F "attachment=@/path/to/file" #{room_bot_messages_url(room, bot.bot_key)}] %>
<% curl_upload_line = %[curl -H "Authorization: Bearer #{bot.bot_key}" -F "attachment=@/path/to/file" #{room_bot_api_messages_url(room)}] %>
<div class="flex align-center gap">
<%= image_tag "attachment.svg", aria: { hidden: "true" }, size: 24, class: "colorize--black" %>

Expand Down
7 changes: 7 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@
resources :messages

nested do
# Literal "bot" must stay above :bot_key so it is not captured as a key.
scope path: "bot", as: :bot_api, defaults: { format: :json } do
resources :messages, controller: "messages/by_bots", only: %i[ index create update destroy ] do
resources :boosts, controller: "messages/boosts/by_bots", only: %i[ create destroy ]
end
end

scope path: ":bot_key", as: :bot, defaults: { format: :json } do
resources :messages, controller: "messages/by_bots", only: %i[ index create update destroy ] do
resources :boosts, controller: "messages/boosts/by_bots", only: %i[ create destroy ]
Expand Down
25 changes: 25 additions & 0 deletions docs/self-hosting.md
Original file line number Diff line number Diff line change
Expand Up @@ -211,3 +211,28 @@ docker run --rm \
```

Then start Campfire again.

### Bot API credentials and logs

Bots should authenticate with `Authorization: Bearer <bot-key>` when calling the
key-free `/rooms/:id/bot/...` endpoints. For example:

```sh
curl -H 'Authorization: Bearer <bot-key>' -d 'Hello!' "$CAMPFIRE_URL/rooms/$ROOM_ID/bot/messages"
```

Bearer authentication is preferred and keeps the key out of the URL. To migrate a
bot, use the key-free `/rooms/:id/bot/...` URL and send its key in the Authorization
header. Keep sending that header when following pagination links. If no Authorization
header is sent, `/rooms/:id/:bot_key/...` paths remain supported as a fallback; an
empty, unsupported, or invalid Authorization header fails with `401` and does not
fall back to the path key. Query-string and request-body `bot_key` credentials are
not accepted. The bots page copies Bearer-authenticated curl commands. Webhooks
retain `room.path` for legacy clients and include `room.api_path` + `room.bot_key`
for clients to call the key-free path with the Authorization header. A browser
session alone does not authenticate bot API requests.

Thruster logs raw request paths, so legacy path credentials may be present in
container logs. Moving a key to the Authorization header does not remove copies
already written to logs. If a key may have been exposed there, generate a new key
from the bot's edit page and update every client using that bot.
7 changes: 3 additions & 4 deletions lib/rails_ext/log_scrubbing_formatter.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Bot requests carry the bot key as a URL path segment (/rooms/:room_id/:bot_key/...).
# config.filter_parameters redacts query and form parameters but never path segments,
# so the key would otherwise be written verbatim to the request log. Redact it wherever
# it appears in a formatted log line.
# Legacy bot requests carry the key in the path (/rooms/:room_id/:bot_key/...).
# Prefer /rooms/:id/bot/... with Authorization: Bearer to keep keys out of paths.
# config.filter_parameters covers query/form params but never path segments.
class LogScrubbingFormatter < ::Logger::Formatter
BOT_KEY_IN_PATH = %r{(/rooms/\d+/)\d+-[A-Za-z0-9]+}

Expand Down
1 change: 1 addition & 0 deletions test/controllers/accounts/bots_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class Accounts::BotsControllerTest < ActionDispatch::IntegrationTest
test "index" do
get account_bots_url
assert_response :ok
assert_includes response.body, "Authorization: Bearer #{users(:bender).bot_key}"
end

test "create" do
Expand Down
39 changes: 36 additions & 3 deletions test/controllers/messages/boosts/by_bots_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class Messages::Boosts::ByBotsControllerTest < ActionDispatch::IntegrationTest
assert_no_difference -> { Boost.count } do
post room_bot_message_boosts_url(@room, "invalid-bot-key", @message), params: +"👀"
end
assert_response :redirect
assert_response :unauthorized
end

test "create is not found for a room the bot is not a member of" do
Expand All @@ -77,7 +77,7 @@ class Messages::Boosts::ByBotsControllerTest < ActionDispatch::IntegrationTest
assert_no_difference -> { Boost.count } do
post room_bot_message_boosts_url(@room, bot_key, @message), params: +"👀"
end
assert_response :redirect
assert_response :unauthorized
end

test "destroy removes the bot's own boost" do
Expand Down Expand Up @@ -107,6 +107,39 @@ class Messages::Boosts::ByBotsControllerTest < ActionDispatch::IntegrationTest
delete room_bot_message_boost_url(@room, "invalid-bot-key", @message, boosts(:fourth_by_bender))
end

assert_response :redirect
assert_response :unauthorized
end

test "create accepts a Bearer token on a path that does not contain the key" do
assert_difference -> { @message.boosts.count }, +1 do
post room_bot_api_message_boosts_url(@room, @message), params: +"🙌", headers: { "Authorization" => "Bearer #{@bot.bot_key}" }
end
assert_response :created
end

test "create does not fall back to the legacy path key for a blank Authorization header" do
assert_no_difference -> { @message.boosts.count } do
post room_bot_message_boosts_url(@room, @bot.bot_key, @message), params: +"🙌", headers: { "Authorization" => " " }
end
assert_response :unauthorized
end

test "create authenticates as the bot with a Bearer token even when a browser session exists" do
sign_in :david

assert_difference -> { @message.boosts.count }, +1 do
post room_bot_api_message_boosts_url(@room, @message), params: +"🙌", headers: { "Authorization" => "Bearer #{@bot.bot_key}" }
end
assert_response :created
assert_equal @bot, @message.boosts.last.booster
end

test "create does not authenticate from a signed-in session alone" do
sign_in :david

assert_no_difference -> { @message.boosts.count } do
post room_bot_api_message_boosts_url(@room, @message), params: +"🙌"
end
assert_response :unauthorized
end
end
Loading
Loading