Consolidate authorization through ActionPolicy and enforce destroy preconditions

app/controllers/application_controller.rb

@@ -1,12 +1,10 @@
 class ApplicationController < ActionController::Base
-  prepend ActionPolicy::Draper
-
   before_action :authenticate_user!  # ensures only logged-in users can access pages
   before_action :set_current_user # for AhoyTrackable in models
   before_action :preload_current_user_associations
 
   before_action do
-    if current_user && current_user.super_user
+    if current_user && allowed_to?(:manage?, with: ApplicationPolicy)
       Rack::MiniProfiler.authorize_request
     end
   end

app/controllers/events_controller.rb

@@ -24,9 +24,6 @@ def new
   def edit
     authorize! @event
     set_form_variables
-    unless @event.created_by == current_user || current_user&.super_user?
-      redirect_to events_path, alert: "You are not authorized to edit this event."
-    end
   end
 
   def create

app/controllers/monthly_reports_controller.rb

@@ -109,11 +109,7 @@ def show
     @answers      = @monthly_report.report_form_field_answers
 
     if @monthly_report
-      if current_user&.super_user? || (@monthly_report.organization && current_user.organization_ids.include?(@monthly_report.organization.id))
-        render :show
-      else
-        redirect_to root_path, error: "You do not have permission to view this page."
-      end
+      render :show
     else
       redirect_to root_path, error: "Unable to find that Workshop Log."
     end

app/controllers/people_controller.rb

@@ -109,15 +109,9 @@ def set_form_variables
     @all_sectors = Sector.published.order(:name)
     @current_sector_ids = @person.sectorable_items.pluck(:sector_id)
 
-    organizations = if current_user&.super_user?
-      Organization.active
-    else
-      current_user.organizations
-    end
-    @organizations_array = organizations.order(:name).pluck(:name, :id)
+    @organizations_array = authorized_scope(Organization.all, as: :affiliated).order(:name).pluck(:name, :id)
   end
 
-
   # Only allow a list of trusted parameters through.
   def person_params
     params.require(:person).permit(

app/controllers/quotes_controller.rb

@@ -58,8 +58,7 @@ def destroy
 
   # Optional hooks for setting variables for forms or index
   def set_form_variables
-    workshops = current_user&.super_user? ? Workshop.all : Workshop.active
-    @workshops = workshops.order(:title)
+    @workshops = authorized_scope(Workshop.all).order(:title)
   end
 
   private

app/controllers/workshop_logs_controller.rb

@@ -112,7 +112,7 @@ def set_index_variables # needs to not be private
       Arel.sql("DISTINCT EXTRACT(YEAR FROM COALESCE(date, created_at, NOW()))")
     ).sort.reverse
 
-    scoped_users = current_user&.super_user? ? User.active : User.where(id: current_user.id)
+    scoped_users = authorized_scope(User.all, as: :colleagues)
     @people = scoped_users.or(User.where(id: @workshop_logs_unpaginated.pluck(:user_id)))
                                 .includes(:workshop_logs, :person)
                                 .joins(:workshop_logs)
@@ -136,11 +136,9 @@ def set_form_variables
       @workshop = Workshop.new
     end
 
-    workshops = Workshop.includes(:windows_type)
-    unless current_user&.super_user?
-      workshops = workshops.published
-    end
-    @workshops = workshops.or(Workshop.where(id: @workshop_log.workshop_id).includes(:windows_type))
+    workshops = authorized_scope(Workshop.all)
+    @workshops = workshops.or(Workshop.where(id: @workshop_log.workshop_id))
+                          .includes(:windows_type)
                           .distinct
                           .order(title: :asc)
 

app/controllers/workshop_variation_ideas_controller.rb

@@ -1,4 +1,5 @@
 class WorkshopVariationIdeasController < ApplicationController
+  include AhoyTracking
   before_action :set_workshop_variation_idea, only: [ :show, :edit, :update, :destroy ]
 
   def index
@@ -13,6 +14,14 @@ def index
 
   def show
     authorize! @workshop_variation_idea
+    track_view(@workshop_variation_idea)
+
+    @workshop = (@workshop_variation_idea.workshop || Workshop.where(id: params[:workshop_id]).last)&.decorate
+    @bookmark = current_user.bookmarks.find_by(bookmarkable: @workshop)
+    @new_bookmark = @workshop.bookmarks.build
+    @quotes = @workshop.quotes
+    @workshop_variation_ideas = WorkshopVariationIdea.where(workshop: @workshop)
+    @sectors = @workshop.sectors
   end
 
   def new
@@ -69,7 +78,7 @@ def destroy
   private
 
   def set_workshop_variation_idea
