Zd#144472

CHANGELOG.md

@@ -1,3 +1,16 @@
+v2.11.4
+- Bug Fix: Handle condition where a certificate store definition that contains an invalid value for `FileTransferProtocol` 
+would return empty inventory. If no value is set or an invalid value is set, the default value of `Both` will be used 
+and a warning status will be returned for the job.
+
+> [!IMPORTANT]
+> Due to an issue in Keyfactor Command versions through 25.2.1, when adding multiple choice store properties to exiting 
+> certificate store types, existing certificate stores receive incorrect initialization data for the new property. 
+> If you have upgraded your store type from something prior to version 2.10.0, ensure that each existing certificate 
+> store has a valid value for `FileTransferProtocol`. Valid values are `SCP`, `SFTP`, `Both`, otherwise inventory jobs **may report 
+> empty certificate store inventories**. Extension version 2.11.4 compensates for this, but upgrading customers should 
+> check their store’s configuration for proper `FileTransferProtocol` values.
+
 v2.11.3
 - Change returned result of a Management-Create job for a store that already exists from 'Failure' to 'Warning'
 

RemoteFile.UnitTests/ApplicationSettingsTests.cs

@@ -0,0 +1,23 @@
+using Xunit;
+using Keyfactor.Extensions.Orchestrator.RemoteFile;
+
+namespace RemoteFile.UnitTests;
+
+public class ApplicationSettingsTests
+{
+    [Fact]
+    public void FileTransferProtocol_WhenPopulatedWithValidValue_ReturnsValue()
+    {
+        var path = Path.Combine(Directory.GetCurrentDirectory(), "fixtures", "config", "valid", "config.json");
+        ApplicationSettings.Initialize(path);
+        Assert.Equal(ApplicationSettings.FileTransferProtocolEnum.SCP, ApplicationSettings.FileTransferProtocol);
+    }
+
+    [Fact]
+    public void FileTransferProtocol_WhenAllThreePopulated_DefaultsToBoth()
+    {
+        var path = Path.Combine(Directory.GetCurrentDirectory(), "fixtures", "config", "file_transfer_protocol_all_three", "config.json");
+        ApplicationSettings.Initialize(path);
+        Assert.Equal(ApplicationSettings.FileTransferProtocolEnum.Both, ApplicationSettings.FileTransferProtocol);
+    }
+}

RemoteFile.UnitTests/PropertyUtilitiesTests.cs

@@ -0,0 +1,48 @@
+using Keyfactor.Extensions.Orchestrator.RemoteFile;
+using Xunit;
+
+namespace RemoteFile.UnitTests;
+
+public class PropertyUtilitiesTests
+{
+    [Theory]
+    [InlineData("SCP", ApplicationSettings.FileTransferProtocolEnum.SCP)]
+    [InlineData("SFTP", ApplicationSettings.FileTransferProtocolEnum.SFTP)]
+    [InlineData("Both", ApplicationSettings.FileTransferProtocolEnum.Both)]
+    public void TryEnumParse_WhenProvidedAValidEnumString_MapsToExpectedEnumValue(string input,
+        ApplicationSettings.FileTransferProtocolEnum expected)
+    {
+        var isValid = PropertyUtilities.TryEnumParse(input, out var isFlagCombination,
+            out ApplicationSettings.FileTransferProtocolEnum result);
+
+        Assert.True(isValid);
+        Assert.Equal(expected, result);
+        Assert.False(isFlagCombination);
+    }
+
+    [Fact]
+    public void TryEnumParse_WhenProvidedAFlagCombination_SetsIsFlagCombination()
+    {
+        var input = "SCP,SFTP,Both";
+
+        var isValid = PropertyUtilities.TryEnumParse(input, out var isFlagCombination,
+            out ApplicationSettings.FileTransferProtocolEnum result);
+
+        Assert.True(isValid);
+        Assert.Equal((ApplicationSettings.FileTransferProtocolEnum) 3, result);
+        Assert.True(isFlagCombination);
+    }
+
+    [Fact]
+    public void TryEnumParse_WhenProvidedAnInvalidMapping_MarksIsValidAsFalse()
+    {
+        var input = "randomstring";
+
+        var isValid = PropertyUtilities.TryEnumParse(input, out var isFlagCombination,
+            out ApplicationSettings.FileTransferProtocolEnum result);
+
+        Assert.False(isValid);
+        Assert.Equal(ApplicationSettings.FileTransferProtocolEnum.SCP, result);
+        Assert.False(isFlagCombination);
+    }
+}

