Skip to content

Revert instance discovery option name change #20746

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion sdk/azidentity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

### Breaking Changes
> These changes affect only code written against a beta version such as v1.3.0-beta.5
* Renamed `DisableInstanceDiscovery` to `DisableAuthorityValidationAndInstanceDiscovery`
* Renamed `NewOnBehalfOfCredentialFromCertificate` to `NewOnBehalfOfCredentialWithCertificate`
* Renamed `NewOnBehalfOfCredentialFromSecret` to `NewOnBehalfOfCredentialWithSecret`

Expand Down
13 changes: 6 additions & 7 deletions sdk/azidentity/client_assertion_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,11 @@ type ClientAssertionCredentialOptions struct {
// Add the wildcard value "*" to allow the credential to acquire tokens for any tenant in which the
// application is registered.
AdditionallyAllowedTenants []string
// DisableAuthorityValidationAndInstanceDiscovery should be set true only by applications authenticating
// in disconnected clouds, or private clouds such as Azure Stack. It determines whether the credential
// requests Azure AD instance metadata from https://login.microsoft.com before authenticating. Setting
// this to true will skip this request, making the application responsible for ensuring the configured
// authority is valid and trustworthy.
DisableAuthorityValidationAndInstanceDiscovery bool
// DisableInstanceDiscovery should be set true only by applications authenticating in disconnected clouds, or
// private clouds such as Azure Stack. It determines whether the credential requests Azure AD instance metadata
// from https://login.microsoft.com before authenticating. Setting this to true will skip this request, making
// the application responsible for ensuring the configured authority is valid and trustworthy.
DisableInstanceDiscovery bool
}

