Skip to content

Commit

Permalink
Enabled globally disabling instance discovery before token acquisition
Browse files Browse the repository at this point in the history
Fix #22535
  • Loading branch information
msJinLei committed Mar 11, 2024
1 parent 3318943 commit d628886
Show file tree
Hide file tree
Showing 15 changed files with 108 additions and 18 deletions.
1 change: 1 addition & 0 deletions src/Accounts/Accounts/ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
-->

## Upcoming Release
* Enabled globally disabling instance discovery before token acquisition [#22535].
* Upgraded Azure.Core to 1.37.0.

## Version 2.16.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
// ----------------------------------------------------------------------------------

using Microsoft.Azure.Commands.Common.Authentication.Abstractions;
using Microsoft.Azure.Commands.Common.Exceptions;
using Microsoft.Azure.Commands.Shared.Config;
using Microsoft.Azure.PowerShell.Common.Config;

namespace Microsoft.Azure.Commands.Common.Authentication
{
Expand All @@ -28,6 +31,8 @@ public abstract class AuthenticationParameters

public string ResourceId { get; set; }

public bool? DisableInstanceDiscovery { get; set; } = null;

public AuthenticationParameters(
PowerShellTokenCacheProvider tokenCacheProvider,
IAzureEnvironment environment,
Expand All @@ -41,6 +46,17 @@ public AuthenticationParameters(
TokenCache = tokenCache;
TenantId = tenantId;
ResourceId = resourceId;
try
{
if (AzureSession.Instance.TryGetComponent<IConfigManager>(nameof(IConfigManager), out var config))
{
DisableInstanceDiscovery = config.GetConfigValue<bool>(ConfigKeys.DisableInstanceDiscovery);
}
}
catch(AzPSArgumentException)
{
DisableInstanceDiscovery = null;
}
}
}
}
1 change: 1 addition & 0 deletions src/Accounts/Authentication/Config/ConfigInitializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ private void RegisterConfigs(IConfigManager configManager)
false,
"AZURE_CLIENTS_SHOW_SECRETS_WARNING",
new[] { AppliesTo.Az }));
configManager.RegisterConfig(new DisableInstanceDiscoveryConfig());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// ----------------------------------------------------------------------------------
//
// Copyright Microsoft Corporation
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
// http://www.apache.org/licenses/LICENSE-2.0
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// ----------------------------------------------------------------------------------

using Microsoft.Azure.Commands.Common.Authentication.Abstractions;
using Microsoft.Azure.Commands.Common.Authentication.Properties;
using Microsoft.Azure.Commands.ResourceManager.Common;
using Microsoft.Azure.Commands.Shared.Config;
using Microsoft.Azure.PowerShell.Common.Config;
using System;
using System.Collections.Generic;

