Addressing PR comments

Author: Nirjan Chapagain

src/VirtualClient/VirtualClient.Dependencies.UnitTests/CertificateInstallationTests.cs

@@ -17,6 +17,7 @@
     /// Virtual Client component that installs certificates from Azure Key Vault
     /// into the appropriate certificate store for the operating system.
     /// </summary>
+    [SupportedPlatforms("linux-arm64,linux-x64,win-arm64,win-x64", true)]
     public class CertificateInstallation : VirtualClientComponent
     {
         private ISystemManagement systemManagement;
@@ -84,11 +85,11 @@ public bool WithPrivateKey
         /// <summary>
         /// Gets the directory where the certificate will be exported. If not provided, the certificate will not be exported to a file.
         /// </summary>
-        public string CertificateDownloadDir
+        public string CertificateInstallationDir
         {
             get
             {
-                return this.Parameters.GetValue<string>(nameof(this.CertificateDownloadDir), string.Empty);
+                return this.Parameters.GetValue<string>(nameof(this.CertificateInstallationDir));
             }
         }
 
@@ -125,39 +126,11 @@ protected override async Task ExecuteAsync(EventContext telemetryContext, Cancel
                 IKeyVaultManager keyVault = this.GetKeyVaultManager();
                 X509Certificate2 certificate = await keyVault.GetCertificateAsync(this.CertificateName, cancellationToken, null, this.WithPrivateKey);
 
-                if (this.Platform == PlatformID.Win32NT)
-                {
-                    await this.InstallCertificateOnWindowsAsync(certificate, cancellationToken);
-                }
-                else if (this.Platform == PlatformID.Unix)
-                {
-                    await this.InstallCertificateOnUnixAsync(certificate, cancellationToken);
-                }
-                else
-                {
-                    throw new PlatformNotSupportedException($"The '{nameof(CertificateInstallation)}' component is not supported on platform '{this.Platform}'.");
-                }
+                await this.InstallCertificateOnMachineAsync(certificate, cancellationToken);
 
-                // Export the certificate if requested
-                if (!string.IsNullOrEmpty(this.CertificateDownloadDir))
+                if (!string.IsNullOrWhiteSpace(this.CertificateInstallationDir))
                 {
-                    string certificateFileName = this.WithPrivateKey
-                        ? $"{this.CertificateName}.pfx"
-                        : $"{this.CertificateName}.cer";
-                    
-                    X509ContentType contentType = this.WithPrivateKey 
-                        ? X509ContentType.Pfx 
-                        : X509ContentType.Cert;
-
-                    string certificatePath = this.Combine(this.CertificateDownloadDir, certificateFileName);
-
-                    if (this.fileSystem.File.Exists(certificatePath))
-                    {
-                        this.fileSystem.File.Delete(certificatePath);
-                    }
-
-                    byte[] certBytes = certificate.Export(contentType, string.Empty);
-                    await this.fileSystem.File.WriteAllBytesAsync(certificatePath, certBytes);
+                    await this.InstallCertificateLocallyAsync(certificate, cancellationToken);
                 }
             }
             catch (Exception exc)
@@ -169,9 +142,9 @@ protected override async Task ExecuteAsync(EventContext telemetryContext, Cancel
         }
 
         /// <summary>
-        /// Installs the certificate in the appropriate certificate store on a Windows system.
+        /// Installs the certificate in the appropriate certificate store.
         /// </summary>
-        protected virtual Task InstallCertificateOnWindowsAsync(X509Certificate2 certificate, CancellationToken cancellationToken)
+        protected virtual Task InstallCertificateOnMachineAsync(X509Certificate2 certificate, CancellationToken cancellationToken)
         {
             return Task.Run(() =>
             {
@@ -185,72 +158,6 @@ protected virtual Task InstallCertificateOnWindowsAsync(X509Certificate2 certifi
             });
         }
 
-        /// <summary>
-        /// Installs the certificate in the appropriate certificate store on a Unix/Linux system.
-        /// </summary>
-        protected virtual async Task InstallCertificateOnUnixAsync(X509Certificate2 certificate, CancellationToken cancellationToken)
-        {
-            // On Unix/Linux systems, we install the certificate in the default location for the
-            // user as well as in a static location. In the future we will likely use the static location
-            // only.
-            string certificateDirectory = null;
-
-            try
-            {
-                // When "sudo" is used to run the installer, we need to know the logged
-                // in user account. On Linux systems, there is an environment variable 'SUDO_USER'
-                // that defines the logged in user.
-
-                string user = this.GetEnvironmentVariable(EnvironmentVariable.USER);
-                string sudoUser = this.GetEnvironmentVariable(EnvironmentVariable.SUDO_USER);
-                certificateDirectory = $"/home/{user}/.dotnet/corefx/cryptography/x509stores/my";
-
-                if (!string.IsNullOrWhiteSpace(sudoUser))
-                {
-                    // The installer is being executed with "sudo" privileges. We want to use the
-                    // logged in user profile vs. "root".
-                    certificateDirectory = $"/home/{sudoUser}/.dotnet/corefx/cryptography/x509stores/my";
-                }
-                else if (user == "root")
-                {
-                    // The installer is being executed from the "root" account on Linux.
-                    certificateDirectory = $"/root/.dotnet/corefx/cryptography/x509stores/my";
-                }
-
-                Console.WriteLine($"Certificate Store = {certificateDirectory}");
-
-                if (!this.fileSystem.Directory.Exists(certificateDirectory))
-                {
-                    this.fileSystem.Directory.CreateDirectory(certificateDirectory);
-                }
-
-                using (X509Store store = new X509Store(StoreName.My, StoreLocation.CurrentUser, OpenFlags.ReadWrite))
-                {
-                    store.Open(OpenFlags.ReadWrite);
-                    store.Add(certificate);
-                    store.Close();
-                }
-
-                await this.fileSystem.File.WriteAllBytesAsync(
-                    this.Combine(certificateDirectory, $"{certificate.Thumbprint}.pfx"),
-                    certificate.Export(X509ContentType.Pfx));
-
-                // Permissions 777 (-rwxrwxrwx)
-                // https://linuxhandbook.com/linux-file-permissions/
-                using (IProcessProxy process = this.processManager.CreateProcess("chmod", $"-R 777 {certificateDirectory}"))
-                {
-                    await process.StartAndWaitAsync(cancellationToken);
-                    process.ThrowIfErrored<DependencyException>();
-                }
-            }
-            catch (UnauthorizedAccessException)
-            {
-                throw new UnauthorizedAccessException(
-                    $"Access permissions denied for certificate directory '{certificateDirectory}'. Execute the installer with " +
-                    $"sudo/root privileges to install SDK certificates in privileged locations.");
-            }
-        }
-
         /// <summary>
         /// Gets the Key Vault manager to use to retrieve certificates from Key Vault.
         /// </summary>
@@ -278,5 +185,50 @@ protected IKeyVaultManager GetKeyVaultManager()
                     $"Either valid --KeyVault or --Token or --TokenPath must be passed in order to set up authentication with Key Vault.");
             }
         }
