Commit 87f6f6f7 authored by Lin Jen-Shin's avatar Lin Jen-Shin

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
parents a2b26577 3bd306dd
...@@ -177,11 +177,17 @@ class ApplicationController < ActionController::Base ...@@ -177,11 +177,17 @@ class ApplicationController < ActionController::Base
# hide existence of the resource, rather tell them they cannot access it using # hide existence of the resource, rather tell them they cannot access it using
# the provided message # the provided message
status ||= message.present? ? :forbidden : :not_found status ||= message.present? ? :forbidden : :not_found
template =
if status == :not_found
"errors/not_found"
else
"errors/access_denied"
end
respond_to do |format| respond_to do |format|
format.any { head status } format.any { head status }
format.html do format.html do
render "errors/access_denied", render template,
layout: "errors", layout: "errors",
status: status, status: status,
locals: { message: message } locals: { message: message }
......
---
title: Show the correct error page when access is denied
merge_request: 23932
author:
type: fixed
...@@ -519,12 +519,14 @@ describe ApplicationController do ...@@ -519,12 +519,14 @@ describe ApplicationController do
get :index get :index
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(404)
expect(response).to render_template('errors/not_found')
end end
it 'renders a 403 when a message is passed to access denied' do it 'renders a 403 when a message is passed to access denied' do
get :index, params: { message: 'None shall pass' } get :index, params: { message: 'None shall pass' }
expect(response).to have_gitlab_http_status(403) expect(response).to have_gitlab_http_status(403)
expect(response).to render_template('errors/access_denied')
end end
it 'renders a status passed to access denied' do it 'renders a status passed to access denied' do
......
...@@ -477,10 +477,11 @@ describe 'Pipeline', :js do ...@@ -477,10 +477,11 @@ describe 'Pipeline', :js do
end end
context 'when accessing failed jobs page' do context 'when accessing failed jobs page' do
it 'fails to access the page' do it 'renders a 404 page' do
subject 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 end
end end
......
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