[deliver] Implements verify with altool for Xcode 14 validation (#…

…20738)

* Adds support for `build_verify_command`

* Updates naming of variable to show the inclusion of `verify` command

* Adds required `platform` option

* Allows passing in an `asset_path` to upload values correctly

* Updates unit tests for `deliver` component

* Fixes failing tests in `itunes_transporter_spec`

* Runs `rubocop -a`

* Uses `**kwargs` for abstract `build_verify_command` method

* Fixes `rubocop` violation

* Removes `altool_compatible_command` argument from `deliver` runner

* Fixes further PR comments

* Reverts changes to temporary path code

---------

Co-authored-by: Pol Piella <24246926+pol-piella@users.noreply.github.com>

deliver/lib/deliver/runner.rb

@@ -179,15 +179,15 @@ def verify_binary
           package_path: "/tmp",
           platform: platform
         )
-        result = transporter.verify(package_path: package_path)
+        result = transporter.verify(package_path: package_path, asset_path: ipa_path, platform: platform)
       when "osx"
         package_path = FastlaneCore::PkgUploadPackageBuilder.new.generate(
           app_id: Deliver.cache[:app].id,
           pkg_path: pkg_path,
           package_path: "/tmp",
           platform: platform
         )
-        result = transporter.verify(package_path: package_path)
+        result = transporter.verify(package_path: package_path, asset_path: pkg_path, platform: platform)
       else
         UI.user_error!("No suitable file found for verify for platform: #{options[:platform]}")
       end
@@ -206,7 +206,7 @@ def upload_binary
       pkg_path = options[:pkg]
 
       platform = options[:platform]
-      transporter = transporter_for_selected_team(upload: true)
+      transporter = transporter_for_selected_team
 
       case platform
       when "ios", "appletvos"
@@ -266,7 +266,7 @@ def submit_for_review
     # If itc_provider was explicitly specified, use it.
     # If there are multiple teams, infer the provider from the selected team name.
     # If there are fewer than two teams, don't infer the provider.
-    def transporter_for_selected_team(upload: false)
+    def transporter_for_selected_team
       # Use JWT auth
       api_token = Spaceship::ConnectAPI.token
       api_key = if options[:api_key].nil? && !api_token.nil?
@@ -281,20 +281,20 @@ def transporter_for_selected_team(upload: false)
 
       unless api_token.nil?
         api_token.refresh! if api_token.expired?
-        return FastlaneCore::ItunesTransporter.new(nil, nil, false, nil, api_token.text, upload: upload, api_key: api_key)
+        return FastlaneCore::ItunesTransporter.new(nil, nil, false, nil, api_token.text, altool_compatible_command: true, api_key: api_key)
       end
 
       tunes_client = Spaceship::ConnectAPI.client.tunes_client
 
-      generic_transporter = FastlaneCore::ItunesTransporter.new(options[:username], nil, false, options[:itc_provider], upload: upload, api_key: api_key)
+      generic_transporter = FastlaneCore::ItunesTransporter.new(options[:username], nil, false, options[:itc_provider], altool_compatible_command: true, api_key: api_key)
       return generic_transporter unless options[:itc_provider].nil? && tunes_client.teams.count > 1
 
       begin
         team = tunes_client.teams.find { |t| t['providerId'].to_s == tunes_client.team_id }
         name = team['name']
         provider_id = generic_transporter.provider_ids[name]
         UI.verbose("Inferred provider id #{provider_id} for team #{name}.")
-        return FastlaneCore::ItunesTransporter.new(options[:username], nil, false, provider_id, upload: upload, api_key: api_key)
+        return FastlaneCore::ItunesTransporter.new(options[:username], nil, false, provider_id, altool_compatible_command: true, api_key: api_key)
       rescue => ex
         UI.verbose("Couldn't infer a provider short name for team with id #{tunes_client.team_id} automatically: #{ex}. Proceeding without provider short name.")
         return generic_transporter

deliver/spec/runner_spec.rb

@@ -110,7 +110,7 @@ def provider_ids
         expect_any_instance_of(FastlaneCore::IpaUploadPackageBuilder).to receive(:generate)
           .with(app_id: 'YI8C2AS', ipa_path: 'ACME.ipa', package_path: '/tmp', platform: 'ios')
           .and_return('path')
