[Feature Request] Region auto enable on env variable

[bgavrilMS]

Region auto-enable

1. On creation of a ConfidentialClientApplication, MSAL shall detect env variable MSAL_FORCE_REGION, which will be set to a specific region (e.g. westus1)
2. If this env variable is set, MSAL shall opt-in to ESTS-R with the value of this variable.

Disabling the behavior

If region is explicitly set in MSAL to the string "DisableMsalForceRegion`, then the above no longer applies

Conflicts with WithAzureRegion(xyz)

Use of the api WithRegion(xyz) takes precedence over the env variable.

Acceptance tests

For all of the tests, assume env variable MSAL_FORCE_REGION=eastus

1. No other config is used. ACTUAL region used: eastus
2. App developer configures region "westus" in MSAL. ACTUAL region used: westus
3. App developer configures region "DisableMsalForceRegion" in MSAL. ACTUAL region used: none

Alternatives

No response

[weinong]

It already does, no?
https://review.learn.microsoft.com/en-us/identity/microsoft-identity-platform/msal-net-regional-adoption?branch=main#providing-a-region-with-azureidentity

[bgavrilMS]

It already does, no? https://review.learn.microsoft.com/en-us/identity/microsoft-identity-platform/msal-net-regional-adoption?branch=main#providing-a-region-with-azureidentity

No. Today app developers must explicitly opt-in to ESTS-R by using the API WithAzureRegion(string). They can either inject the region name directly, or they can use "TryAutoDetect" and MSAL has an algorithm to detect the region.

This proposal is for MSAL to auto-detect an env var, eliminating the need for app developer to use WithAzureRegion API.

[rayluo]

This proposal is for MSAL to auto-detect an env var, eliminating the need for app developer to use WithAzureRegion API.

This line shall be inserted into the very beginning of this feature description as a "Problem Statement" or a "WHY". :-)

Disabling the behavior

If region is explicitly set in MSAL to the string "DisableMsalForceRegion`, then the above no longer applies

Why do we need this special case? Wouldn't it be much more intuitive (and easier to implement) to do "if the env var MSAL_FORCE_REGION is absent or empty, then the above no longer applies"?

[bgavrilMS]

This proposal is for MSAL to auto-detect an env var, eliminating the need for app developer to use WithAzureRegion API.

This line shall be inserted into the very beginning of this feature description as a "Problem Statement" or a "WHY". :-)

Disabling the behavior

If region is explicitly set in MSAL to the string "DisableMsalForceRegion`, then the above no longer applies

Why do we need this special case? Wouldn't it be much more intuitive (and easier to implement) to do "if the env var MSAL_FORCE_REGION is absent or empty, then the above no longer applies"?

Of course, if the env var is empty, the above no longer applies.

However, the "DisableMsalForceRegion" gives the application an opt-out of this behavior in case things go wrong. App owner isn't always able to change env variables. Does this make sense?

[rayluo]

However, the "DisableMsalForceRegion" gives the application an opt-out of this behavior in case things go wrong. App owner isn't always able to change env variables. Does this make sense?

Oh, I see. I misunderstood it as MSAL_FORCE_REGION=DisableMsalForceRegion, but what you meant was .WithRegion("DisableMsalForceRegion").

However, with that new understanding, now I have a broader question. The introduce of a new env var seems complicated.

.WithRegion(...)'s value	Env Var REGION_NAME's value	Env var MSAL_FORCE_REGION's value	Actual region to be used
.WithRegion("westus")	*	*	"westus"
.WithRegion("TryAutoDetect")	"eastus"	"europe"???	MSAL currently uses "eastus". But the new MSAL_FORCE_REGION makes it ambiguous. Shouldn't MSAL also honor MSAL_FORCE_REGION?
.WithRegion("TryAutoDetect")	Empty or absent	???	MSAL used to (and currently still?) performs a probe on the VM's imds endpoint which might hang. Do we also detect MSAL_FORCE_REGION now? Or just switch to use MSAL_FORCE_REGION only?
Absent	Ignored	"europe"	MSAL used to disable region behavior when .WithRegion() is absent. The current proposal will use "europe", which is fine.
.WithRegion("DisableMsalForceRegion")	"eastus"???	*	Shall this new .WithRegion("DisableMsalForceRegion") still honor env var REGION_NAME which is not disabled? Or shall we change it to .WithRegion("DisableRegion") to mimic the old ".WithRegion() is absent" behavior?

How about we consolidate into only one env var, preferably reusing the same "REGION_NAME" one, and adjust the behaviors as below?

.WithRegion(...)'s value	Env Var REGION_NAME's value	Actual region to be used
.WithRegion("westus")	*	"westus"
.WithRegion("TryAutoDetect")	"eastus"	MSAL currently uses "eastus".
.WithRegion("TryAutoDetect")	Empty or absent	MSAL used to (and currently still?) performs a probe on the VM's imds endpoint which might hang. We can remove this probe this time.
Absent	*	MSAL used to disable region behavior when .WithRegion() is absent. We may switch its behavior to be the same as TryAutoDetect, so that we meet the goal of this proposal: for MSAL to auto-detect an env var, eliminating the need for app developer to use WithAzureRegion API
.WithRegion("DisableRegion")	*	Turn off region. Same as the previous behavior of .WithRegion() being absent

[bgavrilMS]

Programatic use of WithAzureRegion, even with TryAutoDetect, overrides the env variable.

We should not try to reuse REGION_NAME env variable, because that will force ESTS-R on apps who currently use ESTS, with possible unintended consequences (e.g. 3p can't use ESTS-R, some 1p who have strong reasons to not use ESTS-R etc.). REGION_NAME is set on azure functions.

[bgavrilMS]

.WithRegion(...)'s value	Env Var REGION_NAME's value	Env var MSAL_FORCE_REGION's value	Actual region to be used
.WithRegion("westus")	*	*	"westus"
.WithRegion("TryAutoDetect")	"eastus"	"europe"	Ignore MSAL_FORCE_REGION. Use existing auto-detection logic, which will return 'eastus"
.WithRegion("TryAutoDetect")	Empty or absent	*	Ignore MSAL_FORCE_REGION. Use existing auto-detection logic, which will probably fail. On failure, MSAL uses ESTS instead of ESTS-R.
Absent	*	"europe"	MSAL used to disable region behavior when .WithRegion() is absent. The current proposal will use "europe", which is fine.
.WithRegion("DisableMsalForceRegion")	"eastus"	*	Use ESTS, not ESTS-R

[rayluo]

OK. One minor suggestion is to change the last .WithRegion("DisableMsalForceRegion") to something generic (for example, .WithRegion("NoRegion") or .WithRegion("False")), because that setting does not just disable the new env var MSAL_FORCE_REGION, it also bypass the env var REGION_NAME.

[bgavrilMS]

OK. One minor suggestion is to change the last .WithRegion("DisableMsalForceRegion") to something generic (for example, .WithRegion("NoRegion") or .WithRegion("False")), because that setting does not just disable the new env var MSAL_FORCE_REGION, it also bypass the env var REGION_NAME.

Let's please leave it as is, because we already implemented it in Node and .NET.