GraphQL: Use scoped context to skip queries and authorization (#765)

* feat: use scoped context to skip authorization and skip unnecessary queries * chore: fix test failure and skip authorization for attachments from resolvers * chore: update samples attachments query test to use nodes instead of edges to retrieve results * chore: change attachments for sample1 in fixtures to fix inconsistent ordering on different os * chore: fix rubocop warning
phac-nml · Sep 18, 2024 · bb885ff · bb885ff
1 parent 45ac26a
commit bb885ff
Show file tree

Hide file tree

Showing 19 changed files with 143 additions and 137 deletions.
diff --git a/app/graphql/resolvers/project_attachments_resolver.rb b/app/graphql/resolvers/project_attachments_resolver.rb
@@ -16,6 +16,7 @@ class ProjectAttachmentsResolver < BaseResolver
     alias project object
 
     def resolve(filter:, order_by:)
+      context.scoped_set!(:attachments_preauthorized, true)
       ransack_obj = project.namespace.attachments.joins(:file_blob).ransack(filter&.to_h)
       ransack_obj.sorts = ["#{order_by.field} #{order_by.direction}"] if order_by.present?
 

diff --git a/app/graphql/resolvers/project_sample_resolver.rb b/app/graphql/resolvers/project_sample_resolver.rb
@@ -18,6 +18,7 @@ class ProjectSampleResolver < BaseResolver
 
     def resolve(args)
       project = Namespaces::ProjectNamespace.find_by(puid: args[:project_puid])&.project
+      context.scoped_set!(:project, project)
       if args[:sample_puid]
         Sample.find_by(puid: args[:sample_puid], project:)
       else

diff --git a/app/graphql/resolvers/project_samples_resolver.rb b/app/graphql/resolvers/project_samples_resolver.rb
@@ -16,6 +16,8 @@ class ProjectSamplesResolver < BaseResolver
              default_value: nil
 
     def resolve(filter:, order_by:)
+      context.scoped_set!(:project, project)
+      context.scoped_set!(:samples_preauthorized, true)
       ransack_obj = project.samples.ransack(filter&.to_h)
       ransack_obj.sorts = ["#{order_by.field} #{order_by.direction}"] if order_by.present?
 

diff --git a/app/graphql/resolvers/projects_resolver.rb b/app/graphql/resolvers/projects_resolver.rb
@@ -21,6 +21,7 @@ class ProjectsResolver < BaseResolver
              default_value: nil
 
     def resolve(group_id:, filter:, order_by:)
+      context.scoped_set!(:projects_preauthorized, true)
       projects = group_id ? projects_by_group_scope(group_id:) : projects_by_scope
       ransack_obj = projects.ransack(filter&.to_h)
       ransack_obj.sorts = ["#{order_by.field} #{order_by.direction}"] if order_by.present?

diff --git a/app/graphql/resolvers/sample_attachments_resolver.rb b/app/graphql/resolvers/sample_attachments_resolver.rb
@@ -16,6 +16,7 @@ class SampleAttachmentsResolver < BaseResolver
     alias sample object
 
     def resolve(filter:, order_by:)
+      context.scoped_set!(:attachments_preauthorized, true)
       ransack_obj = sample.attachments.joins(:file_blob).ransack(filter&.to_h)
       ransack_obj.sorts = ["#{order_by.field} #{order_by.direction}"] if order_by.present?
 

diff --git a/app/graphql/resolvers/samples_resolver.rb b/app/graphql/resolvers/samples_resolver.rb
@@ -21,6 +21,7 @@ class SamplesResolver < BaseResolver
              default_value: nil
 
     def resolve(group_id:, filter:, order_by:)
+      context.scoped_set!(:samples_preauthorized, true)
       samples = group_id ? samples_by_group_scope(group_id:) : samples_by_project_scope
       ransack_obj = samples.ransack(filter&.to_h)
       ransack_obj.sorts = ["#{order_by.field} #{order_by.direction}"] if order_by.present?

diff --git a/app/graphql/types/attachment_type.rb b/app/graphql/types/attachment_type.rb
@@ -26,12 +26,12 @@ def attachment_url
     end
 
     def self.authorized?(object, context)
-      super &&
+      super && (context[:attachments_preauthorized] ||
         allowed_to?(
           :read?,
           object.attachable.project,
           context: { user: context[:current_user], token: context[:token] }
-        )
+        ))
     end
   end
 end
diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb
@@ -35,12 +35,12 @@ class ProjectType < Types::BaseType
     field :parent, NamespaceType, null: false, description: 'Parent namespace'
 
     def self.authorized?(object, context)
-      super &&
+      super && (context[:projects_preauthorized] ||
         allowed_to?(
           :read?,
           object,
           context: { user: context[:current_user], token: context[:token] }
-        )
+        ))
     end
   end
 end
diff --git a/app/graphql/types/sample_type.rb b/app/graphql/types/sample_type.rb
@@ -28,13 +28,18 @@ class SampleType < Types::BaseType
           null: true,
           description: 'Datetime when associated attachments were last updated.'
 
+    def project
+      context.scoped_set!(:projects_preauthorized, true)
+      context[:project] || object.project
+    end
+
     def self.authorized?(object, context)
-      super &&
+      super && (context[:samples_preauthorized] ||
         allowed_to?(
           :read_sample?,
           object.project,
           context: { user: context[:current_user], token: context[:token] }
-        )
+        ))
     end
   end
 end
