Skip to content

Commit

Permalink
Jmprieur/test cert rotation (#2496)
Browse files Browse the repository at this point in the history
Added an integration test validating the certificate rotation (including app registration of short time self-assigned certificates)
Added a way for the apps to observe that the certs are selected or unselected (#2498). This is currently an experimental feature (to get feedback)
Improved the cert rotation and fixed an issue in the rotation of client certificates.
 - Adding a ResetCertificates with an override for an enumeration of CredentialDescription
- (unrelated) fixing mappings in OWIN devapps
  • Loading branch information
jmprieur authored Oct 3, 2023
1 parent 086bf8d commit 826ff82
Show file tree
Hide file tree
Showing 10 changed files with 486 additions and 36 deletions.
18 changes: 18 additions & 0 deletions src/Microsoft.Identity.Web.Certificate/DefaultCertificateLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,24 @@ public static void ResetCertificates(IEnumerable<CertificateDescription>? certif
foreach (var cert in certificateDescriptions)
{
cert.Certificate = null;
cert.CachedValue = null;
}
}
}

/// <summary>
/// Resets all the certificates in the certificate description list.
/// Use, for example, before a retry.
/// </summary>
/// <param name="credentialDescription">Description of the certificates.</param>
public static void ResetCertificates(IEnumerable<CredentialDescription>? credentialDescription)
{
if (credentialDescription != null)
{
foreach (var cert in credentialDescription.Where(c => c.Certificate != null))
{
cert.Certificate = null;
cert.CachedValue = null;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Security.Cryptography.X509Certificates;
using System.Threading;
using System.Threading.Tasks;
using Azure.Identity;
using Microsoft.Extensions.Logging;
using Microsoft.Identity.Abstractions;
using Microsoft.Identity.Client;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System.Security.Cryptography.X509Certificates;
using Microsoft.Identity.Abstractions;

// Types in the Microsoft.Identity.Web.Experimental namespace
// are meant to get feedback from the community on proposed features, and
// may be modified or removed in future releases without obeying to the
// semantic versionning.
namespace Microsoft.Identity.Web.Experimental
{
/// <summary>
/// Action of the token acquirer on the certificate.
/// </summary>
public enum CerticateObserverAction
{
/// <summary>
/// The certificate was selected as a client certificate.
/// </summary>
Selected,

/// <summary>
/// The certificate was deselected as a client certificate. This
/// happens when the STS does not accept the certificate any longer.
/// </summary>
Deselected,
}

/// <summary>
/// Event argument about the certificate consumption by the app
/// </summary>
public class CertificateChangeEventArg
{
/// <summary>
/// Action on the certificate
/// </summary>
public CerticateObserverAction Action { get; set; }

/// <summary>
/// Certificate
/// </summary>
public X509Certificate2? Certificate { get; set; }

/// <summary>
/// Credential description
/// </summary>
public CredentialDescription? CredentialDescription { get; set; }
}

/// <summary>
/// Interface that apps can implement to be notified when a certificate is selected or removed.
/// </summary>
public interface ICertificatesObserver
{
/// <summary>
/// Called when a certificate is selected or removed.
/// </summary>
/// <param name="e"></param>
public void OnClientCertificateChanged(CertificateChangeEventArg e);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ public ServiceCollection Services
{
get
{
if (ServiceProvider != null)
{
throw new InvalidOperationException("Cannot change services once you called Build()");
}
return _services;
}

Expand Down
57 changes: 50 additions & 7 deletions src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@
using System.Linq;
using System.Net.Http;
using System.Security.Claims;
using System.Security.Cryptography.X509Certificates;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Identity.Abstractions;
using Microsoft.Identity.Client;
Expand All @@ -21,6 +23,7 @@
using Microsoft.Identity.Web.TokenCacheProviders.InMemory;
using Microsoft.IdentityModel.JsonWebTokens;
using Microsoft.IdentityModel.Tokens;
using Microsoft.Identity.Web.Experimental;

namespace Microsoft.Identity.Web
{
Expand Down Expand Up @@ -53,6 +56,7 @@ class OAuthConstants
protected readonly IServiceProvider _serviceProvider;
protected readonly ITokenAcquisitionHost _tokenAcquisitionHost;
protected readonly ICredentialsLoader _credentialsLoader;
protected readonly ICertificatesObserver? _certificatesObserver;

/// <summary>
/// Scopes which are already requested by MSAL.NET. They should not be re-requested;.
Expand Down Expand Up @@ -99,6 +103,7 @@ public TokenAcquisition(
_serviceProvider = serviceProvider;
_tokenAcquisitionHost = tokenAcquisitionHost;
_credentialsLoader = credentialsLoader;
_certificatesObserver = serviceProvider.GetService<ICertificatesObserver>();
}

#if NET6_0_OR_GREATER
Expand All @@ -110,9 +115,10 @@ public async Task<AcquireTokenResult> AddAccountToCacheFromAuthorizationCodeAsyn
_ = Throws.IfNull(authCodeRedemptionParameters.Scopes);
MergedOptions mergedOptions = _tokenAcquisitionHost.GetOptions(authCodeRedemptionParameters.AuthenticationScheme, out string effectiveAuthenticationScheme);

IConfidentialClientApplication? application=null;
try
{
var application = GetOrBuildConfidentialClientApplication(mergedOptions);
application = GetOrBuildConfidentialClientApplication(mergedOptions);

// Do not share the access token with ASP.NET Core otherwise ASP.NET will cache it and will not send the OAuth 2.0 request in
// case a further call to AcquireTokenByAuthorizationCodeAsync in the future is required for incremental consent (getting a code requesting more scopes)
Expand Down Expand Up @@ -171,7 +177,8 @@ public async Task<AcquireTokenResult> AddAccountToCacheFromAuthorizationCodeAsyn
}
catch (MsalServiceException exMsal) when (IsInvalidClientCertificateOrSignedAssertionError(exMsal))
{
DefaultCertificateLoader.ResetCertificates(mergedOptions.ClientCertificates);
NotifyCertificateSelection(mergedOptions, application!, CerticateObserverAction.Deselected);
DefaultCertificateLoader.ResetCertificates(mergedOptions.ClientCredentials);
_applicationsByAuthorityClientId[GetApplicationKey(mergedOptions)] = null;

// Retry
Expand Down Expand Up @@ -267,7 +274,8 @@ public async Task<AuthenticationResult> GetAuthenticationResultForUserAsync(
}
catch (MsalServiceException exMsal) when (IsInvalidClientCertificateOrSignedAssertionError(exMsal))
{
DefaultCertificateLoader.ResetCertificates(mergedOptions.ClientCertificates);
NotifyCertificateSelection(mergedOptions, application, CerticateObserverAction.Deselected);
DefaultCertificateLoader.ResetCertificates(mergedOptions.ClientCredentials);
_applicationsByAuthorityClientId[GetApplicationKey(mergedOptions)] = null;

// Retry
Expand Down Expand Up @@ -325,7 +333,7 @@ private void LogAuthResult(AuthenticationResult? authenticationResult)
/// for multi tenant apps or daemons.</param>
/// <param name="tokenAcquisitionOptions">Options passed-in to create the token acquisition object which calls into MSAL .NET.</param>
/// <returns>An authentication result for the app itself, based on its scopes.</returns>
public Task<AuthenticationResult> GetAuthenticationResultForAppAsync(
public async Task<AuthenticationResult> GetAuthenticationResultForAppAsync(
string scope,
string? authenticationScheme = null,
string? tenant = null,
Expand Down Expand Up @@ -415,21 +423,29 @@ public Task<AuthenticationResult> GetAuthenticationResultForAppAsync(

try
{
return builder.ExecuteAsync(tokenAcquisitionOptions != null ? tokenAcquisitionOptions.CancellationToken : CancellationToken.None);
return await builder.ExecuteAsync(tokenAcquisitionOptions != null ? tokenAcquisitionOptions.CancellationToken : CancellationToken.None);
}
catch (MsalServiceException exMsal) when (IsInvalidClientCertificateOrSignedAssertionError(exMsal))
{
DefaultCertificateLoader.ResetCertificates(mergedOptions.ClientCertificates);
NotifyCertificateSelection(mergedOptions, application, CerticateObserverAction.Deselected);
DefaultCertificateLoader.ResetCertificates(mergedOptions.ClientCredentials);
_applicationsByAuthorityClientId[GetApplicationKey(mergedOptions)] = null;

// Retry
_retryClientCertificate = true;
return GetAuthenticationResultForAppAsync(
return await GetAuthenticationResultForAppAsync(
scope,
authenticationScheme: authenticationScheme,
tenant: tenant,
tokenAcquisitionOptions: tokenAcquisitionOptions);
}
catch (MsalException ex)
{
// GetAuthenticationResultForAppAsync is an abstraction that can be called from
// a web app or a web API
Logger.TokenAcquisitionError(_logger, ex.Message, ex);
throw;
}
finally
{
_retryClientCertificate = false;
Expand Down Expand Up @@ -635,6 +651,10 @@ private IConfidentialClientApplication BuildConfidentialClientApplication(Merged

IConfidentialClientApplication app = builder.Build();

// If the client application has set certificate observer,
// fire the event to notify the client app that a certificate was selected.
NotifyCertificateSelection(mergedOptions, app, CerticateObserverAction.Selected);

// Initialize token cache providers
if (!(_tokenCacheProvider is MsalMemoryTokenCacheProvider))
{
Expand All @@ -654,6 +674,29 @@ private IConfidentialClientApplication BuildConfidentialClientApplication(Merged
}
}

/// <summary>
/// Find the certificate used by the app and fire the event to notify the client app that a certificate was selected/unselected.
/// </summary>
/// <param name="mergedOptions"></param>
/// <param name="app"></param>
/// <param name="action"></param>
private void NotifyCertificateSelection(MergedOptions mergedOptions, IConfidentialClientApplication app, CerticateObserverAction action)
{
X509Certificate2 selectedCertificate = app.AppConfig.ClientCredentialCertificate;
if (_certificatesObserver != null
&& selectedCertificate != null)
{
_certificatesObserver.OnClientCertificateChanged(
new CertificateChangeEventArg()
{
Action = action,
Certificate = app.AppConfig.ClientCredentialCertificate,
CredentialDescription = mergedOptions.ClientCredentials?.FirstOrDefault(c => c.Certificate == selectedCertificate)
});
;
}
}

private async Task<AuthenticationResult?> GetAuthenticationResultForWebApiToCallDownstreamApiAsync(
IConfidentialClientApplication application,
string? tenantId,
Expand Down
14 changes: 7 additions & 7 deletions tests/DevApps/aspnet-mvc/OwinWebApi/Web.config
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="System.IdentityModel.Tokens.Jwt" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.0.0" newVersion="6.32.0.0"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.3.0" newVersion="6.32.3.0"/>
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="System.Diagnostics.DiagnosticSource" publicKeyToken="CC7B13FFCD2DDD51" culture="neutral"/>
Expand All @@ -74,31 +74,31 @@
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="Microsoft.IdentityModel.Tokens" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.0.0" newVersion="6.32.0.0"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.3.0" newVersion="6.32.3.0"/>
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="Microsoft.IdentityModel.Protocols.WsFederation" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-5.5.0.0" newVersion="5.5.0.0"/>
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="Microsoft.IdentityModel.Protocols.OpenIdConnect" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.0.0" newVersion="6.32.0.0"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.3.0" newVersion="6.32.3.0"/>
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="Microsoft.IdentityModel.Protocols" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.0.0" newVersion="6.32.0.0"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.3.0" newVersion="6.32.3.0"/>
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="Microsoft.IdentityModel.Logging" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.0.0" newVersion="6.32.0.0"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.3.0" newVersion="6.32.3.0"/>
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="Microsoft.IdentityModel.Abstractions" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.0.0" newVersion="6.32.0.0"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.3.0" newVersion="6.32.3.0"/>
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="Microsoft.Identity.Client" publicKeyToken="0A613F4DD989E8AE" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-4.55.0.0" newVersion="4.55.0.0"/>
<bindingRedirect oldVersion="0.0.0.0-4.56.0.0" newVersion="4.56.0.0"/>
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="Microsoft.Extensions.Primitives" publicKeyToken="ADB9793829DDAE60" culture="neutral"/>
Expand Down
14 changes: 7 additions & 7 deletions tests/DevApps/aspnet-mvc/OwinWebApp/Web.config
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="System.IdentityModel.Tokens.Jwt" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.0.0" newVersion="6.32.0.0"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.3.0" newVersion="6.32.3.0"/>
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="System.Diagnostics.DiagnosticSource" publicKeyToken="CC7B13FFCD2DDD51" culture="neutral"/>
Expand All @@ -75,31 +75,31 @@
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="Microsoft.IdentityModel.Tokens" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.0.0" newVersion="6.32.0.0"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.3.0" newVersion="6.32.3.0"/>
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="Microsoft.IdentityModel.Protocols.WsFederation" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-5.5.0.0" newVersion="5.5.0.0"/>
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="Microsoft.IdentityModel.Protocols.OpenIdConnect" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.0.0" newVersion="6.32.0.0"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.3.0" newVersion="6.32.3.0"/>
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="Microsoft.IdentityModel.Protocols" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.0.0" newVersion="6.32.0.0"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.3.0" newVersion="6.32.3.0"/>
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="Microsoft.IdentityModel.Logging" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.0.0" newVersion="6.32.0.0"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.3.0" newVersion="6.32.3.0"/>
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="Microsoft.IdentityModel.Abstractions" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.0.0" newVersion="6.32.0.0"/>
<bindingRedirect oldVersion="0.0.0.0-6.32.3.0" newVersion="6.32.3.0"/>
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="Microsoft.Identity.Client" publicKeyToken="0A613F4DD989E8AE" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-4.55.0.0" newVersion="4.55.0.0"/>
<bindingRedirect oldVersion="0.0.0.0-4.56.0.0" newVersion="4.56.0.0"/>
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="Microsoft.Extensions.Primitives" publicKeyToken="ADB9793829DDAE60" culture="neutral"/>
Expand Down
Loading

0 comments on commit 826ff82

Please sign in to comment.