Attachments - Policy and Service updates for Group and Project attach…

…ments (#752)

* start group and projects attachments

* add policies to create service, remove controller and routes for now

* add migration for attachments_updated_at column to namespace table

* add service tests for policy

* add policy and destroy tests

* cleanup

* fix tests

* readd schema changes after rebase

* fix create activity in attachment destroy service

* add fix to projects sidebar

* add db trigger and migration to exclude attachments_updated_at for namespaceS

* reset log data tests

app/models/member.rb

@@ -210,6 +210,22 @@ def manager_emails(namespace, locale, member = nil) # rubocop:disable Metrics/Ab
       manager_emails
     end
 
+    def can_view_attachments?(user, object_namespace, include_group_links = true) # rubocop:disable Style/OptionalBooleanParameter
+      effective_access_level(object_namespace, user, include_group_links) >= Member::AccessLevel::ANALYST
+    end
+
+    def can_create_attachment?(user, object_namespace, include_group_links = true) # rubocop:disable Style/OptionalBooleanParameter
+      Member::AccessLevel.manageable.include?(
+        effective_access_level(object_namespace, user, include_group_links)
+      )
+    end
+
+    def can_destroy_attachment?(user, object_namespace, include_group_links = true) # rubocop:disable Style/OptionalBooleanParameter
+      Member::AccessLevel.manageable.include?(
+        effective_access_level(object_namespace, user, include_group_links)
+      )
+    end
+
     def ransackable_attributes(_auth_object = nil)
       %w[access_level created_at expires_at]
     end

app/models/namespace.rb

@@ -21,6 +21,8 @@ class Namespace < ApplicationRecord # rubocop:disable Metrics/ClassLength
            },
            class_name: 'Namespace', foreign_key: :parent_id, inverse_of: :parent, dependent: :destroy
 
+  has_many :attachments, as: :attachable, dependent: :destroy
+
   validates :owner, presence: true, if: ->(n) { n.owner_required? }
   validates :name, presence: true, length: { minimum: 3, maximum: 255 }
   validates :name, uniqueness: { case_sensitive: false, scope: %i[type] }, if: -> { parent_id.blank? }

app/policies/group_policy.rb

@@ -191,6 +191,27 @@ def update_sample_metadata?
     false
   end
 
