Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AzureKeyVault.sln
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
docsource\content.md = docsource\content.md
create_sp_azure.md = create_sp_azure.md
integration-manifest.json = integration-manifest.json
rbac.md = rbac.md
EndProjectSection
EndProject
Global
Expand Down
27 changes: 22 additions & 5 deletions AzureKeyVault/AzureClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
using Keyfactor.Orchestrators.Common.Enums;
using Keyfactor.Orchestrators.Extensions;
using Microsoft.Extensions.Logging;
using Microsoft.VisualBasic;

Copilot AI Apr 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused using Microsoft.VisualBasic; appears to have been added but nothing in this file references it. Please remove the unused import to avoid unnecessary dependencies and compiler warnings.

Suggested change
using Microsoft.VisualBasic;

Copilot uses AI. Check for mistakes.
using Newtonsoft.Json;

namespace Keyfactor.Extensions.Orchestrator.AzureKeyVault
{
Expand All @@ -38,8 +40,6 @@ private Uri AzureCloudEndpoint
case "china":
logger.LogTrace(AzureAuthorityHosts.AzureChina.ToString());
return AzureAuthorityHosts.AzureChina;
//case "germany":
// return AzureAuthorityHosts.AzureGermany; // germany is no longer a valid azure authority host as of 2021
case "government":
logger.LogTrace(AzureAuthorityHosts.AzureGovernment.ToString());
return AzureAuthorityHosts.AzureGovernment;
Expand Down Expand Up @@ -199,7 +199,7 @@ public virtual async Task<KeyVaultResource> CreateVault()
}
}