+
+        /// <summary>
+        /// Installs the certificate in static location
+        /// </summary>
+        protected async Task InstallCertificateLocallyAsync(X509Certificate2 certificate, CancellationToken cancellationToken)
+        {
+            try
+            {
+                string certificateFileName = this.WithPrivateKey
+                    ? $"{this.CertificateName}.pfx"
+                    : $"{this.CertificateName}.cer";
+
+                X509ContentType contentType = this.WithPrivateKey
+                    ? X509ContentType.Pfx
+                    : X509ContentType.Cert;
+
+                byte[] certBytes = certificate.Export(contentType, string.Empty);
+
+                string certificatePath = this.Combine(this.CertificateInstallationDir, certificateFileName);
+
+                if (!this.fileSystem.Directory.Exists(this.CertificateInstallationDir))
+                {
+                    this.fileSystem.Directory.CreateDirectory(this.CertificateInstallationDir);
+                }
+                
+                await this.fileSystem.File.WriteAllBytesAsync(certificatePath, certBytes);
+
+                if (this.Platform == PlatformID.Unix)
+                {
+                    // Permissions 777 (-rwxrwxrwx)
+                    // https://linuxhandbook.com/linux-file-permissions/
+                    using (IProcessProxy process = this.processManager.CreateProcess("chmod", $"-R 777 {this.CertificateInstallationDir}"))
+                    {
+                        await process.StartAndWaitAsync(cancellationToken);
+                        process.ThrowIfErrored<DependencyException>();
+                    }
+                }
+            }
+            catch (UnauthorizedAccessException)
+            {
+                throw new UnauthorizedAccessException(
+                    $"Access permissions denied for certificate directory '{this.CertificateInstallationDir}'. Execute the installer with " +
+                    $"admin/sudo/root privileges to install certificates in privileged locations.");
+            }
+        }
     }
 }

