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.

Sign up for GitHub

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

Jump to bottom

Proposal for the parameters of AddProtectedApi ... methods #69

Merged
merged 3 commits into from
Mar 31, 2020
Merged

Proposal for the parameters of AddProtectedApi ... methods #69

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c066a55
Proposal for the parameters of AddProtected ...
jmprieur Mar 30, 2020
e57392f
Updating the tests to use the new overrides.
jmprieur Mar 30, 2020
8c515c8
Improving the API (removing hybrid solutions between config section a…
jmprieur Mar 31, 2020
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
86 changes: 67 additions & 19 deletions src/Microsoft.Identity.Web/WebApiServiceCollectionExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.Authentication.JwtBearer;
using Microsoft.AspNetCore.Authentication.OpenIdConnect;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Identity.Client;
Expand Down Expand Up @@ -55,15 +54,14 @@ public static IServiceCollection AddProtectedApiCallsWebApis(
public static IServiceCollection AddProtectedWebApi(
this IServiceCollection services,
IConfiguration configuration,
X509Certificate2 tokenDecryptionCertificate = null,
string configSectionName = "AzureAd",
X509Certificate2 tokenDecryptionCertificate = null,
bool subscribeToJwtBearerMiddlewareDiagnosticsEvents = false)
{
services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
.AddProtectedWebApi(
.AddProtectedWebApi(configuration,
configSectionName,
configuration,
options => configuration.Bind(configSectionName, options),
JwtBearerDefaults.AuthenticationScheme,
tokenDecryptionCertificate,
subscribeToJwtBearerMiddlewareDiagnosticsEvents);

Expand All @@ -82,6 +80,7 @@ public static IServiceCollection AddProtectedWebApi(
/// Set to true if you want to debug, or just understand the JwtBearer events.
/// </param>
/// <returns></returns>
[Obsolete("rather use the override with the configSectionName, or the configure option actions")]
jmprieur marked this conversation as resolved.
Show resolved Hide resolved
public static AuthenticationBuilder AddProtectedWebApi(
this AuthenticationBuilder builder,
IConfiguration configuration,
Expand All @@ -91,10 +90,9 @@ public static AuthenticationBuilder AddProtectedWebApi(
{
return AddProtectedWebApi(
builder,
"AzureAd",
configuration,
JwtBearerDefaults.AuthenticationScheme,
configureOptions,
options => configuration.Bind("AzureAd", options),
tokenDecryptionCertificate,
subscribeToJwtBearerMiddlewareDiagnosticsEvents);
}
Expand All @@ -112,6 +110,7 @@ public static AuthenticationBuilder AddProtectedWebApi(
/// Set to true if you want to debug, or just understand the JwtBearer events.
/// </param>
/// <returns></returns>
[Obsolete("rather use the override with the configSectionName, or the configure option actions")]
public static AuthenticationBuilder AddProtectedWebApi(
this AuthenticationBuilder builder,
string configSectionName,
Expand All @@ -124,7 +123,6 @@ public static AuthenticationBuilder AddProtectedWebApi(
builder,
configSectionName,
configuration,
JwtBearerDefaults.AuthenticationScheme,
configureOptions,
tokenDecryptionCertificate,
subscribeToJwtBearerMiddlewareDiagnosticsEvents);
Expand All @@ -135,34 +133,64 @@ public static AuthenticationBuilder AddProtectedWebApi(
/// This method expects the configuration file will have a section, named "AzureAd" as default, with the necessary settings to initialize authentication options.
/// </summary>
/// <param name="builder">AuthenticationBuilder to which to add this configuration</param>
/// <param name="configSectionName">The configuration section with the necessary settings to initialize authentication options</param>
/// <param name="configuration">The Configuration object</param>
/// <param name="configSectionName">The configuration section with the necessary settings to initialize authentication options</param>
/// <param name="jwtBearerScheme">The JwtBearer scheme name to be used. By default it uses "Bearer"</param>
/// <param name="configureOptions">An action to configure JwtBearerOptions</param>
/// <param name="tokenDecryptionCertificate">Token decryption certificate</param>
/// <param name="tokenDecryptionCertificate">Token decryption certificate (null by default)</param>
/// <param name="subscribeToJwtBearerMiddlewareDiagnosticsEvents">
/// Set to true if you want to debug, or just understand the JwtBearer events.
/// </param>
/// <returns></returns>
public static AuthenticationBuilder AddProtectedWebApi(
this AuthenticationBuilder builder,
string configSectionName,
IConfiguration configuration,
string jwtBearerScheme,
Action<JwtBearerOptions> configureOptions,
string configSectionName = "AzureAd",
string jwtBearerScheme = JwtBearerDefaults.AuthenticationScheme,
X509Certificate2 tokenDecryptionCertificate = null,
bool subscribeToJwtBearerMiddlewareDiagnosticsEvents = false)
{
builder.Services.Configure(jwtBearerScheme, configureOptions);
builder.Services.Configure<MicrosoftIdentityOptions>(options => configuration.Bind(configSectionName, options));
return builder.AddProtectedWebApi(jwtBearerScheme,
options => configuration.Bind(configSectionName, options),
options => configuration.Bind(configSectionName, options),
tokenDecryptionCertificate,
subscribeToJwtBearerMiddlewareDiagnosticsEvents);
}

/// <summary>
/// Protects the Web API with Microsoft identity platform (formerly Azure AD v2.0)
/// This method expects the configuration file will have a section, named "AzureAd" as default, with the necessary settings to initialize authentication options.
/// </summary>
/// <param name="builder">AuthenticationBuilder to which to add this configuration</param>
/// <param name="configSectionName">The configuration section with the necessary settings to initialize authentication options</param>
/// <param name="configuration">The Configuration object</param>
/// <param name="jwtBearerScheme">The JwtBearer scheme name to be used. By default it uses "Bearer"</param>
/// <param name="jwtOptionsMapper">An action to configure JwtBearerOptions</param>
/// <param name="tokenDecryptionCertificate">Token decryption certificate</param>
/// <param name="subscribeToJwtBearerMiddlewareDiagnosticsEvents">
/// Set to true if you want to debug, or just understand the JwtBearer events.
/// </param>
/// <returns></returns>
public static AuthenticationBuilder AddProtectedWebApi(
this AuthenticationBuilder builder,
string jwtBearerScheme,
Action<JwtBearerOptions> jwtOptionsMapper,
Action<MicrosoftIdentityOptions> microsoftIdentityOptionsMapper,
jmprieur marked this conversation as resolved.
Show resolved Hide resolved
X509Certificate2 tokenDecryptionCertificate = null,
bool subscribeToJwtBearerMiddlewareDiagnosticsEvents = false)
{
builder.Services.Configure(jwtBearerScheme, jwtOptionsMapper);
builder.Services.Configure<MicrosoftIdentityOptions>(microsoftIdentityOptionsMapper);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the generic type can be removed, since it can be inferred or we can add one in the line above for consistency and if we want to be explicit or not. (Same applied to similar methods below).


builder.Services.AddHttpContextAccessor();
builder.Services.AddSingleton<IJwtBearerMiddlewareDiagnostics, JwtBearerMiddlewareDiagnostics>();

// Change the authentication configuration to accommodate the Microsoft identity platform endpoint (v2.0).
builder.AddJwtBearer(jwtBearerScheme, options =>
{
var microsoftIdentityOptions = configuration.GetSection(configSectionName).Get<MicrosoftIdentityOptions>();
// TODO:
// Suspect. Why not get the IOption<MicrosoftIdentityOptions>?
Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense.

var microsoftIdentityOptions = new MicrosoftIdentityOptions();// configuration.GetSection(configSectionName).Get<MicrosoftIdentityOptions>();
microsoftIdentityOptionsMapper(microsoftIdentityOptions);

if (string.IsNullOrWhiteSpace(options.Authority))
options.Authority = AuthorityHelpers.BuildAuthority(microsoftIdentityOptions);
Expand Down Expand Up @@ -225,17 +253,37 @@ public static AuthenticationBuilder AddProtectedWebApi(
/// </summary>
/// <param name="services">Service collection to which to add authentication</param>
/// <param name="configuration">Configuration</param>
/// <param name="configSectionName">Section name in the config file (by default "AzureAD")</param>
/// <param name="jwtBearerScheme">Scheme for the JwtBearer token</param>
/// <returns></returns>
public static IServiceCollection AddProtectedWebApiCallsProtectedWebApi(
this IServiceCollection services,
IConfiguration configuration,
string configSectionName = "AzureAd",
string jwtBearerScheme = JwtBearerDefaults.AuthenticationScheme)
{
return services.AddProtectedWebApiCallsProtectedWebApi(
jwtBearerScheme,
options => configuration.Bind(configSectionName, options),
options => configuration.Bind(configSectionName, options));
}
/// <summary>
/// Protects the Web API with Microsoft identity platform (formerly Azure AD v2.0)
/// This supposes that the configuration files have a section named configSectionName (typically "AzureAD")
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is no longer applicable.

/// </summary>
/// <param name="services">Service collection to which to add authentication</param>
/// <param name="configuration">Configuration</param>
/// <returns></returns>
public static IServiceCollection AddProtectedWebApiCallsProtectedWebApi(
this IServiceCollection services,
string jwtBearerScheme,
Action<ConfidentialClientApplicationOptions> msalAppOptionsMapper,
Action<MicrosoftIdentityOptions> microsoftIdentityOptionsMapper)
jmprieur marked this conversation as resolved.
Show resolved Hide resolved
{
services.AddTokenAcquisition();
services.AddHttpContextAccessor();
services.Configure<ConfidentialClientApplicationOptions>(options => configuration.Bind(configSectionName, options));
services.Configure<MicrosoftIdentityOptions>(options => configuration.Bind(configSectionName, options));
services.Configure<ConfidentialClientApplicationOptions>(msalAppOptionsMapper);
services.Configure<MicrosoftIdentityOptions>(microsoftIdentityOptionsMapper);

services.Configure<JwtBearerOptions>(jwtBearerScheme, options =>
{
Expand Down