// NewClientAssertionCredential constructs a ClientAssertionCredential. The getAssertion function must be thread safe. Pass nil for options to accept defaults.
Expand All @@ -57,7 +56,7 @@ func NewClientAssertionCredential(tenantID, clientID string, getAssertion func(c
return getAssertion(ctx)
},
)
c, err := getConfidentialClient(clientID, tenantID, cred, &options.ClientOptions, confidential.WithInstanceDiscovery(!options.DisableAuthorityValidationAndInstanceDiscovery))
c, err := getConfidentialClient(clientID, tenantID, cred, &options.ClientOptions, confidential.WithInstanceDiscovery(!options.DisableInstanceDiscovery))
if err != nil {
return nil, err
}
Expand Down
5 changes: 1 addition & 4 deletions sdk/azidentity/client_assertion_credential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,7 @@ func TestClientAssertionCredential_Live(t *testing.T) {
func(context.Context) (string, error) {
return getAssertion(certs[0], key)
},
&ClientAssertionCredentialOptions{
ClientOptions: o,
DisableAuthorityValidationAndInstanceDiscovery: d,
},
&ClientAssertionCredentialOptions{ClientOptions: o, DisableInstanceDiscovery: d},
)
if err != nil {
t.Fatal(err)
Expand Down
13 changes: 6 additions & 7 deletions sdk/azidentity/client_certificate_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,11 @@ type ClientCertificateCredentialOptions struct {
// Add the wildcard value "*" to allow the credential to acquire tokens for any tenant in which the
// application is registered.
AdditionallyAllowedTenants []string
// DisableAuthorityValidationAndInstanceDiscovery should be set true only by applications authenticating
// in disconnected clouds, or private clouds such as Azure Stack. It determines whether the credential
// requests Azure AD instance metadata from https://login.microsoft.com before authenticating. Setting
// this to true will skip this request, making the application responsible for ensuring the configured
// authority is valid and trustworthy.
DisableAuthorityValidationAndInstanceDiscovery bool
// DisableInstanceDiscovery should be set true only by applications authenticating in disconnected clouds, or
// private clouds such as Azure Stack. It determines whether the credential requests Azure AD instance metadata
// from https://login.microsoft.com before authenticating. Setting this to true will skip this request, making
// the application responsible for ensuring the configured authority is valid and trustworthy.
DisableInstanceDiscovery bool
// SendCertificateChain controls whether the credential sends the public certificate chain in the x5c
// header of each token request's JWT. This is required for Subject Name/Issuer (SNI) authentication.
// Defaults to False.
Expand Down Expand Up @@ -63,7 +62,7 @@ func NewClientCertificateCredential(tenantID string, clientID string, certs []*x
if options.SendCertificateChain {
o = append(o, confidential.WithX5C())
}
o = append(o, confidential.WithInstanceDiscovery(!options.DisableAuthorityValidationAndInstanceDiscovery))
o = append(o, confidential.WithInstanceDiscovery(!options.DisableInstanceDiscovery))
c, err := getConfidentialClient(clientID, tenantID, cred, &options.ClientOptions, o...)
if err != nil {
return nil, err
Expand Down
4 changes: 2 additions & 2 deletions sdk/azidentity/client_certificate_credential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ func TestClientCertificateCredential_Live(t *testing.T) {
}
o, stop := initRecording(t)
defer stop()
opts := &ClientCertificateCredentialOptions{ClientOptions: o, DisableAuthorityValidationAndInstanceDiscovery: true}
opts := &ClientCertificateCredentialOptions{ClientOptions: o, DisableInstanceDiscovery: true}
cred, err := NewClientCertificateCredential(liveSP.tenantID, liveSP.clientID, certs, key, opts)
if err != nil {
t.Fatalf("failed to construct credential: %v", err)
Expand All @@ -265,7 +265,7 @@ func TestClientCertificateCredentialADFS_Live(t *testing.T) {
o, stop := initRecording(t)
defer stop()
o.Cloud.ActiveDirectoryAuthorityHost = adfsAuthority
opts := &ClientCertificateCredentialOptions{ClientOptions: o, DisableAuthorityValidationAndInstanceDiscovery: true}
opts := &ClientCertificateCredentialOptions{ClientOptions: o, DisableInstanceDiscovery: true}
cred, err := NewClientCertificateCredential("adfs", adfsLiveSP.clientID, certs, key, opts)
if err != nil {
t.Fatalf("failed to construct credential: %v", err)
Expand Down
13 changes: 6 additions & 7 deletions sdk/azidentity/client_secret_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,11 @@ type ClientSecretCredentialOptions struct {
// Add the wildcard value "*" to allow the credential to acquire tokens for any tenant in which the
// application is registered.
AdditionallyAllowedTenants []string
// DisableAuthorityValidationAndInstanceDiscovery should be set true only by applications authenticating
// in disconnected clouds, or private clouds such as Azure Stack. It determines whether the credential
// requests Azure AD instance metadata from https://login.microsoft.com before authenticating. Setting
// this to true will skip this request, making the application responsible for ensuring the configured
// authority is valid and trustworthy.
DisableAuthorityValidationAndInstanceDiscovery bool
// DisableInstanceDiscovery should be set true only by applications authenticating in disconnected clouds, or
// private clouds such as Azure Stack. It determines whether the credential requests Azure AD instance metadata
// from https://login.microsoft.com before authenticating. Setting this to true will skip this request, making
// the application responsible for ensuring the configured authority is valid and trustworthy.
DisableInstanceDiscovery bool
}

// ClientSecretCredential authenticates an application with a client secret.
Expand All @@ -48,7 +47,7 @@ func NewClientSecretCredential(tenantID string, clientID string, clientSecret st
return nil, err
}
c, err := getConfidentialClient(
clientID, tenantID, cred, &options.ClientOptions, confidential.WithInstanceDiscovery(!options.DisableAuthorityValidationAndInstanceDiscovery),
clientID, tenantID, cred, &options.ClientOptions, confidential.WithInstanceDiscovery(!options.DisableInstanceDiscovery),
)
if err != nil {
return nil, err
Expand Down
4 changes: 2 additions & 2 deletions sdk/azidentity/client_secret_credential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestClientSecretCredential_Live(t *testing.T) {
t.Run(name, func(t *testing.T) {
opts, stop := initRecording(t)
defer stop()
o := ClientSecretCredentialOptions{ClientOptions: opts, DisableAuthorityValidationAndInstanceDiscovery: disabledID}
o := ClientSecretCredentialOptions{ClientOptions: opts, DisableInstanceDiscovery: disabledID}
cred, err := NewClientSecretCredential(liveSP.tenantID, liveSP.clientID, liveSP.secret, &o)
if err != nil {
t.Fatalf("failed to construct credential: %v", err)
Expand All @@ -68,7 +68,7 @@ func TestClientSecretCredentialADFS_Live(t *testing.T) {
opts, stop := initRecording(t)
defer stop()
opts.Cloud.ActiveDirectoryAuthorityHost = adfsAuthority
o := ClientSecretCredentialOptions{ClientOptions: opts, DisableAuthorityValidationAndInstanceDiscovery: true}
o := ClientSecretCredentialOptions{ClientOptions: opts, DisableInstanceDiscovery: true}
cred, err := NewClientSecretCredential("adfs", adfsLiveSP.clientID, adfsLiveSP.secret, &o)
if err != nil {
t.Fatalf("failed to construct credential: %v", err)
Expand Down
19 changes: 9 additions & 10 deletions sdk/azidentity/default_azure_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,11 @@ type DefaultAzureCredentialOptions struct {
// the wildcard value "*" to allow the credential to acquire tokens for any tenant. This value can also be
// set as a semicolon delimited list of tenants in the environment variable AZURE_ADDITIONALLY_ALLOWED_TENANTS.
AdditionallyAllowedTenants []string
// DisableAuthorityValidationAndInstanceDiscovery should be set true only by applications authenticating
// in disconnected clouds, or private clouds such as Azure Stack. It determines whether the credential
// requests Azure AD instance metadata from https://login.microsoft.com before authenticating. Setting
// this to true will skip this request, making the application responsible for ensuring the configured
// authority is valid and trustworthy.
DisableAuthorityValidationAndInstanceDiscovery bool
// DisableInstanceDiscovery should be set true only by applications authenticating in disconnected clouds, or
// private clouds such as Azure Stack. It determines whether the credential requests Azure AD instance metadata
// from https://login.microsoft.com before authenticating. Setting this to true will skip this request, making
// the application responsible for ensuring the configured authority is valid and trustworthy.
DisableInstanceDiscovery bool
// TenantID identifies the tenant the Azure CLI should authenticate in.
// Defaults to the CLI's default tenant, which is typically the home tenant of the user logged in to the CLI.
TenantID string
Expand Down Expand Up @@ -73,9 +72,9 @@ func NewDefaultAzureCredential(options *DefaultAzureCredentialOptions) (*Default
}

envCred, err := NewEnvironmentCredential(&EnvironmentCredentialOptions{
ClientOptions: options.ClientOptions,
DisableAuthorityValidationAndInstanceDiscovery: options.DisableAuthorityValidationAndInstanceDiscovery,
additionallyAllowedTenants: additionalTenants,
ClientOptions: options.ClientOptions,
DisableInstanceDiscovery: options.DisableInstanceDiscovery,
additionallyAllowedTenants: additionalTenants,
})
if err == nil {
creds = append(creds, envCred)
Expand All @@ -88,7 +87,7 @@ func NewDefaultAzureCredential(options *DefaultAzureCredentialOptions) (*Default
wic, err := NewWorkloadIdentityCredential(&WorkloadIdentityCredentialOptions{
AdditionallyAllowedTenants: additionalTenants,
ClientOptions: options.ClientOptions,
DisableAuthorityValidationAndInstanceDiscovery: options.DisableAuthorityValidationAndInstanceDiscovery,
DisableInstanceDiscovery: options.DisableInstanceDiscovery,
})
if err == nil {
creds = append(creds, wic)
Expand Down
13 changes: 6 additions & 7 deletions sdk/azidentity/device_code_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,11 @@ type DeviceCodeCredentialOptions struct {
// ClientID is the ID of the application users will authenticate to.
// Defaults to the ID of an Azure development application.
ClientID string
// DisableAuthorityValidationAndInstanceDiscovery should be set true only by applications authenticating
// in disconnected clouds, or private clouds such as Azure Stack. It determines whether the credential
// requests Azure AD instance metadata from https://login.microsoft.com before authenticating. Setting
// this to true will skip this request, making the application responsible for ensuring the configured
// authority is valid and trustworthy.
DisableAuthorityValidationAndInstanceDiscovery bool
// DisableInstanceDiscovery should be set true only by applications authenticating in disconnected clouds, or
// private clouds such as Azure Stack. It determines whether the credential requests Azure AD instance metadata
// from https://login.microsoft.com before authenticating. Setting this to true will skip this request, making
// the application responsible for ensuring the configured authority is valid and trustworthy.
DisableInstanceDiscovery bool
// TenantID is the Azure Active Directory tenant the credential authenticates in. Defaults to the
// "organizations" tenant, which can authenticate work and school accounts. Required for single-tenant
// applications.
Expand Down Expand Up @@ -89,7 +88,7 @@ func NewDeviceCodeCredential(options *DeviceCodeCredentialOptions) (*DeviceCodeC
}
cp.init()
c, err := getPublicClient(
cp.ClientID, cp.TenantID, &cp.ClientOptions, public.WithInstanceDiscovery(!cp.DisableAuthorityValidationAndInstanceDiscovery),
cp.ClientID, cp.TenantID, &cp.ClientOptions, public.WithInstanceDiscovery(!cp.DisableInstanceDiscovery),
)
if err != nil {
return nil, err
Expand Down
9 changes: 5 additions & 4 deletions sdk/azidentity/device_code_credential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func TestDeviceCodeCredential_Live(t *testing.T) {
},
{
desc: "instance discovery disabled",
opts: DeviceCodeCredentialOptions{DisableAuthorityValidationAndInstanceDiscovery: true, TenantID: liveSP.tenantID},
opts: DeviceCodeCredentialOptions{DisableInstanceDiscovery: true, TenantID: liveSP.tenantID},
},
{
desc: "optional tenant",
Expand Down Expand Up @@ -133,9 +133,10 @@ func TestDeviceCodeCredentialADFS_Live(t *testing.T) {
defer stop()
o.Cloud.ActiveDirectoryAuthorityHost = adfsAuthority
opts := DeviceCodeCredentialOptions{
ClientID: adfsLiveUser.clientID,
ClientOptions: o, DisableAuthorityValidationAndInstanceDiscovery: true,
TenantID: "adfs",
ClientID: adfsLiveUser.clientID,
ClientOptions: o,
DisableInstanceDiscovery: true,
TenantID: "adfs",
}
if recording.GetRecordMode() == recording.PlaybackMode {
opts.UserPrompt = func(ctx context.Context, m DeviceCodeMessage) error { return nil }
Expand Down
17 changes: 8 additions & 9 deletions sdk/azidentity/environment_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,11 @@ const envVarSendCertChain = "AZURE_CLIENT_SEND_CERTIFICATE_CHAIN"
type EnvironmentCredentialOptions struct {
azcore.ClientOptions

// DisableAuthorityValidationAndInstanceDiscovery should be set true only by applications authenticating
// in disconnected clouds, or private clouds such as Azure Stack. It determines whether the credential
// requests Azure AD instance metadata from https://login.microsoft.com before authenticating. Setting
// this to true will skip this request, making the application responsible for ensuring the configured
// authority is valid and trustworthy.
DisableAuthorityValidationAndInstanceDiscovery bool
// DisableInstanceDiscovery should be set true only by applications authenticating in disconnected clouds, or
// private clouds such as Azure Stack. It determines whether the credential requests Azure AD instance metadata
// from https://login.microsoft.com before authenticating. Setting this to true will skip this request, making
// the application responsible for ensuring the configured authority is valid and trustworthy.
DisableInstanceDiscovery bool
// additionallyAllowedTenants is used only by NewDefaultAzureCredential() to enable that constructor's explicit
// option to override the value of AZURE_ADDITIONALLY_ALLOWED_TENANTS. Applications using EnvironmentCredential
// directly should set that variable instead. This field should remain unexported to preserve this credential's
Expand Down Expand Up @@ -102,7 +101,7 @@ func NewEnvironmentCredential(options *EnvironmentCredentialOptions) (*Environme
o := &ClientSecretCredentialOptions{
AdditionallyAllowedTenants: additionalTenants,
ClientOptions: options.ClientOptions,
DisableAuthorityValidationAndInstanceDiscovery: options.DisableAuthorityValidationAndInstanceDiscovery,
DisableInstanceDiscovery: options.DisableInstanceDiscovery,
}
cred, err := NewClientSecretCredential(tenantID, clientID, clientSecret, o)
if err != nil {
Expand All @@ -127,7 +126,7 @@ func NewEnvironmentCredential(options *EnvironmentCredentialOptions) (*Environme
o := &ClientCertificateCredentialOptions{
AdditionallyAllowedTenants: additionalTenants,
ClientOptions: options.ClientOptions,
DisableAuthorityValidationAndInstanceDiscovery: options.DisableAuthorityValidationAndInstanceDiscovery,
DisableInstanceDiscovery: options.DisableInstanceDiscovery,
}
if v, ok := os.LookupEnv(envVarSendCertChain); ok {
o.SendCertificateChain = v == "1" || strings.ToLower(v) == "true"
Expand All @@ -144,7 +143,7 @@ func NewEnvironmentCredential(options *EnvironmentCredentialOptions) (*Environme
o := &UsernamePasswordCredentialOptions{
AdditionallyAllowedTenants: additionalTenants,
ClientOptions: options.ClientOptions,
DisableAuthorityValidationAndInstanceDiscovery: options.DisableAuthorityValidationAndInstanceDiscovery,
DisableInstanceDiscovery: options.DisableInstanceDiscovery,
}
cred, err := NewUsernamePasswordCredential(tenantID, clientID, username, password, o)
if err != nil {
Expand Down
16 changes: 8 additions & 8 deletions sdk/azidentity/environment_credential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,8 @@ func TestEnvironmentCredential_ClientSecretLive(t *testing.T) {
opts, stop := initRecording(t)
defer stop()
cred, err := NewEnvironmentCredential(&EnvironmentCredentialOptions{
ClientOptions: opts,
DisableAuthorityValidationAndInstanceDiscovery: disabledID,
ClientOptions: opts,
DisableInstanceDiscovery: disabledID,
})
if err != nil {
t.Fatalf("failed to construct credential: %v", err)
Expand All @@ -275,8 +275,8 @@ func TestEnvironmentCredentialADFS_ClientSecretLive(t *testing.T) {
opts, stop := initRecording(t)
defer stop()
cred, err := NewEnvironmentCredential(&EnvironmentCredentialOptions{
ClientOptions: opts,
DisableAuthorityValidationAndInstanceDiscovery: true,
ClientOptions: opts,
DisableInstanceDiscovery: true,
})
if err != nil {
t.Fatalf("failed to construct credential: %v", err)
Expand Down Expand Up @@ -330,8 +330,8 @@ func TestEnvironmentCredential_UserPasswordLive(t *testing.T) {
opts, stop := initRecording(t)
defer stop()
cred, err := NewEnvironmentCredential(&EnvironmentCredentialOptions{
ClientOptions: opts,
DisableAuthorityValidationAndInstanceDiscovery: disabledID,
ClientOptions: opts,
DisableInstanceDiscovery: disabledID,
})
if err != nil {
t.Fatalf("failed to construct credential: %v", err)
Expand All @@ -358,8 +358,8 @@ func TestEnvironmentCredentialADFS_UserPasswordLive(t *testing.T) {
opts, stop := initRecording(t)
defer stop()
cred, err := NewEnvironmentCredential(&EnvironmentCredentialOptions{
ClientOptions: opts,
DisableAuthorityValidationAndInstanceDiscovery: true,
ClientOptions: opts,
DisableInstanceDiscovery: true,
})
if err != nil {
t.Fatalf("failed to construct credential: %v", err)
Expand Down
Loading