src/VirtualClient/VirtualClient.Dependencies/CertificateInstallation.cs

@@ -54,12 +54,9 @@ internal class BootstrapCommand : ExecuteProfileCommand
         public string TenantId { get; set; }
 
         /// <summary>
-        /// Directory path where downloaded certificates can be stored.
+        /// Directory path where certificates can be stored. This is optional, but if not provided, certificates will not be saved to disk.
         /// </summary>
-        /// <remarks>Set this property to a valid directory path to ensure that certificates can be
-        /// downloaded and saved.
-        /// </remarks>
-        public string CertificateDownloadDir { get; set; }
+        public string CertificateInstallationDir { get; set; }
 
         /// <summary>
         /// Executes the bootstrap command.
@@ -147,9 +144,9 @@ protected void SetupCertificateInstallation()
                     "The Key Vault URI must be provided (--key-vault) when installing certificates (--cert-name).");
             }
 
-            if (!string.IsNullOrWhiteSpace(this.CertificateDownloadDir))
+            if (!string.IsNullOrWhiteSpace(this.CertificateInstallationDir))
             {
-                this.Parameters["CertificateDownloadDir"] = this.CertificateDownloadDir;
+                this.Parameters["CertificateInstallationDir"] = this.CertificateInstallationDir;
             }
 
             // Set certificate-related parameters

src/VirtualClient/VirtualClient.Main/BootstrapCommand.cs

