1 Commits

Author SHA1 Message Date
0daf413b47 Prevent sole admin from deleting their account
Without this guard, the last admin in the system could delete their own
account, making the application unmanageable. This adds a model method
`User#sole_admin?`, a controller guard in `RegistrationsController#destroy`,
and disables the delete button in the profile edit view when the current
user is the only remaining admin.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-01 06:52:14 +00:00
13 changed files with 79 additions and 167 deletions

View File

@@ -1,74 +0,0 @@
name: Tests
on:
push:
branches: [ master ]
pull_request:
branches: [ master ]
jobs:
test-sqlite:
name: Tests (SQLite)
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: ruby/setup-ruby@v1
with:
ruby-version: '3.3'
bundler-cache: true
env:
BUNDLE_WITH: "sqlite:development:test"
- name: Set up test database
run: bin/rails db:create db:schema:load
env:
RAILS_ENV: test
- name: Run tests
run: bin/rails test
env:
RAILS_ENV: test
CI: "true"
test-mysql:
name: Tests (MySQL)
runs-on: ubuntu-latest
services:
mysql:
image: mysql:8.0
env:
MYSQL_ROOT_PASSWORD: ""
MYSQL_ALLOW_EMPTY_PASSWORD: "yes"
MYSQL_DATABASE: fixin_test
ports:
- 3306:3306
options: >-
--health-cmd="mysqladmin ping"
--health-interval=10s
--health-timeout=5s
--health-retries=3
steps:
- uses: actions/checkout@v4
- uses: ruby/setup-ruby@v1
with:
ruby-version: '3.3'
bundler-cache: true
env:
BUNDLE_WITH: "mysql:development:test"
- name: Set up test database
run: bin/rails db:schema:load
env:
RAILS_ENV: test
DB_ADAPTER: mysql
- name: Run tests
run: bin/rails test
env:
RAILS_ENV: test
CI: "true"
DB_ADAPTER: mysql

View File

@@ -2,7 +2,11 @@ class RegistrationsController < Devise::RegistrationsController
before_action :authenticate_user!, only: [:edit, :update, :destroy]
def destroy
# TODO: Disallow/disable deletion for last admin account; update :edit view
if current_user.sole_admin?
redirect_back fallback_location: edit_user_registration_path,
alert: t(".sole_admin")
return
end
super
end

View File

@@ -26,8 +26,10 @@ class Unit < ApplicationRecord
other_bases_units = arel_table.alias('other_bases_units')
sub_units = arel_table.alias('sub_units')
# TODO: move inner 'with' CTE to outer 'with recursive' - it can have multiple
# CTEs, even non recursive ones.
Unit.with_recursive(actionable_units: [
self.or(Unit.defaults).left_joins(:base)
Unit.with(units: self.or(Unit.defaults)).left_joins(:base)
.where.not(
# Exclude Units that are/have default counterpart
Arel::SelectManager.new.project(1).from(other_units)
@@ -63,14 +65,8 @@ class Unit < ApplicationRecord
),
# Fill base Units to display proper hierarchy. Duplicates will be removed
# by final group() - can't be deduplicated with UNION due to 'portable' field.
# Use ActiveRecord::Relation (not a raw SelectManager) so the SQLite Arel
# visitor does not wrap it in parentheses inside the UNION ALL CTE body.
Unit.joins(
arel_table.create_join(
actionable_units,
arel_table.create_on(actionable_units[:base_id].eq(arel_table[:id]))
)
).select(arel_table[Arel.star], Arel::Nodes.build_quoted(nil).as('portable'))
arel_table.join(actionable_units).on(actionable_units[:base_id].eq(arel_table[:id]))
.project(arel_table[Arel.star], Arel::Nodes.build_quoted(nil).as('portable'))
]).select(units: [:base_id, :symbol])
.select(
units[:id].minimum.as('id'), # can be ANY_VALUE()
@@ -78,7 +74,7 @@ class Unit < ApplicationRecord
Arel::Nodes.build_quoted(1).as('multiplier'), # disregard multiplier when sorting
units[:portable].minimum.as('portable')
)
.from(units).group(units[:base_id], units[:symbol])
.from(units).group(:base_id, :symbol)
}
scope :ordered, ->{
left_outer_joins(:base).order(ordering)
@@ -87,8 +83,8 @@ class Unit < ApplicationRecord
def self.ordering
[arel_table.coalesce(Arel::Table.new(:bases_units)[:symbol], arel_table[:symbol]),
arel_table[:base_id].not_eq(nil),
arel_table[:multiplier],
arel_table[:symbol]]
:multiplier,
:symbol]
end
before_destroy do

View File

@@ -29,4 +29,11 @@ class User < ApplicationRecord
def at_least(status)
User.statuses[self.status] >= User.statuses[status]
end
# Returns true when this user is the only admin account in the system.
# Used to block actions that would leave the application without an admin
# (account deletion, status demotion).
def sole_admin?
admin? && !User.admin.where.not(id: id).exists?
end
end

View File

@@ -4,9 +4,8 @@
<% end %>
<div class="rightside-area buttongrid">
<%#= TODO: Disallow/disable deletion for last admin account, image_button_to_if %>
<%= image_button_to t('.delete'), 'account-remove-outline', user_registration_path,
form_class: 'tools-area', method: :delete, data: {turbo: false},
<%= image_button_to_if !current_user.sole_admin?, t('.delete'), 'account-remove-outline',
user_registration_path, form_class: 'tools-area', method: :delete, data: {turbo: false},
onclick: {confirm: t('.confirm_delete')} %>
</div>