-    @workshop_variation_idea = WorkshopVariationIdea.find(params[:id])
+    @workshop_variation_idea = WorkshopVariationIdea.find(params[:id]).decorate
   end
 
   def set_form_variables

app/helpers/title_display_helper.rb

@@ -1,6 +1,6 @@
 module TitleDisplayHelper
   def title_with_badges(record, font_size: "text-lg", record_title: nil,
-                        show_hidden_badge: false, display_windows_type: false)
+                        show_hidden_badge: true, display_windows_type: false)
     fragments = []
 
     # --- Hidden badge ---

app/models/organization.rb

@@ -9,14 +9,15 @@ class Organization < ApplicationRecord
   has_many :organization_people, dependent: :restrict_with_error
   has_many :people, through: :organization_people
   has_many :users, through: :people
-  has_many :reports, through: :users
-  has_many :workshop_logs, through: :users
+  has_many :reports
+  has_many :workshop_logs
 
   has_many :categorizable_items, dependent: :destroy, inverse_of: :categorizable, as: :categorizable
   has_many :sectorable_items, as: :sectorable, dependent: :destroy
   # has_many through
   has_many :categories, through: :categorizable_items
   has_many :sectors, through: :sectorable_items
+  has_many :workshops, through: :users
 
   # Asset associations
   has_one_attached :logo, dependent: :purge do |attachable|
@@ -82,6 +83,13 @@ def self.search_by_params(params)
     organizations
   end
 
+  def affiliated_workshop_logs
+    direct = WorkshopLog.where(organization_id: id)
+    legacy = WorkshopLog.where(organization_id: nil)
+                        .where(user_id: users.select(:id))
+    direct.or(legacy).distinct
+  end
+
   # Methods
   def led_by?(user)
     return false unless leader

app/models/report.rb

@@ -69,6 +69,7 @@ class Report < ApplicationRecord
       where("EXTRACT(YEAR FROM COALESCE(reports.date, reports.created_at)) = ?", year.to_i)
     end }
   scope :ordered_by_date, -> { order(Arel.sql("COALESCE(reports.date, reports.created_at) DESC")) }
+  scope :workshop_logs, -> { where(type: "WorkshopLog") }
 
   def self.search(params)
     logs = is_a?(ActiveRecord::Relation) ? self : all

app/models/user.rb

@@ -73,13 +73,6 @@ def self.search_by_params(params)
     results
   end
 
-  # TODO Remove once all view's use ActionPolicy
-  def admin?
-    super_user
-  end
-
-
-
   def active_for_authentication?
     super && !inactive?
   end

app/models/workshop.rb

@@ -50,10 +50,12 @@ def self.mentionable_rich_text_fields
            source: :category # needs to be after has_many :categorizable_items
   has_many :categories, through: :categorizable_items
   has_many :category_types, through: :categories
+  has_many :organizations, through: :user
   has_many :quotes, through: :quotable_item_quotes
   has_many :resources, through: :workshop_resources, source: :resource
   has_many :sectors, through: :sectorable_items
 
+
   # Images
   has_one_attached :thumbnail # old paperclip -- TODO convert these to AvatarImage records
   has_one_attached :header # old paperclip -- TODO convert these to MainImage records
@@ -138,7 +140,7 @@ def self.mentionable_rich_text_fields
   scope :created_by_id, ->(created_by_id) { where(user_id: created_by_id) }
   scope :legacy, -> { where(legacy: true) }
   scope :title, ->(title) { where("workshops.title like ?", "%#{ title }%") }