RemoteFile.UnitTests/RemoteFile.UnitTests.csproj

@@ -0,0 +1,40 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+    <PropertyGroup>
+        <TargetFramework>net6.0</TargetFramework>
+        <ImplicitUsings>enable</ImplicitUsings>
+        <Nullable>enable</Nullable>
+
+        <IsPackable>false</IsPackable>
+    </PropertyGroup>
+
+    <ItemGroup>
+        <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.1.0"/>
+        <PackageReference Include="xunit" Version="2.4.1"/>
+        <PackageReference Include="xunit.runner.visualstudio" Version="2.4.3">
+            <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+            <PrivateAssets>all</PrivateAssets>
+        </PackageReference>
+        <PackageReference Include="coverlet.collector" Version="3.1.2">
+            <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+            <PrivateAssets>all</PrivateAssets>
+        </PackageReference>
+    </ItemGroup>
+
+    <ItemGroup>
+      <ProjectReference Include="..\RemoteFile\RemoteFile.csproj" />
+    </ItemGroup>
+
+    <ItemGroup>
+      <None Update="fixtures\config\valid\config.json">
+        <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+      </None>
+      <None Update="fixtures\config\file_transfer_protocol_all_three\config.json">
+        <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+      </None>
+      <None Update="fixtures\config\valid_scp\config.json">
+        <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+      </None>
+    </ItemGroup>
+
+</Project>

RemoteFile.UnitTests/fixtures/config/file_transfer_protocol_all_three/config.json

@@ -0,0 +1,11 @@
+{
+  "UseSudo": "N",
+  "DefaultSudoImpersonatedUser": "",
+  "CreateStoreIfMissing": "N",
+  "UseNegotiate": "N",
+  "SeparateUploadFilePath": "",
+  "FileTransferProtocol": "Both,SCP,SFTP",
+  "DefaultLinuxPermissionsOnStoreCreation": "600",
+  "DefaultOwnerOnStoreCreation": "",
+  "SSHPort":  ""
+}

RemoteFile.UnitTests/fixtures/config/valid/config.json

@@ -0,0 +1,11 @@
+{
+  "UseSudo": "N",
+  "DefaultSudoImpersonatedUser": "",
+  "CreateStoreIfMissing": "N",
+  "UseNegotiate": "N",
+  "SeparateUploadFilePath": "",
+  "FileTransferProtocol": "SCP",
+  "DefaultLinuxPermissionsOnStoreCreation": "600",
+  "DefaultOwnerOnStoreCreation": "",
+  "SSHPort":  ""
+}

RemoteFile.sln

@@ -5,6 +5,10 @@ VisualStudioVersion = 16.0.31702.278
 MinimumVisualStudioVersion = 10.0.40219.1
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "RemoteFile", "RemoteFile\RemoteFile.csproj", "{A006BFAB-20F7-4F42-8B5F-591268ACE836}"
 EndProject
+Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "tests", "tests", "{856DF77E-EB78-45EB-836F-50C3B017B4C2}"
+EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "RemoteFile.UnitTests", "RemoteFile.UnitTests\RemoteFile.UnitTests.csproj", "{2769EBA9-6C62-4409-B637-FFA86E23749E}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -15,11 +19,18 @@ Global
 		{A006BFAB-20F7-4F42-8B5F-591268ACE836}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{A006BFAB-20F7-4F42-8B5F-591268ACE836}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{A006BFAB-20F7-4F42-8B5F-591268ACE836}.Release|Any CPU.Build.0 = Release|Any CPU
+		{2769EBA9-6C62-4409-B637-FFA86E23749E}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{2769EBA9-6C62-4409-B637-FFA86E23749E}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{2769EBA9-6C62-4409-B637-FFA86E23749E}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{2769EBA9-6C62-4409-B637-FFA86E23749E}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {8F3245C7-FCC9-4666-99E0-F8D63BBE8373}
 	EndGlobalSection
+	GlobalSection(NestedProjects) = preSolution
+		{2769EBA9-6C62-4409-B637-FFA86E23749E} = {856DF77E-EB78-45EB-836F-50C3B017B4C2}
+	EndGlobalSection
 EndGlobal

