diff --git a/lib/admin_auth.rb b/lib/admin_auth.rb index 6de8ce7..c464e88 100644 --- a/lib/admin_auth.rb +++ b/lib/admin_auth.rb @@ -58,8 +58,10 @@ module AdminAuth Audit.log(action: action, user: user, target: target, detail: detail, ip: client_ip) end + # Trailing slashes dropped: /admin/ has no route (see the before filter). def safe_next(target) - target.to_s.match?(%r{\A/admin(/[\w\-/]*)?\z}) ? target : '/admin' + target = target.to_s.sub(%r{(?<=.)/+\z}, '') + target.match?(%r{\A/admin(/[\w\-/]*)?\z}) ? target : '/admin' end end @@ -68,6 +70,8 @@ module AdminAuth app.before do next unless admin_path? + # One URL per page: /admin/ and /admin/bingo/ go to the slashless route. + redirect request.fullpath.sub(%r{/+(?=\?|\z)}, ''), 301 if request.path_info.match?(%r{./\z}) halt 503, 'Admin needs DATABASE_URL.' unless defined?(User) next if request.path_info.match?(PUBLIC) unless current_user diff --git a/test/admin_auth_test.rb b/test/admin_auth_test.rb index 3590912..2e711cd 100644 --- a/test/admin_auth_test.rb +++ b/test/admin_auth_test.rb @@ -48,6 +48,20 @@ class AdminAuthTest < Minitest::Test assert_equal 302, last_response.status end + def test_trailing_slash_admin_urls + get '/admin/' + assert_equal 301, last_response.status + assert_match %r{/admin\z}, last_response.location + get '/admin/bingo/?x=1' + assert_match %r{/admin/bingo\?x=1\z}, last_response.location + + User.create(username: 'alice', superuser: true, password: 'alice-password-1') + get '/admin/login?next=/admin/' + token = last_response.body[/name="authenticity_token" value="([^"]+)"/, 1] + post '/admin/login', { authenticity_token: token, username: 'alice', password: 'alice-password-1', next: '/admin/' } + assert_match %r{/admin\z}, last_response.location + end + def test_wrong_password_and_unknown_user User.create(username: 'alice', superuser: true, password: 'alice-password-1') login('alice', 'nope')