[DFCT0010059] Fixing user permissions for project workflow executions (…

…#618)

* updating the view_workflow_execution policy within project namespace

* adding a couple tests

* updating can_create_export policy to include analysts

* fixing flaky submission tests

app/models/member.rb

@@ -29,7 +29,7 @@ class Member < ApplicationRecord # rubocop:disable Metrics/ClassLength
 
   scope :not_expired, -> { where('expires_at IS NULL OR expires_at > ?', Time.zone.now.beginning_of_day) }
 
-  class << self # rubocop:disable Metrics/ClassLength
+  class << self
     def access_levels(member)
       case member.access_level
       when AccessLevel::OWNER
@@ -121,6 +121,10 @@ def can_update_namespace_with_group_link?(user, object_namespace)
       can_modify?(user, object_namespace)
     end
 
+    def can_view_workflows?(user, object_namespace)
+      effective_access_level(object_namespace, user) >= Member::AccessLevel::ANALYST
+    end
+
     def can_submit_workflow?(user, object_namespace)
       effective_access_level(object_namespace, user) >= Member::AccessLevel::ANALYST
     end
@@ -134,9 +138,7 @@ def user_has_namespace_maintainer_access?(user, namespace, include_group_links =
     end
 
     def can_create_export?(user, object_namespace)
-      Member::AccessLevel.manageable.include?(
-        effective_access_level(object_namespace, user)
-      )
+      effective_access_level(object_namespace, user) >= Member::AccessLevel::ANALYST
     end
 
     def access_level_in_namespace_group_links(user, namespace)

app/policies/namespaces/project_namespace_policy.rb

@@ -155,8 +155,7 @@ def submit_workflow?
     end
 
     def view_workflow_executions?
-      return true if record.parent.user_namespace? && record.parent.owner == user
-      return true if Member.can_view?(user, record) == true
+      return true if Member.can_view_workflows?(user, record) == true
 
       details[:name] = record.name
       false

app/views/layouts/projects.html.erb

@@ -9,48 +9,51 @@
             url: project_path(@project),
             icon: "clipboard_document",
             label: t(:"projects.sidebar.details"),
-            selected: @current_page == t(:"projects.sidebar.details").downcase
+            selected: @current_page == t(:"projects.sidebar.details").downcase,
           ) %>
           <%= render section.with_item(
             url: namespace_project_members_path,
             icon: "users",
             label: t(:"projects.sidebar.members"),
-            selected: @current_page == t(:"projects.sidebar.members").downcase
+            selected: @current_page == t(:"projects.sidebar.members").downcase,
           ) %>
           <%= render section.with_item(
             url: project_samples_path(@project),
             icon: "beaker",
             label: t(:"projects.sidebar.samples"),
-            selected: @current_page == t(:"projects.sidebar.samples").downcase
+            selected: @current_page == t(:"projects.sidebar.samples").downcase,
           ) %>
           <% if allowed_to?(:view_history?, @project) %>
             <%= render section.with_item(
               url: project_history_path(@project),
               icon: "list_bullet",
               label: t(:"projects.sidebar.history"),
-              selected: @current_page == t(:"projects.sidebar.history").downcase
+              selected: @current_page == t(:"projects.sidebar.history").downcase,
+            ) %>
+          <% end %>
+          <% if allowed_to?(:view_workflow_executions?, @project.namespace) %>
+            <%= render section.with_item(
+              url: project_workflow_executions_path(@project),
+              icon: "command_line",
+              label: t(:"projects.sidebar.workflow_executions"),
+              selected:
+                @current_page == t(:"projects.sidebar.workflow_executions").downcase,
             ) %>
           <% end %>
-          <%= render section.with_item(
-            url: project_workflow_executions_path(@project),
-            icon: "command_line",
-            label: t(:"projects.sidebar.workflow_executions"),
-            selected: @current_page == t(:"projects.sidebar.workflow_executions").downcase
-          ) %>
           <% if allowed_to?(:update?, @project) %>
             <%= render section.with_multi_level_menu(title: t(:"projects.sidebar.settings"), icon: "cog_6_tooth") do |multi_level_menu| %>
 
               <% multi_level_menu.with_menu_item(
                 url: project_edit_path(@project),
-                label: t(:"projects.sidebar.general")
+                label: t(:"projects.sidebar.general"),
               ) %>
               <% multi_level_menu.with_menu_item(
                 url: project_bots_path(@project),
-                label: t(:"projects.sidebar.bot_accounts")
+                label: t(:"projects.sidebar.bot_accounts"),
               ) %>
               <% multi_level_menu.with_menu_item(
                 url: project_automated_workflow_executions_path(@project),
-                label: t(:"projects.sidebar.automated_workflow_executions")
+                label: t(:"projects.sidebar.automated_workflow_executions"),
               ) %>
             <% end %>
           <% end %>

config/locales/en.yml

@@ -107,6 +107,7 @@ en:
         update_automated_workflow_executions?: "You are not authorized to update automated workflow executions for project %{name}"
         view_automated_workflow_executions?: "You are not authorized to view automated workflow executions for project %{name}"
         submit_workflow?: You are not authorized to submit workflows for samples within project %{name} on this server.
+        view_workflow_executions?: You are not authorized to view workflow executions for project %{name} on this server.
       namespaces/user_namespace:
         create?: You are not authorized to create a project under the %{name} namespace
         transfer_into_namespace?: You are not authorized to transfer to the user %{name} namespace

test/controllers/projects/workflow_executions_controller_test.rb

@@ -27,6 +27,14 @@ class WorfklowExecutionsControllerTest < ActionDispatch::IntegrationTest
       assert_response :unauthorized
     end
 
+    test 'should not show a listing of project workflow executions for guests' do
+      sign_in users(:ryan_doe)
+
+      get namespace_project_workflow_executions_path(@namespace, @project, format: :turbo_stream)
+
+      assert_response :unauthorized
+    end
+
     test 'should show workflow execution' do
       workflow_execution = workflow_executions(:automated_workflow_execution)
 
@@ -44,6 +52,15 @@ class WorfklowExecutionsControllerTest < ActionDispatch::IntegrationTest
       assert_response :unauthorized
     end
 
+    test 'should not show project workflow execution for guests' do
+      sign_in users(:ryan_doe)
+      workflow_execution = workflow_executions(:automated_workflow_execution)
+
+      get namespace_project_workflow_execution_path(@namespace, @project, workflow_execution)
+
+      assert_response :unauthorized
+    end
+
     test 'should cancel a new workflow with valid params' do
       workflow_execution = workflow_executions(:automated_example_new)
 

test/fixtures/members.yml

@@ -92,6 +92,12 @@ project_one_member_john_doe:
   namespace_id: <%= ActiveRecord::FixtureSet.identify(:project1_namespace, :uuid) %>
   access_level: <%= Member::AccessLevel::OWNER %>
 
+project_one_member_james_doe:
+  user_id: <%= ActiveRecord::FixtureSet.identify(:james_doe, :uuid) %>
+  created_by_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %>
+  namespace_id: <%= ActiveRecord::FixtureSet.identify(:project1_namespace, :uuid) %>
+  access_level: <%= Member::AccessLevel::ANALYST %>
+
 project_one_member_ryan_doe:
   user_id: <%= ActiveRecord::FixtureSet.identify(:ryan_doe, :uuid) %>
   created_by_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %>

test/services/data_exports/create_service_test.rb

@@ -134,6 +134,25 @@ def setup
       end
     end
 
+    test 'analyst authorized to create workflow execution export' do
+      valid_params = { 'export_type' => 'analysis',
+                       'export_parameters' => { 'ids' => [@workflow_execution1.id] } }
+      user = users(:james_doe)
+
+      assert_authorized_to(:export_workflow_execution_data?, @workflow_execution1, with: WorkflowExecutionPolicy,
+                                                                                   context: { user: }) do
+        DataExports::CreateService.new(user, valid_params).execute
+      end
+    end
+
+    test 'guest unauthorized to create workflow execution export' do
+      valid_params = { 'export_type' => 'analysis',
+                       'export_parameters' => { 'ids' => [@workflow_execution1.id] } }
+      user = users(:ryan_doe)
+
+      assert_raises(ActionPolicy::Unauthorized) { DataExports::CreateService.new(user, valid_params).execute }
+    end
+
     test 'data export with valid parameters but unauthorized for workflow execution export' do
       valid_params = { 'export_type' => 'analysis',
                        'export_parameters' => { 'ids' => [@workflow_execution1.id] } }

test/system/workflow_executions/submissions_test.rb

@@ -16,6 +16,8 @@ class SubmissionsTest < ApplicationSystemTestCase
 
       visit namespace_project_samples_url(namespace_id: @namespace.path, project_id: @project.path)
 
+      assert_text 'Displaying 3 items'
+
       within 'table' do
         find("input[type='checkbox'][value='#{@sample43.id}']").click
         find("input[type='checkbox'][value='#{@sample44.id}']").click
@@ -46,6 +48,8 @@ class SubmissionsTest < ApplicationSystemTestCase
 
       visit namespace_project_samples_url(namespace_id: @namespace.path, project_id: @project.path)
 
+      assert_text 'Displaying 3 items'
+
       within 'table' do
         find("input[type='checkbox'][value='#{@sample43.id}']").click
         find("input[type='checkbox'][value='#{@sample44.id}']").click
@@ -76,6 +80,8 @@ class SubmissionsTest < ApplicationSystemTestCase
 
       visit namespace_project_samples_url(namespace_id: @namespace.path, project_id: @project.path)
 
+      assert_text 'Displaying 3 items'
+
       within 'table' do
         find("input[type='checkbox'][value='#{@sample43.id}']").click
         find("input[type='checkbox'][value='#{@sample44.id}']").click
@@ -110,6 +116,8 @@ class SubmissionsTest < ApplicationSystemTestCase
 
       visit namespace_project_samples_url(namespace_id: namespace.path, project_id: project.path)
 
+      assert_text 'Displaying 1 item'
+
       within 'table' do
         find("input[type='checkbox'][value='#{sample.id}']").click
       end
@@ -146,6 +154,8 @@ class SubmissionsTest < ApplicationSystemTestCase
 
       visit group_samples_url(@namespace)
 
+      assert_text 'Displaying 3 items'
+
       within 'table' do
         find("input[type='checkbox'][value='#{@sample43.id}']").click
         find("input[type='checkbox'][value='#{@sample44.id}']").click
@@ -176,6 +186,8 @@ class SubmissionsTest < ApplicationSystemTestCase
 
       visit group_samples_url(@namespace)
 
+      assert_text 'Displaying 3 items'
+
       within 'table' do
         find("input[type='checkbox'][value='#{@sample43.id}']").click
         find("input[type='checkbox'][value='#{@sample44.id}']").click
@@ -206,6 +218,8 @@ class SubmissionsTest < ApplicationSystemTestCase
 
       visit group_samples_url(@namespace)
 
+      assert_text 'Displaying 3 items'
+
       within 'table' do
         find("input[type='checkbox'][value='#{@sample43.id}']").click
         find("input[type='checkbox'][value='#{@sample44.id}']").click