From 462d177e70836b51addc9d39ffce1403413d7830 Mon Sep 17 00:00:00 2001 From: Nilesh Choudhary Date: Wed, 30 Oct 2024 18:00:56 +0000 Subject: [PATCH 1/3] Added Region auto enable Added Region auto enable in confidential client and its test --- apps/confidential/confidential.go | 9 +++- apps/confidential/confidential_test.go | 70 ++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/apps/confidential/confidential.go b/apps/confidential/confidential.go index f8628605..0b703282 100644 --- a/apps/confidential/confidential.go +++ b/apps/confidential/confidential.go @@ -18,6 +18,8 @@ import ( "encoding/pem" "errors" "fmt" + "os" + "strings" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/cache" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/base" @@ -315,16 +317,21 @@ func New(authority, clientID string, cred Credential, options ...Option) (Client if err != nil { return Client{}, err } - + region := os.Getenv("MSAL_FORCE_REGION") opts := clientOptions{ authority: authority, // if the caller specified a token provider, it will handle all details of authentication, using Client only as a token cache disableInstanceDiscovery: cred.tokenProvider != nil, httpClient: shared.DefaultClient, + azureRegion: region, } for _, o := range options { o(&opts) } + if strings.EqualFold(opts.azureRegion, "DisableMsalForceRegion") { + opts.azureRegion = "" + } + baseOpts := []base.Option{ base.WithCacheAccessor(opts.accessor), base.WithClientCapabilities(opts.capabilities), diff --git a/apps/confidential/confidential_test.go b/apps/confidential/confidential_test.go index 28bad83e..7c635266 100644 --- a/apps/confidential/confidential_test.go +++ b/apps/confidential/confidential_test.go @@ -164,6 +164,76 @@ func TestAcquireTokenByCredential(t *testing.T) { } } +func TestRegionAutoEnable(t *testing.T) { + cred, err := NewCredFromSecret(fakeSecret) + if err != nil { + t.Fatal(err) + } + tests := []struct { + region string + envRegion string + }{ + { + region: "", + envRegion: "envRegion", + }, + { + region: "region", + envRegion: "envRegion", + }, + { + region: "DisableMsalForceRegion", + envRegion: "envRegion", + }, + } + + for _, test := range tests { + lmo := "login.microsoftonline.com" + tenant := "tenant" + mockClient := mock.Client{} + if test.envRegion != "" { + err := os.Setenv("MSAL_FORCE_REGION", test.envRegion) + if err != nil { + t.Fatal(err) + } + } + var client Client + if test.region != "" { + client, err = New(fmt.Sprintf(authorityFmt, lmo, tenant), fakeClientID, cred, WithHTTPClient(&mockClient), WithAzureRegion(test.region)) + if err != nil { + t.Fatal(err) + } + } else { + client, err = New(fmt.Sprintf(authorityFmt, lmo, tenant), fakeClientID, cred, WithHTTPClient(&mockClient)) + if err != nil { + t.Fatal(err) + } + } + + t.Cleanup(func() { + os.Unsetenv("MSAL_FORCE_REGION") + }) + if test.region == "" { + if test.envRegion != "" { + if client.base.AuthParams.AuthorityInfo.Region != test.envRegion { + t.Fatalf("wanted %q, got %q", test.envRegion, client.base.AuthParams.AuthorityInfo.Region) + } + } + } else { + if test.region == "DisableMsalForceRegion" { + if client.base.AuthParams.AuthorityInfo.Region != "" { + t.Fatalf("wanted empty, got %q", client.base.AuthParams.AuthorityInfo.Region) + } + } else { + + if client.base.AuthParams.AuthorityInfo.Region != test.region { + t.Fatalf("wanted %q, got %q", test.region, client.base.AuthParams.AuthorityInfo.Region) + } + } + } + } +} + func TestAcquireTokenOnBehalfOf(t *testing.T) { // this test is an offline version of TestOnBehalfOf in integration_test.go cred, err := NewCredFromSecret(fakeSecret) From b790013cd9e8fe639584d041e3e6b7a49d7713d0 Mon Sep 17 00:00:00 2001 From: Nilesh Choudhary Date: Thu, 31 Oct 2024 10:41:07 +0000 Subject: [PATCH 2/3] Separated test --- apps/confidential/confidential_test.go | 121 +++++++++++++------------ 1 file changed, 62 insertions(+), 59 deletions(-) diff --git a/apps/confidential/confidential_test.go b/apps/confidential/confidential_test.go index 7c635266..af797b51 100644 --- a/apps/confidential/confidential_test.go +++ b/apps/confidential/confidential_test.go @@ -164,73 +164,76 @@ func TestAcquireTokenByCredential(t *testing.T) { } } -func TestRegionAutoEnable(t *testing.T) { +func TestRegionAutoEnable_EmptyRegion_EnvRegion(t *testing.T) { cred, err := NewCredFromSecret(fakeSecret) if err != nil { t.Fatal(err) } - tests := []struct { - region string - envRegion string - }{ - { - region: "", - envRegion: "envRegion", - }, - { - region: "region", - envRegion: "envRegion", - }, - { - region: "DisableMsalForceRegion", - envRegion: "envRegion", - }, + + envRegion := "envRegion" + err = os.Setenv("MSAL_FORCE_REGION", envRegion) + if err != nil { + t.Fatal(err) } + defer os.Unsetenv("MSAL_FORCE_REGION") - for _, test := range tests { - lmo := "login.microsoftonline.com" - tenant := "tenant" - mockClient := mock.Client{} - if test.envRegion != "" { - err := os.Setenv("MSAL_FORCE_REGION", test.envRegion) - if err != nil { - t.Fatal(err) - } - } - var client Client - if test.region != "" { - client, err = New(fmt.Sprintf(authorityFmt, lmo, tenant), fakeClientID, cred, WithHTTPClient(&mockClient), WithAzureRegion(test.region)) - if err != nil { - t.Fatal(err) - } - } else { - client, err = New(fmt.Sprintf(authorityFmt, lmo, tenant), fakeClientID, cred, WithHTTPClient(&mockClient)) - if err != nil { - t.Fatal(err) - } - } + lmo := "login.microsoftonline.com" + tenant := "tenant" + mockClient := mock.Client{} + client, err := New(fmt.Sprintf(authorityFmt, lmo, tenant), fakeClientID, cred, WithHTTPClient(&mockClient)) + if err != nil { + t.Fatal(err) + } - t.Cleanup(func() { - os.Unsetenv("MSAL_FORCE_REGION") - }) - if test.region == "" { - if test.envRegion != "" { - if client.base.AuthParams.AuthorityInfo.Region != test.envRegion { - t.Fatalf("wanted %q, got %q", test.envRegion, client.base.AuthParams.AuthorityInfo.Region) - } - } - } else { - if test.region == "DisableMsalForceRegion" { - if client.base.AuthParams.AuthorityInfo.Region != "" { - t.Fatalf("wanted empty, got %q", client.base.AuthParams.AuthorityInfo.Region) - } - } else { + if client.base.AuthParams.AuthorityInfo.Region != envRegion { + t.Fatalf("wanted %q, got %q", envRegion, client.base.AuthParams.AuthorityInfo.Region) + } +} - if client.base.AuthParams.AuthorityInfo.Region != test.region { - t.Fatalf("wanted %q, got %q", test.region, client.base.AuthParams.AuthorityInfo.Region) - } - } - } +func TestRegionAutoEnable_SpecifiedRegion_EnvRegion(t *testing.T) { + cred, err := NewCredFromSecret(fakeSecret) + if err != nil { + t.Fatal(err) + } + + envRegion := "envRegion" + err = os.Setenv("MSAL_FORCE_REGION", envRegion) + if err != nil { + t.Fatal(err) + } + defer os.Unsetenv("MSAL_FORCE_REGION") + + lmo := "login.microsoftonline.com" + tenant := "tenant" + mockClient := mock.Client{} + testRegion := "region" + client, err := New(fmt.Sprintf(authorityFmt, lmo, tenant), fakeClientID, cred, WithHTTPClient(&mockClient), WithAzureRegion(testRegion)) + if err != nil { + t.Fatal(err) + } + + if client.base.AuthParams.AuthorityInfo.Region != testRegion { + t.Fatalf("wanted %q, got %q", testRegion, client.base.AuthParams.AuthorityInfo.Region) + } +} + +func TestRegionAutoEnable_DisableMsalForceRegion(t *testing.T) { + cred, err := NewCredFromSecret(fakeSecret) + if err != nil { + t.Fatal(err) + } + + lmo := "login.microsoftonline.com" + tenant := "tenant" + mockClient := mock.Client{} + testRegion := "DisableMsalForceRegion" + client, err := New(fmt.Sprintf(authorityFmt, lmo, tenant), fakeClientID, cred, WithHTTPClient(&mockClient), WithAzureRegion(testRegion)) + if err != nil { + t.Fatal(err) + } + + if client.base.AuthParams.AuthorityInfo.Region != "" { + t.Fatalf("wanted empty, got %q", client.base.AuthParams.AuthorityInfo.Region) } } From efa66ec6d7f60aae45ae721ea5bdf7e7eaa56c0d Mon Sep 17 00:00:00 2001 From: Nilesh Choudhary Date: Thu, 31 Oct 2024 11:38:35 +0000 Subject: [PATCH 3/3] Updated variableName --- apps/confidential/confidential.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/confidential/confidential.go b/apps/confidential/confidential.go index 0b703282..57d0e277 100644 --- a/apps/confidential/confidential.go +++ b/apps/confidential/confidential.go @@ -317,13 +317,13 @@ func New(authority, clientID string, cred Credential, options ...Option) (Client if err != nil { return Client{}, err } - region := os.Getenv("MSAL_FORCE_REGION") + autoEnabledRegion := os.Getenv("MSAL_FORCE_REGION") opts := clientOptions{ authority: authority, // if the caller specified a token provider, it will handle all details of authentication, using Client only as a token cache disableInstanceDiscovery: cred.tokenProvider != nil, httpClient: shared.DefaultClient, - azureRegion: region, + azureRegion: autoEnabledRegion, } for _, o := range options { o(&opts)