Skip to content

Commit

Permalink
[Match][S3] Only download (and decrypt) files in the subfolder of the…
Browse files Browse the repository at this point in the history
… provided TeamID (fastlane#22199)

* [Match][S3] Only download files from the bucket which are corresponding to the provided TeamID

* Better code style/API conventions for helper
  • Loading branch information
AliSoftware authored Sep 27, 2024
1 parent 31b039c commit c7572d2
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 7 deletions.
7 changes: 5 additions & 2 deletions match/lib/match/storage/s3_storage.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,11 @@ def download
# No existing working directory, creating a new one now
self.working_directory = Dir.mktmpdir

s3_client.find_bucket!(s3_bucket).objects(prefix: s3_object_prefix).each do |object|
# If team_id is defined, use `:team/` as a prefix (appending it at the end of the `s3_object_prefix` if one provided by the user),
# so that we limit the download to only files that are specific to this team, and avoid downloads + decryption of unnecessary files.
key_prefix = team_id.nil? ? s3_object_prefix : File.join(s3_object_prefix, team_id, '').delete_prefix('/')

s3_client.find_bucket!(s3_bucket).objects(prefix: key_prefix).each do |object|
# Prevent download if the file path is a directory.
# We need to check if string ends with "/" instead of using `File.directory?` because
# the string represent a remote location, not a local file in disk.
Expand Down Expand Up @@ -181,7 +184,7 @@ def s3_object_path(file_name)
end

def strip_s3_object_prefix(object_path)
object_path.gsub(/^#{s3_object_prefix}/, "")
object_path.delete_prefix(s3_object_prefix.to_s).delete_prefix('/')
end

def sanitize_file_name(file_name)
Expand Down
39 changes: 34 additions & 5 deletions match/spec/storage/s3_storage_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,27 +70,56 @@
describe '#download' do
let(:files_to_download) do
[
instance_double('Aws::S3::Object', key: 'ABCDEFG/certs/development/ABCDEFG.cer', download_file: true),
instance_double('Aws::S3::Object', key: 'ABCDEFG/certs/development/ABCDEFG.p12', download_file: true)
instance_double('Aws::S3::Object', key: 'TEAMID1/certs/development/CERTID1.cer', download_file: true),
instance_double('Aws::S3::Object', key: 'TEAMID1/certs/development/CERTID1.p12', download_file: true),
instance_double('Aws::S3::Object', key: 'TEAMID2/certs/development/CERTID2.cer', download_file: true),
instance_double('Aws::S3::Object', key: 'TEAMID2/certs/development/CERTID2.p12', download_file: true)
]
end
let(:s3_client) { instance_double('Fastlane::Helper::S3ClientHelper', find_bucket!: double(objects: files_to_download)) }
let(:bucket) { instance_double('Aws::S3::Bucket') }
let(:s3_client) { instance_double('Fastlane::Helper::S3ClientHelper', find_bucket!: bucket) }

def stub_bucket_content(objects: files_to_download)
allow(bucket).to receive(:objects) do |options|
objects.select { |file_object| file_object.key.start_with?(options[:prefix] || '') }
end
end

before { class_double('FileUtils', mkdir_p: true).as_stubbed_const }

it 'downloads to correct working directory' do
stub_bucket_content
files_to_download.each do |file_object|
expect(file_object).to receive(:download_file).with("#{working_directory}/#{file_object.key}")
end

subject.download
end

it 'only downloads files specific to the provided team' do
stub_bucket_content
allow(subject).to receive(:team_id).and_return('TEAMID2')
files_to_download.each do |file_object|
if file_object.key.start_with?('TEAMID2')
expect(file_object).to receive(:download_file).with("#{working_directory}/#{file_object.key}")
else
expect(file_object).not_to receive(:download_file)
end
end

subject.download
end

it 'downloads files and strips the s3_object_prefix for working_directory path' do
allow(subject).to receive(:s3_object_prefix).and_return('123456/')

files_to_download.each do |file_object|
expect(file_object).to receive(:download_file).with("#{working_directory}/#{file_object.key}")
prefixed_objects = files_to_download.map do |obj|
instance_double('Aws::S3::Object', key: "123456/#{obj.key}", download_file: true)
end
stub_bucket_content(objects: prefixed_objects)

prefixed_objects.each do |file_object|
expect(file_object).to receive(:download_file).with("#{working_directory}/#{file_object.key.delete_prefix('123456/')}")
end

subject.download
Expand Down

0 comments on commit c7572d2

Please sign in to comment.