namespace Microsoft.Azure.Commands.Common.Authentication.Config.Definitions
{
/// <summary>
/// Definition of the config to control whether login by WAM (web account manager) or not.
/// </summary>
internal class DisableInstanceDiscoveryConfig : TypedConfig<bool>
{
public override object DefaultValue => false; // Opt-in. Will change to opt-out.

public override string Key => ConfigKeys.DisableInstanceDiscovery;

public override string HelpMessage => Resources.HelpMessageOfDisableInstanceDiscovery;

public override IReadOnlyCollection<AppliesTo> CanApplyTo => new[] { AppliesTo.Az };

protected override void ApplyTyped(bool value)
{
base.ApplyTyped(value);
EventHandler<StreamEventArgs> writeWarningEvent;
if (AzureSession.Instance.TryGetComponent(AzureRMCmdlet.WriteWarningKey, out writeWarningEvent) && value)
{
writeWarningEvent(this, new StreamEventArgs() { Message = Resources.DisableInstanceDiscoveryWarning });
}
}
}
}
18 changes: 18 additions & 0 deletions src/Accounts/Authentication/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/Accounts/Authentication/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -404,5 +404,10 @@
</data>
<data name="HelpMessageOfDisplaySecretsWarning" xml:space="preserve">
<value>When enabled, a warning message will be displayed when the cmdlet output contains secrets. Learn more at https://go.microsoft.com/fwlink/?linkid=2258844</value>
<data name="DisableInstanceDiscoveryWarning" xml:space="preserve">
<value>Set it to true to disable both instance discovery and authority validation. This functionality is intended for use in scenarios where the metadata endpoint cannot be reached, such as in private clouds or Azure Stack. It is crucial to ensure that the configured authority host is valid and trustworthy.</value>
</data>
</root>
<data name="HelpMessageOfDisableInstanceDiscovery" xml:space="preserve">
<value>Set it to true to disable both instance discovery and authority validation. This functionality is intended for use in scenarios where the metadata endpoint cannot be reached, such as in private clouds or Azure Stack. The process of instance discovery entails retrieving authority metadata from https://login.microsoft.com/ to validate the authority. By setting this to true, the validation of the authority is disabled. As a result, it is crucial to ensure that the configured authority host is valid and trustworthy.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public override Task<IAccessToken> Authenticate(AuthenticationParameters paramet
AuthorityHost = new Uri(authority),
TokenCachePersistenceOptions = spParameters.TokenCacheProvider.GetTokenCachePersistenceOptions()
};
options.DisableInstanceDiscovery = spParameters.DisableInstanceDiscovery ?? options.DisableInstanceDiscovery;
options.Diagnostics.IsTelemetryEnabled = false; // disable telemetry to avoid error thrown from Azure.Core that AssemblyInformationalVersion is null
TokenCredential tokenCredential = new ClientAssertionCredential(tenantId, spParameters.ClientId, () => GetClientAssertion(spParameters), options);
string parametersLog = $"- ClientId:'{spParameters.ClientId}', TenantId:'{tenantId}', ClientAssertion:'***' Scopes:'{string.Join(",", scopes)}'";
Expand Down
1 change: 1 addition & 0 deletions src/Accounts/Authenticators/DeviceCodeAuthenticator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public override Task<IAccessToken> Authenticate(AuthenticationParameters paramet
TenantId = tenantId,
TokenCachePersistenceOptions = tokenCacheProvider.GetTokenCachePersistenceOptions(),
};
options.DisableInstanceDiscovery = deviceCodeParameters.DisableInstanceDiscovery ?? options.DisableInstanceDiscovery;
var codeCredential = new DeviceCodeCredential(options);

TracingAdapter.Information($"{DateTime.Now:T} - [DeviceCodeAuthenticator] Calling DeviceCodeCredential.AuthenticateAsync - TenantId:'{options.TenantId}', Scopes:'{string.Join(",", scopes)}', AuthorityHost:'{options.AuthorityHost}'");
Expand Down
16 changes: 7 additions & 9 deletions src/Accounts/Authenticators/InteractiveUserAuthenticator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,20 @@
// limitations under the License.
// ----------------------------------------------------------------------------------

using System;
using System.Diagnostics;
using System.Net;
using System.Net.Sockets;
using System.Runtime.InteropServices;
using System.Threading;
using System.Threading.Tasks;

using Azure.Core;
using Azure.Identity;
using Azure.Identity.Broker;

using Hyak.Common;

using Microsoft.Azure.Commands.Common.Authentication;
using Microsoft.Azure.Commands.Common.Authentication.Abstractions;

using System;
using System.Net;
using System.Net.Sockets;
using System.Threading;
using System.Threading.Tasks;

