Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AO3-6760 Check admin roles in UnsortedTagsController #4903

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions app/controllers/unsorted_tags_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,17 @@ class UnsortedTagsController < ApplicationController
before_action :check_permission_to_wrangle

def index
authorize :wrangling if logged_in_as_admin?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to use view_access? directly here instead of going via the action because index is one of the actions where the various wrangling related controllers have different access levels.

That way the index? alias can be removed and we won't have someone accidentally converting e.g. TagWranglersController to using the index? permission and giving more roles access than intended.


@tags = UnsortedTag.page(params[:page])
@counts = tag_counts_per_category
end

def mass_update
unless params[:tags].blank?
params[:tags].delete_if {|tag_id, tag_type| tag_type.blank? }
authorize :wrangling if logged_in_as_admin?

if params[:tags].present?
params[:tags].delete_if { |_, tag_type| tag_type.blank? }
tags = UnsortedTag.where(id: params[:tags].keys)
tags.each do |tag|
new_type = params[:tags][tag.id.to_s]
Expand Down
7 changes: 7 additions & 0 deletions app/policies/wrangling_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,20 @@

class WranglingPolicy < ApplicationPolicy
FULL_ACCESS_ROLES = %w[superadmin tag_wrangling].freeze
VIEW_ACCESS_ROLES = (FULL_ACCESS_ROLES + %w[policy_and_abuse]).freeze

def full_access?
user_has_roles?(FULL_ACCESS_ROLES)
end

def view_access?
user_has_roles?(VIEW_ACCESS_ROLES)
end

alias create? full_access?
alias destroy? full_access?
alias index? view_access?
alias mass_update? full_access?
alias show? full_access?
alias report_csv? full_access?
end
5 changes: 5 additions & 0 deletions features/step_definitions/tag_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,11 @@
assert tag.type == tag_type
end

Then "the {string} tag should be an unsorted tag" do |tagname|
tag = Tag.find_by(name: tagname)
expect(tag).to be_a(UnsortedTag)
end

Then(/^the "([^"]*)" tag should (be|not be) canonical$/) do |tagname, canonical|
tag = Tag.find_by(name: tagname)
expected = canonical == "be"
Expand Down
46 changes: 46 additions & 0 deletions features/tags_and_wrangling/tag_wrangling_unsorted.feature
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,49 @@ Feature: Tag Wrangling - Unsorted Tags
When I select "UnsortedTag" from "tag_type"
And I press "Save changes"
Then I should see "Tag was updated."

Scenario Outline: Editing unsorted tags as a fully authorized admin
Given an unsorted_tag exists with name: "Admin unsorted tag"
And I am logged in as a "<role>" admin
When I go to the unsorted_tags page
And I select "Freeform" for the unsorted tag "Admin unsorted tag"
And I press "Update"
Then I should see "Tags were successfully sorted"
And the "Admin unsorted tag" tag should be a "Freeform" tag

Examples:
| role |
| superadmin |
| tag_wrangling |

Scenario Outline: Editing unsorted tags as a view-only admin
Given an unsorted_tag exists with name: "Admin unsorted tag"
And I am logged in as a "<role>" admin
When I go to the unsorted_tags page
And I select "Freeform" for the unsorted tag "Admin unsorted tag"
And I press "Update"
Then I should see "Sorry, only an authorized admin can access the page you were trying to reach."
And the "Admin unsorted tag" tag should be an unsorted tag

Examples:
| role |
| policy_and_abuse |

Scenario Outline: Editing unsorted tags as an unauthorized admin
Given an unsorted_tag exists with name: "Admin unsorted tag"
And I am logged in as a "<role>" admin
When I go to the unsorted_tags page
Then I should see "Sorry, only an authorized admin can access the page you were trying to reach."

Examples:
| role |
| board |
| board_assistants_team |
| communications |
| development_and_membership |
| docs |
| elections |
| legal |
| translation |
| support |
| open_doors |
63 changes: 63 additions & 0 deletions spec/controllers/unsorted_tags_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# frozen_string_literal: true

require "spec_helper"

describe UnsortedTagsController do
include LoginMacros
include RedirectExpectationHelper

describe "POST #mass_update" do
context "when accessing as a guest" do
before do
post :mass_update
end

it "redirects with an error" do
it_redirects_to_with_error(
new_user_session_path,
"Sorry, you don't have permission to access the page you were trying to reach. Please log in."
)
end
end

context "when logged in as a non-tag-wrangler user" do
let(:user) { create(:user) }

before do
fake_login_known_user(user)
post :mass_update
end

it "redirects with an error" do
it_redirects_to_with_error(
user_path(user),
"Sorry, you don't have permission to access the page you were trying to reach."
)
end
end

context "when logged in as an admin with no roles" do
before do
fake_login_admin(create(:admin))
post :mass_update
end

it "redirects with an error" do
it_redirects_to_with_error(root_path, "Sorry, only an authorized admin can access the page you were trying to reach.")
end
end

(Admin::VALID_ROLES - %w[superadmin tag_wrangling]).each do |admin_role|
context "when logged in as a #{admin_role} admin" do
before do
fake_login_admin(create(:admin, roles: [admin_role]))
post :mass_update
end

it "redirects with an error" do
it_redirects_to_with_error(root_path, "Sorry, only an authorized admin can access the page you were trying to reach.")
end
end
end
end
end
Loading