From 30fe6ab1219a4f0b3e83b098d962159835b0b102 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Wed, 26 Nov 2025 09:43:11 +0000 Subject: [PATCH 1/2] 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). --- app/controllers/accounts_controller.rb | 10 ++- app/controllers/application_controller.rb | 2 +- .../concerns/block_banned_requests.rb | 16 ++++ app/controllers/messages_controller.rb | 2 +- app/controllers/users/bans_controller.rb | 19 +++++ app/jobs/remove_banned_content_job.rb | 5 ++ app/models/ban.rb | 20 +++++ app/models/message/broadcasts.rb | 4 + app/models/user.rb | 11 +-- app/models/user/bannable.rb | 42 +++++++++ app/views/accounts/users/_user.html.erb | 4 +- app/views/users/_ban_button.html.erb | 13 +++ app/views/users/show.html.erb | 26 ++++-- config/routes.rb | 1 + db/migrate/20251126092013_create_bans.rb | 11 +++ ...115722_change_active_to_status_on_users.rb | 13 +++ db/structure.sql | 14 ++- .../concerns/block_banned_requests_test.rb | 32 +++++++ .../controllers/users/bans_controller_test.rb | 85 +++++++++++++++++++ 19 files changed, 306 insertions(+), 24 deletions(-) create mode 100644 app/controllers/concerns/block_banned_requests.rb create mode 100644 app/controllers/users/bans_controller.rb create mode 100644 app/jobs/remove_banned_content_job.rb create mode 100644 app/models/ban.rb create mode 100644 app/models/user/bannable.rb create mode 100644 app/views/users/_ban_button.html.erb create mode 100644 db/migrate/20251126092013_create_bans.rb create mode 100644 db/migrate/20251126115722_change_active_to_status_on_users.rb create mode 100644 test/controllers/concerns/block_banned_requests_test.rb create mode 100644 test/controllers/users/bans_controller_test.rb diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 0691d88..be05d66 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -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 diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6463f2d..5e60f1e 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -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 diff --git a/app/controllers/concerns/block_banned_requests.rb b/app/controllers/concerns/block_banned_requests.rb new file mode 100644 index 0000000..6485c5a --- /dev/null +++ b/app/controllers/concerns/block_banned_requests.rb @@ -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 diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb index d93a9c6..959b652 100644 --- a/app/controllers/messages_controller.rb +++ b/app/controllers/messages_controller.rb @@ -42,7 +42,7 @@ class MessagesController < ApplicationController def destroy @message.destroy - @message.broadcast_remove_to @room, :messages + @message.broadcast_remove end private diff --git a/app/controllers/users/bans_controller.rb b/app/controllers/users/bans_controller.rb new file mode 100644 index 0000000..5360f15 --- /dev/null +++ b/app/controllers/users/bans_controller.rb @@ -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 diff --git a/app/jobs/remove_banned_content_job.rb b/app/jobs/remove_banned_content_job.rb new file mode 100644 index 0000000..9fbc093 --- /dev/null +++ b/app/jobs/remove_banned_content_job.rb @@ -0,0 +1,5 @@ +class RemoveBannedContentJob < ApplicationJob + def perform(user) + user.remove_banned_content + end +end diff --git a/app/models/ban.rb b/app/models/ban.rb new file mode 100644 index 0000000..be5a936 --- /dev/null +++ b/app/models/ban.rb @@ -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 diff --git a/app/models/message/broadcasts.rb b/app/models/message/broadcasts.rb index 78ed381..1f909dc 100644 --- a/app/models/message/broadcasts.rb +++ b/app/models/message/broadcasts.rb @@ -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 diff --git a/app/models/user.rb b/app/models/user.rb index de5ba4c..20d05dd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -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 diff --git a/app/models/user/bannable.rb b/app/models/user/bannable.rb new file mode 100644 index 0000000..9d714ab --- /dev/null +++ b/app/models/user/bannable.rb @@ -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 diff --git a/app/views/accounts/users/_user.html.erb b/app/views/accounts/users/_user.html.erb index 21ebb0a..4f5f952 100644 --- a/app/views/accounts/users/_user.html.erb +++ b/app/views/accounts/users/_user.html.erb @@ -1,4 +1,4 @@ -
  • +
  • ">
    <%= avatar_tag user, loading: :lazy %>
    @@ -9,7 +9,7 @@ - <% 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 | %>
  • "> +
  • ">
    <%= avatar_tag user, loading: :lazy %>
    diff --git a/app/views/users/_ban_button.html.erb b/app/views/users/_ban_button.html.erb index 398b738..3cfab48 100644 --- a/app/views/users/_ban_button.html.erb +++ b/app/views/users/_ban_button.html.erb @@ -1,13 +1,15 @@ <% 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 + class: "btn full-width", + data: { turbo_confirm: "Are you sure you want to ban this user? This will log them out, delete their messages, and block their IP addresses." } do %> + <%= image_tag "cancel.svg", aria: { hidden: "true", label: "Ban #{@user.name}" } %> + Ban <%= user.name %> <% end %> <% else %> <%= button_to user_ban_path(user), method: :delete, - class: "btn full-width", + class: "btn btn--negative full-width", data: { turbo_confirm: "Are you sure you want to remove the ban on this user?" } do %> - Remove ban + <%= image_tag "cancel.svg", aria: { hidden: "true", label: "Ban #{@user.name}" } %> + Remove ban <% end %> <% end %> diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 94872e6..8fb4a28 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -16,7 +16,7 @@ <% end %>
    -
    +
    ">
    <%= image_tag fresh_user_avatar_path(@user), alt: "Profile avatar", class: "avatar" %>