From cd459f615e796ca762a3012e6a46487c830e2a33 Mon Sep 17 00:00:00 2001 From: Bogdan Gavril Date: Fri, 18 Oct 2024 09:39:29 -0700 Subject: [PATCH] Opt-in to regional when env variable MSAL_FORCE_REGION is set (#4954) Co-authored-by: Gladwin Johnson <90415114+gladjohn@users.noreply.github.com> --- .../ConfidentialClientApplicationBuilder.cs | 23 ++++- .../ClientCredentialWithRegionTests.cs | 95 ++++++++++++++++++- 2 files changed, 112 insertions(+), 6 deletions(-) diff --git a/src/client/Microsoft.Identity.Client/AppConfig/ConfidentialClientApplicationBuilder.cs b/src/client/Microsoft.Identity.Client/AppConfig/ConfidentialClientApplicationBuilder.cs index 21a24ae8ba..78606f8282 100644 --- a/src/client/Microsoft.Identity.Client/AppConfig/ConfidentialClientApplicationBuilder.cs +++ b/src/client/Microsoft.Identity.Client/AppConfig/ConfidentialClientApplicationBuilder.cs @@ -9,12 +9,9 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.Identity.Client.AppConfig; -using Microsoft.Identity.Client.Extensibility; using Microsoft.Identity.Client.Instance; using Microsoft.Identity.Client.Internal; using Microsoft.Identity.Client.Internal.ClientCredential; -using Microsoft.Identity.Client.TelemetryCore; -using Microsoft.Identity.Client.TelemetryCore.TelemetryClient; using Microsoft.IdentityModel.Abstractions; namespace Microsoft.Identity.Client @@ -26,6 +23,9 @@ namespace Microsoft.Identity.Client #endif public class ConfidentialClientApplicationBuilder : AbstractApplicationBuilder { + internal const string ForceRegionEnvVariable = "MSAL_FORCE_REGION"; + internal const string DisableForceRegion = "DisableMsalForceRegion"; + /// internal ConfidentialClientApplicationBuilder(ApplicationConfiguration configuration) : base(configuration) @@ -380,10 +380,25 @@ internal override void Validate() throw new InvalidOperationException(MsalErrorMessage.InvalidRedirectUriReceived(Config.RedirectUri)); } - if (!string.IsNullOrEmpty(Config.AzureRegion) && (Config.CustomInstanceDiscoveryMetadata != null || Config.CustomInstanceDiscoveryMetadataUri != null)) + if (!string.IsNullOrEmpty(Config.AzureRegion) && + (Config.CustomInstanceDiscoveryMetadata != null || Config.CustomInstanceDiscoveryMetadataUri != null)) { throw new MsalClientException(MsalError.RegionDiscoveryWithCustomInstanceMetadata, MsalErrorMessage.RegionDiscoveryWithCustomInstanceMetadata); } + + // use regional if MSAL_FORCE_REGION is used, as per #4930 + if (string.Equals(Config.AzureRegion, DisableForceRegion, StringComparison.OrdinalIgnoreCase)) + { + Config.AzureRegion = null; + } + else if (Config.AzureRegion == null) + { + string forcedRegion = Environment.GetEnvironmentVariable(ForceRegionEnvVariable); + if (!string.IsNullOrEmpty(forcedRegion)) + { + Config.AzureRegion = forcedRegion; + } + } } /// diff --git a/tests/Microsoft.Identity.Test.Unit/PublicApiTests/ClientCredentialWithRegionTests.cs b/tests/Microsoft.Identity.Test.Unit/PublicApiTests/ClientCredentialWithRegionTests.cs index 4673b72003..52a3df0cfa 100644 --- a/tests/Microsoft.Identity.Test.Unit/PublicApiTests/ClientCredentialWithRegionTests.cs +++ b/tests/Microsoft.Identity.Test.Unit/PublicApiTests/ClientCredentialWithRegionTests.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -#if !ANDROID && !iOS using System; using System.IO; using System.Net; @@ -304,6 +303,99 @@ public async Task RegionFallbackToGlobalAsync() } } + [TestMethod] + public async Task MsalForceRegionIsSet_RegionIsUsed() + { + using (new EnvVariableContext()) + using (var httpManager = new MockHttpManager()) + { + Environment.SetEnvironmentVariable( + ConfidentialClientApplicationBuilder.ForceRegionEnvVariable, TestConstants.Region); + + httpManager.AddRegionDiscoveryMockHandler(TestConstants.Region); + httpManager.AddMockHandler(CreateTokenResponseHttpHandler(expectRegional: true)); + + var cca = ConfidentialClientApplicationBuilder + .Create(TestConstants.ClientId) + .WithHttpManager(httpManager) + .WithClientSecret(TestConstants.ClientSecret) + .Build(); + + AuthenticationResult result = await cca + .AcquireTokenForClient(TestConstants.s_scope) + .ExecuteAsync() + .ConfigureAwait(false); + + Assert.AreEqual(TestConstants.Region, result.ApiEvent.RegionUsed); + Assert.AreEqual(RegionAutodetectionSource.Imds, result.ApiEvent.RegionAutodetectionSource); + + Assert.AreEqual(TestConstants.Region, result.AuthenticationResultMetadata.RegionDetails.RegionUsed); + Assert.AreEqual(RegionOutcome.UserProvidedValid, result.AuthenticationResultMetadata.RegionDetails.RegionOutcome); + Assert.IsNull(result.AuthenticationResultMetadata.RegionDetails.AutoDetectionError); + } + } + + [TestMethod] + public async Task MsalForceRegionIsSet_WithRegionIsSet_WithRegionWins() + { + using (new EnvVariableContext()) + using (var httpManager = new MockHttpManager()) + { + // this will be ignored, in favor of TestConstants.Region + Environment.SetEnvironmentVariable( + ConfidentialClientApplicationBuilder.ForceRegionEnvVariable, "someOtherRegion"); + + httpManager.AddRegionDiscoveryMockHandler(TestConstants.Region); + httpManager.AddMockHandler(CreateTokenResponseHttpHandler(expectRegional: true)); + + var cca = ConfidentialClientApplicationBuilder + .Create(TestConstants.ClientId) + .WithHttpManager(httpManager) + .WithAzureRegion(TestConstants.Region) + .WithClientSecret(TestConstants.ClientSecret) + .Build(); + + AuthenticationResult result = await cca + .AcquireTokenForClient(TestConstants.s_scope) + .ExecuteAsync() + .ConfigureAwait(false); + + Assert.AreEqual(TestConstants.Region, result.ApiEvent.RegionUsed); + Assert.AreEqual(RegionAutodetectionSource.Imds, result.ApiEvent.RegionAutodetectionSource); + + Assert.AreEqual(TestConstants.Region, result.AuthenticationResultMetadata.RegionDetails.RegionUsed); + Assert.AreEqual(RegionOutcome.UserProvidedValid, result.AuthenticationResultMetadata.RegionDetails.RegionOutcome); + Assert.IsNull(result.AuthenticationResultMetadata.RegionDetails.AutoDetectionError); + } + } + + [TestMethod] + public async Task MsalForceRegionIsSet_WithRegionIsSetToOptOut_NoRegionIsUsed() + { + using (new EnvVariableContext()) + using (var httpManager = new MockHttpManager()) + { + Environment.SetEnvironmentVariable( + ConfidentialClientApplicationBuilder.ForceRegionEnvVariable, TestConstants.Region); + + httpManager.AddInstanceDiscoveryMockHandler(); + httpManager.AddMockHandler(CreateTokenResponseHttpHandler(expectRegional: false)); + + var cca = ConfidentialClientApplicationBuilder + .Create(TestConstants.ClientId) + .WithHttpManager(httpManager) + .WithAzureRegion(ConfidentialClientApplicationBuilder.DisableForceRegion) + .WithClientSecret(TestConstants.ClientSecret) + .Build(); + + AuthenticationResult result = await cca + .AcquireTokenForClient(TestConstants.s_scope) + .ExecuteAsync() + .ConfigureAwait(false); + + Assert.IsNull(result.ApiEvent.RegionUsed); + } + } [TestMethod] public void WithAzureRegionThrowsOnNullArg() { @@ -724,4 +816,3 @@ private static HttpResponseMessage CreateResponse(bool clientCredentialFlow) } } -#endif