From d3c2407df102fec656ac5c048f1f753661123100 Mon Sep 17 00:00:00 2001
From: Benjamin Neff <benjamin@coding4coffee.ch>
Date: Wed, 24 Nov 2021 02:42:31 +0100
Subject: [PATCH] Don't overwrite photos of other users during import

If a photo with the same filename already exists, generate a new random
filename, and re-federate the photo with that filename. This ensures
users can't modify their archive to overwrite other users photos.
---
 app/services/import_service.rb          | 31 +++++++++----
 spec/integration/import_service_spec.rb | 60 +++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 9 deletions(-)
 create mode 100644 spec/integration/import_service_spec.rb

diff --git a/app/services/import_service.rb b/app/services/import_service.rb
index b7805e11a..cb6d02e3b 100644
--- a/app/services/import_service.rb
+++ b/app/services/import_service.rb
@@ -47,27 +47,40 @@ class ImportService
 
     uncompressed_photos_folder = unzip_photos_file(path_to_photos)
     user.posts.find_in_batches do |posts|
-      import_photos_for_posts(posts, uncompressed_photos_folder)
+      import_photos_for_posts(user, posts, uncompressed_photos_folder)
     end
-    FileUtils.rmdir(uncompressed_photos_folder)
+    FileUtils.rm_r(uncompressed_photos_folder)
   end
 
-  def import_photos_for_posts(posts, source_dir)
+  def import_photos_for_posts(user, posts, source_dir)
     posts.each do |post|
       post.photos.each do |photo|
         uploaded_file = "#{source_dir}/#{photo.remote_photo_name}"
         next unless File.exist?(uploaded_file) && photo.remote_photo_name.present?
 
-        File.open(uploaded_file) do |file|
-          photo.random_string = File.basename(uploaded_file, ".*")
-          photo.unprocessed_image = file
-          photo.save(touch: false)
-        end
-        photo.queue_processing_job
+        # Don't overwrite existing photos if they have the same filename.
+        # Generate a new random filename if a conflict exists and re-federate the photo to update on remote pods.
+        random_string = File.basename(uploaded_file, ".*")
+        conflicting_photo_exists = Photo.where.not(id: photo.id).exists?(random_string: random_string)
+        random_string = SecureRandom.hex(10) if conflicting_photo_exists
+
+        store_and_process_photo(photo, uploaded_file, random_string)
+
+        Diaspora::Federation::Dispatcher.build(user, photo).dispatch if conflicting_photo_exists
       end
     end
   end
 
+  def store_and_process_photo(photo, uploaded_file, random_string)
+    File.open(uploaded_file) do |file|
+      photo.random_string = random_string
+      photo.unprocessed_image.store! file
+      photo.update_remote_path
+      photo.save(touch: false)
+    end
+    photo.queue_processing_job
+  end
+
   def unzip_photos_file(photo_file_path)
     folder = create_folder(photo_file_path)
     Zip::File.open(photo_file_path) do |zip_file|
diff --git a/spec/integration/import_service_spec.rb b/spec/integration/import_service_spec.rb
new file mode 100644
index 000000000..878249d6c
--- /dev/null
+++ b/spec/integration/import_service_spec.rb
@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+describe ImportService do
+  context "import photos archive" do
+    let(:user) { FactoryBot.create(:user) }
+    let(:photo) { FactoryBot.create(:status_message_with_photo, author: user.person).photos.first }
+    let(:photo_archive) {
+      user.perform_export_photos!
+      photo_archive = user.exported_photos_file
+
+      # cleanup photo after creating the archive, so it's like it was imported from a remote pod
+      photo.unprocessed_image = nil
+      photo.random_string = nil
+      photo.remote_photo_path = "https://old.example.com/uploads/images/"
+      photo.save
+
+      photo_archive
+    }
+
+    it "imports the photo with the same name" do
+      old_random_string = photo.random_string
+
+      inlined_jobs { ImportService.new.import_by_files(nil, photo_archive.current_path, user.username) }
+
+      imported_photo = photo.reload
+      expect(imported_photo.random_string).to include(old_random_string)
+      expect(imported_photo.unprocessed_image.path).to include(old_random_string)
+      expect(imported_photo.processed_image.path).to include(old_random_string)
+      expect(imported_photo.remote_photo_name).to include(old_random_string)
+      expect(imported_photo.remote_photo_path).to eq("#{AppConfig.pod_uri}uploads/images/")
+    end
+
+    it "imports the photo with a new random name if a conflicting photo already exists" do
+      old_random_string = photo.random_string
+      photo_archive_path = photo_archive.current_path
+
+      sm = FactoryBot.create(:status_message)
+      FactoryBot.create(:photo, author: sm.author, status_message: sm, random_string: old_random_string)
+
+      expect(Diaspora::Federation::Dispatcher).to receive(:build) do |user_param, photo_param|
+        expect(user_param).to eq(user)
+        expect(photo_param.id).to eq(photo.id)
+
+        dispatcher = double
+        expect(dispatcher).to receive(:dispatch)
+        dispatcher
+      end
+
+      inlined_jobs { ImportService.new.import_by_files(nil, photo_archive_path, user.username) }
+
+      imported_photo = photo.reload
+      new_random_string = imported_photo.random_string
+      expect(new_random_string).not_to include(old_random_string)
+      expect(imported_photo.unprocessed_image.path).to include(new_random_string)
+      expect(imported_photo.processed_image.path).to include(new_random_string)
+      expect(imported_photo.remote_photo_name).to include(new_random_string)
+      expect(imported_photo.remote_photo_path).to eq("#{AppConfig.pod_uri}uploads/images/")
+    end
+  end
+end