Merge branch 'fix-403-page-is-rendered-but-404-is-the-response' into 'master'

Show the correct error page when access is denied

Closes #55110

See merge request gitlab-org/gitlab-ce!23932

4 files changed, 18 insertions, 4 deletions

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