diff --git a/app/models/admin.rb b/app/models/admin.rb index e57d07fb6b6..9806ff6cac1 100644 --- a/app/models/admin.rb +++ b/app/models/admin.rb @@ -1,5 +1,5 @@ class Admin < ApplicationRecord - VALID_ROLES = %w[superadmin board board_assistants_team communications development_and_membership docs elections translation tag_wrangling support policy_and_abuse open_doors].freeze + VALID_ROLES = %w[superadmin board board_assistants_team communications development_and_membership docs elections legal translation tag_wrangling support policy_and_abuse open_doors].freeze serialize :roles, Array diff --git a/app/policies/comment_policy.rb b/app/policies/comment_policy.rb index b6d6e11087e..857219cee02 100644 --- a/app/policies/comment_policy.rb +++ b/app/policies/comment_policy.rb @@ -1,10 +1,10 @@ class CommentPolicy < ApplicationPolicy - DESTROY_COMMENT_ROLES = %w[superadmin board policy_and_abuse support].freeze - DESTROY_ADMIN_POST_COMMENT_ROLES = %w[superadmin board board_assistants_team communications elections policy_and_abuse support].freeze + DESTROY_COMMENT_ROLES = %w[superadmin board legal policy_and_abuse support].freeze + DESTROY_ADMIN_POST_COMMENT_ROLES = %w[superadmin board board_assistants_team communications elections legal policy_and_abuse support].freeze FREEZE_TAG_COMMENT_ROLES = %w[superadmin tag_wrangling].freeze FREEZE_WORK_COMMENT_ROLES = %w[superadmin policy_and_abuse].freeze - HIDE_TAG_COMMENT_ROLES = %w[superadmin tag_wrangling].freeze - HIDE_WORK_COMMENT_ROLES = %w[superadmin policy_and_abuse].freeze + HIDE_TAG_COMMENT_ROLES = %w[superadmin legal tag_wrangling].freeze + HIDE_WORK_COMMENT_ROLES = %w[superadmin legal policy_and_abuse].freeze SPAM_ADMIN_POST_COMMENT_ROLES = %w[superadmin board board_assistants_team communications elections policy_and_abuse support].freeze SPAM_COMMENT_ROLES = %w[superadmin board policy_and_abuse support].freeze @@ -63,6 +63,6 @@ def can_review_all? alias review_all? can_review_all? def show_email? - user_has_roles?(%w[policy_and_abuse support superadmin]) + user_has_roles?(%w[legal policy_and_abuse support superadmin]) end end diff --git a/app/policies/user_creation_policy.rb b/app/policies/user_creation_policy.rb index 98746464c02..3eb5f8b247c 100644 --- a/app/policies/user_creation_policy.rb +++ b/app/policies/user_creation_policy.rb @@ -1,21 +1,23 @@ class UserCreationPolicy < ApplicationPolicy + FULL_ACCESS_ROLES = %w[superadmin legal policy_and_abuse].freeze + def show_admin_options? destroy? || hide? || edit? end def destroy? - user_has_roles?(%w[superadmin policy_and_abuse]) + user_has_roles?(FULL_ACCESS_ROLES) end def hide? - user_has_roles?(%w[superadmin policy_and_abuse]) + user_has_roles?(FULL_ACCESS_ROLES) end def show_ip_address? - user_has_roles?(%w[superadmin policy_and_abuse]) + user_has_roles?(FULL_ACCESS_ROLES) end def show_original_creators? - user_has_roles?(%w[superadmin policy_and_abuse]) + user_has_roles?(FULL_ACCESS_ROLES) end end diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 9e8a9fa945b..f93d55bc3a0 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -4,7 +4,7 @@ class UserPolicy < ApplicationPolicy # - managing a user's invitations # - updating a user's email and roles (e.g. wranglers, archivists, not admin roles) # This is further restricted using ALLOWED_ATTRIBUTES_BY_ROLES. - MANAGE_ROLES = %w[superadmin policy_and_abuse open_doors support tag_wrangling].freeze + MANAGE_ROLES = %w[superadmin legal policy_and_abuse open_doors support tag_wrangling].freeze # Roles that allow updating the Fannish Next Of Kin of a user. MANAGE_NEXT_OF_KIN_ROLES = %w[superadmin policy_and_abuse support].freeze diff --git a/config/locales/models/en.yml b/config/locales/models/en.yml index 49f2435dcb3..ea6f31f8b19 100644 --- a/config/locales/models/en.yml +++ b/config/locales/models/en.yml @@ -9,6 +9,7 @@ en: development_and_membership: Development & Membership docs: AO3 Docs elections: Elections + legal: Legal open_doors: Open Doors policy_and_abuse: Policy & Abuse superadmin: Super admin diff --git a/features/admins/admin_works.feature b/features/admins/admin_works.feature index acfd6ca877b..39807cbd650 100644 --- a/features/admins/admin_works.feature +++ b/features/admins/admin_works.feature @@ -13,10 +13,10 @@ Feature: Admin Actions for Works, Comments, Series, Bookmarks And I press "Troubleshoot" Then I should see "Work sent to be reindexed." - Scenario: Can hide works + Scenario Outline: Can hide works Given I am logged in as "regular_user" And I post the work "ToS Violation" - When I am logged in as a "policy_and_abuse" admin + When I am logged in as a "" admin And all emails have been delivered And I view the work "ToS Violation" And I follow "Hide Work" @@ -26,8 +26,14 @@ Feature: Admin Actions for Works, Comments, Series, Bookmarks And "regular_user" should see their work "ToS Violation" is hidden And 1 email should be delivered And the email should contain "you will be required to take action to correct the violation" + + Examples: + | role | + | superadmin | + | legal | + | policy_and_abuse | - Scenario: Can unhide works + Scenario Outline: Can unhide works Given I am logged in as "regular_user" And I post the work "ToS Violation" When I am logged in as a "policy_and_abuse" admin @@ -43,7 +49,13 @@ Feature: Admin Actions for Works, Comments, Series, Bookmarks And logged in users should see the unhidden work "ToS Violation" by "regular_user" And 0 emails should be delivered - Scenario: Can delete works + Examples: + | role | + | superadmin | + | legal | + | policy_and_abuse | + + Scenario: Deleting works as a Policy & Abuse admin Given I am logged in as "regular_user" And I post the work "ToS Violation" When I am logged in as a "policy_and_abuse" admin @@ -66,9 +78,28 @@ Feature: Admin Actions for Works, Comments, Series, Bookmarks And I am on regular_user's works page Then I should not see "ToS Violation" - Scenario: Can hide bookmarks - Given basic tags - And I am logged in as "regular_user" with password "password1" + Scenario: Deleting works as a Legal admin + Given I am logged in as "regular_user" + And I post the work "ToS Violation" + When I am logged in as a "legal" admin + # Don't let the admin password email mess up the count. + And all emails have been delivered + And I view the work "ToS Violation" + And I follow "Delete Work" + And all indexing jobs have been run + Then I should see "Item was successfully deleted." + And 1 email should be delivered + And the email should contain "deleted from the Archive by a site admin" + And the email should not contain "translation missing" + When I log out + And I am on regular_user's works page + Then I should not see "ToS Violation" + When I am logged in + And I am on regular_user's works page + Then I should not see "ToS Violation" + + Scenario Outline: Can hide bookmarks + Given I am logged in as "regular_user" with password "password1" And I post the work "A Nice Work" When I am logged in as "bad_user" And I view the work "A Nice Work" @@ -77,7 +108,7 @@ Feature: Admin Actions for Works, Comments, Series, Bookmarks And I press "Create" And all indexing jobs have been run Then I should see "Bookmark was successfully created" - When I am logged in as a "policy_and_abuse" admin + When I am logged in as a "" admin And I am on bad_user's bookmarks page When I follow "Hide Bookmark" And all indexing jobs have been run @@ -86,9 +117,38 @@ Feature: Admin Actions for Works, Comments, Series, Bookmarks And I am on bad_user's bookmarks page Then I should not see "Rude comment" + Examples: + | role | + | superadmin | + | legal | + | policy_and_abuse | + + Scenario Outline: Deleting bookmarks + Given I am logged in as "regular_user" with password "password1" + And I post the work "A Nice Work" + When I am logged in as "bad_user" + And I view the work "A Nice Work" + When I follow "Bookmark" + And I fill in "bookmark_notes" with "Rude comment" + And I press "Create" + And all indexing jobs have been run + Then I should see "Bookmark was successfully created" + When I am logged in as a "" admin + And I am on bad_user's bookmarks page + And I follow "Delete Bookmark" + Then I should see "Item was successfully deleted." + When I am logged in as "bad_user" + And I am on bad_user's bookmarks page + Then I should not see "Rude comment" + + Examples: + | role | + | superadmin | + | legal | + | policy_and_abuse | + Scenario: Can edit tags on works - Given basic tags - And I am logged in as "regular_user" + Given I am logged in as "regular_user" And I post the work "Changes" with fandom "User-Added Fandom" with freeform "User-Added Freeform" with category "M/M" When I am logged in as a "policy_and_abuse" admin And I view the work "Changes" @@ -124,8 +184,7 @@ Feature: Admin Actions for Works, Comments, Series, Bookmarks And I should not see "Admin-Added Fandom" Scenario: Can edit external works - Given basic tags - And basic languages + Given basic languages And I am logged in as "regular_user" And I bookmark the external work "External Changes" When I am logged in as a "policy_and_abuse" admin @@ -155,15 +214,37 @@ Feature: Admin Actions for Works, Comments, Series, Bookmarks And I should see "M/M" And I should see "Language: Deutsch" - Scenario: Can delete external works - Given basic tags - And I am logged in as "regular_user" + Scenario Outline: Hiding and un-hiding external works + Given I am logged in as "regular_user" And I bookmark the external work "External Changes" - When I am logged in as a "policy_and_abuse" admin + When I am logged in as a "" admin + And I view the external work "External Changes" + And I follow "Hide External Work" + Then I should see "Item has been hidden." + And I should see "Make External Work Visible" + When I follow "Make External Work Visible" + Then I should see "Item is no longer hidden." + + Examples: + | role | + | superadmin | + | legal | + | policy_and_abuse | + + Scenario Outline: Deleting external works + Given I am logged in as "regular_user" + And I bookmark the external work "External Changes" + When I am logged in as a "" admin And I view the external work "External Changes" And I follow "Delete External Work" Then I should see "Item was successfully deleted." + Examples: + | role | + | superadmin | + | legal | + | policy_and_abuse | + Scenario: Can mark a comment as spam Given I have no works or comments And the following activated users exist @@ -250,8 +331,7 @@ Feature: Admin Actions for Works, Comments, Series, Bookmarks And I should not see "This comment has been marked as spam." Scenario: Admin can edit language on works when posting without previewing - Given basic tags - And basic languages + Given basic languages And I am logged in as "regular_user" And I post the work "Wrong Language" When I am logged in as a "policy_and_abuse" admin @@ -264,8 +344,7 @@ Feature: Admin Actions for Works, Comments, Series, Bookmarks And I should not see "English" Scenario: Admin can edit language on works when previewing first - Given basic tags - And basic languages + Given basic languages And I am logged in as "regular_user" And I post the work "Wrong Language" When I am logged in as a "policy_and_abuse" admin @@ -300,10 +379,10 @@ Feature: Admin Actions for Works, Comments, Series, Bookmarks And the work "Spammity Spam" should not be marked as spam And the work "Spammity Spam" should not be hidden - Scenario: Admin can hide a series (e.g. if the series description or notes contain a TOS Violation) + Scenario Outline: Admin can hide a series (e.g. if the series description or notes contain a TOS Violation) Given I am logged in as "tosser" And I add the work "Legit Work" to series "Violation" - When I am logged in as a "policy_and_abuse" admin + When I am logged in as a "" admin And I view the series "Violation" And I follow "Hide Series" Then I should see "Item has been hidden." @@ -328,10 +407,16 @@ Feature: Admin Actions for Works, Comments, Series, Bookmarks When I view the series "Violation" Then I should see the image "title" text "Hidden by Administrator" - Scenario: Admin can un-hide a series + Examples: + | role | + | superadmin | + | legal | + | policy_and_abuse | + + Scenario Outline: Admin can un-hide a series Given I am logged in as "tosser" And I add the work "Legit Work" to series "Violation" - And I am logged in as a "policy_and_abuse" admin + And I am logged in as a "" admin And I view the series "Violation" And I follow "Hide Series" When I follow "Make Series Visible" @@ -357,6 +442,30 @@ Feature: Admin Actions for Works, Comments, Series, Bookmarks When I view the series "Violation" Then I should see "Violation" + Examples: + | role | + | superadmin | + | legal | + | policy_and_abuse | + + Scenario Outline: Deleting series + Given I am logged in as "tosser" + And I add the work "Legit Work" to series "Violation" + And I am logged in as a "" admin + When I view the series "Violation" + And I follow "Delete Series" + Then I should see "Item was successfully deleted." + When I log out + And I go to tosser's series page + Then I should see "Series (0)" + And I should not see "Violation" + + Examples: + | role | + | superadmin | + | legal | + | policy_and_abuse | + Scenario: Admins can see when a work has too many tags Given the user-defined tag limit is 7 And the work "Under the Limit" @@ -371,8 +480,14 @@ Feature: Admin Actions for Works, Comments, Series, Bookmarks When I view the work "Over the Limit" Then I should see "Over Tag Limit: Yes" - Scenario: Policy abuse admins can see original work creators + Scenario Outline: Certain admins can see original work creators Given a work "Orphaned" with the original creator "orphaneer" - When I am logged in as a "policy_and_abuse" admin + When I am logged in as a "" admin And I view the work "Orphaned" Then I should see the original creator "orphaneer" + + Examples: + | role | + | superadmin | + | legal | + | policy_and_abuse | diff --git a/features/comments_and_kudos/admin_info.feature b/features/comments_and_kudos/admin_info.feature index 89e63d71b9f..5caed5e40e7 100644 --- a/features/comments_and_kudos/admin_info.feature +++ b/features/comments_and_kudos/admin_info.feature @@ -29,6 +29,7 @@ Feature: Some admins can see IP addresses and emails for comments Examples: | role | should_ip | should_email | | superadmin | should | should | + | legal | should | should | | policy_and_abuse | should | should | | support | should not | should | | board | should not | should not | diff --git a/features/importing/work_import.feature b/features/importing/work_import.feature index be878c3d78b..689a006c801 100644 --- a/features/importing/work_import.feature +++ b/features/importing/work_import.feature @@ -120,22 +120,32 @@ Feature: Import Works And I should not see "Additional Tags:" And I should not see "Relationship: Detected 1/Detected 2" - Scenario: Admins see IP address on imported works + Scenario Outline: Admins see IP address on imported works Given I import "http://import-site-with-tags" with a mock website And I press "Post" - When I am logged in as a "policy_and_abuse" admin + When I am logged in as a "" admin And I go to the "Detected Title" work page Then I should see "IP Address: 127.0.0.1" - Scenario: Admins see IP address on works imported without preview + Examples: + | role | + | legal | + | policy_and_abuse | + + Scenario Outline: Admins see IP address on works imported without preview Given I start importing "http://import-site-with-tags" with a mock website And I check "Post without previewing" And I press "Import" - When I am logged in as a "policy_and_abuse" admin + When I am logged in as a "" admin And I go to the "Detected Title" work page Then I should see "IP Address: 127.0.0.1" - Scenario: Admins see IP address on multi-chapter works imported without preview + Examples: + | role | + | legal | + | policy_and_abuse | + + Scenario Outline: Admins see IP address on multi-chapter works imported without preview Given I import the urls with mock websites as chapters without preview """ http://import-site-without-tags @@ -146,6 +156,11 @@ Feature: Import Works Then I should see "Chapters:2/2" And I should see "IP Address: 127.0.0.1" + Examples: + | role | + | legal | + | policy_and_abuse | + Scenario: Imported works can be set to restricted When I start importing "http://import-site-with-tags" with a mock website And I check "Only show imported works to registered users" diff --git a/spec/controllers/admin/admin_users_controller_spec.rb b/spec/controllers/admin/admin_users_controller_spec.rb index 9438a380bfa..7dfb2287b15 100644 --- a/spec/controllers/admin/admin_users_controller_spec.rb +++ b/spec/controllers/admin/admin_users_controller_spec.rb @@ -6,71 +6,76 @@ include LoginMacros include RedirectExpectationHelper - describe "GET #index" do - let(:admin) { create(:admin) } + manage_roles = %w[superadmin legal policy_and_abuse open_doors support tag_wrangling].freeze + search_roles = %w[superadmin legal tag_wrangling support policy_and_abuse open_doors].freeze - context "when admin does not have correct authorization" do - it "redirects with error" do - admin.update!(roles: []) - fake_login_admin(admin) - get :index + shared_examples "denies access to unauthorized admins" do |authorized_roles| + context "with no roles" do + let(:admin) { create(:admin) } + it "redirects with error" do it_redirects_to_with_error(root_url, "Sorry, only an authorized admin can access the page you were trying to reach.") end end - context "when admin has correct authorization" do - it "allows access to index" do - admin.update!(roles: ["policy_and_abuse"]) - fake_login_admin(admin) - get :index + (Admin::VALID_ROLES - authorized_roles).each do |role| + context "with role #{role}" do + let(:admin) { create(:admin, roles: [role]) } - expect(response).to have_http_status(:success) + it "redirects with error" do + it_redirects_to_with_error(root_url, "Sorry, only an authorized admin can access the page you were trying to reach.") + end end end end - describe "GET #bulk_search" do - let(:admin) { create(:admin) } - - context "when admin does not have correct authorization" do - it "redirects with error" do - admin.update!(roles: []) - fake_login_admin(admin) - get :bulk_search + shared_examples "permits access to authorized admins" do + search_roles.each do |role| + context "with role #{role}" do + let(:admin) { create(:admin, roles: [role]) } - it_redirects_to_with_error(root_url, "Sorry, only an authorized admin can access the page you were trying to reach.") + it "allows access" do + expect(response).to have_http_status(:success) + end end end + end - context "when admin has correct authorization" do - it "allows access to access bulk search" do - admin.update!(roles: ["policy_and_abuse"]) - fake_login_admin(admin) - get :bulk_search + describe "GET #index" do + before do + fake_login_admin(admin) + get :index + end - expect(response).to have_http_status(:success) - end + it_behaves_like "denies access to unauthorized admins", search_roles + it_behaves_like "permits access to authorized admins" + end + + describe "GET #bulk_search" do + before do + fake_login_admin(admin) + get :bulk_search end + + it_behaves_like "denies access to unauthorized admins", search_roles + it_behaves_like "permits access to authorized admins" end describe "GET #show" do - let(:admin) { create(:admin) } let(:user) { create(:user) } - context "when admin does not have correct authorization" do - it "redirects with error" do - admin.update!(roles: []) - fake_login_admin(admin) - get :show, params: { id: user.login } - - it_redirects_to_with_error(root_url, "Sorry, only an authorized admin can access the page you were trying to reach.") - end + before do + fake_login_admin(admin) + get :show, params: { id: user.login } end + it_behaves_like "denies access to unauthorized admins", search_roles + it_behaves_like "permits access to authorized admins" + context "when admin has correct authorization" do + let(:admin) { create(:policy_and_abuse_admin) } + it "if user exists, allows access to show page" do - admin.update!(roles: ["policy_and_abuse"]) fake_login_admin(admin) get :show, params: { id: user.login } @@ -78,11 +83,11 @@ end it "if user does not exists, raises a 404" do - admin.update!(roles: ["policy_and_abuse"]) fake_login_admin(admin) params = { id: "not_existing_id" } - expect { get :show, params: params }.to raise_error ActiveRecord::RecordNotFound + expect { get :show, params: params } + .to raise_error ActiveRecord::RecordNotFound end end end @@ -93,21 +98,17 @@ let(:role) { create(:role) } let(:user) { create(:user, email: "user@example.com", roles: [old_role]) } - context "when admin does not have correct authorization" do + it_behaves_like "denies access to unauthorized admins", manage_roles do before do fake_login_admin(admin) - admin.update!(roles: []) - end - - it "redirects with error" do put :update, params: { id: user.login, user: { roles: [] } } - - it_redirects_to_with_error(root_url, "Sorry, only an authorized admin can access the page you were trying to reach.") end end context "when admin has correct authorization" do - before { fake_login_admin(admin) } + before do + fake_login_admin(admin) + end %w[policy_and_abuse superadmin].each do |admin_role| context "when admin has #{admin_role} role" do @@ -182,6 +183,26 @@ end end end + + %w[legal].each do |admin_role| + context "when admin has #{admin_role} role" do + let(:admin) { create(:admin, roles: [admin_role]) } + + it "does not allow updating roles" do + expect do + put :update, params: { id: user.login, user: { roles: [role.id.to_s] } } + end.to raise_exception(ActionController::UnpermittedParameters) + expect(user.reload.roles).not_to include(role) + end + + it "does not allow updating email" do + expect do + put :update, params: { id: user.login, user: { email: "updated@example.com" } } + end.to raise_exception(ActionController::UnpermittedParameters) + expect(user.reload.email).not_to eq("updated@example.com") + end + end + end end end @@ -189,140 +210,126 @@ let(:admin) { create(:admin) } let(:user) { create(:user) } let(:kin) { create(:user) } + authorized_roles = %w[superadmin policy_and_abuse support].freeze - before { fake_login_admin(admin) } - - shared_examples "unauthorized admin cannot add next of kin" do - it "redirects with error" do - post :update_next_of_kin, params: { - user_login: user.login, next_of_kin_name: kin.login, next_of_kin_email: kin.email - } - it_redirects_to_with_error(root_path, "Sorry, only an authorized admin can access the page you were trying to reach.") - expect(user.reload.fannish_next_of_kin).to be_nil - end - end - - shared_examples "authorized admin can add next of kin" do - it "adds next of kin and redirects with notice" do + it_behaves_like "denies access to unauthorized admins", authorized_roles do + before do + fake_login_admin(admin) post :update_next_of_kin, params: { user_login: user.login, next_of_kin_name: kin.login, next_of_kin_email: kin.email } - it_redirects_to_with_notice(admin_user_path(user), "Fannish next of kin was updated.") - expect(user.reload.fannish_next_of_kin.kin).to eq(kin) - expect(user.reload.fannish_next_of_kin.kin_email).to eq(kin.email) end end - context "when admin does not have correct authorization" do - before { admin.update!(roles: []) } - - it_behaves_like "unauthorized admin cannot add next of kin" - end - - %w[superadmin policy_and_abuse support].each do |role| + authorized_roles.each do |role| context "when admin has #{role} role" do let(:admin) { create(:admin, roles: [role]) } - it_behaves_like "authorized admin can add next of kin" - end - end - - context "when admin has support role" do - let(:admin) { create(:support_admin) } - - before { fake_login_admin(admin) } - - it "logs adding a fannish next of kin" do - post :update_next_of_kin, params: { - user_login: user.login, next_of_kin_name: kin.login, next_of_kin_email: kin.email - } - user.reload - expect(user.fannish_next_of_kin.kin).to eq(kin) - log_item = user.log_items.last - expect(log_item.action).to eq(ArchiveConfig.ACTION_ADD_FNOK) - expect(log_item.fnok_user.id).to eq(kin.id) + before do + fake_login_admin(admin) + end - added_log_item = kin.reload.log_items.last - expect(added_log_item.action).to eq(ArchiveConfig.ACTION_ADDED_AS_FNOK) - expect(added_log_item.fnok_user.id).to eq(user.id) + it "adds next of kin and redirects with notice" do + post :update_next_of_kin, params: { + user_login: user.login, next_of_kin_name: kin.login, next_of_kin_email: kin.email + } + it_redirects_to_with_notice(admin_user_path(user), "Fannish next of kin was updated.") + expect(user.reload.fannish_next_of_kin.kin).to eq(kin) + expect(user.reload.fannish_next_of_kin.kin_email).to eq(kin.email) + end - expect_changes_made_by(admin, [log_item, added_log_item]) - end + it "logs adding a fannish next of kin" do + post :update_next_of_kin, params: { + user_login: user.login, next_of_kin_name: kin.login, next_of_kin_email: kin.email + } + user.reload + expect(user.fannish_next_of_kin.kin).to eq(kin) + log_item = user.log_items.last + expect(log_item.action).to eq(ArchiveConfig.ACTION_ADD_FNOK) + expect(log_item.fnok_user.id).to eq(kin.id) + + added_log_item = kin.reload.log_items.last + expect(added_log_item.action).to eq(ArchiveConfig.ACTION_ADDED_AS_FNOK) + expect(added_log_item.fnok_user.id).to eq(user.id) + + expect_changes_made_by(admin, [log_item, added_log_item]) + end - it "logs removing a fannish next of kin" do - kin = create(:fannish_next_of_kin, user: user).kin + it "logs removing a fannish next of kin" do + kin = create(:fannish_next_of_kin, user: user).kin - post :update_next_of_kin, params: { - user_login: user.login - } - user.reload - expect(user.fannish_next_of_kin).to be_nil - log_item = user.log_items.last - expect(log_item.action).to eq(ArchiveConfig.ACTION_REMOVE_FNOK) - expect(log_item.fnok_user.id).to eq(kin.id) + post :update_next_of_kin, params: { + user_login: user.login + } + user.reload + expect(user.fannish_next_of_kin).to be_nil + log_item = user.log_items.last + expect(log_item.action).to eq(ArchiveConfig.ACTION_REMOVE_FNOK) + expect(log_item.fnok_user.id).to eq(kin.id) - removed_log_item = kin.reload.log_items.last - expect(removed_log_item.action).to eq(ArchiveConfig.ACTION_REMOVED_AS_FNOK) - expect(removed_log_item.fnok_user.id).to eq(user.id) + removed_log_item = kin.reload.log_items.last + expect(removed_log_item.action).to eq(ArchiveConfig.ACTION_REMOVED_AS_FNOK) + expect(removed_log_item.fnok_user.id).to eq(user.id) - expect_changes_made_by(admin, [log_item, removed_log_item]) - end + expect_changes_made_by(admin, [log_item, removed_log_item]) + end - it "logs updating a fannish next of kin" do - previous_kin = create(:fannish_next_of_kin, user: user).kin + it "logs updating a fannish next of kin" do + previous_kin = create(:fannish_next_of_kin, user: user).kin - post :update_next_of_kin, params: { - user_login: user.login, next_of_kin_name: kin.login, next_of_kin_email: kin.email - } - user.reload - expect(user.fannish_next_of_kin.kin).to eq(kin) + post :update_next_of_kin, params: { + user_login: user.login, next_of_kin_name: kin.login, next_of_kin_email: kin.email + } + user.reload + expect(user.fannish_next_of_kin.kin).to eq(kin) - remove_log_item = user.log_items[-2] - expect(remove_log_item.action).to eq(ArchiveConfig.ACTION_REMOVE_FNOK) - expect(remove_log_item.fnok_user.id).to eq(previous_kin.id) + remove_log_item = user.log_items[-2] + expect(remove_log_item.action).to eq(ArchiveConfig.ACTION_REMOVE_FNOK) + expect(remove_log_item.fnok_user.id).to eq(previous_kin.id) - add_log_item = user.log_items.last - expect(add_log_item.action).to eq(ArchiveConfig.ACTION_ADD_FNOK) - expect(add_log_item.fnok_user.id).to eq(kin.id) + add_log_item = user.log_items.last + expect(add_log_item.action).to eq(ArchiveConfig.ACTION_ADD_FNOK) + expect(add_log_item.fnok_user.id).to eq(kin.id) - removed_log_item = previous_kin.reload.log_items.last - expect(removed_log_item.action).to eq(ArchiveConfig.ACTION_REMOVED_AS_FNOK) - expect(removed_log_item.fnok_user.id).to eq(user.id) + removed_log_item = previous_kin.reload.log_items.last + expect(removed_log_item.action).to eq(ArchiveConfig.ACTION_REMOVED_AS_FNOK) + expect(removed_log_item.fnok_user.id).to eq(user.id) - added_log_item = kin.reload.log_items.last - expect(added_log_item.action).to eq(ArchiveConfig.ACTION_ADDED_AS_FNOK) - expect(added_log_item.fnok_user.id).to eq(user.id) + added_log_item = kin.reload.log_items.last + expect(added_log_item.action).to eq(ArchiveConfig.ACTION_ADDED_AS_FNOK) + expect(added_log_item.fnok_user.id).to eq(user.id) - expect_changes_made_by(admin, [remove_log_item, add_log_item, removed_log_item, added_log_item]) - end + expect_changes_made_by(admin, [remove_log_item, add_log_item, removed_log_item, added_log_item]) + end - def expect_changes_made_by(admin, log_items) - log_items.each do |log_item| - expect(log_item.admin_id).to eq(admin.id) - expect(log_item.note).to eq("Change made by #{admin.login}") + def expect_changes_made_by(admin, log_items) + log_items.each do |log_item| + expect(log_item.admin_id).to eq(admin.id) + expect(log_item.note).to eq("Change made by #{admin.login}") + end end - end - it "does nothing if changing the fnok to themselves" do - previous_kin = create(:fannish_next_of_kin, user: user) + it "does nothing if changing the fnok to themselves" do + previous_kin = create(:fannish_next_of_kin, user: user) - post :update_next_of_kin, params: { - user_login: user.login, next_of_kin_name: previous_kin.kin.login, next_of_kin_email: previous_kin.kin_email - } - it_redirects_to_with_notice(admin_user_path(user), "No change to fannish next of kin.") - expect(user.reload.log_items).to be_empty - end + post :update_next_of_kin, params: { + user_login: user.login, next_of_kin_name: previous_kin.kin.login, next_of_kin_email: previous_kin.kin_email + } + it_redirects_to_with_notice(admin_user_path(user), "No change to fannish next of kin.") + expect(user.reload.log_items).to be_empty + end - it "errors if trying to add an incomplete fnok" do - post :update_next_of_kin, params: { - user_login: user.login, next_of_kin_email: "" - } + it "errors if trying to add an incomplete fnok" do + post :update_next_of_kin, params: { + user_login: user.login, next_of_kin_email: "" + } - kin = assigns(:user).fannish_next_of_kin - expect(kin).not_to be_valid - expect(kin.errors[:kin_email]).to include("can't be blank") + kin = assigns(:user).fannish_next_of_kin + expect(kin).not_to be_valid + expect(kin.errors[:kin_email]).to include("can't be blank") - expect(user.reload.log_items).to be_empty + expect(user.reload.log_items).to be_empty + end end end end @@ -374,15 +381,22 @@ def expect_changes_made_by(admin, log_items) end end - context "when admin does not have correct authorization" do + context "when the admin has no roles" do before { admin.update!(roles: []) } it_behaves_like "unauthorized admin cannot add note to user" it_behaves_like "unauthorized admin cannot suspend user" end + (Admin::VALID_ROLES - %w[policy_and_abuse support superadmin]).each do |role| + context "when the admin has #{role} role" do + it_behaves_like "unauthorized admin cannot add note to user" + it_behaves_like "unauthorized admin cannot suspend user" + end + end + %w[superadmin policy_and_abuse].each do |role| - context "when admin has #{role} role" do + context "when the admin has #{role} role" do let(:admin) { create(:admin, roles: [role]) } it_behaves_like "authorized admin can add note to user" @@ -390,7 +404,7 @@ def expect_changes_made_by(admin, log_items) end end - context "when admin has support role" do + context "when the admin has support role" do let(:admin) { create(:support_admin) } it_behaves_like "authorized admin can add note to user" @@ -399,53 +413,55 @@ def expect_changes_made_by(admin, log_items) end describe "GET #confirm_delete_user_creations" do - let(:admin) { create(:admin) } let(:user) { create(:user, banned: true) } + authorized_roles = %w[superadmin policy_and_abuse].freeze - context "when admin does not have correct authorization" do - it "redirects with error" do - admin.update!(roles: []) - fake_login_admin(admin) - get :confirm_delete_user_creations, params: { id: user.login } + before do + fake_login_admin(admin) + end - it_redirects_to_with_error(root_url, "Sorry, only an authorized admin can access the page you were trying to reach.") + it_behaves_like "denies access to unauthorized admins", authorized_roles do + before do + get :confirm_delete_user_creations, params: { id: user.login } end end - context "when admin has correct authorization" do - context "when user is not banned" do - it "redirects with error" do - admin.update!(roles: ["policy_and_abuse"]) - fake_login_admin(admin) - user.update!(banned: false) - get :confirm_delete_user_creations, params: { id: user.login } + authorized_roles.each do |role| + context "when logged in as a #{role} admin" do + let(:admin) { create(:admin, roles: [role]) } + + context "when the user is not banned" do + it "redirects with error" do + user.update!(banned: false) + get :confirm_delete_user_creations, params: { id: user.login } - it_redirects_to_with_error(admin_users_path, "That user is not banned!") + it_redirects_to_with_error(admin_users_path, "That user is not banned!") + end end - end - context "when user is banned" do - it "allows admins to access delete user creations page" do - admin.update!(roles: ["policy_and_abuse"]) - fake_login_admin(admin) - user.update!(banned: true) - get :confirm_delete_user_creations, params: { id: user.login } + context "when the user is banned" do + it "allows admins to access delete user creations page" do + user.update!(banned: true) + get :confirm_delete_user_creations, params: { id: user.login } - expect(response).to have_http_status(:success) + expect(response).to have_http_status(:success) + end end end end end describe "POST #destroy_user_creations" do - let(:admin) { create(:admin) } let(:user) { create(:user) } let!(:work) { create(:work, authors: [user.default_pseud]) } let(:other_owner) { create(:user, banned: false) } let!(:collection1) { create(:collection) } let!(:collection2) { create(:collection) } + authorized_roles = %w[superadmin policy_and_abuse].freeze before do + fake_login_admin(admin) + # Banning user only after creating works for them user.update!(banned: true) @@ -455,92 +471,83 @@ def expect_changes_made_by(admin, log_items) create(:collection_participant, user: user, collection: collection2, participant_role: CollectionParticipant::MEMBER) end - context "when admin does not have correct authorization" do - it "redirects with error" do - admin.update!(roles: []) - fake_login_admin(admin) + it_behaves_like "denies access to unauthorized admins", authorized_roles do + before do post :destroy_user_creations, params: { id: user.login } - - it_redirects_to_with_error(root_url, "Sorry, only an authorized admin can access the page you were trying to reach.") end end - context "when admin has correct authorization" do - before do - admin.update!(roles: ["policy_and_abuse"]) - fake_login_admin(admin) - end - - context "when user is not banned" do - it "redirects with error" do - user.update!(banned: false) - post :destroy_user_creations, params: { id: user.login } + authorized_roles.each do |role| + context "when logged in as a #{role} admin" do + let(:admin) { create(:admin, roles: [role]) } - it_redirects_to_with_error(admin_users_path, "That user is not banned!") + context "when the user is not banned" do + it "redirects with error" do + user.update!(banned: false) + post :destroy_user_creations, params: { id: user.login } + + it_redirects_to_with_error(admin_users_path, "That user is not banned!") + end end - end - context "when user is banned" do - it "allows admins to destroy user creations" do - post :destroy_user_creations, params: { id: user.login } - # Check that the first user's collection is deleted - expect(Collection.exists?(collection1.id)).to be_falsey - # Check that the second user's collection still exists - expect(Collection.exists?(other_owner.collections.last.id)).to be_truthy + context "when the user is banned" do + it "allows admins to destroy user creations" do + post :destroy_user_creations, params: { id: user.login } + # Check that the first user's collection is deleted + expect(Collection.exists?(collection1.id)).to be_falsey + # Check that the second user's collection still exists + expect(Collection.exists?(other_owner.collections.last.id)).to be_truthy - it_redirects_to_with_notice(admin_users_path, "All creations by user #{user.login} have been deleted.") - expect(Work.exists?(work.id)).to be false + it_redirects_to_with_notice(admin_users_path, "All creations by user #{user.login} have been deleted.") + expect(Work.exists?(work.id)).to be false + end end end end end describe "GET #troubleshoot" do - let(:admin) { create(:admin) } let(:user) { create(:user) } - context "when admin does not have correct authorization" do - it "redirects with error" do - admin.update!(roles: []) - fake_login_admin(admin) - get :troubleshoot, params: { id: user.login } - - it_redirects_to_with_error(root_url, "Sorry, only an authorized admin can access the page you were trying to reach.") - end + before do + fake_login_admin(admin) + get :troubleshoot, params: { id: user.login } end + it_behaves_like "denies access to unauthorized admins", manage_roles + context "when admin has correct authorization" do - it "allows admins to troublehoot user account" do - admin.update!(roles: ["support"]) - fake_login_admin(admin) - get :troubleshoot, params: { id: user.login } + manage_roles.each do |role| + context "with role #{role}" do + let(:admin) { create(:admin, roles: [role]) } - it_redirects_to_with_notice(root_path, "User account troubleshooting complete.") + it "allows admins to troublehoot user account" do + it_redirects_to_with_notice(root_path, "User account troubleshooting complete.") + end + end end end end describe "POST #activate" do - let(:admin) { create(:admin) } let(:user) { create(:user) } - context "when admin does not have correct authorization" do - it "redirects with error" do - admin.update!(roles: []) - fake_login_admin(admin) - post :activate, params: { id: user.login } - - it_redirects_to_with_error(root_url, "Sorry, only an authorized admin can access the page you were trying to reach.") - end + before do + fake_login_admin(admin) + post :activate, params: { id: user.login } end + it_behaves_like "denies access to unauthorized admins", manage_roles + context "when admin has correct authorization" do - it "allows admins to troublehoot user account" do - admin.update!(roles: ["support"]) - fake_login_admin(admin) - post :activate, params: { id: user.login } + manage_roles.each do |role| + context "with role #{role}" do + let(:admin) { create(:admin, roles: [role]) } - it_redirects_to_with_notice(admin_user_path(id: user.login), "User Account Activated") + it "allows admins to activate the user account" do + it_redirects_to_with_notice(admin_user_path(id: user.login), "User Account Activated") + end + end end end end diff --git a/spec/controllers/comments_controller_spec.rb b/spec/controllers/comments_controller_spec.rb index f414db079f3..d52bda54dcf 100644 --- a/spec/controllers/comments_controller_spec.rb +++ b/spec/controllers/comments_controller_spec.rb @@ -2387,7 +2387,7 @@ end end - %w[superadmin tag_wrangling].each do |admin_role| + %w[superadmin legal tag_wrangling].each do |admin_role| context "with the #{admin_role} role" do it "hides comment and redirects with success message" do admin.update!(roles: [admin_role]) @@ -2453,7 +2453,7 @@ end end - %w[superadmin policy_and_abuse].each do |admin_role| + %w[superadmin legal policy_and_abuse].each do |admin_role| context "with the #{admin_role} role" do it "hides comment and redirects with success message" do admin.update!(roles: [admin_role]) @@ -2557,7 +2557,7 @@ end end - %w[superadmin tag_wrangling].each do |admin_role| + %w[superadmin legal tag_wrangling].each do |admin_role| context "with the #{admin_role} role" do it "leaves comment hidden and redirects with error" do admin.update!(roles: [admin_role]) @@ -2623,7 +2623,7 @@ end end - %w[superadmin policy_and_abuse].each do |admin_role| + %w[superadmin legal policy_and_abuse].each do |admin_role| context "with the #{admin_role} role" do it "leaves comment hidden and redirects with error" do admin.update!(roles: [admin_role]) @@ -2729,7 +2729,7 @@ end end - %w[superadmin tag_wrangling].each do |admin_role| + %w[superadmin legal tag_wrangling].each do |admin_role| context "with the #{admin_role} role" do it "unhides comment and redirects with success message" do admin.update!(roles: [admin_role]) @@ -2795,7 +2795,7 @@ end end - %w[superadmin policy_and_abuse].each do |admin_role| + %w[superadmin legal policy_and_abuse].each do |admin_role| context "with the #{admin_role} role" do it "unhides comment and redirects with success message" do admin.update!(roles: [admin_role]) @@ -2899,7 +2899,7 @@ end end - %w[superadmin tag_wrangling].each do |admin_role| + %w[superadmin legal tag_wrangling].each do |admin_role| context "with the #{admin_role} role" do it "leaves comment unhidden and redirects with error" do admin.update!(roles: [admin_role]) @@ -2965,7 +2965,7 @@ end end - %w[superadmin policy_and_abuse].each do |admin_role| + %w[superadmin legal policy_and_abuse].each do |admin_role| context "with the #{admin_role} role" do it "leaves comment unhidden and redirects with error" do admin.update!(roles: [admin_role]) @@ -3218,7 +3218,7 @@ end end - %w[superadmin board board_assistants_team communications elections policy_and_abuse support].each do |admin_role| + %w[superadmin board board_assistants_team communications elections legal policy_and_abuse support].each do |admin_role| context "with role #{admin_role}" do it "destroys comment and redirects with success message" do admin.update!(roles: [admin_role]) @@ -3271,6 +3271,7 @@ context "when logged in as an admin" do let(:admin) { create(:admin) } + authorized_roles = %w[superadmin board legal policy_and_abuse support] context "with no role" do it "doesn't destroy comment and redirects with error" do @@ -3283,7 +3284,7 @@ end end - (Admin::VALID_ROLES - %w[superadmin board policy_and_abuse support]).each do |admin_role| + (Admin::VALID_ROLES - authorized_roles).each do |admin_role| context "with role #{admin_role}" do it "doesn't destroy comment and redirects with error" do admin.update!(roles: [admin_role]) @@ -3296,7 +3297,7 @@ end end - %w[superadmin board policy_and_abuse support].each do |admin_role| + authorized_roles.each do |admin_role| context "with the #{admin_role} role" do it "destroys comment and redirects with success message" do admin.update!(roles: [admin_role]) @@ -3389,6 +3390,7 @@ context "when logged in as an admin" do let(:admin) { create(:admin) } + authorized_roles = %w[superadmin board legal policy_and_abuse support] context "with no role" do it "doesn't destroy comment and redirects with error" do @@ -3401,7 +3403,7 @@ end end - (Admin::VALID_ROLES - %w[superadmin board policy_and_abuse support]).each do |admin_role| + (Admin::VALID_ROLES - authorized_roles).each do |admin_role| context "with role #{admin_role}" do it "doesn't destroy comment and redirects with error" do admin.update!(roles: [admin_role]) @@ -3414,7 +3416,7 @@ end end - %w[superadmin board policy_and_abuse support].each do |admin_role| + authorized_roles.each do |admin_role| context "with the #{admin_role} role" do it "destroys comment and redirects with success message" do admin.update!(roles: [admin_role]) @@ -3848,7 +3850,7 @@ it_redirects_to_with_error(root_url, "Sorry, only an authorized admin can access the page you were trying to reach.") end - %w[superadmin board support policy_and_abuse].each do |admin_role| + %w[superadmin board legal support policy_and_abuse].each do |admin_role| it "successfully deletes the comment when admin has #{admin_role} role" do admin.update!(roles: [admin_role]) fake_login_admin(admin)