Skip to content

Commit

Permalink
Merge branch 'add-retry-handler-to-authentication-providers' of https…
Browse files Browse the repository at this point in the history
  • Loading branch information
jansenbe committed Nov 24, 2021
2 parents 2bad7b2 + b7b24f2 commit b660970
Show file tree
Hide file tree
Showing 11 changed files with 138 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ public sealed class OnBehalfOfAuthenticationProvider : OAuthAuthenticationProvid
// Instance private member, to keep the token cache at service instance level
private IConfidentialClientApplication confidentialClientApplication;

// Instance private member, to keep the Msal Http Client Factory at service instance level
private IMsalHttpClientFactory msalHttpClientFactory;

/// <summary>
/// Public constructor for external consumers of the library
/// </summary>
Expand Down Expand Up @@ -66,7 +69,7 @@ public OnBehalfOfAuthenticationProvider(string clientId, string tenantId,
public OnBehalfOfAuthenticationProvider(string clientId, string tenantId,
PnPCoreAuthenticationOnBehalfOfOptions options,
Func<string> userTokenProvider)
: this(null)
: this(null, null)
{
UserTokenProvider = userTokenProvider;
Init(new PnPCoreAuthenticationCredentialConfigurationOptions
Expand All @@ -89,7 +92,7 @@ public OnBehalfOfAuthenticationProvider(string clientId, string tenantId,
public OnBehalfOfAuthenticationProvider(string clientId, string tenantId,
StoreName storeName, StoreLocation storeLocation, string thumbprint,
Func<string> userTokenProvider)
: this(null)
: this(null, null)
{
UserTokenProvider = userTokenProvider;
Init(new PnPCoreAuthenticationCredentialConfigurationOptions
Expand All @@ -106,12 +109,14 @@ public OnBehalfOfAuthenticationProvider(string clientId, string tenantId,
}

/// <summary>
/// Public constructor leveraging DI to initialize the ILogger interfafce
/// Public constructor leveraging DI to initialize the ILogger and IMsalHttpClientFactory interfaces
/// </summary>
/// <param name="logger">The instance of the logger service provided by DI</param>
public OnBehalfOfAuthenticationProvider(ILogger<OAuthAuthenticationProvider> logger)
/// <param name="msalHttpClientFactory">The instance of the Msal Http Client Factory service provided by DI</param>
public OnBehalfOfAuthenticationProvider(ILogger<OAuthAuthenticationProvider> logger, IMsalHttpClientFactory msalHttpClientFactory)
: base(logger)
{
this.msalHttpClientFactory = msalHttpClientFactory;
}

/// <summary>
Expand Down Expand Up @@ -154,6 +159,7 @@ internal override void Init(PnPCoreAuthenticationCredentialConfigurationOptions
confidentialClientApplication = ConfidentialClientApplicationBuilder
.Create(ClientId)
.WithCertificate(Certificate)
.WithHttpClientFactory(msalHttpClientFactory)
.WithPnPAdditionalAuthenticationSettings(
options.OnBehalfOf.AuthorityUri,
options.OnBehalfOf.RedirectUri,
Expand All @@ -166,6 +172,7 @@ internal override void Init(PnPCoreAuthenticationCredentialConfigurationOptions
confidentialClientApplication = ConfidentialClientApplicationBuilder
.Create(ClientId)
.WithClientSecret(ClientSecret.ToInsecureString())
.WithHttpClientFactory(msalHttpClientFactory)
.WithPnPAdditionalAuthenticationSettings(
options.OnBehalfOf.AuthorityUri,
options.OnBehalfOf.RedirectUri,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ public sealed class X509CertificateAuthenticationProvider : OAuthAuthenticationP
// Instance private member, to keep the token cache at service instance level
private IConfidentialClientApplication confidentialClientApplication;

// Instance private member, to keep the Msal Http Client Factory at service instance level
private IMsalHttpClientFactory msalHttpClientFactory;

/// <summary>
/// Public constructor for external consumers of the library
/// </summary>
Expand Down Expand Up @@ -68,7 +71,7 @@ public X509CertificateAuthenticationProvider(string clientId, string tenantId,
/// <param name="tenantId">The Tenant ID for the Authentication Provider</param>
/// <param name="options">Options for the authentication provider</param>
public X509CertificateAuthenticationProvider(string clientId, string tenantId, PnPCoreAuthenticationX509CertificateOptions options)
: this(null)
: this(null, null)
{
Init(new PnPCoreAuthenticationCredentialConfigurationOptions
{
Expand All @@ -79,12 +82,14 @@ public X509CertificateAuthenticationProvider(string clientId, string tenantId, P
}

/// <summary>
/// Public constructor leveraging DI to initialize the ILogger interfafce
/// Public constructor leveraging DI to initialize the ILogger and IMsalHttpClientFactory interfaces
/// </summary>
/// <param name="logger">The instance of the logger service provided by DI</param>
public X509CertificateAuthenticationProvider(ILogger<OAuthAuthenticationProvider> logger)
/// <param name="msalHttpClientFactory">The instance of the Msal Http Client Factory service provided by DI</param>
public X509CertificateAuthenticationProvider(ILogger<OAuthAuthenticationProvider> logger, IMsalHttpClientFactory msalHttpClientFactory)
: base(logger)
{
this.msalHttpClientFactory = msalHttpClientFactory;
}

/// <summary>
Expand Down Expand Up @@ -116,6 +121,7 @@ internal override void Init(PnPCoreAuthenticationCredentialConfigurationOptions
confidentialClientApplication = ConfidentialClientApplicationBuilder
.Create(ClientId)
.WithCertificate(Certificate)
.WithHttpClientFactory(msalHttpClientFactory)
.WithPnPAdditionalAuthenticationSettings(
options.X509Certificate.AuthorityUri,
options.X509Certificate.RedirectUri,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.Extensions.Logging;
using Microsoft.Identity.Client;
using PnP.Core.Auth.Services.Builder.Configuration;
using System;
using System.Configuration;
Expand Down Expand Up @@ -28,7 +29,7 @@ public sealed class CredentialManagerAuthenticationProvider : OAuthAuthenticatio
/// <param name="options">Options for the authentication provider</param>
public CredentialManagerAuthenticationProvider(string clientId, string tenantId,
PnPCoreAuthenticationCredentialManagerOptions options)
: this(null)
: this(null, null)
{
Init(new PnPCoreAuthenticationCredentialConfigurationOptions
{
Expand All @@ -54,14 +55,15 @@ public CredentialManagerAuthenticationProvider(string clientId, string tenantId,
}

/// <summary>
/// Public constructor leveraging DI to initialize the ILogger interfafce
/// Public constructor leveraging DI to initialize the ILogger and IMsalHttpClientFactory interfaces
/// </summary>
/// <param name="logger">The instance of the logger service provided by DI</param>
public CredentialManagerAuthenticationProvider(ILogger<OAuthAuthenticationProvider> logger)
/// <param name="msalHttpClientFactory">The instance of the Msal Http Client Factory service provided by DI</param>
public CredentialManagerAuthenticationProvider(ILogger<OAuthAuthenticationProvider> logger, IMsalHttpClientFactory msalHttpClientFactory)
: base(logger)
{
// Create an instance of the inner UsernamePasswordAuthenticationProvider
usernamePasswordProvider = new UsernamePasswordAuthenticationProvider(logger);
usernamePasswordProvider = new UsernamePasswordAuthenticationProvider(logger, msalHttpClientFactory);
}


Expand Down
12 changes: 9 additions & 3 deletions src/sdk/PnP.Core.Auth/Public/DeviceCodeAuthenticationProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ public class DeviceCodeAuthenticationProvider : OAuthAuthenticationProvider
// Instance private member, to keep the token cache at service instance level
private IPublicClientApplication publicClientApplication;

// Instance private member, to keep the Msal Http Client Factory at service instance level
private IMsalHttpClientFactory msalHttpClientFactory;

/// <summary>
/// Public constructor for external consumers of the library
/// </summary>
Expand All @@ -54,7 +57,7 @@ public DeviceCodeAuthenticationProvider(string clientId, string tenantId,
/// <param name="deviceCodeVerification">External action to manage the Device Code verification</param>
public DeviceCodeAuthenticationProvider(string clientId, string tenantId,
PnPCoreAuthenticationDeviceCodeOptions options, Action<DeviceCodeNotification> deviceCodeVerification)
: this(null)
: this(null, null)
{
DeviceCodeVerification = deviceCodeVerification;
Init(new PnPCoreAuthenticationCredentialConfigurationOptions
Expand All @@ -66,12 +69,14 @@ public DeviceCodeAuthenticationProvider(string clientId, string tenantId,
}

/// <summary>
/// Public constructor leveraging DI to initialize the ILogger interfafce
/// Public constructor leveraging DI to initialize the ILogger and IMsalHttpClientFactory interfaces
/// </summary>
/// <param name="logger">The instance of the logger service provided by DI</param>
public DeviceCodeAuthenticationProvider(ILogger<OAuthAuthenticationProvider> logger)
/// <param name="msalHttpClientFactory">The instance of the Msal Http Client Factory service provided by DI</param>
public DeviceCodeAuthenticationProvider(ILogger<OAuthAuthenticationProvider> logger, IMsalHttpClientFactory msalHttpClientFactory)
: base(logger)
{
this.msalHttpClientFactory = msalHttpClientFactory;
}

/// <summary>
Expand Down Expand Up @@ -99,6 +104,7 @@ internal override void Init(PnPCoreAuthenticationCredentialConfigurationOptions
RedirectUri,
TenantId,
options.Environment)
.WithHttpClientFactory(msalHttpClientFactory)
.Build();

// Log the initialization information
Expand Down
14 changes: 10 additions & 4 deletions src/sdk/PnP.Core.Auth/Public/InteractiveAuthenticationProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ public class InteractiveAuthenticationProvider : OAuthAuthenticationProvider
// Instance private member, to keep the token cache at service instance level
private IPublicClientApplication publicClientApplication;

// Instance private member, to keep the Msal Http Client Factory at service instance level
private IMsalHttpClientFactory msalHttpClientFactory;

/// <summary>
/// Public constructor for external consumers of the library
/// </summary>
Expand All @@ -44,7 +47,7 @@ public InteractiveAuthenticationProvider(string clientId, string tenantId, Uri r
/// <param name="tenantId">The Tenant ID for the Authentication Provider</param>
/// <param name="options">Options for the authentication provider</param>
public InteractiveAuthenticationProvider(string clientId, string tenantId, PnPCoreAuthenticationInteractiveOptions options)
: this(null)
: this(null, null)
{
Init(new PnPCoreAuthenticationCredentialConfigurationOptions
{
Expand All @@ -58,7 +61,7 @@ public InteractiveAuthenticationProvider(string clientId, string tenantId, PnPCo
/// Public constructor for external consumers of the library
/// </summary>
public InteractiveAuthenticationProvider()
: this(null)
: this(null, null)
{
Init(new PnPCoreAuthenticationCredentialConfigurationOptions
{
Expand All @@ -69,12 +72,14 @@ public InteractiveAuthenticationProvider()
}

/// <summary>
/// Public constructor leveraging DI to initialize the ILogger interfafce
/// Public constructor leveraging DI to initialize the ILogger and IMsalHttpClientFactory interfaces
/// </summary>
/// <param name="logger">The instance of the logger service provided by DI</param>
public InteractiveAuthenticationProvider(ILogger<OAuthAuthenticationProvider> logger)
/// <param name="msalHttpClientFactory">The instance of the Msal Http Client Factory service provided by DI</param>
public InteractiveAuthenticationProvider(ILogger<OAuthAuthenticationProvider> logger, IMsalHttpClientFactory msalHttpClientFactory)
: base(logger)
{
this.msalHttpClientFactory = msalHttpClientFactory;
}

/// <summary>
Expand All @@ -90,6 +95,7 @@ internal override void Init(PnPCoreAuthenticationCredentialConfigurationOptions
// Build the MSAL client
publicClientApplication = PublicClientApplicationBuilder
.Create(ClientId)
.WithHttpClientFactory(msalHttpClientFactory)
.WithPnPAdditionalAuthenticationSettings(
options.Interactive?.AuthorityUri,
RedirectUri,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ public sealed class UsernamePasswordAuthenticationProvider : OAuthAuthentication
// Instance private member, to keep the token cache at service instance level
private IPublicClientApplication publicClientApplication;

// Instance private member, to keep the Msal Http Client Factory at service instance level
private IMsalHttpClientFactory msalHttpClientFactory;

/// <summary>
/// Public constructor for external consumers of the library
/// </summary>
Expand All @@ -58,7 +61,7 @@ public UsernamePasswordAuthenticationProvider(string clientId, string tenantId,
/// <param name="options">Options for the authentication provider</param>
public UsernamePasswordAuthenticationProvider(string clientId, string tenantId,
PnPCoreAuthenticationUsernamePasswordOptions options)
: this(null)
: this(null, null)
{
Init(new PnPCoreAuthenticationCredentialConfigurationOptions
{
Expand All @@ -69,12 +72,14 @@ public UsernamePasswordAuthenticationProvider(string clientId, string tenantId,
}

/// <summary>
/// Public constructor leveraging DI to initialize the ILogger interfafce
/// Public constructor leveraging DI to initialize the ILogger and IMsalHttpClientFactory interfaces
/// </summary>
/// <param name="logger">The instance of the logger service provided by DI</param>
public UsernamePasswordAuthenticationProvider(ILogger<OAuthAuthenticationProvider> logger)
/// <param name="msalHttpClientFactory">The instance of the Msal Http Client Factory service provided by DI</param>
public UsernamePasswordAuthenticationProvider(ILogger<OAuthAuthenticationProvider> logger, IMsalHttpClientFactory msalHttpClientFactory)
: base(logger)
{
this.msalHttpClientFactory = msalHttpClientFactory;
}

/// <summary>
Expand Down Expand Up @@ -115,6 +120,7 @@ internal override void Init(PnPCoreAuthenticationCredentialConfigurationOptions
options.UsernamePassword.RedirectUri,
TenantId,
options.Environment)
.WithHttpClientFactory(msalHttpClientFactory)
.Build();

// Log the initialization information
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
using PnP.Core.Auth;
using Microsoft.Identity.Client;
using PnP.Core.Auth;
using PnP.Core.Auth.Services;
using PnP.Core.Auth.Services.Builder.Configuration;
using PnP.Core.Auth.Services.Http;
using PnP.Core.Services;
using System;

Expand Down Expand Up @@ -40,11 +42,20 @@ public static IServiceCollection AddPnPCoreAuthentication(this IServiceCollectio
}

AddAuthenticationProviders(collection);
AddMsalHttpClientFactory(collection);
collection.ConfigureOptions<AuthenticationProvidersOptionsConfigurator>();

return collection;
}

private static void AddMsalHttpClientFactory(IServiceCollection collection)
{
collection.AddTransient<IMsalHttpClientFactory, MsalHttpClientFactory>();
collection.AddTransient<MsalRetryHandler, MsalRetryHandler>();
collection.AddHttpClient("MsalHttpClient")
.AddHttpMessageHandler<MsalRetryHandler>();
}

private static void AddAuthenticationProviders(IServiceCollection collection)
{
// Add the service farctory for the authentication providers
Expand Down
20 changes: 20 additions & 0 deletions src/sdk/PnP.Core.Auth/Services/Http/MsalHttpClientFactory.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
using Microsoft.Identity.Client;
using PnP.Core.Services;
using System.Net.Http;

namespace PnP.Core.Auth.Services.Http
{
public class MsalHttpClientFactory : IMsalHttpClientFactory
{
private IHttpClientFactory _httpClientFactory;

public MsalHttpClientFactory(IHttpClientFactory httpClientFactory)
{
_httpClientFactory = httpClientFactory;
}
public HttpClient GetHttpClient()
{
return _httpClientFactory.CreateClient("MsalHttpClient");
}
}
}
31 changes: 31 additions & 0 deletions src/sdk/PnP.Core.Auth/Services/Http/MsalRetryHandler.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using PnP.Core.Services;

namespace PnP.Core.Auth.Services.Http
{
/// <summary>
/// Retry handler for Microsoft Graph requests
/// </summary>
internal sealed class MsalRetryHandler : RetryHandlerBase
{
#region Construction
public MsalRetryHandler(ILogger<RetryHandlerBase> log, IOptions<PnPGlobalSettingsOptions> globalSettings) : base(log, globalSettings?.Value)
{
Configure();
}
#endregion

private void Configure()
{
if (GlobalSettings != null)
{
UseRetryAfterHeader = GlobalSettings.HttpMsalUseRetryAfterHeader;
MaxRetries = GlobalSettings.HttpMsalMaxRetries;
DelayInSeconds = GlobalSettings.HttpMsalDelayInSeconds;
IncrementalDelay = GlobalSettings.HttpMsalUseIncrementalDelay;
}
}

}
}
2 changes: 1 addition & 1 deletion src/sdk/PnP.Core/Services/Core/Http/RetryHandlerBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage

// Drain response content to free connections. Need to perform this
// before retry attempt and before the TooManyRetries ServiceException.
if (response.Content != null)
if (response?.Content != null)
{
#if NET5_0_OR_GREATER
await response.Content.ReadAsByteArrayAsync(cancellationToken).ConfigureAwait(false);
Expand Down
20 changes: 20 additions & 0 deletions src/sdk/PnP.Core/Services/Core/PnPGlobalSettingsOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,26 @@ internal set
/// </summary>
public bool HttpMicrosoftGraphUseIncrementalDelay { get; set; } = true;

/// <summary>
/// Use the Retry-After header for calculating the delay in case of a retry. Defaults to true
/// </summary>
public bool HttpMsalUseRetryAfterHeader { get; set; } = true;

/// <summary>
/// When not using retry-after, how many times can a retry be made. Defaults to 10
/// </summary>
public int HttpMsalMaxRetries { get; set; } = 10;

/// <summary>
/// How many seconds to wait for the next retry attempt. Defaults to 3
/// </summary>
public int HttpMsalDelayInSeconds { get; set; } = 3;

/// <summary>
/// Use an incremental strategy for the delay: each retry doubles the previous delay time. Defaults to true
/// </summary>
public bool HttpMsalUseIncrementalDelay { get; set; } = true;

#endregion

#region Internal only settings (supporting, they cannot be assigned from configuration)
Expand Down

0 comments on commit b660970

Please sign in to comment.