Merge pull request #1940 from manyfold3d/verify-authorised

Add permission verification onto all actions

app/controllers/application_controller.rb

@@ -1,11 +1,17 @@
 class ApplicationController < ActionController::Base
   include Pundit::Authorization
+  after_action :verify_authorized, except: :index, unless: :active_admin_controller?
+  after_action :verify_policy_scoped, only: :index, unless: :active_admin_controller?
 
   before_action :auto_login_single_user
   before_action :authenticate_user!
   before_action :check_scan_status
   before_action :remember_ordering
 
+  def index
+    raise NotImplementedError
+  end
+
   def auto_login_single_user
     sign_in(:user, User.first) unless Flipper.enabled? :multiuser
   end
@@ -23,4 +29,8 @@ def remember_ordering
     session["order"] ||= "name"
     session["order"] = params["order"] if params["order"]
   end
+
+  def active_admin_controller?
+    is_a?(ActiveAdmin::BaseController)
+  end
 end

app/controllers/collections_controller.rb

@@ -3,15 +3,15 @@ class CollectionsController < ApplicationController
   before_action :get_collection, except: [:index, :new, :create]
 
   def index
+    @collections = policy_scope(Collection)
     if @filters.empty?
-      @collections = Collection.all
       @commontags = @tags = ActsAsTaggableOn::Tag.all
     else
       process_filters_init
       process_filters_tags_fetchall
       process_filters
       process_filters_tags_highlight
-      @collections = Collection.tree_both(@filters[:collection] || nil, @models.filter_map { |model| model.collection_id })
+      @collections = @collections.tree_both(@filters[:collection] || nil, @models.filter_map { |model| model.collection_id })
     end
 
     # Ordering
@@ -34,6 +34,7 @@ def show
   end
 
   def new
+    authorize Collection
     @collection = Collection.new
     @collection.links.build if @collection.links.empty? # populate empty link
     @title = t("collections.general.new")
@@ -46,6 +47,7 @@ def edit
   end
 
   def create
+    authorize Collection
     @collection = Collection.create(collection_params)
     redirect_to collections_path, notice: t(".success")
   end
@@ -65,9 +67,11 @@ def destroy
   def get_collection
     if params[:id] == "0"
       @collection = nil
+      authorize Collection
       @title = t(".unknown")
     else
       @collection = Collection.find(params[:id])
+      authorize @collection
       @title = @collection.name
     end
   end

app/controllers/concerns/model_filters.rb

@@ -10,7 +10,7 @@ def get_filters
   end
 
   def process_filters_init
-    @models = Model.includes(:tags, :preview_file, :creator, :collection)
+    @models = policy_scope(Model).includes(:tags, :preview_file, :creator, :collection)
   end
 
   def process_filters_tags_fetchall

app/controllers/creators_controller.rb

@@ -3,15 +3,15 @@ class CreatorsController < ApplicationController
   before_action :get_creator, except: [:index, :new, :create]
 
   def index
+    @creators = policy_scope(Creator)
     if @filters.empty?
-      @creators = Creator.all
       @commontags = @tags = ActsAsTaggableOn::Tag.all
     else
       process_filters_init
       process_filters_tags_fetchall
       process_filters
       process_filters_tags_highlight
-      @creators = Creator.where(id: @models.map { |model| model.creator_id })
+      @creators = @creators.where(id: @models.map { |model| model.creator_id })
     end
 
     # Ordering
@@ -34,6 +34,7 @@ def show
   end
 
   def new
+    authorize Creator
     @creator = Creator.new
     @creator.links.build if @creator.links.empty? # populate empty link
     @title = t("creators.general.new")
@@ -44,13 +45,14 @@ def edit
   end
 
   def create
+    authorize Creator
     @creator = Creator.create(creator_params)
     redirect_to creators_path, notice: t(".success")
   end
 
   def update
     @creator.update(creator_params)
