Merge pull request catarse#520 from diogob/migrate_cancan_to_pundit

Migrate cancan to pundit [open review]

Gemfile

@@ -49,6 +49,7 @@ gem 'omniauth-twitter'
 gem 'omniauth-facebook', '1.4.0'
 gem 'devise', '~> 3.0.2'
 gem 'ezcrypto'
+gem 'pundit'
 
 # See https://github.com/ryanb/cancan/tree/2.0 for help about this
 # In resume: this version of cancan allow checking for authorization on specific fields on the model

Gemfile.lock

@@ -18,12 +18,12 @@ GIT
 
 GIT
   remote: git://github.com/catarse/moip-ruby.git
-  revision: 39f9dab38fc33fe9bf614deb8e90ca166ab2d6bf
+  revision: c0225ad71645cd1df35dafa1e45c9f092b3abb9e
   specs:
     moip (1.0.2)
       activesupport (>= 2.3.2)
       httparty (~> 0.6.1)
-      nokogiri (~> 1.5.0)
+      nokogiri (>= 1.5.0)
 
 GIT
   remote: git://github.com/ryanb/cancan.git
@@ -166,13 +166,10 @@ GEM
     fakeweb (1.3.0)
     faraday (0.8.8)
       multipart-post (~> 1.2.0)
-    feedzirra (0.0.24)
-      activesupport (>= 2.3.8)
-      builder (>= 2.1.2)
-      curb (>= 0.2.3)
-      loofah (>= 0.3.1)
-      nokogiri (> 0.0.0)
-      sax-machine (>= 0.0.12)
+    feedzirra (0.7.0)
+      curb (~> 0.8.1)
+      loofah (~> 1.2.1)
+      sax-machine (~> 0.2.1)
     ffi (1.9.3)
     fog (1.19.0)
       builder
@@ -240,6 +237,7 @@ GEM
       httparty
     method_source (0.8.2)
     mime-types (1.25.1)
+    mini_portile (0.5.2)
     minitest (4.7.5)
     monetize (0.1.3)
     money (6.0.1)
@@ -251,7 +249,8 @@ GEM
       net-ssh (>= 2.6.5)
     net-ssh (2.7.0)
     newrelic_rpm (3.6.5.130)
-    nokogiri (1.5.11)
+    nokogiri (1.6.1)
+      mini_portile (~> 0.5.0)
     oauth (0.4.7)
     oauth2 (0.8.1)
       faraday (~> 0.8)
@@ -300,6 +299,8 @@ GEM
       coderay (~> 1.0)
       method_source (~> 0.8)
       slop (~> 3.4)
+    pundit (0.2.1)
+      activesupport (>= 3.0.0)
     rack (1.5.2)
     rack-protection (1.5.1)
       rack
@@ -362,8 +363,8 @@ GEM
       railties (>= 4.0.0, < 5.0)
       sass (>= 3.1.10)
       sprockets-rails (~> 2.0.0)
-    sax-machine (0.1.0)
-      nokogiri (> 0.0.0)
+    sax-machine (0.2.1)
+      nokogiri (~> 1.6.0)
     schema_associations (1.2.0)
       schema_plus (>= 1.2.0)
     schema_plus (1.3.2)
@@ -515,6 +516,7 @@ DEPENDENCIES
   postgres-copy
   protected_attributes (~> 1.0.5)
   pry
+  pundit
   rails (= 4.0.2)
   rails-observers (~> 0.1.2)
   rails_12factor

app/controllers/application_controller.rb

@@ -3,6 +3,7 @@ class ApplicationController < ActionController::Base
   include Concerns::ExceptionHandler
   include Concerns::MenuHandler
   include Concerns::SocialHelpersHandler
+  include Pundit
 
   layout :use_catarse_boostrap
   protect_from_forgery

app/controllers/concerns/exception_handler.rb

@@ -7,17 +7,21 @@ module ExceptionHandler
       rescue_from ActionController::UnknownController, with: :render_404
       rescue_from ActiveRecord::RecordNotFound, with: :render_404
 
-      rescue_from CanCan::Unauthorized do |exception|
-        session[:return_to] = request.env['REQUEST_URI']
-        message = exception.message
+      rescue_from Pundit::NotAuthorizedError, with: :auth_error
+      rescue_from CanCan::Unauthorized, with: :auth_error
 