-  scope :order_by_date, ->(sort_order = "asc") {
+  scope :order_by_date, ->(sort_order = "asc") do
     order(Arel.sql(<<~SQL.squish))
     COALESCE(
       STR_TO_DATE(
@@ -148,14 +150,14 @@ def self.mentionable_rich_text_fields
       DATE(workshops.created_at)
     ) #{sort_order == "asc" ? "ASC" : "DESC"}
     SQL
-  }
+  end
   scope :title, ->(title) { where("workshops.title like ?", "%#{ title }%") }
   scope :windows_type_ids, ->(windows_type_ids) { where(windows_type_id: windows_type_ids) }
-  scope :with_bookmarks_count, -> {
+  scope :with_bookmarks_count, -> do
     left_joins(:bookmarks)
       .select("workshops.*, COUNT(bookmarks.id) AS bookmarks_count")
       .group("workshops.id")
-  }
+  end
 
   # Search Cop
   include SearchCop

app/policies/application_policy.rb

@@ -4,12 +4,16 @@ class ApplicationPolicy < ActionPolicy::Base
   authorize :user, optional: true, allow_nil: true
 
   default_rule :manage?
-  alias_rule :index?, :show?, :new?, :create?, :edit?, :update?, :destroy?, to: :manage?
+  alias_rule :index?, :show?, :new?, :create?, :edit?, :update?, to: :manage?
 
   def manage?
     admin?
   end
 
+  def destroy?
+    record.persisted? && manage?
+  end
+
   private
   # Define shared methods useful for most policies.
 

app/policies/bookmark_policy.rb

@@ -22,7 +22,7 @@ def update?
   end
 
   def destroy?
-    admin? || owner?
+    record.persisted? && (admin? || owner?)
   end
 
   # Scoping

app/policies/category_policy.rb

@@ -7,7 +7,7 @@ def index?   = admin?
   def show?    = admin?
   def create?  = admin?
   def update?  = admin?
-  def destroy? = admin?
+  def destroy? = record.persisted? && admin?
 
   def tags_index?
     true

app/policies/category_type_policy.rb

@@ -7,5 +7,5 @@ def index?   = admin?
   def show?    = admin?
   def create?  = admin?
   def update?  = admin?
-  def destroy? = admin?
+  def destroy? = record.persisted? && admin?
 end

app/policies/event_policy.rb

@@ -15,6 +15,21 @@ def register?
     authenticated? && record.published?
   end
 
+  def edit?
+    admin? || owner?
+  end
+
+  def update?
+    admin? || owner?
+  end
+
+  private
+
+  def owner?
+    return false unless authenticated?
+    record.created_by == user
+  end
+
   relation_scope do |relation|
     next relation if admin?
     if authenticated? # logged in users can see events they are registered for even if registration is closed

app/policies/event_registration_policy.rb

@@ -5,7 +5,7 @@ class EventRegistrationPolicy < ApplicationPolicy
 
   def index?   = admin?
   def create? = admin? || owner?
-  def destroy?   = admin? || owner?
+  def destroy? = record.persisted? && (admin? || owner?)
 
 
   relation_scope do |relation|

app/policies/monthly_report_policy.rb

@@ -0,0 +1,32 @@
+class MonthlyReportPolicy < ApplicationPolicy
+  def index?
+    authenticated?
+  end
+
+  def show?
+    admin? || owner? || member?
+  end
+
+  def update?
+    admin? || owner? || member?
+  end
+
+  def destroy?
+    record.persisted? && (admin? || owner? || member?)
+  end
+
+  private
+
+  def member?
+    @member ||= begin
+      return false unless authenticated?
+      return false unless record.organization
+      record.organization.organization_people.exists?(person_id: user.person_id)
+    end
+  end
+
+  relation_scope do |relation|
+    next relation if admin?
+    relation.where(user_id: user.id)
+  end
+end

app/policies/organization_obligation_policy.rb

@@ -5,7 +5,7 @@ def index?   = admin?
   def show?    = admin?
   def create?  = admin?
   def update?  = admin?
-  def destroy? = admin?
+  def destroy? = record.persisted? && admin?
 
 
   # Scoping

app/policies/organization_person_policy.rb

@@ -2,7 +2,7 @@ class OrganizationPersonPolicy < ApplicationPolicy
   # See https://actionpolicy.evilmartians.io/#/writing_policies
   #
   def destroy?
-    admin? # we don't allow users to edit their own
+    record.persisted? && admin? # we don't allow users to edit their own
   end
 
   # Scoping

app/policies/organization_policy.rb

@@ -9,11 +9,33 @@ def show?
     admin? || (authenticated? && record.published?)
   end
 
+  def show_workshop_logs?
+    admin? || member?
+  end
+
+
   # Scoping
   # See https://actionpolicy.evilmartians.io/#/scoping
 
   relation_scope do |relation|
     next relation if admin?
     relation.published
   end
+
+  relation_scope(:affiliated) do |relation|
+    next relation.active if admin?
+    next relation.none unless user&.person_id
+
+    relation.joins(:organization_people)
+            .where(organization_people: { person_id: user.person_id })
+  end
+
+  private
+
+  def member?
+    @member ||= begin
+      return false unless user&.person_id
+      record.organization_people.exists?(person_id: user.person_id)
+    end
+  end
 end

app/policies/organization_status_policy.rb

@@ -5,7 +5,7 @@ def index?   = admin?
   def show?    = admin?
   def create?  = admin?
   def update?  = admin?
-  def destroy? = admin?
+  def destroy? = record.persisted? && admin?
 
   #
   # Scoping

app/policies/primary_asset_policy.rb

@@ -8,5 +8,5 @@ def show?    = admin?
   def create?  = authenticated?
   def edit?    = admin?
   def update?  = authenticated?
-  def destroy? = admin?
+  def destroy? = record.persisted? && admin?
 end