Skip to content
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

ManagedIdentityClient uses AZURE_CLIENT_ID when available #19062

Closed
Closed
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
17 changes: 6 additions & 11 deletions sdk/identity/Azure.Identity/src/DefaultAzureCredentialOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class DefaultAzureCredentialOptions : TokenCredentialOptions
/// <see cref="InteractiveBrowserCredential"/>. The default is null and will authenticate users to their default tenant.
/// The value can also be set by setting the environment variable AZURE_TENANT_ID.
/// </summary>
public string InteractiveBrowserTenantId { get; set; } = GetNonEmptyStringOrNull(EnvironmentVariables.TenantId);
public string InteractiveBrowserTenantId { get; set; } = EnvironmentVariables.GetNonEmptyStringOrNull(EnvironmentVariables.TenantId);

/// <summary>
/// Specifies the tenant id of the preferred authentication account, to be retrieved from the shared token cache for single sign on authentication with
Expand All @@ -25,21 +25,21 @@ public class DefaultAzureCredentialOptions : TokenCredentialOptions
/// If multiple accounts are found in the shared token cache and no value is specified, or the specified value matches no accounts in
/// the cache the SharedTokenCacheCredential will not be used for authentication.
/// </remarks>
public string SharedTokenCacheTenantId { get; set; } = GetNonEmptyStringOrNull(EnvironmentVariables.TenantId);
public string SharedTokenCacheTenantId { get; set; } = EnvironmentVariables.GetNonEmptyStringOrNull(EnvironmentVariables.TenantId);

/// <summary>
/// The tenant id of the user to authenticate, in the case the <see cref="DefaultAzureCredential"/> authenticates through, the
/// <see cref="VisualStudioCredential"/>. The default is null and will authenticate users to their default tenant.
/// The value can also be set by setting the environment variable AZURE_TENANT_ID.
/// </summary>
public string VisualStudioTenantId { get; set; } = GetNonEmptyStringOrNull(EnvironmentVariables.TenantId);
public string VisualStudioTenantId { get; set; } = EnvironmentVariables.GetNonEmptyStringOrNull(EnvironmentVariables.TenantId);

/// <summary>
/// The tenant id of the user to authenticate, in the case the <see cref="DefaultAzureCredential"/> authenticates through, the
/// <see cref="VisualStudioCodeCredential"/>. The default is null and will authenticate users to their default tenant.
/// The value can also be set by setting the environment variable AZURE_TENANT_ID.
/// </summary>
public string VisualStudioCodeTenantId { get; set; } = GetNonEmptyStringOrNull(EnvironmentVariables.TenantId);
public string VisualStudioCodeTenantId { get; set; } = EnvironmentVariables.GetNonEmptyStringOrNull(EnvironmentVariables.TenantId);

/// <summary>
/// Specifies the preferred authentication account to be retrieved from the shared token cache for single sign on authentication with
Expand All @@ -49,12 +49,12 @@ public class DefaultAzureCredentialOptions : TokenCredentialOptions
/// If multiple accounts are found in the shared token cache and no value is specified, or the specified value matches no accounts in
/// the cache the SharedTokenCacheCredential will not be used for authentication.
/// </remarks>
public string SharedTokenCacheUsername { get; set; } = GetNonEmptyStringOrNull(EnvironmentVariables.Username);
public string SharedTokenCacheUsername { get; set; } = EnvironmentVariables.GetNonEmptyStringOrNull(EnvironmentVariables.Username);

/// <summary>
/// Specifies the client id of the azure ManagedIdentity in the case of user assigned identity.
/// </summary>
public string ManagedIdentityClientId { get; set; } = GetNonEmptyStringOrNull(EnvironmentVariables.ClientId);
public string ManagedIdentityClientId { get; set; } = EnvironmentVariables.GetNonEmptyStringOrNull(EnvironmentVariables.ClientId);

/// <summary>
/// Specifies whether the <see cref="EnvironmentCredential"/> will be excluded from the authentication flow. Setting to true disables reading
Expand Down Expand Up @@ -94,10 +94,5 @@ public class DefaultAzureCredentialOptions : TokenCredentialOptions
/// Specifies whether the <see cref="VisualStudioCodeCredential"/> will be excluded from the <see cref="DefaultAzureCredential"/> authentication flow.
/// </summary>
public bool ExcludeVisualStudioCodeCredential { get; set; }