-        expect(transporter).to receive(:verify).with(package_path: 'path').and_return(true)
+        expect(transporter).to receive(:verify).with(asset_path: "ACME.ipa", package_path: 'path', platform: "ios").and_return(true)
         runner.verify_binary
       end
     end
@@ -124,7 +124,7 @@ def provider_ids
         expect_any_instance_of(FastlaneCore::IpaUploadPackageBuilder).to receive(:generate)
           .with(app_id: 'YI8C2AS', ipa_path: 'ACME.ipa', package_path: '/tmp', platform: 'appletvos')
           .and_return('path')
-        expect(transporter).to receive(:verify).with(package_path: 'path').and_return(true)
+        expect(transporter).to receive(:verify).with(asset_path: "ACME.ipa", package_path: 'path', platform: "appletvos").and_return(true)
         runner.verify_binary
       end
     end
@@ -140,7 +140,7 @@ def provider_ids
         expect_any_instance_of(FastlaneCore::PkgUploadPackageBuilder).to receive(:generate)
           .with(app_id: 'YI8C2AS', pkg_path: 'ACME.pkg', package_path: '/tmp', platform: 'osx')
           .and_return('path')
-        expect(transporter).to receive(:verify).with(package_path: 'path').and_return(true)
+        expect(transporter).to receive(:verify).with(asset_path: "ACME.pkg", package_path: 'path', platform: "osx").and_return(true)
         runner.verify_binary
       end
     end

fastlane_core/lib/fastlane_core/itunes_transporter.rb

