Fix for #1750 - Hardcode tenant and authorize endpoints instead of OI… (

#1816) * Fix for #1750 - Hardcode tenant and authorize endpoints instead of OIDC discovery * fix test
AzureAD · May 15, 2020 · 058554c · 058554c
1 parent 59546f4
commit 058554c
Show file tree

Hide file tree

Showing 49 changed files with 269 additions and 774 deletions.
diff --git a/src/client/Microsoft.Identity.Client/Instance/AadAuthority.cs b/src/client/Microsoft.Identity.Client/Instance/AadAuthority.cs
@@ -4,8 +4,6 @@
 using System;
 using System.Collections.Generic;
 using System.Globalization;
-using Microsoft.Identity.Client.Core;
-using Microsoft.Identity.Client.Utils;
 
 namespace Microsoft.Identity.Client.Instance
 {
@@ -14,6 +12,10 @@ internal class AadAuthority : Authority
         public const string DefaultTrustedHost = "login.microsoftonline.com";
         public const string AADCanonicalAuthorityTemplate = "https://{0}/{1}/";
 
+        private const string TokenEndpointTemplate = "{0}oauth2/v2.0/token";
+        private const string DeviceCodeEndpointTemplate = "{0}oauth2/v2.0/devicecode";
+        private const string AuthorizationEndpointTemplate = "{0}oauth2/v2.0/authorize";
+
         private static readonly ISet<string> s_tenantlessTenantNames = new HashSet<string>(
           new[]
           {
@@ -25,19 +27,15 @@ internal class AadAuthority : Authority
 
         internal AadAuthority(AuthorityInfo authorityInfo) : base(authorityInfo)
         {
+            TenantId = GetFirstPathSegment(AuthorityInfo.CanonicalAuthority);
         }
 
-        internal override string GetTenantId()
-        {
-            return GetFirstPathSegment(AuthorityInfo.CanonicalAuthority);
-        }
+        internal override string TenantId { get; }
 
         internal bool IsCommonOrganizationsOrConsumersTenant()
         {
-            string tenantId = this.GetTenantId();
-
-            return !string.IsNullOrEmpty(tenantId) &&
-                s_tenantlessTenantNames.Contains(tenantId);
+            return !string.IsNullOrEmpty(TenantId) &&
+                s_tenantlessTenantNames.Contains(TenantId);
         }
 
         internal override string GetTenantedAuthority(string tenantId)
@@ -56,5 +54,24 @@ internal override string GetTenantedAuthority(string tenantId)
 
             return AuthorityInfo.CanonicalAuthority;
         }
+
+        internal override AuthorityEndpoints GetHardcodedEndpoints()
+        {
+            string tokenEndpoint = string.Format(
+                    CultureInfo.InvariantCulture,
+                    TokenEndpointTemplate,
+                    AuthorityInfo.CanonicalAuthority);
+
+            string authorizationEndpoint = string.Format(CultureInfo.InvariantCulture,
+                    AuthorizationEndpointTemplate,
+                    AuthorityInfo.CanonicalAuthority);
+
+            string deviceEndpoint = string.Format(
+                CultureInfo.InvariantCulture,
+                DeviceCodeEndpointTemplate,
+                AuthorityInfo.CanonicalAuthority);
+
+            return new AuthorityEndpoints(authorizationEndpoint, tokenEndpoint, deviceEndpoint);
+        }
     }
 }
diff --git a/src/client/Microsoft.Identity.Client/Instance/AdfsAuthority.cs b/src/client/Microsoft.Identity.Client/Instance/AdfsAuthority.cs
@@ -1,12 +1,15 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
-
-using System.Collections.Generic;
+using System.Globalization;
 
 namespace Microsoft.Identity.Client.Instance
 {
     internal class AdfsAuthority : Authority
     {
+        private const string TokenEndpointTemplate = "{0}oauth2/token";
+        private const string AuthorizationEndpointTemplate = "{0}oauth2/authorize";
+        private const string DeviceCodeEndpointTemplate = "{0}oauth2/devicecode";
+
         public AdfsAuthority(AuthorityInfo authorityInfo)
             : base(authorityInfo)
         {
@@ -19,9 +22,25 @@ internal override string GetTenantedAuthority(string tenantId)
             return AuthorityInfo.CanonicalAuthority;
         }
 
-        internal override string GetTenantId()
+        internal override AuthorityEndpoints GetHardcodedEndpoints()
         {
-            return null;
+            string tokenEndpoint = string.Format(
+                   CultureInfo.InvariantCulture,
+                   TokenEndpointTemplate,
+                   AuthorityInfo.CanonicalAuthority);
+
+            string deviceEndpoint = string.Format(
+                  CultureInfo.InvariantCulture,
+                  DeviceCodeEndpointTemplate,
+                  AuthorityInfo.CanonicalAuthority);
+
+            string authEndpoint = string.Format(CultureInfo.InvariantCulture,
+                    AuthorizationEndpointTemplate,
+                    AuthorityInfo.CanonicalAuthority);
+
+            return new AuthorityEndpoints(authEndpoint, tokenEndpoint, deviceEndpoint);
         }
+
+        internal override string TenantId => null;
     }
 }
