handle MRA in graphql audit log

fixes VICE-1022 flag=none TEST PLAN: - specs pass Change-Id: Ifb8b19003ce83ca10e05a082270d4e1dd657dea3 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/254661 Reviewed-by: Rob Orton <[email protected]> QA-Review: Rob Orton <[email protected]> Product-Review: Rob Orton <[email protected]> Tested-by: Service Cloud Jenkins <[email protected]>
rthbound · Mar 31, 2021 · 5c39a69 · 5c39a69
1 parent f720af6
commit 5c39a69
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 36 deletions.
diff --git a/app/graphql/audit_log_field_extension.rb b/app/graphql/audit_log_field_extension.rb
@@ -32,33 +32,37 @@ def initialize(mutation, context, arguments)
     end
 
     def log(entry, field_name)
-      @dynamo.put_item(
-        table_name: AuditLogFieldExtension.ddb_table_name,
-        item: {
-          # TODO: this is where you redirect
-          "object_id" => log_entry_id(entry, field_name),
-          "mutation_id" => mutation_id,
-          "timestamp" => @timestamp.iso8601,
-          "expires" => @ttl,
-          "mutation_name" => @mutation.graphql_name,
-          "current_user_id" => @context[:current_user]&.global_id&.to_s,
-          "real_current_user_id" => @context[:real_current_user]&.global_id&.to_s,
-          "params" => @params,
-        },
-        return_consumed_capacity: "TOTAL"
-      )
+      log_entry_ids(entry, field_name).each do |log_entry_id|
+        @dynamo.put_item(
+          table_name: AuditLogFieldExtension.ddb_table_name,
+          item: {
+            # TODO: this is where you redirect
+            "object_id" => log_entry_id,
+            "mutation_id" => mutation_id,
+            "timestamp" => @timestamp.iso8601,
+            "expires" => @ttl,
+            "mutation_name" => @mutation.graphql_name,
+            "current_user_id" => @context[:current_user]&.global_id&.to_s,
+            "real_current_user_id" => @context[:real_current_user]&.global_id&.to_s,
+            "params" => @params,
+          },
+          return_consumed_capacity: "TOTAL"
+        )
+      end
     rescue Aws::DynamoDB::Errors::ServiceError => e
       ::Canvas::Errors.capture_exception(:graphql_mutation_audit_logs, e)
       Rails.logger.error "Couldn't log mutation: #{e}"
     end
 
-    def log_entry_id(entry, field_name)
+    def log_entry_ids(entry, field_name)
       override_entry_method = :"#{field_name}_log_entry"
       entry = @mutation.send(override_entry_method, entry, @context) if @mutation.respond_to?(override_entry_method)
 
-      domain_root_account = root_account_for(entry)
+      domain_root_account_ids = root_account_ids_for(entry)
 
-      "#{domain_root_account.global_id}-#{entry.asset_string}"
+      domain_root_account_ids.map do |domain_root_account_id|
+        "#{domain_root_account_id}-#{entry.asset_string}"
+      end
     end
 
     ##
@@ -69,26 +73,22 @@ def log_entry_id(entry, field_name)
     #
     # this method will have to know how to resolve a root account for every
     # object that is logged by a mutation
-    def root_account_for(entry)
+    def root_account_ids_for(entry)
       if Progress === entry
         entry = entry.context
       end
 
+      if entry.respond_to? :global_root_account_ids
+        return entry.global_root_account_ids
+      end
+
       if entry.respond_to? :root_account_id
-        return entry.root_account if entry.root_account.present?
+        return [Shard.global_id_for(entry.root_account_id, entry.shard)]
       end
 
       case entry
-      when Course
-        entry.root_account
-      when Assignment, ContextModule, SubmissionComment
-        entry.context.root_account
-      when Submission
-        entry.assignment.course.root_account
       when SubmissionDraft
-        entry.submission.assignment.course.root_account
-      when PostPolicy
-        (entry.assignment&.course || entry.course).root_account
+        [Shard.global_id_for(entry.submission.root_account_id, entry.shard)]
       else
         raise "don't know how to resolve root_account for #{entry.inspect}"
       end
@@ -136,13 +136,9 @@ def resolve(object:, arguments:, context:, **rest)
       next unless AuditLogFieldExtension.enabled?
 
       mutation = field.mutation
-      # TODO: figure out how to resolve root account for user, communication channels, and conversations
-      next if mutation == Mutations::UpdateNotificationPreferences
-      next if mutation == Mutations::CreateConversation
       next if mutation == Mutations::DeleteConversationMessages
       next if mutation == Mutations::DeleteConversations
       next if mutation == Mutations::AddConversationMessage
-      next if mutation == Mutations::UpdateConversationParticipants
 
       logger = Logger.new(mutation, context, arguments)
 

diff --git a/app/models/communication_channel.rb b/app/models/communication_channel.rb
@@ -451,6 +451,12 @@ def destroy
     end
   end
 
+  def global_root_account_ids
+    root_account_ids&.map do |id|
+      Shard.global_id_for(id, self.shard)
+    end
+  end
+
   def set_root_account_ids(persist_changes: false, log: false)
     # communication_channels always are on the same shard as the user object and
     # can be used for any root_account, so just set root_account_ids from user.

diff --git a/app/models/user.rb b/app/models/user.rb
@@ -441,6 +441,12 @@ def self.skip_updating_account_associations?
     !!@skip_updating_account_associations
   end
 
+  def global_root_account_ids
+    root_account_ids&.map do |id|
+      Shard.global_id_for(id, self.shard)
+    end
+  end
+
   # Update the root_account_ids column on the user
   # and all the users CommunicationChannels
   def update_root_account_ids

diff --git a/lib/conversation_helper.rb b/lib/conversation_helper.rb
@@ -37,4 +37,10 @@ def root_account_ids=(ids)
     # ids must be sorted for the scope to work
     write_attribute(:root_account_ids, ids.sort.join(','))
   end
+
+  def global_root_account_ids
+    root_account_ids&.map do |id|
+      Shard.global_id_for(id, Shard.birth)
+    end
+  end
 end
diff --git a/spec/graphql/mutation_audit_log_spec.rb b/spec/graphql/mutation_audit_log_spec.rb
@@ -115,16 +115,16 @@
     })
   end
 
-  context "#log_entry_id" do
+  context "#log_entry_ids" do
     it "uses #asset_string and includes the domain_root_account id for the object_id" do
       logger = AuditLogFieldExtension::Logger.new(mutation, {}, {input: {}})
-      expect(logger.log_entry_id(@entry, "some_field")).to eq "#{@course.root_account.global_id}-assignment_#{@entry.id}"
+      expect(logger.log_entry_ids(@entry, "some_field")).to eq ["#{@course.root_account.global_id}-assignment_#{@entry.id}"]
     end
 
     it "allows overriding the logged object" do
       expect(mutation).to receive(:whatever_log_entry) { @entry.context }
       logger = AuditLogFieldExtension::Logger.new(mutation, {}, {input: {}})
-      expect(logger.log_entry_id(@entry, "whatever")).to eq "#{@course.root_account.global_id}-course_#{@course.id}"
+      expect(logger.log_entry_ids(@entry, "whatever")).to eq ["#{@course.root_account.global_id}-course_#{@course.id}"]
     end
   end