Compare commits

..

1 Commits

Author SHA1 Message Date
barbie-bot 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
12 changed files with 62 additions and 104 deletions
+2 -33
View File
@@ -1,13 +1,7 @@
class MeasurementsController < ApplicationController class MeasurementsController < ApplicationController
before_action except: :index do
raise AccessForbidden unless current_user.at_least(:active)
end
def index def index
readouts = current_user.readouts.includes(:quantity, :unit).order(created_at: :desc) @measurements = []
@measurements = readouts.group_by(&:created_at).map do |created_at, grouped| #@measurements = current_user.units.ordered.includes(:base, :subunits)
Measurement.new(created_at: created_at, readouts: grouped)
end
end end
def new def new
@@ -15,33 +9,8 @@ class MeasurementsController < ApplicationController
end end
def create def create
timestamp = Time.current
@readouts = readout_params.map do |rp|
r = current_user.readouts.new(rp)
r.created_at = timestamp
r
end
if @readouts.all?(&:valid?)
Readout.transaction { @readouts.each(&:save!) }
@measurement = Measurement.new(readouts: @readouts, created_at: timestamp)
flash.now[:notice] = t('.success')
else
render :new, status: :unprocessable_entity
end
end end
def destroy def destroy
@measurement = Measurement.new(id: params[:id].to_i,
created_at: Time.at(params[:id].to_i))
current_user.readouts.where(created_at: @measurement.created_at).delete_all
@measurements_empty = current_user.readouts.empty?
flash.now[:notice] = t('.success')
end
private
def readout_params
params.require(:readouts).map { |r| r.permit(:quantity_id, :value, :unit_id) }
end end
end end
+5 -1
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
-14
View File
@@ -1,17 +1,3 @@
class Measurement class Measurement
include ActiveModel::Model include ActiveModel::Model
attr_accessor :readouts, :created_at
def id
created_at.to_i
end
def to_param
id.to_s
end
def persisted?
true
end
end end
+3 -11
View File
@@ -15,8 +15,8 @@ class Quantity < ApplicationRecord
errors.add(:parent, :descendant_reference) if ancestor_of?(parent) errors.add(:parent, :descendant_reference) if ancestor_of?(parent)
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 || Float::INFINITY} length: {maximum: type_for_attribute(:name).limit}
validates :description, length: {maximum: type_for_attribute(:description).limit || Float::INFINITY} 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,8 +61,8 @@ 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) {
if connection.adapter_name =~ /mysql/i
numbered = Arel::Table.new('numbered') numbered = Arel::Table.new('numbered')
self.model.with(numbered: numbered(:parent_id, :name)).with_recursive(arel_table.name => [ self.model.with(numbered: numbered(:parent_id, :name)).with_recursive(arel_table.name => [
numbered.project( numbered.project(
numbered[Arel.star], numbered[Arel.star],
@@ -73,14 +73,6 @@ class Quantity < ApplicationRecord
arel_table[:path].concat(numbered[:child_number]) arel_table[:path].concat(numbered[:child_number])
).join(arel_table).on(numbered[:parent_id].eq(arel_table[:id])) ).join(arel_table).on(numbered[:parent_id].eq(arel_table[:id]))
]).order(arel_table[:path]) ]).order(arel_table[:path])
elsif root.nil?
# SQLite: pathname column already stores the full hierarchical path
order(:pathname)
else
root_pathname = unscoped.where(id: root).pick(:pathname)
scope = order(:pathname).where("pathname LIKE ?", "#{root_pathname}#{PATHNAME_DELIMITER}%")
include_root ? scope.or(where(id: root)) : scope
end
} }
# TODO: extract named functions to custom Arel extension # TODO: extract named functions to custom Arel extension
+7
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
@@ -1,14 +0,0 @@
<%= tag.tr id: dom_id(measurement) do %>
<td><%= l measurement.created_at, format: :short %></td>
<td>
<% measurement.readouts.each do |readout| %>
<span><%= readout.quantity.name %>: <%= readout.value %> <%= readout.unit %></span>
<% end %>
</td>
<% if current_user.at_least(:active) %>
<td class="actions">
<%= image_button_to t('.destroy'), 'delete-outline', measurement_path(measurement),
method: :delete %>
</td>
<% end %>
<% end %>
@@ -1,5 +0,0 @@
<%= turbo_stream.update :flashes %>
<%= turbo_stream.remove :measurement_form %>
<%= turbo_stream.remove :no_items %>
<%= turbo_stream.enable :new_measurement_link %>
<%= turbo_stream.prepend :measurements, @measurement %>
@@ -1,3 +0,0 @@
<%= turbo_stream.update :flashes %>
<%= turbo_stream.remove @measurement %>
<%= turbo_stream.append(:measurements, render_no_items) if @measurements_empty %>
+2 -3
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>
+3 -6
View File
@@ -88,12 +88,6 @@ en:
select_quantity: select the measured quantities... select_quantity: select the measured quantities...
index: index:
new_measurement: Add measurement new_measurement: Add measurement
create:
success: Measurement saved.
destroy:
success: Measurement deleted.
measurement:
destroy: Delete
readouts: readouts:
form: form:
select_unit: ... select_unit: ...
@@ -168,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
@@ -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
+11 -3
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)"