diff --git a/test/controllers/projects/samples/attachments_controller_test.rb b/test/controllers/projects/samples/attachments_controller_test.rb
@@ -55,7 +55,7 @@ class AttachmentsControllerTest < ActionDispatch::IntegrationTest
           post namespace_project_sample_attachments_url(@namespace, @project, @sample1),
                params: { attachment: {
                  files: [fixture_file_upload('test_file_1.fastq', 'text/plain'),
-                         fixture_file_upload('test_file.fastq', 'text/plain')]
+                         fixture_file_upload('test_file_A.fastq', 'text/plain')]
                } },
                as: :turbo_stream
         end
@@ -65,7 +65,7 @@ class AttachmentsControllerTest < ActionDispatch::IntegrationTest
         assert_no_difference('Attachment.count') do
           post namespace_project_sample_attachments_url(@namespace, @project, @sample1),
                params: { attachment: {
-                 files: [fixture_file_upload('test_file.fastq', 'text/plain')]
+                 files: [fixture_file_upload('test_file_A.fastq', 'text/plain')]
                } },
                as: :turbo_stream
         end

diff --git a/test/fixtures/active_storage/attachments.yml b/test/fixtures/active_storage/attachments.yml
@@ -1,12 +1,12 @@
-attachment1_file_test_file_fastq:
+attachment1_file_test_file_A_fastq:
   name: file
   record: attachment1 (Attachment)
-  blob: attachment1_file_test_file_fastq_blob
+  blob: attachment1_file_test_file_A_fastq_blob
 
-attachment2_file_test_file_A_fastq:
+attachment2_file_test_file_B_fastq:
   name: file
   record: attachment2 (Attachment)
-  blob: attachment2_file_test_file_A_fastq_blob
+  blob: attachment2_file_test_file_B_fastq_blob
 
 attachmentA_file_test_file_fastq:
   name: file

diff --git a/test/fixtures/active_storage/blobs.yml b/test/fixtures/active_storage/blobs.yml
@@ -1,9 +1,11 @@
-attachment1_file_test_file_fastq_blob: <%= ActiveStorage::FixtureSet.blob filename: "test_file.fastq", service_name: "test" %>
+attachment1_file_test_file_A_fastq_blob: <%= ActiveStorage::FixtureSet.blob filename: "test_file_A.fastq", service_name: "test" %>
 
-attachment2_file_test_file_A_fastq_blob: <%= ActiveStorage::FixtureSet.blob filename: "test_file_A.fastq", service_name: "test" %>
+attachment2_file_test_file_B_fastq_blob: <%= ActiveStorage::FixtureSet.blob filename: "test_file_B.fastq", service_name: "test" %>
 
 test_file_fastq_blob: <%= ActiveStorage::FixtureSet.blob filename: "test_file.fastq", service_name: "test" %>
 
+test_file_A_fastq_blob: <%= ActiveStorage::FixtureSet.blob filename: "test_file_A.fastq", service_name: "test" %>
+
 testsample_illumina_pe_forward_blob: <%= ActiveStorage::FixtureSet.blob filename: "TestSample_S1_L001_R1_001.fastq", service_name: "test" %>
 testsample_illumina_pe_reverse_blob: <%= ActiveStorage::FixtureSet.blob filename: "TestSample_S1_L001_R2_001.fastq", service_name: "test" %>
 

diff --git a/test/fixtures/data_exports.yml b/test/fixtures/data_exports.yml
@@ -13,7 +13,7 @@ data_export_one:
   created_at: <%= 1.week.ago %>
   updated_at: <%= 1.day.ago %>
   user_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %>
-  manifest: '{"type":"Samples Export","date": "2024-01-01","children":[{"name":"INXT_PRJ_AAAAAAAAAA","type":"folder","irida-next-type":"project","irida-next-name":"Project 1","children":[{"name":"INXT_SAM_AAAAAAAAAA","type":"folder","irida-next-type":"sample","irida-next-name":"Project 1 Sample 1","children":[{"name":"INXT_ATT_AAAAAAAAAA","type":"folder","irida-next-type":"attachment","children":[{"name":"test_file.fastq","type":"file","metadata":{"format":"fastq"}}]},{"name":"INXT_ATT_AAAAAAAAAB","type":"folder","irida-next-type":"attachment","children":[{"name":"test_file_A.fastq","type":"file","metadata":{"format":"fastq"}}]}]}]}]}'
+  manifest: '{"type":"Samples Export","date": "2024-01-01","children":[{"name":"INXT_PRJ_AAAAAAAAAA","type":"folder","irida-next-type":"project","irida-next-name":"Project 1","children":[{"name":"INXT_SAM_AAAAAAAAAA","type":"folder","irida-next-type":"sample","irida-next-name":"Project 1 Sample 1","children":[{"name":"INXT_ATT_AAAAAAAAAA","type":"folder","irida-next-type":"attachment","children":[{"name":"test_file_A.fastq","type":"file","metadata":{"format":"fastq"}}]},{"name":"INXT_ATT_AAAAAAAAAB","type":"folder","irida-next-type":"attachment","children":[{"name":"test_file_B.fastq","type":"file","metadata":{"format":"fastq"}}]}]}]}]}'
 
 data_export_two:
   export_type: sample