Uploader policy update (DFCT0010069) (#627)

* Updated member permissions docs to include uploader actions, updated graphql resolver, mutation, and object to include the token in the context, added test for project query when user is an uploader

* Added tests for graphql resolver queries for uploader access level

* Updated mutations to work for uploader access level and added tests

* Updated graphql policy to check for active token, added tests for uploader access level with expired token

* Updated mutations to pass the token to the service calls

* Refactored to set user personal access token for Current class instead of having to pass it around

* Fixed formatting, removed fixture not used

* Removed unused scope and attr_accessors

* Removed passing of token in policies, updated member model can_* models to use Current.tokenfor uploader authorization, removed setting of context token from base mutation, resolver, and object

* Removed setting of token in base_service as it's set in the irida schema:

* Fixed flakey test

* Fixed assertion text

app/graphql/irida_schema.rb

@@ -38,6 +38,11 @@ def self.resolve_type(_type, object, _ctx)
   # Stop validating when it encounters this many errors:
   validate_max_errors 100
 
+  def self.execute(query_str = nil, **kwargs)
+    Current.token = kwargs[:context][:token]
+    super
+  end
+
   # Relay-style Object Identification:
 
   # Return a string UUID for `object`

app/models/current.rb

@@ -3,4 +3,5 @@
 # thread-isolated attributes class
 class Current < ActiveSupport::CurrentAttributes
   attribute :user
+  attribute :token
 end

app/models/member.rb

@@ -30,6 +30,10 @@ 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
+    DEFAULT_CAN_OPTIONS = {
+      include_group_links: true
+    }.freeze
+
     def access_levels(member)
       case member.access_level
       when AccessLevel::OWNER
@@ -41,7 +45,7 @@ def access_levels(member)
       end
     end
 
-    def effective_access_level(namespace, user, include_group_links = true) # rubocop:disable Metrics/CyclomaticComplexity, Style/OptionalBooleanParameter
+    def effective_access_level(namespace, user, include_group_links = true) # rubocop:disable Metrics/CyclomaticComplexity,Style/OptionalBooleanParameter
       return AccessLevel::OWNER if namespace.parent&.user_namespace? && namespace.parent.owner == user
 
       access_level = Member.for_namespace_and_ancestors(namespace).not_expired
@@ -64,9 +68,13 @@ def can_create?(user, object_namespace)
       )
     end
 
-    def can_view?(user, object_namespace, include_group_links = true) # rubocop:disable Style/OptionalBooleanParameter
-      effective_access_level = effective_access_level(object_namespace, user, include_group_links)
-      return false if effective_access_level == Member::AccessLevel::UPLOADER
+    def can_view?(user, object_namespace, **options)
+      options = DEFAULT_CAN_OPTIONS.merge(options)
+      effective_access_level = effective_access_level(object_namespace, user, options[:include_group_links])
+      if effective_access_level == Member::AccessLevel::UPLOADER &&
+         !Current.token&.active?
+        return false
+      end
 
       effective_access_level > Member::AccessLevel::NO_ACCESS
     end
