Return null key if access is denied (#13317)

* Return null key if access is denied

Fixes #11574. IKeyEncryptionResolver.Resolve is designed to delegate access verification to the resolver, so that any access issues are thrown downstream at the call site instead of upstream when trying to get the resolver/client.

* Resolve offline and PR feedback

sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/KeyResolver.cs

@@ -6,7 +6,6 @@
 using System.Threading.Tasks;
 using Azure.Core;
 using Azure.Core.Cryptography;
-using Azure.Core.Diagnostics;
 using Azure.Core.Pipeline;
 
 namespace Azure.Security.KeyVault.Keys.Cryptography
@@ -63,11 +62,11 @@ public KeyResolver(TokenCredential credential, CryptographyClientOptions options
         }
 
         /// <summary>
-        /// Retrieves a <see cref="CryptographyClient"/> capable of performing cryptographic operations with the key represented by the specfiied <paramref name="keyId"/>.
+        /// Retrieves a <see cref="CryptographyClient"/> capable of performing cryptographic operations with the key represented by the specified <paramref name="keyId"/>.
         /// </summary>
-        /// <param name="keyId">The key idenitifier of the key used by the created <see cref="CryptographyClient"/>.</param>
+        /// <param name="keyId">The key identifier of the key used by the created <see cref="CryptographyClient"/>.</param>
         /// <param name="cancellationToken">A <see cref="CancellationToken"/> controlling the request lifetime.</param>
-        /// <returns>A new <see cref="CryptographyClient"/> capable of performing cryptographic operations with the key represented by the specfiied <paramref name="keyId"/>.</returns>
+        /// <returns>A new <see cref="CryptographyClient"/> capable of performing cryptographic operations with the key represented by the specified <paramref name="keyId"/>.</returns>
         /// <exception cref="ArgumentNullException"><paramref name="keyId"/> is null.</exception>
         public virtual CryptographyClient Resolve(Uri keyId, CancellationToken cancellationToken = default)
         {
@@ -95,11 +94,11 @@ public virtual CryptographyClient Resolve(Uri keyId, CancellationToken cancellat
         }
 
         /// <summary>
-        /// Retrieves a <see cref="CryptographyClient"/> capable of performing cryptographic operations with the key represented by the specfiied <paramref name="keyId"/>.
+        /// Retrieves a <see cref="CryptographyClient"/> capable of performing cryptographic operations with the key represented by the specified <paramref name="keyId"/>.
         /// </summary>
-        /// <param name="keyId">The key idenitifier of the key used by the created <see cref="CryptographyClient"/>.</param>
+        /// <param name="keyId">The key identifier of the key used by the created <see cref="CryptographyClient"/>.</param>
         /// <param name="cancellationToken">A <see cref="CancellationToken"/> controlling the request lifetime.</param>
-        /// <returns>A new <see cref="CryptographyClient"/> capable of performing cryptographic operations with the key represented by the specfiied <paramref name="keyId"/>.</returns>
+        /// <returns>A new <see cref="CryptographyClient"/> capable of performing cryptographic operations with the key represented by the specified <paramref name="keyId"/>.</returns>
         /// <exception cref="ArgumentNullException"><paramref name="keyId"/> is null.</exception>
         public virtual async Task<CryptographyClient> ResolveAsync(Uri keyId, CancellationToken cancellationToken = default)
         {
@@ -168,6 +167,10 @@ private Response<T> ParseResponse<T>(Response response, T result)
                 case 204:
                     result.Deserialize(response.ContentStream);
                     return Response.FromValue(result, response);
+                case 403 when !(result is SecretKey):
+                    // The "get" permission may not be granted on a key, while other key permissions may be granted.
+                    // To use a key contained within a secret, the "get" permission is required to retrieve the key material.
+                    return Response.FromValue<T>(default, response);
                 default:
                     throw _clientDiagnostics.CreateRequestFailedException(response);
             }

sdk/keyvault/Azure.Security.KeyVault.Keys/tests/KeyResolverMockTests.cs

@@ -0,0 +1,68 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System;
+using System.Net;
+using System.Threading;
+using System.Threading.Tasks;
+using Azure.Core;
+using Azure.Core.Testing;
+using Azure.Security.KeyVault.Keys.Cryptography;
+using NUnit.Framework;
+
+namespace Azure.Security.KeyVault.Keys.Tests
+{
+    public class KeyResolverMockTests : ClientTestBase
+    {
+        public KeyResolverMockTests(bool isAsync) : base(isAsync)
+        {
+        }
+
+        [Test]
+        public async Task ShouldNotRequireGetPermissionForKey()
+        {
+            // Test for https://github.com/Azure/azure-sdk-for-net/issues/11574
+            MockTransport transport = new MockTransport(request => new MockResponse((int)HttpStatusCode.Forbidden, nameof(HttpStatusCode.Forbidden)));
+
+            KeyResolver resolver = GetResolver(transport);
+            CryptographyClient client = await resolver.ResolveAsync(new Uri("https://mock.vault.azure.net/keys/mock-key"));
+
+            RequestFailedException ex = Assert.ThrowsAsync<RequestFailedException>(() => client.UnwrapKeyAsync(KeyWrapAlgorithm.A256KW, new byte[] { 0, 1, 2, 3 }));
+            Assert.AreEqual((int)HttpStatusCode.Forbidden, ex.Status);
+        }
+
+        [Test]
+        public void ShouldRequireGetPermissionForSecret()
+        {
+            // Test for https://github.com/Azure/azure-sdk-for-net/issues/11574
+            MockTransport transport = new MockTransport(request => new MockResponse((int)HttpStatusCode.Forbidden, nameof(HttpStatusCode.Forbidden)));
+
+            KeyResolver resolver = GetResolver(transport);
+
+            RequestFailedException ex = Assert.ThrowsAsync<RequestFailedException>(() => resolver.ResolveAsync(new Uri("https://mock.vault.azure.net/secrets/mock-secret")));
+            Assert.AreEqual((int)HttpStatusCode.Forbidden, ex.Status);
+        }
+
+        protected KeyResolver GetResolver(MockTransport transport)
+        {
+            Assert.NotNull(transport);
+
+            CryptographyClientOptions options = new CryptographyClientOptions
+            {
+                Transport = transport,
+            };
+
+            return InstrumentClient(
+                new KeyResolver(new NullTokenCredential(), options));
+        }
+
+        private class NullTokenCredential : TokenCredential
+        {
+            public override AccessToken GetToken(TokenRequestContext requestContext, CancellationToken cancellationToken) =>
+                new AccessToken("invalid", DateTimeOffset.Now.AddHours(1));
+
+            public override ValueTask<AccessToken> GetTokenAsync(TokenRequestContext requestContext, CancellationToken cancellationToken) =>
+                new ValueTask<AccessToken>(GetToken(requestContext, cancellationToken));
+        }
+    }
+}