diff --git a/src/client/Microsoft.Identity.Client/Instance/Discovery/InstanceDiscoveryManager.cs b/src/client/Microsoft.Identity.Client/Instance/Discovery/InstanceDiscoveryManager.cs index a42d8c4103..70969a3cdb 100644 --- a/src/client/Microsoft.Identity.Client/Instance/Discovery/InstanceDiscoveryManager.cs +++ b/src/client/Microsoft.Identity.Client/Instance/Discovery/InstanceDiscoveryManager.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Net.Http; using System.Threading.Tasks; using Microsoft.Identity.Client.Core; using Microsoft.Identity.Client.Http; @@ -196,28 +197,26 @@ private async Task FetchNetworkMetadataOrFallbac { return await _networkMetadataProvider.GetMetadataAsync(authorityUri, requestContext).ConfigureAwait(false); } - catch (MsalServiceException ex) + catch (MsalServiceException ex) when (ex.ErrorCode == MsalError.InvalidInstance) { if (!requestContext.ServiceBundle.Config.Authority.AuthorityInfo.ValidateAuthority) { - requestContext.Logger.Info("[Instance Discovery] Skipping Instance discovery as validate authority is set to false. "); + requestContext.Logger.Info($"[Instance Discovery] Skipping Instance discovery as validate authority is set to false. {ex}"); return CreateEntryForSingleAuthority(authorityUri); } - // Validate Authority exception - if (ex.ErrorCode == MsalError.InvalidInstance) - { - requestContext.Logger.Error("[Instance Discovery] Instance discovery failed - invalid instance!"); - throw; - } - - string message = - "[Instance Discovery] Instance Discovery failed. Potential cause: no network connection or discovery endpoint is busy. See exception below. MSAL will continue without network instance metadata. "; - - requestContext.Logger.WarningPii(message + " Authority: " + authorityUri, message); - requestContext.Logger.WarningPii(ex); - - return _knownMetadataProvider.GetMetadata(authorityUri.Host, Enumerable.Empty(), requestContext.Logger); + requestContext.Logger.Error($"[Instance Discovery] Instance discovery failed - invalid instance! "); + throw; + } + catch (Exception e) + { + requestContext.Logger.Warning( + $"[Instance Discovery] Instance Discovery failed. MSAL will continue without network instance metadata. \n\r" + + $" Exception: {e} "); + + return + _knownMetadataProvider.GetMetadata(authorityUri.Host, Enumerable.Empty(), requestContext.Logger) + ?? CreateEntryForSingleAuthority(authorityUri); } } diff --git a/tests/Microsoft.Identity.Test.Common/Core/Mocks/MockHttpManager.cs b/tests/Microsoft.Identity.Test.Common/Core/Mocks/MockHttpManager.cs index 9f0bf34cae..782e9b172c 100644 --- a/tests/Microsoft.Identity.Test.Common/Core/Mocks/MockHttpManager.cs +++ b/tests/Microsoft.Identity.Test.Common/Core/Mocks/MockHttpManager.cs @@ -31,8 +31,7 @@ internal sealed class MockHttpManager : IHttpManager, public MockHttpManager( bool disableInternalRetries = false, string testName = null, - Func messageHandlerFunc = null, - Func validateServerCertificateCallback = null, + Func messageHandlerFunc = null, bool invokeNonMtlsHttpManagerFactory = false) { _httpManager = invokeNonMtlsHttpManagerFactory @@ -160,7 +159,6 @@ protected HttpClient GetHttpClientInternal(X509Certificate2 mtlsBindingCert) { messageHandler.ClientCertificates.Add(mtlsBindingCert); } - var httpClient = new HttpClient(messageHandler) { MaxResponseContentBufferSize = HttpClientConfig.MaxResponseContentBufferSizeInBytes diff --git a/tests/Microsoft.Identity.Test.Unit/PublicApiTests/ConfidentialClientApplicationTests.cs b/tests/Microsoft.Identity.Test.Unit/PublicApiTests/ConfidentialClientApplicationTests.cs index 8ccf3568df..e7f7e7a0ff 100644 --- a/tests/Microsoft.Identity.Test.Unit/PublicApiTests/ConfidentialClientApplicationTests.cs +++ b/tests/Microsoft.Identity.Test.Unit/PublicApiTests/ConfidentialClientApplicationTests.cs @@ -151,39 +151,7 @@ public async Task ConfidentialClientUsingSecretNoCacheProvidedTestAsync() appCacheAccess.AssertAccessCounts(1, 1); userCacheAccess.AssertAccessCounts(0, 0); } - } - - [TestMethod] - public async Task ConfidentialClientUsingSecretNoInstanceDiscoveryTestAsync() - { - using (var httpManager = new MockHttpManager()) - { - - var app = ConfidentialClientApplicationBuilder.Create(TestConstants.ClientId) - .WithAuthority(new Uri(ClientApplicationBase.DefaultAuthority), true) - .WithRedirectUri(TestConstants.RedirectUri) - .WithClientSecret(TestConstants.ClientSecret) - .WithHttpManager(httpManager) - .WithInstanceDiscovery(false) - .BuildConcrete(); - - var appCacheAccess = app.AppTokenCache.RecordAccess(); - var userCacheAccess = app.UserTokenCache.RecordAccess(); - - httpManager.AddMockHandlerSuccessfulClientCredentialTokenResponseMessage(); - - var result = await app.AcquireTokenForClient(TestConstants.s_scope.ToArray()).ExecuteAsync(CancellationToken.None).ConfigureAwait(false); - Assert.IsNotNull(result); - Assert.IsNotNull("header.payload.signature", result.AccessToken); - Assert.AreEqual(TestConstants.s_scope.AsSingleString(), result.Scopes.AsSingleString()); - - Assert.IsNotNull(app.UserTokenCache); - Assert.IsNotNull(app.AppTokenCache); - - appCacheAccess.AssertAccessCounts(1, 1); - userCacheAccess.AssertAccessCounts(0, 0); - } - } + } [TestMethod] [TestCategory(TestCategories.Regression)] diff --git a/tests/Microsoft.Identity.Test.Unit/PublicApiTests/InstanceDiscoveryTests.cs b/tests/Microsoft.Identity.Test.Unit/PublicApiTests/InstanceDiscoveryTests.cs new file mode 100644 index 0000000000..6f8a9273c7 --- /dev/null +++ b/tests/Microsoft.Identity.Test.Unit/PublicApiTests/InstanceDiscoveryTests.cs @@ -0,0 +1,131 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Globalization; +using System.IdentityModel.Tokens.Jwt; +using System.Linq; +using System.Net.Http; +using System.Security.Cryptography.X509Certificates; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Identity.Client; +using Microsoft.Identity.Client.RP; +using Microsoft.Identity.Client.Cache; +using Microsoft.Identity.Client.Internal; +using Microsoft.Identity.Client.Internal.ClientCredential; +using Microsoft.Identity.Client.OAuth2; +using Microsoft.Identity.Client.PlatformsCommon.Shared; +using Microsoft.Identity.Client.Utils; +using Microsoft.Identity.Test.Common; +using Microsoft.Identity.Test.Common.Core.Helpers; +using Microsoft.Identity.Test.Common.Core.Mocks; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using NSubstitute; +using Microsoft.Identity.Client.Extensibility; +using System.Net; +using System.Security.Cryptography; +using System.Text; + +namespace Microsoft.Identity.Test.Unit.PublicApiTests +{ + [TestClass] + public class InstanceDiscoveryTests : TestBase + { + [TestMethod] + public async Task ConfidentialClientUsingSecretNoInstanceDiscoveryTestAsync() + { + using (var httpManager = new MockHttpManager()) + { + + var app = ConfidentialClientApplicationBuilder.Create(TestConstants.ClientId) + .WithAuthority(new Uri(ClientApplicationBase.DefaultAuthority), true) + .WithRedirectUri(TestConstants.RedirectUri) + .WithClientSecret(TestConstants.ClientSecret) + .WithHttpManager(httpManager) + .WithInstanceDiscovery(false) + .BuildConcrete(); + + var appCacheAccess = app.AppTokenCache.RecordAccess(); + var userCacheAccess = app.UserTokenCache.RecordAccess(); + + httpManager.AddMockHandlerSuccessfulClientCredentialTokenResponseMessage(); + + var result = await app.AcquireTokenForClient(TestConstants.s_scope.ToArray()).ExecuteAsync(CancellationToken.None).ConfigureAwait(false); + Assert.IsNotNull(result); + Assert.IsNotNull("header.payload.signature", result.AccessToken); + Assert.AreEqual(TestConstants.s_scope.AsSingleString(), result.Scopes.AsSingleString()); + + Assert.IsNotNull(app.UserTokenCache); + Assert.IsNotNull(app.AppTokenCache); + + appCacheAccess.AssertAccessCounts(1, 1); + userCacheAccess.AssertAccessCounts(0, 0); + } + } + + [TestMethod] + [WorkItem(5545)] // https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/5545 + public async Task NoInstanceDiscovery_AirgappedCould_TestAsync() + { + using (var httpManager = new MockHttpManager(disableInternalRetries: true)) + using (new EnvVariableContext()) + { + Environment.SetEnvironmentVariable("REGION_NAME", TestConstants.Region); + + // Instance discovery explicitly disabled + var app = ConfidentialClientApplicationBuilder.Create(TestConstants.ClientId) + .WithAuthority(TestConstants.AuthorityNotKnownTenanted) + .WithClientSecret(TestConstants.ClientSecret) + .WithHttpManager(httpManager) + .WithAzureRegion(ConfidentialClientApplication.AttemptRegionDiscovery) + .WithInstanceDiscovery(false) + .Build(); + + // Direct token request (no instance discovery mock!) + httpManager.AddMockHandlerSuccessfulClientCredentialTokenResponseMessage(); + + // Act + var result = await app.AcquireTokenForClient(TestConstants.s_scope.ToArray()) + .ExecuteAsync(CancellationToken.None) + .ConfigureAwait(false); + + // Assert happens when httpManager disposes and checks for unconsumed handlers + } + } + + [TestMethod] + [WorkItem(5546)] // https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/5546 + public async Task HttpErrorsInDiscoveryShouldBeIgnored_AirgappedCould_TestAsync() + { + using (var httpManager = new MockHttpManager(disableInternalRetries: true)) + using (new EnvVariableContext()) + { + Environment.SetEnvironmentVariable("REGION_NAME", TestConstants.Region); + + // Instance discovery explicitly disabled + var app = ConfidentialClientApplicationBuilder.Create(TestConstants.ClientId) + .WithAuthority(TestConstants.AuthorityNotKnownTenanted) + .WithClientSecret(TestConstants.ClientSecret) + .WithHttpManager(httpManager) + .WithAzureRegion(ConfidentialClientApplication.AttemptRegionDiscovery) + .Build(); + + httpManager.AddMockHandler( + new MockHttpMessageHandler() + { + ExceptionToThrow = new HttpRequestException("Simulated network error") + }); + httpManager.AddMockHandlerSuccessfulClientCredentialTokenResponseMessage(); + + // Act + var result = await app.AcquireTokenForClient(TestConstants.s_scope.ToArray()) + .ExecuteAsync(CancellationToken.None) + .ConfigureAwait(false); + + // Assert happens when httpManager disposes and checks for unconsumed handlers + } + } + } +}