View File

@@ -58,7 +58,4 @@ Rails.application.configure do
# config.action_view.annotate_rendered_view_with_filenames = true
config.log_level = :info
# Allow the default integration test host.
config.hosts << "www.example.com"
end

View File

@@ -162,6 +162,9 @@ en:
New password:
<br><em>leave blank to keep unchanged</em>
%{password_length_hint_html}
registrations:
destroy:
sole_admin: You cannot delete the only admin account.
actions: Actions
setup:
new:

View File

@@ -1,50 +0,0 @@
namespace :test do
desc "Run Rails tests against all supported database adapters (SQLite, MySQL)"
task :all_adapters do
# DATABASE_URL overrides the adapter from database.yml at runtime.
# MySQL requires the mysql2 gem: bundle install --with mysql
adapters = {
"SQLite" => {
"DATABASE_URL" => "sqlite3:db/test.sqlite3"
},
"MySQL" => {
"DATABASE_URL" => format(
"mysql2://%s:%s@%s/%s",
ENV.fetch("DATABASE_USERNAME", "root"),
ENV.fetch("DATABASE_PASSWORD", ""),
ENV.fetch("DATABASE_HOST", "127.0.0.1"),
ENV.fetch("DATABASE_NAME", "fixin_test")
)
}
}
failed = []
adapters.each do |name, extra_env|
puts "\n#{"=" * 60}"
puts " Running tests with #{name}"
puts "=" * 60
env = ENV.to_h.merge("RAILS_ENV" => "test").merge(extra_env)
# Reset test database; db:drop may fail on first run — that's fine
system(env, "bin/rails db:drop")
unless system(env, "bin/rails db:create db:schema:load")
failed << "#{name} (database setup)"
next
end
failed << name unless system(env, "bin/rails test")
end
puts "\n#{"=" * 60}"
if failed.any?
puts " FAILED: #{failed.join(", ")}"
puts "=" * 60
exit 1
else
puts " All adapters passed!"
puts "=" * 60
end
end
end

View File

@@ -1,12 +1,8 @@
require "test_helper"
class Default::UnitsControllerTest < ActionDispatch::IntegrationTest
setup do
sign_in users(:alice)
end
test "should get index" do
get default_units_url
get units_defaults_index_url
assert_response :success
end
end

View File

@@ -0,0 +1,18 @@
require "test_helper"
class RegistrationsControllerTest < ActionDispatch::IntegrationTest
test "sole admin cannot delete account" do
sign_in users(:admin)
delete user_registration_path
assert_redirected_to edit_user_registration_path
assert_equal t("registrations.destroy.sole_admin"), flash[:alert]
assert User.exists?(users(:admin).id)
end
test "non-admin can delete account" do
sign_in users(:alice)
assert_difference ->{ User.count }, -1 do
delete user_registration_path
end
end
end

View File

@@ -2,9 +2,7 @@ require "test_helper"
class UsersControllerTest < ActionDispatch::IntegrationTest
setup do
@admin = users(:admin)
@user = users(:alice)
sign_in @admin
@user = users(:one)
end
test "should get index" do
@@ -12,25 +10,39 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
assert_response :success
end
test "should get new" do
get new_user_url
assert_response :success
end
test "should create user" do
assert_difference("User.count") do
post users_url, params: { user: { email: @user.email, status: @user.status } }
end
assert_redirected_to user_url(User.last)
end
test "should show user" do
get user_url(@user)
assert_response :success
end
test "should get edit" do
get edit_user_url(@user)
assert_response :success
end
test "should update user" do
patch user_url(@user), params: { user: { status: :restricted } }, as: :turbo_stream
assert_equal "restricted", @user.reload.status
patch user_url(@user), params: { user: { email: @user.email, status: @user.status } }
assert_redirected_to user_url(@user)
end
test "should not update self" do
patch user_url(@admin), params: { user: { status: :active } }, as: :turbo_stream,
headers: { "HTTP_REFERER" => users_url }
assert_response :redirect
test "should destroy user" do
assert_difference("User.count", -1) do
delete user_url(@user)
end
test "should forbid non-admin" do
sign_in @user
get users_url
assert_response :forbidden
assert_redirected_to users_url
end
end

View File

@@ -149,7 +149,7 @@ class UsersTest < ApplicationSystemTestCase
end
test "delete profile" do
user = sign_in
user = sign_in user: users.reject(&:admin?).select(&:confirmed?).sample
# TODO: remove condition after root_url changed to different path than
# profile in routes.rb
unless has_current_path?(edit_user_registration_path)
@@ -162,6 +162,14 @@ class UsersTest < ApplicationSystemTestCase
assert_text t("devise.registrations.destroyed")
end
test "sole admin cannot delete profile" do
sign_in user: users(:admin)
unless has_current_path?(edit_user_registration_path)
first(:link_or_button, users(:admin).email).click
end
assert find(:button, t("users.registrations.edit.delete"))[:disabled]
end
test "index forbidden for non admin" do
sign_in user: users.reject(&:admin?).select(&:confirmed?).sample
visit users_path

View File

@@ -2,10 +2,6 @@ ENV["RAILS_ENV"] ||= "test"
require_relative "../config/environment"
require "rails/test_help"
class ActionDispatch::IntegrationTest
include Devise::Test::IntegrationHelpers
end
class ActiveSupport::TestCase
# Run tests in parallel with specified workers
parallelize(workers: :number_of_processors)