From dc14b56461731cc1708fa7398158408c3e650b31 Mon Sep 17 00:00:00 2001
From: Eyal Delarea <eyalde@jfrog.com>
Date: Thu, 9 Jan 2025 10:55:54 +0200
Subject: [PATCH] Enhance JFrog CLI Credentials Input During Setup (#114)

---
 .../java/io/jenkins/plugins/jfrog/JfStep.java | 157 ++++++++++++++----
 .../io/jenkins/plugins/jfrog/JfStepTest.java  |  98 ++++++++++-
 .../jfrog/integration/PipelineTestBase.java   |   7 +-
 3 files changed, 225 insertions(+), 37 deletions(-)

diff --git a/src/main/java/io/jenkins/plugins/jfrog/JfStep.java b/src/main/java/io/jenkins/plugins/jfrog/JfStep.java
index 5d2b022f..d7a8ec01 100644
--- a/src/main/java/io/jenkins/plugins/jfrog/JfStep.java
+++ b/src/main/java/io/jenkins/plugins/jfrog/JfStep.java
@@ -1,6 +1,7 @@
 package io.jenkins.plugins.jfrog;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
+import org.jfrog.build.client.Version;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import hudson.*;
 import hudson.model.Job;
@@ -16,6 +17,7 @@
 import io.jenkins.plugins.jfrog.plugins.PluginsUtils;
 import lombok.Getter;
 import org.apache.commons.io.FilenameUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.exception.ExceptionUtils;
 import org.jenkinsci.plugins.plaincredentials.StringCredentials;
 import org.jenkinsci.plugins.workflow.steps.*;
@@ -23,6 +25,7 @@
 import org.kohsuke.stapler.DataBoundConstructor;
 
 import javax.annotation.Nonnull;
+import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
@@ -42,6 +45,7 @@
 public class JfStep extends Step {
     private static final ObjectMapper mapper = createMapper();
     protected String[] args;
+    static final Version MIN_CLI_VERSION_PASSWORD_STDIN = new Version("2.31.3");
 
     @DataBoundConstructor
     public JfStep(Object args) {
@@ -53,6 +57,33 @@ public JfStep(Object args) {
         this.args = split(args.toString());
     }
 
+    /**
+     * Retrieves the version of the JFrog CLI.
+     *
+     * @param launcher        The process launcher used to execute the JFrog CLI command.
+     * @param jfrogBinaryPath The path to the JFrog CLI binary.
+     * @return The version of the JFrog CLI.
+     * @throws IOException          If an I/O error occurs while executing the command or reading the output.
+     * @throws InterruptedException If the process is interrupted while waiting for the command to complete.
+     */
+    public static Version getJfrogCliVersion(Launcher.ProcStarter launcher, String jfrogBinaryPath) throws IOException, InterruptedException {
+        try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) {
+            ArgumentListBuilder builder = new ArgumentListBuilder();
+            builder.add(jfrogBinaryPath).add("-v");
+            int exitCode = launcher
+                    .cmds(builder)
+                    .pwd(launcher.pwd())
+                    .stdout(outputStream)
+                    .join();
+            if (exitCode != 0) {
+                throw new IOException("Failed to get JFrog CLI version: " + outputStream.toString(StandardCharsets.UTF_8));
+            }
+            String versionOutput = outputStream.toString(StandardCharsets.UTF_8).trim();
+            String version = StringUtils.substringAfterLast(versionOutput, " ");
+            return new Version(version);
+        }
+    }
+
     @Override
     public StepExecution start(StepContext context) {
         return new Execution(args, context);
@@ -80,6 +111,7 @@ protected String run() throws Exception {
             ArgumentListBuilder builder = new ArgumentListBuilder();
             boolean isWindows = !launcher.isUnix();
             String jfrogBinaryPath = getJFrogCLIPath(env, isWindows);
+            boolean passwordStdinSupported = isPasswordStdinSupported(workspace, env, launcher, jfrogBinaryPath);
 
             builder.add(jfrogBinaryPath).add(args);
             if (isWindows) {
@@ -89,7 +121,7 @@ protected String run() throws Exception {
             String output;
             try (ByteArrayOutputStream taskOutputStream = new ByteArrayOutputStream()) {
                 JfTaskListener jfTaskListener = new JfTaskListener(listener, taskOutputStream);
-                Launcher.ProcStarter jfLauncher = setupJFrogEnvironment(run, env, launcher, jfTaskListener, workspace, jfrogBinaryPath, isWindows);
+                Launcher.ProcStarter jfLauncher = setupJFrogEnvironment(run, env, launcher, jfTaskListener, workspace, jfrogBinaryPath, isWindows, passwordStdinSupported);
                 // Running the 'jf' command
                 int exitValue = jfLauncher.cmds(builder).join();
                 output = taskOutputStream.toString(StandardCharsets.UTF_8);
@@ -144,18 +176,19 @@ private void logIfNoToolProvided(EnvVars env, TaskListener listener) {
         /**
          * Configure all JFrog relevant environment variables and all servers (if they haven't been configured yet).
          *
-         * @param run             running as part of a specific build
-         * @param env             environment variables applicable to this step
-         * @param launcher        a way to start processes
-         * @param listener        a place to send output
-         * @param workspace       a workspace to use for any file operations
-         * @param jfrogBinaryPath path to jfrog cli binary on the filesystem
-         * @param isWindows       is Windows the applicable OS
+         * @param run                    running as part of a specific build
+         * @param env                    environment variables applicable to this step
+         * @param launcher               a way to start processes
+         * @param listener               a place to send output
+         * @param workspace              a workspace to use for any file operations
+         * @param jfrogBinaryPath        path to jfrog cli binary on the filesystem
+         * @param isWindows              is Windows the applicable OS
+         * @param passwordStdinSupported indicates if the password can be securely passed via stdin to the CLI
          * @return launcher applicable to this step.
          * @throws InterruptedException if the step is interrupted
          * @throws IOException          in case of any I/O error, or we failed to run the 'jf' command
          */
-        public Launcher.ProcStarter setupJFrogEnvironment(Run<?, ?> run, EnvVars env, Launcher launcher, TaskListener listener, FilePath workspace, String jfrogBinaryPath, boolean isWindows) throws IOException, InterruptedException {
+        public Launcher.ProcStarter setupJFrogEnvironment(Run<?, ?> run, EnvVars env, Launcher launcher, TaskListener listener, FilePath workspace, String jfrogBinaryPath, boolean isWindows, boolean passwordStdinSupported) throws IOException, InterruptedException {
             JFrogCliConfigEncryption jfrogCliConfigEncryption = run.getAction(JFrogCliConfigEncryption.class);
             if (jfrogCliConfigEncryption == null) {
                 // Set up the config encryption action to allow encrypting the JFrog CLI configuration and make sure we only create one key
@@ -168,11 +201,40 @@ public Launcher.ProcStarter setupJFrogEnvironment(Run<?, ?> run, EnvVars env, La
             // Configure all servers, skip if all server ids have already been configured.
             if (shouldConfig(jfrogHomeTempDir)) {
                 logIfNoToolProvided(env, listener);
-                configAllServers(jfLauncher, jfrogBinaryPath, isWindows, run.getParent());
+                configAllServers(jfLauncher, jfrogBinaryPath, isWindows, run.getParent(), passwordStdinSupported);
             }
             return jfLauncher;
         }
 
+        /**
+         * Determines if the password can be securely passed via stdin to the CLI,
+         * rather than using the --password flag. This depends on two factors:
+         * 1. The JFrog CLI version on the agent (minimum supported version is 2.31.3).
+         * 2. Whether the launcher is a custom (plugin) launcher.
+         * <p>
+         * Note: The primary reason for this limitation is that Docker plugin which is widely used
+         * does not support stdin input, because it is a custom launcher.
+         *
+         * @param workspace The workspace file path.
+         * @param env       The environment variables.
+         * @param launcher  The command launcher.
+         * @return true if stdin-based password handling is supported; false otherwise.
+         */
+        public boolean isPasswordStdinSupported(FilePath workspace, EnvVars env, Launcher launcher, String jfrogBinaryPath) throws IOException, InterruptedException {
+            TaskListener listener = getContext().get(TaskListener.class);
+            JenkinsBuildInfoLog buildInfoLog = new JenkinsBuildInfoLog(listener);
+
+            boolean isPluginLauncher = launcher.getClass().getName().contains("org.jenkinsci.plugins");
+            if (isPluginLauncher) {
+                buildInfoLog.debug("Password stdin is not supported,Launcher is a plugin launcher.");
+                return false;
+            }
+            Launcher.ProcStarter procStarter = launcher.launch().envs(env).pwd(workspace);
+            Version currentCliVersion = getJfrogCliVersion(procStarter, jfrogBinaryPath);
+            buildInfoLog.debug("Password stdin is supported");
+            return currentCliVersion.isAtLeast(MIN_CLI_VERSION_PASSWORD_STDIN);
+        }
+
         /**
          * Before we run a 'jf' command for the first time, we want to configure all servers first.
          * We know that all servers have already been configured if there is a "jfrog-cli.conf" file in the ".jfrog" home directory.
@@ -192,14 +254,14 @@ private boolean shouldConfig(FilePath jfrogHomeTempDir) throws IOException, Inte
         /**
          * Locally configure all servers that was configured in the Jenkins UI.
          */
-        private void configAllServers(Launcher.ProcStarter launcher, String jfrogBinaryPath, boolean isWindows, Job<?, ?> job) throws IOException, InterruptedException {
+        private void configAllServers(Launcher.ProcStarter launcher, String jfrogBinaryPath, boolean isWindows, Job<?, ?> job, boolean passwordStdinSupported) throws IOException, InterruptedException {
             // Config all servers using the 'jf c add' command.
             List<JFrogPlatformInstance> jfrogInstances = JFrogPlatformBuilder.getJFrogPlatformInstances();
             if (jfrogInstances != null && !jfrogInstances.isEmpty()) {
                 for (JFrogPlatformInstance jfrogPlatformInstance : jfrogInstances) {
                     // Build 'jf' command
                     ArgumentListBuilder builder = new ArgumentListBuilder();
-                    addConfigArguments(builder, jfrogPlatformInstance, jfrogBinaryPath, job);
+                    addConfigArguments(builder, jfrogPlatformInstance, job, jfrogBinaryPath, job, launcher, passwordStdinSupported);
                     if (isWindows) {
                         builder = builder.toWindowsCommand();
                     }
@@ -212,29 +274,60 @@ private void configAllServers(Launcher.ProcStarter launcher, String jfrogBinaryP
             }
         }
 
-        private void addConfigArguments(ArgumentListBuilder builder, JFrogPlatformInstance jfrogPlatformInstance, String jfrogBinaryPath, Job<?, ?> job) {
-            String credentialsId = jfrogPlatformInstance.getCredentialsConfig().getCredentialsId();
+        private void addConfigArguments(ArgumentListBuilder builder, JFrogPlatformInstance jfrogPlatformInstance, Job<?, ?> job1, String jfrogBinaryPath, Job<?, ?> job, Launcher.ProcStarter launcher, boolean passwordStdinSupported) {
             builder.add(jfrogBinaryPath).add("c").add("add").add(jfrogPlatformInstance.getId());
-            // Add credentials
-            StringCredentials accessTokenCredentials = PluginsUtils.accessTokenCredentialsLookup(credentialsId, job);
-            if (accessTokenCredentials != null) {
-                builder.addMasked("--access-token=" + accessTokenCredentials.getSecret().getPlainText());
-            } else {
-                Credentials credentials = PluginsUtils.credentialsLookup(credentialsId, job);
-                builder.add("--user=" + credentials.getUsername());
-                builder.addMasked("--password=" + credentials.getPassword());
-            }
-            // Add URLs
-            builder.add("--url=" + jfrogPlatformInstance.getUrl());
-            builder.add("--artifactory-url=" + jfrogPlatformInstance.inferArtifactoryUrl());
-            builder.add("--distribution-url=" + jfrogPlatformInstance.inferDistributionUrl());
-            builder.add("--xray-url=" + jfrogPlatformInstance.inferXrayUrl());
-
-            builder.add("--interactive=false");
-            // The installation process takes place more than once per build, so we will configure the same server ID several times.
-            builder.add("--overwrite=true");
+            addCredentialsArguments(builder, jfrogPlatformInstance, job, launcher, passwordStdinSupported);
+            addUrlArguments(builder, jfrogPlatformInstance);
+            builder.add("--interactive=false").add("--overwrite=true");
+        }
+    }
+
+    static void addCredentialsArguments(ArgumentListBuilder builder, JFrogPlatformInstance jfrogPlatformInstance, Job<?, ?> job, Launcher.ProcStarter launcher, boolean passwordStdinSupported) {
+        String credentialsId = jfrogPlatformInstance.getCredentialsConfig().getCredentialsId();
+        StringCredentials accessTokenCredentials = PluginsUtils.accessTokenCredentialsLookup(credentialsId, job);
+
+        if (accessTokenCredentials != null) {
+            builder.addMasked("--access-token=" + accessTokenCredentials.getSecret().getPlainText());
+        } else {
+            Credentials credentials = PluginsUtils.credentialsLookup(credentialsId, job);
+            builder.add("--user=" + credentials.getUsername());
+            addPasswordArgument(builder, credentials, launcher, passwordStdinSupported);
         }
     }
+
+    /**
+     * Configures the CLI command to provide a password via stdin if supported; otherwise,
+     * uses the default `--password` argument. Stdin support requires a minimum CLI version
+     * and does not work with certain plugin launchers, as they may lose stdin input,
+     * potentially causing command failures.
+     * <p>
+     * This method ensures compatibility by switching to the default password argument when
+     * stdin is unsupported or unsuitable for the launcher being used.
+     *
+     * @param builder                The {@link ArgumentListBuilder} used to construct the CLI command arguments.
+     * @param credentials            The {@link Credentials} object containing the user's password.
+     * @param launcher               The {@link Launcher.ProcStarter} used to execute the command.
+     * @param passwordStdinSupported A boolean flag indicating whether the CLI supports password input via stdin.
+     */
+    private static void addPasswordArgument(ArgumentListBuilder builder, Credentials credentials, Launcher.ProcStarter launcher, boolean passwordStdinSupported) {
+        if (passwordStdinSupported) {
+            // Add argument to read password from stdin
+            builder.add("--password-stdin");
+            ByteArrayInputStream inputStream = new ByteArrayInputStream(credentials.getPassword().getPlainText().getBytes(StandardCharsets.UTF_8));
+            launcher.stdin(inputStream);
+        } else {
+            // Add masked password argument directly to the command
+            builder.addMasked("--password=" + credentials.getPassword());
+        }
+    }
+
+    private static void addUrlArguments(ArgumentListBuilder builder, JFrogPlatformInstance jfrogPlatformInstance) {
+        builder.add("--url=" + jfrogPlatformInstance.getUrl());
+        builder.add("--artifactory-url=" + jfrogPlatformInstance.inferArtifactoryUrl());
+        builder.add("--distribution-url=" + jfrogPlatformInstance.inferDistributionUrl());
+        builder.add("--xray-url=" + jfrogPlatformInstance.inferXrayUrl());
+    }
+
     /**
      * Add build-info Action if the command is 'jf rt bp' or 'jf rt build-publish'.
      *
diff --git a/src/test/java/io/jenkins/plugins/jfrog/JfStepTest.java b/src/test/java/io/jenkins/plugins/jfrog/JfStepTest.java
index 2c9e858f..ffed5864 100644
--- a/src/test/java/io/jenkins/plugins/jfrog/JfStepTest.java
+++ b/src/test/java/io/jenkins/plugins/jfrog/JfStepTest.java
@@ -1,16 +1,29 @@
 package io.jenkins.plugins.jfrog;
 
 import hudson.EnvVars;
+import hudson.FilePath;
+import hudson.Launcher;
+import hudson.model.Job;
+import hudson.util.ArgumentListBuilder;
+import io.jenkins.plugins.jfrog.configuration.CredentialsConfig;
+import io.jenkins.plugins.jfrog.configuration.JFrogPlatformInstance;
+import org.jfrog.build.client.Version;
 import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
 
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
 import java.util.stream.Stream;
 
 import static io.jenkins.plugins.jfrog.JfStep.Execution.getJFrogCLIPath;
+import static io.jenkins.plugins.jfrog.JfStep.MIN_CLI_VERSION_PASSWORD_STDIN;
 import static io.jenkins.plugins.jfrog.JfrogInstallation.JFROG_BINARY_PATH;
-
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.*;
 /**
  * @author yahavi
  **/
@@ -37,4 +50,87 @@ private static Stream<Arguments> jfrogCLIPathProvider() {
                 Arguments.of(new EnvVars(), true, "jf.exe")
         );
     }
+
+    @Test
+    void getJfrogCliVersionTest() throws IOException, InterruptedException {
+        // Mock the Launcher
+        Launcher launcher = mock(Launcher.class);
+        // Mock the Launcher.ProcStarter
+        Launcher.ProcStarter procStarter = mock(Launcher.ProcStarter.class);
+        ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+        // Mocks the return value of --version command
+        outputStream.write("jf version 2.31.0 ".getBytes());
+        // Mock the behavior of the Launcher and ProcStarter
+        when(launcher.launch()).thenReturn(procStarter);
+        when(procStarter.cmds(any(ArgumentListBuilder.class))).thenReturn(procStarter);
+        when(procStarter.pwd((FilePath) any())).thenReturn(procStarter);
+        when(procStarter.stdout(any(ByteArrayOutputStream.class))).thenAnswer(invocation -> {
+            ByteArrayOutputStream out = invocation.getArgument(0);
+            out.write(outputStream.toByteArray());
+            return procStarter;
+        });
+        when(procStarter.join()).thenReturn(0);
+
+        // Create an instance of JfStep and call the method
+        String jfrogBinaryPath = "path/to/jfrog";
+        Version version = JfStep.getJfrogCliVersion(procStarter, jfrogBinaryPath);
+
+        // Verify the result
+        assertEquals("2.31.0", version.toString());
+    }
+
+    /**
+     * Tests the addCredentialsArguments method logic with password-stdin vs.-- password flag.
+     * Password-stdin flag should only be set if the CLI version is supported
+     * AND the launcher is not the plugin launcher.
+     * Plugin launchers do not support password-stdin, as they do not have access to the standard input by default.
+     *
+     * @param cliVersion       The CLI version
+     * @param isPluginLauncher Whether the launcher is a plugin launcher
+     * @param expectedOutput   The expected output
+     */
+    @ParameterizedTest
+    @MethodSource("provideTestArguments")
+    void testAddCredentialsArguments(String cliVersion, boolean isPluginLauncher, String expectedOutput) {
+        // Mock the necessary objects
+        JFrogPlatformInstance jfrogPlatformInstance = mock(JFrogPlatformInstance.class);
+        CredentialsConfig credentialsConfig = mock(CredentialsConfig.class);
+        when(jfrogPlatformInstance.getId()).thenReturn("instance-id");
+        when(jfrogPlatformInstance.getCredentialsConfig()).thenReturn(credentialsConfig);
+        when(credentialsConfig.getCredentialsId()).thenReturn("credentials-id");
+
+        Job<?, ?> job = mock(Job.class);
+        Launcher.ProcStarter launcher = mock(Launcher.ProcStarter.class);
+
+        // Determine if password stdin is supported
+        boolean passwordStdinSupported = new Version(cliVersion).isAtLeast(MIN_CLI_VERSION_PASSWORD_STDIN) && !isPluginLauncher;
+
+        // Create an ArgumentListBuilder
+        ArgumentListBuilder builder = new ArgumentListBuilder();
+
+        // Call the addCredentialsArguments method
+        JfStep.addCredentialsArguments(builder, jfrogPlatformInstance, job, launcher, passwordStdinSupported);
+
+        // Verify the arguments
+        assertTrue(builder.toList().contains(expectedOutput));
+    }
+
+    private static Stream<Arguments> provideTestArguments() {
+        String passwordFlag = "--password=";
+        String passwordStdinFlag = "--password-stdin";
+        // Min version for password stdin is 2.31.3
+        return Stream.of(
+                // Supported CLI version but Plugin Launcher
+                Arguments.of("2.57.0", true, passwordFlag),
+                // Unsupported Version
+                Arguments.of("2.31.0", false, passwordFlag),
+                // Supported CLI version and local launcher
+                Arguments.of("2.57.0", false, passwordStdinFlag),
+                // Unsupported CLI version and Plugin Launcher
+                Arguments.of("2.31.0", true, passwordFlag),
+                // Minimum supported CLI version for password stdin
+                Arguments.of("2.31.3", false, passwordStdinFlag)
+        );
+    }
 }
+
diff --git a/src/test/java/io/jenkins/plugins/jfrog/integration/PipelineTestBase.java b/src/test/java/io/jenkins/plugins/jfrog/integration/PipelineTestBase.java
index 9cba91a9..d6cb11b4 100644
--- a/src/test/java/io/jenkins/plugins/jfrog/integration/PipelineTestBase.java
+++ b/src/test/java/io/jenkins/plugins/jfrog/integration/PipelineTestBase.java
@@ -19,7 +19,6 @@
 import io.jenkins.plugins.jfrog.BinaryInstaller;
 import io.jenkins.plugins.jfrog.JfrogInstallation;
 import io.jenkins.plugins.jfrog.ReleasesInstaller;
-import io.jenkins.plugins.jfrog.configuration.Credentials;
 import io.jenkins.plugins.jfrog.configuration.CredentialsConfig;
 import io.jenkins.plugins.jfrog.configuration.JFrogPlatformBuilder;
 import io.jenkins.plugins.jfrog.configuration.JFrogPlatformInstance;
@@ -161,12 +160,12 @@ private static void verifyEnvironment() {
     private void setGlobalConfiguration() throws IOException {
         JFrogPlatformBuilder.DescriptorImpl jfrogBuilder = (JFrogPlatformBuilder.DescriptorImpl) jenkins.getInstance().getDescriptor(JFrogPlatformBuilder.class);
         Assert.assertNotNull(jfrogBuilder);
-        CredentialsConfig emptyCred = new CredentialsConfig(StringUtils.EMPTY, Credentials.EMPTY_CREDENTIALS);
         CredentialsConfig platformCred = new CredentialsConfig(Secret.fromString(ARTIFACTORY_USERNAME), Secret.fromString(ARTIFACTORY_PASSWORD), Secret.fromString(ACCESS_TOKEN), "credentials");
-        List<JFrogPlatformInstance> artifactoryServers = new ArrayList<JFrogPlatformInstance>() {{
+        List<JFrogPlatformInstance> artifactoryServers = new ArrayList<>() {{
             // Dummy server to test multiple configured servers.
             // The dummy server should be configured first to ensure the right server is being used (and not the first one).
-            add(new JFrogPlatformInstance("dummyServerId", "", emptyCred, "", "", ""));
+            // platformCred cannot be empty since it is used to config the server.
+            add(new JFrogPlatformInstance("dummyServerId", "https://not-a-real-platform", platformCred, ARTIFACTORY_URL, "", ""));
             add(new JFrogPlatformInstance(TEST_CONFIGURED_SERVER_ID, PLATFORM_URL, platformCred, ARTIFACTORY_URL, "", ""));
         }};
         jfrogBuilder.setJfrogInstances(artifactoryServers);