Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add credential support to ASP.NET Core integration layer #6805

Merged
merged 21 commits into from
Jul 17, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 4 additions & 3 deletions eng/Directory.Build.Data.props
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,10 @@

<PropertyGroup>
<IsTestProject Condition="$(MSBuildProjectName.EndsWith('.Tests'))">true</IsTestProject>
<IsSamplesProject Condition="$(MSBuildProjectName.EndsWith('.Samples'))">true</IsSamplesProject>
Copy link
Member

Choose a reason for hiding this comment

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

Nit - any reason not to follow our existing .Samples.Tests approach for testable samples?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really want to give folks outside our team an easy way to play with new APIs in ASP.NET Core app

Copy link
Member

Choose a reason for hiding this comment

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

We can also consider checking for /samples/ in the path.

<IsTestSupportProject Condition="'$(IsTestProject)' != 'true' and ($(MSBuildProjectDirectory.Contains('/tests/')) or $(MSBuildProjectDirectory.Contains('\tests\')))">true</IsTestSupportProject>
<EnableClientSdkAnalyzers Condition="'$(IsClientLibrary)' == 'true' and '$(IsTestProject)' != 'true' and '$(IsTestSupportProject)' != 'true'">true</EnableClientSdkAnalyzers>
<EnableFxCopAnalyzers Condition="'$(IsClientLibrary)' == 'true' and '$(IsTestProject)' != 'true' and '$(IsTestSupportProject)' != 'true'">true</EnableFxCopAnalyzers>
<EnableClientSdkAnalyzers Condition="'$(IsClientLibrary)' == 'true' and '$(IsTestProject)' != 'true' and '$(IsTestSupportProject)' != 'true' and '$(IsSamplesProject)' != 'true'">true</EnableClientSdkAnalyzers>
<EnableFxCopAnalyzers Condition="'$(IsClientLibrary)' == 'true' and '$(IsTestProject)' != 'true' and '$(IsTestSupportProject)' != 'true' and '$(IsSamplesProject)' != 'true'">true</EnableFxCopAnalyzers>
</PropertyGroup>

<!-- TargetFramework default properties -->
Expand Down Expand Up @@ -112,7 +113,7 @@
<DocumentationFile>$(IntermediateOutputPath)$(TargetFramework)\$(MSBuildProjectName).xml</DocumentationFile>
</PropertyGroup>

<PropertyGroup Condition="'$(IsTestProject)' == 'true' or '$(IsTestSupportProject)' == 'true'">
<PropertyGroup Condition="'$(IsTestProject)' == 'true' or '$(IsTestSupportProject)' == 'true' or '$(IsSamplesProject)' == 'true'">
<!-- Always fully sign test assemblies since we have a full public/private key -->
<PublicSign>false</PublicSign>
<DelaySign>false</DelaySign>
Expand Down
12 changes: 6 additions & 6 deletions eng/Packages.Data.props
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,10 @@
<PackageReference Update="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="2.6.1" />
<PackageReference Update="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="2.6.2" />
<PackageReference Update="Microsoft.CodeAnalysis" Version="2.3.0" />
<PackageReference Update="Microsoft.Extensions.Configuration.Abstractions" Version="2.2.0" />
<PackageReference Update="Microsoft.Extensions.Configuration.Binder" Version="2.2.0" />
<PackageReference Update="Microsoft.Extensions.Configuration.Abstractions" Version="2.1.0" />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've downgraded versions of dependencies to the current LTS version. Azure.Core.Extensions is the only project using them

