From 1bfd994b8b3b54aeaaaeba159662b737a62fbd1e Mon Sep 17 00:00:00 2001 From: Scott Schaab Date: Tue, 1 Sep 2020 10:20:30 -0700 Subject: [PATCH 1/3] [Identity] Throw CredentialUnavailableException from credentials not supporting ADFS --- sdk/identity/Azure.Identity/CHANGELOG.md | 1 + sdk/identity/Azure.Identity/src/Constants.cs | 2 ++ .../src/VisualStudioCodeCredential.cs | 5 ++++ .../src/VisualStudioCredential.cs | 5 ++++ .../tests/VisualStudioCodeCredentialTests.cs | 28 +++++++++++++++++++ .../tests/VisualStudioCredentialTests.cs | 10 +++++++ 6 files changed, 51 insertions(+) create mode 100644 sdk/identity/Azure.Identity/tests/VisualStudioCodeCredentialTests.cs diff --git a/sdk/identity/Azure.Identity/CHANGELOG.md b/sdk/identity/Azure.Identity/CHANGELOG.md index 996201d289e59..5fc906b60987a 100644 --- a/sdk/identity/Azure.Identity/CHANGELOG.md +++ b/sdk/identity/Azure.Identity/CHANGELOG.md @@ -8,6 +8,7 @@ ### Fixes and improvements - Fixed issue with non GUID Client Ids (Issue [#14585](https://github.com/Azure/azure-sdk-for-net/issues/14585)) +- Update `VisualStudioCredential` and `VisualStudioCodeCredential` to throw `CredentialUnavailableException` for ADFS tenant (Issue [#14639](https://github.com/Azure/azure-sdk-for-net/issues/14639)) ## 1.2.2 (2020-08-20) diff --git a/sdk/identity/Azure.Identity/src/Constants.cs b/sdk/identity/Azure.Identity/src/Constants.cs index d1239a09afb30..d726535fd9db1 100644 --- a/sdk/identity/Azure.Identity/src/Constants.cs +++ b/sdk/identity/Azure.Identity/src/Constants.cs @@ -11,6 +11,8 @@ internal class Constants { public const string OrganizationsTenantId = "organizations"; + public const string AdfsTenantId = "adfs"; + // TODO: Currently this is piggybacking off the Azure CLI client ID, but needs to be switched once the Developer Sign On application is available public const string DeveloperSignOnClientId = "04b07795-8ddb-461a-bbee-02f9e1bf7b46"; diff --git a/sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs b/sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs index bab0a91e4a342..50f64cb17eb22 100644 --- a/sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs +++ b/sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs @@ -61,6 +61,11 @@ private async ValueTask GetTokenImplAsync(TokenRequestContext reque try { + if (string.Equals(_tenantId, Constants.AdfsTenantId, StringComparison.Ordinal)) + { + throw new CredentialUnavailableException("VisualStudioCodeCredential authentication unavailable. ADFS tenant / authorities are not supported."); + } + GetUserSettings(out var tenant, out var environmentName); var cloudInstance = GetAzureCloudInstance(environmentName); diff --git a/sdk/identity/Azure.Identity/src/VisualStudioCredential.cs b/sdk/identity/Azure.Identity/src/VisualStudioCredential.cs index cfe47a0be66c3..d34a6e1df598a 100644 --- a/sdk/identity/Azure.Identity/src/VisualStudioCredential.cs +++ b/sdk/identity/Azure.Identity/src/VisualStudioCredential.cs @@ -64,6 +64,11 @@ private async ValueTask GetTokenImplAsync(TokenRequestContext reque try { + if (string.Equals(_tenantId, Constants.AdfsTenantId, StringComparison.Ordinal)) + { + throw new CredentialUnavailableException("VisualStudioCredential authentication unavailable. ADFS tenant/authorities are not supported."); + } + var tokenProviderPath = GetTokenProviderPath(); var tokenProviders = GetTokenProviders(tokenProviderPath); diff --git a/sdk/identity/Azure.Identity/tests/VisualStudioCodeCredentialTests.cs b/sdk/identity/Azure.Identity/tests/VisualStudioCodeCredentialTests.cs new file mode 100644 index 0000000000000..ed45e7c39affa --- /dev/null +++ b/sdk/identity/Azure.Identity/tests/VisualStudioCodeCredentialTests.cs @@ -0,0 +1,28 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Threading; +using Azure.Core; +using Azure.Core.TestFramework; +using NUnit.Framework; + +namespace Azure.Identity.Tests +{ + public class VisualStudioCodeCredentialTests : ClientTestBase + { + public VisualStudioCodeCredentialTests(bool isAsync) : base(isAsync) + { + + } + + [Test] + public void AdfsTenantThrowsCredentialUnavailable() + { + var options = new VisualStudioCodeCredentialOptions { TenantId = "adfs", Transport = new MockTransport() }; + + VisualStudioCodeCredential credential = InstrumentClient(new VisualStudioCodeCredential(options)); + + Assert.ThrowsAsync(async () => await credential.GetTokenAsync(new TokenRequestContext(new[] { "https://vault.azure.net/.default" }), CancellationToken.None)); + } + } +} diff --git a/sdk/identity/Azure.Identity/tests/VisualStudioCredentialTests.cs b/sdk/identity/Azure.Identity/tests/VisualStudioCredentialTests.cs index 08eda9fc5ce3e..24604e707a199 100644 --- a/sdk/identity/Azure.Identity/tests/VisualStudioCredentialTests.cs +++ b/sdk/identity/Azure.Identity/tests/VisualStudioCredentialTests.cs @@ -178,5 +178,15 @@ public void AuthenticateWithVsCredential_CredentialUnavailableExceptionPassThrou var credential = InstrumentClient(new VisualStudioCredential(default, default, fileSystem, testProcessFactory)); Assert.ThrowsAsync(async () => await credential.GetTokenAsync(new TokenRequestContext(new[]{"https://vault.azure.net/"}), CancellationToken.None)); } + + [Test] + public void AdfsTenantThrowsCredentialUnavailable() + { + var options = new VisualStudioCredentialOptions { TenantId = "adfs", Transport = new MockTransport() }; + + VisualStudioCredential credential = InstrumentClient(new VisualStudioCredential(options)); + + Assert.ThrowsAsync(async () => await credential.GetTokenAsync(new TokenRequestContext(new[] { "https://vault.azure.net/.default" }), CancellationToken.None)); + } } } From bf52b33b1607a207547d82153a4f707ab17b29f4 Mon Sep 17 00:00:00 2001 From: Scott Schaab Date: Tue, 1 Sep 2020 15:49:13 -0700 Subject: [PATCH 2/3] moving tenantId check to after it's read from settings --- sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs b/sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs index 50f64cb17eb22..3fae81cd2a842 100644 --- a/sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs +++ b/sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs @@ -61,13 +61,13 @@ private async ValueTask GetTokenImplAsync(TokenRequestContext reque try { + GetUserSettings(out var tenant, out var environmentName); + if (string.Equals(_tenantId, Constants.AdfsTenantId, StringComparison.Ordinal)) { throw new CredentialUnavailableException("VisualStudioCodeCredential authentication unavailable. ADFS tenant / authorities are not supported."); } - GetUserSettings(out var tenant, out var environmentName); - var cloudInstance = GetAzureCloudInstance(environmentName); var storedCredentials = _vscAdapter.GetCredentials(CredentialsSection, environmentName); From 4c0321287f775679838000ac21bb53f4cdbdd6bd Mon Sep 17 00:00:00 2001 From: Scott Schaab Date: Tue, 1 Sep 2020 15:51:11 -0700 Subject: [PATCH 3/3] fix check to use local variable --- sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs b/sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs index 3fae81cd2a842..ef29cb99b82e5 100644 --- a/sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs +++ b/sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs @@ -63,7 +63,7 @@ private async ValueTask GetTokenImplAsync(TokenRequestContext reque { GetUserSettings(out var tenant, out var environmentName); - if (string.Equals(_tenantId, Constants.AdfsTenantId, StringComparison.Ordinal)) + if (string.Equals(tenant, Constants.AdfsTenantId, StringComparison.Ordinal)) { throw new CredentialUnavailableException("VisualStudioCodeCredential authentication unavailable. ADFS tenant / authorities are not supported."); }