@@ -86,7 +94,9 @@ def can_transfer_into_namespace?(user, object_namespace, include_group_links = t
     end
 
     def can_transfer_sample?(user, object_namespace)
-      namespace_owners_include_user?(user, object_namespace)
+      Member::AccessLevel.manageable.include?(
+        effective_access_level(object_namespace, user, false)
+      )
     end
 
     def can_transfer_sample_to_project?(user, object_namespace, include_group_links = true) # rubocop:disable Style/OptionalBooleanParameter
@@ -141,6 +151,26 @@ def can_create_export?(user, object_namespace)
       effective_access_level(object_namespace, user) >= Member::AccessLevel::ANALYST
     end
 
+    def can_create_sample?(user, object_namespace)
+      effective_access_level = effective_access_level(object_namespace, user)
+
+      return true if (effective_access_level == Member::AccessLevel::UPLOADER) && Current.token&.active?
+
+      Member::AccessLevel.manageable.include?(
+        effective_access_level
+      )
+    end
+
+    def can_modify_sample?(user, object_namespace)
+      effective_access_level = effective_access_level(object_namespace, user)
+
+      return true if (effective_access_level == Member::AccessLevel::UPLOADER) && Current.token&.active?
+
+      Member::AccessLevel.manageable.include?(
+        effective_access_level
+      )
+    end
+
     def access_level_in_namespace_group_links(user, namespace)
       effective_namespace_group_link = NamespaceGroupLink.for_namespace_and_ancestors(namespace)
                                                          .where(group: user.groups.self_and_descendants)

app/policies/group_policy.rb

@@ -17,7 +17,7 @@ def create_subgroup?
   end
 
   def view_history?
-    return true if Member.can_view?(user, record, false) == true
+    return true if Member.can_view?(user, record, include_group_links: false) == true
 
     details[:name] = record.name
     false

app/policies/project_policy.rb

@@ -12,7 +12,7 @@ def activity?
 
   def view_history?
     return true if record.namespace.parent.user_namespace? && record.namespace.parent.owner == user
-    return true if Member.can_view?(user, record.namespace, false) == true
+    return true if Member.can_view?(user, record.namespace, include_group_links: false) == true
 
     details[:name] = record.name
     false
@@ -76,7 +76,7 @@ def sample_listing?
 
   def create_sample?
     return true if record.namespace.parent.user_namespace? && record.namespace.parent.owner == user
-    return true if Member.can_create?(user, record.namespace) == true
+    return true if Member.can_create_sample?(user, record.namespace) == true
 
     details[:name] = record.name
     false
@@ -100,16 +100,13 @@ def read_sample?
 
   def update_sample?
     return true if record.namespace.parent.user_namespace? && record.namespace.parent.owner == user
-    return true if Member.can_modify?(user, record.namespace) == true
+    return true if Member.can_modify_sample?(user, record.namespace) == true
 
     details[:name] = record.name
     false
   end
 
-  def transfer_sample? # rubocop:disable Metrics/AbcSize
-    # Maintainer is allowed to transfer a sample to another namespace which has the same parent or ancestor
-    return true if Member.user_has_namespace_maintainer_access?(user, record.namespace, false)
-    return true if record.namespace.parent.user_namespace? && record.namespace.parent.owner == user
+  def transfer_sample?
     return true if Member.can_transfer_sample?(user, record.namespace) == true
 
     details[:name] = record.name

app/services/members/create_service.rb

@@ -25,7 +25,7 @@ def execute # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Met
                                         namespace_type: namespace.class.model_name.human)
       end
 
-      has_previous_access = Member.can_view?(member.user, namespace, true) if member.valid?
+      has_previous_access = Member.can_view?(member.user, namespace) if member.valid?
       send_emails if member.save && @email_notification && !has_previous_access
       member
     rescue Members::CreateService::MemberCreateError => e

app/services/members/destroy_service.rb

@@ -34,7 +34,7 @@ def execute # rubocop:disable Metrics/AbcSize
     private
 
     def send_emails
-      return if Member.can_view?(member.user, namespace, true)
+      return if Member.can_view?(member.user, namespace)
 
       MemberMailer.access_revoked_user_email(member, namespace).deliver_later
 

docs-site/docs/user/organization/members/member-permissions.md

@@ -35,36 +35,42 @@ The following table lists group permissions available for each role:
 | Create Group and Subgroups   |       |          |         | ✓          | ✓     |
 | Edit Group and Subgroups     |       |          |         | ✓          | ✓     |
 | Delete Group and Subgroups   |       |          |         |            | ✓     |