+  def view_attachments?
+    return true if Member.can_view_attachments?(user, record) == true
+
+    details[:name] = record.name
+    false
+  end
+
+  def create_attachment?
+    return true if Member.can_create_attachment?(user, record) == true
+
+    details[:name] = record.name
+    false
+  end
+
+  def destroy_attachment?
+    return true if Member.can_destroy_attachment?(user, record) == true
+
+    details[:name] = record.name
+    false
+  end
+
   scope_for :relation do |relation|
     relation.with(
       user_groups: relation.where(id: user.members.not_expired.select(:namespace_id)).self_and_descendant_ids,

app/policies/project_policy.rb

@@ -151,6 +151,30 @@ def submit_workflow?
     false
   end
 
+  def view_attachments?
+    return true if record.namespace.parent.user_namespace? && record.namespace.parent.owner == user
+    return true if Member.can_view_attachments?(user, record.namespace) == true
+
+    details[:name] = record.name
+    false
+  end
+
+  def create_attachment?
+    return true if record.namespace.parent.user_namespace? && record.namespace.parent.owner == user
+    return true if Member.can_create_attachment?(user, record.namespace) == true
+
+    details[:name] = record.name
+    false
+  end
+
+  def destroy_attachment?
+    return true if record.namespace.parent.user_namespace? && record.namespace.parent.owner == user
+    return true if Member.can_destroy_attachment?(user, record.namespace) == true
+
+    details[:name] = record.name
+    false
+  end
+
   scope_for :relation do |relation| # rubocop:disable Metrics/BlockLength
     relation
       .with(

app/services/attachments/create_service.rb

@@ -28,7 +28,7 @@ def initialize(user = nil, attachable = nil, params = {})
     end
 
     def execute # rubocop:disable Metrics/CyclomaticComplexity, Metrics/AbcSize, Metrics/PerceivedComplexity, Metrics/MethodLength
-      authorize! @attachable.project, to: :update_sample? if @attachable.instance_of?(Sample)
+      attachable_authorization
 
       valid_fastq_attachments = @attachments.select { |attachment| attachment.valid? && attachment.fastq? }
 
@@ -64,6 +64,16 @@ def execute # rubocop:disable Metrics/CyclomaticComplexity, Metrics/AbcSize, Met
 
     private
 
+    def attachable_authorization
+      if @attachable.instance_of?(Sample)
+        authorize! @attachable.project, to: :update_sample?
+      elsif @attachable.instance_of?(Namespaces::ProjectNamespace)
+        authorize! @attachable.project, to: :create_attachment?
+      elsif @attachable.instance_of?(Group)
+        authorize! @attachable, to: :create_attachment?
+      end
+    end
+
     def identify_illumina_paired_end_files(attachments)
       # auto-vivify hash, as found on stack overflow http://stackoverflow.com/questions/5878529/how-to-assign-hashab-c-if-hasha-doesnt-exist
       illumina_pe = Hash.new { |h, k| h[k] = {} }

app/services/attachments/destroy_service.rb

@@ -11,7 +11,8 @@ def initialize(attachable, attachment, user = nil)
     end
 
     def execute # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
-      authorize! @attachable, to: :destroy_attachment?
+      attachable_authorization
+
       destroyed_attachments = []
       if @attachment.attachable_id != @attachable.id
         raise AttachmentsDestroyError, I18n.t('services.attachments.destroy.does_not_belong_to_attachable')
@@ -30,22 +31,36 @@ def execute # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
 
       attachment_destroyed = @attachment.destroy
 
-      if attachment_destroyed
-        @attachable.project.namespace.create_activity key: 'namespaces_project_namespace.samples.attachment.destroy',
-                                                      owner: current_user,
-                                                      trackable_id: @attachable.id,
-                                                      parameters: {
-                                                        sample_puid: @attachable.puid,
-                                                        sample_id: @attachable.id,
-                                                        action: 'attachment_destroy'
-                                                      }
-      end
+      create_activity if attachment_destroyed
 
       destroyed_attachments.append(@attachment)
       destroyed_attachments
     rescue Attachments::DestroyService::AttachmentsDestroyError => e
       @attachment.errors.add(:base, e.message)
       destroyed_attachments
     end
+
+    private
+
+    def attachable_authorization
+      if @attachable.instance_of?(Namespaces::ProjectNamespace)
+        authorize! @attachable.project, to: :destroy_attachment?
+      else
+        authorize! @attachable, to: :destroy_attachment?
+      end
+    end
+
+    def create_activity
+      return unless @attachable.instance_of?(Sample)
+
+      @attachable.project.namespace.create_activity key: 'namespaces_project_namespace.samples.attachment.destroy',
+                                                    owner: current_user,
+                                                    trackable_id: @attachable.id,
+                                                    parameters: {
+                                                      sample_puid: @attachable.puid,
+                                                      sample_id: @attachable.id,
+                                                      action: 'attachment_destroy'
+                                                    }
+    end
   end
 end

app/views/layouts/projects.html.erb

@@ -47,7 +47,7 @@
                 t(:"projects.sidebar.general"),
                 t(:"projects.sidebar.bot_accounts"),
                 t(:"projects.sidebar.automated_workflow_executions"),
-                t(:"projects.sidebar.activity")
+                t(:"projects.sidebar.history")
               ],
               current_page: @current_page
             ) do |multi_level_menu| %>

config/locales/en.yml

@@ -9,10 +9,12 @@ en:
       edit?: You are not authorized to update this resource on this server.
       group:
         create?: You are not authorized to create a project under group %{name} on this server.
+        create_attachment?: You are not authorized to create attachments for group %{name} on this server.
         create_bot_accounts?: You are not authorized to create bot accounts for group %{name} on this server.
         create_member?: You are not authorized to create members for group %{name} on this server.
         create_subgroup?: You are not authorized to create a subgroup within group %{name} on this server.
         destroy?: You are not authorized to remove group %{name} from this server.
