From 434e3688276082c729f8c4a452e4156032f819c9 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Mon, 29 Jul 2019 09:39:09 -0700 Subject: [PATCH 1/2] Add anonymous auth for ASP.NET Core integration --- .../src/AzureClientFactoryBuilder.cs | 4 +++- .../src/Internal/ClientInformation.cs | 7 +++++++ .../Internal/ConfigurationClientFactory.cs | 6 ++++++ .../tests/AzureClientFactoryTests.cs | 11 ++++++++++ .../tests/ClientFactoryTests.cs | 12 +++++++++++ .../tests/TestClientWithCredentials.cs | 5 +++++ ...AzureClientFactoryBuilderWithCredential.cs | 2 +- .../src/AzureClientBuilderExtensions.cs | 6 ++++-- .../src/AzureClientBuilderExtensions.cs | 6 ++++-- .../src/FileServiceClient.cs | 20 +++++++++++++++++++ .../src/AzureClientBuilderExtensions.cs | 6 ++++-- 11 files changed, 77 insertions(+), 8 deletions(-) diff --git a/sdk/core/Azure.Core.Extensions/src/AzureClientFactoryBuilder.cs b/sdk/core/Azure.Core.Extensions/src/AzureClientFactoryBuilder.cs index f19013c15ca9f..53d80004da173 100644 --- a/sdk/core/Azure.Core.Extensions/src/AzureClientFactoryBuilder.cs +++ b/sdk/core/Azure.Core.Extensions/src/AzureClientFactoryBuilder.cs @@ -70,9 +70,11 @@ public AzureClientFactoryBuilder ConfigureDefaults(IConfiguration configuration) return this; } - IAzureClientBuilder IAzureClientFactoryBuilderWithCredential.RegisterClientFactory(Func clientFactory) + IAzureClientBuilder IAzureClientFactoryBuilderWithCredential.RegisterClientFactory(Func clientFactory, bool supportsAnonymous) { var clientRegistration = new ClientRegistration(DefaultClientName, clientFactory); + clientRegistration.RequiresTokenCredential = !supportsAnonymous; + _serviceCollection.AddSingleton(clientRegistration); _serviceCollection.TryAddSingleton(typeof(IConfigureOptions>), typeof(DefaultCredentialClientOptionsSetup)); diff --git a/sdk/core/Azure.Core.Extensions/src/Internal/ClientInformation.cs b/sdk/core/Azure.Core.Extensions/src/Internal/ClientInformation.cs index 2e5f77125e10f..f5c7113d5c8f4 100644 --- a/sdk/core/Azure.Core.Extensions/src/Internal/ClientInformation.cs +++ b/sdk/core/Azure.Core.Extensions/src/Internal/ClientInformation.cs @@ -10,6 +10,7 @@ internal class ClientRegistration { public string Name { get; set; } public object Version { get; set; } + public bool RequiresTokenCredential { get; set; } private readonly Func _factory; @@ -43,6 +44,12 @@ public TClient GetClient(TOptions options, TokenCredential tokenCredential) return _cachedClient; } + + if (RequiresTokenCredential && tokenCredential == null) + { + throw new InvalidOperationException("Client registration requires a token credential. Configure it using UseCredential method."); + } + try { _cachedClient = _factory(options, tokenCredential); diff --git a/sdk/core/Azure.Core.Extensions/src/Internal/ConfigurationClientFactory.cs b/sdk/core/Azure.Core.Extensions/src/Internal/ConfigurationClientFactory.cs index 13cf599d46f4d..065d139857886 100644 --- a/sdk/core/Azure.Core.Extensions/src/Internal/ConfigurationClientFactory.cs +++ b/sdk/core/Azure.Core.Extensions/src/Internal/ConfigurationClientFactory.cs @@ -31,6 +31,12 @@ public static object CreateClient(Type clientType, Type optionsType, object opti { if (IsCredentialParameter(parameter)) { + if (credential == null) + { + match = false; + break; + } + arguments.Add(credential); continue; } diff --git a/sdk/core/Azure.Core.Extensions/tests/AzureClientFactoryTests.cs b/sdk/core/Azure.Core.Extensions/tests/AzureClientFactoryTests.cs index fd0c680eb3218..63232c5d9abb9 100644 --- a/sdk/core/Azure.Core.Extensions/tests/AzureClientFactoryTests.cs +++ b/sdk/core/Azure.Core.Extensions/tests/AzureClientFactoryTests.cs @@ -319,6 +319,17 @@ public void SupportsSettingVersion() Assert.AreEqual(TestClientOptions.ServiceVersion.B, client.Options.Version); } + [Test] + public void ThrowsIfCredentialIsNullButIsRequired() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddAzureClients(builder => builder.AddTestClient(new Uri("http://localhost/")).WithCredential((TokenCredential)null)); + + ServiceProvider provider = serviceCollection.BuildServiceProvider(); + InvalidOperationException exception = Assert.Throws(() => provider.GetService()); + Assert.AreEqual("Client registration requires a token credential. Configure it using UseCredential method.", exception.Message); + } + private IConfiguration GetConfiguration(params KeyValuePair[] items) { return new ConfigurationBuilder().AddInMemoryCollection(items).Build(); diff --git a/sdk/core/Azure.Core.Extensions/tests/ClientFactoryTests.cs b/sdk/core/Azure.Core.Extensions/tests/ClientFactoryTests.cs index ca41e1524b7ba..8e775c87f98b3 100644 --- a/sdk/core/Azure.Core.Extensions/tests/ClientFactoryTests.cs +++ b/sdk/core/Azure.Core.Extensions/tests/ClientFactoryTests.cs @@ -153,6 +153,18 @@ public void CreatesClientSecretCredentials() Assert.AreEqual("ConfigurationTenantId", clientSecretCredential.TenantId); } + [Test] + public void IgnoresConstructorWhenCredentialsNull() + { + IConfiguration configuration = GetConfiguration(new KeyValuePair("uri", "http://localhost")); + + var clientOptions = new TestClientOptions(); + var client = (TestClientWithCredentials)ClientFactory.CreateClient(typeof(TestClientWithCredentials), typeof(TestClientOptions), clientOptions, configuration, null); + + Assert.AreEqual("http://localhost/", client.Uri.ToString()); + Assert.AreSame(clientOptions, client.Options); + } + private IConfiguration GetConfiguration(params KeyValuePair[] items) { return new ConfigurationBuilder().AddInMemoryCollection(items).Build(); diff --git a/sdk/core/Azure.Core.Extensions/tests/TestClientWithCredentials.cs b/sdk/core/Azure.Core.Extensions/tests/TestClientWithCredentials.cs index 413b355d0ac3d..ce329617c6ba8 100644 --- a/sdk/core/Azure.Core.Extensions/tests/TestClientWithCredentials.cs +++ b/sdk/core/Azure.Core.Extensions/tests/TestClientWithCredentials.cs @@ -9,8 +9,13 @@ internal class TestClientWithCredentials : TestClient { public TokenCredential Credential { get; } + public TestClientWithCredentials(Uri uri, TestClientOptions options) : base(uri, options) + { + } + public TestClientWithCredentials(Uri uri, TokenCredential credential, TestClientOptions options) : base(uri, options) { + if (credential == null) throw new ArgumentNullException(nameof(credential)); Credential = credential; } } diff --git a/sdk/core/Azure.Core/src/Extensions/IAzureClientFactoryBuilderWithCredential.cs b/sdk/core/Azure.Core/src/Extensions/IAzureClientFactoryBuilderWithCredential.cs index bb6088898fc40..9667a43f4b8ee 100644 --- a/sdk/core/Azure.Core/src/Extensions/IAzureClientFactoryBuilderWithCredential.cs +++ b/sdk/core/Azure.Core/src/Extensions/IAzureClientFactoryBuilderWithCredential.cs @@ -7,6 +7,6 @@ namespace Azure.Core.Extensions { public interface IAzureClientFactoryBuilderWithCredential { - IAzureClientBuilder RegisterClientFactory(Func clientFactory) where TOptions: class; + IAzureClientBuilder RegisterClientFactory(Func clientFactory, bool supportsAnonymous = false) where TOptions: class; } } diff --git a/sdk/storage/Azure.Storage.Blobs/src/AzureClientBuilderExtensions.cs b/sdk/storage/Azure.Storage.Blobs/src/AzureClientBuilderExtensions.cs index 8b934c6d850a7..fae10a9273b62 100644 --- a/sdk/storage/Azure.Storage.Blobs/src/AzureClientBuilderExtensions.cs +++ b/sdk/storage/Azure.Storage.Blobs/src/AzureClientBuilderExtensions.cs @@ -24,9 +24,11 @@ public static IAzureClientBuilder AddBlobS /// Registers a instance with the provided /// public static IAzureClientBuilder AddBlobServiceClient(this TBuilder builder, Uri serviceUri) - where TBuilder: IAzureClientFactoryBuilder + where TBuilder: IAzureClientFactoryBuilderWithCredential { - return builder.RegisterClientFactory(options => new BlobServiceClient(serviceUri, options)); + return builder.RegisterClientFactory( + (options, token) => token != null ? new BlobServiceClient(serviceUri, token, options) : new BlobServiceClient(serviceUri, options), + supportsAnonymous: true); } /// diff --git a/sdk/storage/Azure.Storage.Files/src/AzureClientBuilderExtensions.cs b/sdk/storage/Azure.Storage.Files/src/AzureClientBuilderExtensions.cs index d55e5ecbb73d2..60a55eb707d88 100644 --- a/sdk/storage/Azure.Storage.Files/src/AzureClientBuilderExtensions.cs +++ b/sdk/storage/Azure.Storage.Files/src/AzureClientBuilderExtensions.cs @@ -25,9 +25,11 @@ public static IAzureClientBuilder AddFileS /// Registers a instance with the provided /// public static IAzureClientBuilder AddFileServiceClient(this TBuilder builder, Uri serviceUri) - where TBuilder: IAzureClientFactoryBuilder + where TBuilder: IAzureClientFactoryBuilderWithCredential { - return builder.RegisterClientFactory(options => new FileServiceClient(serviceUri, options)); + return builder.RegisterClientFactory( + (options, token) => new FileServiceClient(serviceUri, options), + supportsAnonymous: true); } /// diff --git a/sdk/storage/Azure.Storage.Files/src/FileServiceClient.cs b/sdk/storage/Azure.Storage.Files/src/FileServiceClient.cs index 1be34600ee3c7..f83ecb8562bc6 100644 --- a/sdk/storage/Azure.Storage.Files/src/FileServiceClient.cs +++ b/sdk/storage/Azure.Storage.Files/src/FileServiceClient.cs @@ -128,6 +128,26 @@ public FileServiceClient(Uri serviceUri, StorageSharedKeyCredential credential, { } + /// + /// Initializes a new instance of the + /// class. + /// + /// + /// A referencing the file service. + /// + /// + /// The shared key credential used to sign requests. + /// + /// + /// Optional client options that define the transport pipeline + /// policies for authentication, retries, etc., that are applied to + /// every request. + /// + public FileServiceClient(Uri serviceUri, TokenCredential credential, FileClientOptions options = default) + : this(serviceUri, credential.AsPolicy(), options) + { + } + /// /// Initializes a new instance of the /// class. diff --git a/sdk/storage/Azure.Storage.Queues/src/AzureClientBuilderExtensions.cs b/sdk/storage/Azure.Storage.Queues/src/AzureClientBuilderExtensions.cs index 159636186895a..431636f3a747f 100644 --- a/sdk/storage/Azure.Storage.Queues/src/AzureClientBuilderExtensions.cs +++ b/sdk/storage/Azure.Storage.Queues/src/AzureClientBuilderExtensions.cs @@ -25,9 +25,11 @@ public static IAzureClientBuilder AddQue /// Registers a instance with the provided /// public static IAzureClientBuilder AddQueueServiceClient(this TBuilder builder, Uri serviceUri) - where TBuilder: IAzureClientFactoryBuilder + where TBuilder: IAzureClientFactoryBuilderWithCredential { - return builder.RegisterClientFactory(options => new QueueServiceClient(serviceUri, options)); + return builder.RegisterClientFactory( + (options, token) => token != null ? new QueueServiceClient(serviceUri, token, options) : new QueueServiceClient(serviceUri, options), + supportsAnonymous: true); } /// From acdbfb54b5b64894d01dfac089307f0ca425b2a0 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Mon, 29 Jul 2019 15:34:07 -0700 Subject: [PATCH 2/2] fb --- .../src/AzureClientFactoryBuilder.cs | 4 ++-- .../src/Internal/ClientInformation.cs | 2 +- .../tests/AzureClientFactoryTests.cs | 2 +- ...AzureClientFactoryBuilderWithCredential.cs | 2 +- .../src/AzureClientBuilderExtensions.cs | 2 +- .../src/AzureClientBuilderExtensions.cs | 6 ++---- .../src/FileServiceClient.cs | 20 ------------------- .../src/AzureClientBuilderExtensions.cs | 2 +- 8 files changed, 9 insertions(+), 31 deletions(-) diff --git a/sdk/core/Azure.Core.Extensions/src/AzureClientFactoryBuilder.cs b/sdk/core/Azure.Core.Extensions/src/AzureClientFactoryBuilder.cs index 53d80004da173..2e80683f53e50 100644 --- a/sdk/core/Azure.Core.Extensions/src/AzureClientFactoryBuilder.cs +++ b/sdk/core/Azure.Core.Extensions/src/AzureClientFactoryBuilder.cs @@ -70,10 +70,10 @@ public AzureClientFactoryBuilder ConfigureDefaults(IConfiguration configuration) return this; } - IAzureClientBuilder IAzureClientFactoryBuilderWithCredential.RegisterClientFactory(Func clientFactory, bool supportsAnonymous) + IAzureClientBuilder IAzureClientFactoryBuilderWithCredential.RegisterClientFactory(Func clientFactory, bool requiresCredential) { var clientRegistration = new ClientRegistration(DefaultClientName, clientFactory); - clientRegistration.RequiresTokenCredential = !supportsAnonymous; + clientRegistration.RequiresTokenCredential = requiresCredential; _serviceCollection.AddSingleton(clientRegistration); diff --git a/sdk/core/Azure.Core.Extensions/src/Internal/ClientInformation.cs b/sdk/core/Azure.Core.Extensions/src/Internal/ClientInformation.cs index f5c7113d5c8f4..0fb2b39fecf7f 100644 --- a/sdk/core/Azure.Core.Extensions/src/Internal/ClientInformation.cs +++ b/sdk/core/Azure.Core.Extensions/src/Internal/ClientInformation.cs @@ -47,7 +47,7 @@ public TClient GetClient(TOptions options, TokenCredential tokenCredential) if (RequiresTokenCredential && tokenCredential == null) { - throw new InvalidOperationException("Client registration requires a token credential. Configure it using UseCredential method."); + throw new InvalidOperationException("Client registration requires a TokenCredential. Configure it using UseCredential method."); } try diff --git a/sdk/core/Azure.Core.Extensions/tests/AzureClientFactoryTests.cs b/sdk/core/Azure.Core.Extensions/tests/AzureClientFactoryTests.cs index 63232c5d9abb9..69ea516e8381b 100644 --- a/sdk/core/Azure.Core.Extensions/tests/AzureClientFactoryTests.cs +++ b/sdk/core/Azure.Core.Extensions/tests/AzureClientFactoryTests.cs @@ -327,7 +327,7 @@ public void ThrowsIfCredentialIsNullButIsRequired() ServiceProvider provider = serviceCollection.BuildServiceProvider(); InvalidOperationException exception = Assert.Throws(() => provider.GetService()); - Assert.AreEqual("Client registration requires a token credential. Configure it using UseCredential method.", exception.Message); + Assert.AreEqual("Client registration requires a TokenCredential. Configure it using UseCredential method.", exception.Message); } private IConfiguration GetConfiguration(params KeyValuePair[] items) diff --git a/sdk/core/Azure.Core/src/Extensions/IAzureClientFactoryBuilderWithCredential.cs b/sdk/core/Azure.Core/src/Extensions/IAzureClientFactoryBuilderWithCredential.cs index 9667a43f4b8ee..ac995688a66d7 100644 --- a/sdk/core/Azure.Core/src/Extensions/IAzureClientFactoryBuilderWithCredential.cs +++ b/sdk/core/Azure.Core/src/Extensions/IAzureClientFactoryBuilderWithCredential.cs @@ -7,6 +7,6 @@ namespace Azure.Core.Extensions { public interface IAzureClientFactoryBuilderWithCredential { - IAzureClientBuilder RegisterClientFactory(Func clientFactory, bool supportsAnonymous = false) where TOptions: class; + IAzureClientBuilder RegisterClientFactory(Func clientFactory, bool requiresCredential = true) where TOptions: class; } } diff --git a/sdk/storage/Azure.Storage.Blobs/src/AzureClientBuilderExtensions.cs b/sdk/storage/Azure.Storage.Blobs/src/AzureClientBuilderExtensions.cs index fae10a9273b62..1da0f377fe2c1 100644 --- a/sdk/storage/Azure.Storage.Blobs/src/AzureClientBuilderExtensions.cs +++ b/sdk/storage/Azure.Storage.Blobs/src/AzureClientBuilderExtensions.cs @@ -28,7 +28,7 @@ public static IAzureClientBuilder AddBlobS { return builder.RegisterClientFactory( (options, token) => token != null ? new BlobServiceClient(serviceUri, token, options) : new BlobServiceClient(serviceUri, options), - supportsAnonymous: true); + requiresCredential: false); } /// diff --git a/sdk/storage/Azure.Storage.Files/src/AzureClientBuilderExtensions.cs b/sdk/storage/Azure.Storage.Files/src/AzureClientBuilderExtensions.cs index 60a55eb707d88..d55e5ecbb73d2 100644 --- a/sdk/storage/Azure.Storage.Files/src/AzureClientBuilderExtensions.cs +++ b/sdk/storage/Azure.Storage.Files/src/AzureClientBuilderExtensions.cs @@ -25,11 +25,9 @@ public static IAzureClientBuilder AddFileS /// Registers a instance with the provided /// public static IAzureClientBuilder AddFileServiceClient(this TBuilder builder, Uri serviceUri) - where TBuilder: IAzureClientFactoryBuilderWithCredential + where TBuilder: IAzureClientFactoryBuilder { - return builder.RegisterClientFactory( - (options, token) => new FileServiceClient(serviceUri, options), - supportsAnonymous: true); + return builder.RegisterClientFactory(options => new FileServiceClient(serviceUri, options)); } /// diff --git a/sdk/storage/Azure.Storage.Files/src/FileServiceClient.cs b/sdk/storage/Azure.Storage.Files/src/FileServiceClient.cs index f83ecb8562bc6..1be34600ee3c7 100644 --- a/sdk/storage/Azure.Storage.Files/src/FileServiceClient.cs +++ b/sdk/storage/Azure.Storage.Files/src/FileServiceClient.cs @@ -128,26 +128,6 @@ public FileServiceClient(Uri serviceUri, StorageSharedKeyCredential credential, { } - /// - /// Initializes a new instance of the - /// class. - /// - /// - /// A referencing the file service. - /// - /// - /// The shared key credential used to sign requests. - /// - /// - /// Optional client options that define the transport pipeline - /// policies for authentication, retries, etc., that are applied to - /// every request. - /// - public FileServiceClient(Uri serviceUri, TokenCredential credential, FileClientOptions options = default) - : this(serviceUri, credential.AsPolicy(), options) - { - } - /// /// Initializes a new instance of the /// class. diff --git a/sdk/storage/Azure.Storage.Queues/src/AzureClientBuilderExtensions.cs b/sdk/storage/Azure.Storage.Queues/src/AzureClientBuilderExtensions.cs index 431636f3a747f..b6bcda0dec73e 100644 --- a/sdk/storage/Azure.Storage.Queues/src/AzureClientBuilderExtensions.cs +++ b/sdk/storage/Azure.Storage.Queues/src/AzureClientBuilderExtensions.cs @@ -29,7 +29,7 @@ public static IAzureClientBuilder AddQue { return builder.RegisterClientFactory( (options, token) => token != null ? new QueueServiceClient(serviceUri, token, options) : new QueueServiceClient(serviceUri, options), - supportsAnonymous: true); + requiresCredential: false); } ///