-| View Group and Subgroups     | ✓     |          | ✓       | ✓          | ✓     |
+| View Group and Subgroups     | ✓     | ✓ (2)    | ✓       | ✓          | ✓     |
 | Transfer Group and Subgroups |       |          |         |            | ✓     |
 | Add Group Member             |       |          |         | ✓(1)       | ✓     |
 | Edit Group Member            |       |          |         | ✓(1)       | ✓     |
 | Remove Group Member          |       |          |         | ✓(1)       | ✓     |
 | View Group Members           | ✓     |          | ✓       | ✓          | ✓     |
-<!-- TODO: Add uploader actions -->
+
+<!-- TODO: Add bot account actions -->
+
 1. A user with the **Maintainer** role can only modify members upto and including their role
+2. A user or bot account with the **Uploader** role can only perform these actions via the api
 
 ## Subgroup permissions
 
 When you add a member to a subgroup where they are also a member of one of the parent groups, they inherit the member role from the parent groups.
 
 ## Project Members Permissions
 
-| Action                | Guest | Analyst | Maintainer | Owner |
-| :-------------------- | :---- | ------- | ---------- | ----- |
-| Create Project        |       |         | ✓          | ✓     |
-| Edit Project          |       |         | ✓          | ✓     |
-| Delete Project        |       |         |            | ✓     |
-| View Project          | ✓     | ✓       | ✓          | ✓     |
-| Transfer Project      |       |         |            | ✓     |
-| Add Project Member    |       |         | ✓(1)       | ✓     |
-| Edit Project Member   |       |         | ✓(1)       | ✓     |
-| Remove Project Member |       |         | ✓(1)       | ✓     |
-| View Project Members  | ✓     | ✓       | ✓          | ✓     |
-| Create Samples        |       |         | ✓          | ✓     |
-| Edit Samples          |       |         | ✓          | ✓     |
-| Delete Samples        |       |         |            | ✓     |
-| Transfer Samples      |       |         | ✓(2)       | ✓     |
+| Action                | Guest | Uploader | Analyst | Maintainer | Owner |
+| :-------------------- | :---- | -------- | ------- | ---------- | ----- |
+| Create Project        |       |          |         | ✓          | ✓     |
+| Edit Project          |       |          |         | ✓          | ✓     |
+| Delete Project        |       |          |         |            | ✓     |
+| View Project          | ✓     | ✓(3)     | ✓       | ✓          | ✓     |
+| Transfer Project      |       |          |         |            | ✓     |
+| Add Project Member    |       |          |         | ✓(1)       | ✓     |
+| Edit Project Member   |       |          |         | ✓(1)       | ✓     |
+| Remove Project Member |       |          |         | ✓(1)       | ✓     |
+| View Project Members  | ✓     |          | ✓       | ✓          | ✓     |
+| Create Samples        |       | ✓(3)     |         | ✓          | ✓     |
+| Edit Samples          |       | ✓(3)     |         | ✓          | ✓     |
+| Delete Samples        |       |          |         |            | ✓     |
+| Transfer Samples      |       |          |         | ✓(2)       | ✓     |
+
 <!-- TODO: Add metadata, files, history, bot account permissions to project members permissions table -->
+
 1. A user with the **Maintainer** role can only modify members upto and including their role
 2. A user with the **Maintainer** role can only transfer samples to another project under the common ancestor for the current project
+3. A user or bot account with the **Uploader** role can only perform these actions via the api

test/fixtures/members.yml

@@ -535,3 +535,9 @@ projectA_member_joan_doe:
   created_by_id: <%= ActiveRecord::FixtureSet.identify(:jeff_doe, :uuid) %>
   namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectA_namespace, :uuid) %>
   access_level: <%= Member::AccessLevel::MAINTAINER %>
+
+project_jeff_member_user_bot_account:
+  user_id: <%= ActiveRecord::FixtureSet.identify(:projectJeff_bot, :uuid) %>
+  created_by_id: <%= ActiveRecord::FixtureSet.identify(:jeff_doe, :uuid) %>
+  namespace_id: <%= ActiveRecord::FixtureSet.identify(:project_jeff_namespace, :uuid) %>
+  access_level: <%= Member::AccessLevel::UPLOADER %>

