Commit 491e1fc9 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Render a 403 when showing an access denied message

When we want to show an access denied message to a user, we don't have
to hide the resource's existence.

So in that case we render a 403, this 403 is not handled by nginx on
omnibus installs, making sure the message is visible to the user.
parent 04236363
...@@ -130,12 +130,17 @@ class ApplicationController < ActionController::Base ...@@ -130,12 +130,17 @@ class ApplicationController < ActionController::Base
end end
def access_denied!(message = nil) def access_denied!(message = nil)
# If we display a custom access denied message to the user, we don't want to
# hide existence of the resource, rather tell them they cannot access it using
# the provided message
status = message.present? ? :forbidden : :not_found
respond_to do |format| respond_to do |format|
format.any { head :not_found } format.any { head status }
format.html do format.html do
render "errors/access_denied", render "errors/access_denied",
layout: "errors", layout: "errors",
status: 404, status: status,
locals: { message: message } locals: { message: message }
end end
end end
......
...@@ -477,4 +477,28 @@ describe ApplicationController do ...@@ -477,4 +477,28 @@ describe ApplicationController do
end end
end end
end end
describe '#access_denied' do
controller(described_class) do
def index
access_denied!(params[:message])
end
end
before do
sign_in user
end
it 'renders a 404 without a message' do
get :index
expect(response).to have_gitlab_http_status(404)
end
it 'renders a 403 when a message is passed to access denied' do
get :index, message: 'None shall pass'
expect(response).to have_gitlab_http_status(403)
end
end
end end
...@@ -43,13 +43,13 @@ describe ControllerWithCrossProjectAccessCheck do ...@@ -43,13 +43,13 @@ describe ControllerWithCrossProjectAccessCheck do
end end
end end
it 'renders a 404 with trying to access a cross project page' do it 'renders a 403 with trying to access a cross project page' do
message = "This page is unavailable because you are not allowed to read "\ message = "This page is unavailable because you are not allowed to read "\
"information across multiple projects." "information across multiple projects."
get :index get :index
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(403)
expect(response.body).to match(/#{message}/) expect(response.body).to match(/#{message}/)
end end
...@@ -119,7 +119,7 @@ describe ControllerWithCrossProjectAccessCheck do ...@@ -119,7 +119,7 @@ describe ControllerWithCrossProjectAccessCheck do
get :index get :index
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(403)
end end
it 'is executed when the `unless` condition returns true' do it 'is executed when the `unless` condition returns true' do
...@@ -127,19 +127,19 @@ describe ControllerWithCrossProjectAccessCheck do ...@@ -127,19 +127,19 @@ describe ControllerWithCrossProjectAccessCheck do
get :index get :index
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(403)
end end
it 'does not skip the check on an action that is not skipped' do it 'does not skip the check on an action that is not skipped' do
get :show, id: 'hello' get :show, id: 'hello'
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(403)
end end
it 'does not skip the check on an action that was not defined to skip' do it 'does not skip the check on an action that was not defined to skip' do
get :edit, id: 'hello' get :edit, id: 'hello'
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(403)
end end
end end
end end
......
...@@ -32,7 +32,7 @@ describe SearchController do ...@@ -32,7 +32,7 @@ describe SearchController do
it 'still blocks searches without a project_id' do it 'still blocks searches without a project_id' do
get :show, search: 'hello' get :show, search: 'hello'
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(403)
end end
it 'allows searches with a project_id' do it 'allows searches with a project_id' 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