Skip to content

Commit f31e3a5

Browse files
Aijing2333Aijing Zeng
Aijing2333
and
Aijing Zeng
authored
Add disable-instance-discovery option in interactive pop mode (#593)
* Add disable-instance-discovery option in interactive pop mode * Specify tenant id * reuse MsalClientOptions --------- Co-authored-by: Aijing Zeng <aizen@microsoft.com>
1 parent c63435d commit f31e3a5

File tree

9 files changed

+107
-79
lines changed

9 files changed

+107
-79
lines changed

‎docs/book/src/cli/get-token.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ ZURE_CLIENT_CERTIFICATE_PASSWORD environment variable
2020
--client-id string AAD client application ID. It may be specified in AAD_SERVICE_PRINCIPAL_CLIENT_ID or AZURE_CLIENT_ID environment variable
2121
--client-secret string AAD client application secret. Used in spn login. It may be specified in AAD_SERVICE_PRINCIPAL_CLIENT_SECRET or AZURE_CLIENT_S
2222
ECRET environment variable
23+
--disable-instance-discovery set to true to disable instance discovery in environments with their own Identity Provider (not Entra ID/AAD) that does not have instance metadata discovery endpoint.
2324
-e, --environment string Azure environment name (default "AzurePublicCloud")
2425
--federated-token-file string Workload Identity federated token file. It may be specified in AZURE_FEDERATED_TOKEN_FILE environment variable
2526
-h, --help help for get-token

‎pkg/internal/pop/msal_public.go‎

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"fmt"
66
"net/http"
77

8-
"github.com/Azure/azure-sdk-for-go/sdk/azcore"
98
"github.com/AzureAD/microsoft-authentication-library-for-go/apps/public"
109
)
1110

