From 524d71712b5dd48426259be710e5d06356a981df Mon Sep 17 00:00:00 2001 From: Rahul Alapati Date: Mon, 4 May 2026 17:02:29 -0400 Subject: [PATCH 1/2] Fix New-AzKeyVault RequestDisallowedByPolicy by explicitly setting enableSoftDelete Explicitly set EnableSoftDelete=true in the request body for New-AzKeyVault to satisfy Azure Policy checks that require the property to be present. While soft delete is already enabled by default on the service side, Azure Policy checks may require the property to be explicitly present in the request body. Without it, New-AzKeyVault fails with a RequestDisallowedByPolicy error when such policies are enforced. This is a non-breaking fix - no new parameters are exposed and customer behavior is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../NewAzureKeyVaultSoftDeleteTests.cs | 97 +++++++++++++++++++ src/KeyVault/KeyVault/ChangeLog.md | 1 + .../Commands/KeyVault/NewAzureKeyVault.cs | 4 +- 3 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 src/KeyVault/KeyVault.Test/UnitTests/NewAzureKeyVaultSoftDeleteTests.cs diff --git a/src/KeyVault/KeyVault.Test/UnitTests/NewAzureKeyVaultSoftDeleteTests.cs b/src/KeyVault/KeyVault.Test/UnitTests/NewAzureKeyVaultSoftDeleteTests.cs new file mode 100644 index 000000000000..71832d1cd999 --- /dev/null +++ b/src/KeyVault/KeyVault.Test/UnitTests/NewAzureKeyVaultSoftDeleteTests.cs @@ -0,0 +1,97 @@ +// ---------------------------------------------------------------------------------- +// +// Copyright Microsoft Corporation +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ---------------------------------------------------------------------------------- + +using Microsoft.Azure.Commands.KeyVault.Models; +using Microsoft.Azure.Management.KeyVault; +using Microsoft.Azure.Management.KeyVault.Models; +using Microsoft.Rest.Azure; +using Microsoft.WindowsAzure.Commands.ScenarioTest; +using Moq; +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.Azure.Commands.KeyVault.Test.UnitTests +{ + /// + /// Verify that New-AzKeyVault explicitly sets EnableSoftDelete=true in the request body + /// so that Azure Policy checks requiring the property to be present are satisfied. + /// + public class NewAzureKeyVaultSoftDeleteTests : KeyVaultUnitTestBase + { + [Fact] + [Trait(Category.AcceptanceType, Category.CheckIn)] + public void CreateNewVault_Sets_EnableSoftDelete_True_In_VaultProperties() + { + // Arrange + VaultCreateOrUpdateParameters capturedParameters = null; + var mockVaultsOperations = new Mock(); + mockVaultsOperations + .Setup(v => v.CreateOrUpdateWithHttpMessagesAsync( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny>>(), + It.IsAny())) + .Callback>, CancellationToken>( + (rg, name, parameters, headers, ct) => capturedParameters = parameters) + .Returns(Task.FromResult(new AzureOperationResponse + { + Body = new Vault( + properties: new VaultProperties + { + TenantId = Guid.NewGuid(), + Sku = new Sku(SkuName.Standard), + AccessPolicies = new AccessPolicyEntry[] { }, + VaultUri = "https://testvault.vault.azure.net" + }, + name: "testvault", + location: "eastus") + })); + + var mockClient = new Mock(); + mockClient.Setup(c => c.Vaults).Returns(mockVaultsOperations.Object); + + var vaultManagementClient = new VaultManagementClient(); + // Use reflection to set the private KeyVaultManagementClient property + var prop = typeof(VaultManagementClient).GetProperty("KeyVaultManagementClient", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + prop.SetValue(vaultManagementClient, mockClient.Object); + + var createParameters = new VaultCreationOrUpdateParameters + { + Name = "testvault", + ResourceGroupName = "testrg", + Location = "eastus", + SkuFamilyName = "A", + SkuName = "Standard", + TenantId = Guid.NewGuid(), + EnableSoftDelete = true, + SoftDeleteRetentionInDays = 90, + NetworkAcls = new NetworkRuleSet() + }; + + // Act + vaultManagementClient.CreateNewVault(createParameters); + + // Assert + Assert.NotNull(capturedParameters); + Assert.True(capturedParameters.Properties.EnableSoftDelete, + "CreateNewVault must set EnableSoftDelete=true in the request body " + + "to satisfy Azure Policy checks"); + } + } +} diff --git a/src/KeyVault/KeyVault/ChangeLog.md b/src/KeyVault/KeyVault/ChangeLog.md index 3411677ed0f7..3abf98eb6a97 100644 --- a/src/KeyVault/KeyVault/ChangeLog.md +++ b/src/KeyVault/KeyVault/ChangeLog.md @@ -18,6 +18,7 @@ - Additional information about change #1 --> ## Upcoming Release +* Fixed `New-AzKeyVault` `RequestDisallowedByPolicy` error by explicitly setting `enableSoftDelete` in the request body to satisfy Azure Policy checks ## Version 6.4.3 * Added upcoming breaking change warning messages to `Get-AzKeyVaultKey` and `Get-AzKeyVaultSecret` for filtering certificate-backed keys and secrets. diff --git a/src/KeyVault/KeyVault/Commands/KeyVault/NewAzureKeyVault.cs b/src/KeyVault/KeyVault/Commands/KeyVault/NewAzureKeyVault.cs index e6c80edabe6a..ae27d84f1c21 100644 --- a/src/KeyVault/KeyVault/Commands/KeyVault/NewAzureKeyVault.cs +++ b/src/KeyVault/KeyVault/Commands/KeyVault/NewAzureKeyVault.cs @@ -166,7 +166,9 @@ public override void ExecuteCmdlet() EnabledForDeployment = this.EnabledForDeployment.IsPresent ? true : null as bool?, EnabledForTemplateDeployment = EnabledForTemplateDeployment.IsPresent ? true : null as bool?, EnabledForDiskEncryption = EnabledForDiskEncryption.IsPresent ? true : null as bool?, - EnableSoftDelete = null, + // Explicitly set to satisfy Azure Policy checks that require + // enableSoftDelete to be present in the request body. + EnableSoftDelete = true, EnablePurgeProtection = EnablePurgeProtection.IsPresent ? true : (bool?)null, // false is not accepted EnableRbacAuthorization = DisableRbacAuthorization.IsPresent ? false : true, /* From 2a7334231477a80923e6ede49128e1ad3e53a53f Mon Sep 17 00:00:00 2001 From: Rahul Alapati Date: Tue, 5 May 2026 12:04:21 -0400 Subject: [PATCH 2/2] [KeyVault] Add cmdlet-level unit test for EnableSoftDelete=true Add a proper regression test that exercises NewAzureKeyVault.ExecuteCmdlet() end-to-end and verifies that EnableSoftDelete=true is always present in the outgoing SDK request body. This ensures Azure Policy checks requiring the property to be present are satisfied. The previous test only verified the pass-through client layer with EnableSoftDelete already set to true in the input, so it would keep passing even if the cmdlet's hardcoded assignment regressed. New tests: - ExecuteCmdlet_Sets_EnableSoftDelete_True_In_Request: full cmdlet path test - VaultManagementClient_Does_Not_Default_EnableSoftDelete: proves client is a pure pass-through (null in = null out), establishing that only the cmdlet provides the value Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../NewAzureKeyVaultSoftDeleteTests.cs | 129 ++++++++++++++---- 1 file changed, 106 insertions(+), 23 deletions(-) diff --git a/src/KeyVault/KeyVault.Test/UnitTests/NewAzureKeyVaultSoftDeleteTests.cs b/src/KeyVault/KeyVault.Test/UnitTests/NewAzureKeyVaultSoftDeleteTests.cs index 71832d1cd999..20cc76d9f1bd 100644 --- a/src/KeyVault/KeyVault.Test/UnitTests/NewAzureKeyVaultSoftDeleteTests.cs +++ b/src/KeyVault/KeyVault.Test/UnitTests/NewAzureKeyVaultSoftDeleteTests.cs @@ -13,6 +13,7 @@ // ---------------------------------------------------------------------------------- using Microsoft.Azure.Commands.KeyVault.Models; +using Microsoft.Azure.Management.Internal.Resources; using Microsoft.Azure.Management.KeyVault; using Microsoft.Azure.Management.KeyVault.Models; using Microsoft.Rest.Azure; @@ -20,9 +21,14 @@ using Moq; using System; using System.Collections.Generic; +using System.Management.Automation; +using System.Net; +using System.Net.Http; +using System.Reflection; using System.Threading; using System.Threading.Tasks; using Xunit; +using KVSku = Microsoft.Azure.Management.KeyVault.Models.Sku; namespace Microsoft.Azure.Commands.KeyVault.Test.UnitTests { @@ -32,14 +38,32 @@ namespace Microsoft.Azure.Commands.KeyVault.Test.UnitTests /// public class NewAzureKeyVaultSoftDeleteTests : KeyVaultUnitTestBase { - [Fact] - [Trait(Category.AcceptanceType, Category.CheckIn)] - public void CreateNewVault_Sets_EnableSoftDelete_True_In_VaultProperties() + private VaultCreateOrUpdateParameters _capturedSdkParameters; + private VaultManagementClient _vaultManagementClient; + + /// + /// A fake HTTP handler that returns an empty resource list for any GET (FilterResources) + /// and prevents real network calls during unit tests. + /// + private class FakeHttpHandler : HttpClientHandler { - // Arrange - VaultCreateOrUpdateParameters capturedParameters = null; - var mockVaultsOperations = new Mock(); - mockVaultsOperations + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + var response = new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{\"value\":[]}") + }; + response.Content.Headers.ContentType = new System.Net.Http.Headers.MediaTypeHeaderValue("application/json"); + return Task.FromResult(response); + } + } + + private void SetupVaultManagementClient() + { + _capturedSdkParameters = null; + + var mockVaultsOps = new Mock(); + mockVaultsOps .Setup(v => v.CreateOrUpdateWithHttpMessagesAsync( It.IsAny(), It.IsAny(), @@ -47,14 +71,14 @@ public void CreateNewVault_Sets_EnableSoftDelete_True_In_VaultProperties() It.IsAny>>(), It.IsAny())) .Callback>, CancellationToken>( - (rg, name, parameters, headers, ct) => capturedParameters = parameters) + (rg, name, parameters, headers, ct) => _capturedSdkParameters = parameters) .Returns(Task.FromResult(new AzureOperationResponse { Body = new Vault( properties: new VaultProperties { TenantId = Guid.NewGuid(), - Sku = new Sku(SkuName.Standard), + Sku = new KVSku(SkuName.Standard), AccessPolicies = new AccessPolicyEntry[] { }, VaultUri = "https://testvault.vault.azure.net" }, @@ -62,14 +86,75 @@ public void CreateNewVault_Sets_EnableSoftDelete_True_In_VaultProperties() location: "eastus") })); - var mockClient = new Mock(); - mockClient.Setup(c => c.Vaults).Returns(mockVaultsOperations.Object); + var mockKvMgmtClient = new Mock(); + mockKvMgmtClient.Setup(c => c.Vaults).Returns(mockVaultsOps.Object); + + _vaultManagementClient = new VaultManagementClient(); + typeof(VaultManagementClient) + .GetProperty("KeyVaultManagementClient", BindingFlags.NonPublic | BindingFlags.Instance) + .SetValue(_vaultManagementClient, mockKvMgmtClient.Object); + } + + private ResourceManagementClient CreateFakeResourceClient() + { + var client = new ResourceManagementClient( + new Microsoft.Rest.TokenCredentials("fake-token"), + new FakeHttpHandler()); + client.SubscriptionId = "00000000-0000-0000-0000-000000000000"; + return client; + } + + /// + /// Exercises NewAzureKeyVault.ExecuteCmdlet() end-to-end and verifies that + /// the cmdlet hardcodes EnableSoftDelete=true in the outgoing request. + /// This test would FAIL if the assignment at NewAzureKeyVault.cs line 171 + /// were removed, because VaultManagementClient is a pure pass-through + /// (as proved by VaultManagementClient_Does_Not_Default_EnableSoftDelete). + /// + [Fact] + [Trait(Category.AcceptanceType, Category.CheckIn)] + public void ExecuteCmdlet_Sets_EnableSoftDelete_True_In_Request() + { + // Arrange + SetupVaultManagementClient(); + + var cmdRuntimeMock = new Mock(); + cmdRuntimeMock.Setup(cr => cr.ShouldProcess(It.IsAny(), It.IsAny())).Returns(true); - var vaultManagementClient = new VaultManagementClient(); - // Use reflection to set the private KeyVaultManagementClient property - var prop = typeof(VaultManagementClient).GetProperty("KeyVaultManagementClient", - System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); - prop.SetValue(vaultManagementClient, mockClient.Object); + var cmdlet = new NewAzureKeyVault + { + CommandRuntime = cmdRuntimeMock.Object, + Name = "test-vault", + ResourceGroupName = "test-rg", + Location = "eastus" + }; + cmdlet.KeyVaultManagementClient = _vaultManagementClient; + cmdlet.ResourceClient = CreateFakeResourceClient(); + + // Act + cmdlet.ExecuteCmdlet(); + + // Assert + Assert.NotNull(_capturedSdkParameters); + Assert.True(_capturedSdkParameters.Properties.EnableSoftDelete, + "NewAzureKeyVault.ExecuteCmdlet() must set EnableSoftDelete=true in the request body " + + "so that Azure Policy checks requiring the property to be present are satisfied."); + } + + /// + /// Proves VaultManagementClient.CreateNewVault is a pure pass-through: + /// it does NOT inject a default for EnableSoftDelete. + /// Combined with ExecuteCmdlet_Sets_EnableSoftDelete_True_In_Request, this proves + /// that only the cmdlet's hardcoded assignment provides the value. + /// If someone removes EnableSoftDelete=true from NewAzureKeyVault.cs, + /// the request would go out with EnableSoftDelete=null and Azure Policy would reject it. + /// + [Fact] + [Trait(Category.AcceptanceType, Category.CheckIn)] + public void VaultManagementClient_Does_Not_Default_EnableSoftDelete() + { + // Arrange + SetupVaultManagementClient(); var createParameters = new VaultCreationOrUpdateParameters { @@ -79,19 +164,17 @@ public void CreateNewVault_Sets_EnableSoftDelete_True_In_VaultProperties() SkuFamilyName = "A", SkuName = "Standard", TenantId = Guid.NewGuid(), - EnableSoftDelete = true, + EnableSoftDelete = null, // Deliberately not set SoftDeleteRetentionInDays = 90, NetworkAcls = new NetworkRuleSet() }; // Act - vaultManagementClient.CreateNewVault(createParameters); + _vaultManagementClient.CreateNewVault(createParameters); - // Assert - Assert.NotNull(capturedParameters); - Assert.True(capturedParameters.Properties.EnableSoftDelete, - "CreateNewVault must set EnableSoftDelete=true in the request body " + - "to satisfy Azure Policy checks"); + // Assert - null in means null out; client does NOT inject a default + Assert.NotNull(_capturedSdkParameters); + Assert.Null(_capturedSdkParameters.Properties.EnableSoftDelete); } } }