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 14 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
5 changes: 3 additions & 2 deletions eng/Directory.Build.Data.props
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,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
12 changes: 6 additions & 6 deletions eng/Packages.Data.props
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@
<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.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="Microsoft.Extensions.PlatformAbstractions" Version="1.1.0" />
<PackageReference Update="Microsoft.IdentityModel.Clients.ActiveDirectory" Version="[3.14.2, 4.5.1]" />
<PackageReference Update="Microsoft.NET.Test.Sdk" Version="16.1.0" />
Expand Down

This file was deleted.

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

using Azure.Core.Extensions;

namespace Azure.ApplicationModel.Configuration
{
public static class ConfigurationClientBuilderExtensions
pakrym marked this conversation as resolved.
Show resolved Hide resolved
{
public static IAzureClientBuilder<ConfigurationClient, ConfigurationClientOptions> AddConfigurationClient<TBuilder>(this TBuilder builder, string connectionString)
where TBuilder: IAzureClientsBuilder
{
return builder.RegisterClient<ConfigurationClient, ConfigurationClientOptions>(options => new ConfigurationClient(connectionString, options));
}

public static IAzureClientBuilder<ConfigurationClient, ConfigurationClientOptions> AddConfigurationClient<TBuilder, TConfiguration>(this TBuilder builder, TConfiguration configuration)
where TBuilder: IAzureClientsBuilderWithConfiguration<TConfiguration>
{
return builder.RegisterClient<ConfigurationClient, ConfigurationClientOptions>(configuration);
}
}
}
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 DIEnabledPolicy : SynchronousHttpPipelinePolicy
pakrym marked this conversation as resolved.
Show resolved Hide resolved
{
private readonly IHostingEnvironment _environment;

public DIEnabledPolicy(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>();
}
}
74 changes: 74 additions & 0 deletions sdk/core/Azure.Core.Extensions/samples/Startup.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using Azure.Core.Pipeline;
using Azure.Core.Pipeline.Policies;
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)
{
services.AddSingleton<DIEnabledPolicy>();
pakrym marked this conversation as resolved.
Show resolved Hide resolved

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<DIEnabledPolicy>()));

// Configure default credential
// builder.UseDefaultCredential(new ClientSecretCredential("tenantId","clientId","clientSecret"));
});
}

// 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": "",
pakrym marked this conversation as resolved.
Show resolved Hide resolved
"ClientSecret": "",
"TenantId": "",

"TelemetryPolicy": {
"ApplicationId": "AppId"
}
},
"KeyVault": {
"VaultUri": "https://pakrym-keyvault.vault.azure.net/"
pakrym marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,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.ToBuilderImpl().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.ToBuilderImpl();
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.ToBuilderImpl();
impl.ServiceCollection.AddSingleton<IConfigureOptions<TOptions>>(provider => new ConfigureClientOptions<TClient,TOptions>(provider, impl.Registration, configureOptions));;
return builder;
}

private static AzureClientBuilder<TClient, TOptions> ToBuilderImpl<TClient, TOptions>(this IAzureClientBuilder<TClient, TOptions> builder) where TOptions : class
pakrym marked this conversation as resolved.
Show resolved Hide resolved
{
return (AzureClientBuilder<TClient, TOptions>)builder;
}
}
}
Loading