Compare commits

..

1 Commits

Author SHA1 Message Date
f626a814a8 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-10 17:27:15 +00:00
8 changed files with 138 additions and 119 deletions

View File

@@ -1,6 +1,10 @@
class User::ProfilesController < Devise::RegistrationsController class User::ProfilesController < Devise::RegistrationsController
def 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 super
end end

View File

@@ -16,7 +16,7 @@ class Quantity < ApplicationRecord
end end
validates :name, presence: true, uniqueness: {scope: [:user_id, :parent_id]}, validates :name, presence: true, uniqueness: {scope: [:user_id, :parent_id]},
length: {maximum: type_for_attribute(:name).limit} length: {maximum: type_for_attribute(:name).limit}
validates :description, length: {maximum: type_for_attribute(:description).limit} if type_for_attribute(:description).limit validates :description, length: {maximum: type_for_attribute(:description).limit}
# Update :depths of progenies after parent change # Update :depths of progenies after parent change
before_save if: :parent_changed? do before_save if: :parent_changed? do
@@ -61,22 +61,18 @@ class Quantity < ApplicationRecord
# Return: ordered [sub]hierarchy # Return: ordered [sub]hierarchy
scope :ordered, ->(root: nil, include_root: true) { scope :ordered, ->(root: nil, include_root: true) {
q_ordered = Arel::Table.new('q_ordered') numbered = Arel::Table.new('numbered')
cast_type = connection.adapter_name == 'Mysql2' ? 'BINARY' : 'BLOB'
self.model.with(numbered: numbered(:parent_id, :name)).with_recursive(q_ordered: [ self.model.with(numbered: numbered(:parent_id, :name)).with_recursive(arel_table.name => [
Quantity.from(Arel::Table.new('numbered').as(arel_table.name)) numbered.project(
.select( numbered[Arel.star],
arel_table[Arel.star], numbered.cast(numbered[:child_number], 'BINARY').as('path')
arel_table.cast(arel_table[:child_number], cast_type).as('path') ).where(numbered[root && include_root ? :id : :parent_id].eq(root)),
).where(arel_table[root && include_root ? :id : :parent_id].eq(root)), numbered.project(
Quantity.from(Arel::Table.new('numbered').as(arel_table.name)) numbered[Arel.star],
.joins("INNER JOIN q_ordered ON quantities.parent_id = q_ordered.id") arel_table[:path].concat(numbered[:child_number])
.select( ).join(arel_table).on(numbered[:parent_id].eq(arel_table[:id]))
arel_table[Arel.star], ]).order(arel_table[:path])
q_ordered[:path].concat(arel_table[:child_number])
)
]).from(q_ordered.as(arel_table.name)).order(arel_table[:path])
} }
# TODO: extract named functions to custom Arel extension # TODO: extract named functions to custom Arel extension
@@ -84,24 +80,20 @@ class Quantity < ApplicationRecord
# be merged with :ordered # be merged with :ordered
# https://gist.github.com/ProGM/c6df08da14708dcc28b5ca325df37ceb#extending-arel # https://gist.github.com/ProGM/c6df08da14708dcc28b5ca325df37ceb#extending-arel
scope :numbered, ->(parent_column, order_column) { scope :numbered, ->(parent_column, order_column) {
row_number = Arel::Nodes::NamedFunction.new('ROW_NUMBER', []) select(
.over(Arel::Nodes::Window.new.partition(parent_column).order(order_column)) arel_table[Arel.star],
width = Arel::SelectManager.new.project( Arel::Nodes::NamedFunction.new(
'LPAD',
[
Arel::Nodes::NamedFunction.new('ROW_NUMBER', [])
.over(Arel::Nodes::Window.new.partition(parent_column).order(order_column)),
Arel::SelectManager.new.project(
Arel::Nodes::NamedFunction.new('LENGTH', [Arel.star.count]) Arel::Nodes::NamedFunction.new('LENGTH', [Arel.star.count])
).from(arel_table) ).from(arel_table),
Arel::Nodes.build_quoted('0')
pad_expr = if connection.adapter_name == 'Mysql2' ],
Arel::Nodes::NamedFunction.new('LPAD', [row_number, width, Arel::Nodes.build_quoted('0')]) ).as('child_number')
else
# SQLite: printf('%0' || width || 'd', row_number)
fmt = Arel::Nodes::InfixOperation.new('||',
Arel::Nodes::InfixOperation.new('||', Arel::Nodes.build_quoted('%0'), width),
Arel::Nodes.build_quoted('d')
) )
Arel::Nodes::NamedFunction.new('printf', [fmt, row_number])
end
select(arel_table[Arel.star], pad_expr.as('child_number'))
} }
def to_s def to_s
@@ -110,7 +102,7 @@ class Quantity < ApplicationRecord
def to_s_with_depth def to_s_with_depth
# em space, U+2003 # em space, U+2003
' ' * depth + name '' * depth + name
end end
def destroyable? def destroyable?
@@ -123,18 +115,16 @@ class Quantity < ApplicationRecord
# Common ancestors, assuming node is a descendant of itself # Common ancestors, assuming node is a descendant of itself
scope :common_ancestors, ->(of) { scope :common_ancestors, ->(of) {
q_common = Arel::Table.new('q_common') selected = Arel::Table.new('selected')
# Take unique IDs, so self can be called with parent nodes of collection to # Take unique IDs, so self can be called with parent nodes of collection to
# get common ancestors of collection _excluding_ nodes in collection. # get common ancestors of collection _excluding_ nodes in collection.
uniq_of = of.uniq uniq_of = of.uniq
model.with(selected: self).with_recursive(q_common: [ model.with(selected: self).with_recursive(arel_table.name => [
Quantity.from(Arel::Table.new('selected').as(arel_table.name)) selected.project(selected[Arel.star]).where(selected[:id].in(uniq_of)),
.where(arel_table[:id].in(uniq_of)), selected.project(selected[Arel.star])
Quantity.from(Arel::Table.new('selected').as(arel_table.name)) .join(arel_table).on(selected[:id].eq(arel_table[:parent_id]))
.joins("INNER JOIN q_common ON selected.id = q_common.parent_id")
]).select(arel_table[Arel.star]) ]).select(arel_table[Arel.star])
.from(q_common.as(arel_table.name))
.group(column_names) .group(column_names)
.having(arel_table[:id].count.eq(uniq_of.size)) .having(arel_table[:id].count.eq(uniq_of.size))
.order(arel_table[:depth].desc) .order(arel_table[:depth].desc)
@@ -142,9 +132,13 @@ class Quantity < ApplicationRecord
# Return: successive record in order of appearance; used for partial view reload # Return: successive record in order of appearance; used for partial view reload
def successive def successive
ordered = user.quantities.ordered.to_a quantities = Quantity.arel_table
idx = ordered.index { |q| q.id == id } Quantity.with(
ordered[idx + 1] if idx quantities: user.quantities.ordered.select(
quantities[Arel.star],
Arel::Nodes::NamedFunction.new('LAG', [quantities[:id]]).over.as('lag_id')
)
).where(quantities[:lag_id].eq(id)).first
end end
def with_progenies def with_progenies
@@ -157,14 +151,13 @@ class Quantity < ApplicationRecord
# Return: record with ID `of` with its ancestors, sorted by :depth # Return: record with ID `of` with its ancestors, sorted by :depth
scope :with_ancestors, ->(of) { scope :with_ancestors, ->(of) {
q_ancestors = Arel::Table.new('q_ancestors') selected = Arel::Table.new('selected')
model.with(selected: self).with_recursive(q_ancestors: [ model.with(selected: self).with_recursive(arel_table.name => [
Quantity.from(Arel::Table.new('selected').as(arel_table.name)) selected.project(selected[Arel.star]).where(selected[:id].eq(of)),
.where(arel_table[:id].eq(of)), selected.project(selected[Arel.star])
Quantity.from(Arel::Table.new('selected').as(arel_table.name)) .join(arel_table).on(selected[:id].eq(arel_table[:parent_id]))
.joins("INNER JOIN q_ancestors ON selected.id = q_ancestors.parent_id") ])
]).from(q_ancestors.as(arel_table.name))
} }
# Return: ancestors of (possibly destroyed) self # Return: ancestors of (possibly destroyed) self

View File

@@ -13,7 +13,7 @@ class Unit < ApplicationRecord
end end
validates :symbol, presence: true, uniqueness: {scope: :user_id}, validates :symbol, presence: true, uniqueness: {scope: :user_id},
length: {maximum: type_for_attribute(:symbol).limit} length: {maximum: type_for_attribute(:symbol).limit}
validates :description, length: {maximum: type_for_attribute(:description).limit} if type_for_attribute(:description).limit validates :description, length: {maximum: type_for_attribute(:description).limit}
validates :multiplier, numericality: {equal_to: 1}, unless: :base validates :multiplier, numericality: {equal_to: 1}, unless: :base
validates :multiplier, numericality: {greater_than: 0, precision: true, scale: true}, if: :base validates :multiplier, numericality: {greater_than: 0, precision: true, scale: true}, if: :base
@@ -22,23 +22,14 @@ class Unit < ApplicationRecord
actionable_units = Arel::Table.new('actionable_units') actionable_units = Arel::Table.new('actionable_units')
units = actionable_units.alias('units') units = actionable_units.alias('units')
bases_units = arel_table.alias('bases_units') bases_units = arel_table.alias('bases_units')
# Use 'all_units' alias for correlated subqueries to reference the all_units CTE. other_units = arel_table.alias('other_units')
# Cannot use arel_table.alias here because the CTE is named 'all_units', not 'units'. other_bases_units = arel_table.alias('other_bases_units')
other_units = Arel::Table.new('all_units').alias('other_units')
other_bases_units = Arel::Table.new('all_units').alias('other_bases_units')
sub_units = arel_table.alias('sub_units') sub_units = arel_table.alias('sub_units')
Unit.with_recursive( # TODO: move inner 'with' CTE to outer 'with recursive' - it can have multiple
# Renamed from 'units' to 'all_units' to avoid SQLite circular reference: # CTEs, even non recursive ones.
# SQLite treats any CTE in WITH RECURSIVE that references a table with the same Unit.with_recursive(actionable_units: [
# name as the CTE itself as a circular reference, even for non-recursive CTEs. Unit.with(units: self.or(Unit.defaults)).left_joins(:base)
all_units: self.or(Unit.defaults),
actionable_units: [
# Read from all_units CTE (user+defaults) aliased as the units table name.
# Using AR::Relation (not Arel::SelectManager) to avoid extra parentheses
# around the UNION part (visit_Arel_SelectManager always wraps in parens).
Unit.from(Arel::Table.new('all_units').as(arel_table.name))
.joins("LEFT OUTER JOIN all_units bases_units ON bases_units.id = units.base_id")
.where.not( .where.not(
# Exclude Units that are/have default counterpart # Exclude Units that are/have default counterpart
Arel::SelectManager.new.project(1).from(other_units) Arel::SelectManager.new.project(1).from(other_units)
@@ -74,20 +65,16 @@ class Unit < ApplicationRecord
), ),
# Fill base Units to display proper hierarchy. Duplicates will be removed # Fill base Units to display proper hierarchy. Duplicates will be removed
# by final group() - can't be deduplicated with UNION due to 'portable' field. # by final group() - can't be deduplicated with UNION due to 'portable' field.
# Using AR::Relation instead of Arel::SelectManager to avoid extra parentheses arel_table.join(actionable_units).on(actionable_units[:base_id].eq(arel_table[:id]))
# around the UNION part (visit_Arel_SelectManager always wraps in parens). .project(arel_table[Arel.star], Arel::Nodes.build_quoted(nil).as('portable'))
Unit.from(Arel::Table.new('all_units').as(arel_table.name)) ]).select(units: [:base_id, :symbol])
.joins("INNER JOIN actionable_units ON actionable_units.base_id = units.id")
.select(arel_table[Arel.star], Arel::Nodes.build_quoted(nil).as('portable'))
]
).select(units: [:base_id, :symbol])
.select( .select(
units[:id].minimum.as('id'), # can be ANY_VALUE() units[:id].minimum.as('id'), # can be ANY_VALUE()
units[:user_id].minimum.as('user_id'), # prefer non-default units[:user_id].minimum.as('user_id'), # prefer non-default
Arel::Nodes.build_quoted(1).as('multiplier'), # disregard multiplier when sorting Arel::Nodes.build_quoted(1).as('multiplier'), # disregard multiplier when sorting
units[:portable].minimum.as('portable') units[:portable].minimum.as('portable')
) )
.from(units).group(units[:base_id], units[:symbol]) .from(units).group(:base_id, :symbol)
} }
scope :ordered, ->{ scope :ordered, ->{
left_outer_joins(:base).order(ordering) left_outer_joins(:base).order(ordering)
@@ -97,7 +84,7 @@ class Unit < ApplicationRecord
[arel_table.coalesce(Arel::Table.new(:bases_units)[:symbol], arel_table[:symbol]), [arel_table.coalesce(Arel::Table.new(:bases_units)[:symbol], arel_table[:symbol]),
arel_table[:base_id].not_eq(nil), arel_table[:base_id].not_eq(nil),
:multiplier, :multiplier,
arel_table[:symbol]] :symbol]
end end
before_destroy do before_destroy do

View File

@@ -29,4 +29,11 @@ class User < ApplicationRecord
def at_least(status) def at_least(status)
User.statuses[self.status] >= User.statuses[status] User.statuses[self.status] >= User.statuses[status]
end 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 end

View File

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

View File

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

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

@@ -182,8 +182,8 @@ class UsersTest < ApplicationSystemTestCase
assert_title 'Access is forbidden to this page (403)' assert_title 'Access is forbidden to this page (403)'
end end
test 'delete profile' do 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 # TODO: remove condition after root_url changed to different path than
# profile in routes.rb # profile in routes.rb
unless has_current_path?(edit_user_registration_path) unless has_current_path?(edit_user_registration_path)
@@ -196,7 +196,15 @@ class UsersTest < ApplicationSystemTestCase
assert_text t("devise.registrations.destroyed") assert_text t("devise.registrations.destroyed")
end end
test 'index forbidden for non admin' do 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 sign_in user: users.reject(&:admin?).select(&:confirmed?).sample
visit users_path visit users_path
assert_title "Access is forbidden to this page (403)" assert_title "Access is forbidden to this page (403)"