public virtual async Task<KeyVaultCertificateWithPolicy> ImportCertificateAsync(string certName, string contents, string pfxPassword, Dictionary<string,string> tags)
public virtual async Task<KeyVaultCertificateWithPolicy> ImportCertificateAsync(string certName, string contents, string pfxPassword, Dictionary<string, string> tags, bool nonExportable)
{
try
{
Expand All @@ -221,6 +221,7 @@ public virtual async Task<KeyVaultCertificateWithPolicy> ImportCertificateAsync(
logger.LogTrace($"calling ImportCertificateAsync on the KeyVault certificate client to import certificate {certName}");

var options = new ImportCertificateOptions(certName, p12bytes);
options.Policy = new CertificatePolicy { Exportable = !nonExportable, ContentType = CertificateContentType.Pkcs12 };

if (tags.Any())
{
Expand Down Expand Up @@ -303,15 +304,31 @@ public virtual async Task<IEnumerable<CurrentInventoryItem>> GetCertificatesAsyn
{
var cert = await CertClient.GetCertificateAsync(certificate.Name);
logger.LogTrace($"got certificate details");
logger.LogTrace($"cert properties: {JsonConvert.SerializeObject(cert.Value?.Properties)}");
var itemEntryParams = new Dictionary<string, object>();

if (cert.Value?.Properties?.Tags != null && cert.Value.Properties.Tags.Count > 0) { // set tags entry parameter to value
itemEntryParams.Add(EntryParameters.TAGS, JsonConvert.SerializeObject(cert.Value.Properties.Tags));
}

if (cert.Value.Policy != null) // set nonexportable entry parameter to value
{
var exportable = cert.Value.Policy?.Exportable;
itemEntryParams.Add(EntryParameters.NON_EXPORTABLE, !exportable);

Copilot AI Apr 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cert.Value.Policy?.Exportable is nullable; using !exportable can yield null, which means NonExportable may be returned as a null value even though the store type defines it as a Bool. Consider normalizing this to a concrete boolean (or omitting the parameter when the exportable state is unknown) so inventory consistently returns the expected type.

Suggested change
itemEntryParams.Add(EntryParameters.NON_EXPORTABLE, !exportable);
if (exportable.HasValue)
{
itemEntryParams.Add(EntryParameters.NON_EXPORTABLE, !exportable.Value);
}

Copilot uses AI. Check for mistakes.
}

itemEntryParams.Add(EntryParameters.PRESERVE_TAGS, null); // we can never know this; it's only evaluated on enrollment; set to null

logger.LogTrace($"evaluated entry parameters to be returned: {JsonConvert.SerializeObject(itemEntryParams)}");

inventoryItems.Add(new CurrentInventoryItem()
{
Alias = cert.Value.Name,
PrivateKeyEntry = true,
ItemStatus = OrchestratorInventoryItemStatus.Unknown,
UseChainLevel = true,
Certificates = new List<string>() { Convert.ToBase64String(cert.Value.Cer) },
Parameters = cert.Value.Properties.Tags as Dictionary<string, object>
Parameters = itemEntryParams
});
}
catch (Exception ex)
Expand Down Expand Up @@ -388,7 +405,7 @@ public virtual (List<string>, List<string>) GetVaults()
var warning = $"Exception thrown performing discovery on tenantId {searchTenantId} and subscription ID {searchSubscription}. Exception message: {ex.Message}";

logger.LogWarning(warning);
warnings.Add(warning);
warnings.Add(warning);
}

return (vaultNames, warnings);
Expand Down
31 changes: 16 additions & 15 deletions AzureKeyVault/AzureKeyVault.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,27 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Azure.Core" Version="1.45.0" />
<PackageReference Include="Azure.Identity" Version="1.13.2" />
<PackageReference Include="Azure.ResourceManager" Version="1.13.0" />
<PackageReference Include="Azure.ResourceManager.KeyVault" Version="1.3.0" />
<PackageReference Include="Azure.ResourceManager.Resources" Version="1.9.0" />
<PackageReference Include="Azure.Security.KeyVault.Administration" Version="4.5.0" />
<PackageReference Include="Azure.Security.KeyVault.Certificates" Version="4.7.0" />
<PackageReference Include="Azure.Security.KeyVault.Secrets" Version="4.7.0" />
<PackageReference Include="Azure.Storage.Blobs" Version="12.23.0" />
<PackageReference Include="Azure.Core" Version="1.51.1" />
<PackageReference Include="Azure.Identity" Version="1.17.1" />
<PackageReference Include="Azure.ResourceManager" Version="1.13.2" />
<PackageReference Include="Azure.ResourceManager.KeyVault" Version="1.3.3" />
<PackageReference Include="Azure.ResourceManager.Resources" Version="1.11.2" />
<PackageReference Include="Azure.Security.KeyVault.Administration" Version="4.6.0" />
<PackageReference Include="Azure.Security.KeyVault.Certificates" Version="4.8.0" />
<PackageReference Include="Azure.Security.KeyVault.Secrets" Version="4.8.0" />
<PackageReference Include="Azure.Storage.Blobs" Version="12.27.0" />
<PackageReference Include="BouncyCastle.NetCore" Version="2.2.1" />
<PackageReference Include="Keyfactor.Logging" Version="1.1.2" />
<PackageReference Include="Keyfactor.Orchestrators.Common" Version="3.2.0" />
<PackageReference Include="Keyfactor.Logging" Version="1.3.0" />
<PackageReference Include="Keyfactor.Orchestrators.Common" Version="3.3.0" />
<PackageReference Include="Keyfactor.Orchestrators.IOrchestratorJobExtensions" Version="1.0.0" />
<PackageReference Include="Keyfactor.Platform.IPAMProvider" Version="1.0.0" />
<PackageReference Include="Microsoft.AspNet.WebApi.Client" Version="6.0.0" />
<PackageReference Include="Microsoft.Identity.Client" Version="4.68.0" />
<PackageReference Include="Microsoft.Identity.Client.Extensions.Msal" Version="4.68.0" />
<PackageReference Include="System.Drawing.Common" Version="9.0.2" />
<PackageReference Include="Microsoft.Identity.Client" Version="4.82.1" />
<PackageReference Include="Microsoft.Identity.Client.Extensions.Msal" Version="4.82.1" />
<PackageReference Include="Newtonsoft.Json.Bson" Version="1.0.3" />

Copilot AI Apr 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newtonsoft.Json.Bson is referenced in the project file but does not appear to be used anywhere in the codebase. Removing unused package references helps reduce build time, artifact size, and dependency surface area.

Suggested change
<PackageReference Include="Newtonsoft.Json.Bson" Version="1.0.3" />

Copilot uses AI. Check for mistakes.
<PackageReference Include="System.Drawing.Common" Version="10.0.3" />
<PackageReference Include="System.Linq" Version="4.3.0" />
<PackageReference Include="System.Linq.Async" Version="6.0.1" />
<PackageReference Include="System.Linq.Async" Version="7.0.0" />
</ItemGroup>

<ItemGroup>
Expand Down
1 change: 1 addition & 0 deletions AzureKeyVault/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ static class AzureKeyVaultConstants
static class EntryParameters {
public const string TAGS = "CertificateTags";
public const string PRESERVE_TAGS = "PreserveExistingTags";
public const string NON_EXPORTABLE = "NonExportable";
}

static class JobTypes
Expand Down
25 changes: 13 additions & 12 deletions AzureKeyVault/Jobs/Management.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
using Keyfactor.Orchestrators.Extensions.Interfaces;
using System.Collections.Generic;
using Newtonsoft.Json;
using System.Security.AccessControl;

namespace Keyfactor.Extensions.Orchestrator.AzureKeyVault
{
Expand Down Expand Up @@ -46,11 +45,13 @@ public JobResult ProcessJob(ManagementJobConfiguration config)

string tagsJSON;
bool preserveTags;
bool nonExportable;

logger.LogTrace("parsing entry parameters.. ");

tagsJSON = config.JobProperties[EntryParameters.TAGS] as string ?? string.Empty;
preserveTags = config.JobProperties[EntryParameters.PRESERVE_TAGS] as bool? ?? false;
preserveTags = config.JobProperties[EntryParameters.PRESERVE_TAGS] as bool? ?? true;

Copilot AI Apr 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PreserveExistingTags is documented/declared with a default of False (see integration-manifest and store-type scripts), but the runtime default here is true when the entry parameter is missing/null. This changes behavior (tags will be preserved even if the user didn't opt in). Consider defaulting to false and only enabling preservation when the entry parameter is explicitly set to true.

Suggested change
preserveTags = config.JobProperties[EntryParameters.PRESERVE_TAGS] as bool? ?? true;
preserveTags = config.JobProperties[EntryParameters.PRESERVE_TAGS] as bool? ?? false;

Copilot uses AI. Check for mistakes.
nonExportable = config.JobProperties[EntryParameters.NON_EXPORTABLE] as bool? ?? false;

switch (config.OperationType)
{
Expand All @@ -61,7 +62,7 @@ public JobResult ProcessJob(ManagementJobConfiguration config)
case CertStoreOperationType.Add:
logger.LogDebug($"Begin Management > Add...");

complete = PerformAddition(config.JobCertificate.Alias, config.JobCertificate.PrivateKeyPassword, config.JobCertificate.Contents, tagsJSON, config.JobHistoryId, config.Overwrite, preserveTags);
complete = PerformAddition(config.JobCertificate.Alias, config.JobCertificate.PrivateKeyPassword, config.JobCertificate.Contents, tagsJSON, config.JobHistoryId, config.Overwrite, preserveTags, nonExportable);
break;
case CertStoreOperationType.Remove:
logger.LogDebug($"Begin Management > Remove...");
Expand Down Expand Up @@ -103,7 +104,7 @@ protected async Task<JobResult> PerformCreateVault(long jobHistoryId)
#endregion

#region Add
protected virtual JobResult PerformAddition(string alias, string pfxPassword, string entryContents, string tagsJSON, long jobHistoryId, bool overwrite, bool preserveTags)
protected virtual JobResult PerformAddition(string alias, string pfxPassword, string entryContents, string tagsJSON, long jobHistoryId, bool overwrite, bool preserveTags, bool nonExportable)
{
var complete = new JobResult() { Result = OrchestratorJobStatusJobResult.Failure, JobHistoryId = jobHistoryId };

Expand Down Expand Up @@ -138,16 +139,16 @@ protected virtual JobResult PerformAddition(string alias, string pfxPassword, st
if (existing != null)
{
logger.LogTrace($"there is an existing cert..");
}

existingTags = existing?.Properties.Tags as Dictionary<string, string> ?? new Dictionary<string, string>();
existingTags = existing?.Properties.Tags as Dictionary<string, string> ?? new Dictionary<string, string>();

logger.LogTrace("existing cert tags: ");
if (!existingTags.Any()) logger.LogTrace("(none)");
logger.LogTrace("existing cert tags: ");
if (!existingTags.Any()) logger.LogTrace("(none)");

foreach (var tag in existingTags)
{
logger.LogTrace(tag.Key + " : " + tag.Value);
foreach (var tag in existingTags)
{
logger.LogTrace(tag.Key + " : " + tag.Value);
}
}

// if overwrite is unchecked, check for an existing cert first
Expand All @@ -173,7 +174,7 @@ protected virtual JobResult PerformAddition(string alias, string pfxPassword, st
}
}

var cert = AzClient.ImportCertificateAsync(alias, entryContents, pfxPassword, tagDict).Result;
var cert = AzClient.ImportCertificateAsync(alias, entryContents, pfxPassword, tagDict, nonExportable).Result;

// Ensure the return object has a AKV version tag, and Thumbprint
if (!string.IsNullOrEmpty(cert.Properties.Version) &&
Expand Down
14 changes: 12 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
- 3.2.2
- Updated screenshots in README
- Returning entry parameters along with inventory

- 3.2.1
- Documentation updates and improvements
- Updated NuGet packages

- 3.2.0
- Added an optional entry parameter to indicate whether the private key of the cert should be not exportable when stored in KeyVault
- Now specifying the pkcs12 format when wirting certs to Azure KeyVault. This should prevent the error when a PEM cert was added outside of Command and then we attempt to update without specifying the format (Azure assumes PEM and throws an error if not).

Comment on lines +8 to +11

Copilot AI Apr 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spelling: wirting should be writing. Also consider removing the stray tab-only line above to keep the changelog clean and consistent.

Copilot uses AI. Check for mistakes.
- 3.1.9
- Added optional entry parameter to indicate that existing tags should be preserved if certificate is replaced
- bug fix for government cloud host name resolution

- 3.1.8
- Fixed bug where enrollment would fail if the CertificateTags field was not defined as an entry parameter
Expand All @@ -11,7 +22,6 @@
- Added support for Azure KeyVault Certificate Metadata via Entry Parameters
- Fixed issue where an error would be returned during Inventory if 0 certificates were found
- Converted to BouncyCastle crypto libraries


- 3.1.6
- Preventing CertStore parameters from getting used if present but empty.
Expand Down
Loading
Loading