@@ -605,7 +605,7 @@ public static Option CreateKeyVaultOption(bool required = false, object defaultV
         /// <param name="defaultValue">Sets the default value when none is provided.</param>
         public static Option CreateTokenOption(bool required = false, object defaultValue = null)
         {
-            Option<string> option = new Option<string>(new string[] { "--token", "--access-token" })
+            Option<string> option = new Option<string>(new string[] { "--token" })
             {
                 Name = "AccessToken",
                 Description = "Authentication token for Azure Key Vault access. When not provided, uses default Azure credential authentication (Azure CLI, Managed Identity, etc.).",
@@ -624,7 +624,7 @@ public static Option CreateTokenOption(bool required = false, object defaultValu
         /// <param name="defaultValue">Sets the default value when none is provided.</param>
         public static Option CreateCertificateNameOption(bool required = false, object defaultValue = null)
         {
-            Option<string> option = new Option<string>(new string[] { "--certname", "--certificate-name", "--cert-name" })
+            Option<string> option = new Option<string>(new string[] {"--certificate-name", "--cert-name" })
             {
                 Name = "CertificateName",
                 Description = "The name of the certificate in Azure Key Vault to install to the local certificate store.",
@@ -643,7 +643,7 @@ public static Option CreateCertificateNameOption(bool required = false, object d
         /// <param name="defaultValue">Sets the default value when none is provided.</param>
         public static Option CreateTenantIdOption(bool required = false, object defaultValue = null)
         {
-            Option<string> option = new Option<string>(new string[] { "--tenant-id", "--tid" })
+            Option<string> option = new Option<string>(new string[] { "--tenant-id" })
             {
                 Name = "TenantId",
                 Description = "The tenant ID associated with your Microsoft Entra ID.",
@@ -660,11 +660,11 @@ public static Option CreateTenantIdOption(bool required = false, object defaultV
         /// </summary>
         /// <param name="required">Sets this option as required.</param>
         /// <param name="defaultValue">Sets the default value when none is provided.</param>
-        public static Option CreateCertificateDownloadDirectoryOption(bool required = false, object defaultValue = null)
+        public static Option CreateCertificateInstallationDirectoryOption(bool required = false, object defaultValue = null)
         {
-            Option<string> option = new Option<string>(new string[] { "--certificateDownloadDir", "--certDownloadDir" })
+            Option<string> option = new Option<string>(new string[] { "--cert-installation-dir" })
             {
-                Name = "CertificateDownloadDir",
+                Name = "CertificateInstallationDir",
                 Description = "Defines the directory where certificates downloaded from Key Vault will be saved.",
                 ArgumentHelpName = "path",
                 AllowMultipleArgumentsPerToken = false

src/VirtualClient/VirtualClient.Main/OptionFactory.cs

@@ -311,13 +311,7 @@ internal static CommandLineBuilder SetupCommandLine(string[] args, CancellationT
 
                 // --verbose
                 OptionFactory.CreateVerboseFlag(required: false, false),
-                
-                // --token
-                OptionFactory.CreateTokenOption(required: false),
-
-                // --CertificateDownloadDir
-                OptionFactory.CreateCertificateDownloadDirectoryOption(required: false),
-                
+                                
                 // --tenant-id
                 OptionFactory.CreateTenantIdOption(required: false),
             };
@@ -481,8 +475,8 @@ private static Command CreateBootstrapSubcommand(DefaultSettings settings)
                 // --token
                 OptionFactory.CreateTokenOption(required: false),
 
-                // --CertificateDownloadDir
-                OptionFactory.CreateCertificateDownloadDirectoryOption(required: false),
+                // --cert-installation-dir
+                OptionFactory.CreateCertificateInstallationDirectoryOption(required: false),
 
                 // --tenant-id
                 OptionFactory.CreateTenantIdOption(required: false),

src/VirtualClient/VirtualClient.Main/Program.cs

@@ -5,7 +5,8 @@
     "CertificateName": null,
     "AccessToken": null,
     "LogFileName": null,
-    "CertificateDownloadDir": null
+    "CertificateInstallationDir": null,
+    "WithPrivateKey": true
   },
   "Actions": [
     {
@@ -16,7 +17,8 @@
         "CertificateName": "$.Parameters.CertificateName",
         "AccessToken": "$.Parameters.AccessToken",
         "AccessTokenPath": "$.Parameters.LogFileName",
-        "CertificateDownloadDir": "$.Parameters.CertificateDownloadDir"
+        "CertificateInstallationDir": "$.Parameters.CertificateInstallationDir",
+        "WithPrivateKey": "$.Parameters.WithPrivateKey"
       }
     }
   ]

src/VirtualClient/VirtualClient.Main/profiles/BOOTSTRAP-CERTIFICATE.json

@@ -137,14 +137,14 @@ public void SetupCertificateInstallation_AllowsCertificateDownloadDirectory()
                 KeyVault = "https://myvault.vault.azure.net/",
                 AccessToken = "token123",
                 TenantId = "00000000-0000-0000-0000-000000000001",
-                CertificateDownloadDir = "C:\\certs"
+                CertificateInstallationDir = "C:\\certs"
             };
 
             command.SetupCertificateInstallationPublic();
 
             Assert.AreEqual(command.Parameters["KeyVaultUri"], command.KeyVault);
             Assert.AreEqual(command.Parameters["CertificateName"], command.CertificateName);
-            Assert.AreEqual(command.Parameters["CertificateDownloadDir"], command.CertificateDownloadDir);
+            Assert.AreEqual(command.Parameters["CertificateInstallationDir"], command.CertificateInstallationDir);
             Assert.AreEqual(command.Parameters.Count, 3);
         }
 

src/VirtualClient/VirtualClient.UnitTests/BootstrapCommandTests.cs

@@ -27,7 +27,6 @@ public void BootstrapCommand_RequiresAtLeastOneOperation()
 
         [Test]
         [TestCase("--cert-name")]
-        [TestCase("--certname")]
         [TestCase("--certificate-name")]
         public void BootstrapCommand_CertificateInstall_RequiresKeyVault(string certAlias)
         {