-    redirect_to creators_path, notice: t(".success")
+    redirect_to @creator, notice: t(".success")
   end
 
   def destroy
@@ -63,9 +65,11 @@ def destroy
   def get_creator
     if params[:id] == "0"
       @creator = nil
+      authorize Creator
       @title = t(".unknown")
     else
       @creator = Creator.find(params[:id])
+      authorize @creator
       @title = @creator.name
     end
   end

app/controllers/home_controller.rb

@@ -3,7 +3,7 @@ class HomeController < ApplicationController
   before_action :check_for_first_use
 
   def index
-    @recent = Model.recent.limit(20)
+    @recent = policy_scope(Model).recent.limit(20)
   end
 
   private

app/controllers/libraries_controller.rb

@@ -1,7 +1,10 @@
 class LibrariesController < ApplicationController
   before_action :get_library, except: [:index, :new, :create, :scan_all]
+  after_action :verify_authorized, only: [:index]
+  skip_after_action :verify_policy_scoped, only: [:index]
 
   def index
+    authorize Library
     if Library.count === 0
       redirect_to new_library_path
     else
@@ -10,21 +13,18 @@ def index
   end
 
   def show
-    redirect_to models_path(library: params[:id])
+    redirect_to models_path(library: @library.id)
   end
 
   def new
     @library = Library.new
     @title = t("libraries.general.new")
-    authorize @library
   end
 
   def edit
-    authorize @library
   end
 
   def create
-    authorize Library
     @library = Library.create(library_params)
     @library.tag_regex = params[:tag_regex]
     if @library.valid?
@@ -37,9 +37,8 @@ def create
   end
 
   def update
-    authorize @library
     @library.update(library_params)
-    uptags = library_params[:tag_regex].reject(&:empty?)
+    uptags = library_params[:tag_regex]&.reject(&:empty?)
     @library.tag_regex = uptags
     if @library.save
       redirect_to models_path, notice: t(".success")
@@ -66,7 +65,6 @@ def scan_all
   end
 
   def destroy
-    authorize @library
     @library.destroy
     redirect_to libraries_path, notice: t(".success")
   end
@@ -79,6 +77,7 @@ def library_params
 
   def get_library
     @library = Library.find(params[:id])
+    authorize @library
     @title = @library.name
   end
 end

app/controllers/model_files_controller.rb

@@ -5,6 +5,9 @@ class ModelFilesController < ApplicationController
   before_action :get_model
   before_action :get_file, except: [:bulk_edit, :bulk_update]
 
+  skip_after_action :verify_authorized, only: [:bulk_edit, :bulk_update]
+  after_action :verify_policy_scoped, only: [:bulk_edit, :bulk_update]
+
   def show
     if stale?(@file)
       @duplicates = @file.duplicates
@@ -51,7 +54,7 @@ def bulk_update
   def destroy
     authorize @file
     @file.delete_from_disk_and_destroy
-    if URI.parse(request.referer).path == library_model_model_file_path(@library, @model, @file)
+    if request.referer && (URI.parse(request.referer).path == library_model_model_file_path(@library, @model, @file))
       # If we're coming from the file page itself, we can't go back there
       redirect_to library_model_path(@library, @model), notice: t(".success")
     else
@@ -96,6 +99,7 @@ def get_model
 
   def get_file
     @file = @model.model_files.find(params[:id])
+    authorize @file
     @title = @file.name
   end
 end

app/controllers/models_controller.rb

@@ -4,6 +4,8 @@ class ModelsController < ApplicationController
   include ModelFilters
   before_action :get_library, except: [:index, :bulk_edit, :bulk_update]
   before_action :get_model, except: [:bulk_edit, :bulk_update, :index]
+  skip_after_action :verify_authorized, only: [:bulk_edit, :bulk_update]
+  after_action :verify_policy_scoped, only: [:bulk_edit, :bulk_update]
 
   def index
     process_filters_init
