Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
28 changes: 25 additions & 3 deletions AzureKeyVault/AzureClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,17 @@ public virtual async Task<KeyVaultResource> CreateVault()

logger.LogTrace($"getting subscription info for provided subscription id {VaultProperties.SubscriptionId}");

SubscriptionResource subscription = KvManagementClient.GetSubscriptionResource(SubscriptionResource.CreateResourceIdentifier(VaultProperties.SubscriptionId));
ResourceGroupResource resourceGroup = subscription.GetResourceGroup(VaultProperties.ResourceGroupName);
var subscription = KvManagementClient.GetSubscriptionResource(SubscriptionResource.CreateResourceIdentifier(VaultProperties.SubscriptionId));

var resourceGroups = subscription.GetResourceGroups();
ResourceGroupResource resourceGroup = await resourceGroups.GetAsync(VaultProperties.ResourceGroupName);
logger.LogTrace("calling getAsync on resourcegroup...");
await resourceGroup.GetAsync();
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

Unnecessary API call: await resourceGroup.GetAsync() is called but its result is not used. The previous line already calls GetAsync() to retrieve the resource group. This redundant call should be removed to avoid unnecessary network overhead.

Suggested change
await resourceGroup.GetAsync();

Copilot uses AI. Check for mistakes.
logger.LogTrace("completed getAsync on resource group...");

var s = resourceGroup.HasData.ToString();

logger.LogTrace($"resource group has data?: {s}");

Comment on lines +165 to 172
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

This appears to be debug code that should be removed. The variable s is created but never used, and the logging statements around GetAsync() seem excessive for production code. Consider removing lines 165-171 or clarifying if this debug logging is intentional.

Suggested change
logger.LogTrace("calling getAsync on resourcegroup...");
await resourceGroup.GetAsync();
logger.LogTrace("completed getAsync on resource group...");
var s = resourceGroup.HasData.ToString();
logger.LogTrace($"resource group has data?: {s}");

Copilot uses AI. Check for mistakes.
AzureLocation loc;

Expand All @@ -170,7 +179,7 @@ public virtual async Task<KeyVaultResource> CreateVault()
{
logger.LogTrace($"no Vault Region location specified for new Vault, Getting available regions for resource group {resourceGroup.Data.Name}.");
var locOptions = await resourceGroup.GetAvailableLocationsAsync();
logger.LogTrace($"got location options for subscription {subscription.Data.SubscriptionId}", locOptions);
logger.LogTrace($"got location options for subscription {VaultProperties.SubscriptionId}", locOptions);
loc = locOptions.Value.FirstOrDefault();
}
catch (Exception ex)
Expand Down Expand Up @@ -276,9 +285,22 @@ public virtual async Task<IEnumerable<CurrentInventoryItem>> GetCertificatesAsyn

logger.LogTrace($"got a pageable response");
}
catch (AuthenticationFailedException ex)
{
logger.LogError($"Authentication failed: {ex.Message}");
logger.LogError(LogHandler.FlattenException(ex));
throw;
}
catch (RequestFailedException ex) // Catch other potential Azure-specific errors
{
logger.LogError($"Azure Key Vault operation failed: {ex.Status} - {ex.Message}");
logger.LogError(LogHandler.FlattenException(ex));
throw;
}
catch (Exception ex)
{
logger.LogError($"Error performing inventory. {ex.Message}", ex);
logger.LogError(LogHandler.FlattenException(ex));
throw;
}

Expand Down
2 changes: 1 addition & 1 deletion AzureKeyVault/Jobs/AzureKeyVaultJob.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void InitializeStore(dynamic config)
// ClientSecret can be omitted for managed identities, required for service principal auth
VaultProperties.ClientSecret = PAMUtilities.ResolvePAMField(PamSecretResolver, logger, "Server Password", config.ServerPassword);

if (VaultProperties.ClientSecret == null)
if (string.IsNullOrEmpty(VaultProperties.ClientSecret))
{
logger.LogTrace("No client secret provided, assuming Managed Identity authentication");
}
Expand Down
18 changes: 10 additions & 8 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 All @@ -42,15 +41,18 @@ public JobResult ProcessJob(ManagementJobConfiguration config)
{
Result = OrchestratorJobStatusJobResult.Failure,
FailureMessage = "Invalid Management Operation"
};

string tagsJSON;
bool preserveTags;
};

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

tagsJSON = config.JobProperties[EntryParameters.TAGS] as string ?? string.Empty;
preserveTags = config.JobProperties[EntryParameters.PRESERVE_TAGS] as bool? ?? false;
string tagsJSON = string.Empty;
bool preserveTags = false;
if (config.JobProperties != null)
{
config.JobProperties.TryGetValue(EntryParameters.TAGS, out object tagsJSONObj);
config.JobProperties.TryGetValue(EntryParameters.PRESERVE_TAGS, out object preserveTagsObj);
tagsJSON = tagsJSONObj == null ? string.Empty : tagsJSONObj.ToString();
preserveTags = preserveTagsObj == null ? false : Boolean.Parse(preserveTagsObj.ToString());
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

The expression 'A ? false : B' can be simplified to '!A && B'.

Suggested change
preserveTags = preserveTagsObj == null ? false : Boolean.Parse(preserveTagsObj.ToString());
preserveTags = preserveTagsObj != null && Boolean.Parse(preserveTagsObj.ToString());

Copilot uses AI. Check for mistakes.
}