private static string GetNonEmptyStringOrNull(string str)
{
return !string.IsNullOrEmpty(str) ? str : null;
}
}
}
5 changes: 5 additions & 0 deletions sdk/identity/Azure.Identity/src/EnvironmentVariables.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,10 @@ internal class EnvironmentVariables
public static string ProgramFilesX86 => Environment.GetEnvironmentVariable("ProgramFiles(x86)");
public static string ProgramFiles => Environment.GetEnvironmentVariable("ProgramFiles");
public static string AuthorityHost => Environment.GetEnvironmentVariable("AZURE_AUTHORITY_HOST");

public static string GetNonEmptyStringOrNull(string str)
{
return !string.IsNullOrEmpty(str) ? str : null;
}
}
}
5 changes: 2 additions & 3 deletions sdk/identity/Azure.Identity/src/ManagedIdentityClient.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System.Text.Json;
using System.Threading;
using System.Threading.Tasks;
using Azure.Core;
Expand All @@ -27,13 +26,13 @@ public ManagedIdentityClient(CredentialPipeline pipeline, string clientId = null
public ManagedIdentityClient(ManagedIdentityClientOptions options)
{
_options = options;
ClientId = options.ClientId;
ClientId = options.ClientId ?? EnvironmentVariables.GetNonEmptyStringOrNull(EnvironmentVariables.ClientId);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to change this to read the environment variable. By design only the DefaultAzureCredential and EnvironmentCredential are configurable through the environment.

Pipeline = options.Pipeline;
}

internal CredentialPipeline Pipeline { get; }

protected string ClientId { get; }
protected internal string ClientId { get; }

public virtual async ValueTask<AccessToken> AuthenticateAsync(bool async, TokenRequestContext context, CancellationToken cancellationToken)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ internal class ManagedIdentityClientOptions
{
public TokenCredentialOptions Options { get; set; }

public string ClientId { get; set; }
public string ClientId { get; set; } = EnvironmentVariables.GetNonEmptyStringOrNull(EnvironmentVariables.ClientId);

public bool PreserveTransport { get; set; }

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using Azure.Identity.Tests;
using NUnit.Framework;

namespace Azure.Identity.Tests
{
public class ManagedIdentityClientOptionsTests
{
public const string ExplicitClientId = "1234";
public const string ClientIdFromEnvironment = "4567";

[Test]
public void ClientIdIsReadFromEnvironmentVariableWhenAvailable(
[Values(null, ExplicitClientId)] string explicitClientId,
[Values(null, ClientIdFromEnvironment)] string clientIdFromEnvironment)
{
using (new TestEnvVar(new ()
{
{ "AZURE_CLIENT_ID", clientIdFromEnvironment }
}))
{
var options = new ManagedIdentityClientOptions();
if (explicitClientId != null)
{
options.ClientId = explicitClientId;
}

string expectedClientId = explicitClientId switch
{
null when clientIdFromEnvironment is null => null,
null when clientIdFromEnvironment is not null => clientIdFromEnvironment,
_ => explicitClientId,
};

Assert.That(options.ClientId, Is.EqualTo(expectedClientId));
}
}
}
}
36 changes: 36 additions & 0 deletions sdk/identity/Azure.Identity/tests/ManagedIdentityClientTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using Azure.Identity.Tests;
using NUnit.Framework;

namespace Azure.Identity.Tests
{
public class ManagedIdentityClientTests
{
public const string ClientIdFromCtor = "1234";
public const string ClientIdFromEnvironment = "4567";

[Test]
public void ClientIdIsReadFromEnvironmentVariableWhenAvailable(
[Values(null, ClientIdFromCtor)] string clientIdFromCtor,
[Values(null, ClientIdFromEnvironment)] string clientIdFromEnvironment)
{
using (new TestEnvVar(new ()
{
{ "AZURE_CLIENT_ID", clientIdFromEnvironment }}))
{
var client = new ManagedIdentityClient(default, clientIdFromCtor);

string expectedClientId = clientIdFromCtor switch
{
null when clientIdFromEnvironment is null => null,
null when clientIdFromEnvironment is not null => clientIdFromEnvironment,
_ => clientIdFromCtor,
};

Assert.That(client.ClientId, Is.EqualTo(expectedClientId));
}
}
}
}