-        if current_user.nil?
-          redirect_to new_user_registration_path, alert: I18n.t('devise.failure.unauthenticated')
-        elsif request.env["HTTP_REFERER"]
-          redirect_to :back, alert: message
-        else
-          redirect_to root_path, alert: message
-        end
+    end
+
+    def auth_error(exception)
+      session[:return_to] = request.env['REQUEST_URI']
+      message = exception.message
+
+      if current_user.nil?
+        redirect_to new_user_registration_path, alert: I18n.t('devise.failure.unauthenticated')
+      elsif request.env["HTTP_REFERER"]
+        redirect_to :back, alert: message
+      else
+        redirect_to root_path, alert: message
       end
     end
 

app/controllers/projects_controller.rb

@@ -1,6 +1,6 @@
 # coding: utf-8
 class ProjectsController < ApplicationController
-  load_and_authorize_resource only: [ :new, :create, :update, :destroy, :send_to_analysis ]
+  after_filter :verify_authorized, except: %i[index video video_embed embed embed_panel]
   inherit_resources
   has_scope :pg_search, :by_category_id, :near_of
   has_scope :recent, :expiring, :successful, :recommended, :not_expired, type: :boolean
@@ -32,30 +32,38 @@ def index
   end
 
   def new
-    new! do
-      @title = t('projects.new.title')
-      @project.rewards.build
-    end
+    @project = Project.new user: current_user
+    authorize @project
+    @title = t('projects.new.title')
+    @project.rewards.build
   end
 
   def create
-    @project = current_user.projects.new(params[:project])
-
+    @project = Project.new params[:project].merge(user: current_user)
+    authorize @project
     create! { project_by_slug_path(@project.permalink) }
   end
 
+  def destroy
+    authorize resource
+    destroy!
+  end
+
   def send_to_analysis
     resource.send_to_analysis
+    authorize @project
     flash[:notice] = t('projects.send_to_analysis')
     redirect_to project_by_slug_path(@project.permalink)
   end
 
   def update
+    authorize resource
     update!(notice: t('projects.update.success')) { project_by_slug_path(@project.permalink, anchor: 'edit') }
   end
 
   def show
     @title = resource.name
+    authorize @project
     fb_admins_add(resource.user.facebook_id) if resource.user.facebook_id
     @updates_count = resource.updates.count
     @update = resource.updates.where(id: params[:update_id]).first if params[:update_id].present?
@@ -81,6 +89,9 @@ def embed_panel
   end
 
   protected
+  def permitted_params
+    params.permit(policy(resource).permitted_attributes)
+  end
 
   def resource
     @project ||= (params[:permalink].present? ? Project.by_permalink(params[:permalink]).first! : Project.find(params[:id]))

app/models/ability.rb

@@ -16,8 +16,6 @@ def initialize(current_user, options = {})
     end
 
     # NOTE: Project authorizations
-    can :create, :projects if current_user.persisted?
-
     can :update, :projects, [:about, :video_url, :uploaded_image, :headline ] do |project|
       project.user == current_user && ( project.online? || project.waiting_funds? || project.successful? || project.failed? )
     end
@@ -26,11 +24,6 @@ def initialize(current_user, options = {})
       project.user == current_user && ( project.draft? || project.rejected? || project.in_analysis? )
     end
 
-    can :send_to_analysis, :projects do |project|
-      project.user == current_user
-    end
-
-
     # NOTE: Reward authorizations
     can :create, :rewards do |reward|
       reward.project.user == current_user

app/models/project.rb

@@ -178,6 +178,10 @@ def should_fail?
     expired? && !reached_goal?
   end
 
+  def state_warning_template
+    "#{state}_warning"
+  end
+
   private
   def self.between_dates(attribute, starts_at, ends_at)
     return scoped unless starts_at.present? && ends_at.present?

app/policies/application_policy.rb

