From 286e0ca878b80c321d165c460dc625407cec2a30 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Wed, 24 Apr 2019 15:44:05 +0200
Subject: [PATCH] Handle AccessDenied in management sessions

We were raising a `CanCan::AcessDenied` and were getting a 500 Internal
Server Error.

I've chosen to do the same thing we do in the ApplicationController.
There are other options to handle this request, like redirecting to the
login page or returning a 401 Unauthorized HTTP status.
---
 app/controllers/application_controller.rb           |  8 +-------
 app/controllers/concerns/access_denied_handler.rb   | 12 ++++++++++++
 app/controllers/management/sessions_controller.rb   |  1 +
 .../management/sessions_controller_spec.rb          | 13 +++++++++----
 4 files changed, 23 insertions(+), 11 deletions(-)
 create mode 100644 app/controllers/concerns/access_denied_handler.rb

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index d22ec7269..09dc42034 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -3,6 +3,7 @@ require "application_responder"
 class ApplicationController < ActionController::Base
   include HasFilters
   include HasOrders
+  include AccessDeniedHandler
 
   protect_from_forgery with: :exception
 
@@ -17,13 +18,6 @@ class ApplicationController < ActionController::Base
   check_authorization unless: :devise_controller?
   self.responder = ApplicationResponder
 
-  rescue_from CanCan::AccessDenied do |exception|
-    respond_to do |format|
-      format.html { redirect_to main_app.root_url, alert: exception.message }
-      format.json { render json: {error: exception.message}, status: :forbidden }
-    end
-  end
-
   layout :set_layout
   respond_to :html
   helper_method :current_budget
diff --git a/app/controllers/concerns/access_denied_handler.rb b/app/controllers/concerns/access_denied_handler.rb
new file mode 100644
index 000000000..d924baf56
--- /dev/null
+++ b/app/controllers/concerns/access_denied_handler.rb
@@ -0,0 +1,12 @@
+module AccessDeniedHandler
+  extend ActiveSupport::Concern
+
+  included do
+    rescue_from CanCan::AccessDenied do |exception|
+      respond_to do |format|
+        format.html { redirect_to main_app.root_url, alert: exception.message }
+        format.json { render json: { error: exception.message }, status: :forbidden }
+      end
+    end
+  end
+end
diff --git a/app/controllers/management/sessions_controller.rb b/app/controllers/management/sessions_controller.rb
index 6db303a39..a88c1de9f 100644
--- a/app/controllers/management/sessions_controller.rb
+++ b/app/controllers/management/sessions_controller.rb
@@ -1,6 +1,7 @@
 require "manager_authenticator"
 
 class Management::SessionsController < ActionController::Base
+  include AccessDeniedHandler
 
   def create
     destroy_session
diff --git a/spec/controllers/management/sessions_controller_spec.rb b/spec/controllers/management/sessions_controller_spec.rb
index 650f53246..2d58b0202 100644
--- a/spec/controllers/management/sessions_controller_spec.rb
+++ b/spec/controllers/management/sessions_controller_spec.rb
@@ -3,11 +3,13 @@ require "rails_helper"
 describe Management::SessionsController do
 
   describe "Sign in" do
+
     it "denies access if wrong manager credentials" do
       allow_any_instance_of(ManagerAuthenticator).to receive(:auth).and_return(false)
-      expect {
-        get :create, params: { login: "nonexistent", clave_usuario: "wrong" }
-      }.to raise_error CanCan::AccessDenied
+      get :create, params: { login: "nonexistent", clave_usuario: "wrong" }
+
+      expect(response).to redirect_to "/"
+      expect(flash[:alert]).to eq "You do not have permission to access this page."
       expect(session[:manager]).to be_nil
     end
 
@@ -42,7 +44,10 @@ describe Management::SessionsController do
 
     it "denies access if user is not admin or manager" do
       sign_in create(:user)
-      expect { get :create}.to raise_error CanCan::AccessDenied
+      get :create
+
+      expect(response).to redirect_to "/"
+      expect(flash[:alert]).to eq "You do not have permission to access this page."
       expect(session[:manager]).to be_nil
     end
   end