@@ -47,7 +47,7 @@ def build_upload_command(username, password, source = "/tmp", provider_short_nam
       not_implemented(__method__)
     end
 
-    def build_verify_command(username, password, source = "/tmp", provider_short_name = "", jwt = nil)
+    def build_verify_command(username, password, source = "/tmp", provider_short_name = "", **kwargs)
       not_implemented(__method__)
     end
 
@@ -307,8 +307,22 @@ def build_download_command(username, password, apple_id, destination = "/tmp", p
       raise "This feature has not been implemented yet with altool for Xcode 14"
     end
 
-    def build_verify_command(username, password, source = "/tmp", provider_short_name = "", jwt = nil)
-      raise "This feature has not been implemented yet with altool for Xcode 14"
+    def build_verify_command(username, password, source = "/tmp", provider_short_name = "", **kwargs)
+      api_key = kwargs[:api_key]
+      platform = kwargs[:platform]
+      use_api_key = !api_key.nil?
+      [
+        ("API_PRIVATE_KEYS_DIR=#{api_key[:key_dir]}" if use_api_key),
+        "xcrun altool",
+        "--validate-app",
+        ("-u #{username.shellescape}" unless use_api_key),
+        ("-p #{password.shellescape}" unless use_api_key),
+        ("--apiKey #{api_key[:key_id]}" if use_api_key),
+        ("--apiIssuer #{api_key[:issuer_id]}" if use_api_key),
+        ("--asc-provider #{provider_short_name}" unless use_api_key || provider_short_name.to_s.empty?),
+        platform_option(platform),
+        file_upload_option(source)
+      ].compact.join(' ')
     end
 
     def additional_upload_parameters
@@ -409,7 +423,8 @@ def build_provider_ids_command(username, password, jwt = nil, api_key = nil)
       ].compact.join(' ')
     end
 
-    def build_verify_command(username, password, source = "/tmp", provider_short_name = "", jwt = nil)
+    def build_verify_command(username, password, source = "/tmp", provider_short_name = "", **kwargs)
+      jwt = kwargs[:jwt]
       use_jwt = !jwt.to_s.empty?
       [
         '"' + Helper.transporter_path + '"',
@@ -504,7 +519,8 @@ def build_upload_command(username, password, source = "/tmp", provider_short_nam
       end
     end
 
-    def build_verify_command(username, password, source = "/tmp", provider_short_name = "", jwt = nil)
+    def build_verify_command(username, password, source = "/tmp", provider_short_name = "", **kwargs)
+      jwt = kwargs[:jwt]
       use_jwt = !jwt.to_s.empty?
       if !Helper.user_defined_itms_path? && Helper.mac? && Helper.xcode_at_least?(11)
         [
@@ -660,7 +676,7 @@ def self.hide_transporter_output?
     #                            see: https://github.com/fastlane/fastlane/issues/1524#issuecomment-196370628
     #                            for more information about how to use the iTMSTransporter to list your provider
     #                            short names
-    def initialize(user = nil, password = nil, use_shell_script = false, provider_short_name = nil, jwt = nil, upload: false, api_key: nil)
+    def initialize(user = nil, password = nil, use_shell_script = false, provider_short_name = nil, jwt = nil, altool_compatible_command: false, api_key: nil)
       # Xcode 6.x doesn't have the same iTMSTransporter Java setup as later Xcode versions, so
       # we can't default to using the newer direct Java invocation strategy for those versions.
       use_shell_script ||= Helper.is_mac? && Helper.xcode_version.start_with?('6.')
@@ -675,7 +691,7 @@ def initialize(user = nil, password = nil, use_shell_script = false, provider_sh
       @jwt = jwt
       @api_key = api_key
 
-      if should_use_altool?(upload, use_shell_script)
+      if should_use_altool?(altool_compatible_command, use_shell_script)
         UI.verbose("Using altool as transporter.")
         @transporter_executor = AltoolTransporterExecutor.new
       else
@@ -800,10 +816,20 @@ def upload(app_id = nil, dir = nil, package_path: nil, asset_path: nil, platform
     # @return (Bool) True if everything worked fine
     # @raise [Deliver::TransporterTransferError] when something went wrong
     #   when transferring
-    def verify(app_id = nil, dir = nil, package_path: nil)
-      raise "Either a combination of app id and directory or a package_path are required" if (app_id.nil? || dir.nil?) && package_path.nil?
+    def verify(app_id = nil, dir = nil, package_path: nil, asset_path: nil, platform: nil)
+      raise "app_id and dir are required or package_path or asset_path is required" if (app_id.nil? || dir.nil?) && package_path.nil? && asset_path.nil?
+
+      force_itmsp = FastlaneCore::Env.truthy?("ITMSTRANSPORTER_FORCE_ITMS_PACKAGE_UPLOAD")
+      can_use_asset_path = Helper.is_mac? && asset_path
 
-      actual_dir = if package_path
+      actual_dir = if can_use_asset_path && !force_itmsp
+                     # The asset gets deleted upon completion so copying to a temp directory
+                     # (with randomized filename, for multibyte-mixed filename upload fails)
+                     new_file_name = "#{SecureRandom.uuid}#{File.extname(asset_path)}"
+                     tmp_asset_path = File.join(Dir.tmpdir, new_file_name)
+                     FileUtils.cp(asset_path, tmp_asset_path)
+                     tmp_asset_path
+                   elsif package_path
                      package_path
                    else
                      File.join(dir, "#{app_id}.itmsp")
@@ -812,8 +838,16 @@ def verify(app_id = nil, dir = nil, package_path: nil)
       password_placeholder = @jwt.nil? ? 'YourPassword' : nil
       jwt_placeholder = @jwt.nil? ? nil : 'YourJWT'
 
-      command = @transporter_executor.build_verify_command(@user, @password, actual_dir, @provider_short_name, @jwt)
-      UI.verbose(@transporter_executor.build_verify_command(@user, password_placeholder, actual_dir, @provider_short_name, jwt_placeholder))
+      # Handle AppStore Connect API
+      use_api_key = !@api_key.nil?
+
+      # Masking credentials for verbose outputs
+      api_key_placeholder = use_api_key ? { key_id: "YourKeyID", issuer_id: "YourIssuerID", key_dir: "YourTmpP8KeyDir" } : nil
+
+      api_key = api_key_with_p8_file_path(@api_key) if use_api_key
+
+      command = @transporter_executor.build_verify_command(@user, @password, actual_dir, @provider_short_name, jwt: @jwt, platform: platform, api_key: api_key)
+      UI.verbose(@transporter_executor.build_verify_command(@user, password_placeholder, actual_dir, @provider_short_name, jwt: jwt_placeholder, platform: platform, api_key: api_key_placeholder))
 
       begin
         result = @transporter_executor.execute(command, ItunesTransporter.hide_transporter_output?)
@@ -883,9 +917,9 @@ def api_key_with_p8_file_path(original_api_key)
     end
 
     # Returns whether altool should be used or ItunesTransporter should be used
-    def should_use_altool?(upload, use_shell_script)
+    def should_use_altool?(altool_compatible_command, use_shell_script)
       # Xcode 14 no longer supports iTMSTransporter. Use altool instead
-      !use_shell_script && upload && !Helper.user_defined_itms_path? && Helper.mac? && Helper.xcode_at_least?(14)
+      !use_shell_script && altool_compatible_command && !Helper.user_defined_itms_path? && Helper.mac? && Helper.xcode_at_least?(14)
     end
 
     # Returns the password to be used with the transporter

fastlane_core/spec/itunes_transporter_spec.rb

@@ -1157,14 +1157,14 @@ def xcrun_download_command(provider_short_name = nil, jwt: nil)
           end
           context "upload command generation" do
             it 'generates a call to altool' do
-              transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', upload: true)
+              transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', altool_compatible_command: true)
               expect(transporter.upload('my.app.id', '/tmp', package_path: '/tmp/my.app.id.itmsp', platform: "osx")).to eq(altool_upload_command(provider_short_name: 'abcd123'))
             end
           end
 
           context "provider IDs command generation" do
             it 'generates a call to altool' do
-              transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', upload: true)
+              transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', altool_compatible_command: true)
               expect(transporter.provider_ids).to eq(altool_provider_id_command)
             end
           end
@@ -1177,7 +1177,7 @@ def xcrun_download_command(provider_short_name = nil, jwt: nil)
           end
           context "upload command generation" do
             it 'generates a call to xcrun iTMSTransporter instead altool' do
-              transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', upload: true)
+              transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', altool_compatible_command: true)
               expect(transporter.upload('my.app.id', '/tmp', platform: "osx")).to eq(java_upload_command(provider_short_name: 'abcd123', classpath: false))
             end
           end
@@ -1194,15 +1194,15 @@ def xcrun_download_command(provider_short_name = nil, jwt: nil)
           end
           context "upload command generation" do
             it 'generates a call to altool' do
-              transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', upload: true, api_key: api_key)
+              transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', altool_compatible_command: true, api_key: api_key)
               expected = Regexp.new("API_PRIVATE_KEYS_DIR=#{Regexp.escape(Dir.tmpdir)}.*\s#{Regexp.escape(altool_upload_command(api_key: api_key, provider_short_name: 'abcd123'))}")
               expect(transporter.upload('my.app.id', '/tmp', platform: "osx")).to match(expected)
             end
           end
 
           context "provider IDs command generation" do
             it 'generates a call to altool' do
-              transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', upload: true, api_key: api_key)
+              transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', altool_compatible_command: true, api_key: api_key)
               expected = Regexp.new("API_PRIVATE_KEYS_DIR=#{Regexp.escape(Dir.tmpdir)}.*\s#{Regexp.escape(altool_provider_id_command(api_key: api_key))}")
               expect(transporter.provider_ids).to match(expected)
             end

pilot/lib/pilot/build_manager.rb

@@ -401,13 +401,13 @@ def transporter_for_selected_team(options)
 
       unless api_token.nil?
         api_token.refresh! if api_token.expired?
-        return FastlaneCore::ItunesTransporter.new(nil, nil, false, nil, api_token.text, upload: true, api_key: api_key)
+        return FastlaneCore::ItunesTransporter.new(nil, nil, false, nil, api_token.text, altool_compatible_command: true, api_key: api_key)
       end
 
       # Otherwise use username and password
       tunes_client = Spaceship::ConnectAPI.client ? Spaceship::ConnectAPI.client.tunes_client : nil
 
-      generic_transporter = FastlaneCore::ItunesTransporter.new(options[:username], nil, false, options[:itc_provider], upload: true, api_key: api_key)
+      generic_transporter = FastlaneCore::ItunesTransporter.new(options[:username], nil, false, options[:itc_provider], altool_compatible_command: true, api_key: api_key)
       return generic_transporter if options[:itc_provider] || tunes_client.nil?
       return generic_transporter unless tunes_client.teams.count > 1
 
@@ -416,7 +416,7 @@ def transporter_for_selected_team(options)
         name = team['name']
         provider_id = generic_transporter.provider_ids[name]
         UI.verbose("Inferred provider id #{provider_id} for team #{name}.")
-        return FastlaneCore::ItunesTransporter.new(options[:username], nil, false, provider_id, upload: true, api_key: api_key)
+        return FastlaneCore::ItunesTransporter.new(options[:username], nil, false, provider_id, altool_compatible_command: true, api_key: api_key)
       rescue => ex
         STDERR.puts(ex.to_s)
         UI.verbose("Couldn't infer a provider short name for team with id #{tunes_client.team_id} automatically: #{ex}. Proceeding without provider short name.")