+        destroy_attachment?: You are not authorized to destroy attachments for group %{name} on this server.
         destroy_bot_accounts?: You are not authorized to destroy bot accounts for group %{name} on this server.
         destroy_member?: You are not authorized to remove members for group %{name} on this server.
         edit?: You are not authorized to modify group %{name} on this server.
@@ -58,8 +60,10 @@ en:
         activity?: You are not authorized to view the activity for project %{name} on this server.
         clone_sample?: You are not authorized to clone samples from project %{name} on this server.
         clone_sample_into_project?: You are not authorized to clone samples to project %{name} on this server.
+        create_attachment?: You are not authorized to create attachments for project %{name} on this server.
         create_sample?: You are not authorized to create samples for project %{name} on this server.
         destroy?: You are not authorized to remove project %{name} from this server.
+        destroy_attachment?: You are not authorized to destroy attachments for project %{name} on this server.
         destroy_sample?: You are not authorized to remove samples from project %{name} on this server.
         edit?: You are not authorized to modify project %{name} on this server.
         new?: You are not authorized to create a project under the %{name} namespace on this server.

config/locales/fr.yml

@@ -9,10 +9,12 @@ fr:
       edit?: You are not authorized to update this resource on this server.
       group:
         create?: You are not authorized to create a project under group %{name} on this server.
+        create_attachment?: You are not authorized to create attachments for group %{name} on this server.
         create_bot_accounts?: You are not authorized to create bot accounts for group %{name} on this server.
         create_member?: You are not authorized to create members for group %{name} on this server.
         create_subgroup?: You are not authorized to create a subgroup within group %{name} on this server.
         destroy?: You are not authorized to remove group %{name} from this server.
+        destroy_attachment?: You are not authorized to destroy attachments for group %{name} on this server.
         destroy_bot_accounts?: You are not authorized to destroy bot accounts for group %{name} on this server.
         destroy_member?: You are not authorized to remove members for group %{name} on this server.
         edit?: You are not authorized to modify group %{name} on this server.
@@ -58,8 +60,10 @@ fr:
         activity?: You are not authorized to view the activity for project %{name} on this server.
         clone_sample?: You are not authorized to clone samples from project %{name} on this server.
         clone_sample_into_project?: You are not authorized to clone samples to project %{name} on this server.
+        create_attachment?: You are not authorized to create attachments for project %{name} on this server.
         create_sample?: You are not authorized to create samples for project %{name} on this server.
         destroy?: You are not authorized to remove project %{name} from this server.
+        destroy_attachment?: You are not authorized to destroy attachments for project %{name} on this server.
         destroy_sample?: You are not authorized to remove samples from project %{name} on this server.
         edit?: You are not authorized to modify project %{name} on this server.
         new?: You are not authorized to create a project under the %{name} namespace on this server.

db/migrate/20240912152940_add_attachments_updated_at_to_namespace.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+# Migration to add attachments_updated_at column to namespaces table
+class AddAttachmentsUpdatedAtToNamespace < ActiveRecord::Migration[7.2]
+  def change
+    add_column :namespaces, :attachments_updated_at, :datetime
+  end
+end

db/migrate/20240912193934_modify_namespaces_logidze_trigger_v03.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+# Migration to modify namespace logidze trigger for attachments_updated_at
+class ModifyNamespacesLogidzeTriggerV03 < ActiveRecord::Migration[7.2]
+  def change
+    update_trigger :logidze_on_namespaces, on: :namespaces, version: 3
+  end
+end

db/triggers/logidze_on_namespaces_v03.sql

@@ -0,0 +1,6 @@
+CREATE OR REPLACE TRIGGER "logidze_on_namespaces"
+BEFORE UPDATE OR INSERT ON "namespaces" FOR EACH ROW
+WHEN (coalesce(current_setting('logidze.disabled', true), '') <> 'on')
+-- Parameters: history_size_limit (integer), timestamp_column (text), filtered_columns (text[]),
+-- include_columns (boolean), debounce_time_ms (integer)
+EXECUTE PROCEDURE logidze_logger(null, 'updated_at', '{created_at,metadata_summary,updated_at,attachments_updated_at}');