Skip to content

Commit 2d2d761

Browse files
authored
Fix for 5223 - env var to disable ESTS-R (#5224)
* Fix for 5223 - env var to disable ESTS-R * Fix for 5223 - env var to disable ESTS-R * Address PR comments * PR * whitespace
1 parent aac4dfe commit 2d2d761

File tree

2 files changed

+100
-2
lines changed

2 files changed

+100
-2
lines changed

src/client/Microsoft.Identity.Client/AppConfig/ConfidentialClientApplicationBuilder.cs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ namespace Microsoft.Identity.Client
2424
public class ConfidentialClientApplicationBuilder : AbstractApplicationBuilder<ConfidentialClientApplicationBuilder>
2525
{
2626
internal const string ForceRegionEnvVariable = "MSAL_FORCE_REGION";
27+
internal const string DisableRegionEnvVariable = "MSAL_DISABLE_REGION";
2728
internal const string DisableForceRegion = "DisableMsalForceRegion";
2829

2930
/// <inheritdoc/>
@@ -380,8 +381,22 @@ internal override void Validate()
380381
throw new InvalidOperationException(MsalErrorMessage.InvalidRedirectUriReceived(Config.RedirectUri));
381382
}
382383

383-
if (!string.IsNullOrEmpty(Config.AzureRegion) &&
384-
(Config.CustomInstanceDiscoveryMetadata != null || Config.CustomInstanceDiscoveryMetadataUri != null))
384+
ValidateAndUpdateRegion();
385+
}
386+
387+
private void ValidateAndUpdateRegion()
388+
{
389+
// master override - do not use region if this env variable is set, as per #5223
390+
// this is needed because MSAL is used in other SDKs and it's difficult for apps to
391+
// disable ESTS-R for some calls and to enable it for others
392+
if (!string.IsNullOrEmpty(Environment.GetEnvironmentVariable(DisableRegionEnvVariable)))
393+
{
394+
Config.AzureRegion = null;
395+
return;
396+
}
397+
398+
if (!string.IsNullOrEmpty(Config.AzureRegion) &&
399+
(Config.CustomInstanceDiscoveryMetadata != null || Config.CustomInstanceDiscoveryMetadataUri != null))
385400
{
386401
throw new MsalClientException(MsalError.RegionDiscoveryWithCustomInstanceMetadata, MsalErrorMessage.RegionDiscoveryWithCustomInstanceMetadata);
387402
}

tests/Microsoft.Identity.Test.Unit/PublicApiTests/ClientCredentialWithRegionTests.cs

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,89 @@ public async Task MsalForceRegionIsSet_RegionIsUsed()
329329
}
330330
}
331331

332+
[TestMethod]
333+
public async Task MsalDisableRegionEnvVariable_API()
334+
{
335+
using (new EnvVariableContext())
336+
using (var harness = base.CreateTestHarness())
337+
{
338+
Environment.SetEnvironmentVariable(
339+
ConfidentialClientApplicationBuilder.DisableRegionEnvVariable, "1");
340+
341+
var httpManager = harness.HttpManager;
342+
343+
var cca = ConfidentialClientApplicationBuilder
344+
.Create(TestConstants.ClientId)
345+
.WithHttpManager(httpManager)
346+
.WithAzureRegion(TestConstants.Region)
347+
.WithClientSecret(TestConstants.ClientSecret)
348+
.Build();
349+
350+
await ExpectNoRegionAsync(httpManager, cca).ConfigureAwait(false);
351+
}
352+
}
353+
354+
[TestMethod]
355+
public async Task MsalDisableRegionEnvVariable_OtherEnv()
356+
{
357+
using (new EnvVariableContext())
358+
using (var harness = base.CreateTestHarness())
359+
{
360+
Environment.SetEnvironmentVariable(
361+
ConfidentialClientApplicationBuilder.ForceRegionEnvVariable, "someregion");
362+
Environment.SetEnvironmentVariable(
363+
ConfidentialClientApplicationBuilder.DisableRegionEnvVariable, "1");
364+
365+
var httpManager = harness.HttpManager;
366+
367+
var cca = ConfidentialClientApplicationBuilder
368+
.Create(TestConstants.ClientId)
369+
.WithHttpManager(httpManager)
370+
.WithClientSecret(TestConstants.ClientSecret)
371+
.Build();
372+
373+
await ExpectNoRegionAsync(httpManager, cca).ConfigureAwait(false);
374+
}
375+
}
376+
377+
[TestMethod]
378+
public async Task MsalDisableRegionEnvVariable_ApiAndOtherEnv()
379+
{
380+
using (new EnvVariableContext())
381+
using (var harness = base.CreateTestHarness())
382+
{
383+
Environment.SetEnvironmentVariable(
384+
ConfidentialClientApplicationBuilder.DisableRegionEnvVariable, "1");
385+
386+
Environment.SetEnvironmentVariable(
387+
ConfidentialClientApplicationBuilder.ForceRegionEnvVariable, "someregion");
388+
389+
var httpManager = harness.HttpManager;
390+
391+
var cca = ConfidentialClientApplicationBuilder
392+
.Create(TestConstants.ClientId)
393+
.WithHttpManager(httpManager)
394+
.WithAzureRegion(TestConstants.Region)
395+
.WithClientSecret(TestConstants.ClientSecret)
396+
.Build();
397+
398+
await ExpectNoRegionAsync(httpManager, cca).ConfigureAwait(false);
399+
}
400+
}
401+
402+
private static async Task ExpectNoRegionAsync(MockHttpManager httpManager, IConfidentialClientApplication cca)
403+
{
404+
httpManager.AddInstanceDiscoveryMockHandler();
405+
httpManager.AddMockHandler(CreateTokenResponseHttpHandler(expectRegional: false));
406+
407+
AuthenticationResult result = await cca
408+
.AcquireTokenForClient(TestConstants.s_scope)
409+
.ExecuteAsync()
410+
.ConfigureAwait(false);
411+
412+
Assert.IsNull(result.ApiEvent.RegionUsed);
413+
}
414+
332415
[TestMethod]
333416
public async Task MsalForceRegionIsSet_WithRegionIsSet_WithRegionWins()
334417
{

0 commit comments

Comments
 (0)