test/fixtures/namespace_bots.yml

@@ -15,3 +15,7 @@ group1_bot<%= (n) %>:
 invalid_user_bot:
   namespace_id: <%= ActiveRecord::FixtureSet.identify(:john_doe_namespace, :uuid) %>
   user_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %>
+
+projectJeff_bot:
+  namespace_id: <%= ActiveRecord::FixtureSet.identify(:project_jeff_namespace, :uuid) %>
+  user_id: <%= ActiveRecord::FixtureSet.identify(:projectJeff_bot, :uuid) %>

test/fixtures/personal_access_tokens.yml

@@ -93,7 +93,6 @@ user_bot_account<%= (n) %>_valid_pat:
   last_used_at: nil
 <% end %>
 
-
 <% 21.times do |n| %>
 user_group_bot_account<%= (n) %>_valid_pat:
   user_id: <%= ActiveRecord::FixtureSet.identify(:"user_group_bot_account#{n}", :uuid) %>
@@ -106,3 +105,36 @@ user_group_bot_account<%= (n) %>_valid_pat:
   token_digest: <%= Devise.token_generator.digest(PersonalAccessToken, :token_digest, "AX3wwK3sWCjn#{n}xdCyZtH") %>
   last_used_at: nil
 <% end %>
+
+projectJeff_bot_account_valid_pat:
+  user_id: <%= ActiveRecord::FixtureSet.identify(:projectJeff_bot, :uuid) %>
+  scopes:
+    - read_api
+    - api
+  name: Valid PAT1 %>
+  revoked: false
+  expires_at: <%= 10.days.from_now.to_date %>
+  token_digest: <%= Devise.token_generator.digest(PersonalAccessToken, :token_digest, "FX3wwK3sWCjn1xdCyZtA") %>
+  last_used_at: nil
+
+user_bot_account0_expired_pat:
+  user_id: <%= ActiveRecord::FixtureSet.identify(:user_bot_account0, :uuid) %>
+  scopes:
+    - read_api
+    - api
+  name: Expired Valid PAT0
+  revoked: false
+  expires_at: <%= 10.days.ago.to_date %>
+  token_digest: <%= Devise.token_generator.digest(PersonalAccessToken, :token_digest, "VX3wwK3sWCjn0xdCyZtH") %>
+  last_used_at: nil
+
+user_group_bot_account0_expired_pat:
+  user_id: <%= ActiveRecord::FixtureSet.identify(:user_group_bot_account0, :uuid) %>
+  scopes:
+    - read_api
+    - api
+  name: Expired PAT 0
+  revoked: false
+  expires_at: <%= 5.days.ago.to_date %>
+  token_digest: <%= Devise.token_generator.digest(PersonalAccessToken, :token_digest, "TA3wwK3sWCjn0xdCyZtH") %>
+  last_used_at: nil

test/fixtures/users.yml

@@ -211,3 +211,10 @@ janitor_doe:
   first_name: project
   last_name: "end to end user"
   locale: en
+
+projectJeff_bot:
+  email: <%= "inxt_prj_aaaaaaaabp_001@localhost" %>
+  encrypted_password: <%= User.new.send :password_digest, "password1" %>
+  first_name: INXT_PRJ_AAAAAAAABP
+  last_name: Bot
+  user_type: <%= User.user_types[:project_bot] %>

test/graphql/attach_files_to_sample_test.rb

@@ -85,6 +85,34 @@ def setup
     assert_equal 'afts.fastq', sample.attachments[0].filename.to_s
   end
 
