diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-06-04 18:04:04 +0300 |
---|---|---|
committer | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-06-05 11:29:27 +0300 |
commit | 491e1fc905ef52dcc2e7df7deabd3c1f6e42aa52 (patch) | |
tree | 74e4732b0f66f2a6828740278c1914cd4faa0869 | |
parent | 04236363bce399fbde36f396fdcf51d61735e1b0 (diff) |
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.
4 files changed, 38 insertions, 9 deletions
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index db8a8cdc0d2..bc60a0a02e8 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -130,12 +130,17 @@ class ApplicationController < ActionController::Base end 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| - format.any { head :not_found } + format.any { head status } format.html do render "errors/access_denied", layout: "errors", - status: 404, + status: status, locals: { message: message } end end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index b048da1991c..683c57c96f8 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -477,4 +477,28 @@ describe ApplicationController do 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 diff --git a/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb b/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb index 27f558e1b5d..d20471ef603 100644 --- a/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb +++ b/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb @@ -43,13 +43,13 @@ describe ControllerWithCrossProjectAccessCheck do 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 "\ "information across multiple projects." get :index - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) expect(response.body).to match(/#{message}/) end @@ -119,7 +119,7 @@ describe ControllerWithCrossProjectAccessCheck do get :index - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end it 'is executed when the `unless` condition returns true' do @@ -127,19 +127,19 @@ describe ControllerWithCrossProjectAccessCheck do get :index - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end it 'does not skip the check on an action that is not skipped' do get :show, id: 'hello' - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end it 'does not skip the check on an action that was not defined to skip' do get :edit, id: 'hello' - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end end end diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index 30c06ddf744..416a09e1684 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -32,7 +32,7 @@ describe SearchController do it 'still blocks searches without a project_id' do get :show, search: 'hello' - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end it 'allows searches with a project_id' do |