From f06b959f18451c39eb5930cc9d3a4f4e7fa3284c Mon Sep 17 00:00:00 2001 From: Bala Ganapathy Date: Sat, 11 Jan 2025 10:22:59 -0800 Subject: [PATCH 1/3] Adding parameter to disable instance discovery --- pkg/internal/pop/msal_confidential.go | 4 ++++ pkg/internal/pop/msal_confidential_test.go | 1 + pkg/internal/token/serviceprincipaltokencertificate.go | 1 + pkg/internal/token/serviceprincipaltokensecret.go | 1 + 4 files changed, 7 insertions(+) diff --git a/pkg/internal/pop/msal_confidential.go b/pkg/internal/pop/msal_confidential.go index 6511ac8..60fa70e 100644 --- a/pkg/internal/pop/msal_confidential.go +++ b/pkg/internal/pop/msal_confidential.go @@ -12,6 +12,7 @@ import ( // AcquirePoPTokenConfidential acquires a PoP token using MSAL's confidential login flow. // This flow does not require user interaction as the credentials for the request have // already been provided +// instanceDisovery is to be false only in disconnected clouds to disable instance discovery and authoority validation func AcquirePoPTokenConfidential( context context.Context, popClaims map[string]string, @@ -20,6 +21,7 @@ func AcquirePoPTokenConfidential( authority, clientID, tenantID string, + instanceDiscovery bool, options *azcore.ClientOptions, popKeyFunc func() (*SwKey, error), ) (string, int64, error) { @@ -43,6 +45,7 @@ func AcquirePoPTokenConfidential( cred, confidential.WithHTTPClient(options.Transport.(*http.Client)), confidential.WithX5C(), + confidential.WithInstanceDiscovery(instanceDiscovery), ) } else { client, err = confidential.New( @@ -50,6 +53,7 @@ func AcquirePoPTokenConfidential( clientID, cred, confidential.WithX5C(), + confidential.WithInstanceDiscovery(instanceDiscovery), ) } if err != nil { diff --git a/pkg/internal/pop/msal_confidential_test.go b/pkg/internal/pop/msal_confidential_test.go index 43cd670..f5be78d 100644 --- a/pkg/internal/pop/msal_confidential_test.go +++ b/pkg/internal/pop/msal_confidential_test.go @@ -115,6 +115,7 @@ func TestAcquirePoPTokenConfidential(t *testing.T) { authority, tc.p.clientID, tc.p.tenantID, + true, &clientOpts, GetSwPoPKey, ) diff --git a/pkg/internal/token/serviceprincipaltokencertificate.go b/pkg/internal/token/serviceprincipaltokencertificate.go index 148e4b3..0b2c556 100644 --- a/pkg/internal/token/serviceprincipaltokencertificate.go +++ b/pkg/internal/token/serviceprincipaltokencertificate.go @@ -90,6 +90,7 @@ func (p *servicePrincipalToken) getPoPTokenWithClientCert( p.cloud.ActiveDirectoryAuthorityHost, p.clientID, p.tenantID, + true, options, pop.GetSwPoPKey, ) diff --git a/pkg/internal/token/serviceprincipaltokensecret.go b/pkg/internal/token/serviceprincipaltokensecret.go index f58c74e..a9b3240 100644 --- a/pkg/internal/token/serviceprincipaltokensecret.go +++ b/pkg/internal/token/serviceprincipaltokensecret.go @@ -69,6 +69,7 @@ func (p *servicePrincipalToken) getPoPTokenWithClientSecret( p.cloud.ActiveDirectoryAuthorityHost, p.clientID, p.tenantID, + true, options, pop.GetSwPoPKey, ) From 00144b4287951f59f5d7dd57e8a4645525da246d Mon Sep 17 00:00:00 2001 From: Bala Ganapathy Date: Tue, 21 Jan 2025 16:13:29 -0800 Subject: [PATCH 2/3] Refactoring and consolidating params as MsalCLientOptions --- pkg/internal/pop/msal_confidential.go | 39 ++++++++++++------- pkg/internal/pop/msal_confidential_test.go | 12 +++--- .../token/serviceprincipaltokencertificate.go | 12 +++--- .../token/serviceprincipaltokensecret.go | 12 +++--- pkg/pop/types.go | 2 + 5 files changed, 47 insertions(+), 30 deletions(-) diff --git a/pkg/internal/pop/msal_confidential.go b/pkg/internal/pop/msal_confidential.go index 60fa70e..84a1049 100644 --- a/pkg/internal/pop/msal_confidential.go +++ b/pkg/internal/pop/msal_confidential.go @@ -9,6 +9,14 @@ import ( "github.com/AzureAD/microsoft-authentication-library-for-go/apps/confidential" ) +type MsalClientOptions struct { + Authority string + ClientID string + TenantID string + DisableInstanceDiscovery bool + Options *azcore.ClientOptions +} + // AcquirePoPTokenConfidential acquires a PoP token using MSAL's confidential login flow. // This flow does not require user interaction as the credentials for the request have // already been provided @@ -18,11 +26,7 @@ func AcquirePoPTokenConfidential( popClaims map[string]string, scopes []string, cred confidential.Credential, - authority, - clientID, - tenantID string, - instanceDiscovery bool, - options *azcore.ClientOptions, + msalOptions *MsalClientOptions, popKeyFunc func() (*SwKey, error), ) (string, int64, error) { if popKeyFunc == nil { @@ -38,22 +42,27 @@ func AcquirePoPTokenConfidential( PoPKey: popKey, } var client confidential.Client - if options != nil && options.Transport != nil { + + if msalOptions == nil { + return "", -1, fmt.Errorf("unable to create confidential client: msalClientOptions is empty") + } + + if msalOptions.Options != nil && msalOptions.Options.Transport != nil { client, err = confidential.New( - authority, - clientID, + msalOptions.Authority, + msalOptions.ClientID, cred, - confidential.WithHTTPClient(options.Transport.(*http.Client)), + confidential.WithHTTPClient(msalOptions.Options.Transport.(*http.Client)), confidential.WithX5C(), - confidential.WithInstanceDiscovery(instanceDiscovery), + confidential.WithInstanceDiscovery(!msalOptions.DisableInstanceDiscovery), ) } else { client, err = confidential.New( - authority, - clientID, + msalOptions.Authority, + msalOptions.ClientID, cred, confidential.WithX5C(), - confidential.WithInstanceDiscovery(instanceDiscovery), + confidential.WithInstanceDiscovery(!msalOptions.DisableInstanceDiscovery), ) } if err != nil { @@ -63,14 +72,14 @@ func AcquirePoPTokenConfidential( context, scopes, confidential.WithAuthenticationScheme(authnScheme), - confidential.WithTenantID(tenantID), + confidential.WithTenantID(msalOptions.TenantID), ) if err != nil { result, err = client.AcquireTokenByCredential( context, scopes, confidential.WithAuthenticationScheme(authnScheme), - confidential.WithTenantID(tenantID), + confidential.WithTenantID(msalOptions.TenantID), ) if err != nil { return "", -1, fmt.Errorf("failed to create service principal PoP token using secret: %w", err) diff --git a/pkg/internal/pop/msal_confidential_test.go b/pkg/internal/pop/msal_confidential_test.go index f5be78d..a2cb598 100644 --- a/pkg/internal/pop/msal_confidential_test.go +++ b/pkg/internal/pop/msal_confidential_test.go @@ -112,11 +112,13 @@ func TestAcquirePoPTokenConfidential(t *testing.T) { tc.p.popClaims, scopes, cred, - authority, - tc.p.clientID, - tc.p.tenantID, - true, - &clientOpts, + &MsalClientOptions{ + Authority: authority, + ClientID: tc.p.clientID, + TenantID: tc.p.tenantID, + Options: &clientOpts, + DisableInstanceDiscovery: false, + }, GetSwPoPKey, ) defer vcrRecorder.Stop() diff --git a/pkg/internal/token/serviceprincipaltokencertificate.go b/pkg/internal/token/serviceprincipaltokencertificate.go index 0b2c556..af7acf7 100644 --- a/pkg/internal/token/serviceprincipaltokencertificate.go +++ b/pkg/internal/token/serviceprincipaltokencertificate.go @@ -87,11 +87,13 @@ func (p *servicePrincipalToken) getPoPTokenWithClientCert( p.popClaims, scopes, cred, - p.cloud.ActiveDirectoryAuthorityHost, - p.clientID, - p.tenantID, - true, - options, + &pop.MsalClientOptions{ + Authority: p.cloud.ActiveDirectoryAuthorityHost, + ClientID: p.clientID, + TenantID: p.tenantID, + DisableInstanceDiscovery: false, + Options: options, + }, pop.GetSwPoPKey, ) if err != nil { diff --git a/pkg/internal/token/serviceprincipaltokensecret.go b/pkg/internal/token/serviceprincipaltokensecret.go index a9b3240..e4f6731 100644 --- a/pkg/internal/token/serviceprincipaltokensecret.go +++ b/pkg/internal/token/serviceprincipaltokensecret.go @@ -66,11 +66,13 @@ func (p *servicePrincipalToken) getPoPTokenWithClientSecret( p.popClaims, scopes, cred, - p.cloud.ActiveDirectoryAuthorityHost, - p.clientID, - p.tenantID, - true, - options, + &pop.MsalClientOptions{ + Authority: p.cloud.ActiveDirectoryAuthorityHost, + ClientID: p.clientID, + TenantID: p.tenantID, + DisableInstanceDiscovery: false, + Options: options, + }, pop.GetSwPoPKey, ) if err != nil { diff --git a/pkg/pop/types.go b/pkg/pop/types.go index 9793ae0..ca805a4 100644 --- a/pkg/pop/types.go +++ b/pkg/pop/types.go @@ -10,3 +10,5 @@ import ( type PoPAuthenticationScheme = pop.PoPAuthenticationScheme type SwKey = pop.SwKey + +type MsalClientOptions = pop.MsalClientOptions From 5845debfaefc352d54621e9a97cc51cfc704df81 Mon Sep 17 00:00:00 2001 From: Bala Ganapathy Date: Wed, 22 Jan 2025 12:53:01 -0800 Subject: [PATCH 3/3] Refactoring and adding the disableInstanceDiscovery option in ServicePrincipalToken --- pkg/internal/token/provider.go | 2 +- pkg/internal/token/serviceprincipaltoken.go | 35 ++++++++++--------- .../token/serviceprincipaltoken_test.go | 1 + .../token/serviceprincipaltokencertificate.go | 2 +- .../token/serviceprincipaltokensecret.go | 2 +- 5 files changed, 23 insertions(+), 19 deletions(-) diff --git a/pkg/internal/token/provider.go b/pkg/internal/token/provider.go index c9446e1..1f1b257 100644 --- a/pkg/internal/token/provider.go +++ b/pkg/internal/token/provider.go @@ -39,7 +39,7 @@ func NewTokenProvider(o *Options) (TokenProvider, error) { if o.IsLegacy { return newLegacyServicePrincipalToken(*oAuthConfig, o.ClientID, o.ClientSecret, o.ClientCert, o.ClientCertPassword, o.ServerID, o.TenantID) } - return newServicePrincipalTokenProvider(cloudConfiguration, o.ClientID, o.ClientSecret, o.ClientCert, o.ClientCertPassword, o.ServerID, o.TenantID, popClaimsMap) + return newServicePrincipalTokenProvider(cloudConfiguration, o.ClientID, o.ClientSecret, o.ClientCert, o.ClientCertPassword, o.ServerID, o.TenantID, false, popClaimsMap) case ROPCLogin: return newResourceOwnerTokenProvider(*oAuthConfig, o.ClientID, o.Username, o.Password, o.ServerID, o.TenantID, popClaimsMap) case MSILogin: diff --git a/pkg/internal/token/serviceprincipaltoken.go b/pkg/internal/token/serviceprincipaltoken.go index 5c03999..9c2e249 100644 --- a/pkg/internal/token/serviceprincipaltoken.go +++ b/pkg/internal/token/serviceprincipaltoken.go @@ -18,14 +18,15 @@ const ( ) type servicePrincipalToken struct { - clientID string - clientSecret string - clientCert string - clientCertPassword string - resourceID string - tenantID string - cloud cloud.Configuration - popClaims map[string]string + clientID string + clientSecret string + clientCert string + clientCertPassword string + resourceID string + tenantID string + cloud cloud.Configuration + disableInstanceDiscovery bool + popClaims map[string]string } func newServicePrincipalTokenProvider( @@ -36,6 +37,7 @@ func newServicePrincipalTokenProvider( clientCertPassword, resourceID, tenantID string, + disableInstanceDiscovery bool, popClaims map[string]string, ) (TokenProvider, error) { if clientID == "" { @@ -55,14 +57,15 @@ func newServicePrincipalTokenProvider( } return &servicePrincipalToken{ - clientID: clientID, - clientSecret: clientSecret, - clientCert: clientCert, - clientCertPassword: clientCertPassword, - resourceID: resourceID, - tenantID: tenantID, - cloud: cloud, - popClaims: popClaims, + clientID: clientID, + clientSecret: clientSecret, + clientCert: clientCert, + clientCertPassword: clientCertPassword, + resourceID: resourceID, + tenantID: tenantID, + cloud: cloud, + popClaims: popClaims, + disableInstanceDiscovery: disableInstanceDiscovery, }, nil } diff --git a/pkg/internal/token/serviceprincipaltoken_test.go b/pkg/internal/token/serviceprincipaltoken_test.go index 8813b0d..1ee6465 100644 --- a/pkg/internal/token/serviceprincipaltoken_test.go +++ b/pkg/internal/token/serviceprincipaltoken_test.go @@ -92,6 +92,7 @@ func TestNewServicePrincipalTokenProvider(t *testing.T) { tc.clientCertPassword, tc.resourceID, tc.tenantID, + false, tc.popClaims, ) diff --git a/pkg/internal/token/serviceprincipaltokencertificate.go b/pkg/internal/token/serviceprincipaltokencertificate.go index af7acf7..1f04d76 100644 --- a/pkg/internal/token/serviceprincipaltokencertificate.go +++ b/pkg/internal/token/serviceprincipaltokencertificate.go @@ -91,7 +91,7 @@ func (p *servicePrincipalToken) getPoPTokenWithClientCert( Authority: p.cloud.ActiveDirectoryAuthorityHost, ClientID: p.clientID, TenantID: p.tenantID, - DisableInstanceDiscovery: false, + DisableInstanceDiscovery: p.disableInstanceDiscovery, Options: options, }, pop.GetSwPoPKey, diff --git a/pkg/internal/token/serviceprincipaltokensecret.go b/pkg/internal/token/serviceprincipaltokensecret.go index e4f6731..19ee0ef 100644 --- a/pkg/internal/token/serviceprincipaltokensecret.go +++ b/pkg/internal/token/serviceprincipaltokensecret.go @@ -70,7 +70,7 @@ func (p *servicePrincipalToken) getPoPTokenWithClientSecret( Authority: p.cloud.ActiveDirectoryAuthorityHost, ClientID: p.clientID, TenantID: p.tenantID, - DisableInstanceDiscovery: false, + DisableInstanceDiscovery: p.disableInstanceDiscovery, Options: options, }, pop.GetSwPoPKey,