From 6094d29a3f42866dc600920e04f7b5d0718fa111 Mon Sep 17 00:00:00 2001 From: Sergei Poljanski Date: Sat, 10 Oct 2026 08:45:20 +0400 Subject: [PATCH] admin: let superusers rename accounts ADMIN_USER only seeds the first superuser on an empty table, so renaming the secret afterwards had no effect and there was no other way to change a login name. Superusers can now edit usernames in /admin/users, including their own; sessions key on the user id and survive, duplicates are refused, and the rename is audited. Assisted-by: Claude:opus-5 --- CLAUDE.md | 2 +- lib/users_admin.rb | 12 ++++++++++-- test/users_admin_test.rb | 22 ++++++++++++++++++++++ views/admin/users/form.erb | 2 +- 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index fb29079..5cdef88 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -38,7 +38,7 @@ Banking details and registration number are deliberately not on the public page - `Rack::Protection::AuthenticityToken` for CSRF (uses session-stored token) - `AdminAuth` (`lib/admin_auth.rb`, a Sinatra extension): session login at `/admin/login` against the `users` table (`lib/user.rb`, scrypt via OpenSSL, no gem). Session holds user id + `session_version` (bump = logged out everywhere). `ADMIN_USER`/`ADMIN_PASSWORD` bootstrap the first superuser on an empty table and otherwise work only while no active superuser exists (recovery; it re-creates that superuser). Login is rate-limited per IP (`LOGIN_RATE_LIMIT`, default 10/15 min). - Permissions (`lib/admin_acl.rb`): areas `invoices`, `bingo`, `users` × independent flags read/write/delete in `user_permissions` (read + delete without write is valid); superusers bypass. GET → read (…/new, …/duplicate → write), POST …/delete|…/disable → delete, other POST → write. Unmapped /admin paths are superuser-only, and `test/admin_acl_test.rb` fails if a registered admin route isn't mapped — add new admin routes to `AdminACL::RULES`. Views hide forms via `can?(area, perm)`; the server check is the real gate. -- Users UI (`lib/users_admin.rb`): `/admin/users` create/edit/disable/delete, permission grid (you can only grant flags you hold; flags you can't grant keep their value; only superusers touch superuser accounts; no self-lockout; last active superuser can't be deleted). New users get a one-time invite link (48 h) by email via `Mailer.account_link`, also shown once to the admin. `/admin/reset` emails a 1 h reset link (same answer for unknown accounts). Tokens are stored as SHA-256 only (`user_tokens`). `/admin/audit` lists `audit_log` (logins, user changes, bingo and invoice actions) — call `audit!(action, target, detail)` in new admin actions. +- Users UI (`lib/users_admin.rb`): `/admin/users` create/edit/disable/delete (superusers can also rename; sessions key on id, the audit log keeps old names), permission grid (you can only grant flags you hold; flags you can't grant keep their value; only superusers touch superuser accounts; no self-lockout; last active superuser can't be deleted). New users get a one-time invite link (48 h) by email via `Mailer.account_link`, also shown once to the admin. `/admin/reset` emails a 1 h reset link (same answer for unknown accounts). Tokens are stored as SHA-256 only (`user_tokens`). `/admin/audit` lists `audit_log` (logins, user changes, bingo and invoice actions) — call `audit!(action, target, detail)` in new admin actions. - Timestamps: `Sequel.default_timezone = :utc` (lib/db.rb); always write `Time.now.utc`. - Honeypot field named `website` for spam - Static assets in `public/`; views in `views/` diff --git a/lib/users_admin.rb b/lib/users_admin.rb index 0e7c659..10025fc 100644 --- a/lib/users_admin.rb +++ b/lib/users_admin.rb @@ -112,6 +112,11 @@ module UsersAdmin manageable!(user) self_edit = user.id == current_user.id before = perm_summary(user) + old_name = user.username + # Superusers may rename accounts; login sessions survive (they key on id). + if current_user.superuser && params[:username] && !params[:username].strip.empty? + user.username = params[:username].strip.downcase + end user.name = params[:name].to_s.strip user.email = params[:email].to_s.strip.downcase.then { |e| e.empty? ? nil : e } unless self_edit # you can't lock yourself out @@ -120,13 +125,16 @@ module UsersAdmin user.active = active user.superuser = params[:superuser] == '1' if current_user.superuser end - if !user.valid? || (user.email && User.exclude(id: user.id).where(email: user.email).any?) - session[:users_flash] = user.errors.full_messages.join('; ').then { |m| m.empty? ? 'Email already in use.' : m } + taken = User.exclude(id: user.id).where(username: user.username).any? || + (user.email && User.exclude(id: user.id).where(email: user.email).any?) + if !user.valid? || taken + session[:users_flash] = user.errors.full_messages.join('; ').then { |m| m.empty? ? 'Username or email already in use.' : m } redirect "/admin/users/#{user.id}" end perms = permission_params(user) # read before saving changes user.save save_permissions!(user, perms) unless user.superuser + audit!('user.rename', user.username, "#{old_name} → #{user.username}") if old_name != user.username audit!('user.update', user.username, "#{before} → #{perm_summary(user.refresh)}#{' (disabled)' unless user.active}") session[:users_flash] = 'Saved.' redirect "/admin/users/#{user.id}" diff --git a/test/users_admin_test.rb b/test/users_admin_test.rb index ae86642..802d911 100644 --- a/test/users_admin_test.rb +++ b/test/users_admin_test.rb @@ -151,6 +151,28 @@ class UsersAdminTest < Minitest::Test assert_includes last_response.body, 'judy' end + def test_superuser_renames_including_self + admin_login! + me = User[username: ENV['ADMIN_USER']] + post "/admin/users/#{me.id}", { authenticity_token: token, username: 'Renamed', name: '' } + assert_equal 'renamed', me.refresh.username + get '/admin' + assert last_response.ok? # session survives + other = User.create(username: 'taken', password: 'taken-password-1') + post "/admin/users/#{me.id}", { authenticity_token: token, username: 'taken', name: '' } + assert_equal 'renamed', me.refresh.username # duplicate refused + assert_includes DB.connection[:audit_log].select_map(:action), 'user.rename' + end + + def test_non_superuser_cannot_rename + boss = User.create(username: 'manager', password: 'manager-pass-1') + UserPermission.create(user_id: boss.id, area: 'users', can_read: true, can_write: true) + t = User.create(username: 'target', password: 'target-password1') + admin_login!(username: 'manager', password: 'manager-pass-1') + post "/admin/users/#{t.id}", { authenticity_token: token, username: 'hijacked', name: '', active: '1' } + assert_equal 'target', t.refresh.username + end + def test_users_read_only_cannot_create u = User.create(username: 'viewer', password: 'viewer-password1') UserPermission.create(user_id: u.id, area: 'users', can_read: true) diff --git a/views/admin/users/form.erb b/views/admin/users/form.erb index 32fbf91..fd6a7e8 100644 --- a/views/admin/users/form.erb +++ b/views/admin/users/form.erb @@ -14,7 +14,7 @@
>
-
/>
+
/>