Open bgavrilMS opened 3 weeks ago
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.
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"?
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?
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 |
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.
.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 |
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.
Region auto-enable
MSAL_FORCE_REGION
, which will be set to a specific region (e.g.westus1
)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
Alternatives
No response