Commit 2e597518 authored by Emelia Smith's avatar Emelia Smith Committed by Eugen Rochko

Improve require_admin! and require_staff! filters (#7018)

Previously these returns 302 redirects instead of 403s, which meant posting links to admin pages in slack caused them to unfurl, rather than stay as a link. Additionally, require_admin! doesn't appear to be actively used, on require_staff!
parent 1c293086
...@@ -39,11 +39,11 @@ class ApplicationController < ActionController::Base ...@@ -39,11 +39,11 @@ class ApplicationController < ActionController::Base
end end
def require_admin! def require_admin!
redirect_to root_path unless current_user&.admin? forbidden unless current_user&.admin?
end end
def require_staff! def require_staff!
redirect_to root_path unless current_user&.staff? forbidden unless current_user&.staff?
end end
def check_suspension def check_suspension
......
...@@ -9,18 +9,25 @@ describe Admin::BaseController, type: :controller do ...@@ -9,18 +9,25 @@ describe Admin::BaseController, type: :controller do
end end
end end
it 'renders admin layout' do it 'requires administrator or moderator' do
routes.draw { get 'success' => 'admin/base#success' } routes.draw { get 'success' => 'admin/base#success' }
sign_in(Fabricate(:user, admin: true)) sign_in(Fabricate(:user, admin: false, moderator: false))
get :success get :success
expect(response).to render_template layout: 'admin'
expect(response).to have_http_status(:forbidden)
end end
it 'requires administrator' do it 'renders admin layout as a moderator' do
routes.draw { get 'success' => 'admin/base#success' } routes.draw { get 'success' => 'admin/base#success' }
sign_in(Fabricate(:user, admin: false)) sign_in(Fabricate(:user, moderator: true))
get :success get :success
expect(response).to render_template layout: 'admin'
end
expect(response).to redirect_to('/') it 'renders admin layout as an admin' do
routes.draw { get 'success' => 'admin/base#success' }
sign_in(Fabricate(:user, admin: true))
get :success
expect(response).to render_template layout: 'admin'
end end
end end
...@@ -181,10 +181,48 @@ describe ApplicationController, type: :controller do ...@@ -181,10 +181,48 @@ describe ApplicationController, type: :controller do
routes.draw { get 'sucesss' => 'anonymous#sucesss' } routes.draw { get 'sucesss' => 'anonymous#sucesss' }
end end
it 'redirects to root path if current user is not admin' do it 'returns a 403 if current user is not admin' do
sign_in(Fabricate(:user, admin: false)) sign_in(Fabricate(:user, admin: false))
get 'sucesss' get 'sucesss'
expect(response).to redirect_to('/') expect(response).to have_http_status(403)
end
it 'returns a 403 if current user is only a moderator' do
sign_in(Fabricate(:user, moderator: true))
get 'sucesss'
expect(response).to have_http_status(403)
end
it 'does nothing if current user is admin' do
sign_in(Fabricate(:user, admin: true))
get 'sucesss'
expect(response).to have_http_status(200)
end
end
describe 'require_staff!' do
controller do
before_action :require_staff!
def sucesss
head 200
end
end
before do
routes.draw { get 'sucesss' => 'anonymous#sucesss' }
end
it 'returns a 403 if current user is not admin or moderator' do
sign_in(Fabricate(:user, admin: false, moderator: false))
get 'sucesss'
expect(response).to have_http_status(403)
end
it 'does nothing if current user is moderator' do
sign_in(Fabricate(:user, moderator: true))
get 'sucesss'
expect(response).to have_http_status(200)
end end
it 'does nothing if current user is admin' do it 'does nothing if current user is admin' do
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment