Skip to content

Commit

Permalink
Enable Globally Disabling Instance Discovery Before Token Acquisition (
Browse files Browse the repository at this point in the history
…#24105)

* Enabled globally disabling instance discovery before token acquisition

Fix #22535

* Address review comments

---------

Co-authored-by: Beisi Zhou <[email protected]>
  • Loading branch information
msJinLei and BethanyZhou authored Mar 21, 2024
1 parent 338fa3b commit dc828b7
Show file tree
Hide file tree
Showing 18 changed files with 164 additions and 28 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].
* Implemented secrets detection feature for autorest modules.
* Added `AsSecureString` to `Get-AzAccessToken` to convert the returned token to SecureString [#24190].
* Upgraded Azure.Core to 1.37.0.
Expand Down
23 changes: 19 additions & 4 deletions src/Accounts/Accounts/help/Clear-AzConfig.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ Clear-AzConfig [-Force] [-PassThru] [-AppliesTo <String>] [-Scope <ConfigScope>]
### ClearByKey
```
Clear-AzConfig [-PassThru] [-AppliesTo <String>] [-Scope <ConfigScope>]
[-DefaultProfile <IAzureContextContainer>] [-WhatIf] [-Confirm]
[-CheckForUpgrade] [-DefaultSubscriptionForLogin] [-DisableErrorRecordsPersistence]
[-DisplayBreakingChangeWarning] [-DisplayRegionIdentified] [-DisplaySecretsWarning]
[-DisplaySurveyMessage] [-EnableDataCollection] [-EnableLoginByWam] [<CommonParameters>]
[-DefaultProfile <IAzureContextContainer>] [-WhatIf] [-Confirm] [-CheckForUpgrade]
[-DefaultSubscriptionForLogin] [-DisableErrorRecordsPersistence] [-DisableInstanceDiscovery]
[-DisplayBreakingChangeWarning] [-DisplayRegionIdentified] [-DisplaySurveyMessage] [-EnableDataCollection]
[-EnableLoginByWam] [<CommonParameters>]
```

## DESCRIPTION
Expand Down Expand Up @@ -131,6 +131,21 @@ Accept pipeline input: False
Accept wildcard characters: False
```
### -DisableInstanceDiscovery
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.
```yaml
Type: System.Management.Automation.SwitchParameter
Parameter Sets: ClearByKey
Aliases:

Required: False
Position: Named
Default value: None
Accept pipeline input: False
Accept wildcard characters: False
```
### -DisplayBreakingChangeWarning
Controls if warning messages for breaking changes are displayed or suppressed. When enabled, a breaking change warning is displayed when executing cmdlets with breaking changes in a future release.
Expand Down
19 changes: 17 additions & 2 deletions src/Accounts/Accounts/help/Get-AzConfig.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ Gets the configs of Azure PowerShell.
```
Get-AzConfig [-AppliesTo <String>] [-Scope <ConfigScope>] [-DefaultProfile <IAzureContextContainer>]
[-CheckForUpgrade] [-DefaultSubscriptionForLogin] [-DisableErrorRecordsPersistence]
[-DisplayBreakingChangeWarning] [-DisplayRegionIdentified] [-DisplaySecretsWarning]
[-DisplaySurveyMessage] [-EnableDataCollection] [-EnableLoginByWam] [<CommonParameters>]
[-DisableInstanceDiscovery] [-DisplayBreakingChangeWarning] [-DisplayRegionIdentified] [-DisplaySurveyMessage]
[-EnableDataCollection] [-EnableLoginByWam] [<CommonParameters>]
```

## DESCRIPTION
Expand Down Expand Up @@ -142,6 +142,21 @@ Accept pipeline input: False
Accept wildcard characters: False
```
### -DisableInstanceDiscovery
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.
```yaml
Type: System.Management.Automation.SwitchParameter
Parameter Sets: (All)
Aliases:

Required: False
Position: Named
Default value: None
Accept pipeline input: False
Accept wildcard characters: False
```
### -DisplayBreakingChangeWarning
Controls if warning messages for breaking changes are displayed or suppressed. When enabled, a breaking change warning is displayed when executing cmdlets with breaking changes in a future release.
Expand Down
23 changes: 19 additions & 4 deletions src/Accounts/Accounts/help/Update-AzConfig.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ Updates the configs of Azure PowerShell.
```
Update-AzConfig [-AppliesTo <String>] [-Scope <ConfigScope>] [-DefaultProfile <IAzureContextContainer>]
[-WhatIf] [-Confirm] [-CheckForUpgrade <Boolean>] [-DefaultSubscriptionForLogin <String>]
[-DisableErrorRecordsPersistence <Boolean>] [-DisplayBreakingChangeWarning <Boolean>]
[-DisplayRegionIdentified <Boolean>] [-DisplaySecretsWarning <Boolean>]
[-DisplaySurveyMessage <Boolean>] [-EnableDataCollection <Boolean>]
[-EnableLoginByWam <Boolean>] [<CommonParameters>]
[-DisableErrorRecordsPersistence <Boolean>] [-DisableInstanceDiscovery <Boolean>]
[-DisplayBreakingChangeWarning <Boolean>] [-DisplayRegionIdentified <Boolean>]
[-DisplaySurveyMessage <Boolean>] [-EnableDataCollection <Boolean>] [-EnableLoginByWam <Boolean>]
[<CommonParameters>]
```

## DESCRIPTION
Expand Down Expand Up @@ -168,6 +168,21 @@ Accept pipeline input: True (ByPropertyName)
Accept wildcard characters: False
```
### -DisableInstanceDiscovery
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.
```yaml
Type: System.Boolean
Parameter Sets: (All)
Aliases:

Required: False
Position: Named
Default value: None
Accept pipeline input: True (ByPropertyName)
Accept wildcard characters: False
```
### -DisplayBreakingChangeWarning
Controls if warning messages for breaking changes are displayed or suppressed. When enabled, a breaking change warning is displayed when executing cmdlets with breaking changes in a future release.
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 to disable both instance discovery and authority validation.
/// </summary>
internal class DisableInstanceDiscoveryConfig : TypedConfig<bool>
{
public override object DefaultValue => false;

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.

8 changes: 7 additions & 1 deletion src/Accounts/Authentication/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -405,4 +405,10 @@
<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>
</root>
<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>
<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 dc828b7

Please sign in to comment.