From 7fe92d998125d3dc8be3544346de8dbd5c64b240 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Wed, 13 Jun 2018 16:05:55 +0200
Subject: Render access denied without message

The `errors/access_denied` page should not fail to render when no
message is provided.

When accessing something as a sessionless user, we should also display
the terms message if possible.
---
 app/controllers/application_controller.rb         |  6 ++++--
 app/views/errors/access_denied.html.haml          |  2 +-
 spec/controllers/application_controller_spec.rb   | 10 ++++++++++
 spec/views/errors/access_denied.html.haml_spec.rb |  7 +++++++
 4 files changed, 22 insertions(+), 3 deletions(-)
 create mode 100644 spec/views/errors/access_denied.html.haml_spec.rb

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 041837c5410..56312f801fb 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -284,8 +284,10 @@ class ApplicationController < ActionController::Base
     return unless current_user
     return if current_user.terms_accepted?
 
+    message = _("Please accept the Terms of Service before continuing.")
+
     if sessionless_user?
-      render_403
+      access_denied!(message)
     else
       # Redirect to the destination if the request is a get.
       # Redirect to the source if it was a post, so the user can re-submit after
@@ -296,7 +298,7 @@ class ApplicationController < ActionController::Base
                         URI(request.referer).path if request.referer
                       end
 
-      flash[:notice] = _("Please accept the Terms of Service before continuing.")
+      flash[:notice] = message
       redirect_to terms_path(redirect: redirect_path), status: :found
     end
   end
diff --git a/app/views/errors/access_denied.html.haml b/app/views/errors/access_denied.html.haml
index 227c7884915..8ae29b9d337 100644
--- a/app/views/errors/access_denied.html.haml
+++ b/app/views/errors/access_denied.html.haml
@@ -1,4 +1,4 @@
-- message = local_assigns.fetch(:message)
+- message = local_assigns.fetch(:message, nil)
 - content_for(:title, 'Access Denied')
 
 = image_tag('illustrations/error-403.svg', alt: '403', lazy: false)
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index 773bf25ed44..fbafb4a4de8 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/spec/controllers/application_controller_spec.rb
@@ -458,6 +458,8 @@ describe ApplicationController do
       end
 
       context 'for sessionless users' do
+        render_views
+
         before do
           sign_out user
         end
@@ -468,6 +470,14 @@ describe ApplicationController do
           expect(response).to have_gitlab_http_status(403)
         end
 
+        it 'renders the error message when the format was html' do
+          get :index,
+              private_token: create(:personal_access_token, user: user).token,
+              format: :html
+
+          expect(response.body).to have_content /accept the terms of service/i
+        end
+
         it 'renders a 200 when the sessionless user accepted the terms' do
           accept_terms(user)
 
diff --git a/spec/views/errors/access_denied.html.haml_spec.rb b/spec/views/errors/access_denied.html.haml_spec.rb
new file mode 100644
index 00000000000..bde2f6f0169
--- /dev/null
+++ b/spec/views/errors/access_denied.html.haml_spec.rb
@@ -0,0 +1,7 @@
+require 'spec_helper'
+
+describe 'errors/access_denied' do
+  it 'does not fail to render when there is no message provided' do
+    expect { render }.not_to raise_error
+  end
+end
-- 
cgit v1.2.1