switch (config.OperationType)
{
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
- 3.1.11
- bug fix for error when creating new Azure Keyvaults
- documentation updates

Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

Inconsistent whitespace: this line uses spaces for indentation while the rest of the file likely uses tabs (based on the CHANGELOG file structure). This should use tabs to be consistent with the formatting of line 5.

Suggested change

Copilot uses AI. Check for mistakes.
- 3.1.10
- bug fix for government cloud host name resolution

Expand Down
11 changes: 8 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ The high level steps required to configure the Azure Keyvault Orchestrator exten

1) [Configure the Azure Keyvault for client access](#configure-the-azure-keyvault-for-client-access)

1) [Create the Store Type in Keyfactor](#create-the-akv-certificate-store-type)
1) [Create the Store Type in Keyfactor](#akv-certificate-store-type)

1) [Install the Extension on the Orchestrator](#installation)

Expand Down Expand Up @@ -544,7 +544,7 @@ To use the Azure Key Vault Universal Orchestrator extension, you **must** create


The Azure Keyvault Certificate Store Type is designed to integrate with Microsoft Azure Key Vault, enabling users to
manage and automate the lifecycle of cryptographic certificates stored in Azure Key Vault through Keyfactor Command.
manage and automate the lifecycle of cryptographic certificates stored in Azure Keyvault through Keyfactor Command.
This Certificate Store Type represents the connection and configuration necessary to interact with specific instances of
Azure Key Vault, allowing for operations such as inventory, addition, removal, and discovery of certificates and
certificate stores.
Expand All @@ -565,6 +565,11 @@ However, ensuring that the orchestrator has network access to Azure endpoints is
mindful of these caveats and limitations will help ensure successful deployment and use of the Azure Keyvault
Certificate Store Type within your organization’s security framework.

> :warning:
> The alias you provide when enrolling a certificate will be used as the certificate name in Azure Keyvault.
> Consequently; [it must _only_ contain alphanumeric characters and hyphens](https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules#microsoftkeyvault).
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

Inconsistent punctuation: A semicolon is used here instead of a comma. Change "Consequently;" to "Consequently," for proper grammar.

Suggested change
> Consequently; [it must _only_ contain alphanumeric characters and hyphens](https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules#microsoftkeyvault).
> Consequently, [it must _only_ contain alphanumeric characters and hyphens](https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules#microsoftkeyvault).

Copilot uses AI. Check for mistakes.
> If you encounter the error "The request URI contains an invalid name" when attempting to perform an enrollment, it is likely due to the use of disallowed characters in the alias.




Expand Down Expand Up @@ -633,7 +638,7 @@ the Keyfactor Command Portal
##### Advanced Tab
| Attribute | Value | Description |
| --------- | ----- | ----- |
| Supports Custom Alias | Optional | Determines if an individual entry within a store can have a custom Alias. |
| Supports Custom Alias | Required | Determines if an individual entry within a store can have a custom Alias. |
| Private Key Handling | Optional | This determines if Keyfactor can send the private key associated with a certificate to the store. Required because IIS certificates without private keys would be invalid. |
| PFX Password Style | Default | 'Default' - PFX password is randomly generated, 'Custom' - PFX password may be specified when the enrollment job is created (Requires the Allow Custom Password application setting to be enabled.) |

Expand Down
7 changes: 6 additions & 1 deletion docsource/akv.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
## Overview

The Azure Keyvault Certificate Store Type is designed to integrate with Microsoft Azure Key Vault, enabling users to
manage and automate the lifecycle of cryptographic certificates stored in Azure Key Vault through Keyfactor Command.
manage and automate the lifecycle of cryptographic certificates stored in Azure Keyvault through Keyfactor Command.
This Certificate Store Type represents the connection and configuration necessary to interact with specific instances of
Azure Key Vault, allowing for operations such as inventory, addition, removal, and discovery of certificates and
certificate stores.
Expand All @@ -22,3 +22,8 @@ However, ensuring that the orchestrator has network access to Azure endpoints is
mindful of these caveats and limitations will help ensure successful deployment and use of the Azure Keyvault
Certificate Store Type within your organization’s security framework.

> :warning:
> The alias you provide when enrolling a certificate will be used as the certificate name in Azure Keyvault.
> Consequently; [it must _only_ contain alphanumeric characters and hyphens](https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules#microsoftkeyvault).
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

Inconsistent punctuation: A semicolon is used here instead of a comma. Change "Consequently;" to "Consequently," for proper grammar.

Suggested change
> Consequently; [it must _only_ contain alphanumeric characters and hyphens](https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules#microsoftkeyvault).
> Consequently, [it must _only_ contain alphanumeric characters and hyphens](https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules#microsoftkeyvault).

Copilot uses AI. Check for mistakes.
> If you encounter the error "The request URI contains an invalid name" when attempting to perform an enrollment, it is likely due to the use of disallowed characters in the alias.

2 changes: 1 addition & 1 deletion docsource/content.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ The high level steps required to configure the Azure Keyvault Orchestrator exten

1) [Configure the Azure Keyvault for client access](#configure-the-azure-keyvault-for-client-access)

1) [Create the Store Type in Keyfactor](#create-the-akv-certificate-store-type)
1) [Create the Store Type in Keyfactor](#akv-certificate-store-type)

1) [Install the Extension on the Orchestrator](#installation)

Expand Down
2 changes: 1 addition & 1 deletion integration-manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"BlueprintAllowed": false,
"Capability": "AKV",
"ClientMachineDescription": "The GUID of the tenant ID of the Azure Keyvault instance; for example, '12345678-1234-1234-1234-123456789abc'.",
"CustomAliasAllowed": "Optional",
"CustomAliasAllowed": "Required",
"EntryParameters": [
{
"Name": "CertificateTags",
Expand Down
Loading