AO3-6697 Limit roles that can edit known issues (otwcode#4891)

* AO3-6697 Limit roles that can edit known issues * Tests without weird workarounds * Test consolidation and formatting cleanup * Make things translatable * Add missing tests * Hide posts button in admin nav when unauthorised * Fix issue from merge
Bilka2 · Nov 28, 2024 · dfaa140 · dfaa140
1 parent 453f0e6
commit dfaa140
Show file tree

Hide file tree

Showing 9 changed files with 269 additions and 40 deletions.
diff --git a/app/controllers/known_issues_controller.rb b/app/controllers/known_issues_controller.rb
@@ -1,5 +1,4 @@
 class KnownIssuesController < ApplicationController
-
   before_action :admin_only, except: [:index]
 
   # GET /known_issues
@@ -9,25 +8,24 @@ def index
 
   # GET /known_issues/1
   def show
-    @known_issue = KnownIssue.find(params[:id])
+    @known_issue = authorize KnownIssue.find(params[:id])
   end
 
   # GET /known_issues/new
   def new
-    @known_issue = KnownIssue.new
+    @known_issue = authorize KnownIssue.new
   end
 
   # GET /known_issues/1/edit
   def edit
-    @known_issue = KnownIssue.find(params[:id])
+    @known_issue = authorize KnownIssue.find(params[:id])
   end
 
   # POST /known_issues
   def create
-    @known_issue = KnownIssue.new(known_issue_params)
-
+    @known_issue = authorize KnownIssue.new(known_issue_params)
     if @known_issue.save
-      flash[:notice] = 'Known issue was successfully created.'
+      flash[:notice] = "Known issue was successfully created."
       redirect_to(@known_issue)
     else
       render action: "new"
@@ -36,10 +34,9 @@ def create
 
   # PUT /known_issues/1
   def update
-    @known_issue = KnownIssue.find(params[:id])
-
+    @known_issue = authorize KnownIssue.find(params[:id])
     if @known_issue.update(known_issue_params)
-      flash[:notice] = 'Known issue was successfully updated.'
+      flash[:notice] = "Known issue was successfully updated."
       redirect_to(@known_issue)
     else
       render action: "edit"
@@ -48,7 +45,7 @@ def update
 
   # DELETE /known_issues/1
   def destroy
-    @known_issue = KnownIssue.find(params[:id])
+    @known_issue = authorize KnownIssue.find(params[:id])
     @known_issue.destroy
     redirect_to(known_issues_path)
   end

diff --git a/app/policies/known_issue_policy.rb b/app/policies/known_issue_policy.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+class KnownIssuePolicy < ApplicationPolicy
+  MANAGE_ROLES = %w[superadmin support].freeze
+
+  def admin_index?
+    user_has_roles?(MANAGE_ROLES)
+  end
+
+  alias destroy? admin_index?
+  alias edit? admin_index?
+  alias create? admin_index?
+  alias new? admin_index?
+  alias show? admin_index?
+  alias update? admin_index?
+end
diff --git a/app/views/admin/_admin_nav.html.erb b/app/views/admin/_admin_nav.html.erb
@@ -1,12 +1,13 @@
-<h3 class="landmark heading"><%= ts("Admin Navigation") %></h3>
+<h3 class="landmark heading"><%= t(".landmark") %></h3>
 <ul class="navigation actions" role="navigation">
   <li>
-    <%= span_if_current ts("AO3 News", key: "header"), admin_posts_path %>
-   </li>
-  <li>
-    <%= span_if_current ts("Post AO3 News", key: "header"),
-                        new_admin_post_path %>
-</li>
+    <%= span_if_current t(".ao3_news"), admin_posts_path %>
+  </li>
+  <% if policy(AdminPost).can_post? %>
+    <li>
+      <%= span_if_current t(".post_ao3_news"), new_admin_post_path %>
+    </li>
+  <% end %>
   <% if params[:controller] == "admin_posts" && params[:action] == "edit" %>
     <li>
       <%= link_to t(".news.delete_post"),
@@ -18,8 +19,7 @@
   <% if params[:controller] == "archive_faqs" && params[:action] == "edit" &&
       Globalize.locale.to_s == "en" %>
     <li>
-      <%= link_to ts("Reorder Questions"),
-                  manage_archive_faq_questions_path(@archive_faq) %>
+      <%= link_to t(".faq.reorder_questions"), manage_archive_faq_questions_path(@archive_faq) %>
     </li>
   <% end %>
 
@@ -28,9 +28,11 @@
       <%= span_if_current t(".archive_faq"), archive_faqs_path %>
     </li>
   <% end %>
-  <li>
-    <%= span_if_current ts("Known Issues", key: "header"), known_issues_path %>
-  </li>
+  <% if policy(KnownIssue).admin_index? %>
+    <li>
+      <%= span_if_current t(".known_issues"), known_issues_path %>
+    </li>
+  <% end %>
   <% if policy(:wrangling).new? %>
     <li>
       <%= span_if_current t(".wrangling_guidelines"), wrangling_guidelines_path %>

diff --git a/app/views/admin/_header.html.erb b/app/views/admin/_header.html.erb
@@ -30,7 +30,9 @@
         <% if policy(ArchiveFaq).translation_access? %>
           <li><%= link_to t(".nav.posts.faqs"), archive_faqs_path %></li>
         <% end %>
-        <li><%= link_to t(".nav.posts.known_issues"), known_issues_path %></li>
+        <% if policy(KnownIssue).admin_index? %>
+          <li><%= link_to t(".nav.posts.known_issues"), known_issues_path %></li>
+        <% end %>
         <% if policy(:wrangling).new? %>
           <li><%= link_to t(".nav.posts.wrangling_guidelines"), wrangling_guidelines_path %></li>
         <% end %>

diff --git a/app/views/known_issues/index.html.erb b/app/views/known_issues/index.html.erb
@@ -1,5 +1,5 @@
 <!--Descriptive page name, messages and instructions-->
-<% if logged_in_as_admin? %>
+<% if policy(KnownIssue).admin_index? %>
   <%= render :partial => "admin_index" %>
 <% else %>
   <h2 class="heading"><%= ts("Known Issues") %></h2>

diff --git a/config/locales/views/en.yml b/config/locales/views/en.yml
@@ -92,9 +92,15 @@ en:
           requests: Manage Requests
         page_heading: Invite New Users
     admin_nav:
+      ao3_news: AO3 News
       archive_faq: Archive FAQ
+      faq:
+        reorder_questions: Reorder Questions
+      known_issues: Known Issues
+      landmark: Admin Navigation
       news:
         delete_post: Delete Post
+      post_ao3_news: Post AO3 News
       wrangling_guidelines: Wrangling Guidelines
     admin_options:
       delete:

diff --git a/features/admins/admin_post_issues.feature b/features/admins/admin_post_issues.feature
@@ -3,9 +3,9 @@ Feature: Admin Actions to Post Known Issues
   As an an admin
   I want to be able to report known issues
 
-  Scenario: Post known issues
-    When I am logged in as an admin
-      And I follow "Admin Posts"
+  Scenario Outline: Authorized admin posts, edits, and deletes known issues
+    Given I am logged in as a "<role>" admin
+    When I follow "Admin Posts"
       And I follow "Known Issues" within "#header"
       And I follow "make a new known issues post"
       And I fill in "known_issue_title" with "First known problem"
@@ -18,15 +18,36 @@ Feature: Admin Actions to Post Known Issues
       And I follow "Known Issues" within "#header"
       And I follow "Show"
     Then I should see "First known problem"
-
-  Scenario: Edit known issues
-    Given I have posted known issues
     When I edit known issues
     Then I should see "Known issue was successfully updated"
       And I should not see "First known problem"
       And I should see "This is a bit of a problem, and this is too"
-
-  Scenario: Delete known issues
-    Given I have posted known issues
     When I delete known issues
     Then I should not see "First known problem"
+
+    Examples:
+    | role       |
+    | support    |
+    | superadmin |
+
+  Scenario Outline: Links to edit and create known issues are not shown to unauthorized admins
+    Given I have posted known issues
+      And I am logged in as a "<role>" admin
+    When I follow "Admin Posts"
+    Then I should not see "Known Issues" within "#header"
+    When I go to the known issues page
+    Then I should not see "Edit" within ".actions"
+
+    Examples:
+    | role                       |
+    | board                      |
+    | board_assistants_team      |
+    | communications             |
+    | development_and_membership |
+    | docs                       |
+    | elections                  |
+    | legal                      |
+    | translation                |
+    | tag_wrangling              |
+    | policy_and_abuse           |
+    | open_doors                 |
diff --git a/features/step_definitions/admin_steps.rb b/features/step_definitions/admin_steps.rb
@@ -102,8 +102,8 @@
   click_button("Update")
 end
 
-Given /^I have posted known issues$/ do
-  step %{I am logged in as an admin}
+Given "I have posted known issues" do
+  step %{I am logged in as a super admin}
   step %{I follow "Admin Posts"}
   step %{I follow "Known Issues" within "#header"}
   step %{I follow "make a new known issues post"}
@@ -299,8 +299,7 @@
   Resque.enqueue(AdminSetting, :check_queue)
 end
 
-When /^I edit known issues$/ do
-  step %{I am logged in as an admin}
+When "I edit known issues" do
   step %{I follow "Admin Posts"}
   step %{I follow "Known Issues" within "#header"}
   step %{I follow "Edit"}
@@ -309,8 +308,7 @@
   step %{I press "Post"}
 end
 
-When /^I delete known issues$/ do
-  step %{I am logged in as an admin}
+When "I delete known issues" do
   step %{I follow "Admin Posts"}
   step %{I follow "Known Issues" within "#header"}
   step %{I follow "Delete"}