namespace Microsoft.Azure.PowerShell.Authenticators
{
/// <summary>
Expand Down Expand Up @@ -68,6 +65,7 @@ public override Task<IAccessToken> Authenticate(AuthenticationParameters paramet
RedirectUri = GetReplyUrl(onPremise, interactiveParameters.PromptAction),
LoginHint = interactiveParameters.UserId,
};
options.DisableInstanceDiscovery = interactiveParameters.DisableInstanceDiscovery ?? options.DisableInstanceDiscovery;
var browserCredential = new InteractiveBrowserCredential(options);

TracingAdapter.Information($"{DateTime.Now:T} - [InteractiveUserAuthenticator] Calling InteractiveBrowserCredential.AuthenticateAsync with TenantId:'{options.TenantId}', Scopes:'{string.Join(",", scopes)}', AuthorityHost:'{options.AuthorityHost}', RedirectUri:'{options.RedirectUri}'");
Expand Down
1 change: 1 addition & 0 deletions src/Accounts/Authenticators/InteractiveWamAuthenticator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public override Task<IAccessToken> Authenticate(AuthenticationParameters paramet
RedirectUri = GetReplyUrl(onPremise, interactiveParameters.PromptAction),
LoginHint = interactiveParameters.UserId
};
options.DisableInstanceDiscovery = interactiveParameters.DisableInstanceDiscovery ?? options.DisableInstanceDiscovery;
var browserCredential = new InteractiveBrowserCredential(options);

TracingAdapter.Information($"{DateTime.Now:T} - [InteractiveWamAuthenticator] Calling InteractiveBrowserCredential.AuthenticateAsync with TenantId:'{options.TenantId}', Scopes:'{string.Join(",", scopes)}', AuthorityHost:'{options.AuthorityHost}', RedirectUri:'{options.RedirectUri}'");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,16 @@
// limitations under the License.
// ----------------------------------------------------------------------------------

using System;
using System.Text.RegularExpressions;
using System.Threading;
using System.Threading.Tasks;

using Azure.Core;
using Azure.Identity;

using Hyak.Common;

using Microsoft.Azure.Commands.Common.Authentication;
using Microsoft.Azure.Commands.Common.Authentication.Abstractions;
using Microsoft.Azure.PowerShell.Authenticators.Factories;

using System.Text.RegularExpressions;
using System.Threading;
using System.Threading.Tasks;

namespace Microsoft.Azure.PowerShell.Authenticators
{
public class ManagedServiceIdentityAuthenticator : DelegatingAuthenticator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public override Task<IAccessToken> Authenticate(AuthenticationParameters paramet
SendCertificateChain = spParameters.SendCertificateChain ?? default(bool)
};

options.DisableInstanceDiscovery = spParameters.DisableInstanceDiscovery ?? options.DisableInstanceDiscovery;
TokenCredential tokenCredential = null;
string parametersLog = null;
if (!string.IsNullOrEmpty(spParameters.Thumbprint))
Expand Down
1 change: 1 addition & 0 deletions src/Accounts/Authenticators/SilentAuthenticator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ private static SharedTokenCacheCredentialOptions GetTokenCredentialOptions(Silen
options.Username = silentParameters.UserId;
options.AuthorityHost = new Uri(authority);
options.TenantId = tenantId;
options.DisableInstanceDiscovery = silentParameters.DisableInstanceDiscovery ?? options.DisableInstanceDiscovery;
return options;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public override Task<IAccessToken> Authenticate(AuthenticationParameters paramet
AuthorityHost = new Uri(authority),
TokenCachePersistenceOptions = tokenCacheProvider.GetTokenCachePersistenceOptions()
};
credentialOptions.DisableInstanceDiscovery = upParameters.DisableInstanceDiscovery ?? credentialOptions.DisableInstanceDiscovery;
if (upParameters.Password != null)
{
passwordCredential = new UsernamePasswordCredential(upParameters.UserId, upParameters.Password.ConvertToString(), tenantId, clientId, credentialOptions);
Expand Down
1 change: 1 addition & 0 deletions src/shared/ConfigKeys.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ internal static class ConfigKeys
//Use DisableErrorRecordsPersistence as opt-out for now, will replace it with EnableErrorRecordsPersistence as opt-in at next major release (November 2023)
public const string DisableErrorRecordsPersistence = "DisableErrorRecordsPersistence";
public const string EnableErrorRecordsPersistence = "EnableErrorRecordsPersistence";
public const string DisableInstanceDiscovery = "DisableInstanceDiscovery";
public const string CheckForUpgrade = "CheckForUpgrade";
public const string EnvCheckForUpgrade = "AZUREPS_CHECK_FOR_UPGRADE";
public const string DisplaySecretsWarning = "DisplaySecretsWarning";
Expand Down

0 comments on commit d628886

Please sign in to comment.