diff --git a/src/client/Microsoft.Identity.Client/Instance/Authority.cs b/src/client/Microsoft.Identity.Client/Instance/Authority.cs
@@ -5,9 +5,9 @@
 
 namespace Microsoft.Identity.Client.Instance
 {
-    /// <summary>
-    ///
-    /// </summary>
+    /// <remarks>
+    /// Must be kept immutable
+    /// </remarks>
     internal abstract class Authority
     {
         protected Authority(AuthorityInfo authorityInfo)
@@ -196,14 +196,19 @@ internal static AuthorityType GetAuthorityType(string authority)
             }
         }
 
-        internal abstract string GetTenantId();
+        internal abstract string TenantId { get; }
 
         /// <summary>
         /// Gets a tenanted authority if the current authority is tenantless.
         /// Returns the original authority on B2C and ADFS
         /// </summary>
         internal abstract string GetTenantedAuthority(string tenantId);
 
+        /// <summary>
+        /// Gets the authority endpoints based on the host and tenant id. Does not rely on OIDC discovery.
+        /// </summary>
+        internal abstract AuthorityEndpoints GetHardcodedEndpoints();
+
         private static void CheckB2CAuthorityHost(AuthorityInfo requestAuthorityInfo, AuthorityInfo configAuthorityInfo)
         {
             if (configAuthorityInfo.Host != requestAuthorityInfo.Host)

diff --git a/src/client/Microsoft.Identity.Client/Instance/AuthorityEndpointResolutionManager.cs b/src/client/Microsoft.Identity.Client/Instance/AuthorityEndpointResolutionManager.cs
@@ -1,19 +1,17 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
-using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
-using System.Net.Http;
 using System.Threading.Tasks;
 using Microsoft.Identity.Client.Core;
-using Microsoft.Identity.Client.OAuth2;
-using Microsoft.Identity.Client.PlatformsCommon.Shared;
-using Microsoft.Identity.Client.TelemetryCore.Internal;
-using Microsoft.Identity.Client.Utils;
+using Microsoft.Identity.Client.Instance.Validation;
 
 namespace Microsoft.Identity.Client.Instance
 {
+    /// <summary>
+    /// Responsible for figuring out authority endpoints
+    /// </summary>
     internal class AuthorityEndpointResolutionManager : IAuthorityEndpointResolutionManager
     {
         private static readonly ConcurrentDictionary<string, AuthorityEndpointCacheEntry> s_endpointCacheEntries =
@@ -43,60 +41,19 @@ public async Task<AuthorityEndpoints> ResolveEndpointsAsync(
 
             requestContext.Logger.Info(LogMessages.ResolvingAuthorityEndpointsFalse);
 
-            var endpointManager = OpenIdConfigurationEndpointManagerFactory.Create(authorityInfo, _serviceBundle);
+            var validator = AuthorityValidatorFactory.Create(authorityInfo, _serviceBundle);
 
-            string openIdConfigurationEndpoint = await endpointManager.ValidateAuthorityAndGetOpenIdDiscoveryEndpointAsync(
-                                                     authorityInfo,
-                                                     userPrincipalName,
-                                                     requestContext).ConfigureAwait(false);
-
-            // Discover endpoints via openid-configuration
-            var edr = await DiscoverEndpointsAsync(openIdConfigurationEndpoint, requestContext).ConfigureAwait(false);
-
-            if (string.IsNullOrEmpty(edr.AuthorizationEndpoint))
-            {
-                throw new MsalClientException(
-                    MsalError.TenantDiscoveryFailedError,
-                    MsalErrorMessage.AuthorizeEndpointWasNotFoundInTheOpenIdConfiguration);
-            }
-
-            if (string.IsNullOrEmpty(edr.TokenEndpoint))
-            {
-                throw new MsalClientException(
-                    MsalError.TenantDiscoveryFailedError,
-                    MsalErrorMessage.TokenEndpointWasNotFoundInTheOpenIdConfiguration);
-            }
-
-            if (string.IsNullOrEmpty(edr.Issuer))
-            {
-                throw new MsalClientException(
-                    MsalError.TenantDiscoveryFailedError,
-                    MsalErrorMessage.IssuerWasNotFoundInTheOpenIdConfiguration);
-            }
+            // TODO: move this away from here?
+            await validator.ValidateAuthorityAsync(authorityInfo, requestContext).ConfigureAwait(false);
 
+            //TODO: stop using AuthorityInfo on its own
             var authority = Authority.CreateAuthority(authorityInfo);
-            var tenantId = authority.GetTenantId();
-
-            string authorizationEndpoint = ReplaceTenantToken(edr.AuthorizationEndpoint, tenantId);
-            string tokenEndpoint = ReplaceTenantToken(edr.TokenEndpoint, tenantId);
-
-            endpoints = new AuthorityEndpoints(
-                authorizationEndpoint,
-                tokenEndpoint,
-                tokenEndpoint);
 
+            endpoints = authority.GetHardcodedEndpoints();
             Add(authorityInfo, userPrincipalName, endpoints);
             return endpoints;
         }
 
-        private static string ReplaceTenantToken(string template, string tenantId)
-        {
-            // some templates use {tenant}, some {tenantid}
-            template = template.Replace(Constants.Tenant, tenantId, StringComparison.OrdinalIgnoreCase);
-            template = template.Replace(Constants.TenantId, tenantId, StringComparison.OrdinalIgnoreCase);
-            return template;
-        }
-
         private bool TryGetCacheValue(AuthorityInfo authorityInfo, string userPrincipalName, out AuthorityEndpoints endpoints)
         {
             endpoints = null;
@@ -112,12 +69,10 @@ private bool TryGetCacheValue(AuthorityInfo authorityInfo, string userPrincipalN
                 return true;
             }
 
-            if (!string.IsNullOrEmpty(userPrincipalName))
+            if (!string.IsNullOrEmpty(userPrincipalName) &&
+                !cacheEntry.ValidForDomainsList.Contains(AdfsUpnHelper.GetDomainFromUpn(userPrincipalName)))
             {
-                if (!cacheEntry.ValidForDomainsList.Contains(AdfsUpnHelper.GetDomainFromUpn(userPrincipalName)))
-                {
-                    return false;
-                }
+                return false;
             }
 
             endpoints = cacheEntry.Endpoints;
@@ -149,17 +104,6 @@ private void Add(AuthorityInfo authorityInfo, string userPrincipalName, Authorit
             s_endpointCacheEntries.TryAdd(authorityInfo.CanonicalAuthority, updatedCacheEntry);
         }
 
-        private async Task<TenantDiscoveryResponse> DiscoverEndpointsAsync(
-             string openIdConfigurationEndpoint,
-             RequestContext requestContext)
-        {
-            var client = new OAuth2Client(requestContext.Logger, _serviceBundle.HttpManager, _serviceBundle.MatsTelemetryManager);
-            return await client.ExecuteRequestAsync<TenantDiscoveryResponse>(
-                       new Uri(openIdConfigurationEndpoint),
-                       HttpMethod.Get,
-                       requestContext).ConfigureAwait(false);
-        }
-
         private class AuthorityEndpointCacheEntry
         {
             public AuthorityEndpointCacheEntry(AuthorityEndpoints endpoints)

diff --git a/src/client/Microsoft.Identity.Client/Instance/AuthorityEndpoints.cs b/src/client/Microsoft.Identity.Client/Instance/AuthorityEndpoints.cs
@@ -9,16 +9,18 @@ namespace Microsoft.Identity.Client.Instance
 {
     internal class AuthorityEndpoints
     {
-        public AuthorityEndpoints(string authorizationEndpoint, string tokenEndpoint, string selfSignedJwtAudience)
+        public AuthorityEndpoints(string authorizationEndpoint, string tokenEndpoint, string deviceCodeEndpoint)
         {
             AuthorizationEndpoint = authorizationEndpoint;
             TokenEndpoint = tokenEndpoint;
-            SelfSignedJwtAudience = selfSignedJwtAudience;
+            SelfSignedJwtAudience = tokenEndpoint;
+            DeviceCodeEndpoint = deviceCodeEndpoint;
         }
 
         public string AuthorizationEndpoint { get; }
         public string TokenEndpoint { get; }
         public string SelfSignedJwtAudience { get; }
+        public string DeviceCodeEndpoint { get; }
 
         public static async Task UpdateAuthorityEndpointsAsync(
             AuthenticationRequestParameters requestParameters)

diff --git a/src/client/Microsoft.Identity.Client/Instance/B2CAuthority.cs b/src/client/Microsoft.Identity.Client/Instance/B2CAuthority.cs
@@ -2,10 +2,6 @@
 // Licensed under the MIT License.
 
 using System;
-using System.Collections.Generic;
-using System.Globalization;
-using System.Threading.Tasks;
-using Microsoft.Identity.Client.Core;
 
 namespace Microsoft.Identity.Client.Instance
 {
@@ -17,12 +13,10 @@ internal class B2CAuthority : AadAuthority
         internal B2CAuthority(AuthorityInfo authorityInfo)
             : base(authorityInfo)
         {
+            TenantId = new Uri(AuthorityInfo.CanonicalAuthority).Segments[2].TrimEnd('/');
         }
 
-        internal override string GetTenantId()
-        {
-            return new Uri(AuthorityInfo.CanonicalAuthority).Segments[2].TrimEnd('/');
-        }
+        internal override string TenantId { get; }
 
         internal override string GetTenantedAuthority(string tenantId)
         {

diff --git a/src/client/Microsoft.Identity.Client/Instance/B2COpenIdConfigurationEndpointManager.cs b/src/client/Microsoft.Identity.Client/Instance/B2COpenIdConfigurationEndpointManager.cs
diff --git a/src/client/Microsoft.Identity.Client/Instance/DrsMetadataResponse.cs b/src/client/Microsoft.Identity.Client/Instance/DrsMetadataResponse.cs
diff --git a/src/client/Microsoft.Identity.Client/Instance/IOpenIdConfigurationEndpointManager.cs b/src/client/Microsoft.Identity.Client/Instance/IOpenIdConfigurationEndpointManager.cs