<PackageReference Update="Microsoft.Extensions.Configuration.Binder" Version="2.1.0" />
<PackageReference Update="Microsoft.Extensions.Configuration.Json" Version="1.0.2" />
<PackageReference Update="Microsoft.Extensions.Configuration" Version="1.0.2" />
<PackageReference Update="Microsoft.Extensions.DependencyInjection.Abstractions" Version="2.2.0" />
<PackageReference Update="Microsoft.Extensions.DependencyInjection" Version="2.2.0" />
<PackageReference Update="Microsoft.Extensions.Logging.Abstractions" Version="2.2.0" />
<PackageReference Update="Microsoft.Extensions.Options" Version="2.2.0" />
<PackageReference Update="Microsoft.Extensions.PlatformAbstractions" Version="1.1.0" />
<PackageReference Update="Microsoft.IdentityModel.Clients.ActiveDirectory" Version="4.5.1" />
<PackageReference Update="Microsoft.NET.Test.Sdk" Version="16.1.0" />
Expand Down Expand Up @@ -104,6 +100,10 @@
<PackageReference Update="Azure.ClientSdk.Analyzers" Version="0.1.1-dev.20190716.1" />
<PackageReference Update="System.Memory" Version="4.5.3" />
<PackageReference Update="Microsoft.Bcl.AsyncInterfaces" Version="1.0.0-preview6.19303.8" />
<PackageReference Update="Microsoft.Extensions.DependencyInjection.Abstractions" Version="2.1.0" />
<PackageReference Update="Microsoft.Extensions.DependencyInjection" Version="2.1.0" />
<PackageReference Update="Microsoft.Extensions.Logging.Abstractions" Version="2.1.0" />
<PackageReference Update="Microsoft.Extensions.Options" Version="2.1.0" />
<PackageReference Update="System.Text.Json" Version="4.6.0-preview6.19303.8" />
<PackageReference Update="System.Diagnostics.DiagnosticSource" Version="4.6.0-preview6.19303.8" />
<PackageReference Update="System.Runtime.CompilerServices.Unsafe" Version="4.6.0-preview6.19303.8" />
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@
<TargetFrameworks>$(RequiredTargetFrameworks)</TargetFrameworks>
<NoWarn>$(NoWarn);3021</NoWarn>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>

<!-- Disable warning for missing xml doc comments until we can add all the missing ones -->
<NoWarn>$(NoWarn);1591</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using Azure.Core.Extensions;

