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-6759 Check admin roles for actions in TagWranglersController #4885

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
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
10 changes: 9 additions & 1 deletion app/controllers/tag_wranglers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ class TagWranglersController < ApplicationController
before_action :check_permission_to_wrangle, except: [:report_csv]

def index
authorize :wrangling, :full_access? if logged_in_as_admin?

@wranglers = Role.find_by(name: "tag_wrangler").users.alphabetical
conditions = ["canonical = 1"]
joins = "LEFT JOIN wrangling_assignments ON (wrangling_assignments.fandom_id = tags.id)
Expand Down Expand Up @@ -39,14 +41,16 @@ def index
end

def show
authorize :wrangling if logged_in_as_admin?

@wrangler = User.find_by!(login: params[:id])
@page_subtitle = @wrangler.login
@fandoms = @wrangler.fandoms.by_name
@counts = tag_counts_per_category
end

def report_csv
authorize :tag_wrangler, :report_csv?
authorize :wrangling

wrangler = User.find_by!(login: params[:id])
wrangled_tags = Tag
Expand All @@ -64,6 +68,8 @@ def report_csv
end

def create
authorize :wrangling if logged_in_as_admin?

unless params[:tag_fandom_string].blank?
names = params[:tag_fandom_string].gsub(/$/, ',').split(',').map(&:strip)
fandoms = Fandom.where('name IN (?)', names)
Expand Down Expand Up @@ -95,6 +101,8 @@ def create
end

def destroy
authorize :wrangling if logged_in_as_admin?

wrangler = User.find_by(login: params[:id])
assignment = WranglingAssignment.where(user_id: wrangler.id, fandom_id: params[:fandom_id]).first
assignment.destroy
Expand Down
7 changes: 0 additions & 7 deletions app/policies/tag_wrangler_policy.rb

This file was deleted.

14 changes: 14 additions & 0 deletions app/policies/wrangling_policy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# frozen_string_literal: true

class WranglingPolicy < ApplicationPolicy
FULL_ACCESS_ROLES = %w[superadmin tag_wrangling].freeze

def full_access?
user_has_roles?(FULL_ACCESS_ROLES)
end

alias create? full_access?
alias destroy? full_access?
alias show? full_access?
alias report_csv? full_access?
end
2 changes: 1 addition & 1 deletion app/views/tag_wranglers/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<p class="icon"><%= icon_display(@wrangler) %></p>
<!--/descriptions-->
<!--subnav-->
<% if policy(:tag_wrangler).report_csv? %>
<% if policy(:wrangling).report_csv? %>
<ul class="navigation actions" role="navigation">
<li><%= link_to t(".tags_wrangled_csv"), report_csv_tag_wrangler_path %></li>
</ul>
Expand Down
8 changes: 8 additions & 0 deletions factories/wrangling_assignments.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# frozen_string_literal: true

FactoryBot.define do
factory :wrangling_assignment do
user
fandom
end
end
2 changes: 1 addition & 1 deletion features/tags_and_wrangling/tag_wrangling_admin.feature
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ Feature: Tag wrangling
Scenario: Admin can remove a user's wrangling assignments

Given the tag wrangler "tangler" with password "wr@ngl3r" is wrangler of "Testing"
When I am logged in as an admin
When I am logged in as a "tag_wrangling" admin
And I am on the wranglers page
And I follow "x"
Then I should see "Wranglers were successfully unassigned!"
Expand Down
158 changes: 135 additions & 23 deletions spec/controllers/tag_wranglers_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,79 @@
include LoginMacros
include RedirectExpectationHelper

wrangling_roles = %w[superadmin tag_wrangling]

let(:user) { create(:tag_wrangler) }

shared_examples "denies access to unauthorized admins" do |verb, action, **kwargs|
context "when logged in as an admin with no role" do
let(:admin) { create(:admin) }

before do
fake_login_admin(admin)
send(verb, action, **kwargs)
brianjaustin marked this conversation as resolved.
Show resolved Hide resolved
end

it "redirects with an 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

(Admin::VALID_ROLES - wrangling_roles).each do |admin_role|
context "when logged in as an admin with role #{admin_role}" do
let(:admin) { create(:admin, roles: [admin_role]) }

before do
fake_login_admin(admin)
send(verb, action, **kwargs)
end

it "redirects with an 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

before(:all) do
Role.create!(name: "tag_wrangler")
end

after(:all) do
# Clean up when we're done, a few other specs become unhappy otherwise
Role.find_by(name: "tag_wrangler").destroy!
end
brianjaustin marked this conversation as resolved.
Show resolved Hide resolved

describe "#index" do
it_behaves_like "denies access to unauthorized admins", :get, :index

wrangling_roles.each do |admin_role|
context "when logged in as an admin with role #{admin_role}" do
let(:admin) { create(:admin, roles: [admin_role]) }

before do
fake_login_admin(admin)
end

it "allows access" do
get :index
expect(response).to have_http_status(:success)
end
end
end

context "when logged in as a tag wrangler" do
before do
fake_login_known_user(user)
end

it "allows access" do
get :index
expect(response).to have_http_status(:success)
end
end
end

describe "#show" do
brianjaustin marked this conversation as resolved.
Show resolved Hide resolved
before { fake_login_known_user(user) }

