Move "validate_[activity]!" authorization into a module

Add Pundit

Move "validate_chapter_leader!" and "validate_region_leader!"
to Pundit classes.

Gemfile

@@ -6,6 +6,7 @@ gem 'dotenv-rails', groups: [:development, :test]
 
 gem 'rails', '4.2.5.1'
 gem 'devise', '~> 3.5.0'
+gem 'pundit'
 gem 'puma'
 gem 'jquery-rails'
 gem 'nested_form'

Gemfile.lock

@@ -222,6 +222,8 @@ GEM
       multi_json (~> 1.0)
       websocket-driver (>= 0.2.0)
     puma (2.16.0)
+    pundit (1.1.0)
+      activesupport (>= 3.0.0)
     quiet_assets (1.1.0)
       railties (>= 3.1, < 5.0)
     rack (1.6.4)
@@ -379,6 +381,7 @@ DEPENDENCIES
   pg
   poltergeist
   puma
+  pundit
   quiet_assets
   rack-canonical-host
   rack-mini-profiler

app/authorization/controller_authorization.rb

@@ -0,0 +1,35 @@
+module ControllerAuthorization
+  def validate_admin!
+    unless current_user.admin?
+      flash[:error] = "You must be an Admin to see this page"
+      redirect_to events_path
+    end
+  end
+
+  def validate_organizer!
+    @event ||= Event.find(params[:event_id])
+    if @event.historical?
+      flash[:error] = "This feature is not available for historical events"
+      return redirect_to events_path
+    end
+
+    unless @event.editable_by?(current_user)
+      flash[:error] = "You must be an organizer for the event or an admin/chapter leader to see this page"
+      redirect_to events_path
+    end
+  end
+
+  def validate_checkiner!
+    unless @event.checkiner?(current_user) || current_user.admin?
+      flash[:error] = "You must be a checkiner, organizer, or admin to see this page."
+      redirect_to events_path
+    end
+  end
+
+  def validate_publisher!
+    unless current_user.publisher? || current_user.admin?
+      flash[:error] = "You must be authorized to publish events to see this page."
+      redirect_to events_path
+    end
+  end
+end

app/controllers/application_controller.rb

@@ -1,4 +1,8 @@
 class ApplicationController < ActionController::Base
+  include ControllerAuthorization
+  include Pundit
+  rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized
+
   before_action :configure_permitted_parameters, if: :devise_controller?
   force_ssl if: -> { Rails.env.production? }, unless: :allow_insecure?
 
@@ -18,58 +22,6 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  def validate_admin!
-    unless current_user.admin?
-      flash[:error] = "You must be an Admin to see this page"
-      redirect_to events_path
-    end
-  end
-
-  def validate_organizer!
-    @event ||= Event.find(params[:event_id])
-    if @event.historical?
-      flash[:error] = "This feature is not available for historical events"
-      return redirect_to events_path
-    end
-
-    unless @event.editable_by?(current_user)
-      flash[:error] = "You must be an organizer for the event or an admin/chapter leader to see this page"
-      redirect_to events_path
-    end
-  end
-
-  def validate_checkiner!
-    unless @event.checkiner?(current_user) || current_user.admin?
-      flash[:error] = "You must be a checkiner, organizer, or admin to see this page."
-      redirect_to events_path
-    end
-  end
-
-  def validate_publisher!
-    unless current_user.publisher? || current_user.admin?
-      flash[:error] = "You must be authorized to publish events to see this page."
-      redirect_to events_path
-    end
-  end
-
-  def validate_region_leader!
-    @region ||= Region.find(params[:region_id])
-
-    unless @region.has_leader?(current_user)
-      flash[:error] = "You must be a region leader or admin to view this page."
-      redirect_to events_path
-    end
-  end
-
-  def validate_chapter_leader!
-    @chapter ||= Chapter.find(params[:chapter_id])
-
-    unless @chapter.has_leader?(current_user)
-      flash[:error] = "You must be a chapter leader or admin to view this page."
-      redirect_to events_path
-    end
-  end
-
   def after_sign_in_path_for(resource)
     params[:return_to] || super
   end
@@ -85,4 +37,9 @@ def configure_permitted_parameters
   def allow_insecure?
     false
   end
+
+  def user_not_authorized
+    flash[:error] = "You are not authorized to perform this action."
+    redirect_to(request.referrer || root_path)
+  end
 end

app/controllers/chapter_leaderships_controller.rb

@@ -1,7 +1,7 @@
 class ChapterLeadershipsController < ApplicationController
   before_action :authenticate_user!
   before_action :load_chapter