+  test 'attachFilesToSample mutation should work with valid params, puid, and api scope token for uploader access level' do # rubocop:disable Layout/LineLength
+    user = users(:projectJeff_bot)
+    token = personal_access_tokens(:projectJeff_bot_account_valid_pat)
+    sample = samples(:sampleJeff)
+    blob_file = active_storage_blobs(:attachment_attach_files_to_sample_test_blob)
+
+    assert_equal 0, sample.attachments.count
+
+    result = IridaSchema.execute(ATTACH_FILES_TO_SAMPLE_BY_SAMPLE_PUID_MUTATION,
+                                 context: { current_user: user, token: },
+                                 variables: { files: [blob_file.signed_id],
+                                              samplePuid: sample.puid })
+
+    assert_nil result['errors'], 'should work and have no errors.'
+
+    data = result['data']['attachFilesToSample']
+
+    assert_not_empty data, 'attachFilesToSample should be populated when no authorization errors'
+    expected_status = { blob_file.signed_id => :success }
+    assert_equal expected_status, data['status']
+    assert_not_empty data['sample']
+
+    assert_equal 1, sample.attachments.count
+
+    # check that filename matches
+    assert_equal 'afts.fastq', sample.attachments[0].filename.to_s
+  end
+
   test 'attachFilesToSample mutation should not work with read api scope token' do
     sample = samples(:sampleJeff)
     blob_file = active_storage_blobs(:attachment_attach_files_to_sample_test_blob)

test/graphql/attachments_query_test.rb

@@ -107,6 +107,36 @@ def setup
     assert_equal 'none', attachments[1]['node']['metadata']['compression']
   end
 
+  test 'attachment query should work for uploader access level' do
+    user = users(:user_bot_account0)
+    token = personal_access_tokens(:user_bot_account0_valid_pat)
+    result = IridaSchema.execute(ATTACHMENTS_QUERY, context: { current_user: user, token: },
+                                                    variables: { puid: @sample.puid })
+
+    assert_nil result['errors'], 'should work and have no errors.'
+
+    data = result['data']['sample']
+
+    assert_not_empty data, 'sample type should work'
+
+    assert_not_empty data['attachments']
+    assert_not_empty data['attachments']['edges']
+
+    attachments = data['attachments']['edges']
+    assert_equal 2, attachments.count
+
+    assert_equal 'test_file.fastq', attachments[0]['node']['filename']
+    assert_equal 'test_file_A.fastq', attachments[1]['node']['filename']
+
+    assert_equal 2102, attachments[0]['node']['byteSize']
+    assert_equal 2101, attachments[1]['node']['byteSize']
+
+    assert_equal 'fastq', attachments[0]['node']['metadata']['format']
+    assert_equal 'none', attachments[0]['node']['metadata']['compression']
+    assert_equal 'fastq', attachments[1]['node']['metadata']['format']
+    assert_equal 'none', attachments[1]['node']['metadata']['compression']
+  end
+
   test 'attachment url query should work' do
     result = IridaSchema.execute(ATTACHMENTS_URL_QUERY, context: { current_user: @user },
                                                         variables: { puid: @sample.puid })
@@ -126,6 +156,19 @@ def setup
     assert_equal file_url2, attachments[1]['node']['attachmentUrl']
   end
 
+  test 'attachment url query should not work due to expired token for uploader access level' do
+    user = users(:user_bot_account0)
+    token = personal_access_tokens(:user_bot_account0_expired_pat)
+    result = IridaSchema.execute(ATTACHMENTS_URL_QUERY, context: { current_user: user, token: },
+                                                        variables: { puid: @sample.puid })
+
+    assert_not_nil result['errors'], 'shouldn\'t work and have errors.'
+
+    error_message = result['errors'][0]['message']
+
+    assert_equal 'An object of type Sample was hidden due to permissions', error_message
+  end
+
   test 'attachment metadata delimit query should work' do
     result = IridaSchema.execute(ATTACHMENTS_METADATA_QUERY, context: { current_user: @user },
                                                              variables: { puid: @sample.puid })