diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index f7ae2d2b6e9..cdc5932ba12 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -56,12 +56,14 @@ def search flash_search_warnings(@tags) end - # if user is Admin or Tag Wrangler, show them details about the tag + # if user is admin with view access or Tag Wrangler, show them details about the tag # if user is not logged in or a regular user, show them # 1. the works, if the tag had been wrangled and we can redirect them to works using it or its canonical merger # 2. the tag, the works and the bookmarks using it, if the tag is unwrangled (because we can't redirect them # to the works controller) def show + authorize :wrangling, :read_access? if logged_in_as_admin? + @page_subtitle = @tag.name if @tag.is_a?(Banned) && !logged_in_as_admin? flash[:error] = ts('Please log in as admin') @@ -166,6 +168,8 @@ def show_hidden # GET /tags/new def new + authorize :wrangling if logged_in_as_admin? + @tag = Tag.new respond_to do |format| @@ -209,6 +213,8 @@ def create end def edit + authorize :wrangling, :read_access? if logged_in_as_admin? + @page_subtitle = ts('%{tag_name} - Edit', tag_name: @tag.name) if @tag.is_a?(Banned) && !logged_in_as_admin? @@ -241,6 +247,8 @@ def edit end def update + authorize :wrangling if logged_in_as_admin? + # update everything except for the synonym, # so that the associations are there to move when the synonym is created syn_string = params[:tag].delete(:syn_string) @@ -272,6 +280,8 @@ def update end def wrangle + authorize :wrangling, :read_access? if logged_in_as_admin? + @page_subtitle = ts('%{tag_name} - Wrangle', tag_name: @tag.name) @counts = {} @tag.child_types.map { |t| t.underscore.pluralize.to_sym }.each do |tag_type| @@ -303,6 +313,8 @@ def wrangle end def mass_update + authorize :wrangling if logged_in_as_admin? + params[:page] = '1' if params[:page].blank? params[:sort_column] = 'name' unless valid_sort_column(params[:sort_column], 'tag') params[:sort_direction] = 'ASC' unless valid_sort_direction(params[:sort_direction]) diff --git a/app/policies/wrangling_policy.rb b/app/policies/wrangling_policy.rb index 40acf51c97a..b7e4d142537 100644 --- a/app/policies/wrangling_policy.rb +++ b/app/policies/wrangling_policy.rb @@ -2,13 +2,21 @@ class WranglingPolicy < ApplicationPolicy FULL_ACCESS_ROLES = %w[superadmin tag_wrangling].freeze - + READ_ACCESS_ROLES = (FULL_ACCESS_ROLES + %w[policy_and_abuse]).freeze + def full_access? user_has_roles?(FULL_ACCESS_ROLES) end + def read_access? + user_has_roles?(READ_ACCESS_ROLES) + end + alias create? full_access? alias destroy? full_access? + alias mass_update? full_access? + alias new? full_access? alias show? full_access? alias report_csv? full_access? + alias update? full_access? end diff --git a/features/bookmarks/bookmark_indexing.feature b/features/bookmarks/bookmark_indexing.feature index 7bd9a41cd36..e32a131c35e 100644 --- a/features/bookmarks/bookmark_indexing.feature +++ b/features/bookmarks/bookmark_indexing.feature @@ -63,7 +63,7 @@ Feature: Bookmark Indexing Given a canonical fandom "Veronica Mars" And a canonical fandom "Veronica Mars (TV)" And bookmarks of external works and series tagged with the fandom tag "Veronica Mars" - And I am logged in as an admin + And I am logged in as a "tag_wrangling" admin When I syn the tag "Veronica Mars" to "Veronica Mars (TV)" And I go to the bookmarks tagged "Veronica Mars (TV)" Then I should see "BookmarkedExternalWork" diff --git a/features/tags_and_wrangling/tag_wrangling.feature b/features/tags_and_wrangling/tag_wrangling.feature index 10c6e7b9544..a43aa92b77b 100644 --- a/features/tags_and_wrangling/tag_wrangling.feature +++ b/features/tags_and_wrangling/tag_wrangling.feature @@ -311,7 +311,7 @@ Feature: Tag wrangling Scenario: An admin can see the troubleshoot button on a tag page Given a canonical fandom "Cowboy Bebop" - And I am logged in as an admin + And I am logged in as a "tag_wrangling" admin When I view the tag "Cowboy Bebop" Then I should see "Troubleshoot" diff --git a/features/tags_and_wrangling/tag_wrangling_admin.feature b/features/tags_and_wrangling/tag_wrangling_admin.feature index 5d70da67b07..967ee99aa60 100644 --- a/features/tags_and_wrangling/tag_wrangling_admin.feature +++ b/features/tags_and_wrangling/tag_wrangling_admin.feature @@ -3,7 +3,7 @@ Feature: Tag wrangling Scenario: Admin can rename a tag - Given I am logged in as an admin + Given I am logged in as a "tag_wrangling" admin And a fandom exists with name: "Amelie", canonical: false When I edit the tag "Amelie" And I fill in "Synonym of" with "Amélie" @@ -18,7 +18,7 @@ Feature: Tag wrangling Scenario: Admin can rename a tag using Eastern characters - Given I am logged in as an admin + Given I am logged in as a "tag_wrangling" admin And a fandom exists with name: "先生", canonical: false When I edit the tag "先生" And I fill in "Name" with "てりやき" diff --git a/features/tags_and_wrangling/tag_wrangling_characters.feature b/features/tags_and_wrangling/tag_wrangling_characters.feature index 9d65608c959..6c9c2e3561f 100644 --- a/features/tags_and_wrangling/tag_wrangling_characters.feature +++ b/features/tags_and_wrangling/tag_wrangling_characters.feature @@ -53,7 +53,7 @@ Scenario: character wrangling - syns, mergers, characters, autocompletes When I follow "Edit The First Doctor" Then I should not see "Make tag non-canonical and unhook all associations" - Given I am logged in as an admin + Given I am logged in as a "tag_wrangling" admin When I edit the tag "The First Doctor" Then I should see "Make tag non-canonical and unhook all associations" And I should see "The Doctor (1st)" @@ -129,7 +129,7 @@ Scenario: character wrangling - syns, mergers, characters, autocompletes When I follow "First Doctor" Then I should see "John Smith" And I should see "The Doctor" - When I am logged in as an admin + When I am logged in as a "tag_wrangling" admin And I edit the tag "First Doctor" And I fill in "Synonym of" with "First Doctor (DW)" And I press "Save changes" diff --git a/features/tags_and_wrangling/tag_wrangling_fandoms.feature b/features/tags_and_wrangling/tag_wrangling_fandoms.feature index 7ca8a94ceb9..a440ccfd39f 100644 --- a/features/tags_and_wrangling/tag_wrangling_fandoms.feature +++ b/features/tags_and_wrangling/tag_wrangling_fandoms.feature @@ -116,7 +116,7 @@ Scenario: fandoms wrangling - syns, mergers, autocompletes, metatags When I edit the tag "Stargate SG-1" Then I should see "Stargate SG-1: Ark of Truth" within "div#child_SubTag_associations_to_remove_checkboxes" And I should see "Stargate Franchise" within "div#parent_MetaTag_associations_to_remove_checkboxes" - When I am logged in as an admin + When I am logged in as a "tag_wrangling" admin And I edit the tag "Stargate SG-1" And I fill in "Synonym of" with "Stargate SG-1: Greatest Show in the Universe" And I press "Save changes" diff --git a/features/tags_and_wrangling/tag_wrangling_freeforms.feature b/features/tags_and_wrangling/tag_wrangling_freeforms.feature index 5f5e2a18627..f1a8786b92f 100644 --- a/features/tags_and_wrangling/tag_wrangling_freeforms.feature +++ b/features/tags_and_wrangling/tag_wrangling_freeforms.feature @@ -95,7 +95,7 @@ Scenario: freeforms wrangling - syns, mergers, autocompletes, metatags Then I should see "Tag was updated" When I follow "Alternate Universe Pirates" Then I should see "Alternate Universe Space Pirates" - When I am logged in as an admin + When I am logged in as a "tag_wrangling" admin And I edit the tag "Alternate Universe Pirates" And I fill in "Synonym of" with "Alternate Universe Pirrrates" And I press "Save changes" diff --git a/features/tags_and_wrangling/tag_wrangling_more.feature b/features/tags_and_wrangling/tag_wrangling_more.feature index 4ebfef407b2..40647c3cc72 100644 --- a/features/tags_and_wrangling/tag_wrangling_more.feature +++ b/features/tags_and_wrangling/tag_wrangling_more.feature @@ -222,7 +222,7 @@ Feature: Tag wrangling: assigning wranglers, using the filters on the Wranglers When I am logged in as a random user And I view the tag "Cowboy Bebop" Then I should see "Sorry, you don't have permission to access the page you were trying to reach." - When I am logged in as an admin + When I am logged in as a "tag_wrangling" admin And I view the tag "Cowboy Bebop" Then I should not see "Please log in as an admin" And I should see "Cowboy Bebop" diff --git a/features/tags_and_wrangling/tag_wrangling_relationships.feature b/features/tags_and_wrangling/tag_wrangling_relationships.feature index 5c49e38dabd..f9977998532 100644 --- a/features/tags_and_wrangling/tag_wrangling_relationships.feature +++ b/features/tags_and_wrangling/tag_wrangling_relationships.feature @@ -14,7 +14,7 @@ Scenario: relationship wrangling - syns, mergers, characters, autocompletes And a canonical character "Zoe Washburne" And a canonical character "Jack Harkness" And a canonical character "Ianto Jones" - And I am logged in as an admin + And I am logged in as a "tag_wrangling" admin And I follow "Tag Wrangling" # create a new canonical relationship from tag wrangling interface @@ -126,7 +126,7 @@ Scenario: relationship wrangling - syns, mergers, characters, autocompletes When I follow "Jack Harkness/Ianto Jones" Then I should see "Jack Harkness/Robot Ianto Jones" And I should see "Jack Harkness/Male Character" - When I am logged in as an admin + When I am logged in as a "tag_wrangling" admin And I edit the tag "Jack Harkness/Ianto Jones" And I fill in "Synonym of" with "Captain Jack Harkness/Ianto Jones" And I press "Save changes" @@ -270,7 +270,7 @@ Scenario: AO3-2147 Creating a new merger to a non-can tag while adding character And I should see "Testypants/Testyskirt" And the "Canonical" checkbox should be checked and disabled - When I am logged in as an admin + When I am logged in as a "tag_wrangling" admin And I edit the tag "Testing McTestypants/Testing McTestySkirt" And I fill in "Synonym of" with "Dame Tester/Sir Tester" And I press "Save changes" diff --git a/spec/controllers/tags_controller_spec.rb b/spec/controllers/tags_controller_spec.rb index 9be54190b26..d5a174e9ab2 100644 --- a/spec/controllers/tags_controller_spec.rb +++ b/spec/controllers/tags_controller_spec.rb @@ -1,9 +1,12 @@ -require 'spec_helper' +require "spec_helper" describe TagsController do include LoginMacros include RedirectExpectationHelper + wrangling_full_access_roles = %w[superadmin tag_wrangling].freeze + wrangling_read_access_roles = (wrangling_full_access_roles + %w[policy_and_abuse]).freeze + let(:user) { create(:tag_wrangler) } before { fake_login_known_user(user) } @@ -14,6 +17,41 @@ end end + shared_examples "an action only authorized admins can access" do |authorized_roles:| + before { fake_login_admin(admin) } + + context "with no role" do + let(:admin) { create(:admin, roles: []) } + + it "redirects with an error" do + subject + it_redirects_to_with_error(root_url, "Sorry, only an authorized admin can access the page you were trying to reach.") + end + end + + (Admin::VALID_ROLES - authorized_roles).each do |role| + context "with role #{role}" do + let(:admin) { create(:admin, roles: [role]) } + + it "redirects with an error" do + subject + it_redirects_to_with_error(root_url, "Sorry, only an authorized admin can access the page you were trying to reach.") + end + end + end + + authorized_roles.each do |role| + context "with role #{role}" do + let(:admin) { create(:admin, roles: [role]) } + + it "succeeds" do + subject + success + end + end + end + end + describe "#create" do let(:tag_params) do { name: Faker::FunnyName.name, canonical: "0", type: "Character" } @@ -72,6 +110,13 @@ run_all_indexing_jobs end + subject { get :wrangle, params: { id: fandom.name, show: "freeforms", status: "unwrangled" } } + let(:success) do + expect(assigns(:tags)).to include(freeform1) + end + + it_behaves_like "an action only authorized admins can access", authorized_roles: wrangling_read_access_roles + it "includes unwrangled freeforms" do get :wrangle, params: { id: fandom.name, show: "freeforms", status: "unwrangled" } expect(assigns(:tags)).to include(freeform1) @@ -121,40 +166,35 @@ @character3 = FactoryBot.create(:character, canonical: false) @character2 = FactoryBot.create(:character, canonical: false, merger: @character3) @work = FactoryBot.create(:work, - fandom_string: "#{@fandom1.name}", - character_string: "#{@character1.name},#{@character2.name}", - freeform_string: "#{@freeform1.name}") + fandom_string: @fandom1.name.to_s, + character_string: "#{@character1.name},#{@character2.name}", + freeform_string: @freeform1.name.to_s) end it "should redirect to the wrangle action for that tag" do - expect(put :mass_update, params: { id: @fandom1.name, show: 'freeforms', status: 'unwrangled' }). - to redirect_to wrangle_tag_path(id: @fandom1.name, - show: 'freeforms', - status: 'unwrangled', - page: 1, - sort_column: 'name', - sort_direction: 'ASC') + expect(put(:mass_update, params: { id: @fandom1.name, show: "freeforms", status: "unwrangled" })) + .to redirect_to wrangle_tag_path(id: @fandom1.name, + show: "freeforms", + status: "unwrangled", + page: 1, + sort_column: "name", + sort_direction: "ASC") end - context "with one canonical fandom in the fandom string and a selected freeform" do - before do - put :mass_update, params: { id: @fandom1.name, show: 'freeforms', status: 'unwrangled', fandom_string: @fandom2.name, selected_tags: [@freeform1.id] } - end - - it "updates the tags successfully" do - get :wrangle, params: { id: @fandom1.name, show: 'freeforms', status: 'unwrangled' } - expect(assigns(:tags)).not_to include(@freeform1) - - @freeform1.reload - expect(@freeform1.fandoms).to include(@fandom2) - end + subject { put :mass_update, params: { id: @fandom1.name, show: "freeforms", status: "unwrangled", fandom_string: @fandom2.name, selected_tags: [@freeform1.id] } } + let(:success) do + get :wrangle, params: { id: @fandom1.name, show: "freeforms", status: "unwrangled" } + expect(assigns(:tags)).not_to include(@freeform1) - include_examples "set last wrangling activity" + @freeform1.reload + expect(@freeform1.fandoms).to include(@fandom2) end + it_behaves_like "an action only authorized admins can access", authorized_roles: wrangling_full_access_roles + context "with one canonical and one noncanonical fandoms in the fandom string and a selected freeform" do before do - put :mass_update, params: { id: @fandom1.name, show: 'freeforms', status: 'unwrangled', fandom_string: "#{@fandom2.name},#{@fandom3.name}", selected_tags: [@freeform1.id] } + put :mass_update, params: { id: @fandom1.name, show: "freeforms", status: "unwrangled", fandom_string: "#{@fandom2.name},#{@fandom3.name}", selected_tags: [@freeform1.id] } end it "updates the tags successfully" do @@ -168,7 +208,7 @@ context "with two canonical fandoms in the fandom string and a selected character" do before do - put :mass_update, params: { id: @fandom1.name, show: 'characters', status: 'unwrangled', fandom_string: "#{@fandom1.name},#{@fandom2.name}", selected_tags: [@character1.id] } + put :mass_update, params: { id: @fandom1.name, show: "characters", status: "unwrangled", fandom_string: "#{@fandom1.name},#{@fandom2.name}", selected_tags: [@character1.id] } end it "updates the tags successfully" do @@ -182,7 +222,7 @@ context "with a canonical fandom in the fandom string, a selected unwrangled character, and the same character to be made canonical" do before do - put :mass_update, params: { id: @fandom1.name, show: 'characters', status: 'unwrangled', fandom_string: "#{@fandom1.name}", selected_tags: [@character1.id], canonicals: [@character1.id] } + put :mass_update, params: { id: @fandom1.name, show: "characters", status: "unwrangled", fandom_string: @fandom1.name.to_s, selected_tags: [@character1.id], canonicals: [@character1.id] } end it "updates the tags successfully" do @@ -196,7 +236,7 @@ context "with a canonical fandom in the fandom string, a selected synonym character, and the same character to be made canonical" do before do - put :mass_update, params: { id: @fandom1.name, show: 'characters', status: 'unfilterable', fandom_string: "#{@fandom2.name}", selected_tags: [@character2.id], canonicals: [@character2.id] } + put :mass_update, params: { id: @fandom1.name, show: "characters", status: "unfilterable", fandom_string: @fandom2.name.to_s, selected_tags: [@character2.id], canonicals: [@character2.id] } end it "updates the tags successfully" do @@ -231,6 +271,41 @@ end end + describe "new" do + subject { get :new } + let(:success) do + expect(response).to have_http_status(:success) + end + + it_behaves_like "an action only authorized admins can access", authorized_roles: wrangling_full_access_roles + + context "when logged in as a tag wrangler" do + it "allows access" do + get :new + expect(response).to have_http_status(:success) + end + end + end + + describe "show" do + context "when showing a banned tag" do + let(:tag) { create(:banned) } + + subject { get :edit, params: { id: tag.name } } + let(:success) do + expect(response).to have_http_status(:success) + end + + it_behaves_like "an action only authorized admins can access", authorized_roles: wrangling_read_access_roles + + it "redirects with an error when not an admin" do + get :show, params: { id: tag.name } + it_redirects_to_with_error(tag_wranglings_path, + "Please log in as admin") + end + end + end + describe "show_hidden" do let(:work) { create(:work) } @@ -251,12 +326,17 @@ describe "edit" do context "when editing a banned tag" do - before do - @tag = FactoryBot.create(:banned) + let(:tag) { create(:banned) } + + subject { get :edit, params: { id: tag.name } } + let(:success) do + expect(response).to have_http_status(:success) end + it_behaves_like "an action only authorized admins can access", authorized_roles: wrangling_read_access_roles + it "redirects with an error when not an admin" do - get :edit, params: { id: @tag.name } + get :edit, params: { id: tag.name } it_redirects_to_with_error(tag_wranglings_path, "Please log in as admin") end @@ -299,16 +379,15 @@ end end - context "when logged in as an admin" do - it "succeeds and redirects to the edit page" do - fake_login_admin(create(:admin)) - put :update, params: { id: tag, tag: { syn_string: synonym.name }, commit: "Save changes" } - it_redirects_to_with_notice(edit_tag_path(tag), "Tag was updated.") - - tag.reload - expect(tag.merger_id).to eq(synonym.id) - end + subject { put :update, params: { id: tag, tag: { syn_string: synonym.name }, commit: "Save changes" } } + let(:success) do + it_redirects_to_with_notice(edit_tag_path(tag), "Tag was updated.") + tag.reload + expect(tag.merger_id).to eq(synonym.id) end + + it_behaves_like "an action only authorized admins can access", authorized_roles: wrangling_full_access_roles + end shared_examples "success message" do @@ -369,7 +448,7 @@ end context "when the associated tag has an invalid type" do - # NOTE This will enter the associated tag into the freeform_string + # NOTE: This will enter the associated tag into the freeform_string # field, which is not displayed on the form. This still might come up # in the extremely rare case where a tag wrangler loads the form, a # different tag wrangler goes in and changes the type of the tag being