Expand All @@ -16,6 +87,23 @@
end.to raise_exception(ActiveRecord::RecordNotFound)
end
end

it_behaves_like "denies access to unauthorized admins", :get, :show, params: { id: -1 }
brianjaustin marked this conversation as resolved.
Show resolved Hide resolved

wrangling_roles.each do |admin_role|
context "when logged in as an admin with role #{admin_role}" do
let(:admin) { create(:admin, roles: [admin_role]) }

before do
fake_login_admin(admin)
end

it "allows access" do
get :show, params: { id: user.login }
expect(response).to have_http_status(:success)
end
end
end
end

describe "#report_csv" do
Expand All @@ -36,28 +124,12 @@
it_behaves_like "prevents access to the report"
end

context "when logged in as an admin without proper authorization" do
before { fake_login_admin(admin) }

context "with no role" do
let(:admin) { create(:admin) }

it_behaves_like "prevents access to the report"
end

(Admin::VALID_ROLES - %w[superadmin tag_wrangling]).each do |admin_role|
context "with role #{admin_role}" do
let(:admin) { create(:admin, roles: [admin_role]) }

it_behaves_like "prevents access to the report"
end
end
end
it_behaves_like "denies access to unauthorized admins", :get, :report_csv, params: { id: 0 }

context "when logged in as an admin with proper authorization" do
before { fake_login_admin(admin) }

%w[superadmin tag_wrangling].each do |admin_role|
wrangling_roles.each do |admin_role|
context "with role #{admin_role}" do
let(:admin) { create(:admin, roles: [admin_role]) }

Expand All @@ -75,7 +147,7 @@
result = CSV.parse(response.body.encode("utf-8")[1..], col_sep: "\t")

expect(result)
.to eq([%w[Name Last\ Updated Type Merger Fandoms Unwrangleable],
.to eq([["Name", "Last Updated", "Type", "Merger", "Fandoms", "Unwrangleable"],
[tag1.name, tag1.updated_at.to_s, tag1.type, "", "", "false"]])
end

Expand All @@ -100,7 +172,7 @@
result = CSV.parse(response.body.encode("utf-8")[1..], col_sep: "\t")

expect(result)
.to eq([%w[Name Last\ Updated Type Merger Fandoms Unwrangleable],
.to eq([["Name", "Last Updated", "Type", "Merger", "Fandoms", "Unwrangleable"],
[tag2.name, tag2.updated_at.to_s, tag2.type, tag1.name, "", "false"],
[tag1.name, tag1.updated_at.to_s, tag1.type, "", "", "false"]])
end
Expand All @@ -114,7 +186,7 @@
result = CSV.parse(response.body.encode("utf-8")[1..], col_sep: "\t")

expect(result)
.to eq([%w[Name Last\ Updated Type Merger Fandoms Unwrangleable],
.to eq([["Name", "Last Updated", "Type", "Merger", "Fandoms", "Unwrangleable"],
[tag.name, tag.updated_at.to_s, tag.type, "", fandom.name, "false"]])
end

Expand All @@ -129,7 +201,7 @@
result = CSV.parse(response.body.encode("utf-8")[1..], col_sep: "\t")

expect(result)
.to eq([%w[Name Last\ Updated Type Merger Fandoms Unwrangleable],
.to eq([["Name", "Last Updated", "Type", "Merger", "Fandoms", "Unwrangleable"],
[tag.name, tag.updated_at.to_s, tag.type, "", "#{fandom1.name}, #{fandom2.name}", "false"]])
end

Expand All @@ -151,11 +223,51 @@
result = CSV.parse(response.body.encode("utf-8")[1..], col_sep: "\t")

expect(result)
.to eq([%w[Name Last\ Updated Type Merger Fandoms Unwrangleable],
.to eq([["Name", "Last Updated", "Type", "Merger", "Fandoms", "Unwrangleable"],
[tag.name, tag.updated_at.to_s, tag.type, "", "", "true"]])
end
end
end
end
end

describe "#create" do
it_behaves_like "denies access to unauthorized admins", :post, :create

wrangling_roles.each do |admin_role|
context "when logged in as an admin with role #{admin_role}" do
let(:admin) { create(:admin, roles: [admin_role]) }
let(:fandom) { create(:fandom) }

before do
fake_login_admin(admin)
end

it "creates wrangling assignments" do
post :create, params: { assignments: { fandom.id.to_s => [user.login] } }
it_redirects_to_with_notice(tag_wranglers_path, "Wranglers were successfully assigned!")
end
end
end
end

describe "#destroy" do
it_behaves_like "denies access to unauthorized admins", :delete, :destroy, params: { id: 0, fandom_id: 0 }

wrangling_roles.each do |admin_role|
context "when logged in as an admin with role #{admin_role}" do
let(:admin) { create(:admin, roles: [admin_role]) }
let(:wrangling_assignment) { create(:wrangling_assignment) }

before do
fake_login_admin(admin)
end

it "removes the wrangling assignment" do
delete :destroy, params: { id: wrangling_assignment.user.login, fandom_id: wrangling_assignment.fandom.id }
it_redirects_to_with_notice(tag_wranglers_path, "Wranglers were successfully unassigned!")
end
end
end
end
end
Loading