general code cleanup focus on AadIssuerValidator (#79)

Support JwtSecurityToken and JsonWebToken
Get 'tid' right from payload, no need to create claims
Use ConcurrentDictionary
Create ConfigurationManager once on startup.
Set the dictionary rather than Add as duplicates will fault.
Add try / catch to stop faulting
System usings first

Author: jmprieur

Microsoft.Identity.Web/Constants.cs renamed to Microsoft.Identity.Web/ClaimConstants.cs

@@ -25,12 +25,15 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 namespace Microsoft.Identity.Web
 {
     /// <summary>
-    /// claim keys constants
+    /// Constants for claim types.
     /// </summary>
     public static class ClaimConstants
     {
+        public const string Name = "name";
         public const string ObjectId = "http://schemas.microsoft.com/identity/claims/objectidentifier";
+        public const string Oid = "oid";
+        public const string PreferredUserName = "preferred_username";
         public const string TenantId = "http://schemas.microsoft.com/identity/claims/tenantid";
-        public const string tid = "tid";
+        public const string Tid = "tid";
     }
-}
+}

Microsoft.Identity.Web/ClaimPrincipalExtension.cs

@@ -1,20 +1,43 @@
-using Microsoft.Identity.Client;
+/************************************************************************************************
+The MIT License (MIT)
+
+Copyright (c) 2015 Microsoft Corporation
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.
+***********************************************************************************************/
+
+using Microsoft.Identity.Client;
 using System.Security.Claims;
 
 namespace Microsoft.Identity.Web
 {
     public static class ClaimsPrincipalExtension
     {
         /// <summary>
-        /// Get the Account identifier for an MSAL.NET account from a ClaimsPrincipal
+        /// Gets the Account identifier for an MSAL.NET account from a <see cref="ClaimsPrincipal"/>
         /// </summary>
         /// <param name="claimsPrincipal">Claims principal</param>
         /// <returns>A string corresponding to an account identifier as defined in <see cref="Microsoft.Identity.Client.AccountId.Identifier"/></returns>
         public static string GetMsalAccountId(this ClaimsPrincipal claimsPrincipal)
         {
             string userObjectId = GetObjectId(claimsPrincipal);
             string tenantId = GetTenantId(claimsPrincipal);
-
             if (!string.IsNullOrWhiteSpace(userObjectId) && !string.IsNullOrWhiteSpace(tenantId))
             {
                 return $"{userObjectId}.{tenantId}";
@@ -24,41 +47,37 @@ public static string GetMsalAccountId(this ClaimsPrincipal claimsPrincipal)
         }
 
         /// <summary>
-        /// Get the unique object ID associated with the claimsPrincipal
+        /// Gets the unique object ID associated with the <see cref="ClaimsPrincipal"/>
         /// </summary>
-        /// <param name="claimsPrincipal">Claims principal from which to retrieve the unique object id</param>
+        /// <param name="claimsPrincipal">the <see cref="ClaimsPrincipal"/> from which to retrieve the unique object id</param>
         /// <returns>Unique object ID of the identity, or <c>null</c> if it cannot be found</returns>
         public static string GetObjectId(this ClaimsPrincipal claimsPrincipal)
-        {
-            string userObjectId = claimsPrincipal.FindFirstValue(ClaimConstants.ObjectId);
+        {
+            string userObjectId = claimsPrincipal.FindFirstValue(ClaimConstants.Oid);
             if (string.IsNullOrEmpty(userObjectId))
-            {
-                userObjectId = claimsPrincipal.FindFirstValue("oid");
-            }
+                userObjectId = claimsPrincipal.FindFirstValue(ClaimConstants.ObjectId);
 
             return userObjectId;
         }
 
         /// <summary>
-        /// Tenant ID of the identity
+        /// Gets the Tenant ID associated with the <see cref="ClaimsPrincipal"/>
         /// </summary>
-        /// <param name="claimsPrincipal">Claims principal from which to retrieve the tenant id</param>
+        /// <param name="claimsPrincipal">the <see cref="ClaimsPrincipal"/> from which to retrieve the tenant id</param>
         /// <returns>Tenant ID of the identity, or <c>null</c> if it cannot be found</returns>
         public static string GetTenantId(this ClaimsPrincipal claimsPrincipal)
         {
-            string tenantId = claimsPrincipal.FindFirstValue(ClaimConstants.TenantId);
+            string tenantId = claimsPrincipal.FindFirstValue(ClaimConstants.Tid);
             if (string.IsNullOrEmpty(tenantId))
-            {
-                tenantId = claimsPrincipal.FindFirstValue("tid");
-            }
+                tenantId = claimsPrincipal.FindFirstValue(ClaimConstants.TenantId);
 
             return tenantId;
         }
 
         /// <summary>
-        /// Gets the login-hint associated with an identity
+        /// Gets the login-hint associated with a <see cref="ClaimsPrincipal"/>
         /// </summary>
-        /// <param name="claimsPrincipal">Identity for which to compte the login-hint</param>
+        /// <param name="claimsPrincipal">Identity for which to complete the login-hint</param>
         /// <returns>login-hint for the identity, or <c>null</c> if it cannot be found</returns>
         public static string GetLoginHint(this ClaimsPrincipal claimsPrincipal)
         {
@@ -76,13 +95,11 @@ public static string GetDomainHint(this ClaimsPrincipal claimsPrincipal)
             const string msaTenantId = "9188040d-6c67-4c5b-b112-36a304b66dad";
 
             var tenantId = GetTenantId(claimsPrincipal);
-            string domainHint = string.IsNullOrWhiteSpace(tenantId) ? null :
-                tenantId == msaTenantId ? "consumers" : "organizations";
-            return domainHint;
+            return string.IsNullOrWhiteSpace(tenantId) ? null : tenantId == msaTenantId ? "consumers" : "organizations";
         }
 
         /// <summary>
-        /// Get the display name for the signed-in user, based on their claims principal
+        /// Get the display name for the signed-in user, from the <see cref="ClaimsPrincipal"/>
         /// </summary>
         /// <param name="claimsPrincipal">Claims about the user/account</param>
         /// <returns>A string containing the display name for the user, as brought by Azure AD v1.0 and v2.0 tokens,
@@ -91,7 +108,7 @@ public static string GetDomainHint(this ClaimsPrincipal claimsPrincipal)
         public static string GetDisplayName(this ClaimsPrincipal claimsPrincipal)
         {
             // Attempting the claims brought by an Azure AD v2.0 token first
-            string displayName = claimsPrincipal.FindFirstValue("preferred_username");
+            string displayName = claimsPrincipal.FindFirstValue(ClaimConstants.PreferredUserName);
 
             // Otherwise falling back to the claims brought by an Azure AD v1.0 token
             if (string.IsNullOrWhiteSpace(displayName))
@@ -102,8 +119,9 @@ public static string GetDisplayName(this ClaimsPrincipal claimsPrincipal)
             // Finally falling back to name
             if (string.IsNullOrWhiteSpace(displayName))
             {
-                displayName = claimsPrincipal.FindFirstValue("name");
+                displayName = claimsPrincipal.FindFirstValue(ClaimConstants.Name);
             }
+
             return displayName;
         }
 
@@ -131,32 +149,35 @@ public static string GetDisplayName(this ClaimsPrincipal claimsPrincipal)
         /// </example>
         public static ClaimsPrincipal FromTenantIdAndObjectId(string tenantId, string objectId)
         {
-            var tidClaim = new Claim("tid", tenantId);
-            var oidClaim = new Claim("oid", objectId);
-            var claimsIdentity = new ClaimsIdentity();
-            claimsIdentity.AddClaims(new Claim[] { oidClaim, tidClaim });
-            var principal = new ClaimsPrincipal();
-            principal.AddIdentity(claimsIdentity);
-            return principal;
+            return new ClaimsPrincipal(
+                new ClaimsIdentity(new Claim[]
+                {
+                    new Claim(ClaimConstants.Tid, tenantId),
+                    new Claim(ClaimConstants.Oid, objectId)
+                })
+            );
         }
 
         /// <summary>
-        /// Builds a ClaimsPrincipal from an IAccount
+        /// Creates the <see cref="ClaimsPrincipal"/> from the values found in an <see cref="IAccount"/>
         /// </summary>
-        /// <param name="account">The IAccount instance.</param>
-        /// <returns>A ClaimsPrincipal built from IAccount</returns>
+        /// <param name="account">The IAccount instance</param>
+        /// <returns>A <see cref="ClaimsPrincipal"/> built from IAccount</returns>
         public static ClaimsPrincipal ToClaimsPrincipal(this IAccount account)
         {
             if (account != null)
             {
-                var identity = new ClaimsIdentity();
-                identity.AddClaim(new Claim(ClaimConstants.ObjectId, account.HomeAccountId.ObjectId));
-                identity.AddClaim(new Claim(ClaimConstants.TenantId, account.HomeAccountId.TenantId));
-                identity.AddClaim(new Claim(ClaimTypes.Upn, account.Username));
-                return new ClaimsPrincipal(identity);
+                return new ClaimsPrincipal(
+                    new ClaimsIdentity(new Claim[]
+                    {
+                        new Claim(ClaimConstants.Oid, account.HomeAccountId.ObjectId),
+                        new Claim(ClaimConstants.Tid, account.HomeAccountId.TenantId),
+                        new Claim(ClaimTypes.Upn, account.Username)
+                    })
+                );
             }
 
             return null;
         }
     }
-}
+}

Microsoft.Identity.Web/Client/TokenAcquisition.cs

@@ -64,6 +64,9 @@ public class TokenAcquisition : ITokenAcquisition
         /// <param name="configuration"></param>
         public TokenAcquisition(IConfiguration configuration, IMSALAppTokenCacheProvider appTokenCacheProvider, IMSALUserTokenCacheProvider userTokenCacheProvider)
         {
+            if (configuration == null)
+                throw new ArgumentNullException(nameof(configuration));
+
             azureAdOptions = new AzureADOptions();
             configuration.Bind("AzureAD", azureAdOptions);
 
@@ -136,6 +139,7 @@ public async Task AddAccountToCacheFromAuthorizationCode(AuthorizationCodeReceiv
             }
             catch (MsalException ex)
             {
+                // brentsch - todo, write to a log
                 Debug.WriteLine(ex.Message);
                 throw;
             }

Microsoft.Identity.Web/Client/TokenCacheProviders/Session/MSALAppSessionTokenCacheProviderExtension.cs

@@ -48,9 +48,7 @@ public static IServiceCollection AddSessionAppTokenCache(this IServiceCollection
         {
             services.AddScoped<IMSALAppTokenCacheProvider>(factory =>
             {
-                var optionsMonitor = factory.GetRequiredService<IOptionsMonitor<AzureADOptions>>();
-
-                return new MSALAppSessionTokenCacheProvider(optionsMonitor);
+                return new MSALAppSessionTokenCacheProvider(factory.GetRequiredService<IOptionsMonitor<AzureADOptions>>());
             });
 
             return services;