From 3bd306ddfa97562ad81789c4cdde17901f8909eb Mon Sep 17 00:00:00 2001 From: Luke Bennett Date: Wed, 19 Dec 2018 12:04:24 +0000 Subject: Show the correct error page when access is denied --- app/controllers/application_controller.rb | 8 +++++++- .../fix-403-page-is-rendered-but-404-is-the-response.yml | 5 +++++ spec/controllers/application_controller_spec.rb | 2 ++ spec/features/projects/pipelines/pipeline_spec.rb | 7 ++++--- 4 files changed, 18 insertions(+), 4 deletions(-) create mode 100644 changelogs/unreleased/fix-403-page-is-rendered-but-404-is-the-response.yml diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index a8fc848c879..26cd5dc801f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -177,11 +177,17 @@ class ApplicationController < ActionController::Base # hide existence of the resource, rather tell them they cannot access it using # the provided message status ||= message.present? ? :forbidden : :not_found + template = + if status == :not_found + "errors/not_found" + else + "errors/access_denied" + end respond_to do |format| format.any { head status } format.html do - render "errors/access_denied", + render template, layout: "errors", status: status, locals: { message: message } diff --git a/changelogs/unreleased/fix-403-page-is-rendered-but-404-is-the-response.yml b/changelogs/unreleased/fix-403-page-is-rendered-but-404-is-the-response.yml new file mode 100644 index 00000000000..eda69b32094 --- /dev/null +++ b/changelogs/unreleased/fix-403-page-is-rendered-but-404-is-the-response.yml @@ -0,0 +1,5 @@ +--- +title: Show the correct error page when access is denied +merge_request: 23932 +author: +type: fixed diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 43f561f7a25..c290acb72aa 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -519,12 +519,14 @@ describe ApplicationController do get :index expect(response).to have_gitlab_http_status(404) + expect(response).to render_template('errors/not_found') end it 'renders a 403 when a message is passed to access denied' do get :index, params: { message: 'None shall pass' } expect(response).to have_gitlab_http_status(403) + expect(response).to render_template('errors/access_denied') end it 'renders a status passed to access denied' do diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index 4706c28bb3d..3192c9ffad4 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -477,10 +477,11 @@ describe 'Pipeline', :js do end context 'when accessing failed jobs page' do - it 'fails to access the page' do - subject + it 'renders a 404 page' do + requests = inspect_requests { subject } - expect(page).to have_title('Access Denied') + expect(page).to have_title('Not Found') + expect(requests.first.status_code).to eq(404) end end end -- cgit v1.2.1