From 8471570187489f49e9a5968a621359a9b47cac6b Mon Sep 17 00:00:00 2001 From: Sergei Poljanski Date: Sat, 10 Oct 2026 08:27:37 +0400 Subject: [PATCH] admin: per-area permissions on every route Give each user independent read, write and delete flags per area (invoices, bingo, users), so someone can e.g. review and remove bingo incidents without being able to add any. The request method and path decide the flag needed: GET is read except the new/duplicate forms, POST to .../delete is delete, any other POST is write. Unmapped admin paths are superuser-only, and a test enumerates every registered admin route so a new one cannot ship without a rule. Pages follow the same flags: nav and dashboard tiles show readable areas only, write forms and delete buttons are hidden, and the datacenter form is shown read-only. Assisted-by: Claude:opus-5 --- CLAUDE.md | 1 + asxpio.rb | 38 ++++++----- lib/admin_acl.rb | 33 ++++++++++ lib/admin_auth.rb | 10 ++- public/style.css | 1 + test/admin_acl_test.rb | 116 +++++++++++++++++++++++++++++++++ test/admin_auth_test.rb | 2 +- views/admin/_nav.erb | 3 +- views/admin/bingo/edit.erb | 10 ++- views/admin/bingo/index.erb | 10 ++- views/admin/dashboard.erb | 9 ++- views/admin/invoices/index.erb | 4 +- views/admin/invoices/show.erb | 4 +- 13 files changed, 214 insertions(+), 27 deletions(-) create mode 100644 lib/admin_acl.rb create mode 100644 test/admin_acl_test.rb diff --git a/CLAUDE.md b/CLAUDE.md index 67298af..7eb7ed4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,6 +37,7 @@ Banking details and registration number are deliberately not on the public page - In-memory `RateLimit` (5 req/hour/IP) — see `lib/rate_limit.rb` - `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. - Honeypot field named `website` for spam - Static assets in `public/`; views in `views/` - Deploy: `Dockerfile` + `docker-compose.yml` (Traefik labels for asxp.io / www.asxp.io). Container joins two external Docker networks: `traefik` (ingress) and `storage` (Postgres + MinIO). diff --git a/asxpio.rb b/asxpio.rb index bdf5bea..87b4761 100644 --- a/asxpio.rb +++ b/asxpio.rb @@ -136,23 +136,27 @@ class AsxpioWeb < Sinatra::Base get '/admin' do @page_title = 'Dashboard · admin' @noindex = true - unpaid = Invoice.where(paid_at: nil).all - @inv = { - count: Invoice.count, - unpaid: unpaid, - overdue: unpaid.count { |i| i.due_on < Date.today }, - unpaid_totals: unpaid.group_by(&:currency).transform_values { |l| l.sum(&:total) }, - recent: Invoice.order(Sequel.desc(:created_at)).limit(5).all - } - board = Bingo::Board.load - @bingo = { - cells: board.cells.size, - hit: board.hit.count(true), - lines: board.wins.size, - reserve: [Bingo::Datacenter.count - board.cells.size, 0].max, - last_incident: board.incidents.first, - no_photo: board.cells.reject(&:photo_key) - } + if can?('invoices', 'read') + unpaid = Invoice.where(paid_at: nil).all + @inv = { + count: Invoice.count, + unpaid: unpaid, + overdue: unpaid.count { |i| i.due_on < Date.today }, + unpaid_totals: unpaid.group_by(&:currency).transform_values { |l| l.sum(&:total) }, + recent: Invoice.order(Sequel.desc(:created_at)).limit(5).all + } + end + if can?('bingo', 'read') + board = Bingo::Board.load + @bingo = { + cells: board.cells.size, + hit: board.hit.count(true), + lines: board.wins.size, + reserve: [Bingo::Datacenter.count - board.cells.size, 0].max, + last_incident: board.incidents.first, + no_photo: board.cells.reject(&:photo_key) + } + end erb :'admin/dashboard' end diff --git a/lib/admin_acl.rb b/lib/admin_acl.rb new file mode 100644 index 0000000..4e66420 --- /dev/null +++ b/lib/admin_acl.rb @@ -0,0 +1,33 @@ +# Which permission an /admin request needs. Every admin path must map to +# an area (or be public/open); unmapped paths are superuser-only, and a +# test fails if any registered route is unmapped. +# +# GET/HEAD → read (except …/new and …/duplicate: write) +# POST …/delete, …/disable → delete +# any other non-GET → write +module AdminACL + RULES = [ + [%r{\A/admin/(invoices|gel-rate|crypto-rate)(/|\z)}, 'invoices'], + [%r{\A/admin/bingo(/|\z)}, 'bingo'], + [%r{\A/admin/(users|audit)(/|\z)}, 'users'], + ].freeze + + module_function + + def area_for(path) + RULES.find { |re, _| path.match?(re) }&.last + end + + def perm_for(method, path) + get = %w[GET HEAD].include?(method) + return 'delete' if !get && path.match?(%r{/(delete|disable)\z}) + return 'write' if !get || path.match?(%r{/(new|duplicate)\z}) + 'read' + end + + def allowed?(user, method, path) + return true if user.superuser || path.match?(AdminAuth::OPEN) + area = area_for(path) or return false + user.can?(area, perm_for(method, path)) + end +end diff --git a/lib/admin_auth.rb b/lib/admin_auth.rb index 45f560f..774c5a8 100644 --- a/lib/admin_auth.rb +++ b/lib/admin_auth.rb @@ -1,5 +1,6 @@ require 'rack/utils' require_relative 'rate_limit' +require_relative 'admin_acl' # Session login for /admin/*, replacing the old shared HTTP Basic gate. # @@ -29,6 +30,11 @@ module AdminAuth @current_user = user if user&.active && user.session_version == session[:uv] end + # For views: show a form/button only if the user may use it. + def can?(area, perm) + current_user&.can?(area, perm) || false + end + def admin_path?(path = request.path_info) path == '/admin' || path.start_with?('/admin/') end @@ -67,7 +73,9 @@ module AdminAuth if current_user.must_change_password && !request.path_info.match?(%r{\A/admin/(account/password|logout)\z}) redirect '/admin/account/password' end - halt 403, 'Forbidden' unless current_user.superuser || request.path_info.match?(OPEN) + unless AdminACL.allowed?(current_user, request.request_method, request.path_info) + halt 403, 'You do not have access to this.' + end end app.get '/admin/login' do diff --git a/public/style.css b/public/style.css index 7a21f26..7fa3ef0 100644 --- a/public/style.css +++ b/public/style.css @@ -359,6 +359,7 @@ a:hover { .admin-nav-user { display: flex; gap: 8px; align-items: baseline; margin-left: 16px; padding-left: 16px; border-left: 1px solid var(--border); } .admin-nav-user form { display: inline; } .auth-form { max-width: 360px; margin: 0 auto; } +fieldset.plain { border: 0; margin: 0; padding: 0; min-width: 0; } .admin-nav a.active, .admin-nav a:hover { color: var(--text-bright); } .dash { display: grid; gap: 24px; } diff --git a/test/admin_acl_test.rb b/test/admin_acl_test.rb new file mode 100644 index 0000000..93dd76d --- /dev/null +++ b/test/admin_acl_test.rb @@ -0,0 +1,116 @@ +require_relative 'test_helper' + +class AdminAclTest < Minitest::Test + include Rack::Test::Methods + include AdminLogin + + def app + AsxpioWeb + end + + def setup + skip 'TEST_DATABASE_URL not set' unless TestDb.available? + TestDb.clean_users! + DB.connection[:bingo_incidents].delete + @dc = Bingo::Datacenter.order(:position).first || + Bingo::Datacenter.create(slug: 'acl-dc', position: 1, name_ru: 'ЦОД', city_ru: 'Москва', operator: 'Op') + end + + def as(perms) + u = User.create(username: 'user1', password: 'user-password-1') + perms.each do |area, flags| + UserPermission.create(user_id: u.id, area: area.to_s, + **flags.to_h { |f| [:"can_#{f}", true] }) + end + admin_login!(username: 'user1', password: 'user-password-1') + end + + def token + get '/admin' + last_response.body[/name="authenticity_token" value="([^"]+)"/, 1] + end + + def forbidden? + last_response.status == 403 + end + + # Every registered /admin route is public, open, or mapped to an area. + def test_every_admin_route_is_mapped + unmapped = AsxpioWeb.routes.flat_map do |verb, routes| + routes.map { |r| [verb, r.first.to_s] } + end.select { |_, p| p.start_with?('/admin') }.reject do |_, p| + sample = p.gsub(/:\w+/, 'x') + sample.match?(AdminAuth::PUBLIC) || sample.match?(AdminAuth::OPEN) || AdminACL.area_for(sample) + end + assert_empty unmapped, "unmapped admin routes: #{unmapped.inspect}" + end + + def test_perm_for + assert_equal 'read', AdminACL.perm_for('GET', '/admin/bingo') + assert_equal 'write', AdminACL.perm_for('GET', '/admin/bingo/dc/new') + assert_equal 'write', AdminACL.perm_for('GET', '/admin/invoices/abc/duplicate') + assert_equal 'write', AdminACL.perm_for('POST', '/admin/bingo/dc/1') + assert_equal 'delete', AdminACL.perm_for('POST', '/admin/bingo/incidents/1/delete') + assert_equal 'delete', AdminACL.perm_for('POST', '/admin/bingo/dc/1/photo/delete') + end + + def test_bingo_read_only + as(bingo: %i[read]) + get '/admin/bingo' + assert last_response.ok? + refute_includes last_response.body, 'Record incident' + refute_includes last_response.body, 'href="/admin/invoices"' # nav hides invoices + get "/admin/bingo/dc/#{@dc.id}" + assert last_response.ok? + assert_includes last_response.body, 'disabled' + refute_includes last_response.body, 'Danger zone' + get '/admin/bingo/dc/new' + assert forbidden? + post '/admin/bingo/incidents', { authenticity_token: token, datacenter_id: @dc.id, occurred_on: '2026-10-01', kind: 'fire', title_ru: 'x' } + assert forbidden? + get '/admin/invoices' + assert forbidden? + end + + def test_read_and_delete_without_write + inc = Bingo::Incident.create(datacenter_id: @dc.id, occurred_on: Date.today, kind: 'fire', title_ru: 'x') + as(bingo: %i[read delete]) + post '/admin/bingo/incidents', { authenticity_token: token, datacenter_id: @dc.id, occurred_on: '2026-10-01', kind: 'fire', title_ru: 'y' } + assert forbidden? + get '/admin/bingo' + assert_includes last_response.body, "/admin/bingo/incidents/#{inc.id}/delete" + post "/admin/bingo/incidents/#{inc.id}/delete", { authenticity_token: token } + refute forbidden? + assert_nil Bingo::Incident[inc.id] + end + + def test_invoices_read_only + as(invoices: %i[read]) + get '/admin/invoices' + assert last_response.ok? + refute_includes last_response.body, '+ New invoice' + get '/admin/invoices/new' + assert forbidden? + post '/admin/invoices', { authenticity_token: token } + assert forbidden? + get '/admin/bingo' + assert forbidden? + get '/admin' + refute_includes last_response.body, '

' + end + + def test_user_without_areas_sees_notice + as({}) + get '/admin' + assert last_response.ok? + assert_includes last_response.body, 'No areas are assigned' + end + + def test_superuser_sees_everything + admin_login! + get '/admin/bingo/dc/new' + assert last_response.ok? + get '/admin/invoices/new' + assert last_response.ok? + end +end diff --git a/test/admin_auth_test.rb b/test/admin_auth_test.rb index 39472d7..3590912 100644 --- a/test/admin_auth_test.rb +++ b/test/admin_auth_test.rb @@ -84,7 +84,7 @@ class AdminAuthTest < Minitest::Test assert_equal 302, last_response.status end - def test_non_superuser_limited_to_open_pages_in_phase_one + def test_user_without_permissions_limited_to_open_pages User.create(username: 'bob', password: 'bob-password-12') login('bob', 'bob-password-12') get '/admin' diff --git a/views/admin/_nav.erb b/views/admin/_nav.erb index 0f1a51d..4b2e1f9 100644 --- a/views/admin/_nav.erb +++ b/views/admin/_nav.erb @@ -1,5 +1,6 @@