namespace Azure.ApplicationModel.Configuration
{
/// <summary>
/// Extension methods to add <see cref="ConfigurationClient"/> client to clients builder
/// </summary>
public static class AzureClientBuilderExtensions
{
/// <summary>
/// Registers a <see cref="ConfigurationClient"/> instance with the provided <paramref name="connectionString"/>
/// </summary>
public static IAzureClientBuilder<ConfigurationClient, ConfigurationClientOptions> AddConfigurationClient<TBuilder>(this TBuilder builder, string connectionString)
where TBuilder: IAzureClientFactoryBuilder
{
return builder.RegisterClientFactory<ConfigurationClient, ConfigurationClientOptions>(options => new ConfigurationClient(connectionString, options));
}

/// <summary>
/// Registers a <see cref="ConfigurationClient"/> instance with connection options loaded from the provided <paramref name="configuration"/> instance.
/// </summary>
public static IAzureClientBuilder<ConfigurationClient, ConfigurationClientOptions> AddConfigurationClient<TBuilder, TConfiguration>(this TBuilder builder, TConfiguration configuration)
where TBuilder: IAzureClientFactoryBuilderWithConfiguration<TConfiguration>
{
return builder.RegisterClientFactory<ConfigurationClient, ConfigurationClientOptions>(configuration);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ public class ConfigurationClientOptions: ClientOptions
/// </summary>
public enum ServiceVersion
{
/// <summary>
/// Uses the latest service version
/// </summary>
Default = 0
}

Expand All @@ -35,7 +38,7 @@ public enum ServiceVersion
/// </summary>
/// <param name="version">
/// The <see cref="ServiceVersion"/> of the service API used when
/// making requests.
/// making requests.
/// </param>
public ConfigurationClientOptions(ServiceVersion version = ServiceVersion.Default)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<Project Sdk="Microsoft.NET.Sdk.Web">
<PropertyGroup>
<RequiredTargetFrameworks>netcoreapp2.1</RequiredTargetFrameworks>
<TargetFrameworks>$(RequiredTargetFrameworks)</TargetFrameworks>
<WarnOnPackingNonPackableProject>false</WarnOnPackingNonPackableProject>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.AspNetCore.App" IsImplicitlyDefined="true" />
Copy link
Member

Choose a reason for hiding this comment

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

Orthogonal to these changes: I'm not familiar with IsImplicitlyDefined and a quick search didn't turn up anything. I'd be grateful for a quick pointer to a reference or explanation of what it is for. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sooooo, you are not supposed to have package version for Microsoft.AspNetCore.App package, SDK would pick the highest patch for you based on the target framework.

Unfortunately, this conflicts with our central version management strategy that produces a warning when something outside of Packages.Data.props set a package version. IsImplicitlyDefined suppresses this warning.

<ProjectReference Include="..\..\..\keyvault\Azure.Security.KeyVault.Secrets\src\Azure.Security.KeyVault.Secrets.csproj" />
<ProjectReference Include="..\src\Azure.Core.Extensions.csproj" />
</ItemGroup>
</Project>
24 changes: 24 additions & 0 deletions sdk/core/Azure.Core.Extensions/samples/CustomPolicy.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using Azure.Core.Pipeline;
using Microsoft.AspNetCore.Hosting;

namespace Azure.Core.Extensions.Samples
{
internal class DependencyInjectionEnabledPolicy : SynchronousHttpPipelinePolicy
{
private readonly IHostingEnvironment _environment;

public DependencyInjectionEnabledPolicy(IHostingEnvironment environment)
{
this._environment = environment;
}

public override void OnSendingRequest(HttpPipelineMessage message)
{
message.Request.Headers.Add("application-name", _environment.ApplicationName);
base.OnSendingRequest(message);
}
}
}
20 changes: 20 additions & 0 deletions sdk/core/Azure.Core.Extensions/samples/Program.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using Microsoft.AspNetCore;
using Microsoft.AspNetCore.Hosting;

namespace Azure.Core.Extensions.Samples
{
public class Program
{
public static void Main(string[] args)
{
CreateHostBuilder(args).Build().Run();
}

public static IWebHostBuilder CreateHostBuilder(string[] args) =>
WebHost.CreateDefaultBuilder(args)
.UseStartup<Startup>();
}
}
71 changes: 71 additions & 0 deletions sdk/core/Azure.Core.Extensions/samples/Startup.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using Azure.Core.Pipeline;
using Azure.Identity;
using Azure.Security.KeyVault.Secrets;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;

namespace Azure.Core.Extensions.Samples
{
public class Startup
{
public IConfiguration Configuration { get; }

public Startup(IConfiguration configuration)
{
Configuration = configuration;
}

// This method gets called by the runtime. Use this method to add services to the container.
// For more information on how to configure your application, visit https://go.microsoft.com/fwlink/?LinkID=398940
public void ConfigureServices(IServiceCollection services)
{
// Registering policy to use in ConfigureDefaults later
services.AddSingleton<DependencyInjectionEnabledPolicy>();

services.AddAzureClients(builder => {

builder.AddSecretClient(Configuration.GetSection("KeyVault"))
Copy link
Member

Choose a reason for hiding this comment

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

When I see it written like this, I wonder if we should be calling it AddKeyVaultSecretClient.

Copy link
Contributor Author

@pakrym pakrym Jul 17, 2019

Choose a reason for hiding this comment

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

We've been back and forth on this couple of times with @KrzysztofCwalina, I'm open to any naming schema but would prefer to follow up on this later.

.WithName("Default")
.WithCredential(new DefaultAzureCredential())
.ConfigureOptions(options => options.Retry.MaxRetries = 10);

builder.AddSecretClient(new Uri("http://my.keyvault.com"));

builder.UseCredential(new DefaultAzureCredential());
pakrym marked this conversation as resolved.
Show resolved Hide resolved

// This would use configuration for auth and client settings
builder.ConfigureDefaults(Configuration.GetSection("Default"));

// Configure global defaults
builder.ConfigureDefaults(options => options.Retry.Mode = RetryMode.Exponential);

// Advanced configure global defaults
builder.ConfigureDefaults((options, provider) => options.AddPolicy(HttpPipelinePosition.PerCall, provider.GetService<DependencyInjectionEnabledPolicy>()));
});
}

// This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
public void Configure(IApplicationBuilder app, IHostingEnvironment env, SecretClient secretClient)
{
if (env.IsDevelopment())
{
app.UseDeveloperExceptionPage();
}

app.Run(async context => {
context.Response.ContentType = "text";
foreach (var secret in secretClient.GetSecrets())
{
await context.Response.WriteAsync(secret.Value.Name + Environment.NewLine);
}
});
}
}
}
20 changes: 20 additions & 0 deletions sdk/core/Azure.Core.Extensions/samples/appsettings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"Logging": {
"LogLevel": {
"Default": "Debug"
}
},
"AllowedHosts": "*",
"Default": {
"ClientId": "<client_id>",
"ClientSecret": "<client_secret>",
"TenantId": "<tenant_id>",

"TelemetryPolicy": {
"ApplicationId": "AppId"
}
},
"KeyVault": {
"VaultUri": "<vault_uri>"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\..\..\identity\Azure.Identity\src\Azure.Identity.csproj" />
Copy link
Member

Choose a reason for hiding this comment

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

Should this use a package reference rather than a project reference?

Copy link
Contributor Author

@pakrym pakrym Jul 17, 2019

Choose a reason for hiding this comment

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

I'll keep it a package reference until preview2 so we can iterate them in parallel.

<ProjectReference Include="..\..\Azure.Core\src\Azure.Core.csproj" />
</ItemGroup>

Expand Down
53 changes: 53 additions & 0 deletions sdk/core/Azure.Core.Extensions/src/AzureClientBuilderExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;

namespace Azure.Core.Extensions
{
public static class AzureClientBuilderExtensions
{
public static IAzureClientBuilder<TClient, TOptions> WithName<TClient, TOptions>(this IAzureClientBuilder<TClient, TOptions> builder, string name) where TOptions : class
{
builder.ToBuilder().Registration.Name = name;
return builder;
}

public static IAzureClientBuilder<TClient, TOptions> WithCredential<TClient, TOptions>(this IAzureClientBuilder<TClient, TOptions> builder, TokenCredential credential) where TOptions : class
Copy link
Member

Choose a reason for hiding this comment

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

Should we add a base credential type? We have other types like StorageSharedKeyCredential that won't work here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll follow up on this while working on storage extension methods.

{
return builder.WithCredential(_ => credential);
}

public static IAzureClientBuilder<TClient, TOptions> WithCredential<TClient, TOptions>(this IAzureClientBuilder<TClient, TOptions> builder, Func<IServiceProvider, TokenCredential> credentialFactory) where TOptions : class
{
var impl = builder.ToBuilder();
impl.ServiceCollection.AddSingleton<IConfigureOptions<AzureClientCredentialOptions<TClient>>>(new ConfigureClientCredentials<TClient,TOptions>(impl.Registration, credentialFactory));
return builder;
}

public static IAzureClientBuilder<TClient, TOptions> ConfigureOptions<TClient, TOptions>(this IAzureClientBuilder<TClient, TOptions> builder, Action<TOptions> configureOptions) where TOptions : class
{
return builder.ConfigureOptions((options, _) => configureOptions(options));
}

public static IAzureClientBuilder<TClient, TOptions> ConfigureOptions<TClient, TOptions>(this IAzureClientBuilder<TClient, TOptions> builder, IConfiguration configuration) where TOptions : class
{
return builder.ConfigureOptions(options => configuration.Bind(options));
}

public static IAzureClientBuilder<TClient, TOptions> ConfigureOptions<TClient, TOptions>(this IAzureClientBuilder<TClient, TOptions> builder, Action<TOptions, IServiceProvider> configureOptions) where TOptions : class
{
var impl = builder.ToBuilder();
impl.ServiceCollection.AddSingleton<IConfigureOptions<TOptions>>(provider => new ConfigureClientOptions<TClient,TOptions>(provider, impl.Registration, configureOptions));;
return builder;
}

private static AzureClientBuilder<TClient, TOptions> ToBuilder<TClient, TOptions>(this IAzureClientBuilder<TClient, TOptions> builder) where TOptions : class
{
return (AzureClientBuilder<TClient, TOptions>)builder;
}
}
}
Loading