@@ -15,13 +14,11 @@ func AcquirePoPTokenInteractive(
1514
context context.Context,
1615
popClaims map[string]string,
1716
scopes []string,
18-
authority,
19-
clientID string,
20-
options *azcore.ClientOptions,
17+
msalOptions *MsalClientOptions,
2118
) (string, int64, error) {
2219
var client *public.Client
2320
var err error
24-
client, err = getPublicClient(authority, clientID, options)
21+
client, err = getPublicClient(msalOptions)
2522
if err != nil {
2623
return "", -1, err
2724
}
@@ -39,6 +36,7 @@ func AcquirePoPTokenInteractive(
3936
PoPKey: popKey,
4037
},
4138
),
39+
public.WithTenantID(msalOptions.TenantID),
4240
)
4341
if err != nil {
4442
return "", -1, fmt.Errorf("failed to create PoP token with interactive flow: %w", err)
@@ -53,13 +51,11 @@ func AcquirePoPTokenByUsernamePassword(
5351
context context.Context,
5452
popClaims map[string]string,
5553
scopes []string,
56-
authority,
57-
clientID,
5854
username,
5955
password string,
60-
options *azcore.ClientOptions,
56+
msalOptions *MsalClientOptions,
6157
) (string, int64, error) {
62-
client, err := getPublicClient(authority, clientID, options)
58+
client, err := getPublicClient(msalOptions)
6359
if err != nil {
6460
return "", -1, err
6561
}
@@ -79,6 +75,7 @@ func AcquirePoPTokenByUsernamePassword(
7975
PoPKey: popKey,
8076
},
8177
),
78+
public.WithTenantID(msalOptions.TenantID),
8279
)
8380
if err != nil {
8481
return "", -1, fmt.Errorf("failed to create PoP token with username/password flow: %w", err)
@@ -88,23 +85,25 @@ func AcquirePoPTokenByUsernamePassword(
8885
}
8986

9087
// getPublicClient returns an instance of the msal `public` client based on the provided options
91-
func getPublicClient(
92-
authority,
93-
clientID string,
94-
options *azcore.ClientOptions,
95-
) (*public.Client, error) {
88+
// The instance discovery should be disabled on private cloud
89+
func getPublicClient(msalOptions *MsalClientOptions) (*public.Client, error) {
9690
var client public.Client
9791
var err error
98-
if options != nil && options.Transport != nil {
92+
if msalOptions == nil {
93+
return nil, fmt.Errorf("unable to create public client: MsalClientOptions is empty")
94+
}
95+
if msalOptions.Options != nil && msalOptions.Options.Transport != nil {
9996
client, err = public.New(
100-
clientID,
101-
public.WithAuthority(authority),
102-
public.WithHTTPClient(options.Transport.(*http.Client)),
97+
msalOptions.ClientID,
98+
public.WithAuthority(msalOptions.Authority),
99+
public.WithHTTPClient(msalOptions.Options.Transport.(*http.Client)),
100+
public.WithInstanceDiscovery(!msalOptions.DisableInstanceDiscovery),
103101
)
104102
} else {
105103
client, err = public.New(
106-
clientID,
107-
public.WithAuthority(authority),
104+
msalOptions.ClientID,
105+
public.WithAuthority(msalOptions.Authority),
106+
public.WithInstanceDiscovery(!msalOptions.DisableInstanceDiscovery),
108107
)
109108
}
110109
if err != nil {

‎pkg/internal/pop/msal_public_test.go‎

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,14 @@ func TestAcquirePoPTokenByUsernamePassword(t *testing.T) {
118118
ctx,
119119
tc.p.popClaims,
120120
scopes,
121-
authority,
122-
tc.p.clientID,
123121
tc.p.username,
124122
tc.p.password,
125-
&clientOpts,
123+
&MsalClientOptions{
124+
Authority: authority,
125+
ClientID: tc.p.clientID,
126+
Options: &clientOpts,
127+
TenantID: tc.p.tenantID,
128+
},
126129
)
127130
defer vcrRecorder.Stop()
128131
if tc.expectedError != nil {
@@ -156,35 +159,46 @@ func TestGetPublicClient(t *testing.T) {
156159

157160
testCase := []struct {
158161
testName string
159-
authority string
160-
options *azcore.ClientOptions
162+
msalOptions *MsalClientOptions
161163
expectedError error
162164
}{
163165
{
164166
// Test using custom HTTP transport
165-
testName: "TestGetPublicClientWithCustomTransport",
166-
authority: authority,
167-
options: &azcore.ClientOptions{
168-
Cloud: cloud.AzurePublic,
169-
Transport: httpClient,
167+
testName: "TestGetPublicClientWithCustomTransport",
168+
msalOptions: &MsalClientOptions{
169+
Authority: authority,
170+
ClientID: testutils.ClientID,
171+
Options: &azcore.ClientOptions{
172+
Cloud: cloud.AzurePublic,
173+
Transport: httpClient,
174+
},
175+
TenantID: testutils.TenantID,
170176
},
171177
expectedError: nil,
172178
},
173179
{
174180
// Test using default HTTP transport
175-
testName: "TestGetPublicClientWithDefaultTransport",
176-
authority: authority,
177-
options: &azcore.ClientOptions{
178-
Cloud: cloud.AzurePublic,
181+
testName: "TestGetPublicClientWithDefaultTransport",
182+
msalOptions: &MsalClientOptions{
183+
Authority: authority,
184+
ClientID: testutils.ClientID,
185+
Options: &azcore.ClientOptions{
186+
Cloud: cloud.AzurePublic,
187+
},
188+
TenantID: testutils.TenantID,
179189
},
180190
expectedError: nil,
181191
},
182192
{
183193
// Test using incorrectly formatted authority
184-
testName: "TestGetPublicClientWithBadAuthority",
185-
authority: "login.microsoft.com",
186-
options: &azcore.ClientOptions{
187-
Cloud: cloud.AzurePublic,
194+
testName: "TestGetPublicClientWithBadAuthority",
195+
msalOptions: &MsalClientOptions{
196+
Authority: "login.microsoft.com",
197+
ClientID: testutils.ClientID,
198+
Options: &azcore.ClientOptions{
199+
Cloud: cloud.AzurePublic,
200+
},
201+
TenantID: testutils.TenantID,
188202
},
189203
expectedError: fmt.Errorf("unable to create public client"),
190204
},
@@ -195,11 +209,7 @@ func TestGetPublicClient(t *testing.T) {
195209

196210
for _, tc := range testCase {
197211
t.Run(tc.testName, func(t *testing.T) {
198-
client, err = getPublicClient(
199-
tc.authority,
200-
testutils.ClientID,
201-
tc.options,
202-
)
212+
client, err = getPublicClient(tc.msalOptions)
203213

204214
if tc.expectedError != nil {
205215
if !testutils.ErrorContains(err, tc.expectedError.Error()) {

‎pkg/internal/token/interactive.go‎

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,17 @@ import (
1616
)
1717

1818
type InteractiveToken struct {
19-
clientID string
20-
resourceID string
21-
tenantID string
22-
oAuthConfig adal.OAuthConfig
23-
popClaims map[string]string
19+
clientID string
20+
resourceID string
21+
tenantID string
22+
oAuthConfig adal.OAuthConfig
23+
popClaims map[string]string
24+
disableInstanceDiscovery bool
2425
}
2526

2627
// newInteractiveTokenProvider returns a TokenProvider that will fetch a token for the user currently logged into the Interactive.
2728
// Required arguments include an oAuthConfiguration object and the resourceID (which is used as the scope)
28-
func newInteractiveTokenProvider(oAuthConfig adal.OAuthConfig, clientID, resourceID, tenantID string, popClaims map[string]string) (TokenProvider, error) {
29+
func newInteractiveTokenProvider(oAuthConfig adal.OAuthConfig, clientID, resourceID, tenantID string, popClaims map[string]string, disableInstanceDiscovery bool) (TokenProvider, error) {
2930
if clientID == "" {
3031
return nil, errors.New("clientID cannot be empty")
3132
}
@@ -37,11 +38,12 @@ func newInteractiveTokenProvider(oAuthConfig adal.OAuthConfig, clientID, resourc
3738
}
3839

3940
return &InteractiveToken{
40-
clientID: clientID,
41-
resourceID: resourceID,
42-
tenantID: tenantID,
43-
oAuthConfig: oAuthConfig,
44-
popClaims: popClaims,
41+
clientID: clientID,
42+
resourceID: resourceID,
43+
tenantID: tenantID,
44+
oAuthConfig: oAuthConfig,
45+
popClaims: popClaims,
46+
disableInstanceDiscovery: disableInstanceDiscovery,
4547
}, nil
4648
}
4749

@@ -73,18 +75,23 @@ func (p *InteractiveToken) TokenWithOptions(ctx context.Context, options *azcore
7375
ctx,
7476
p.popClaims,
7577
scopes,
76-
authorityFromConfig.String(),
77-
p.clientID,
78-
&clientOpts,
78+
&pop.MsalClientOptions{
79+
Authority: authorityFromConfig.String(),
80+
ClientID: p.clientID,
81+
DisableInstanceDiscovery: p.disableInstanceDiscovery,
82+
Options: &clientOpts,
83+
TenantID: p.tenantID,
84+
},
7985
)
8086
if err != nil {
8187
return emptyToken, fmt.Errorf("failed to create PoP token using interactive login: %w", err)
8288
}
8389
} else {
8490
cred, err := azidentity.NewInteractiveBrowserCredential(&azidentity.InteractiveBrowserCredentialOptions{
85-
ClientOptions: clientOpts,
86-
TenantID: p.tenantID,
87-
ClientID: p.clientID,
91+
ClientOptions: clientOpts,
92+
TenantID: p.tenantID,
93+
ClientID: p.clientID,
94+
DisableInstanceDiscovery: p.disableInstanceDiscovery,
8895
})
8996
if err != nil {
9097
return emptyToken, fmt.Errorf("unable to create credential. Received: %w", err)

‎pkg/internal/token/interactive_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ func TestNewInteractiveToken(t *testing.T) {
5959
tc.resourceID,
6060
tc.tenantID,
6161
tc.popClaims,
62+
false,
6263
)
6364

6465
if tc.expectedError != "" {

‎pkg/internal/token/options.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ type Options struct {
3535
IsPoPTokenEnabled bool
3636
PoPTokenClaims string
3737
DisableEnvironmentOverride bool
38+
DisableInstanceDiscovery bool
3839
}
3940

4041
const (
@@ -110,6 +111,7 @@ func (o *Options) AddFlags(fs *pflag.FlagSet) {
110111
fmt.Sprintf("Timeout duration for Azure CLI token requests. It may be specified in %s environment variable", "AZURE_CLI_TIMEOUT"))
111112
fs.StringVar(&o.PoPTokenClaims, "pop-claims", o.PoPTokenClaims, "contains a comma-separated list of claims to attach to the pop token in the format `key=val,key2=val2`. At minimum, specify the ARM ID of the cluster as `u=ARM_ID`")
112113
fs.BoolVar(&o.DisableEnvironmentOverride, "disable-environment-override", o.DisableEnvironmentOverride, "Enable or disable the use of env-variables. Default false")
114+
fs.BoolVar(&o.DisableInstanceDiscovery, "disable-instance-discovery", o.DisableInstanceDiscovery, "set to true to disable instance discovery in environments with their own simple Identity Provider (not AAD) that do not have instance metadata discovery endpoint. Default false")
113115
}
114116

115117
func (o *Options) Validate() error {

‎pkg/internal/token/provider.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,14 @@ func NewTokenProvider(o *Options) (TokenProvider, error) {
3434
case DeviceCodeLogin:
3535
return newDeviceCodeTokenProvider(*oAuthConfig, o.ClientID, o.ServerID, o.TenantID)
3636
case InteractiveLogin:
37-
return newInteractiveTokenProvider(*oAuthConfig, o.ClientID, o.ServerID, o.TenantID, popClaimsMap)
37+
return newInteractiveTokenProvider(*oAuthConfig, o.ClientID, o.ServerID, o.TenantID, popClaimsMap, o.DisableInstanceDiscovery)
3838
case ServicePrincipalLogin:
3939
if o.IsLegacy {
4040
return newLegacyServicePrincipalToken(*oAuthConfig, o.ClientID, o.ClientSecret, o.ClientCert, o.ClientCertPassword, o.ServerID, o.TenantID)
4141
}
4242
return newServicePrincipalTokenProvider(cloudConfiguration, o.ClientID, o.ClientSecret, o.ClientCert, o.ClientCertPassword, o.ServerID, o.TenantID, false, popClaimsMap)
4343
case ROPCLogin:
44-
return newResourceOwnerTokenProvider(*oAuthConfig, o.ClientID, o.Username, o.Password, o.ServerID, o.TenantID, popClaimsMap)
44+
return newResourceOwnerTokenProvider(*oAuthConfig, o.ClientID, o.Username, o.Password, o.ServerID, o.TenantID, popClaimsMap, o.DisableInstanceDiscovery)
4545
case MSILogin:
4646
return newManagedIdentityToken(o.ClientID, o.IdentityResourceID, o.ServerID)
4747
case AzureCLILogin:

‎pkg/internal/token/ropc.go‎

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,14 @@ import (
1414
)
1515

1616
type resourceOwnerToken struct {
17-
clientID string
18-
username string
19-
password string
20-
resourceID string
21-
tenantID string
22-
oAuthConfig adal.OAuthConfig
23-
popClaims map[string]string
17+
clientID string
18+
username string
19+
password string
20+
resourceID string
21+
tenantID string
22+
oAuthConfig adal.OAuthConfig
23+
popClaims map[string]string
24+
disableInstanceDiscovery bool
2425
}
2526

2627
func newResourceOwnerTokenProvider(
@@ -31,6 +32,7 @@ func newResourceOwnerTokenProvider(
3132
resourceID,
3233
tenantID string,
3334
popClaims map[string]string,
35+
disableInstanceDiscovery bool,
3436
) (TokenProvider, error) {
3537
if clientID == "" {
3638
return nil, errors.New("clientID cannot be empty")
@@ -49,13 +51,14 @@ func newResourceOwnerTokenProvider(
4951
}
5052

5153
return &resourceOwnerToken{
52-
clientID: clientID,
53-
username: username,
54-
password: password,
55-
resourceID: resourceID,
56-
tenantID: tenantID,
57-
oAuthConfig: oAuthConfig,
58-
popClaims: popClaims,
54+
clientID: clientID,
55+
username: username,
56+
password: password,
57+
resourceID: resourceID,
58+
tenantID: tenantID,
59+
oAuthConfig: oAuthConfig,
60+
popClaims: popClaims,
61+
disableInstanceDiscovery: disableInstanceDiscovery,
5962
}, nil
6063
}
6164

@@ -82,11 +85,15 @@ func (p *resourceOwnerToken) tokenWithOptions(ctx context.Context, options *azco
8285
ctx,
8386
p.popClaims,
8487
scopes,
85-
authorityFromConfig.String(),
86-
p.clientID,
8788
p.username,
8889
p.password,
89-
&clientOpts,
90+
&pop.MsalClientOptions{
91+
Authority: authorityFromConfig.String(),
92+
ClientID: p.clientID,
93+
DisableInstanceDiscovery: p.disableInstanceDiscovery,
94+
Options: &clientOpts,
95+
TenantID: p.tenantID,
96+
},
9097
)
9198
if err != nil {
9299
return emptyToken, fmt.Errorf("failed to create PoP token using resource owner flow: %w", err)

‎pkg/internal/token/ropc_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ func TestNewResourceOwnerTokenProvider(t *testing.T) {
9191
tc.resourceID,
9292
tc.tenantID,
9393
tc.popClaims,
94+
false,
9495
)
9596

9697
if tc.expectedError != "" {

0 commit comments

Comments
 (0)