From 5cb642ae130350ad04188b75dbd6ed49cf536457 Mon Sep 17 00:00:00 2001 From: James Smith Date: Wed, 6 Mar 2024 14:36:37 +0000 Subject: [PATCH 01/34] ensure that pundit policies are being used --- app/controllers/application_controller.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 7530ddfd1..abd39b4d0 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,11 +1,17 @@ class ApplicationController < ActionController::Base include Pundit::Authorization + after_action :verify_authorized, except: :index + after_action :verify_policy_scoped, only: :index 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 From 106996168be1d3f6bac2d8a00d92128928379fdf Mon Sep 17 00:00:00 2001 From: James Smith Date: Wed, 6 Mar 2024 14:38:48 +0000 Subject: [PATCH 02/34] add policy for collections/index --- app/controllers/collections_controller.rb | 4 ++-- app/policies/collection_policy.rb | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 app/policies/collection_policy.rb diff --git a/app/controllers/collections_controller.rb b/app/controllers/collections_controller.rb index 738c85077..69496ea8c 100644 --- a/app/controllers/collections_controller.rb +++ b/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 diff --git a/app/policies/collection_policy.rb b/app/policies/collection_policy.rb new file mode 100644 index 000000000..856aab9d3 --- /dev/null +++ b/app/policies/collection_policy.rb @@ -0,0 +1,7 @@ +class CollectionPolicy < ApplicationPolicy + class Scope < ApplicationPolicy::Scope + def resolve + scope.all + end + end +end From df4c48c596da13a8ca93004930a68f29dfdf127f Mon Sep 17 00:00:00 2001 From: James Smith Date: Wed, 6 Mar 2024 14:40:30 +0000 Subject: [PATCH 03/34] add creators#index policy --- app/controllers/creators_controller.rb | 4 ++-- app/policies/creator_policy.rb | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 app/policies/creator_policy.rb diff --git a/app/controllers/creators_controller.rb b/app/controllers/creators_controller.rb index 8f982d3ba..e00bdbcdc 100644 --- a/app/controllers/creators_controller.rb +++ b/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 diff --git a/app/policies/creator_policy.rb b/app/policies/creator_policy.rb new file mode 100644 index 000000000..25a8cb954 --- /dev/null +++ b/app/policies/creator_policy.rb @@ -0,0 +1,7 @@ +class CreatorPolicy < ApplicationPolicy + class Scope < ApplicationPolicy::Scope + def resolve + scope.all + end + end +end From 343b33d0ba3c613b1ebd8071d53dea8987eca4fd Mon Sep 17 00:00:00 2001 From: James Smith Date: Wed, 6 Mar 2024 21:58:04 +0000 Subject: [PATCH 04/34] use instance variable for library in show --- app/controllers/libraries_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/libraries_controller.rb b/app/controllers/libraries_controller.rb index d484b09b8..3a2200596 100644 --- a/app/controllers/libraries_controller.rb +++ b/app/controllers/libraries_controller.rb @@ -10,7 +10,7 @@ def index end def show - redirect_to models_path(library: params[:id]) + redirect_to models_path(library: @library.id) end def new From 82f8175f0473e1d62fe90248587d0c34f0bccd44 Mon Sep 17 00:00:00 2001 From: James Smith Date: Wed, 6 Mar 2024 22:02:57 +0000 Subject: [PATCH 05/34] add policy for library show and index actions --- app/controllers/libraries_controller.rb | 9 ++++----- app/policies/library_policy.rb | 8 ++++++++ spec/requests/libraries_spec.rb | 4 ++-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/app/controllers/libraries_controller.rb b/app/controllers/libraries_controller.rb index 3a2200596..60f8901fc 100644 --- a/app/controllers/libraries_controller.rb +++ b/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 @@ -16,15 +19,12 @@ def show 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,7 +37,6 @@ def create end def update - authorize @library @library.update(library_params) uptags = library_params[:tag_regex].reject(&:empty?) @library.tag_regex = uptags @@ -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 diff --git a/app/policies/library_policy.rb b/app/policies/library_policy.rb index af44653cb..13d6ebd00 100644 --- a/app/policies/library_policy.rb +++ b/app/policies/library_policy.rb @@ -1,4 +1,12 @@ class LibraryPolicy < ApplicationPolicy + def index? + true + end + + def show? + true + end + def create? !Flipper.enabled? :demo_mode end diff --git a/spec/requests/libraries_spec.rb b/spec/requests/libraries_spec.rb index 2f0e68589..2f11d4b38 100644 --- a/spec/requests/libraries_spec.rb +++ b/spec/requests/libraries_spec.rb @@ -35,7 +35,7 @@ describe "GET /libraries" do it "redirects to models index" do get "/libraries" - expect(response).to have_http_status(:redirect) + expect(response).to redirect_to("/models") end end @@ -54,7 +54,7 @@ describe "GET /libraries/:id" do it "redirects to models index with library filter" do get "/libraries/1" - expect(response).to have_http_status(:redirect) + expect(response).to redirect_to("/models?library=1") end end From a24d544e2d2a90fbcd5ab1f1ae1d20bdb63bab64 Mon Sep 17 00:00:00 2001 From: James Smith Date: Wed, 6 Mar 2024 22:14:44 +0000 Subject: [PATCH 06/34] authorize models using pundit --- app/controllers/concerns/model_filters.rb | 2 +- app/controllers/models_controller.rb | 15 +++++++++------ app/policies/model_policy.rb | 14 ++++++++++++++ 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/app/controllers/concerns/model_filters.rb b/app/controllers/concerns/model_filters.rb index 532acfe97..c559b99b5 100644 --- a/app/controllers/concerns/model_filters.rb +++ b/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 diff --git a/app/controllers/models_controller.rb b/app/controllers/models_controller.rb index de380f92a..2fcc2394f 100644 --- a/app/controllers/models_controller.rb +++ b/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 @@ -71,9 +73,9 @@ def merge 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,7 +100,6 @@ 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 we're coming from the model page itself, we can't go back there @@ -145,10 +146,12 @@ def model_params def get_library @library = Model.find(params[:id]).library + authorize @library end def get_model @model = Model.includes(:model_files, :creator).find(params[:id]) + authorize @model @title = @model.name end end diff --git a/app/policies/model_policy.rb b/app/policies/model_policy.rb index ab3fc1774..9ce304d4c 100644 --- a/app/policies/model_policy.rb +++ b/app/policies/model_policy.rb @@ -1,4 +1,18 @@ class ModelPolicy < ApplicationPolicy + class Scope < ApplicationPolicy::Scope + def resolve + scope.all + end + end + + def show? + true + end + + def update? + true + end + def destroy? !Flipper.enabled? :demo_mode end From 6a1c52e056a9b010ba8dc763cd72730375a9e8af Mon Sep 17 00:00:00 2001 From: James Smith Date: Wed, 6 Mar 2024 22:17:50 +0000 Subject: [PATCH 07/34] add policy scope for problems --- app/controllers/problems_controller.rb | 3 ++- app/policies/problem_policy.rb | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 app/policies/problem_policy.rb diff --git a/app/controllers/problems_controller.rb b/app/controllers/problems_controller.rb index c7f1582bc..f4ce1bf31 100644 --- a/app/controllers/problems_controller.rb +++ b/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"), diff --git a/app/policies/problem_policy.rb b/app/policies/problem_policy.rb new file mode 100644 index 000000000..5d489e3fe --- /dev/null +++ b/app/policies/problem_policy.rb @@ -0,0 +1,11 @@ +class ProblemPolicy < ApplicationPolicy + class Scope < ApplicationPolicy::Scope + def resolve + scope.all + end + end + + def update? + true + end +end From 38c668d5ab4538640f57bd5d433458fb5dcc3ea8 Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 12:08:10 +0000 Subject: [PATCH 08/34] get model file tests passing again --- app/controllers/model_files_controller.rb | 6 ++++++ app/policies/model_file_policy.rb | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/app/controllers/model_files_controller.rb b/app/controllers/model_files_controller.rb index b530bd28c..2e8684418 100644 --- a/app/controllers/model_files_controller.rb +++ b/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 @@ -88,14 +91,17 @@ def file_params def get_library @library = Library.find(params[:library_id]) + authorize @library end def get_model @model = @library.models.find(params[:model_id]) + authorize @model end def get_file @file = @model.model_files.find(params[:id]) + authorize @file @title = @file.name end end diff --git a/app/policies/model_file_policy.rb b/app/policies/model_file_policy.rb index f0e7b589c..d91dc050e 100644 --- a/app/policies/model_file_policy.rb +++ b/app/policies/model_file_policy.rb @@ -1,4 +1,8 @@ class ModelFilePolicy < ApplicationPolicy + def show? + true + end + def destroy? !Flipper.enabled? :demo_mode end From 16361e821c2d3d7568e7ebdc2c7ccdb3507b411c Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 12:10:11 +0000 Subject: [PATCH 09/34] scope models on homepage --- app/controllers/home_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index 528b9f91a..a237adf59 100644 --- a/app/controllers/home_controller.rb +++ b/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 From e4899d2e5a3c67e47f4fe950d3287cfb96d390cd Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 14:45:43 +0000 Subject: [PATCH 10/34] remove the specific activeadmin access policy --- app/policies/active_admin_policy.rb | 44 ----------------------------- config/initializers/active_admin.rb | 2 +- 2 files changed, 1 insertion(+), 45 deletions(-) delete mode 100644 app/policies/active_admin_policy.rb diff --git a/app/policies/active_admin_policy.rb b/app/policies/active_admin_policy.rb deleted file mode 100644 index 9aa982ad1..000000000 --- a/app/policies/active_admin_policy.rb +++ /dev/null @@ -1,44 +0,0 @@ -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 - - def index? - true - end - - def show? - true - end - - def create? - true - end - - def update? - true - end - - def destroy? - true - end - - def destroy_all? - true - end - - class Scope - attr_reader :user, :scope - - def initialize(user, scope) - @user = user - @scope = scope - end - - def resolve - scope - end - end -end diff --git a/config/initializers/active_admin.rb b/config/initializers/active_admin.rb index 6e936577e..b14529f4f 100644 --- a/config/initializers/active_admin.rb +++ b/config/initializers/active_admin.rb @@ -59,7 +59,7 @@ # == User Authorization # config.authorization_adapter = ActiveAdmin::PunditAdapter - config.pundit_default_policy = "ActiveAdminPolicy" + # config.pundit_default_policy = "ActiveAdminPolicy" # If you wish to maintain a separate set of Pundit policies for admin # resources, you may set a namespace here that Pundit will search From 13b95c02c21e908475cc2246c05d5081b3c019b4 Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 15:01:08 +0000 Subject: [PATCH 11/34] only admins can delete, create or update libraries --- app/policies/library_policy.rb | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/app/policies/library_policy.rb b/app/policies/library_policy.rb index 13d6ebd00..f4a07c405 100644 --- a/app/policies/library_policy.rb +++ b/app/policies/library_policy.rb @@ -8,14 +8,27 @@ def show? end def create? - !Flipper.enabled? :demo_mode + !Flipper.enabled?(:demo_mode) && user.admin? end def update? - !Flipper.enabled? :demo_mode + !Flipper.enabled?(:demo_mode) && user.admin? end def destroy? - !Flipper.enabled? :demo_mode + !Flipper.enabled?(:demo_mode) && user.admin? + end + + class Scope + attr_reader :user, :scope + + def initialize(user, scope) + @user = user + @scope = scope + end + + def resolve + scope.all + end end end From 93244d61c7e57dcbd47ae11cd5518a8d38dd1ae0 Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 15:03:39 +0000 Subject: [PATCH 12/34] add brackets to flipper check --- app/policies/model_file_policy.rb | 2 +- app/policies/model_policy.rb | 2 +- app/policies/upload_policy.rb | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/policies/model_file_policy.rb b/app/policies/model_file_policy.rb index d91dc050e..584451af9 100644 --- a/app/policies/model_file_policy.rb +++ b/app/policies/model_file_policy.rb @@ -4,6 +4,6 @@ def show? end def destroy? - !Flipper.enabled? :demo_mode + !Flipper.enabled?(:demo_mode) end end diff --git a/app/policies/model_policy.rb b/app/policies/model_policy.rb index 9ce304d4c..a95ae986d 100644 --- a/app/policies/model_policy.rb +++ b/app/policies/model_policy.rb @@ -14,6 +14,6 @@ def update? end def destroy? - !Flipper.enabled? :demo_mode + !Flipper.enabled?(:demo_mode) end end diff --git a/app/policies/upload_policy.rb b/app/policies/upload_policy.rb index a7ebc879f..88dd4efda 100644 --- a/app/policies/upload_policy.rb +++ b/app/policies/upload_policy.rb @@ -1,9 +1,9 @@ class UploadPolicy < ApplicationPolicy def index? - !Flipper.enabled? :demo_mode + !Flipper.enabled?(:demo_mode) end def create? - !Flipper.enabled? :demo_mode + !Flipper.enabled?(:demo_mode) end end From ecf08da655af886cd84fee2917879fe33a4b94fb Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 15:04:05 +0000 Subject: [PATCH 13/34] don't authorize library in models controller --- app/controllers/models_controller.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/controllers/models_controller.rb b/app/controllers/models_controller.rb index 2fcc2394f..1a73e1c75 100644 --- a/app/controllers/models_controller.rb +++ b/app/controllers/models_controller.rb @@ -146,7 +146,6 @@ def model_params def get_library @library = Model.find(params[:id]).library - authorize @library end def get_model From 3b01f177edb8632257e53309b1647cb4afae152c Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 15:08:51 +0000 Subject: [PATCH 14/34] remove unwritten view specs --- spec/views/collections/create.html.erb_spec.rb | 5 ----- spec/views/collections/edit.html.erb_spec.rb | 5 ----- spec/views/collections/index.html.erb_spec.rb | 5 ----- spec/views/collections/new.html.erb_spec.rb | 5 ----- spec/views/collections/show.html.erb_spec.rb | 5 ----- spec/views/collections/update.html.erb_spec.rb | 5 ----- spec/views/creators/create.html.erb_spec.rb | 5 ----- spec/views/creators/edit.html.erb_spec.rb | 5 ----- spec/views/creators/index.html.erb_spec.rb | 5 ----- spec/views/creators/new.html.erb_spec.rb | 5 ----- spec/views/creators/show.html.erb_spec.rb | 5 ----- spec/views/creators/update.html.erb_spec.rb | 5 ----- spec/views/home/index.html.erb_spec.rb | 5 ----- spec/views/libraries/index.html.erb_spec.rb | 5 ----- spec/views/libraries/show.html.erb_spec.rb | 5 ----- spec/views/models/show.html.erb_spec.rb | 5 ----- spec/views/parts/show.html.erb_spec.rb | 5 ----- spec/views/problems/index.html.erb_spec.rb | 5 ----- spec/views/settings/show.html.erb_spec.rb | 5 ----- 19 files changed, 95 deletions(-) delete mode 100644 spec/views/collections/create.html.erb_spec.rb delete mode 100644 spec/views/collections/edit.html.erb_spec.rb delete mode 100644 spec/views/collections/index.html.erb_spec.rb delete mode 100644 spec/views/collections/new.html.erb_spec.rb delete mode 100644 spec/views/collections/show.html.erb_spec.rb delete mode 100644 spec/views/collections/update.html.erb_spec.rb delete mode 100644 spec/views/creators/create.html.erb_spec.rb delete mode 100644 spec/views/creators/edit.html.erb_spec.rb delete mode 100644 spec/views/creators/index.html.erb_spec.rb delete mode 100644 spec/views/creators/new.html.erb_spec.rb delete mode 100644 spec/views/creators/show.html.erb_spec.rb delete mode 100644 spec/views/creators/update.html.erb_spec.rb delete mode 100644 spec/views/home/index.html.erb_spec.rb delete mode 100644 spec/views/libraries/index.html.erb_spec.rb delete mode 100644 spec/views/libraries/show.html.erb_spec.rb delete mode 100644 spec/views/models/show.html.erb_spec.rb delete mode 100644 spec/views/parts/show.html.erb_spec.rb delete mode 100644 spec/views/problems/index.html.erb_spec.rb delete mode 100644 spec/views/settings/show.html.erb_spec.rb diff --git a/spec/views/collections/create.html.erb_spec.rb b/spec/views/collections/create.html.erb_spec.rb deleted file mode 100644 index 97b32d3c1..000000000 --- a/spec/views/collections/create.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "collections/create.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/views/collections/edit.html.erb_spec.rb b/spec/views/collections/edit.html.erb_spec.rb deleted file mode 100644 index b35f19cfc..000000000 --- a/spec/views/collections/edit.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "collections/edit.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/views/collections/index.html.erb_spec.rb b/spec/views/collections/index.html.erb_spec.rb deleted file mode 100644 index 44fc88fe4..000000000 --- a/spec/views/collections/index.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "collections/index.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/views/collections/new.html.erb_spec.rb b/spec/views/collections/new.html.erb_spec.rb deleted file mode 100644 index f4472784f..000000000 --- a/spec/views/collections/new.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "collections/new.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/views/collections/show.html.erb_spec.rb b/spec/views/collections/show.html.erb_spec.rb deleted file mode 100644 index 9eac7390d..000000000 --- a/spec/views/collections/show.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "collections/show.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/views/collections/update.html.erb_spec.rb b/spec/views/collections/update.html.erb_spec.rb deleted file mode 100644 index 0a8a4e700..000000000 --- a/spec/views/collections/update.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "collections/update.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/views/creators/create.html.erb_spec.rb b/spec/views/creators/create.html.erb_spec.rb deleted file mode 100644 index 515427a11..000000000 --- a/spec/views/creators/create.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "creators/create.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/views/creators/edit.html.erb_spec.rb b/spec/views/creators/edit.html.erb_spec.rb deleted file mode 100644 index ad77dc27a..000000000 --- a/spec/views/creators/edit.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "creators/edit.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/views/creators/index.html.erb_spec.rb b/spec/views/creators/index.html.erb_spec.rb deleted file mode 100644 index 029f64e47..000000000 --- a/spec/views/creators/index.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "creators/index.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/views/creators/new.html.erb_spec.rb b/spec/views/creators/new.html.erb_spec.rb deleted file mode 100644 index 3fce2b5c1..000000000 --- a/spec/views/creators/new.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "creators/new.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/views/creators/show.html.erb_spec.rb b/spec/views/creators/show.html.erb_spec.rb deleted file mode 100644 index 959dbb1f2..000000000 --- a/spec/views/creators/show.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "creators/show.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/views/creators/update.html.erb_spec.rb b/spec/views/creators/update.html.erb_spec.rb deleted file mode 100644 index 5e3dd6804..000000000 --- a/spec/views/creators/update.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "creators/update.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/views/home/index.html.erb_spec.rb b/spec/views/home/index.html.erb_spec.rb deleted file mode 100644 index 514014740..000000000 --- a/spec/views/home/index.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "home/index.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/views/libraries/index.html.erb_spec.rb b/spec/views/libraries/index.html.erb_spec.rb deleted file mode 100644 index af9d75d0c..000000000 --- a/spec/views/libraries/index.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "libraries/index.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/views/libraries/show.html.erb_spec.rb b/spec/views/libraries/show.html.erb_spec.rb deleted file mode 100644 index b6ed202fd..000000000 --- a/spec/views/libraries/show.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "libraries/show.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/views/models/show.html.erb_spec.rb b/spec/views/models/show.html.erb_spec.rb deleted file mode 100644 index 0a79e754f..000000000 --- a/spec/views/models/show.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "models/show.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/views/parts/show.html.erb_spec.rb b/spec/views/parts/show.html.erb_spec.rb deleted file mode 100644 index 8ba332be9..000000000 --- a/spec/views/parts/show.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "files/show.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/views/problems/index.html.erb_spec.rb b/spec/views/problems/index.html.erb_spec.rb deleted file mode 100644 index 62a3d1ea6..000000000 --- a/spec/views/problems/index.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "problems/index.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/views/settings/show.html.erb_spec.rb b/spec/views/settings/show.html.erb_spec.rb deleted file mode 100644 index 26f5c6333..000000000 --- a/spec/views/settings/show.html.erb_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe "settings/index.html.erb" do - pending "add some examples to (or delete) #{__FILE__}" -end From 7f59f7f3418eadb69f758dac657a454197149820 Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 15:12:21 +0000 Subject: [PATCH 15/34] add user policy for settings --- app/controllers/settings_controller.rb | 1 + app/policies/user_policy.rb | 34 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 app/policies/user_policy.rb diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb index 233f52235..2416fb3d7 100644 --- a/app/controllers/settings_controller.rb +++ b/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 diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb new file mode 100644 index 000000000..9273de4e9 --- /dev/null +++ b/app/policies/user_policy.rb @@ -0,0 +1,34 @@ +class UserPolicy < ApplicationPolicy + def index? + !Flipper.enabled?(:demo_mode) + end + + def show? + !Flipper.enabled?(:demo_mode) + end + + def create? + !Flipper.enabled?(:demo_mode) + end + + def update? + !Flipper.enabled?(:demo_mode) + end + + def destroy? + !Flipper.enabled?(:demo_mode) + end + + class Scope + attr_reader :user, :scope + + def initialize(user, scope) + @user = user + @scope = scope + end + + def resolve + scope.all + end + end +end From b0b52ba783c5f419523722e97cbc15eeb580f2ae Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 15:27:42 +0000 Subject: [PATCH 16/34] add activeadmin authorization exemption it handles it internally --- app/controllers/application_controller.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index abd39b4d0..e9c54b156 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,7 +1,7 @@ class ApplicationController < ActionController::Base include Pundit::Authorization - after_action :verify_authorized, except: :index - after_action :verify_policy_scoped, only: :index + 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! @@ -29,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 From 6509c5ccf3a279c43827566f3d0bb1680ce3d35f Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 15:36:09 +0000 Subject: [PATCH 17/34] add missing policies for admin mode --- app/policies/active_admin/page_policy.rb | 7 ++++ .../acts_as_taggable_on/tag_policy.rb | 36 +++++++++++++++++++ .../backend/active_record/job_policy.rb | 36 +++++++++++++++++++ app/policies/link_policy.rb | 34 ++++++++++++++++++ 4 files changed, 113 insertions(+) create mode 100644 app/policies/active_admin/page_policy.rb create mode 100644 app/policies/acts_as_taggable_on/tag_policy.rb create mode 100644 app/policies/delayed/backend/active_record/job_policy.rb create mode 100644 app/policies/link_policy.rb diff --git a/app/policies/active_admin/page_policy.rb b/app/policies/active_admin/page_policy.rb new file mode 100644 index 000000000..a69dd8988 --- /dev/null +++ b/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 diff --git a/app/policies/acts_as_taggable_on/tag_policy.rb b/app/policies/acts_as_taggable_on/tag_policy.rb new file mode 100644 index 000000000..a597d0963 --- /dev/null +++ b/app/policies/acts_as_taggable_on/tag_policy.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +class ActsAsTaggableOn::TagPolicy < ApplicationPolicy + def index? + true + end + + def show? + true + end + + def create? + true + end + + def update? + true + end + + def destroy? + true + end + + class Scope + attr_reader :user, :scope + + def initialize(user, scope) + @user = user + @scope = scope + end + + def resolve + scope.all + end + end +end diff --git a/app/policies/delayed/backend/active_record/job_policy.rb b/app/policies/delayed/backend/active_record/job_policy.rb new file mode 100644 index 000000000..f3a86a8dc --- /dev/null +++ b/app/policies/delayed/backend/active_record/job_policy.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +class Delayed::Backend::ActiveRecord::JobPolicy < ApplicationPolicy + def index? + true + end + + def show? + true + end + + def create? + true + end + + def update? + true + end + + def destroy? + true + end + + class Scope + attr_reader :user, :scope + + def initialize(user, scope) + @user = user + @scope = scope + end + + def resolve + scope.all + end + end +end diff --git a/app/policies/link_policy.rb b/app/policies/link_policy.rb new file mode 100644 index 000000000..8420af854 --- /dev/null +++ b/app/policies/link_policy.rb @@ -0,0 +1,34 @@ +class LinkPolicy < ApplicationPolicy + def index? + true + end + + def show? + true + end + + def create? + true + end + + def update? + true + end + + def destroy? + true + end + + class Scope + attr_reader :user, :scope + + def initialize(user, scope) + @user = user + @scope = scope + end + + def resolve + scope.all + end + end +end From d2f97dcca54d7e6ce3c7de0514c39ba7e6f79435 Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 15:42:54 +0000 Subject: [PATCH 18/34] make default policy be allowed for logged-in admins, but not for anyone else this lets us cover a good range of current defaults, but upgrade access in a fail-safe way. --- app/policies/application_policy.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index e000cba51..21c830f4d 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -9,15 +9,15 @@ def initialize(user, record) end def index? - false + user&.admin? end def show? - false + user&.admin? end def create? - false + user&.admin? end def new? @@ -25,7 +25,7 @@ def new? end def update? - false + user&.admin? end def edit? @@ -33,7 +33,7 @@ def edit? end def destroy? - false + user&.admin? end class Scope @@ -43,7 +43,7 @@ def initialize(user, scope) end def resolve - raise NotImplementedError, "You must define #resolve in #{self.class}" + scope.all end private From 37fa380979501de85de082b6900c7a0017841048 Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 15:55:42 +0000 Subject: [PATCH 19/34] blanket ban admin area in demo mode using a controller filter in activeadmin config --- config/initializers/active_admin.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/config/initializers/active_admin.rb b/config/initializers/active_admin.rb index b14529f4f..72ee2878b 100644 --- a/config/initializers/active_admin.rb +++ b/config/initializers/active_admin.rb @@ -139,7 +139,9 @@ # You can add before, after and around filters to all of your # Active Admin resources and pages from here. # - # config.before_action :do_something_awesome + config.before_action do + raise Pundit::NotAuthorizedError, I18n.t("active_admin.demo_mode") if Flipper.enabled? :demo_mode + end # == Attribute Filters # From 12086e1dab50dc45ffb9d4583581a52ec5d0d957 Mon Sep 17 00:00:00 2001 From: Rukongai Date: Sun, 18 Feb 2024 23:13:27 -0700 Subject: [PATCH 20/34] test: additional admin view spec files --- spec/requests/admin/collections_spec.rb | 25 +++++++++++++++++++++++++ spec/requests/admin/delayed_job_spec.rb | 25 +++++++++++++++++++++++++ spec/requests/admin/link_spec.rb | 25 +++++++++++++++++++++++++ spec/requests/admin/log_spec.rb | 25 +++++++++++++++++++++++++ spec/requests/admin/model_files_spec.rb | 25 +++++++++++++++++++++++++ spec/requests/admin/models_spec.rb | 25 +++++++++++++++++++++++++ spec/requests/admin/problem_spec.rb | 25 +++++++++++++++++++++++++ spec/requests/admin/tags_spec.rb | 25 +++++++++++++++++++++++++ spec/requests/admin/users_spec.rb | 25 +++++++++++++++++++++++++ 9 files changed, 225 insertions(+) create mode 100644 spec/requests/admin/collections_spec.rb create mode 100644 spec/requests/admin/delayed_job_spec.rb create mode 100644 spec/requests/admin/link_spec.rb create mode 100644 spec/requests/admin/log_spec.rb create mode 100644 spec/requests/admin/model_files_spec.rb create mode 100644 spec/requests/admin/models_spec.rb create mode 100644 spec/requests/admin/problem_spec.rb create mode 100644 spec/requests/admin/tags_spec.rb create mode 100644 spec/requests/admin/users_spec.rb diff --git a/spec/requests/admin/collections_spec.rb b/spec/requests/admin/collections_spec.rb new file mode 100644 index 000000000..346ede789 --- /dev/null +++ b/spec/requests/admin/collections_spec.rb @@ -0,0 +1,25 @@ +require "rails_helper" + +RSpec.describe "Admin::Collections" do + it "is inaccessible to normal user" do + sign_in create(:user, admin: false) + get "/admin/collections" + expect(response).to have_http_status(:unauthorized) + end + + context "with admin permission" do + before do + sign_in create(:user, admin: true) + end + + it "is accessible" do + get "/admin/collections" + expect(response).to have_http_status(:success) + end + + it "is inaccessible in demo mode" do + allow(SiteSettings).to receive(:demo_mode?).and_return(true) + expect { get("/admin/collections") }.to raise_error(Pundit::NotAuthorizedError) + end + end +end diff --git a/spec/requests/admin/delayed_job_spec.rb b/spec/requests/admin/delayed_job_spec.rb new file mode 100644 index 000000000..51d5604d9 --- /dev/null +++ b/spec/requests/admin/delayed_job_spec.rb @@ -0,0 +1,25 @@ +require "rails_helper" + +RSpec.describe "Admin::Delayed_Job" do + it "is inaccessible to normal user" do + sign_in create(:user, admin: false) + get "/admin/delayed_backend_active_record_jobs" + expect(response).to have_http_status(:unauthorized) + end + + context "with admin permission" do + before do + sign_in create(:user, admin: true) + end + + it "is accessible" do + get "/admin/delayed_backend_active_record_jobs" + expect(response).to have_http_status(:success) + end + + it "is inaccessible in demo mode" do + allow(SiteSettings).to receive(:demo_mode?).and_return(true) + expect { get("/admin/delayed_backend_active_record_jobs") }.to raise_error(Pundit::NotAuthorizedError) + end + end +end diff --git a/spec/requests/admin/link_spec.rb b/spec/requests/admin/link_spec.rb new file mode 100644 index 000000000..4159d5a86 --- /dev/null +++ b/spec/requests/admin/link_spec.rb @@ -0,0 +1,25 @@ +require "rails_helper" + +RSpec.describe "Admin::Links" do + it "is inaccessible to normal user" do + sign_in create(:user, admin: false) + get "/admin/links" + expect(response).to have_http_status(:unauthorized) + end + + context "with admin permission" do + before do + sign_in create(:user, admin: true) + end + + it "is accessible" do + get "/admin/links" + expect(response).to have_http_status(:success) + end + + it "is inaccessible in demo mode" do + allow(SiteSettings).to receive(:demo_mode?).and_return(true) + expect { get("/admin/links") }.to raise_error(Pundit::NotAuthorizedError) + end + end +end diff --git a/spec/requests/admin/log_spec.rb b/spec/requests/admin/log_spec.rb new file mode 100644 index 000000000..e30e3a32b --- /dev/null +++ b/spec/requests/admin/log_spec.rb @@ -0,0 +1,25 @@ +require "rails_helper" + +RSpec.describe "Admin::Log" do + it "is inaccessible to normal user" do + sign_in create(:user, admin: false) + get "/admin/log" + expect(response).to have_http_status(:unauthorized) + end + + context "with admin permission" do + before do + sign_in create(:user, admin: true) + end + + it "is accessible" do + get "/admin/log" + expect(response).to have_http_status(:success) + end + + it "is inaccessible in demo mode" do + allow(SiteSettings).to receive(:demo_mode?).and_return(true) + expect { get("/admin/log") }.to raise_error(Pundit::NotAuthorizedError) + end + end +end diff --git a/spec/requests/admin/model_files_spec.rb b/spec/requests/admin/model_files_spec.rb new file mode 100644 index 000000000..ae2b91444 --- /dev/null +++ b/spec/requests/admin/model_files_spec.rb @@ -0,0 +1,25 @@ +require "rails_helper" + +RSpec.describe "Admin::Model_Files" do + it "is inaccessible to normal user" do + sign_in create(:user, admin: false) + get "/admin/model_files" + expect(response).to have_http_status(:unauthorized) + end + + context "with admin permission" do + before do + sign_in create(:user, admin: true) + end + + it "is accessible" do + get "/admin/model_files" + expect(response).to have_http_status(:success) + end + + it "is inaccessible in demo mode" do + allow(SiteSettings).to receive(:demo_mode?).and_return(true) + expect { get("/admin/model_files") }.to raise_error(Pundit::NotAuthorizedError) + end + end +end diff --git a/spec/requests/admin/models_spec.rb b/spec/requests/admin/models_spec.rb new file mode 100644 index 000000000..3d272a1ee --- /dev/null +++ b/spec/requests/admin/models_spec.rb @@ -0,0 +1,25 @@ +require "rails_helper" + +RSpec.describe "Admin::Models" do + it "is inaccessible to normal user" do + sign_in create(:user, admin: false) + get "/admin/models" + expect(response).to have_http_status(:unauthorized) + end + + context "with admin permission" do + before do + sign_in create(:user, admin: true) + end + + it "is accessible" do + get "/admin/models" + expect(response).to have_http_status(:success) + end + + it "is inaccessible in demo mode" do + allow(SiteSettings).to receive(:demo_mode?).and_return(true) + expect { get("/admin/models") }.to raise_error(Pundit::NotAuthorizedError) + end + end +end diff --git a/spec/requests/admin/problem_spec.rb b/spec/requests/admin/problem_spec.rb new file mode 100644 index 000000000..b3f7789a4 --- /dev/null +++ b/spec/requests/admin/problem_spec.rb @@ -0,0 +1,25 @@ +require "rails_helper" + +RSpec.describe "Admin::Problems" do + it "is inaccessible to normal user" do + sign_in create(:user, admin: false) + get "/admin/problems" + expect(response).to have_http_status(:unauthorized) + end + + context "with admin permission" do + before do + sign_in create(:user, admin: true) + end + + it "is accessible" do + get "/admin/problems" + expect(response).to have_http_status(:success) + end + + it "is inaccessible in demo mode" do + allow(SiteSettings).to receive(:demo_mode?).and_return(true) + expect { get("/admin/problems") }.to raise_error(Pundit::NotAuthorizedError) + end + end +end diff --git a/spec/requests/admin/tags_spec.rb b/spec/requests/admin/tags_spec.rb new file mode 100644 index 000000000..faba2fb40 --- /dev/null +++ b/spec/requests/admin/tags_spec.rb @@ -0,0 +1,25 @@ +require "rails_helper" + +RSpec.describe "Admin::Tags" do + it "is inaccessible to normal user" do + sign_in create(:user, admin: false) + get "/admin/acts_as_taggable_on_tags" + expect(response).to have_http_status(:unauthorized) + end + + context "with admin permission" do + before do + sign_in create(:user, admin: true) + end + + it "is accessible" do + get "/admin/acts_as_taggable_on_tags" + expect(response).to have_http_status(:success) + end + + it "is inaccessible in demo mode" do + allow(SiteSettings).to receive(:demo_mode?).and_return(true) + expect { get("/admin/acts_as_taggable_on_tags") }.to raise_error(Pundit::NotAuthorizedError) + end + end +end diff --git a/spec/requests/admin/users_spec.rb b/spec/requests/admin/users_spec.rb new file mode 100644 index 000000000..81c689272 --- /dev/null +++ b/spec/requests/admin/users_spec.rb @@ -0,0 +1,25 @@ +require "rails_helper" + +RSpec.describe "Admin::Users" do + it "is inaccessible to normal user" do + sign_in create(:user, admin: false) + get "/admin/users" + expect(response).to have_http_status(:unauthorized) + end + + context "with admin permission" do + before do + sign_in create(:user, admin: true) + end + + it "is accessible" do + get "/admin/users" + expect(response).to have_http_status(:success) + end + + it "is inaccessible in demo mode" do + allow(SiteSettings).to receive(:demo_mode?).and_return(true) + expect { get("/admin/users") }.to raise_error(Pundit::NotAuthorizedError) + end + end +end From b29ce5e927d1845774af46116255c5803cf1d8fe Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 16:23:59 +0000 Subject: [PATCH 21/34] update demo mode stubs for change to Flipper --- spec/requests/admin/collections_spec.rb | 2 +- spec/requests/admin/delayed_job_spec.rb | 2 +- spec/requests/admin/link_spec.rb | 2 +- spec/requests/admin/log_spec.rb | 2 +- spec/requests/admin/model_files_spec.rb | 2 +- spec/requests/admin/models_spec.rb | 2 +- spec/requests/admin/problem_spec.rb | 2 +- spec/requests/admin/tags_spec.rb | 2 +- spec/requests/admin/users_spec.rb | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/spec/requests/admin/collections_spec.rb b/spec/requests/admin/collections_spec.rb index 346ede789..38c54c94e 100644 --- a/spec/requests/admin/collections_spec.rb +++ b/spec/requests/admin/collections_spec.rb @@ -18,7 +18,7 @@ end it "is inaccessible in demo mode" do - allow(SiteSettings).to receive(:demo_mode?).and_return(true) + Flipper.enable :demo_mode expect { get("/admin/collections") }.to raise_error(Pundit::NotAuthorizedError) end end diff --git a/spec/requests/admin/delayed_job_spec.rb b/spec/requests/admin/delayed_job_spec.rb index 51d5604d9..cd37b68f9 100644 --- a/spec/requests/admin/delayed_job_spec.rb +++ b/spec/requests/admin/delayed_job_spec.rb @@ -18,7 +18,7 @@ end it "is inaccessible in demo mode" do - allow(SiteSettings).to receive(:demo_mode?).and_return(true) + Flipper.enable :demo_mode expect { get("/admin/delayed_backend_active_record_jobs") }.to raise_error(Pundit::NotAuthorizedError) end end diff --git a/spec/requests/admin/link_spec.rb b/spec/requests/admin/link_spec.rb index 4159d5a86..221408197 100644 --- a/spec/requests/admin/link_spec.rb +++ b/spec/requests/admin/link_spec.rb @@ -18,7 +18,7 @@ end it "is inaccessible in demo mode" do - allow(SiteSettings).to receive(:demo_mode?).and_return(true) + Flipper.enable :demo_mode expect { get("/admin/links") }.to raise_error(Pundit::NotAuthorizedError) end end diff --git a/spec/requests/admin/log_spec.rb b/spec/requests/admin/log_spec.rb index e30e3a32b..3129c431d 100644 --- a/spec/requests/admin/log_spec.rb +++ b/spec/requests/admin/log_spec.rb @@ -18,7 +18,7 @@ end it "is inaccessible in demo mode" do - allow(SiteSettings).to receive(:demo_mode?).and_return(true) + Flipper.enable :demo_mode expect { get("/admin/log") }.to raise_error(Pundit::NotAuthorizedError) end end diff --git a/spec/requests/admin/model_files_spec.rb b/spec/requests/admin/model_files_spec.rb index ae2b91444..c93a00751 100644 --- a/spec/requests/admin/model_files_spec.rb +++ b/spec/requests/admin/model_files_spec.rb @@ -18,7 +18,7 @@ end it "is inaccessible in demo mode" do - allow(SiteSettings).to receive(:demo_mode?).and_return(true) + Flipper.enable :demo_mode expect { get("/admin/model_files") }.to raise_error(Pundit::NotAuthorizedError) end end diff --git a/spec/requests/admin/models_spec.rb b/spec/requests/admin/models_spec.rb index 3d272a1ee..13f12bef2 100644 --- a/spec/requests/admin/models_spec.rb +++ b/spec/requests/admin/models_spec.rb @@ -18,7 +18,7 @@ end it "is inaccessible in demo mode" do - allow(SiteSettings).to receive(:demo_mode?).and_return(true) + Flipper.enable :demo_mode expect { get("/admin/models") }.to raise_error(Pundit::NotAuthorizedError) end end diff --git a/spec/requests/admin/problem_spec.rb b/spec/requests/admin/problem_spec.rb index b3f7789a4..6ec52eb9b 100644 --- a/spec/requests/admin/problem_spec.rb +++ b/spec/requests/admin/problem_spec.rb @@ -18,7 +18,7 @@ end it "is inaccessible in demo mode" do - allow(SiteSettings).to receive(:demo_mode?).and_return(true) + Flipper.enable :demo_mode expect { get("/admin/problems") }.to raise_error(Pundit::NotAuthorizedError) end end diff --git a/spec/requests/admin/tags_spec.rb b/spec/requests/admin/tags_spec.rb index faba2fb40..b7eb36386 100644 --- a/spec/requests/admin/tags_spec.rb +++ b/spec/requests/admin/tags_spec.rb @@ -18,7 +18,7 @@ end it "is inaccessible in demo mode" do - allow(SiteSettings).to receive(:demo_mode?).and_return(true) + Flipper.enable :demo_mode expect { get("/admin/acts_as_taggable_on_tags") }.to raise_error(Pundit::NotAuthorizedError) end end diff --git a/spec/requests/admin/users_spec.rb b/spec/requests/admin/users_spec.rb index 81c689272..8577a1164 100644 --- a/spec/requests/admin/users_spec.rb +++ b/spec/requests/admin/users_spec.rb @@ -18,7 +18,7 @@ end it "is inaccessible in demo mode" do - allow(SiteSettings).to receive(:demo_mode?).and_return(true) + Flipper.enable :demo_mode expect { get("/admin/users") }.to raise_error(Pundit::NotAuthorizedError) end end From 023bfd48057f993dd264fbaabb250d3c7592abfe Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 16:29:08 +0000 Subject: [PATCH 22/34] remove user spec, as whether or not it exists depends on a config change --- spec/requests/admin/users_spec.rb | 25 ------------------------- 1 file changed, 25 deletions(-) delete mode 100644 spec/requests/admin/users_spec.rb diff --git a/spec/requests/admin/users_spec.rb b/spec/requests/admin/users_spec.rb deleted file mode 100644 index 8577a1164..000000000 --- a/spec/requests/admin/users_spec.rb +++ /dev/null @@ -1,25 +0,0 @@ -require "rails_helper" - -RSpec.describe "Admin::Users" do - it "is inaccessible to normal user" do - sign_in create(:user, admin: false) - get "/admin/users" - expect(response).to have_http_status(:unauthorized) - end - - context "with admin permission" do - before do - sign_in create(:user, admin: true) - end - - it "is accessible" do - get "/admin/users" - expect(response).to have_http_status(:success) - end - - it "is inaccessible in demo mode" do - Flipper.enable :demo_mode - expect { get("/admin/users") }.to raise_error(Pundit::NotAuthorizedError) - end - end -end From 931320584d7ec2edf32937ba1e1f6e9f4a2ad73a Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 16:33:17 +0000 Subject: [PATCH 23/34] remove unnecessary helper specs --- spec/helpers/collections_helper_spec.rb | 15 --------------- spec/helpers/creators_helper_spec.rb | 15 --------------- spec/helpers/home_helper_spec.rb | 15 --------------- spec/helpers/libraries_helper_spec.rb | 15 --------------- spec/helpers/model_files_helper_spec.rb | 15 --------------- spec/helpers/settings_helper_spec.rb | 15 --------------- spec/models/link_spec.rb | 5 ----- 7 files changed, 95 deletions(-) delete mode 100644 spec/helpers/collections_helper_spec.rb delete mode 100644 spec/helpers/creators_helper_spec.rb delete mode 100644 spec/helpers/home_helper_spec.rb delete mode 100644 spec/helpers/libraries_helper_spec.rb delete mode 100644 spec/helpers/model_files_helper_spec.rb delete mode 100644 spec/helpers/settings_helper_spec.rb delete mode 100644 spec/models/link_spec.rb diff --git a/spec/helpers/collections_helper_spec.rb b/spec/helpers/collections_helper_spec.rb deleted file mode 100644 index 90b787eec..000000000 --- a/spec/helpers/collections_helper_spec.rb +++ /dev/null @@ -1,15 +0,0 @@ -require "rails_helper" - -# Specs in this file have access to a helper object that includes -# the CollectionsHelper. For example: -# -# describe CollectionsHelper do -# describe "string concat" do -# it "concats two strings with spaces" do -# expect(helper.concat_strings("this","that")).to eq("this that") -# end -# end -# end -RSpec.describe CollectionsHelper do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/helpers/creators_helper_spec.rb b/spec/helpers/creators_helper_spec.rb deleted file mode 100644 index 3c94ef028..000000000 --- a/spec/helpers/creators_helper_spec.rb +++ /dev/null @@ -1,15 +0,0 @@ -require "rails_helper" - -# Specs in this file have access to a helper object that includes -# the CreatorsHelper. For example: -# -# describe CreatorsHelper do -# describe "string concat" do -# it "concats two strings with spaces" do -# expect(helper.concat_strings("this","that")).to eq("this that") -# end -# end -# end -RSpec.describe CreatorsHelper do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/helpers/home_helper_spec.rb b/spec/helpers/home_helper_spec.rb deleted file mode 100644 index 92e274603..000000000 --- a/spec/helpers/home_helper_spec.rb +++ /dev/null @@ -1,15 +0,0 @@ -require "rails_helper" - -# Specs in this file have access to a helper object that includes -# the HomeHelper. For example: -# -# describe HomeHelper do -# describe "string concat" do -# it "concats two strings with spaces" do -# expect(helper.concat_strings("this","that")).to eq("this that") -# end -# end -# end -RSpec.describe HomeHelper do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/helpers/libraries_helper_spec.rb b/spec/helpers/libraries_helper_spec.rb deleted file mode 100644 index 5d8a343e6..000000000 --- a/spec/helpers/libraries_helper_spec.rb +++ /dev/null @@ -1,15 +0,0 @@ -require "rails_helper" - -# Specs in this file have access to a helper object that includes -# the LibrariesHelper. For example: -# -# describe LibrariesHelper do -# describe "string concat" do -# it "concats two strings with spaces" do -# expect(helper.concat_strings("this","that")).to eq("this that") -# end -# end -# end -RSpec.describe LibrariesHelper do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/helpers/model_files_helper_spec.rb b/spec/helpers/model_files_helper_spec.rb deleted file mode 100644 index 26ab9f17d..000000000 --- a/spec/helpers/model_files_helper_spec.rb +++ /dev/null @@ -1,15 +0,0 @@ -require "rails_helper" - -# Specs in this file have access to a helper object that includes -# the ModelFilesHelper. For example: -# -# describe ModelFilesHelper do -# describe "string concat" do -# it "concats two strings with spaces" do -# expect(helper.concat_strings("this","that")).to eq("this that") -# end -# end -# end -RSpec.describe ModelFilesHelper do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/helpers/settings_helper_spec.rb b/spec/helpers/settings_helper_spec.rb deleted file mode 100644 index 16d829a41..000000000 --- a/spec/helpers/settings_helper_spec.rb +++ /dev/null @@ -1,15 +0,0 @@ -require "rails_helper" - -# Specs in this file have access to a helper object that includes -# the SettingsHelper. For example: -# -# describe SettingsHelper do -# describe "string concat" do -# it "concats two strings with spaces" do -# expect(helper.concat_strings("this","that")).to eq("this that") -# end -# end -# end -RSpec.describe SettingsHelper do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/models/link_spec.rb b/spec/models/link_spec.rb deleted file mode 100644 index 4021fbded..000000000 --- a/spec/models/link_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -RSpec.describe Link do - pending "add some examples to (or delete) #{__FILE__}" -end From de212de1f85b92a60b70da800096d6e67d5647a8 Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 16:33:47 +0000 Subject: [PATCH 24/34] change modelcomponent pending test message --- spec/components/model_component_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/components/model_component_spec.rb b/spec/components/model_component_spec.rb index 1443b5000..281acc3ab 100644 --- a/spec/components/model_component_spec.rb +++ b/spec/components/model_component_spec.rb @@ -3,7 +3,7 @@ require "rails_helper" RSpec.describe ModelComponent, type: :component do - pending "add some examples to (or delete) #{__FILE__}" + it "needs testing" # it "renders something useful" do # expect( From 8236bbdd66c7a4d8b42315f65bedd1abd3a79a45 Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 18:16:25 +0000 Subject: [PATCH 25/34] add tests and authorisation for creators --- app/controllers/creators_controller.rb | 6 +++- spec/requests/creators_spec.rb | 47 +++++++++++++++++++------- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/app/controllers/creators_controller.rb b/app/controllers/creators_controller.rb index e00bdbcdc..f7d3eab61 100644 --- a/app/controllers/creators_controller.rb +++ b/app/controllers/creators_controller.rb @@ -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 diff --git a/spec/requests/creators_spec.rb b/spec/requests/creators_spec.rb index 29b66d6d7..a96c966ef 100644 --- a/spec/requests/creators_spec.rb +++ b/spec/requests/creators_spec.rb @@ -10,13 +10,16 @@ # DELETE /creators/:id(.:format) creators#destroy RSpec.describe "Creators" do + let(:admin) { create(:user, admin: true) } + let(:creator) { create(:creator) } + context "when signed out" do it "needs testing" end context "when signed in" do before do - sign_in create(:user) + sign_in admin build_list(:creator, 13) do |creator| creator.save! # See https://dev.to/hernamvel/the-optimal-way-to-create-a-set-of-records-with-factorybot-createlist-factorybot-buildlist-1j64 create_list(:link, 1, linkable: creator) @@ -32,28 +35,46 @@ end end - describe "POST /creators" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "POST /creators" do + it "creates a new creator" do + post "/creators", params: {creator: {name: "newname"}} + expect(response).to redirect_to("/creators") + end end - describe "GET /creators/new" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "GET /creators/new" do + it "Shows the new creator form" do + get "/creators/new" + expect(response).to have_http_status(:success) + end end - describe "GET /creators/:id/edit" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "GET /creators/:id/edit" do + it "Shows the new creator form" do + get "/creators/#{creator.id}/edit" + expect(response).to have_http_status(:success) + end end - describe "GET /creators/:id" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "GET /creators/:id" do + it "Redirects to a list of models with that creator" do + get "/creators/#{creator.id}" + expect(response).to redirect_to("/models?creator=#{creator.id}") + end end - describe "PATCH /creators/:id" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "PATCH /creators/:id" do + it "saves details" do + patch "/creators/#{creator.id}", params: {creator: {name: "newname"}} + expect(response).to redirect_to("/creators/#{creator.id}") + end end - describe "DELETE /creators/:id" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "DELETE /creators/:id" do + it "removes creator" do + delete "/creators/#{creator.id}" + expect(response).to redirect_to("/creators") + end end end end From 6e170f04698243a560f348aa29d957a82b48894d Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 22:52:07 +0000 Subject: [PATCH 26/34] better pending messages for logged-out tests --- spec/requests/collections_spec.rb | 2 +- spec/requests/creators_spec.rb | 2 +- spec/requests/devise/sessions_spec.rb | 8 ++++---- spec/requests/home_spec.rb | 2 +- spec/requests/libraries_spec.rb | 2 +- spec/requests/model_files_spec.rb | 2 +- spec/requests/models_spec.rb | 2 +- spec/requests/problems_spec.rb | 2 +- spec/requests/settings_spec.rb | 12 +----------- spec/requests/uploads_spec.rb | 8 +------- spec/requests/users/registrations_spec.rb | 4 ++-- 11 files changed, 15 insertions(+), 31 deletions(-) diff --git a/spec/requests/collections_spec.rb b/spec/requests/collections_spec.rb index f40a4ac53..cc4bba993 100644 --- a/spec/requests/collections_spec.rb +++ b/spec/requests/collections_spec.rb @@ -11,7 +11,7 @@ RSpec.describe "Collections" do context "when signed out" do - it "needs testing" + it "needs testing when multiuser is enabled" end context "when signed in" do diff --git a/spec/requests/creators_spec.rb b/spec/requests/creators_spec.rb index a96c966ef..f7360e402 100644 --- a/spec/requests/creators_spec.rb +++ b/spec/requests/creators_spec.rb @@ -14,7 +14,7 @@ let(:creator) { create(:creator) } context "when signed out" do - it "needs testing" + it "needs testing when multiuser is enabled" end context "when signed in" do diff --git a/spec/requests/devise/sessions_spec.rb b/spec/requests/devise/sessions_spec.rb index 3f4e10529..fdbed4f5b 100644 --- a/spec/requests/devise/sessions_spec.rb +++ b/spec/requests/devise/sessions_spec.rb @@ -5,11 +5,11 @@ # destroy_user_session DELETE /users/sign_out(.:format) devise/sessions#destroy RSpec.describe "Devise::Sessions" do - context "when signed out" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + context "when signed out" do + it "needs testing when multiuser is enabled" end - context "when signed in" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + context "when signed in" do + it "needs testing when multiuser is enabled" end end diff --git a/spec/requests/home_spec.rb b/spec/requests/home_spec.rb index d18dd6d6b..a1bb8e6cf 100644 --- a/spec/requests/home_spec.rb +++ b/spec/requests/home_spec.rb @@ -4,7 +4,7 @@ RSpec.describe "Home" do context "when signed out" do - it "needs testing" + it "needs testing when multiuser is enabled" end context "when signed in" do diff --git a/spec/requests/libraries_spec.rb b/spec/requests/libraries_spec.rb index 2f11d4b38..e2a5faf49 100644 --- a/spec/requests/libraries_spec.rb +++ b/spec/requests/libraries_spec.rb @@ -13,7 +13,7 @@ RSpec.describe "Libraries" do context "when signed out" do - it "needs testing" + it "needs testing when multiuser is enabled" end context "when signed in" do diff --git a/spec/requests/model_files_spec.rb b/spec/requests/model_files_spec.rb index 678be8258..f60152f24 100644 --- a/spec/requests/model_files_spec.rb +++ b/spec/requests/model_files_spec.rb @@ -13,7 +13,7 @@ RSpec.describe "Model Files" do context "when signed out" do - it "needs testing" + it "needs testing when multiuser is enabled" end context "when signed in" do diff --git a/spec/requests/models_spec.rb b/spec/requests/models_spec.rb index 35d972e2b..b648c854c 100644 --- a/spec/requests/models_spec.rb +++ b/spec/requests/models_spec.rb @@ -14,7 +14,7 @@ RSpec.describe "Models" do context "when signed out" do - it "needs testing" + it "needs testing when multiuser is enabled" end context "when signed in" do diff --git a/spec/requests/problems_spec.rb b/spec/requests/problems_spec.rb index 6ac78bdbb..1c76fce25 100644 --- a/spec/requests/problems_spec.rb +++ b/spec/requests/problems_spec.rb @@ -6,7 +6,7 @@ RSpec.describe "Problems" do context "when signed out" do - it "needs testing" + it "needs testing when multiuser is enabled" end context "when signed in" do diff --git a/spec/requests/settings_spec.rb b/spec/requests/settings_spec.rb index 731b80445..295ee163d 100644 --- a/spec/requests/settings_spec.rb +++ b/spec/requests/settings_spec.rb @@ -8,17 +8,7 @@ let(:user) { create(:user) } context "when signed out" do - describe "GET /users/:user_id/settings" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "is forbidden" - end - - describe "PATCH /users/:user_id/settings" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "is forbidden" - end - - describe "PUT /users/:user_id/settings" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "is forbidden" - end + it "needs testing when multiuser is enabled" end context "when signed in" do diff --git a/spec/requests/uploads_spec.rb b/spec/requests/uploads_spec.rb index 9fd7c6ee9..7316334eb 100644 --- a/spec/requests/uploads_spec.rb +++ b/spec/requests/uploads_spec.rb @@ -5,13 +5,7 @@ RSpec.describe "Uploads" do context "when signed out" do - describe "GET /uploads" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "should be forbidden" - end - - describe "POST /uploads" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "should be forbidden" - end + it "needs testing when multiuser is enabled" end context "when signed in" do diff --git a/spec/requests/users/registrations_spec.rb b/spec/requests/users/registrations_spec.rb index 59ed079a4..bc2c386f3 100644 --- a/spec/requests/users/registrations_spec.rb +++ b/spec/requests/users/registrations_spec.rb @@ -10,10 +10,10 @@ RSpec.describe "Users::Registrations" do context "when signed out" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + it "needs testing when multiuser is enabled" end context "when signed in" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + it "needs testing when multiuser is enabled" end end From 2f497bd83dbb4b2551a84d1050621d50105ce12a Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 23:02:34 +0000 Subject: [PATCH 27/34] add tests and authorization for collection --- app/controllers/collections_controller.rb | 4 ++ spec/requests/collections_spec.rb | 49 ++++++++++++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/app/controllers/collections_controller.rb b/app/controllers/collections_controller.rb index 69496ea8c..c7d2a587c 100644 --- a/app/controllers/collections_controller.rb +++ b/app/controllers/collections_controller.rb @@ -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 diff --git a/spec/requests/collections_spec.rb b/spec/requests/collections_spec.rb index cc4bba993..98e717beb 100644 --- a/spec/requests/collections_spec.rb +++ b/spec/requests/collections_spec.rb @@ -10,13 +10,16 @@ # DELETE /collections/:id(.:format) collections#destroy RSpec.describe "Collections" do + let(:admin) { create(:user, admin: true) } + let(:collection) { create(:collection) } + context "when signed out" do it "needs testing when multiuser is enabled" end context "when signed in" do before do - sign_in create(:user) + sign_in admin build_list(:collection, 13) do |collection| collection.save! # See https://dev.to/hernamvel/the-optimal-way-to-create-a-set-of-records-with-factorybot-createlist-factorybot-buildlist-1j64 create_list(:link, 1, linkable: collection) @@ -24,12 +27,54 @@ end end - describe "GET /collections?page=2" do + describe "GET /collections" do it "returns paginated collections" do # rubocop:todo RSpec/MultipleExpectations get "/collections?page=2" expect(response).to have_http_status(:success) expect(response.body).to match(/pagination/) end end + + describe "POST /collections" do + it "creates a new collection" do + post "/collections", params: {collection: {name: "newname"}} + expect(response).to redirect_to("/collections") + end + end + + describe "GET /collections/new" do + it "Shows the new collection form" do + get "/collections/new" + expect(response).to have_http_status(:success) + end + end + + describe "GET /collections/:id/edit" do + it "Shows the new collection form" do + get "/collections/#{collection.id}/edit" + expect(response).to have_http_status(:success) + end + end + + describe "GET /collections/:id" do + it "Redirects to a list of models with that collection" do + get "/collections/#{collection.id}" + expect(response).to redirect_to("/models?collection=#{collection.id}") + end + end + + describe "PATCH /collections/:id" do + it "saves details" do + patch "/collections/#{collection.id}", params: {collection: {name: "newname"}} + expect(response).to redirect_to("/collections") + end + end + + describe "DELETE /collections/:id" do + it "removes collection" do + delete "/collections/#{collection.id}" + expect(response).to redirect_to("/collections") + end + end end end From 52655d3a2822ab2400a13fc54ffb6973eb111aaa Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 15 Mar 2024 23:46:23 +0000 Subject: [PATCH 28/34] authorization for library controller --- app/controllers/libraries_controller.rb | 2 +- app/policies/library_policy.rb | 4 ++ spec/requests/libraries_spec.rb | 53 ++++++++++++++++++------- 3 files changed, 43 insertions(+), 16 deletions(-) diff --git a/app/controllers/libraries_controller.rb b/app/controllers/libraries_controller.rb index 60f8901fc..e4c43e937 100644 --- a/app/controllers/libraries_controller.rb +++ b/app/controllers/libraries_controller.rb @@ -38,7 +38,7 @@ def create def update @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") diff --git a/app/policies/library_policy.rb b/app/policies/library_policy.rb index f4a07c405..265384c07 100644 --- a/app/policies/library_policy.rb +++ b/app/policies/library_policy.rb @@ -19,6 +19,10 @@ def destroy? !Flipper.enabled?(:demo_mode) && user.admin? end + def scan? + true + end + class Scope attr_reader :user, :scope diff --git a/spec/requests/libraries_spec.rb b/spec/requests/libraries_spec.rb index e2a5faf49..85e68cf93 100644 --- a/spec/requests/libraries_spec.rb +++ b/spec/requests/libraries_spec.rb @@ -12,24 +12,32 @@ # DELETE /libraries/:id(.:format) libraries#destroy RSpec.describe "Libraries" do + let(:admin) { create(:user, admin: true) } + context "when signed out" do it "needs testing when multiuser is enabled" end context "when signed in" do before do - sign_in create(:user) + sign_in admin @library = create(:library) do |library| create_list(:model, 2, library: library) end end - describe "POST /libraries/:id/scan" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "POST /libraries/:id/scan" do + it "scans a single library" do + expect { post "/libraries/#{@library.id}/scan" }.to have_enqueued_job(Scan::DetectFilesystemChangesJob).exactly(:once) + expect(response).to redirect_to("/libraries/#{@library.id}") + end end - describe "POST /libraries/scan" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "POST /libraries/scan" do + it "scans all libraries" do + expect { post "/libraries/scan" }.to have_enqueued_job(Scan::DetectFilesystemChangesJob).exactly(:once) + expect(response).to redirect_to("/models") + end end describe "GET /libraries" do @@ -39,16 +47,25 @@ end end - describe "POST /libraries/" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "POST /libraries/" do + it "creates a new library" do + post "/libraries", params: {library: {name: "new"}} + expect(response).to have_http_status(:success) + end end - describe "GET /libraries/new" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "GET /libraries/new" do + it "shows the new library form" do + get "/libraries/new" + expect(response).to have_http_status(:success) + end end - describe "GET /libraries/:id/edit" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "GET /libraries/:id/edit" do + it "shows the edit library form" do + get "/libraries/#{@library.id}/edit" + expect(response).to have_http_status(:success) + end end describe "GET /libraries/:id" do @@ -58,12 +75,18 @@ end end - describe "PATCH /libraries/:id" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "PATCH /libraries/:id" do + it "updates the library" do + patch "/libraries/#{@library.id}", params: {library: {name: "new"}} + expect(response).to redirect_to("/models") + end end - describe "DELETE /libraries/:id" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "DELETE /libraries/:id" do + it "removes the library" do + delete "/libraries/#{@library.id}" + expect(response).to redirect_to("/libraries") + end end end end From 772df3be283b0edebca7f685edd9a926fb2e34da Mon Sep 17 00:00:00 2001 From: James Smith Date: Sat, 16 Mar 2024 00:32:39 +0000 Subject: [PATCH 29/34] authorisation and policy for modelfiles --- app/controllers/model_files_controller.rb | 4 +- app/policies/model_file_policy.rb | 8 ++++ config/routes.rb | 2 +- spec/requests/model_files_spec.rb | 51 +++++++++++++---------- 4 files changed, 39 insertions(+), 26 deletions(-) diff --git a/app/controllers/model_files_controller.rb b/app/controllers/model_files_controller.rb index 2e8684418..c7bde57ac 100644 --- a/app/controllers/model_files_controller.rb +++ b/app/controllers/model_files_controller.rb @@ -54,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 @@ -91,12 +91,10 @@ def file_params def get_library @library = Library.find(params[:library_id]) - authorize @library end def get_model @model = @library.models.find(params[:model_id]) - authorize @model end def get_file diff --git a/app/policies/model_file_policy.rb b/app/policies/model_file_policy.rb index 584451af9..6fb9187e3 100644 --- a/app/policies/model_file_policy.rb +++ b/app/policies/model_file_policy.rb @@ -6,4 +6,12 @@ def show? def destroy? !Flipper.enabled?(:demo_mode) end + + def bulk_edit? + true + end + + def bulk_update? + true + end end diff --git a/config/routes.rb b/config/routes.rb index 3259022e6..2eb9d4ed6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -24,7 +24,7 @@ member do post "merge" end - resources :model_files, except: [:index] do + resources :model_files, except: [:index, :new, :create] do collection do get "edit", action: "bulk_edit" patch "update", action: "bulk_update" diff --git a/spec/requests/model_files_spec.rb b/spec/requests/model_files_spec.rb index f60152f24..3150aa4a1 100644 --- a/spec/requests/model_files_spec.rb +++ b/spec/requests/model_files_spec.rb @@ -3,8 +3,6 @@ # edit_library_model_model_files GET /libraries/:library_id/models/:model_id/model_files/edit(.:format) model_files#bulk_edit # library_model_model_files PATCH /libraries/:library_id/models/:model_id/model_files/update(.:format) model_files#bulk_update -# POST /libraries/:library_id/models/:model_id/model_files(.:format) model_files#create -# new_library_model_model_file GET /libraries/:library_id/models/:model_id/model_files/new(.:format) model_files#new # edit_library_model_model_file GET /libraries/:library_id/models/:model_id/model_files/:id/edit(.:format) model_files#edit # library_model_model_file GET /libraries/:library_id/models/:model_id/model_files/:id(.:format) model_files#show # PATCH /libraries/:library_id/models/:model_id/model_files/:id(.:format) model_files#update @@ -12,13 +10,15 @@ # DELETE /libraries/:library_id/models/:model_id/model_files/:id(.:format) model_files#destroy RSpec.describe "Model Files" do + let(:admin) { create(:user, admin: true) } + context "when signed out" do it "needs testing when multiuser is enabled" end context "when signed in" do before do - sign_in create(:user) + sign_in admin end let(:jpg_file) { create(:model_file, model: model, filename: "test.jpg") } @@ -36,27 +36,28 @@ end end - describe "GET /libraries/:library_id/models/:model_id/model_files/edit" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" - end - - describe "PATCH /libraries/:library_id/models/:model_id/model_files/update" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" - end - - describe "POST /libraries/:library_id/models/:model_id/model_files" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "GET /libraries/:library_id/models/:model_id/model_files/edit" do + it "shows bulk update form" do + get edit_library_model_model_files_path(library, model, stl_file) + expect(response).to have_http_status(:success) + end end - describe "GET /libraries/:library_id/models/:model_id/model_files/new" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "PATCH /libraries/:library_id/models/:model_id/model_files/update" do + it "bulk updates the files" do + patch library_model_model_file_path(library, model, stl_file), params: {model_file: {name: "name"}} + expect(response).to redirect_to(library_model_model_file_path(library, model, stl_file)) + end end - describe "GET /libraries/:library_id/models/:model_id/model_files/:id/edit" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "GET /libraries/:library_id/models/:model_id/model_files/:id/edit" do + it "shows edit page for file" do + get edit_library_model_model_file_path(library, model, stl_file) + expect(response).to have_http_status(:success) + end end - describe "GET /libraries/:library_id/models/:model_id/model_files/:id" do # rubocop:todo RSpec/RepeatedExampleGroupBody + describe "GET /libraries/:library_id/models/:model_id/model_files/:id" do describe "GET a model file in its original file format" do before do get library_model_model_file_path(library, model, stl_file, format: :stl) @@ -86,12 +87,18 @@ end end - describe "PATCH /libraries/:library_id/models/:model_id/model_files/:id" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "PATCH /libraries/:library_id/models/:model_id/model_files/:id" do + it "updates the file" do + patch library_model_model_file_path(library, model, stl_file), params: {model_file: {name: "name"}} + expect(response).to redirect_to(library_model_model_file_path(library, model, stl_file)) + end end - describe "DELETE /libraries/:library_id/models/:model_id/model_files/:id" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "DELETE /libraries/:library_id/models/:model_id/model_files/:id" do + it "removes the file" do + delete library_model_model_file_path(library, model, stl_file) + expect(response).to redirect_to(library_model_path(library, model)) + end end end end From 701ceb4cb3212532a890b1a856f76c96e8a09244 Mon Sep 17 00:00:00 2001 From: James Smith Date: Sat, 16 Mar 2024 21:16:46 +0000 Subject: [PATCH 30/34] authorisation and request tests for models --- app/controllers/models_controller.rb | 4 +-- app/policies/model_policy.rb | 4 +++ config/routes.rb | 2 +- spec/requests/models_spec.rb | 38 +++++++++++++++------------- 4 files changed, 28 insertions(+), 20 deletions(-) diff --git a/app/controllers/models_controller.rb b/app/controllers/models_controller.rb index 1a73e1c75..45fd1db52 100644 --- a/app/controllers/models_controller.rb +++ b/app/controllers/models_controller.rb @@ -68,7 +68,7 @@ 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 @@ -101,7 +101,7 @@ def bulk_update def destroy @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 diff --git a/app/policies/model_policy.rb b/app/policies/model_policy.rb index a95ae986d..dd0f11de0 100644 --- a/app/policies/model_policy.rb +++ b/app/policies/model_policy.rb @@ -13,6 +13,10 @@ def update? true end + def merge? + true + end + def destroy? !Flipper.enabled?(:demo_mode) end diff --git a/config/routes.rb b/config/routes.rb index 2eb9d4ed6..bdbb9874a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -20,7 +20,7 @@ collection do post "scan", action: :scan_all end - resources :models, except: [:index] do + resources :models, except: [:index, :new, :create] do member do post "merge" end diff --git a/spec/requests/models_spec.rb b/spec/requests/models_spec.rb index b648c854c..2be0f6f5d 100644 --- a/spec/requests/models_spec.rb +++ b/spec/requests/models_spec.rb @@ -1,7 +1,5 @@ require "rails_helper" -# library_models POST /libraries/:library_id/models(.:format) models#create -# new_library_model GET /libraries/:library_id/models/new(.:format) models#new # edit_library_model GET /libraries/:library_id/models/:id/edit(.:format) models#edit # library_model GET /libraries/:library_id/models/:id(.:format) models#show # PATCH /libraries/:library_id/models/:id(.:format) models#update @@ -13,13 +11,15 @@ # merge_library_model POST /libraries/:library_id/models/:id/merge(.:format) models#merge RSpec.describe "Models" do + let(:admin) { create(:user, admin: true) } + context "when signed out" do it "needs testing when multiuser is enabled" end context "when signed in" do before do - sign_in create(:user) + sign_in admin end let(:library) do @@ -29,14 +29,6 @@ end let(:creator) { create(:creator) } - describe "POST /libraries/:library_id/models/" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" - end - - describe "GET /libraries/:library_id/models/:id/new" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" - end - describe "GET /libraries/:library_id/models/:id" do it "returns http success" do get "/libraries/#{library.id}/models/#{library.models.first.id}" @@ -44,8 +36,11 @@ end end - describe "GET /libraries/:library_id/models/:id/edit" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "GET /libraries/:library_id/models/:id/edit" do + it "shows edit page for file" do + get "/libraries/#{library.id}/models/#{library.models.first.id}/edit" + expect(response).to have_http_status(:success) + end end describe "PUT /libraries/:library_id/models/:id" do @@ -88,11 +83,17 @@ end describe "DELETE /libraries/:library_id/models/:id" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + it "redirects to model list after deletion" do + delete "/libraries/#{library.id}/models/#{library.models.first.id}" + expect(response).to redirect_to("/libraries/#{library.id}") + end end describe "GET /models/edit" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + it "shows bulk edit page" do + get "/models/edit" + expect(response).to have_http_status(:success) + end end describe "PATCH /models/edit" do @@ -149,8 +150,11 @@ end end - describe "POST /libraries/:library_id/models/:id/merge" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + describe "POST /libraries/:library_id/models/:id/merge" do + it "gives a bad request response if no merge parameter is provided" do + post "/libraries/#{library.id}/models/#{library.models.first.id}/merge" + expect(response).to have_http_status(:bad_request) + end end end end From c860853db2775ab83091c36b8c5789210c021173 Mon Sep 17 00:00:00 2001 From: James Smith Date: Sat, 16 Mar 2024 21:21:47 +0000 Subject: [PATCH 31/34] lint fixes --- spec/requests/devise/sessions_spec.rb | 2 +- spec/requests/libraries_spec.rb | 27 +++++++++++++++------------ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/spec/requests/devise/sessions_spec.rb b/spec/requests/devise/sessions_spec.rb index fdbed4f5b..12b8b2501 100644 --- a/spec/requests/devise/sessions_spec.rb +++ b/spec/requests/devise/sessions_spec.rb @@ -10,6 +10,6 @@ end context "when signed in" do - it "needs testing when multiuser is enabled" + it "also needs testing when multiuser is enabled" end end diff --git a/spec/requests/libraries_spec.rb b/spec/requests/libraries_spec.rb index 85e68cf93..799399f6b 100644 --- a/spec/requests/libraries_spec.rb +++ b/spec/requests/libraries_spec.rb @@ -19,22 +19,25 @@ end context "when signed in" do + let!(:library) do + create(:library) do |l| + create_list(:model, 2, library: l) + end + end + before do sign_in admin - @library = create(:library) do |library| - create_list(:model, 2, library: library) - end end describe "POST /libraries/:id/scan" do - it "scans a single library" do - expect { post "/libraries/#{@library.id}/scan" }.to have_enqueued_job(Scan::DetectFilesystemChangesJob).exactly(:once) - expect(response).to redirect_to("/libraries/#{@library.id}") + it "scans a single library" do # rubocop:todo RSpec/MultipleExpectations + expect { post "/libraries/#{library.id}/scan" }.to have_enqueued_job(Scan::DetectFilesystemChangesJob).exactly(:once) + expect(response).to redirect_to("/libraries/#{library.id}") end end describe "POST /libraries/scan" do - it "scans all libraries" do + it "scans all libraries" do # rubocop:todo RSpec/MultipleExpectations expect { post "/libraries/scan" }.to have_enqueued_job(Scan::DetectFilesystemChangesJob).exactly(:once) expect(response).to redirect_to("/models") end @@ -63,28 +66,28 @@ describe "GET /libraries/:id/edit" do it "shows the edit library form" do - get "/libraries/#{@library.id}/edit" + get "/libraries/#{library.id}/edit" expect(response).to have_http_status(:success) end end describe "GET /libraries/:id" do it "redirects to models index with library filter" do - get "/libraries/1" - expect(response).to redirect_to("/models?library=1") + get "/libraries/#{library.id}" + expect(response).to redirect_to("/models?library=#{library.id}") end end describe "PATCH /libraries/:id" do it "updates the library" do - patch "/libraries/#{@library.id}", params: {library: {name: "new"}} + patch "/libraries/#{library.id}", params: {library: {name: "new"}} expect(response).to redirect_to("/models") end end describe "DELETE /libraries/:id" do it "removes the library" do - delete "/libraries/#{@library.id}" + delete "/libraries/#{library.id}" expect(response).to redirect_to("/libraries") end end From 3271fba7bd37b0f6d387f7991c3566adb25389bc Mon Sep 17 00:00:00 2001 From: James Smith Date: Sat, 16 Mar 2024 21:45:27 +0000 Subject: [PATCH 32/34] test PATCH problems --- spec/requests/problems_spec.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/spec/requests/problems_spec.rb b/spec/requests/problems_spec.rb index 1c76fce25..cf4a7a646 100644 --- a/spec/requests/problems_spec.rb +++ b/spec/requests/problems_spec.rb @@ -2,17 +2,16 @@ # problems GET /problems(.:format) problems#index # problem PATCH /problems/:id(.:format) problems#update -# PUT /problems/:id(.:format) problems#update RSpec.describe "Problems" do + let(:admin) { create :user, admin: true} + context "when signed out" do it "needs testing when multiuser is enabled" end context "when signed in" do - before do - sign_in create(:user) - end + before { sign_in admin } describe "GET /problems" do before do @@ -88,12 +87,13 @@ end end - describe "PUT /problems/:id" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" - end + describe "PATCH /problems/:id" do + let(:problem) { create :problem } - describe "PATCH /problems/:id" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + it "updates the problem and returns to list" do + patch "/problems/#{problem.id}", params: { problem: { ignored: true } } + expect(response).to redirect_to("/problems") + end end end end From c65cb6ef63649fbe33b3e4d3a77288990ae0813f Mon Sep 17 00:00:00 2001 From: James Smith Date: Sat, 16 Mar 2024 22:38:57 +0000 Subject: [PATCH 33/34] authorize and test uploads --- app/controllers/uploads_controller.rb | 6 ++++++ spec/requests/uploads_spec.rb | 17 ++++++++++++++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 6a5992c7a..43515cb64 100644 --- a/app/controllers/uploads_controller.rb +++ b/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, "")) diff --git a/spec/requests/uploads_spec.rb b/spec/requests/uploads_spec.rb index 7316334eb..d520ae7de 100644 --- a/spec/requests/uploads_spec.rb +++ b/spec/requests/uploads_spec.rb @@ -4,17 +4,28 @@ # POST /uploads(.:format) uploads#create RSpec.describe "Uploads" do + let(:admin) { create :user, admin: true} + let(:library) { create :library } + context "when signed out" do it "needs testing when multiuser is enabled" end context "when signed in" do - describe "GET /uploads" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + before { sign_in admin } + + describe "GET /uploads" do + it "shows upload form" do + get "/uploads" + expect(response).to have_http_status(:success) + end end describe "POST /uploads" do # rubocop:todo RSpec/RepeatedExampleGroupBody - it "needs testing" + it "redirect back to index after upload" do + post "/uploads", params: { post: { library_pick: library.id, scan_after_upload: "1" }, upload: { datafiles: []} } + expect(response).to redirect_to("/libraries") + end end end end From 84672d4898ad3b5584ad7172bd4f0153ed669b4b Mon Sep 17 00:00:00 2001 From: James Smith Date: Sat, 16 Mar 2024 22:41:48 +0000 Subject: [PATCH 34/34] lint --- spec/requests/problems_spec.rb | 6 +++--- spec/requests/uploads_spec.rb | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/requests/problems_spec.rb b/spec/requests/problems_spec.rb index cf4a7a646..19974404e 100644 --- a/spec/requests/problems_spec.rb +++ b/spec/requests/problems_spec.rb @@ -4,7 +4,7 @@ # problem PATCH /problems/:id(.:format) problems#update RSpec.describe "Problems" do - let(:admin) { create :user, admin: true} + let(:admin) { create(:user, admin: true) } context "when signed out" do it "needs testing when multiuser is enabled" @@ -88,10 +88,10 @@ end describe "PATCH /problems/:id" do - let(:problem) { create :problem } + let(:problem) { create(:problem) } it "updates the problem and returns to list" do - patch "/problems/#{problem.id}", params: { problem: { ignored: true } } + patch "/problems/#{problem.id}", params: {problem: {ignored: true}} expect(response).to redirect_to("/problems") end end diff --git a/spec/requests/uploads_spec.rb b/spec/requests/uploads_spec.rb index d520ae7de..2df18214d 100644 --- a/spec/requests/uploads_spec.rb +++ b/spec/requests/uploads_spec.rb @@ -4,8 +4,8 @@ # POST /uploads(.:format) uploads#create RSpec.describe "Uploads" do - let(:admin) { create :user, admin: true} - let(:library) { create :library } + let(:admin) { create(:user, admin: true) } + let(:library) { create(:library) } context "when signed out" do it "needs testing when multiuser is enabled" @@ -23,7 +23,7 @@ describe "POST /uploads" do # rubocop:todo RSpec/RepeatedExampleGroupBody it "redirect back to index after upload" do - post "/uploads", params: { post: { library_pick: library.id, scan_after_upload: "1" }, upload: { datafiles: []} } + post "/uploads", params: {post: {library_pick: library.id, scan_after_upload: "1"}, upload: {datafiles: []}} expect(response).to redirect_to("/libraries") end end