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
12 changed files with 62 additions and 104 deletions

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

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

@@ -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

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,26 +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) {
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],
numbered.cast(numbered[:child_number], 'BINARY').as('path') numbered.cast(numbered[:child_number], 'BINARY').as('path')
).where(numbered[root && include_root ? :id : :parent_id].eq(root)), ).where(numbered[root && include_root ? :id : :parent_id].eq(root)),
numbered.project( numbered.project(
numbered[Arel.star], numbered[Arel.star],
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

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

@@ -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 %>

View File

@@ -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 %>

View File

@@ -1,3 +0,0 @@
<%= turbo_stream.update :flashes %>
<%= turbo_stream.remove @measurement %>
<%= turbo_stream.append(:measurements, render_no_items) if @measurements_empty %>

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

@@ -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

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)"