@@ -0,0 +1,58 @@
+class ApplicationPolicy
+  attr_reader :user, :record
+
+  def initialize(user, record)
+    @user = user
+    @record = record
+  end
+
+  def index?
+    true
+  end
+
+  def show?
+    true
+  end
+
+  def create?
+    user.admin? || false
+  end
+
+  def new?
+    create?
+  end
+
+  def update?
+    user.admin? || false
+  end
+
+  def edit?
+    update?
+  end
+
+  def destroy?
+    user.admin? || false
+  end
+
+  def scope
+    Pundit.policy_scope!(user, record.class)
+  end
+
+  def permitted_for?(field, operation)
+    permitted?(field) && send("#{operation}?")
+  end
+
+  def permitted?(field)
+    permitted_attributes.values.first.include? field
+  end
+
+  protected
+  def done_by_onwer_or_admin?
+    is_owned_by?(user) || user.try(:admin?)
+  end
+
+  def is_owned_by?(user)
+    user.present? && record.user == user
+  end
+end
+

app/policies/project_policy.rb

@@ -0,0 +1,22 @@
+class ProjectPolicy < ApplicationPolicy
+  def create?
+    done_by_onwer_or_admin?
+  end
+
+  def update?
+    create?
+  end
+
+  def send_to_analysis?
+    create?
+  end
+
+  def permitted_attributes
+    if user.present? && (user.admin? || (record.draft? || record.rejected? || record.in_analysis?))
+      {project: record.attribute_names.map(&:to_sym)}
+    else
+      {project: [:about, :video_url, :uploaded_image, :headline]}
+    end
+  end
+end
+

app/views/projects/_draft_warning.html.slim

@@ -1,5 +1,5 @@
 br
-- if can? :update, @project
+- if policy(@project).update?
   #project_warning.bootstrap-alert
     .alert.alert-warning
       i Bem-vindo ao Rascunho!

app/views/projects/_failed_warning.html.slim

@@ -1,4 +1,4 @@
-- if can? :update, @project
+- if policy(@project).update?
   br
   #project_warning.bootstrap-alert
     .alert.alert-warning

app/views/projects/_analysis_warning.html.slim → app/views/projects/_in_analysis_warning.html.slim

@@ -1,5 +1,5 @@
 br
-- if can? :update, @project
+- if policy(@project).update?
   #project_warning.bootstrap-alert
     .alert.alert-warning
       i Seu projeto está em análise.

app/views/projects/_online_warning.html.slim

@@ -1,4 +1,4 @@
-- if can? :update, @project
+- if policy(@project).update?
   br
   #project_warning.bootstrap-alert
     .alert.alert-warning

app/views/projects/_project_edit.html.slim

@@ -8,22 +8,22 @@
       - if current_user.admin
         = form.input :user_id
         = form.input :permalink, as: :string
-      - if can? :update, @project, :name
+      - if policy(@project).permitted_for?(:name, :update)
         = form.input :name, as: :string, required: false
-      - if can? :update, @project, :headline
+      - if policy(@project).permitted_for?(:headline, :update)
         = form.input :headline, as: :string, required: false
-      - if can? :update, @project, :category
+      - if policy(@project).permitted_for?(:category_id, :update)
         = form.input :category, as: :select, required: false
-      - if can? :update, @project, :video_url
+      - if policy(@project).permitted_for?(:video_url, :update)
         = form.input :video_url, as: :string, hint: t('formtastic.hints.project.video_url_edit'),required: false
-      - if can? :update, @project, :uploaded_image
+      - if policy(@project).permitted_for?(:uploaded_image, :update)
         = form.input :uploaded_image, as: :file, required: false,
           hint: (@project.uploaded_image.present? ? image_tag(@project.uploaded_image.project_thumb.url, size: '220x172') : t('formtastic.hints.project.uploaded_image') )
-      - if can? :update, @project, :online_days
+      - if policy(@project).permitted_for?(:online_days, :update)
         = form.input :online_days, as: :number, required: false
-      - if can? :update, @project, :goal
+      - if policy(@project).permitted_for?(:goal, :update)
         = form.input :goal, as: :number, required: false, hint: t('formtastic.hints.project.goal').html_safe
-      - if can? :update, @project, :about
+      - if policy(@project).permitted_for?(:about, :update)
         = form.input :about, required: false
         = render 'formatting_tips'
 

app/views/projects/_successful_warning.html.slim

@@ -1,4 +1,4 @@
-- if can? :update, @project
+- if policy(@project).update?
   br
   #project_warning.bootstrap-alert
     .alert.alert-warning

app/views/projects/_waiting_funds_warning.html.slim

@@ -1,4 +1,4 @@
-- if can? :udpate, @project
+- if policy(@project).update?
   br
   #project_warning.bootstrap-alert
     .alert.alert-warning