@@ -66,14 +68,14 @@ def merge
       Scan::CheckModelIntegrityJob.perform_later(@model.id)
       redirect_to [@library, @model], notice: t(".success")
     else
-      render status: :bad_request
+      head :bad_request
     end
   end
 
   def bulk_edit
-    @creators = Creator.all
-    @collections = Collection.all
-    @models = Model.all
+    @creators = policy_scope(Creator)
+    @collections = policy_scope(Collection)
+    @models = policy_scope(Model)
     process_filters
   end
 
@@ -86,8 +88,8 @@ def bulk_update
 
     params[:models].each_pair do |id, selected|
       if selected == "1"
-        model = Model.find(id)
-        if model.update(hash)
+        model = policy_scope(Model).find(id)
+        if model&.update(hash)
           existing_tags = Set.new(model.tag_list)
           model.tag_list = existing_tags + add_tags - remove_tags
           model.save
@@ -98,9 +100,8 @@ def bulk_update
   end
 
   def destroy
-    authorize @model
     @model.delete_from_disk_and_destroy
-    if URI.parse(request.referer).path == library_model_path(@library, @model)
+    if request.referer && (URI.parse(request.referer).path == library_model_path(@library, @model))
       # If we're coming from the model page itself, we can't go back there
       redirect_to library_path(@library), notice: t(".success")
     else
@@ -149,6 +150,7 @@ def get_library
 
   def get_model
     @model = Model.includes(:model_files, :creator).find(params[:id])
+    authorize @model
     @title = @model.name
   end
 end

app/controllers/problems_controller.rb

@@ -2,7 +2,7 @@ class ProblemsController < ApplicationController
   def index
     # Are we showing ignored problems?
     @show_ignored = (params[:show_ignored] == "true")
-    query = @show_ignored ? Problem.unscoped : Problem
+    query = @show_ignored ? policy_scope(Problem.unscoped) : policy_scope(Problem)
     # Now, which page are we on?
     page = params[:page] || 1
     # What categories are we showing?
@@ -28,6 +28,7 @@ def index
 
   def update
     @problem = Problem.unscoped.find(params[:id])
+    authorize @problem
     @problem.update!(permitted_params)
     notice = t(
       (@problem.ignored ? ".ignored" : ".unignored"),

app/controllers/settings_controller.rb

@@ -88,6 +88,7 @@ def update_tagging_settings(settings)
 
   def get_user
     @user = User.find_by(username: params[:user_id])
+    authorize @user
   end
 
   def check_owner_permission

app/controllers/uploads_controller.rb

@@ -1,6 +1,12 @@
 class UploadsController < ApplicationController
   before_action { authorize :upload }
 
+  after_action :verify_authorized
+  skip_after_action :verify_policy_scoped, only: :index
+
+  def index
+  end
+
   def create
     library = Library.find(params[:post][:library_pick])
     save_files(params[:upload], File.join(library.path, ""))

app/policies/active_admin/page_policy.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class ActiveAdmin::PagePolicy < ApplicationPolicy
+  def show?
+    user.admin?
+  end
+end

app/policies/active_admin_policy.rb → app/policies/acts_as_taggable_on/tag_policy.rb

@@ -1,10 +1,6 @@
-class ActiveAdminPolicy < ApplicationPolicy
-  def initialize(user, record)
-    raise Pundit::NotAuthorizedError, I18n.t("active_admin.demo_mode") if Flipper.enabled? :demo_mode
-    @user = user
-    @record = record
-  end
+# frozen_string_literal: true
 
+class ActsAsTaggableOn::TagPolicy < ApplicationPolicy
   def index?
     true
   end
@@ -25,10 +21,6 @@ def destroy?
     true
   end
 
-  def destroy_all?
-    true
-  end
-
   class Scope
     attr_reader :user, :scope
 
@@ -38,7 +30,7 @@ def initialize(user, scope)
     end
 
     def resolve
-      scope
+      scope.all
     end
   end
 end