RemoteFile/ApplicationSettings.cs

@@ -16,7 +16,7 @@
 
 namespace Keyfactor.Extensions.Orchestrator.RemoteFile 
 {
-    class ApplicationSettings
+    public class ApplicationSettings
     {
         public enum FileTransferProtocolEnum
         {
@@ -63,15 +63,27 @@
         { 
             get 
             {
+                ILogger logger = LogHandler.GetClassLogger<ApplicationSettings>();
+
                 string protocolNames = string.Empty;
                 foreach (string protocolName in Enum.GetNames(typeof(FileTransferProtocolEnum)))
                 {
                     protocolNames += protocolName + ", ";
                 }
                 protocolNames = protocolNames.Substring(0, protocolNames.Length - 2);
+                string? protocolValue = configuration["FileTransferProtocol"];
 
-                if (!Enum.TryParse(configuration["FileTransferProtocol"], out FileTransferProtocolEnum protocol))
-                    throw new RemoteFileException($"Invalid optional config.json FileTransferProtocol option of {configuration["FileTransferProtocol"]}.  If present, must be one of these values: {protocolNames}.");
+                if (!PropertyUtilities.TryEnumParse(protocolValue, out bool isFlagCombination, out FileTransferProtocolEnum protocol))
+                    throw new RemoteFileException($"Invalid optional config.json FileTransferProtocol option of {protocolValue}.  If present, must be one of these values: {protocolNames}.");
+
+                // Issue: If received a comma-delimited list ("SCP,SFTP,Both"), it's treating it as a flag combination (i.e. mapping it to 0+1+2=3)
+                // If this happens, we want to default it to Both so it's resolved as a valid mapping.
+                if (isFlagCombination)
+                {
+                    logger.LogWarning($"FileTransferProtocol config value {protocolValue} mapped to a flag combination. Setting FileTransferProtocol explicitly to Both.");
+                    protocol = FileTransferProtocolEnum.Both;
+                }
+
                 return protocol; 
             }
         }

RemoteFile/ImplementedStoreTypes/PEM/PEMCertificateStoreSerializer.cs

@@ -200,6 +200,9 @@ private void LoadCustomProperties(string storeProperties)
             IncludesChain = properties.IncludesChain == null || string.IsNullOrEmpty(properties.IncludesChain.Value) ? false : bool.Parse(properties.IncludesChain.Value);
             SeparatePrivateKeyFilePath = properties.SeparatePrivateKeyFilePath == null || string.IsNullOrEmpty(properties.SeparatePrivateKeyFilePath.Value) ? String.Empty : properties.SeparatePrivateKeyFilePath.Value;
             IgnorePrivateKeyOnInventory = properties.IgnorePrivateKeyOnInventory == null || string.IsNullOrEmpty(properties.IgnorePrivateKeyOnInventory.Value) ? false : bool.Parse(properties.IgnorePrivateKeyOnInventory.Value);
+
+            logger.LogDebug("Custom Properties have been loaded:");
+            logger.LogDebug($"IsTrustStore: {IsTrustStore}, IncludesChain: {IncludesChain}, SeparatePrivateKeyFilePath: {SeparatePrivateKeyFilePath}, IgnorePrivateKeyOnInventory: {IgnorePrivateKeyOnInventory}");
 
             logger.MethodExit(LogLevel.Debug);
         }

RemoteFile/InventoryBase.cs

@@ -7,6 +7,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Security.Cryptography.X509Certificates;
 
 using Keyfactor.Orchestrators.Extensions;
@@ -73,7 +74,12 @@ public JobResult ProcessJob(InventoryJobConfiguration config, SubmitInventoryUpd
             catch (Exception ex)
             {
                 logger.LogError($"Exception for {config.Capability}: {RemoteFileException.FlattenExceptionMessages(ex, string.Empty)} for job id {config.JobId}");
-                return new JobResult() { Result = OrchestratorJobStatusJobResult.Failure, JobHistoryId = config.JobHistoryId, FailureMessage = RemoteFileException.FlattenExceptionMessages(ex, $"Site {config.CertificateStoreDetails.StorePath} on server {config.CertificateStoreDetails.ClientMachine}:") };
+                return new JobResult 
+                    { 
+                        Result = OrchestratorJobStatusJobResult.Failure, 
+                        JobHistoryId = config.JobHistoryId, 
+                        FailureMessage = RemoteFileException.FlattenExceptionMessages(ex, $"Site {config.CertificateStoreDetails.StorePath} on server {config.CertificateStoreDetails.ClientMachine}:") 
+                    };
             }
             finally
             {
@@ -84,14 +90,31 @@ public JobResult ProcessJob(InventoryJobConfiguration config, SubmitInventoryUpd
             try
             {
                 submitInventory.Invoke(inventoryItems);
-                logger.LogDebug($"...End {config.Capability} job for job id {config.JobId}");
-                return new JobResult() { Result = OrchestratorJobStatusJobResult.Success, JobHistoryId = config.JobHistoryId };
+                logger.LogDebug("...End {ConfigCapability} job for job id {ConfigJobId}", config.Capability, config.JobId);
+                var jobResultStatus = OrchestratorJobStatusJobResult.Success;
+                var jobMsg = string.Empty;
+                if (certificateStore.RemoteHandler != null && certificateStore.RemoteHandler.Warnings.Length > 0)
+                {
+                    jobResultStatus = OrchestratorJobStatusJobResult.Warning;
+                    jobMsg = certificateStore.RemoteHandler.Warnings.Aggregate(jobMsg, (current, warning) => current + (warning + Environment.NewLine));
+                }
+                return new JobResult
+                {
+                    Result = jobResultStatus, 
+                    JobHistoryId = config.JobHistoryId,
+                    FailureMessage = jobMsg
+                };
             }
             catch (Exception ex)
             {
                 string errorMessage = RemoteFileException.FlattenExceptionMessages(ex, string.Empty);
                 logger.LogError($"Exception returning certificates for {config.Capability}: {errorMessage} for job id {config.JobId}");
-                return new JobResult() { Result = OrchestratorJobStatusJobResult.Failure, JobHistoryId = config.JobHistoryId, FailureMessage = RemoteFileException.FlattenExceptionMessages(ex, $"Site {config.CertificateStoreDetails.StorePath} on server {config.CertificateStoreDetails.ClientMachine}:") };
+                return new JobResult
+                {
+                    Result = OrchestratorJobStatusJobResult.Failure, 
+                    JobHistoryId = config.JobHistoryId, 
+                    FailureMessage = RemoteFileException.FlattenExceptionMessages(ex, $"Site {config.CertificateStoreDetails.StorePath} on server {config.CertificateStoreDetails.ClientMachine}:")
+                };
             }
         }
     }

RemoteFile/PropertyUtilities.cs

@@ -0,0 +1,35 @@
+using System;
+
+namespace Keyfactor.Extensions.Orchestrator.RemoteFile;
+
+public static class PropertyUtilities
+{
+    public static bool TryEnumParse<T>(string value, out bool isFlagCombination, out T result) where T : struct, Enum
+    {
+        isFlagCombination = false;
+        result = default(T);
+
+        // First, do the normal TryParse
+        if (!Enum.TryParse<T>(value, out result))
+        {
+            return false;
+        }
+
+        // Check if the enum has the Flags attribute
+        bool hasFlags = typeof(T).GetCustomAttributes(typeof(FlagsAttribute), false).Length > 0;
+
+        // If it doesn't have Flags attribute but the input contains commas,
+        // this might be unintended flag parsing
+        if (!hasFlags && value.Contains(','))
+        {
+            // Check if the parsed result corresponds to a defined enum value
+            if (!Enum.IsDefined(typeof(T), result))
+            {
+                isFlagCombination = true;
+                return true;
+            }
+        }
+
+        return true;
+    }
+}

RemoteFile/RemoteCertificateStore.cs

@@ -252,6 +252,7 @@ internal void CreateCertificateStore(ICertificateStoreSerializer certificateStor
         internal void AddCertificate(string alias, string certificateEntry, bool overwrite, string pfxPassword, bool removeRootCertificate)
         {
             logger.MethodEntry(LogLevel.Debug);
+            logger.LogDebug($"Alias: {alias}, Certificate Entry: {certificateEntry}, Overwrite: {overwrite}, RemoveRootCertificate: {removeRootCertificate}");
 
             try
             {