-  before_action :validate_chapter_leader!
+  before_action :validate_authorized!
 
   def index
     @users = User.all
@@ -36,4 +36,8 @@ def load_chapter
   def leader_params
     params.permit(:id, :chapter_id)
   end
+
+  def validate_authorized!
+    authorize @chapter, :modify_leadership?
+  end
 end

app/controllers/chapters_controller.rb

@@ -1,8 +1,6 @@
 class ChaptersController < ApplicationController
   before_action :authenticate_user!, except: [:show, :index]
   before_action :assign_chapter, except: [:index, :new, :create]
-  before_action :validate_chapter_leader!, only: [:edit]
-  before_action :validate_admin!, only: [:new, :create, :destroy]
 
   def index
     @chapters = Chapter.all
@@ -25,13 +23,16 @@ def show
   end
 
   def new
+    authorize Chapter
     @chapter = Chapter.new
   end
 
   def edit
+    authorize @chapter
   end
 
   def create
+    authorize Chapter
     @chapter = Chapter.new(chapter_params)
 
     if @chapter.save
@@ -42,6 +43,7 @@ def create
   end
 
   def update
+    authorize @chapter
     if @chapter.update_attributes(chapter_params)
       redirect_to @chapter, notice: 'Chapter was successfully updated.'
     else
@@ -50,6 +52,7 @@ def update
   end
 
   def destroy
+    authorize @chapter
     unless @chapter.destroyable?
       return redirect_to root_url, alert: "Can't delete a chapter that's still assigned to an event or external event."
     end

app/controllers/region_leaderships_controller.rb

@@ -1,7 +1,7 @@
 class RegionLeadershipsController < ApplicationController
   before_action :authenticate_user!
   before_action :load_region
-  before_action :validate_region_leader!
+  before_action :validate_authorized!
 
   def index
     @users = @region.users
@@ -36,4 +36,8 @@ def load_region
   def leader_params
     params.permit(:id, :region_id)
   end
+
+  def validate_authorized!
+    authorize @region, :modify_leadership?
+  end
 end

app/controllers/regions_controller.rb

@@ -1,7 +1,6 @@
 class RegionsController < ApplicationController
   before_action :authenticate_user!, except: [:show, :index]
   before_action :assign_region, only: [:show, :edit, :update, :destroy]
-  before_action :validate_region_leader!, only: [:edit, :update]
 
   def index
     @regions = Region.includes(:locations, :leaders).all
@@ -28,6 +27,7 @@ def new
   end
 
   def edit
+    authorize @region
   end
 
   def create
@@ -42,6 +42,7 @@ def create
   end
 
   def update
+    authorize @region
     if @region.update_attributes(region_params)
       redirect_to @region, notice: 'Region was successfully updated.'
     else

app/policies/application_policy.rb

@@ -0,0 +1,8 @@
+class ApplicationPolicy
+  attr_reader :user, :record
+
+  def initialize(user, record)
+    @user = user
+    @record = record
+  end
+end

app/policies/chapter_policy.rb

@@ -0,0 +1,25 @@
+class ChapterPolicy < ApplicationPolicy
+  def new?
+    user.admin?
+  end
+
+  def edit?
+    record.has_leader?(user)
+  end
+
+  def update?
+    record.has_leader?(user)
+  end
+
+  def create?
+    user.admin?
+  end
+
+  def destroy?
+    user.admin?
+  end
+
+  def modify_leadership?
+    record.has_leader?(user)
+  end
+end

app/policies/event_policy.rb

@@ -0,0 +1,3 @@
+class EventPolicy < ApplicationPolicy
+
+end

app/policies/region_policy.rb

@@ -0,0 +1,13 @@
+class RegionPolicy < ApplicationPolicy
+  def edit?
+    record.has_leader?(user)
+  end
+
+  def update?
+    record.has_leader?(user)
+  end
+
+  def modify_leadership?
+    record.has_leader?(user)
+  end
+end

spec/controllers/region_leaderships_controller_spec.rb

@@ -27,15 +27,15 @@
     it "can not edit, create, or delete an event organizer" do
       expect(
         get :index, region_id: region.id
-      ).to redirect_to(events_path)
+      ).to be_redirect
 
       expect(
         post :create, region_id: region.id, event_organizer: {region_id: region.id, user_id: leader.id}
-      ).to redirect_to(events_path)
+      ).to be_redirect
 
       expect(
         delete :destroy, region_id: region.id, id: 12345
-      ).to redirect_to(events_path)
+      ).to be_redirect
     end
   end