mirror of
https://github.com/basecamp/once-campfire.git
synced 2026-05-25 11:38:43 +09:00
Add IP-based user banning
This adds the ability to ban a user by their IP address. When an admin is viewing a user profile, a new "Ban user" button is present. Clicking on that will: - Create a ban on the IP addresses that were tracked for that user's sessions - Remove all the messages authored by that user - Log the user out immediately In addition, that user will no longer be shown in most user lists in the app. They are still shown to admins, in account settings. Viewing their profile from there will now show a "Remove ban" button which can be used to restore their access (it doesn't restore their messages though -- those are already gone -- it just removes the blocks so they can log in again).
This commit is contained in:
@@ -3,7 +3,7 @@ class AccountsController < ApplicationController
|
||||
before_action :set_account
|
||||
|
||||
def edit
|
||||
set_page_and_extract_portion_from User.active.ordered, per_page: 500
|
||||
set_page_and_extract_portion_from account_users.ordered, per_page: 500
|
||||
end
|
||||
|
||||
def update
|
||||
@@ -19,4 +19,12 @@ class AccountsController < ApplicationController
|
||||
def account_params
|
||||
params.require(:account).permit(:name, :logo)
|
||||
end
|
||||
|
||||
def account_users
|
||||
if Current.user.can_administer?
|
||||
User.where(status: [ :active, :banned ])
|
||||
else
|
||||
User.active
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
class ApplicationController < ActionController::Base
|
||||
include AllowBrowser, Authentication, Authorization, SetCurrentRequest, SetPlatform, TrackedRoomVisit, VersionHeaders
|
||||
include AllowBrowser, Authentication, Authorization, BlockBannedRequests, SetCurrentRequest, SetPlatform, TrackedRoomVisit, VersionHeaders
|
||||
include Turbo::Streams::Broadcasts, Turbo::Streams::StreamName
|
||||
end
|
||||
|
||||
16
app/controllers/concerns/block_banned_requests.rb
Normal file
16
app/controllers/concerns/block_banned_requests.rb
Normal file
@@ -0,0 +1,16 @@
|
||||
module BlockBannedRequests
|
||||
extend ActiveSupport::Concern
|
||||
|
||||
included do
|
||||
before_action :reject_banned_ip, unless: :safe_request?
|
||||
end
|
||||
|
||||
private
|
||||
def reject_banned_ip
|
||||
head :too_many_requests if Ban.banned?(request.remote_ip)
|
||||
end
|
||||
|
||||
def safe_request?
|
||||
request.get? || request.head?
|
||||
end
|
||||
end
|
||||
@@ -42,7 +42,7 @@ class MessagesController < ApplicationController
|
||||
|
||||
def destroy
|
||||
@message.destroy
|
||||
@message.broadcast_remove_to @room, :messages
|
||||
@message.broadcast_remove
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
19
app/controllers/users/bans_controller.rb
Normal file
19
app/controllers/users/bans_controller.rb
Normal file
@@ -0,0 +1,19 @@
|
||||
class Users::BansController < ApplicationController
|
||||
before_action :ensure_can_administer
|
||||
before_action :set_user
|
||||
|
||||
def create
|
||||
@user.ban
|
||||
redirect_to @user
|
||||
end
|
||||
|
||||
def destroy
|
||||
@user.unban
|
||||
redirect_to @user
|
||||
end
|
||||
|
||||
private
|
||||
def set_user
|
||||
@user = User.find(params[:user_id])
|
||||
end
|
||||
end
|
||||
5
app/jobs/remove_banned_content_job.rb
Normal file
5
app/jobs/remove_banned_content_job.rb
Normal file
@@ -0,0 +1,5 @@
|
||||
class RemoveBannedContentJob < ApplicationJob
|
||||
def perform(user)
|
||||
user.remove_banned_content
|
||||
end
|
||||
end
|
||||
20
app/models/ban.rb
Normal file
20
app/models/ban.rb
Normal file
@@ -0,0 +1,20 @@
|
||||
class Ban < ApplicationRecord
|
||||
belongs_to :user
|
||||
|
||||
validate :ip_address_is_public
|
||||
|
||||
def self.banned?(ip_address)
|
||||
exists?(ip_address: ip_address)
|
||||
end
|
||||
|
||||
private
|
||||
def ip_address_is_public
|
||||
ip = IPAddr.new(ip_address)
|
||||
|
||||
if ip.loopback? || ip.private? || ip.link_local?
|
||||
errors.add(:ip_address, "cannot be a private or internal IP address")
|
||||
end
|
||||
rescue IPAddr::InvalidAddressError
|
||||
errors.add(:ip_address, "is not a valid IP address")
|
||||
end
|
||||
end
|
||||
@@ -3,4 +3,8 @@ module Message::Broadcasts
|
||||
broadcast_append_to room, :messages, target: [ room, :messages ]
|
||||
ActionCable.server.broadcast("unread_rooms", { roomId: room.id })
|
||||
end
|
||||
|
||||
def broadcast_remove
|
||||
broadcast_remove_to room, :messages
|
||||
end
|
||||
end
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
class User < ApplicationRecord
|
||||
include Avatar, Bot, Mentionable, Role, Transferable
|
||||
include Avatar, Bannable, Bot, Mentionable, Role, Transferable
|
||||
|
||||
has_many :memberships, dependent: :delete_all
|
||||
has_many :rooms, through: :memberships
|
||||
@@ -13,8 +13,9 @@ class User < ApplicationRecord
|
||||
has_many :searches, dependent: :delete_all
|
||||
|
||||
has_many :sessions, dependent: :destroy
|
||||
has_many :bans, dependent: :destroy
|
||||
|
||||
scope :active, -> { where(active: true) }
|
||||
enum :status, %i[ active deactivated banned ], default: :active
|
||||
|
||||
has_secure_password validations: false
|
||||
|
||||
@@ -40,14 +41,10 @@ class User < ApplicationRecord
|
||||
searches.delete_all
|
||||
sessions.delete_all
|
||||
|
||||
update! active: false, email_address: deactived_email_address
|
||||
update! status: :deactivated, email_address: deactived_email_address
|
||||
end
|
||||
end
|
||||
|
||||
def deactivated?
|
||||
!active?
|
||||
end
|
||||
|
||||
def reset_remote_connections
|
||||
close_remote_connections reconnect: true
|
||||
end
|
||||
|
||||
42
app/models/user/bannable.rb
Normal file
42
app/models/user/bannable.rb
Normal file
@@ -0,0 +1,42 @@
|
||||
module User::Bannable
|
||||
extend ActiveSupport::Concern
|
||||
|
||||
def ban
|
||||
transaction do
|
||||
create_bans_from_sessions
|
||||
apply_ban
|
||||
banned!
|
||||
end
|
||||
end
|
||||
|
||||
def unban
|
||||
transaction do
|
||||
bans.delete_all
|
||||
active!
|
||||
end
|
||||
end
|
||||
|
||||
def remove_banned_content_later
|
||||
RemoveBannedContentJob.perform_later(self)
|
||||
end
|
||||
|
||||
def remove_banned_content
|
||||
messages.each do |message|
|
||||
message.destroy
|
||||
message.broadcast_remove
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
def create_bans_from_sessions
|
||||
sessions.pluck(:ip_address).compact_blank.uniq.each do |ip|
|
||||
bans.create!(ip_address: ip)
|
||||
end
|
||||
end
|
||||
|
||||
def apply_ban
|
||||
close_remote_connections
|
||||
sessions.delete_all
|
||||
remove_banned_content_later
|
||||
end
|
||||
end
|
||||
@@ -1,4 +1,4 @@
|
||||
<li class="flex align-center gap margin-none">
|
||||
<li class="flex align-center gap margin-none <%= "translucent" if user.banned? %>">
|
||||
<figure class="avatar flex-item-no-shrink" style="--avatar-size: 3.75ch;">
|
||||
<%= avatar_tag user, loading: :lazy %>
|
||||
</figure>
|
||||
@@ -9,7 +9,7 @@
|
||||
|
||||
<hr class="separator" aria-hidden="true">
|
||||
|
||||
<% if Current.user.can_administer? && user != Current.user %>
|
||||
<% if Current.user.can_administer? && user != Current.user && user.active? %>
|
||||
<% unless user.bot? %>
|
||||
<%= form_with model: user, url: account_user_path(user), data: { controller: "form" }, method: :patch do | form | %>
|
||||
<label class="btn txt-small flex-item-no-shrink" for="<%= dom_id(user, :role) %>">
|
||||
|
||||
13
app/views/users/_ban_button.html.erb
Normal file
13
app/views/users/_ban_button.html.erb
Normal file
@@ -0,0 +1,13 @@
|
||||
<% if user.active? %>
|
||||
<%= button_to user_ban_path(user), method: :post,
|
||||
class: "btn btn--negative full-width",
|
||||
data: { turbo_confirm: "Are you sure you want to ban this user?" } do %>
|
||||
Ban user
|
||||
<% end %>
|
||||
<% else %>
|
||||
<%= button_to user_ban_path(user), method: :delete,
|
||||
class: "btn full-width",
|
||||
data: { turbo_confirm: "Are you sure you want to remove the ban on this user?" } do %>
|
||||
Remove ban
|
||||
<% end %>
|
||||
<% end %>
|
||||
@@ -30,7 +30,7 @@
|
||||
<% end %>
|
||||
</div>
|
||||
<% else %>
|
||||
<% if @user.active? %>
|
||||
<% unless @user.deactivated? %>
|
||||
<div class="flex flex-column gap" style="--row-gap: calc(var(--block-space) / 3)">
|
||||
<h1 class="txt-x-large txt-tight-lines margin-none"><%= @user.name %></h1>
|
||||
<% if Current.user.can_administer? %>
|
||||
@@ -39,16 +39,24 @@
|
||||
<div><%= @user.bio %></div>
|
||||
</div>
|
||||
|
||||
<div class="pad-inline-double margin-inline margin-block-start">
|
||||
<%= button_to rooms_directs_path(user_ids: [ @user.id ]), class: "btn btn--reversed full-width txt-large" do %>
|
||||
<%= image_tag "messages.svg", aria: { hidden: "true", label: "Ping #{@user.name}" } %>
|
||||
<% if @user.active? %>
|
||||
<div class="pad-inline-double margin-inline margin-block-start">
|
||||
<%= button_to rooms_directs_path(user_ids: [ @user.id ]), class: "btn btn--reversed full-width txt-large" do %>
|
||||
<%= image_tag "messages.svg", aria: { hidden: "true", label: "Ping #{@user.name}" } %>
|
||||
<% end %>
|
||||
</div>
|
||||
|
||||
<% if Current.user.can_administer? %>
|
||||
<hr class="margin-block-start borderless">
|
||||
|
||||
<%= render "users/profiles/transfer", user: @user %>
|
||||
<% end %>
|
||||
</div>
|
||||
<% end %>
|
||||
|
||||
<% if Current.user.can_administer? %>
|
||||
<hr class="margin-block-start borderless">
|
||||
|
||||
<%= render "users/profiles/transfer", user: @user %>
|
||||
<% if Current.user.can_administer? && Current.user != @user %>
|
||||
<div class="margin-block-start">
|
||||
<%= render "users/ban_button", user: @user %>
|
||||
</div>
|
||||
<% end %>
|
||||
<% else %>
|
||||
<div>
|
||||
|
||||
@@ -37,6 +37,7 @@ Rails.application.routes.draw do
|
||||
resources :users, only: :show do
|
||||
scope module: "users" do
|
||||
resource :avatar, only: %i[ show destroy ]
|
||||
resource :ban, only: %i[ create destroy ]
|
||||
|
||||
scope defaults: { user_id: "me" } do
|
||||
resource :sidebar, only: :show
|
||||
|
||||
11
db/migrate/20251126092013_create_bans.rb
Normal file
11
db/migrate/20251126092013_create_bans.rb
Normal file
@@ -0,0 +1,11 @@
|
||||
class CreateBans < ActiveRecord::Migration[8.1]
|
||||
def change
|
||||
create_table :bans do |t|
|
||||
t.references :user, null: false, foreign_key: true
|
||||
t.string :ip_address, null: false
|
||||
t.timestamps
|
||||
end
|
||||
|
||||
add_index :bans, :ip_address
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,13 @@
|
||||
class ChangeActiveToStatusOnUsers < ActiveRecord::Migration[8.1]
|
||||
def change
|
||||
add_column :users, :status, :integer, default: 0, null: false
|
||||
|
||||
reversible do |dir|
|
||||
dir.up do
|
||||
execute "UPDATE users SET status = 1 WHERE active = 0"
|
||||
end
|
||||
end
|
||||
|
||||
remove_column :users, :active, :boolean
|
||||
end
|
||||
end
|
||||
@@ -64,12 +64,20 @@ FOREIGN KEY ("user_id")
|
||||
REFERENCES "users" ("id")
|
||||
);
|
||||
CREATE INDEX "index_webhooks_on_user_id" ON "webhooks" ("user_id");
|
||||
CREATE TABLE IF NOT EXISTS "users" ("id" integer PRIMARY KEY AUTOINCREMENT NOT NULL, "name" varchar NOT NULL, "created_at" datetime(6) NOT NULL, "updated_at" datetime(6) NOT NULL, "role" integer DEFAULT 0 NOT NULL, "email_address" varchar DEFAULT NULL, "password_digest" varchar DEFAULT NULL, "active" boolean DEFAULT 1, "bio" text DEFAULT NULL, "bot_token" varchar DEFAULT NULL);
|
||||
CREATE UNIQUE INDEX "index_users_on_email_address" ON "users" ("email_address");
|
||||
CREATE UNIQUE INDEX "index_users_on_bot_token" ON "users" ("bot_token");
|
||||
CREATE TABLE IF NOT EXISTS "active_storage_blobs" ("id" integer PRIMARY KEY AUTOINCREMENT NOT NULL, "key" varchar NOT NULL, "filename" varchar NOT NULL, "content_type" varchar, "metadata" text, "service_name" varchar NOT NULL, "byte_size" bigint NOT NULL, "checksum" varchar, "created_at" datetime(6) NOT NULL);
|
||||
CREATE UNIQUE INDEX "index_active_storage_blobs_on_key" ON "active_storage_blobs" ("key") /*application='Campfire'*/;
|
||||
CREATE TABLE IF NOT EXISTS "bans" ("id" integer PRIMARY KEY AUTOINCREMENT NOT NULL, "user_id" integer NOT NULL, "ip_address" varchar NOT NULL, "created_at" datetime(6) NOT NULL, "updated_at" datetime(6) NOT NULL, CONSTRAINT "fk_rails_070022cd76"
|
||||
FOREIGN KEY ("user_id")
|
||||
REFERENCES "users" ("id")
|
||||
);
|
||||
CREATE INDEX "index_bans_on_user_id" ON "bans" ("user_id") /*application='Campfire'*/;
|
||||
CREATE INDEX "index_bans_on_ip_address" ON "bans" ("ip_address") /*application='Campfire'*/;
|
||||
CREATE TABLE IF NOT EXISTS "users" ("id" integer PRIMARY KEY AUTOINCREMENT NOT NULL, "name" varchar NOT NULL, "created_at" datetime(6) NOT NULL, "updated_at" datetime(6) NOT NULL, "role" integer DEFAULT 0 NOT NULL, "email_address" varchar, "password_digest" varchar, "bio" text, "bot_token" varchar, "status" integer DEFAULT 0 NOT NULL);
|
||||
CREATE UNIQUE INDEX "index_users_on_email_address" ON "users" ("email_address") /*application='Campfire'*/;
|
||||
CREATE UNIQUE INDEX "index_users_on_bot_token" ON "users" ("bot_token") /*application='Campfire'*/;
|
||||
INSERT INTO "schema_migrations" (version) VALUES
|
||||
('20251126115722'),
|
||||
('20251126092013'),
|
||||
('20250825100959'),
|
||||
('20250825100958'),
|
||||
('20250825100957'),
|
||||
|
||||
32
test/controllers/concerns/block_banned_requests_test.rb
Normal file
32
test/controllers/concerns/block_banned_requests_test.rb
Normal file
@@ -0,0 +1,32 @@
|
||||
require "test_helper"
|
||||
|
||||
class BlockBannedRequestsTest < ActionDispatch::IntegrationTest
|
||||
setup do
|
||||
sign_in :david
|
||||
@room = rooms(:watercooler)
|
||||
|
||||
Ban.create!(user: users(:kevin), ip_address: "203.0.113.1")
|
||||
end
|
||||
|
||||
test "POST requests from banned IPs are blocked with 429" do
|
||||
post room_messages_url(@room),
|
||||
params: { message: { body: "Test", client_message_id: "test-123" } },
|
||||
headers: { "REMOTE_ADDR" => "203.0.113.1" }
|
||||
|
||||
assert_response :too_many_requests
|
||||
end
|
||||
|
||||
test "POST requests from non-banned IPs are allowed" do
|
||||
post room_messages_url(@room, format: :turbo_stream),
|
||||
params: { message: { body: "Test", client_message_id: "test-123" } },
|
||||
headers: { "REMOTE_ADDR" => "203.0.113.99" }
|
||||
|
||||
assert_response :success
|
||||
end
|
||||
|
||||
test "GET requests from banned IPs are allowed" do
|
||||
get room_messages_url(@room), headers: { "REMOTE_ADDR" => "203.0.113.1" }
|
||||
|
||||
assert_response :success
|
||||
end
|
||||
end
|
||||
85
test/controllers/users/bans_controller_test.rb
Normal file
85
test/controllers/users/bans_controller_test.rb
Normal file
@@ -0,0 +1,85 @@
|
||||
require "test_helper"
|
||||
|
||||
class Users::BansControllerTest < ActionDispatch::IntegrationTest
|
||||
setup do
|
||||
sign_in :david
|
||||
end
|
||||
|
||||
test "create bans user and creates ban records from sessions" do
|
||||
user = users(:kevin)
|
||||
user.sessions.create!(ip_address: "203.0.113.1", user_agent: "Test")
|
||||
user.sessions.create!(ip_address: "203.0.113.2", user_agent: "Test")
|
||||
|
||||
assert_difference -> { Ban.count }, 2 do
|
||||
post user_ban_url(user)
|
||||
end
|
||||
|
||||
assert_redirected_to user_url(user)
|
||||
assert Ban.exists?(ip_address: "203.0.113.1", user: user)
|
||||
assert Ban.exists?(ip_address: "203.0.113.2", user: user)
|
||||
end
|
||||
|
||||
test "create destroys user sessions" do
|
||||
user = users(:kevin)
|
||||
user.sessions.create!(ip_address: "203.0.113.1", user_agent: "Test")
|
||||
|
||||
assert_difference -> { user.sessions.count }, -1 do
|
||||
post user_ban_url(user)
|
||||
end
|
||||
end
|
||||
|
||||
test "create enqueues RemoveBannedContentJob" do
|
||||
user = users(:kevin)
|
||||
|
||||
assert_enqueued_with(job: RemoveBannedContentJob, args: [ user ]) do
|
||||
post user_ban_url(user)
|
||||
end
|
||||
end
|
||||
|
||||
test "RemoveBannedContentJob deletes messages" do
|
||||
user = users(:kevin)
|
||||
user.sessions.create!(ip_address: "203.0.113.1", user_agent: "Test")
|
||||
user.messages.create!(room: rooms(:hq), body: "Test message", client_message_id: "test-123")
|
||||
|
||||
perform_enqueued_jobs do
|
||||
post user_ban_url(user)
|
||||
end
|
||||
|
||||
assert_empty user.reload.messages
|
||||
end
|
||||
|
||||
test "non-admins cannot ban users" do
|
||||
sign_in :kevin
|
||||
|
||||
post user_ban_url(users(:jz))
|
||||
|
||||
assert_response :forbidden
|
||||
end
|
||||
|
||||
test "destroy removes ban records and sets user to active" do
|
||||
user = users(:kevin)
|
||||
user.sessions.create!(ip_address: "203.0.113.1", user_agent: "Test")
|
||||
user.ban
|
||||
|
||||
assert user.reload.banned?
|
||||
assert_equal 1, user.bans.count
|
||||
|
||||
assert_difference -> { Ban.count }, -1 do
|
||||
delete user_ban_url(user)
|
||||
end
|
||||
|
||||
assert_redirected_to user_url(user)
|
||||
assert user.reload.active?
|
||||
end
|
||||
|
||||
test "non-admins cannot unban users" do
|
||||
sign_in :kevin
|
||||
|
||||
user = users(:jz)
|
||||
user.banned!
|
||||
|
||||
delete user_ban_url(user)
|
||||
|
||||
assert_response :forbidden
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user