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
This commit is contained in:
parent
1dd8f9081f
commit
6094d29a3f
4 changed files with 34 additions and 4 deletions
|
|
@ -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/`
|
||||
|
|
|
|||
|
|
@ -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}"
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@
|
|||
<input type="hidden" name="authenticity_token" value="<%= csrf_token %>" />
|
||||
<fieldset class="plain" <%= 'disabled' unless editable %>>
|
||||
<div class="invoice-grid">
|
||||
<div class="field"><label for="username">Username</label><input type="text" id="username" name="username" value="<%= @user.username %>" <%= fresh ? 'required pattern=[a-z0-9][a-z0-9_.@-]{1,63}' : 'disabled' %> /></div>
|
||||
<div class="field"><label for="username">Username</label><input type="text" id="username" name="username" value="<%= @user.username %>" <%= fresh || current_user.superuser ? 'required pattern=[a-z0-9][a-z0-9_.@-]{1,63}' : 'disabled' %> /></div>
|
||||
<div class="field"><label for="name">Name <span class="optional">(optional)</span></label><input type="text" id="name" name="name" value="<%= @user.name %>" /></div>
|
||||
<div class="field"><label for="email">Email <span class="optional">(for invite and password reset)</span></label><input type="email" id="email" name="email" value="<%= @user.email %>" /></div>
|
||||
</div>
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue