Skip to content

Commit

Permalink
[Region] Redesign regional (#2509)
Browse files Browse the repository at this point in the history
* [Region] Public API changes

* [Region] Refactor region discovery logic

* [Region] Updated RegionalTestApp: refactor + test cases. Nit changes to logging.

* [Region] Refactored regional integration tests but still needs more work.

* [Region] Do not apply regionalization twice

* [Region] Enhanced E2E unit tests

* Update tests

* Address PR comments

Co-authored-by: pmaytak <[email protected]>
  • Loading branch information
bgavrilMS and pmaytak authored Mar 29, 2021
1 parent 98c9f7b commit 2ad5514
Show file tree
Hide file tree
Showing 33 changed files with 1,228 additions and 959 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -72,51 +72,21 @@ public AcquireTokenForClientParameterBuilder WithSendX5C(bool withSendX5C)
}

/// <summary>
/// Specifies if the token request should be sent to regional ESTS.
/// If set, MSAL tries to auto-detect and use a regional Azure authority. This helps keep the authentication traffic inside the Azure region.
/// If the region cannot be determined (e.g. not running on Azure), MSALClientException is thrown with error code region_discovery_failed.
/// This feature requires configuration at tenant level.
/// By default the value for this variable is false.
/// See https://aka.ms/msal-net-region-discovery for more details.
/// Please use WithAzureRegion on the ConfidentialClientApplicationBuilder object
/// </summary>
/// <param name="useAzureRegion"><c>true</c> if the token request should be sent to regional ESTS. The default is <c>false</c>.
/// </param>
/// <returns>The builder to chain the .With methods</returns>
[Obsolete("This method name has been changed to a more relevant name, please use WithPreferredAzureRegion instead which also includes added features.", true)]
[Obsolete("Use WithAzureRegion on the ConfidentialClientApplicationBuilder object", true)]
public AcquireTokenForClientParameterBuilder WithAzureRegion(bool useAzureRegion)
{
ValidateUseOfExpirementalFeature();

CommonParameters.AddApiTelemetryFeature(ApiTelemetryFeature.WithAzureRegion, useAzureRegion);
Parameters.AutoDetectRegion = useAzureRegion;
return this;
throw new NotImplementedException();
}

/// <summary>
/// Specifies if the token request should be sent to regional ESTS.
/// If set, MSAL tries to auto-detect and use a regional Azure authority. This helps keep the authentication traffic inside the Azure region.
/// If the region cannot be determined (e.g. not running on Azure), MSALClientException is thrown with error code region_discovery_failed.
/// This feature requires configuration at tenant level.
/// By default the value for this variable is false.
/// See https://aka.ms/msal-net-region-discovery for more details.
/// </summary>
/// <param name="useAzureRegion"><c>true</c> if the token request should be sent to regional ESTS. The default is <c>false</c>.
/// </param>
/// <param name="regionUsedIfAutoDetectFails"> optional parameter to provide region to MSAL. This parameter will be used along with auto detection of region.
/// If the region is auto detected, the provided region will be compared with the detected region and used in telemetry to do analysis on correctness of the region provided.
/// If auto region detection fails, the provided region will be used for instance metadata.</param>
/// <param name="fallbackToGlobal"><c>true</c> to fallback to global ESTS endpoint when calls to regional ESTS fail.
/// This will only happen when MSAL is not able to detect a region or if there is no provided region.</param>
/// <returns>The builder to chain the .With methods</returns>
/// Please use WithAzureRegion on the ConfidentialClientApplicationBuilder object
/// </summary>
[Obsolete("Use WithAzureRegion on the ConfidentialClientApplicationBuilder object", true)]
public AcquireTokenForClientParameterBuilder WithPreferredAzureRegion(bool useAzureRegion = true, string regionUsedIfAutoDetectFails = "", bool fallbackToGlobal = true)
{
ValidateUseOfExpirementalFeature();

CommonParameters.AddApiTelemetryFeature(ApiTelemetryFeature.WithAzureRegion, useAzureRegion);
Parameters.AutoDetectRegion = useAzureRegion;
Parameters.RegionToUse = regionUsedIfAutoDetectFails;
Parameters.FallbackToGlobal = fallbackToGlobal;
return this;
throw new NotImplementedException();
}

/// <inheritdoc />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,8 @@ public async Task<AuthenticationResult> ExecuteAsync(
requestContext,
_confidentialClientApplication.AppTokenCacheInternal);

if (clientParameters.AutoDetectRegion && requestContext.ServiceBundle.Config.AuthorityInfo.AuthorityType == AuthorityType.Adfs)
{
throw new MsalClientException(MsalError.RegionDiscoveryNotEnabled, MsalErrorMessage.RegionDiscoveryNotAvailable);
}


requestParams.SendX5C = clientParameters.SendX5C;
requestContext.ServiceBundle.Config.AuthorityInfo.AutoDetectRegion = clientParameters.AutoDetectRegion;
requestContext.ServiceBundle.Config.AuthorityInfo.RegionToUse = clientParameters.RegionToUse;
requestContext.ServiceBundle.Config.AuthorityInfo.FallbackToGlobal = clientParameters.FallbackToGlobal;

var handler = new ClientCredentialRequest(
ServiceBundle,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,12 @@ internal class AcquireTokenForClientParameters : IAcquireTokenParameters
/// </summary>
public bool SendX5C { get; set; }

/// <summary>
/// When set to true, the request is sent to regional endpoint.
/// </summary>
public bool AutoDetectRegion { get; set; }

/// <summary>
/// This field wil contain the region provided by user and will be used along with region auto detection.
/// </summary>
public string RegionToUse { get; set; }

/// <summary>
/// </summary>
public bool FallbackToGlobal { get; set; }

/// <inheritdoc />
public void LogParameters(ICoreLogger logger)
{
var builder = new StringBuilder();
builder.AppendLine("=== AcquireTokenForClientParameters ===");
builder.AppendLine("SendX5C: " + SendX5C);
builder.AppendLine("WithAzureRegion: " + AutoDetectRegion);
builder.AppendLine("RegionToUse: " + RegionToUse);
builder.AppendLine("ForceRefresh: " + ForceRefresh);
logger.Info(builder.ToString());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ public string ClientVersion

public bool LegacyCacheCompatibilityEnabled { get; internal set; } = true;

#region Region
public string AzureRegion { get; set; }
#endregion

#region Authority

public InstanceDiscoveryResponse CustomInstanceDiscoveryMetadata { get; set; }
Expand All @@ -93,32 +97,32 @@ public string ClientVersion
/// <summary>
/// Should _not_ go in the interface, only for builder usage while determining authorities with ApplicationOptions
/// </summary>
internal AadAuthorityAudience AadAuthorityAudience { get; set; }
public AadAuthorityAudience AadAuthorityAudience { get; set; }

/// <summary>
/// Should _not_ go in the interface, only for builder usage while determining authorities with ApplicationOptions
/// </summary>
internal AzureCloudInstance AzureCloudInstance { get; set; }
public AzureCloudInstance AzureCloudInstance { get; set; }

/// <summary>
/// Should _not_ go in the interface, only for builder usage while determining authorities with ApplicationOptions
/// </summary>
internal string Instance { get; set; }
public string Instance { get; set; }

/// <summary>
/// Should _not_ go in the interface, only for builder usage while determining authorities with ApplicationOptions
/// </summary>
internal bool ValidateAuthority { get; set; }
public bool ValidateAuthority { get; set; }

#endregion

#region Test Hooks
internal ILegacyCachePersistence UserTokenLegacyCachePersistenceForTest { get; set; }
public ILegacyCachePersistence UserTokenLegacyCachePersistenceForTest { get; set; }

internal ITokenCacheInternal UserTokenCacheInternalForTest { get; set; }
internal ITokenCacheInternal AppTokenCacheInternalForTest { get; set; }
public ITokenCacheInternal UserTokenCacheInternalForTest { get; set; }
public ITokenCacheInternal AppTokenCacheInternalForTest { get; set; }

internal IDeviceAuthManager DeviceAuthManagerForTest { get; set; }
public IDeviceAuthManager DeviceAuthManagerForTest { get; set; }
#endregion

}
Expand Down
20 changes: 8 additions & 12 deletions src/client/Microsoft.Identity.Client/AppConfig/AuthorityInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,18 @@ public AuthorityInfo(
}
}

private AuthorityInfo(string host, string canonicalAuthority, AuthorityType authorityType, string userRealmUriPrefix, bool validateAuthority, bool autoDetectRegion, string regionToUse, bool fallbackToGlobal)
private AuthorityInfo(
string host,
string canonicalAuthority,
AuthorityType authorityType,
string userRealmUriPrefix,
bool validateAuthority)
{
Host = host;
CanonicalAuthority = canonicalAuthority;
AuthorityType = authorityType;
UserRealmUriPrefix = userRealmUriPrefix;
ValidateAuthority = validateAuthority;
AutoDetectRegion = autoDetectRegion;
RegionToUse = regionToUse;
FallbackToGlobal = fallbackToGlobal;
ValidateAuthority = validateAuthority;
}

public AuthorityInfo(AuthorityInfo other) :
Expand All @@ -78,10 +80,7 @@ public AuthorityInfo(AuthorityInfo other) :
other.CanonicalAuthority,
other.AuthorityType,
other.UserRealmUriPrefix,
other.ValidateAuthority,
other.AutoDetectRegion,
other.RegionToUse,
other.FallbackToGlobal)
other.ValidateAuthority)
{
}

Expand All @@ -90,9 +89,6 @@ public AuthorityInfo(AuthorityInfo other) :
public AuthorityType AuthorityType { get; }
public string UserRealmUriPrefix { get; }
public bool ValidateAuthority { get; }
public bool AutoDetectRegion { get; set; }
public string RegionToUse { get; set; }
public bool FallbackToGlobal { get; set; }

#region Builders
internal static AuthorityInfo FromAuthorityUri(string authorityUri, bool validateAuthority)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ public static ConfidentialClientApplicationBuilder CreateWithApplicationOptions(
{
builder = builder.WithClientSecret(options.ClientSecret);
}

if (!string.IsNullOrWhiteSpace(options.AzureRegion))
{
builder = builder.WithAzureRegion(options.AzureRegion);
}

return builder;
}

Expand Down Expand Up @@ -174,6 +180,36 @@ public ConfidentialClientApplicationBuilder WithClientAssertion(Func<string> cli
return this;
}


/// <summary>
/// Instructs MSAL.NET to use an Azure regional token service.
/// </summary>
/// <param name="azureRegion">Either the string with the region (preferred) or
/// use <see cref="ConfidentialClientApplication.AttemptRegionDiscovery"/> and MSAL.NET will attempt to auto-detect the region.
/// </param>
/// <remarks>
/// Region names as per https://docs.microsoft.com/en-us/dotnet/api/microsoft.azure.management.resourcemanager.fluent.core.region?view=azure-dotnet.
/// Not all auth flows can use the regional token service.
/// Service To Service (client credential flow) tokens can be obtained from the regional service.
/// Requires configuration at the tenant level.
/// Auto-detection works on a limited number of Azure artifacts (VMs, Azure functions).
/// If auto-detection fails, the non-regional endpoint will be used.
/// If an invalid region name is provided, the non-regional endpoint MIGHT be used or the token request MIGHT fail.
/// See https://aka.ms/msal-net-region-discovery for more details.
/// </remarks>
/// <returns>The builder to chain the .With methods</returns>
public ConfidentialClientApplicationBuilder WithAzureRegion(string azureRegion = ConfidentialClientApplication.AttemptRegionDiscovery)
{
if (string.IsNullOrEmpty(azureRegion))
{
throw new ArgumentNullException(nameof(azureRegion));
}

Config.AzureRegion = azureRegion;

return this;
}

internal ConfidentialClientApplicationBuilder WithAppTokenCacheInternalForTest(ITokenCacheInternal tokenCacheInternal)
{
Config.AppTokenCacheInternalForTest = tokenCacheInternal;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,22 @@ public class ConfidentialClientApplicationOptions : ApplicationOptions
/// application registration with PowerShell AzureAD, PowerShell AzureRM, or Azure CLI.
/// </summary>
public string ClientSecret { get; set; }

/// <summary>
/// Instructs MSAL.NET to use an Azure regional token service.
/// This setting should be set to either the string with the region (preferred) or to
/// "TryAutoDetect" and MSAL.NET will attempt to auto-detect the region.
/// </summary>
/// <remarks>
/// Region names as per https://docs.microsoft.com/en-us/dotnet/api/microsoft.azure.management.resourcemanager.fluent.core.region?view=azure-dotnet.
/// Not all auth flows can use the regional token service.
/// Service To Service (client credential flow) tokens can be obtained from the regional service.
/// Requires configuration at the tenant level.
/// Auto-detection works on a limited number of Azure artifacts (VMs, Azure functions).
/// If auto-detection fails, the non-regional endpoint will be used.
/// If an invalid region name is provided, the non-regional endpoint MIGHT be used or the token request MIGHT fail.
/// See https://aka.ms/msal-net-region-discovery for more details.
/// </remarks>
public string AzureRegion { get; set; }
}
}
7 changes: 6 additions & 1 deletion src/client/Microsoft.Identity.Client/AuthenticationResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Globalization;
using Microsoft.Identity.Client.AuthScheme;
using Microsoft.Identity.Client.Cache.Items;
using Microsoft.Identity.Client.TelemetryCore.Internal.Events;

namespace Microsoft.Identity.Client
{
Expand Down Expand Up @@ -115,7 +116,8 @@ internal AuthenticationResult(
MsalIdTokenCacheItem msalIdTokenCacheItem,
IAuthenticationScheme authenticationScheme,
Guid correlationID,
TokenSource tokenSource)
TokenSource tokenSource,
ApiEvent apiEvent)
{
_authenticationScheme = authenticationScheme ?? throw new ArgumentNullException(nameof(authenticationScheme));
string homeAccountId =
Expand Down Expand Up @@ -154,6 +156,7 @@ internal AuthenticationResult(
TenantId = msalIdTokenCacheItem?.IdToken?.TenantId;
IdToken = msalIdTokenCacheItem?.Secret;
CorrelationId = correlationID;
ApiEvent = apiEvent;
AuthenticationResultMetadata = new AuthenticationResultMetadata(tokenSource);
}

Expand Down Expand Up @@ -225,6 +228,8 @@ internal AuthenticationResult() { }
/// </summary>
public Guid CorrelationId { get; }

internal ApiEvent ApiEvent { get; }

/// <summary>
/// Identifies the type of access token. By default tokens returned by Azure Active Directory are Bearer tokens.
/// <seealso cref="CreateAuthorizationHeader"/> for getting an HTTP authorization header from an AuthenticationResult.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ public sealed partial class ConfidentialClientApplication
IConfidentialClientApplicationWithCertificate,
IByRefreshToken
{
/// <summary>
/// Instructs MSAL to try to auto discover the Azure region.
/// </summary>
public const string AttemptRegionDiscovery = "TryAutoDetect";

internal ConfidentialClientApplication(
ApplicationConfiguration configuration)
: base(configuration)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,15 @@
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Microsoft.Identity.Client.Http;
using Microsoft.Identity.Client.Instance.Discovery;
using Microsoft.Identity.Client.Internal;

namespace Microsoft.Identity.Client.Region
{
internal interface IRegionDiscoveryProvider
{
Task<InstanceDiscoveryMetadataEntry> TryGetMetadataAsync(Uri authority, RequestContext requestContext);

void Clear();
Task<InstanceDiscoveryMetadataEntry> GetMetadataAsync(Uri authority, RequestContext requestContext);
}
}
Loading

0 comments on commit 2ad5514

Please sign in to comment.