Minor feedback items

dotnet · Aug 21, 2020 · e38ee28 · e38ee28
1 parent ee2c883
commit e38ee28
Show file tree

Hide file tree

Showing 4 changed files with 7 additions and 9 deletions.
diff --git a/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs b/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs
@@ -12,14 +12,14 @@ namespace Microsoft.AspNetCore.Authentication.Negotiate
 {
     internal static class LdapAdapter
     {
-        public static async Task RetrieveClaimsAsync(LdapOptions options, AuthenticatedContext context, ILogger logger)
+        public static async Task RetrieveClaimsAsync(LdapOptions options, ClaimsIdentity identity, ILogger logger)
         {
             if (!options.EnableLdapRoleClaimResolution)
             {
                 return;
             }
 
-            var user = context.Principal.Identity.Name;
+            var user = identity.Name;
             var userAccountName = user.Substring(0, user.IndexOf('@'));
             var distinguishedName = options.Domain.Split('.').Select(name => $"dc={name}").Aggregate((a, b) => $"{a},{b}");
 
@@ -43,8 +43,6 @@ public static async Task RetrieveClaimsAsync(LdapOptions options, AuthenticatedC
                 var userFound = searchResponse.Entries[0]; //Get the object that was found on ldap
                 var memberof = userFound.Attributes["memberof"]; // You can access ldap Attributes with Attributes property
 
-                var claimsIdentity = context.Principal.Identity as ClaimsIdentity;
-
                 foreach (var group in memberof)
                 {
                     // Example distinguished name: CN=TestGroup,DC=KERB,DC=local
@@ -53,11 +51,11 @@ public static async Task RetrieveClaimsAsync(LdapOptions options, AuthenticatedC
 
                     if (options.ResolveNestedGroups)
                     {
-                        GetNestedGroups(options.LdapConnection, claimsIdentity, distinguishedName, groupCN, logger);
+                        GetNestedGroups(options.LdapConnection, identity, distinguishedName, groupCN, logger);
                     }
                     else
                     {
-                        AddRole(claimsIdentity, groupCN);
+                        AddRole(identity, groupCN);
                     }
                 }
             }

diff --git a/src/Security/Authentication/Negotiate/src/LdapOptions.cs b/src/Security/Authentication/Negotiate/src/LdapOptions.cs
@@ -40,7 +40,7 @@ public class LdapOptions
 
         /// <summary>
         /// This option indicates whether nested groups should be examined when
-        /// resolving AD Roles. 
+        /// resolving Roles. The default is true.
         /// </summary>
         public bool ResolveNestedGroups { get; set; } = true;
 

diff --git a/src/Security/Authentication/Negotiate/src/NegotiateHandler.cs b/src/Security/Authentication/Negotiate/src/NegotiateHandler.cs
@@ -331,7 +331,7 @@ protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
             };
 
             // TODO: persist results
-            await LdapAdapter.RetrieveClaimsAsync(Options.LdapOptions, authenticatedContext, Logger);
+            await LdapAdapter.RetrieveClaimsAsync(Options.LdapOptions, authenticatedContext.Principal.Identity as ClaimsIdentity, Logger);
 
             await Events.Authenticated(authenticatedContext);
 

diff --git a/src/Security/Authentication/Negotiate/src/NegotiateOptions.cs b/src/Security/Authentication/Negotiate/src/NegotiateOptions.cs
@@ -34,7 +34,7 @@ public class NegotiateOptions : AuthenticationSchemeOptions
         public bool PersistNtlmCredentials { get; set; } = true;
 
         /// <summary>
-        /// Configuration settings for LDAP connections used to retrieve AD Role claims.
+        /// Configuration settings for LDAP connections used to retrieve Role claims.
         /// This is only used on Linux systems.
         /// </summary>
         public LdapOptions LdapOptions { get; } = new LdapOptions();