diff --git a/CLAUDE.md b/CLAUDE.md index a24a0c5..67298af 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -36,7 +36,7 @@ Banking details and registration number are deliberately not on the public page - **aws-sdk-s3** against MinIO on the storage stack - In-memory `RateLimit` (5 req/hour/IP) — see `lib/rate_limit.rb` - `Rack::Protection::AuthenticityToken` for CSRF (uses session-stored token) -- `AdminAuth` Rack middleware: HTTP Basic on `/admin/*`, credentials from env. Greedy-prefix bug fixed; only `/admin` exact and `/admin/` paths are gated, so sibling assets like `/admin-invoice-form.js` stay public. +- `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). - 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 cfd1478..bdf5bea 100644 --- a/asxpio.rb +++ b/asxpio.rb @@ -37,6 +37,8 @@ if ENV['DATABASE_URL'] require_relative 'lib/invoice_pdf' require_relative 'lib/bingo' Bingo.seed! + require_relative 'lib/user' + User.bootstrap!(ENV['ADMIN_USER'], ENV['ADMIN_PASSWORD']) else $logger.warn('DATABASE_URL not set — invoicing disabled') end @@ -63,7 +65,7 @@ class AsxpioWeb < Sinatra::Base same_site: :lax use Rack::Protection::AuthenticityToken - use AdminAuth + register AdminAuth register BingoAdmin Mailer.configure! diff --git a/db/migrations/007_users.rb b/db/migrations/007_users.rb new file mode 100644 index 0000000..3b4d5fb --- /dev/null +++ b/db/migrations/007_users.rb @@ -0,0 +1,28 @@ +Sequel.migration do + change do + create_table(:users) do + primary_key :id + String :username, null: false, unique: true # login, lowercase + String :email, unique: true # invites / resets + String :name + String :password_digest # NULL until an invite is accepted + TrueClass :superuser, null: false, default: false + TrueClass :active, null: false, default: true + Integer :session_version, null: false, default: 1 # bump = log out everywhere + TrueClass :must_change_password, null: false, default: false + DateTime :last_login_at + DateTime :created_at, null: false + end + + # One row per user and area; flags are independent (read + delete + # without write is allowed). Superusers ignore this table. + create_table(:user_permissions) do + foreign_key :user_id, :users, null: false, on_delete: :cascade + String :area, null: false + TrueClass :can_read, null: false, default: false + TrueClass :can_write, null: false, default: false + TrueClass :can_delete, null: false, default: false + primary_key [:user_id, :area] + end + end +end diff --git a/lib/admin_auth.rb b/lib/admin_auth.rb index acb9ea2..45f560f 100644 --- a/lib/admin_auth.rb +++ b/lib/admin_auth.rb @@ -1,35 +1,141 @@ -require 'rack/auth/basic' require 'rack/utils' +require_relative 'rate_limit' -# Rack middleware that gates everything under /admin/* behind HTTP Basic. -# Credentials come from ENV at boot; missing vars fail closed (401 always). -class AdminAuth - def initialize(app) - @app = app - @user = ENV['ADMIN_USER'] - @pass = ENV['ADMIN_PASSWORD'] - end +# Session login for /admin/*, replacing the old shared HTTP Basic gate. +# +# - Accounts live in `users` (lib/user.rb); the session stores user id + +# session_version, so bumping the version logs that user out everywhere. +# - ADMIN_USER/ADMIN_PASSWORD only work while no active superuser exists +# (first boot, or recovery); using them (re)creates that superuser. +# - Paths in PUBLIC need no login (login itself, invite/reset links). +module AdminAuth + PUBLIC = %r{\A/admin/(login|invite/[^/]+|reset(/[^/]+)?)\z} + # Reachable by any logged-in user regardless of permissions. + OPEN = %r{\A/admin(/account/password|/logout)?\z} - def call(env) - path = env['PATH_INFO'].to_s - return @app.call(env) unless path == '/admin' || path.start_with?('/admin/') + class << self + attr_writer :login_limit - auth = Rack::Auth::Basic::Request.new(env) - if @user && @pass && auth.provided? && auth.basic? && credentials_match?(auth.credentials) - @app.call(env) - else - [401, - { 'content-type' => 'text/plain', 'www-authenticate' => 'Basic realm="asxp.io admin"' }, - ["Unauthorized\n"]] + def login_limit + @login_limit ||= RateLimit.new(limit: Integer(ENV.fetch('LOGIN_RATE_LIMIT', '10')), window: 900) end end - private + module Helpers + def current_user + return @current_user if defined?(@current_user) + uid = session[:uid] + user = uid && User[uid] + @current_user = user if user&.active && user.session_version == session[:uv] + end - def credentials_match?(creds) - user, pass = creds - # Single & so both comparisons always run (no short-circuit timing signal). - Rack::Utils.secure_compare(@user, user.to_s) & - Rack::Utils.secure_compare(@pass, pass.to_s) + def admin_path?(path = request.path_info) + path == '/admin' || path.start_with?('/admin/') + end + + def log_in!(user) + session.clear # fresh session id/CSRF token after authentication + session[:uid] = user.id + session[:uv] = user.session_version + user.update(last_login_at: Time.now.utc) + @current_user = user + end + + def env_credentials?(username, pass) + u, p = ENV['ADMIN_USER'].to_s, ENV['ADMIN_PASSWORD'].to_s + return false if u.empty? || p.empty? + Rack::Utils.secure_compare(u.downcase, username.to_s.strip.downcase) & + Rack::Utils.secure_compare(p, pass.to_s) + end + + def safe_next(target) + target.to_s.match?(%r{\A/admin(/[\w\-/]*)?\z}) ? target : '/admin' + end + end + + def self.registered(app) + app.helpers Helpers + + app.before do + next unless admin_path? + halt 503, 'Admin needs DATABASE_URL.' unless defined?(User) + next if request.path_info.match?(PUBLIC) + unless current_user + redirect "/admin/login?next=#{Rack::Utils.escape(request.fullpath)}" if request.get? + halt 401, 'Login required' + end + 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) + end + + app.get '/admin/login' do + redirect '/admin' if current_user + @page_title = 'Log in · admin' + @noindex = true + @next = safe_next(params[:next]) + erb :'admin/login' + end + + app.post '/admin/login' do + @next = safe_next(params[:next]) + unless AdminAuth.login_limit.allow?(client_ip) + @error = 'Too many attempts. Try again in 15 minutes.' + status 429 + return erb :'admin/login' + end + user = User.authenticate(params[:username], params[:password]) + if !user && !User.active_superuser? && env_credentials?(params[:username], params[:password]) + # Recovery: no usable superuser left, so the env credentials restore one. + name = ENV['ADMIN_USER'].strip.downcase + user = User[username: name] || User.new(username: name) + user.set(superuser: true, active: true) + user.password = ENV['ADMIN_PASSWORD'] + user.save + $logger.warn("users: emergency login restored superuser #{name} from #{client_ip}") + end + if user + log_in!(user) + redirect @next + else + $logger.info("users: failed login for #{params[:username].to_s[0, 64].inspect} from #{client_ip}") + @error = 'Wrong username or password.' + status 401 + erb :'admin/login' + end + end + + app.post '/admin/logout' do + session.clear + redirect '/admin/login' + end + + app.get '/admin/account/password' do + @page_title = 'Change password · admin' + @noindex = true + erb :'admin/account_password' + end + + app.post '/admin/account/password' do + u = current_user + @error = + if !u.authenticate(params[:current]) then 'Current password is wrong.' + elsif params[:password].to_s.length < User::MIN_PASS then "New password needs at least #{User::MIN_PASS} characters." + elsif params[:password] != params[:confirm] then 'Passwords do not match.' + end + if @error + @page_title = 'Change password · admin' + @noindex = true + status 422 + return erb :'admin/account_password' + end + u.password = params[:password] + u.must_change_password = false + u.session_version += 1 # other sessions out + u.save + log_in!(u) + redirect '/admin' + end end end diff --git a/lib/user.rb b/lib/user.rb new file mode 100644 index 0000000..338dc18 --- /dev/null +++ b/lib/user.rb @@ -0,0 +1,87 @@ +require 'sequel' +require 'openssl' +require 'securerandom' +require 'base64' +require 'rack/utils' +require 'uri' + +# Admin accounts. Passwords are scrypt hashes from Ruby's OpenSSL (no gem): +# "scrypt$$$

$$", base64 salt/hash. +class User < Sequel::Model(:users) + AREAS = %w[invoices bingo users].freeze + PERMS = %w[read write delete].freeze + NAME_RE = /\A[a-z0-9][a-z0-9_.@-]{1,63}\z/ + MIN_PASS = 12 + # 2^15 → 32 MiB, ~0.1 s per check; tests lower it via SCRYPT_LOG_N. + SCRYPT = { log_n: Integer(ENV.fetch('SCRYPT_LOG_N', '15')), r: 8, p: 1 }.freeze + + one_to_many :permissions, class: 'UserPermission', key: :user_id + + def self.hash_password(pass, log_n: SCRYPT[:log_n], r: SCRYPT[:r], p: SCRYPT[:p]) + salt = SecureRandom.random_bytes(16) + dk = OpenSSL::KDF.scrypt(pass.to_s, salt: salt, N: 2**log_n, r: r, p: p, length: 32) + ['scrypt', log_n, r, p, Base64.strict_encode64(salt), Base64.strict_encode64(dk)].join('$') + end + + def self.verify_password(pass, digest) + algo, log_n, r, p, salt, hash = digest.to_s.split('$') + return false unless algo == 'scrypt' && hash + dk = OpenSSL::KDF.scrypt(pass.to_s, salt: Base64.strict_decode64(salt), N: 2**log_n.to_i, + r: r.to_i, p: p.to_i, length: 32) + Rack::Utils.secure_compare(Base64.strict_encode64(dk), hash) + end + + # Same cost as a real check, so unknown usernames aren't faster to reject. + DUMMY_DIGEST = hash_password(SecureRandom.hex(16)) + + def self.authenticate(username, pass) + user = self[username: username.to_s.strip.downcase] + ok = verify_password(pass, user&.password_digest || DUMMY_DIGEST) + user if ok && user&.active && user.password_digest + end + + def self.active_superuser? + where(superuser: true, active: true).exclude(password_digest: nil).any? + end + + # First boot: turn ADMIN_USER/ADMIN_PASSWORD into the first superuser. + def self.bootstrap!(username, pass) + return if count.positive? || username.to_s.strip.empty? || pass.to_s.empty? + create(username: username.strip.downcase, superuser: true, password: pass) + $logger.info("users: bootstrapped superuser #{username.strip.downcase}") + end + + def before_create + self.created_at ||= Time.now.utc + super + end + + def validate + super + errors.add(:username, 'lowercase letters, digits, . _ @ - (2–64)') unless username.to_s.match?(NAME_RE) + errors.add(:email, 'invalid') if email && !email.match?(URI::MailTo::EMAIL_REGEXP) + end + + def password=(pass) + self.password_digest = self.class.hash_password(pass) + end + + def authenticate(pass) + password_digest && self.class.verify_password(pass, password_digest) + end + + def can?(area, perm) + return true if superuser + row = permissions.find { |p| p.area == area.to_s } + row ? row[:"can_#{perm}"] : false + end + + def logout_everywhere! + update(session_version: session_version + 1) + end +end + +class UserPermission < Sequel::Model(:user_permissions) + unrestrict_primary_key + many_to_one :user +end diff --git a/public/style.css b/public/style.css index 5144a9f..7a21f26 100644 --- a/public/style.css +++ b/public/style.css @@ -356,6 +356,9 @@ a:hover { font-size: 0.85rem; } .admin-nav a { color: var(--text-dim); text-decoration: none; } +.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; } .admin-nav a.active, .admin-nav a:hover { color: var(--text-bright); } .dash { display: grid; gap: 24px; } diff --git a/test/admin_auth_test.rb b/test/admin_auth_test.rb index 76908ff..39472d7 100644 --- a/test/admin_auth_test.rb +++ b/test/admin_auth_test.rb @@ -1,40 +1,129 @@ require_relative 'test_helper' -require 'base64' - class AdminAuthTest < Minitest::Test - OK_APP = ->(_env) { [200, {}, ['ok']] } + include Rack::Test::Methods + include AdminLogin + + def app + AsxpioWeb + end def setup - @auth = AdminAuth.new(OK_APP) + skip 'TEST_DATABASE_URL not set' unless TestDb.available? + TestDb.clean_users! end - def env_for(path, user: nil, pass: nil) - env = { 'PATH_INFO' => path } - if user - env['HTTP_AUTHORIZATION'] = "Basic #{Base64.strict_encode64("#{user}:#{pass}")}" - end - env + def login(username, password) + get '/admin/login' + token = last_response.body[/name="authenticity_token" value="([^"]+)"/, 1] + post '/admin/login', { authenticity_token: token, username: username, password: password } end - def test_correct_credentials_pass - assert_equal 200, @auth.call(env_for('/admin/invoices', user: 'admin', pass: 'test-password'))[0] + def test_password_hashing + d = User.hash_password('correct horse') + assert_match(/\Ascrypt\$\d+\$8\$1\$/, d) + assert User.verify_password('correct horse', d) + refute User.verify_password('wrong', d) + refute_equal d, User.hash_password('correct horse') # salted end - def test_wrong_password_rejected - assert_equal 401, @auth.call(env_for('/admin/invoices', user: 'admin', pass: 'nope'))[0] + def test_bootstrap_only_once + User.bootstrap!('Boss', 'first-password-1') + assert User[username: 'boss'].superuser + User.bootstrap!('other', 'x' * 12) + assert_equal 1, User.count end - def test_wrong_user_rejected - assert_equal 401, @auth.call(env_for('/admin/invoices', user: 'other', pass: 'test-password'))[0] + def test_login_and_logout + User.create(username: 'alice', superuser: true, password: 'alice-password-1') + login('alice', 'alice-password-1') + assert_equal 302, last_response.status + get '/admin' + assert last_response.ok? + assert_includes last_response.body, 'alice' + get '/admin' + token = last_response.body[/name="authenticity_token" value="([^"]+)"/, 1] + post '/admin/logout', { authenticity_token: token } + get '/admin' + assert_equal 302, last_response.status end - def test_missing_header_rejected - assert_equal 401, @auth.call(env_for('/admin'))[0] + def test_wrong_password_and_unknown_user + User.create(username: 'alice', superuser: true, password: 'alice-password-1') + login('alice', 'nope') + assert_equal 401, last_response.status + login('mallory', 'alice-password-1') + assert_equal 401, last_response.status end - def test_sibling_paths_not_gated - assert_equal 200, @auth.call(env_for('/'))[0] - assert_equal 200, @auth.call(env_for('/admin-invoice-form.js'))[0] + def test_inactive_user_cannot_log_in + User.create(username: 'alice', superuser: true, active: false, password: 'alice-password-1') + User.create(username: 'root', superuser: true, password: 'root-password-11') + login('alice', 'alice-password-1') + assert_equal 401, last_response.status + end + + def test_env_credentials_only_without_active_superuser + User.create(username: 'alice', superuser: true, password: 'alice-password-1') + login(ENV['ADMIN_USER'], ENV['ADMIN_PASSWORD']) + assert_equal 401, last_response.status # dormant: alice exists + + User[username: 'alice'].update(active: false) + login(ENV['ADMIN_USER'], ENV['ADMIN_PASSWORD']) + assert_equal 302, last_response.status # recovery + assert User[username: ENV['ADMIN_USER']].superuser + end + + def test_session_revoked_by_version_bump + u = User.create(username: 'alice', superuser: true, password: 'alice-password-1') + login('alice', 'alice-password-1') + get '/admin' + assert last_response.ok? + u.logout_everywhere! + get '/admin' + assert_equal 302, last_response.status + end + + def test_non_superuser_limited_to_open_pages_in_phase_one + User.create(username: 'bob', password: 'bob-password-12') + login('bob', 'bob-password-12') + get '/admin' + assert last_response.ok? + get '/admin/account/password' + assert last_response.ok? + get '/admin/invoices' + assert_equal 403, last_response.status + end + + def test_must_change_password_flow + User.create(username: 'alice', superuser: true, must_change_password: true, password: 'temporary-pass-1') + login('alice', 'temporary-pass-1') + get '/admin/invoices' + assert_equal '/admin/account/password', URI(last_response.location).path + get '/admin/account/password' + token = last_response.body[/name="authenticity_token" value="([^"]+)"/, 1] + post '/admin/account/password', { authenticity_token: token, current: 'temporary-pass-1', password: 'short', confirm: 'short' } + assert_equal 422, last_response.status + post '/admin/account/password', { authenticity_token: token, current: 'temporary-pass-1', + password: 'brand-new-pass-1', confirm: 'brand-new-pass-1' } + assert_equal 302, last_response.status + refute User[username: 'alice'].must_change_password + assert User[username: 'alice'].authenticate('brand-new-pass-1') + end + + def test_next_param_is_restricted_to_admin + User.create(username: 'alice', superuser: true, password: 'alice-password-1') + get '/admin/login?next=https://evil.example/' + token = last_response.body[/name="authenticity_token" value="([^"]+)"/, 1] + post '/admin/login', { authenticity_token: token, username: 'alice', password: 'alice-password-1', next: 'https://evil.example/' } + assert_equal '/admin', URI(last_response.location).path + end + + def test_login_rate_limited + AdminAuth.login_limit = RateLimit.new(limit: 2, window: 900) + 3.times { login('x', 'y') } + assert_equal 429, last_response.status + ensure + AdminAuth.login_limit = nil end end diff --git a/test/app_test.rb b/test/app_test.rb index 729a859..192565d 100644 --- a/test/app_test.rb +++ b/test/app_test.rb @@ -2,6 +2,7 @@ require_relative 'test_helper' class AppTest < Minitest::Test include Rack::Test::Methods + include AdminLogin def app AsxpioWeb @@ -19,10 +20,6 @@ class AppTest < Minitest::Test flunk("no CSRF token found on #{path}") end - def admin_env(extra = {}) - { 'HTTP_AUTHORIZATION' => - "Basic #{Base64.strict_encode64("#{ENV['ADMIN_USER']}:#{ENV['ADMIN_PASSWORD']}")}" }.merge(extra) - end # --- public pages --------------------------------------------------------- @@ -55,9 +52,13 @@ class AppTest < Minitest::Test # --- admin ------------------------------------------------------------ - def test_admin_requires_auth + def test_admin_requires_login + skip 'TEST_DATABASE_URL not set' unless TestDb.available? get '/admin/invoices' - assert_equal 401, last_response.status + assert_equal 302, last_response.status + assert_match %r{/admin/login\?next=%2Fadmin%2Finvoices\z}, last_response.location + post '/admin/invoices' + assert_includes [401, 403], last_response.status end def test_admin_list_with_auth diff --git a/test/bingo_test.rb b/test/bingo_test.rb index bcc542a..5780d38 100644 --- a/test/bingo_test.rb +++ b/test/bingo_test.rb @@ -2,6 +2,7 @@ require_relative 'test_helper' class BingoTest < Minitest::Test include Rack::Test::Methods + include AdminLogin # Minimal well-formed JPEG: SOI, start-of-scan, data, EOI. JPEG = ("\xFF\xD8\xFF\xDA".b + [8].pack('n') + "\1\2\3\4\5\6data\xFF\xD9".b).freeze @@ -20,10 +21,6 @@ class BingoTest < Minitest::Test end end - def admin_env - { 'HTTP_AUTHORIZATION' => - "Basic #{Base64.strict_encode64("#{ENV['ADMIN_USER']}:#{ENV['ADMIN_PASSWORD']}")}" } - end def hit!(idx, kind: 'outage') Bingo::Incident.create(datacenter_id: @dcs[idx].id, occurred_on: Date.today, kind: kind, @@ -247,10 +244,10 @@ class BingoTest < Minitest::Test # --- admin (served by AsxpioWeb) ------------------------------------------- - def test_admin_requires_auth + def test_admin_requires_login @app = AsxpioWeb get '/admin/bingo' - assert_equal 401, last_response.status + assert_equal 302, last_response.status end def test_admin_dashboard diff --git a/test/test_helper.rb b/test/test_helper.rb index e25d351..2157e93 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -10,6 +10,8 @@ ENV['MAIL_FROM'] ||= 'Test ' ENV['MAIL_TO'] ||= 'owner@example.com' ENV['ADMIN_USER'] ||= 'admin' ENV['ADMIN_PASSWORD'] ||= 'test-password' +ENV['SCRYPT_LOG_N'] ||= '10' # cheap hashes in tests +ENV['LOGIN_RATE_LIMIT'] ||= '10000' # DB-backed tests run only against an explicit test database (bin/test # provisions an ephemeral one). Never fall through to a developer DATABASE_URL. @@ -33,6 +35,26 @@ require 'rack/test' Mail.defaults { delivery_method :test } +# Logs the rack-test session in through the real form (CSRF included). +# `admin_env` keeps the old call sites working: it logs in once per test +# and returns extra env to merge. +module AdminLogin + def admin_login!(username: ENV['ADMIN_USER'], password: ENV['ADMIN_PASSWORD']) + User[username: username] || + User.create(username: username, superuser: true, password: password) + get '/admin/login' + token = last_response.body[/name="authenticity_token" value="([^"]+)"/, 1] + post '/admin/login', { authenticity_token: token, username: username, password: password } + raise "login failed: #{last_response.status}" unless last_response.redirect? + @admin_logged_in = true + end + + def admin_env(extra = {}) + admin_login! unless @admin_logged_in + extra + end +end + module TestDb def self.available? !ENV['DATABASE_URL'].nil? @@ -41,4 +63,8 @@ module TestDb def self.clean! DB.connection[:invoices].delete end + + def self.clean_users! + DB.connection[:users].delete + end end diff --git a/views/admin/_nav.erb b/views/admin/_nav.erb index b9a38f9..0f1a51d 100644 --- a/views/admin/_nav.erb +++ b/views/admin/_nav.erb @@ -2,4 +2,10 @@ <% [['/admin', 'dashboard'], ['/admin/invoices', 'invoices'], ['/admin/bingo', 'bingo']].each do |href, label| %> <%= label %> <% end %> + <% if current_user %> + + <%= current_user.username %> +

+ + <% end %> diff --git a/views/admin/account_password.erb b/views/admin/account_password.erb new file mode 100644 index 0000000..f751866 --- /dev/null +++ b/views/admin/account_password.erb @@ -0,0 +1,17 @@ +<%== erb :'admin/_nav', layout: false %> +
+

Change password

+ <% if current_user.must_change_password %>

Set a new password to continue.

<% end %> +
+ +
+ <% if @error %>
<%= @error %>
<% end %> +
+ + +
+
+
+ +
+
diff --git a/views/admin/login.erb b/views/admin/login.erb new file mode 100644 index 0000000..0c1675e --- /dev/null +++ b/views/admin/login.erb @@ -0,0 +1,15 @@ +
+

Admin

+

asxp.io

+
+ +
+ <% if @error %>
<%= @error %>
<% end %> +
+ + +
+
+ +
+